diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 3b3efec16..15ca01ebe 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -3455,17 +3455,6 @@ async fn reject_if_archived(state: &AppState, run_id: &RunId) -> Option Result { - match state.store.get_cached_summary(run_id).await { - Ok(Some(summary)) => Ok(summary.title), - Ok(None) => Err(ApiError::not_found("Run not found.")), - Err(err) => Err(ApiError::new( - StatusCode::INTERNAL_SERVER_ERROR, - err.to_string(), - )), - } -} - #[cfg(test)] #[expect( clippy::disallowed_methods, diff --git a/lib/crates/fabro-server/src/server/handler/lifecycle.rs b/lib/crates/fabro-server/src/server/handler/lifecycle.rs index 36696f3b0..07a13cc0e 100644 --- a/lib/crates/fabro-server/src/server/handler/lifecycle.rs +++ b/lib/crates/fabro-server/src/server/handler/lifecycle.rs @@ -5,9 +5,9 @@ use super::super::{ Principal, RequiredUser, Response, RewindRequest, RewindResponse, Router, RunAnswerTransport, RunControlAction, RunExecutionMode, RunId, RunStatus, RunStatusResponse, StartRunRequest, State, StatusCode, Storage, TimelineEntryResponse, WORKER_CANCEL_GRACE, WorkflowError, - append_control_request, get, load_pending_control, load_run_title, managed_run, operations, - parse_run_id_path, persist_cancelled_run_status, post, reject_if_archived, sleep, - update_live_run_from_event, workflow_event, + append_control_request, get, load_pending_control, managed_run, operations, parse_run_id_path, + persist_cancelled_run_status, post, reject_if_archived, sleep, update_live_run_from_event, + workflow_event, }; pub(super) fn routes() -> Router> { @@ -101,6 +101,7 @@ async fn start_run( ) .into_response(); } + let title = run_state.title().into_owned(); let run_dir = Storage::new(state.server_storage_dir()) .run_scratch(&id) .root() @@ -130,10 +131,6 @@ async fn start_run( ); } - let title = match load_run_title(state.as_ref(), &id).await { - Ok(title) => title, - Err(err) => return err.into_response(), - }; let web_url = state.run_web_url(&id); state.scheduler_notify.notify_one(); ( @@ -287,9 +284,13 @@ async fn cancel_run( .into_response(); } }; - let title = match load_run_title(state.as_ref(), &id).await { - Ok(title) => title, - Err(err) => return err.into_response(), + let title = match state.store.get_cached_run(&id).await { + Ok(Some(cached)) => cached.projection.title().into_owned(), + Ok(None) => return ApiError::not_found("Run not found.").into_response(), + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } }; let web_url = state.run_web_url(&id); @@ -416,9 +417,13 @@ async fn pause_run( .into_response(); } }; - let title = match load_run_title(state.as_ref(), &id).await { - Ok(title) => title, - Err(err) => return err.into_response(), + let title = match state.store.get_cached_run(&id).await { + Ok(Some(cached)) => cached.projection.title().into_owned(), + Ok(None) => return ApiError::not_found("Run not found.").into_response(), + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } }; let web_url = state.run_web_url(&id); @@ -528,9 +533,13 @@ async fn unpause_run( .into_response(); } }; - let title = match load_run_title(state.as_ref(), &id).await { - Ok(title) => title, - Err(err) => return err.into_response(), + let title = match state.store.get_cached_run(&id).await { + Ok(Some(cached)) => cached.projection.title().into_owned(), + Ok(None) => return ApiError::not_found("Run not found.").into_response(), + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } }; let web_url = state.run_web_url(&id); @@ -780,14 +789,7 @@ async fn archive_status_response(state: &AppState, id: RunId) -> Response { ) .into_response(); }; - let title = if projection.title.is_empty() { - projection.spec.as_ref().map_or_else( - || fabro_types::infer_run_title(""), - |spec| fabro_types::infer_run_title(spec.graph.goal()), - ) - } else { - projection.title - }; + let title = projection.title().into_owned(); let web_url = state.run_web_url(&id); ( StatusCode::OK, diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 3be6bce4a..56394c3f8 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -30,7 +30,7 @@ use tracing::info; use super::super::{ AppState, ListResponse, MAX_PAGE_OFFSET, PaginationParams, RunExecutionMode, answer_from_request, api_question_from_pending_interview, default_page_limit, - delete_run_internal, load_pending_interview, load_run_title, managed_run, parse_run_id_path, + delete_run_internal, load_pending_interview, managed_run, parse_run_id_path, reject_if_archived, resolve_interp_string, submit_pending_interview_answer, workflow_event, }; use crate::error::ApiError; @@ -520,9 +520,13 @@ async fn create_run( } }; let created_at = created.run_id.created_at(); - let title = match load_run_title(state.as_ref(), &created.run_id).await { - Ok(title) => title, - Err(err) => return err.into_response(), + let title = match state.store.get_cached_run(&created.run_id).await { + Ok(Some(cached)) => cached.projection.title().into_owned(), + Ok(None) => return ApiError::not_found("Run not found.").into_response(), + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } }; { diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 1baa7ada6..9ef5dc6a0 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -520,11 +520,6 @@ pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> RunSummary .as_ref() .map(|spec| spec.graph.goal().to_string()) .unwrap_or_default(); - let title = if state.title.is_empty() { - fabro_types::infer_run_title(&goal) - } else { - state.title.clone() - }; RunSummary::new( *run_id, workflow_name, @@ -533,7 +528,7 @@ pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> RunSummary .as_ref() .and_then(|spec| spec.workflow_slug.clone()), goal, - title, + state.title().into_owned(), state .spec .as_ref() diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs index a7794d893..bfa51c5b2 100644 --- a/lib/crates/fabro-types/src/run_projection.rs +++ b/lib/crates/fabro-types/src/run_projection.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; use std::num::NonZeroU32; @@ -153,6 +154,17 @@ impl StageProjection { } impl RunProjection { + #[must_use] + pub fn title(&self) -> Cow<'_, str> { + if !self.title.trim().is_empty() { + return Cow::Borrowed(&self.title); + } + + Cow::Owned(crate::infer_run_title( + self.spec.as_ref().map_or("", |spec| spec.graph.goal()), + )) + } + pub fn stage(&self, stage: &StageId) -> Option<&StageProjection> { self.stages.get(stage) } @@ -265,6 +277,71 @@ impl RunProjection { } } +#[cfg(test)] +mod title_tests { + use std::collections::HashMap; + + use crate::{AttrValue, Graph, RunId, RunProjection, RunSpec, WorkflowSettings}; + + fn projection_with_goal(goal: Option<&str>) -> RunProjection { + let mut graph = Graph::new("test"); + if let Some(goal) = goal { + graph + .attrs + .insert("goal".to_string(), AttrValue::String(goal.to_string())); + } + + RunProjection { + spec: Some(RunSpec { + run_id: RunId::new(), + settings: WorkflowSettings::default(), + graph, + workflow_slug: None, + source_directory: None, + labels: HashMap::new(), + provenance: None, + manifest_blob: None, + definition_blob: None, + git: None, + fork_source_ref: None, + in_place: false, + }), + ..RunProjection::default() + } + } + + #[test] + fn run_title_returns_stored_title_when_present() { + let projection = RunProjection { + title: "Stored title".to_string(), + ..RunProjection::default() + }; + + assert_eq!(projection.title(), "Stored title"); + } + + #[test] + fn run_title_infers_from_goal_when_stored_title_is_empty() { + let projection = projection_with_goal(Some("## Plan: Legacy title\n\nDetails")); + + assert_eq!(projection.title(), "Legacy title"); + } + + #[test] + fn run_title_falls_back_when_stored_title_and_goal_are_blank() { + let projection = projection_with_goal(Some(" \nmore detail")); + + assert_eq!(projection.title(), "Untitled run"); + } + + #[test] + fn run_title_falls_back_when_spec_is_unavailable() { + let projection = RunProjection::default(); + + assert_eq!(projection.title(), "Untitled run"); + } +} + #[cfg(test)] mod iter_stages_tests { use std::num::NonZeroU32;