mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-10 22:43:37 +00:00
refactor(runs): resolve titles from run projection
Centralize legacy title fallback behind RunProjection::title() so summaries and lifecycle responses use the same resolved title behavior.
This commit is contained in:
parent
697dc1294f
commit
ba790ea1e8
5 changed files with 112 additions and 45 deletions
|
|
@ -3455,17 +3455,6 @@ async fn reject_if_archived(state: &AppState, run_id: &RunId) -> Option<Response
|
|||
})
|
||||
}
|
||||
|
||||
async fn load_run_title(state: &AppState, run_id: &RunId) -> Result<String, ApiError> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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<Arc<AppState>> {
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
};
|
||||
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue