From 23cb211cce777eb1d8cd5302dc62a70ffc97b3cf Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 17:34:32 -0700 Subject: [PATCH] feat(runs): surface diff summary counts Compute cheap diff stats on checkpoint and terminal events, roll them into run summaries, and use them for the Files Changed tab badge without fetching full file diffs. --- apps/fabro-web/app/routes/run-detail.test.ts | 35 ++++- apps/fabro-web/app/routes/run-detail.tsx | 7 +- docs/public/api-reference/fabro-api.yaml | 29 ++++ lib/crates/fabro-api/build.rs | 1 + lib/crates/fabro-api/src/lib.rs | 2 +- .../tests/diff_summary_round_trip.rs | 37 +++++ .../fabro-api/tests/run_summary_round_trip.rs | 15 +- .../fabro-cli/src/commands/run/attach.rs | 4 +- .../fabro-cli/src/commands/run/runner.rs | 3 + lib/crates/fabro-server/src/demo/mod.rs | 1 + lib/crates/fabro-server/src/server.rs | 10 ++ lib/crates/fabro-server/src/server/tests.rs | 9 ++ .../fabro-server/tests/it/api/run_files.rs | 1 + lib/crates/fabro-store/src/run_state.rs | 113 ++++++++++++++ lib/crates/fabro-types/src/diff.rs | 7 + lib/crates/fabro-types/src/event_envelope.rs | 2 + lib/crates/fabro-types/src/lib.rs | 2 +- lib/crates/fabro-types/src/run_event/mod.rs | 68 +++++++++ lib/crates/fabro-types/src/run_event/run.rs | 8 +- lib/crates/fabro-types/src/run_event/stage.rs | 4 +- lib/crates/fabro-types/src/run_projection.rs | 8 +- lib/crates/fabro-types/src/run_summary.rs | 39 ++++- .../fabro-workflow/src/event/convert.rs | 8 + lib/crates/fabro-workflow/src/event/events.rs | 12 +- lib/crates/fabro-workflow/src/git.rs | 1 + .../fabro-workflow/src/lifecycle/event.rs | 2 + .../fabro-workflow/src/lifecycle/git.rs | 143 +++++++++++++++++- .../fabro-workflow/src/operations/archive.rs | 2 + .../fabro-workflow/src/operations/fork.rs | 2 + .../fabro-workflow/src/operations/start.rs | 8 + .../fabro-workflow/src/pipeline/finalize.rs | 136 ++++++++++++++++- .../src/pipeline/pull_request.rs | 2 + .../fabro-workflow/src/pipeline/retro.rs | 1 + lib/crates/fabro-workflow/src/sandbox_git.rs | 23 ++- lib/crates/fabro-workflow/src/test_support.rs | 1 + .../src/.openapi-generator/FILES | 1 + .../src/models/diff-summary.ts | 33 ++++ .../fabro-api-client/src/models/index.ts | 1 + .../src/models/run-projection.ts | 4 + .../src/models/run-summary.ts | 4 + 40 files changed, 759 insertions(+), 30 deletions(-) create mode 100644 lib/crates/fabro-api/tests/diff_summary_round_trip.rs create mode 100644 lib/packages/fabro-api-client/src/models/diff-summary.ts diff --git a/apps/fabro-web/app/routes/run-detail.test.ts b/apps/fabro-web/app/routes/run-detail.test.ts index 564391980..4cffe15fa 100644 --- a/apps/fabro-web/app/routes/run-detail.test.ts +++ b/apps/fabro-web/app/routes/run-detail.test.ts @@ -69,7 +69,7 @@ type RunDetailActionResult = import("./run-detail").RunDetailActionResult; const h = createElement; -function makeRunSummary(status = "succeeded") { +function makeRunSummary(status = "succeeded", diffSummary: any = null) { return { run_id: "run_1", title: "Run 1", @@ -80,6 +80,7 @@ function makeRunSummary(status = "succeeded") { duration_ms: null, elapsed_secs: null, source_directory: null, + diff_summary: diffSummary, }; } @@ -105,12 +106,14 @@ async function renderRunDetail({ initialEntry, status = "succeeded", questions = [], + diffSummary = null, }: { initialEntry: string; status?: string; questions?: any[]; + diffSummary?: any; }) { - currentRunSummary = makeRunSummary(status); + currentRunSummary = makeRunSummary(status, diffSummary); currentQuestions = questions; (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -154,6 +157,12 @@ function hasClasses(value: unknown, classes: string[]) { return classes.every((className) => tokens.includes(className)); } +function tabCountBadges(renderer: TestRenderer.ReactTestRenderer) { + return renderer.root.findAll( + (node) => node.type === "span" && hasClasses(node.props.className, ["tabular-nums"]), + ); +} + describe("lifecycleActionVisibility", () => { test("shows cancel for active cancellable states and hides it elsewhere", () => { expect(lifecycleActionVisibility("submitted").showPrimaryCancel).toBe(true); @@ -330,6 +339,28 @@ describe("RunDetail full-height child routes", () => { expect(outletWrappers).toHaveLength(1); }); + test("shows the Files Changed tab badge from run summary diff stats", async () => { + const renderer = await renderRunDetail({ + initialEntry: "/runs/run_1/files", + diffSummary: { + files_changed: 7, + additions: 30, + deletions: 11, + }, + }); + + const badges = tabCountBadges(renderer); + expect(badges.map((badge) => badge.children.join(""))).toContain("7"); + }); + + test("hides the Files Changed tab badge when diff stats are absent", async () => { + const renderer = await renderRunDetail({ + initialEntry: "/runs/run_1/files", + }); + + expect(tabCountBadges(renderer)).toHaveLength(0); + }); + test("keeps blocked full-height children clear of the interview dock without an h-72 sibling", async () => { const renderer = await renderRunDetail({ initialEntry: "/runs/run_1/files", diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index e1162e641..041c3ae72 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -149,7 +149,12 @@ export default function RunDetail({ params }: { params: { id: string } }) { const unarchiveMutation = useUnarchiveRun(params.id); const interruptMutation = useInterruptRun(params.id); const { push, dismiss } = useToast(); - const tabs = allTabs.filter((t) => !t.demoOnly || demoMode); + const filesCount = runQuery.data?.diff_summary?.files_changed ?? null; + const tabs = allTabs + .map((tab) => + tab.name === "Files Changed" ? { ...tab, count: filesCount } : tab, + ) + .filter((t) => !t.demoOnly || demoMode); const lifecycleToastStateRef = useRef(INITIAL_LIFECYCLE_TOAST_STATE); const steerBarRef = useRef(null); const [steerOpen, setSteerOpen] = useState(false); diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 08991e412..aa0965337 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -5652,6 +5652,10 @@ components: additionalProperties: true final_patch: type: ["string", "null"] + diff_summary: + oneOf: + - $ref: "#/components/schemas/DiffSummary" + - type: "null" pull_request: type: ["object", "null"] additionalProperties: true @@ -5727,6 +5731,10 @@ components: format: int64 superseded_by: type: ["string", "null"] + diff_summary: + oneOf: + - $ref: "#/components/schemas/DiffSummary" + - type: "null" ForkRequest: description: Request body for creating a new run from a source run checkpoint. @@ -6615,6 +6623,27 @@ components: description: Total lines deleted. example: 234 + DiffSummary: + description: Cheap aggregate file and line counts for a run diff. + type: object + required: + - files_changed + - additions + - deletions + properties: + files_changed: + type: integer + description: Total number of changed files, including binary files. + example: 42 + additions: + type: integer + description: Total lines added across text files. + example: 567 + deletions: + type: integer + description: Total lines deleted across text files. + example: 234 + RunFilesMeta: description: | Metadata for a `PaginatedRunFileList` response. diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index b63e23029..50b868312 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -179,6 +179,7 @@ fn main() { &[], ), ("RunSummary", "fabro_types::RunSummary", &[]), + ("DiffSummary", "fabro_types::DiffSummary", &[]), ( "RepositoryReference", "fabro_types::RepositoryReference", diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index 37a92ec8c..41433565c 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -30,7 +30,7 @@ pub mod types { }; pub use fabro_types::{ AuthMethod, BilledTokenCounts, CommandOutputStream, CommandTermination, DiffStats, - DirtyStatus, EventEnvelope, GitContext, IdpIdentity, InterviewOption, + DiffSummary, DirtyStatus, EventEnvelope, GitContext, IdpIdentity, InterviewOption, InterviewQuestionRecord, PendingInterviewRecord, PreRunPushOutcome, Principal, QuestionType, RepositoryReference, RunClientProvenance, RunEvent, RunProjection, RunProvenance, RunServerProvenance, RunSummary, SecretMetadata, SecretType, ServerSettings, diff --git a/lib/crates/fabro-api/tests/diff_summary_round_trip.rs b/lib/crates/fabro-api/tests/diff_summary_round_trip.rs new file mode 100644 index 000000000..09f79bb34 --- /dev/null +++ b/lib/crates/fabro-api/tests/diff_summary_round_trip.rs @@ -0,0 +1,37 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::DiffSummary as ApiDiffSummary; +use fabro_types::DiffSummary; +use serde_json::json; + +#[test] +fn diff_summary_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn diff_summary_serializes_with_required_integer_fields() { + let summary = DiffSummary { + files_changed: 3, + additions: 12, + deletions: 4, + }; + assert_eq!( + serde_json::to_value(summary).unwrap(), + json!({ + "files_changed": 3, + "additions": 12, + "deletions": 4, + }) + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/run_summary_round_trip.rs b/lib/crates/fabro-api/tests/run_summary_round_trip.rs index 8aef77553..69b32fb66 100644 --- a/lib/crates/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/crates/fabro-api/tests/run_summary_round_trip.rs @@ -6,7 +6,7 @@ use fabro_api::types::{ RepositoryReference as ApiRepositoryReference, RunSummary as ApiRunSummary, }; use fabro_types::status::{RunStatus, SuccessReason, TerminalStatus}; -use fabro_types::{RepositoryReference, RunId, RunSummary}; +use fabro_types::{DiffSummary, RepositoryReference, RunId, RunSummary}; use serde_json::json; #[test] @@ -41,6 +41,11 @@ fn run_summary_json_matches_openapi_shape() { Some(42_000), Some(123), Some(superseded_by), + Some(DiffSummary { + files_changed: 3, + additions: 12, + deletions: 4, + }), ); assert_eq!( @@ -74,7 +79,12 @@ fn run_summary_json_matches_openapi_shape() { "duration_ms": 42000, "elapsed_secs": 42.0, "total_usd_micros": 123, - "superseded_by": superseded_by.to_string() + "superseded_by": superseded_by.to_string(), + "diff_summary": { + "files_changed": 3, + "additions": 12, + "deletions": 4 + } }) ); } @@ -117,6 +127,7 @@ fn run_summary_deserializes_when_optional_fields_are_absent() { assert_eq!(summary.elapsed_secs, None); assert_eq!(summary.total_usd_micros, None); assert_eq!(summary.superseded_by, None); + assert_eq!(summary.diff_summary, None); } fn assert_same_type() { diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index fe97b1d5e..5684932b1 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -106,7 +106,7 @@ pub(crate) async fn attach_run_with_client( } let stream = client.attach_run_events(run_id, Some(next_seq)).await?; - attach_live_run_with_client( + Box::pin(attach_live_run_with_client( client, run_id, replay_events, @@ -119,7 +119,7 @@ pub(crate) async fn attach_run_with_client( json_output, }, printer, - ) + )) .await } diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index f7c9e59e3..607468fb4 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -773,6 +773,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, })), Some(WorkerTitlePhase::Succeeded) @@ -785,6 +786,7 @@ mod tests { reason: FailureReason::Cancelled, git_commit_sha: None, final_patch: None, + diff_summary: None, })), Some(WorkerTitlePhase::Cancelled) ); @@ -796,6 +798,7 @@ mod tests { reason: FailureReason::Terminated, git_commit_sha: None, final_patch: None, + diff_summary: None, })), Some(WorkerTitlePhase::Failed) ); diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 836051585..474a08f43 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -889,6 +889,7 @@ mod runs { elapsed_secs.and_then(duration_ms_from_secs), total_usd_micros, None, + None, ) } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index f08a63c7c..ce4fca90f 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1983,6 +1983,7 @@ pub(crate) async fn reconcile_incomplete_runs_on_startup( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await?; @@ -2036,6 +2037,7 @@ async fn persist_shutdown_run_failures( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await?; @@ -2110,6 +2112,7 @@ async fn persist_cancelled_run_status(state: &AppState, run_id: RunId) -> anyhow reason: FailureReason::Cancelled, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -2148,6 +2151,7 @@ async fn fail_run_before_execution( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -2425,6 +2429,7 @@ async fn append_worker_exit_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -3085,6 +3090,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3112,6 +3118,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3142,6 +3149,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3163,6 +3171,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3196,6 +3205,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::Terminated, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index de9c20afe..8e1e8d608 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -2668,6 +2668,7 @@ async fn run_billing_dedups_retried_nodes_and_sums_their_durations() { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("verify".to_string(), 2usize)]), diff: None, + diff_summary: None, }, ) .await @@ -2796,6 +2797,7 @@ async fn run_billing_sums_usage_across_retry_visits_and_uses_latest_model() { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("verify".to_string(), 2usize)]), diff: None, + diff_summary: None, }, ) .await @@ -3407,6 +3409,7 @@ async fn create_completed_run_ready_for_pull_request( total_usd_micros: None, final_git_commit_sha: None, final_patch: Some(final_patch.to_string()), + diff_summary: None, billing: None, }, ]) @@ -6964,6 +6967,7 @@ async fn archive_and_unarchive_updates_listing_visibility() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8304,6 +8308,7 @@ async fn boards_runs_excludes_archived_by_default() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, workflow_event::Event::RunArchived { actor: None }, @@ -8352,6 +8357,7 @@ async fn boards_runs_includes_archived_when_flag_set() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, workflow_event::Event::RunArchived { actor: None }, @@ -8371,6 +8377,7 @@ async fn boards_runs_includes_archived_when_flag_set() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8440,6 +8447,7 @@ async fn get_run_exposes_canonical_operator_statuses() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8521,6 +8529,7 @@ async fn boards_runs_maps_statuses_to_columns() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) diff --git a/lib/crates/fabro-server/tests/it/api/run_files.rs b/lib/crates/fabro-server/tests/it/api/run_files.rs index 079544c3f..fb6d469a1 100644 --- a/lib/crates/fabro-server/tests/it/api/run_files.rs +++ b/lib/crates/fabro-server/tests/it/api/run_files.rs @@ -73,6 +73,7 @@ async fn append_completed_run_with_final_patch( total_usd_micros: None, final_git_commit_sha: Some("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string()), final_patch: Some(final_patch.to_string()), + diff_summary: None, billing: None, }, ) diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 6476483ab..8b38ea485 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -155,6 +155,7 @@ impl RunProjectionReducer for RunProjection { self.pending_control = None; self.conclusion = Some(conclusion_from_completed(props, ts)?); self.final_patch.clone_from(&props.final_patch); + self.diff_summary = props.diff_summary.or(self.diff_summary); self.pending_interviews.clear(); } EventBody::RunFailed(props) => { @@ -167,6 +168,7 @@ impl RunProjectionReducer for RunProjection { self.pending_control = None; self.conclusion = Some(conclusion_from_failed(props, ts)); self.final_patch.clone_from(&props.final_patch); + self.diff_summary = props.diff_summary.or(self.diff_summary); self.pending_interviews.clear(); } EventBody::RunSupersededBy(props) => { @@ -196,6 +198,7 @@ impl RunProjectionReducer for RunProjection { } EventBody::CheckpointCompleted(props) => { let checkpoint = checkpoint_from_props(props, ts); + self.diff_summary = props.diff_summary.or(self.diff_summary); if let Some(node_id) = stored.node_id.as_deref() { let visit = checkpoint .node_visits @@ -562,6 +565,7 @@ pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> RunSummary .and_then(|conclusion| conclusion.billing.as_ref()) .and_then(|billing| billing.total_usd_micros), state.superseded_by, + state.diff_summary, ) } @@ -1416,6 +1420,7 @@ mod tests { restart_failure_signatures: BTreeMap::new(), node_visits: BTreeMap::from([("skip_me".to_string(), 1usize)]), diff: None, + diff_summary: None, }), None, )) @@ -1750,6 +1755,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: Some("abc123".to_string()), final_patch: Some(patch.to_string()), + diff_summary: None, }), None, )) @@ -1758,6 +1764,109 @@ mod tests { assert_eq!(state.final_patch.as_deref(), Some(patch)); } + #[test] + fn patch_bearing_events_roll_up_diff_summary_without_blanking_prior_value() { + let mut state = RunProjection::default(); + + state + .apply_event(&test_raw_event( + 1, + "checkpoint.completed", + &json!({ + "status": "running", + "current_node": "build", + "completed_nodes": ["build"], + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + Some("build"), + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 2, + "additions": 10, + "deletions": 3 + }) + ); + + state + .apply_event(&test_raw_event( + 2, + "checkpoint.completed", + &json!({ + "status": "running", + "current_node": "review", + "completed_nodes": ["build", "review"] + }), + Some("review"), + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"] + ["files_changed"], + 2 + ); + + state + .apply_event(&test_raw_event( + 3, + "run.completed", + &json!({ + "duration_ms": 42, + "artifact_count": 0, + "status": "succeeded", + "reason": "completed", + "diff_summary": { + "files_changed": 4, + "additions": 18, + "deletions": 7 + } + }), + None, + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 4, + "additions": 18, + "deletions": 7 + }) + ); + + let mut failed_state = RunProjection::default(); + failed_state + .apply_event(&test_raw_event( + 1, + "run.failed", + &json!({ + "error": "boom", + "duration_ms": 42, + "reason": "workflow_error", + "diff_summary": { + "files_changed": 5, + "additions": 20, + "deletions": 8 + } + }), + None, + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&failed_state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 5, + "additions": 20, + "deletions": 8 + }) + ); + } + #[test] fn run_failed_projection_renders_causes() { let mut state = RunProjection::default(); @@ -1774,6 +1883,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }), None, )) @@ -1803,6 +1913,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, @@ -1866,6 +1977,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, @@ -1996,6 +2108,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, diff --git a/lib/crates/fabro-types/src/diff.rs b/lib/crates/fabro-types/src/diff.rs index 201c26515..0f893973e 100644 --- a/lib/crates/fabro-types/src/diff.rs +++ b/lib/crates/fabro-types/src/diff.rs @@ -5,3 +5,10 @@ pub struct DiffStats { pub additions: i64, pub deletions: i64, } + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct DiffSummary { + pub files_changed: i64, + pub additions: i64, + pub deletions: i64, +} diff --git a/lib/crates/fabro-types/src/event_envelope.rs b/lib/crates/fabro-types/src/event_envelope.rs index cac7dc13f..b36b176cd 100644 --- a/lib/crates/fabro-types/src/event_envelope.rs +++ b/lib/crates/fabro-types/src/event_envelope.rs @@ -42,6 +42,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), }; @@ -85,6 +86,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), }; diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 7b1073bca..b3bc4715f 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -45,7 +45,7 @@ pub use checkpoint::Checkpoint; pub use command_output::{CommandOutputStream, CommandTermination}; pub use conclusion::{Conclusion, StageSummary}; pub use dense::{ServerSettings, UserSettings, WorkflowSettings}; -pub use diff::DiffStats; +pub use diff::{DiffStats, DiffSummary}; pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; pub use graph::{ diff --git a/lib/crates/fabro-types/src/run_event/mod.rs b/lib/crates/fabro-types/src/run_event/mod.rs index 6ac1f331d..339c8c22d 100644 --- a/lib/crates/fabro-types/src/run_event/mod.rs +++ b/lib/crates/fabro-types/src/run_event/mod.rs @@ -981,6 +981,74 @@ mod tests { )); } + #[test] + fn patch_bearing_events_round_trip_diff_summary() { + for (event_name, properties) in [ + ( + "checkpoint.completed", + json!({ + "status": "running", + "current_node": "build", + "completed_nodes": ["build"], + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ( + "run.completed", + json!({ + "duration_ms": 42, + "artifact_count": 0, + "status": "succeeded", + "reason": "completed", + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ( + "run.failed", + json!({ + "error": "boom", + "duration_ms": 42, + "reason": "workflow_error", + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ] { + let line = json!({ + "id": format!("evt_{event_name}"), + "ts": "2026-04-04T12:00:00Z", + "run_id": fixtures::RUN_1, + "event": event_name, + "node_id": "build", + "properties": properties + }); + + let parsed = RunEvent::from_value(line).unwrap(); + let serialized = parsed.to_value().unwrap(); + + assert_eq!( + serialized["properties"]["diff_summary"], + json!({ + "files_changed": 2, + "additions": 10, + "deletions": 3 + }), + "{event_name} should preserve diff_summary" + ); + } + } + #[test] fn run_submitted_round_trip_preserves_definition_blob() { let line = json!({ diff --git a/lib/crates/fabro-types/src/run_event/run.rs b/lib/crates/fabro-types/src/run_event/run.rs index 176260ac6..aeef18879 100644 --- a/lib/crates/fabro-types/src/run_event/run.rs +++ b/lib/crates/fabro-types/src/run_event/run.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; use super::{BilledTokenCounts, ExecOutputTail, RunNoticeLevel}; use crate::status::{BlockedReason, FailureReason, SuccessReason}; use crate::{ - ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunId, RunProvenance, - WorkflowSettings, + DiffSummary, ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunId, + RunProvenance, WorkflowSettings, }; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -139,6 +139,8 @@ pub struct RunCompletedProps { #[serde(default, skip_serializing_if = "Option::is_none")] pub final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub billing: Option, } @@ -155,6 +157,8 @@ pub struct RunFailedProps { // pre-change events replay with `final_patch: None` via serde default. #[serde(default, skip_serializing_if = "Option::is_none")] pub final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_event/stage.rs b/lib/crates/fabro-types/src/run_event/stage.rs index 1b6609884..188441d6c 100644 --- a/lib/crates/fabro-types/src/run_event/stage.rs +++ b/lib/crates/fabro-types/src/run_event/stage.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use super::ExecOutputTail; -use crate::{BilledModelUsage, FailureDetail, Outcome, StageOutcome}; +use crate::{BilledModelUsage, DiffSummary, FailureDetail, Outcome, StageOutcome}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct StageStartedProps { @@ -114,6 +114,8 @@ pub struct CheckpointCompletedProps { pub node_visits: BTreeMap, #[serde(default, skip_serializing_if = "Option::is_none")] pub diff: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs index 29842afdb..9d7bdc105 100644 --- a/lib/crates/fabro-types/src/run_projection.rs +++ b/lib/crates/fabro-types/src/run_projection.rs @@ -4,9 +4,9 @@ use std::num::NonZeroU32; use chrono::{DateTime, Utc}; use crate::{ - BilledModelUsage, Checkpoint, Conclusion, InterviewQuestionRecord, InvalidTransition, - PullRequestRecord, Retro, RunControlAction, RunId, RunSpec, RunStatus, SandboxRecord, - StageCompletion, StageId, StageState, StartRecord, + BilledModelUsage, Checkpoint, Conclusion, DiffSummary, InterviewQuestionRecord, + InvalidTransition, PullRequestRecord, Retro, RunControlAction, RunId, RunSpec, RunStatus, + SandboxRecord, StageCompletion, StageId, StageState, StartRecord, }; #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] @@ -27,6 +27,8 @@ pub struct RunProjection { pub retro_response: Option, pub sandbox: Option, pub final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, pub pull_request: Option, pub superseded_by: Option, pub pending_interviews: BTreeMap, diff --git a/lib/crates/fabro-types/src/run_summary.rs b/lib/crates/fabro-types/src/run_summary.rs index 7ce93769c..b756cef4e 100644 --- a/lib/crates/fabro-types/src/run_summary.rs +++ b/lib/crates/fabro-types/src/run_summary.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc}; use fabro_util::text::strip_goal_decoration; use serde::{Deserialize, Serialize}; -use crate::{RepositoryReference, RunControlAction, RunId, RunStatus}; +use crate::{DiffSummary, RepositoryReference, RunControlAction, RunId, RunStatus}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunSummary { @@ -39,6 +39,8 @@ pub struct RunSummary { pub total_usd_micros: Option, #[serde(default)] pub superseded_by: Option, + #[serde(default)] + pub diff_summary: Option, } impl RunSummary { @@ -62,6 +64,7 @@ impl RunSummary { duration_ms: Option, total_usd_micros: Option, superseded_by: Option, + diff_summary: Option, ) -> Self { let title = truncate_goal(&goal); let repository = RepositoryReference { @@ -90,6 +93,7 @@ impl RunSummary { elapsed_secs, total_usd_micros, superseded_by, + diff_summary, } } } @@ -161,6 +165,7 @@ mod tests { use std::collections::HashMap; use chrono::{TimeZone, Utc}; + use serde_json::json; use super::RunSummary; use crate::{BlockedReason, RepositoryReference, RunControlAction, RunStatus, fixtures}; @@ -185,6 +190,7 @@ mod tests { Some(42), Some(123), Some(fixtures::RUN_2), + None, ); assert_eq!(summary.title, "ship it"); @@ -214,6 +220,35 @@ mod tests { assert_eq!(parsed, summary); } + #[test] + fn summary_round_trips_diff_summary() { + let summary: RunSummary = serde_json::from_value(json!({ + "run_id": fixtures::RUN_1, + "goal": "ship it", + "title": "ship it", + "labels": {}, + "status": { "kind": "running" }, + "repository": { "name": "fabro" }, + "created_at": fixtures::RUN_1.created_at(), + "diff_summary": { + "files_changed": 3, + "additions": 12, + "deletions": 4 + } + })) + .unwrap(); + + let value = serde_json::to_value(&summary).unwrap(); + assert_eq!( + value["diff_summary"], + json!({ + "files_changed": 3, + "additions": 12, + "deletions": 4 + }) + ); + } + #[test] fn summary_falls_back_to_source_directory_then_unknown() { let source_only = RunSummary::new( @@ -232,6 +267,7 @@ mod tests { None, None, None, + None, ); assert_eq!(source_only.repository.name, "local-checkout"); assert_eq!(source_only.last_event_at, None); @@ -252,6 +288,7 @@ mod tests { None, None, None, + None, ); assert_eq!(unknown.repository.name, "unknown"); } diff --git a/lib/crates/fabro-workflow/src/event/convert.rs b/lib/crates/fabro-workflow/src/event/convert.rs index e5a287443..a6deda2c5 100644 --- a/lib/crates/fabro-workflow/src/event/convert.rs +++ b/lib/crates/fabro-workflow/src/event/convert.rs @@ -159,6 +159,7 @@ fn event_body_from_event(event: &Event) -> EventBody { total_usd_micros, final_git_commit_sha, final_patch, + diff_summary, billing, } => EventBody::RunCompleted(fabro_types::RunCompletedProps { duration_ms: *duration_ms, @@ -168,6 +169,7 @@ fn event_body_from_event(event: &Event) -> EventBody { total_usd_micros: *total_usd_micros, final_git_commit_sha: final_git_commit_sha.clone(), final_patch: final_patch.clone(), + diff_summary: *diff_summary, billing: billing.clone(), }), Event::WorkflowRunFailed { @@ -176,6 +178,7 @@ fn event_body_from_event(event: &Event) -> EventBody { reason, git_commit_sha, final_patch, + diff_summary, } => EventBody::RunFailed(fabro_types::RunFailedProps { error: error.to_string(), causes: error.causes(), @@ -183,6 +186,7 @@ fn event_body_from_event(event: &Event) -> EventBody { reason: *reason, git_commit_sha: git_commit_sha.clone(), final_patch: final_patch.clone(), + diff_summary: *diff_summary, }), Event::RunNotice { level, @@ -428,6 +432,7 @@ fn event_body_from_event(event: &Event) -> EventBody { restart_failure_signatures, node_visits, diff, + diff_summary, .. } => EventBody::CheckpointCompleted(fabro_types::CheckpointCompletedProps { status: status.clone(), @@ -442,6 +447,7 @@ fn event_body_from_event(event: &Event) -> EventBody { restart_failure_signatures: restart_failure_signatures.clone(), node_visits: node_visits.clone(), diff: diff.clone(), + diff_summary: *diff_summary, }), Event::CheckpointFailed { error, @@ -1458,6 +1464,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: Some("abc123".to_string()), final_patch: None, + diff_summary: None, }); assert_eq!(stored.event_name(), "run.failed"); @@ -1475,6 +1482,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }); let properties = stored.properties().unwrap(); diff --git a/lib/crates/fabro-workflow/src/event/events.rs b/lib/crates/fabro-workflow/src/event/events.rs index f752b7925..5db0ae1a2 100644 --- a/lib/crates/fabro-workflow/src/event/events.rs +++ b/lib/crates/fabro-workflow/src/event/events.rs @@ -1,9 +1,9 @@ use std::collections::BTreeMap; use ::fabro_types::{ - BilledTokenCounts, BlockedReason, CommandTermination, FailureReason, ForkSourceRef, GitContext, - ParallelBranchId, Principal, PullRequestRecord, RunBlobId, RunId, RunNoticeLevel, - RunProvenance, StageId, SuccessReason, run_event as fabro_types, + BilledTokenCounts, BlockedReason, CommandTermination, DiffSummary, FailureReason, + ForkSourceRef, GitContext, ParallelBranchId, Principal, PullRequestRecord, RunBlobId, RunId, + RunNoticeLevel, RunProvenance, StageId, SuccessReason, run_event as fabro_types, }; use fabro_agent::{AgentEvent, SandboxEvent}; use serde::{Deserialize, Serialize}; @@ -123,6 +123,8 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] billing: Option, }, WorkflowRunFailed { @@ -133,6 +135,8 @@ pub enum Event { git_commit_sha: Option, #[serde(default, skip_serializing_if = "Option::is_none")] final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, }, RunNotice { level: RunNoticeLevel, @@ -321,6 +325,8 @@ pub enum Event { node_visits: BTreeMap, #[serde(default, skip_serializing_if = "Option::is_none")] diff: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, }, CheckpointFailed { node_id: String, diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index 70c438479..b574ad662 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -545,6 +545,7 @@ mod tests { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("work".into(), 2)]), diff: Some("diff --git a/story.txt b/story.txt".into()), + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/lifecycle/event.rs b/lib/crates/fabro-workflow/src/lifecycle/event.rs index 82dbd0501..fd04a186f 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/event.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/event.rs @@ -369,6 +369,7 @@ impl RunLifecycle for EventLifecycle { let git_sha = git_result.as_ref().and_then(|r| r.commit_sha.clone()); let diff = git_result.as_ref().and_then(|r| r.diff.clone()); + let diff_summary = git_result.as_ref().and_then(|r| r.diff_summary); let (loop_failure_signatures, restart_failure_signatures) = snapshot_failure_signatures(&self.circuit_breaker); let context_values = artifact::durable_context_snapshot(&state.context); @@ -400,6 +401,7 @@ impl RunLifecycle for EventLifecycle { .into_iter() .collect::>(), diff, + diff_summary, }, &scope, ); diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index 840894756..0c74c5f27 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -8,8 +8,8 @@ use fabro_core::lifecycle::RunLifecycle; use fabro_core::outcome::NodeResult; use fabro_core::state::ExecutionState; use fabro_dump::RunDump; -use fabro_types::RunId; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; +use fabro_types::{DiffSummary, RunId}; use fabro_util::error::collect_causes; use fabro_util::time::elapsed_ms; @@ -21,7 +21,9 @@ use crate::outcome::BilledModelUsage; use crate::run_metadata::{MetadataSnapshot, RunMetadataRuntime, RunMetadataWriterHandle}; use crate::run_options::RunOptions; use crate::runtime_store::RunStoreHandle; -use crate::sandbox_git::{checked_git_checkpoint, git_diff}; +use crate::sandbox_git::{ + checked_git_checkpoint, git_diff, list_diff_numstat, summarize_diff_numstat, +}; use crate::sandbox_git_runtime::SandboxGitRuntime; type WfRunState = ExecutionState>; @@ -61,6 +63,7 @@ pub(crate) struct GitCheckpointResult { pub commit_sha: Option, pub push_results: Vec, pub diff: Option, + pub diff_summary: Option, } #[derive(Debug, Clone)] @@ -279,6 +282,7 @@ impl RunLifecycle for GitLifecycle { commit_sha: Some(sha.clone()), push_results: Vec::new(), diff: None, + diff_summary: None, }; // Push run branch (skip in dry-run mode) @@ -326,7 +330,21 @@ impl RunLifecycle for GitLifecycle { .and_then(|g| g.base_sha.clone()) }); if let Some(prev) = prev.filter(|p| p != &sha) { - match git_diff(&*self.sandbox, &prev).await { + let summary_base = self + .run_options + .git + .as_ref() + .and_then(|git| git.base_sha.clone()); + let (patch_result, numstat_result) = + tokio::join!(git_diff(&*self.sandbox, &prev), async { + match summary_base.as_deref() { + Some(base) if base != sha => { + Some(list_diff_numstat(&*self.sandbox, base, &sha).await) + } + _ => None, + } + },); + match patch_result { Ok(patch) if !patch.is_empty() => { git_result.diff = Some(patch); } @@ -342,6 +360,22 @@ impl RunLifecycle for GitLifecycle { ); } } + match numstat_result { + Some(Ok(numstat)) => { + git_result.diff_summary = Some(summarize_diff_numstat(&numstat)); + } + Some(Err(err)) => { + let exec_output_tail = + fabro_sandbox::default_redacted_output_tail(&err); + self.emitter.notice_with_tail( + RunNoticeLevel::Warn, + RunNoticeCode::GitDiffFailed, + format!("[node: {node_id}] git diff stats failed: {err}"), + exec_output_tail, + ); + } + None => {} + } } // Update shared state @@ -586,6 +620,39 @@ mod tests { assert!(commit.status.success()); } + #[expect( + clippy::disallowed_methods, + reason = "metadata event tests use synchronous git commands to set up temporary repositories" + )] + fn git_commit_all(repo: &Path, msg: &str) -> String { + let add = std::process::Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + assert!(add.status.success()); + let commit = std::process::Command::new("git") + .args(["commit", "-m", msg]) + .current_dir(repo) + .output() + .unwrap(); + assert!( + commit.status.success(), + "git commit failed: {}", + String::from_utf8_lossy(&commit.stderr) + ); + let rev_parse = std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap(); + assert!(rev_parse.status.success()); + String::from_utf8(rev_parse.stdout) + .unwrap() + .trim() + .to_string() + } + fn workflow_graph() -> WorkflowGraph { let mut graph = Graph::new("metadata"); let mut start = Node::new("start"); @@ -954,6 +1021,76 @@ mod tests { ); } + #[tokio::test] + async fn checkpoint_git_result_includes_diff_summary() { + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + init_git_repo(repo); + tokio::fs::write(repo.join("notes.txt"), "one\n") + .await + .unwrap(); + let base = git_commit_all(repo, "base"); + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\n") + .await + .unwrap(); + + let mut options = run_options(repo, "fabro/metadata/run").as_ref().clone(); + options.git = Some(GitCheckpointOptions { + base_sha: Some(base), + run_branch: None, + meta_branch: None, + }); + let lifecycle = git_lifecycle_with_writer( + repo, + Arc::new(Emitter::new(fixtures::RUN_1)), + RunStoreHandle::local(run_store(fixtures::RUN_1).await), + Arc::new(options), + Arc::new(RunMetadataRuntime::new()), + None, + ); + let graph = workflow_graph(); + let node = graph.get_node("build").unwrap(); + let mut state = ExecutionState::new(&graph).unwrap(); + state.increment_visits("build"); + let result = WfNodeResult::new(Outcome::success(), Duration::from_millis(10), 1, 1); + + lifecycle + .on_checkpoint(&node, &result, Some("exit"), &state) + .await + .unwrap(); + + let git_result = lifecycle + .checkpoint_git_result + .lock() + .unwrap() + .clone() + .unwrap(); + let diff_summary = git_result.diff_summary.expect("diff summary"); + assert_eq!(diff_summary.files_changed, 1); + assert_eq!(diff_summary.additions, 1); + assert_eq!(diff_summary.deletions, 0); + + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\nthree\n") + .await + .unwrap(); + state.increment_visits("build"); + lifecycle + .on_checkpoint(&node, &result, Some("exit"), &state) + .await + .unwrap(); + + let git_result = lifecycle + .checkpoint_git_result + .lock() + .unwrap() + .clone() + .unwrap(); + let diff_summary = git_result.diff_summary.expect("diff summary"); + assert_eq!(diff_summary.files_changed, 1); + assert_eq!(diff_summary.additions, 2); + assert_eq!(diff_summary.deletions, 0); + } + #[tokio::test] async fn degraded_metadata_runtime_skips_snapshot_events() { let repo_dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/archive.rs b/lib/crates/fabro-workflow/src/operations/archive.rs index 87f2e5b50..9d1065b91 100644 --- a/lib/crates/fabro-workflow/src/operations/archive.rs +++ b/lib/crates/fabro-workflow/src/operations/archive.rs @@ -159,6 +159,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }) .await @@ -173,6 +174,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/fork.rs b/lib/crates/fabro-workflow/src/operations/fork.rs index 66f6baa67..58759b0aa 100644 --- a/lib/crates/fabro-workflow/src/operations/fork.rs +++ b/lib/crates/fabro-workflow/src/operations/fork.rs @@ -289,6 +289,7 @@ fn checkpoint_completed_event(checkpoint: &Checkpoint) -> Event { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, } } @@ -412,6 +413,7 @@ mod tests { restart_failure_signatures: BTreeMap::new(), node_visits, diff: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index cedce5386..e6b3337b4 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -274,6 +274,7 @@ async fn persist_terminal_engine_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await { @@ -899,6 +900,7 @@ impl Drop for DetachedRunBootstrapGuard { reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await; }); @@ -964,6 +966,7 @@ impl Drop for DetachedRunCompletionGuard { reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await; let _ = append_event_to_sink(&event_sink, &run_id, &Event::RunNotice { @@ -994,6 +997,7 @@ async fn persist_detached_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await { @@ -1177,6 +1181,7 @@ mod tests { restart_failure_signatures: HashMap::new().into_iter().collect(), node_visits: HashMap::new().into_iter().collect(), diff: None, + diff_summary: None, }); } }); @@ -1389,6 +1394,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }, ) .await @@ -1479,6 +1485,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }) .await .unwrap(); @@ -1496,6 +1503,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }) .await diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index a1cc84d19..3189ee21f 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -4,7 +4,7 @@ use std::time::Instant; use fabro_dump::RunDump; use fabro_hooks::{HookContext, HookEvent}; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; -use fabro_types::{BilledTokenCounts, EventBody, RunProjection}; +use fabro_types::{BilledTokenCounts, DiffSummary, EventBody, RunProjection}; use fabro_util::error::collect_causes; use fabro_util::time::elapsed_ms; @@ -17,7 +17,7 @@ use crate::run_metadata::MetadataSnapshot; use crate::run_options::RunOptions; use crate::run_status::{FailureReason, RunStatus, SuccessReason}; use crate::runtime_store::RunStoreHandle; -use crate::sandbox_git::git_diff_with_timeout; +use crate::sandbox_git::{git_diff_with_timeout, list_diff_numstat, summarize_diff_numstat}; use crate::services::RunServices; use crate::{ProjectionBillingRollup, billing_rollup_from_projection}; @@ -391,13 +391,20 @@ async fn compute_final_patch( run_options: &RunOptions, services: &RunServices, status: StageOutcome, -) -> Option { - let base_sha = run_options.git.as_ref().and_then(|g| g.base_sha.clone())?; +) -> (Option, Option) { + let Some(base_sha) = run_options.git.as_ref().and_then(|g| g.base_sha.clone()) else { + return (None, None); + }; let timeout_ms = match status { StageOutcome::Succeeded | StageOutcome::PartiallySucceeded => 30_000, _ => 10_000, }; - match git_diff_with_timeout(&*services.sandbox, &base_sha, timeout_ms).await { + let to_sha = "HEAD"; + let (patch_result, numstat_result) = tokio::join!( + git_diff_with_timeout(&*services.sandbox, &base_sha, timeout_ms), + list_diff_numstat(&*services.sandbox, &base_sha, to_sha), + ); + let final_patch = match patch_result { Ok(patch) if !patch.is_empty() => Some(patch), Ok(_) => None, Err(err) => { @@ -408,7 +415,19 @@ async fn compute_final_patch( ); None } - } + }; + let diff_summary = match numstat_result { + Ok(numstat) => Some(summarize_diff_numstat(&numstat)), + Err(err) => { + services.emitter.notice( + RunNoticeLevel::Warn, + RunNoticeCode::GitDiffFailed, + format!("final diff stats failed: {err}"), + ); + None + } + }; + (final_patch, diff_summary) } pub(crate) fn billing_from_projection(projection: &RunProjection) -> Option { @@ -421,6 +440,7 @@ pub(crate) fn build_terminal_event( artifact_count: usize, final_git_commit_sha: Option, final_patch: Option, + diff_summary: Option, billing: Option, ) -> Event { if matches!(outcome, Err(Error::Cancelled)) { @@ -430,6 +450,7 @@ pub(crate) fn build_terminal_event( reason: FailureReason::Cancelled, git_commit_sha: final_git_commit_sha, final_patch, + diff_summary, }; } @@ -455,6 +476,7 @@ pub(crate) fn build_terminal_event( total_usd_micros, final_git_commit_sha, final_patch, + diff_summary, billing, }; } @@ -473,6 +495,7 @@ pub(crate) fn build_terminal_event( reason: FailureReason::WorkflowError, git_commit_sha: final_git_commit_sha, final_patch, + diff_summary, } } @@ -542,7 +565,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result Result String { + let add = std::process::Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + assert!(add.status.success()); + let commit = std::process::Command::new("git") + .args(["commit", "-m", msg]) + .current_dir(repo) + .output() + .unwrap(); + assert!( + commit.status.success(), + "git commit failed: {}", + String::from_utf8_lossy(&commit.stderr) + ); + let rev_parse = std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap(); + assert!(rev_parse.status.success()); + String::from_utf8(rev_parse.stdout) + .unwrap() + .trim() + .to_string() + } + fn record_events(emitter: &Arc) -> Arc>> { let events = Arc::new(std::sync::Mutex::new(Vec::new())); let captured = Arc::clone(&events); @@ -1160,6 +1217,71 @@ mod tests { ]); } + #[tokio::test] + async fn finalize_terminal_event_includes_diff_summary() { + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + init_git_repo(repo); + tokio::fs::write(repo.join("notes.txt"), "one\n") + .await + .unwrap(); + let base = git_commit_all(repo, "base"); + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\nthree\n") + .await + .unwrap(); + let head = git_commit_all(repo, "head"); + + let run_store = seeded_run_store().await; + let emitter = Arc::new(Emitter::new(test_run_id())); + let events = record_events(&emitter); + let services = test_services( + RunStoreHandle::local(run_store), + Arc::clone(&emitter), + Arc::new(fabro_agent::LocalSandbox::new(repo.to_path_buf())), + Arc::new(RunMetadataRuntime::new()), + None, + ); + let mut run_options = test_git_run_options(repo, "fabro/metadata/run"); + run_options.git = Some(GitCheckpointOptions { + base_sha: Some(base), + run_branch: None, + meta_branch: None, + }); + let retroed = Retroed { + graph: Graph::new("test"), + outcome: Ok(Outcome::success()), + run_options, + duration_ms: 5, + services, + retro: None, + }; + + finalize(retroed, &FinalizeOptions { + run_dir: repo.to_path_buf(), + run_id: test_run_id(), + workflow_name: "test".to_string(), + preserve_sandbox: true, + last_git_sha: Some(head), + }) + .await + .unwrap(); + + let events = events.lock().unwrap(); + let run_completed = events + .iter() + .find(|event| event.event_name() == "run.completed") + .expect("run.completed event"); + let properties = run_completed.properties().unwrap(); + assert_eq!( + properties["diff_summary"], + serde_json::json!({ + "files_changed": 1, + "additions": 2, + "deletions": 0 + }) + ); + } + struct FailingStateStore; #[async_trait] diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index 03edcbc0e..cfef50473 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -1709,6 +1709,7 @@ mod tests { final_patch: Some( "diff --git a/src/lib.rs b/src/lib.rs\n+fn from_store() {}\n".to_string(), ), + diff_summary: None, billing: None, }) .await @@ -1997,6 +1998,7 @@ mod tests { final_patch: Some( "diff --git a/src/lib.rs b/src/lib.rs\n+fn from_store() {}\n".to_string(), ), + diff_summary: None, billing: None, }) .await diff --git a/lib/crates/fabro-workflow/src/pipeline/retro.rs b/lib/crates/fabro-workflow/src/pipeline/retro.rs index 2749cf023..224ab5043 100644 --- a/lib/crates/fabro-workflow/src/pipeline/retro.rs +++ b/lib/crates/fabro-workflow/src/pipeline/retro.rs @@ -294,6 +294,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/sandbox_git.rs b/lib/crates/fabro-workflow/src/sandbox_git.rs index c370a2d89..817c0fa53 100644 --- a/lib/crates/fabro-workflow/src/sandbox_git.rs +++ b/lib/crates/fabro-workflow/src/sandbox_git.rs @@ -564,7 +564,7 @@ fn classify_entry( }) } -pub use fabro_types::DiffStats; +pub use fabro_types::{DiffStats, DiffSummary}; /// Output of `git diff --numstat`: which paths are binary, plus per-path /// `+/-` line totals for text files in the range. Both pieces come from a @@ -577,6 +577,27 @@ pub struct DiffNumstat { pub line_stats_by_path: HashMap, } +pub fn summarize_diff_numstat(numstat: &DiffNumstat) -> DiffSummary { + let text_files = i64::try_from(numstat.line_stats_by_path.len()).unwrap_or(i64::MAX); + let binary_files = i64::try_from(numstat.binary_paths.len()).unwrap_or(i64::MAX); + let (additions, deletions) = + numstat + .line_stats_by_path + .values() + .fold((0_i64, 0_i64), |(adds, dels), stats| { + ( + adds.saturating_add(stats.additions), + dels.saturating_add(stats.deletions), + ) + }); + + DiffSummary { + files_changed: text_files.saturating_add(binary_files), + additions, + deletions, + } +} + /// Run `git diff --numstat` once and return both the set of binary paths and /// text-file `+/-` totals. The single call replaces the previous binary-only /// helper. diff --git a/lib/crates/fabro-workflow/src/test_support.rs b/lib/crates/fabro-workflow/src/test_support.rs index c3e1fe517..487e395e9 100644 --- a/lib/crates/fabro-workflow/src/test_support.rs +++ b/lib/crates/fabro-workflow/src/test_support.rs @@ -43,6 +43,7 @@ async fn execute_and_emit_terminal(initialized: InitializedState) -> Executed { 0, None, None, + None, billing, ); executed.engine.run.emitter.emit(&event); diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 0f91dd09c..564c81ccc 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -65,6 +65,7 @@ models/diagnostics-report.ts models/diagnostics-section.ts models/diff-file.ts models/diff-stats.ts +models/diff-summary.ts models/dirty-status.ts models/discord-integration-settings.ts models/disk-usage-response.ts diff --git a/lib/packages/fabro-api-client/src/models/diff-summary.ts b/lib/packages/fabro-api-client/src/models/diff-summary.ts new file mode 100644 index 000000000..f5dbcea5e --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/diff-summary.ts @@ -0,0 +1,33 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Cheap aggregate file and line counts for a run diff. + */ +export interface DiffSummary { + /** + * Total number of changed files, including binary files. + */ + 'files_changed': number; + /** + * Total lines added across text files. + */ + 'additions': number; + /** + * Total lines deleted across text files. + */ + 'deletions': number; +} diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 34479400e..361a4f7ec 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -45,6 +45,7 @@ export * from './diagnostics-report'; export * from './diagnostics-section'; export * from './diff-file'; export * from './diff-stats'; +export * from './diff-summary'; export * from './dirty-status'; export * from './discord-integration-settings'; export * from './disk-usage-response'; diff --git a/lib/packages/fabro-api-client/src/models/run-projection.ts b/lib/packages/fabro-api-client/src/models/run-projection.ts index 6deb882ea..4bd3b6512 100644 --- a/lib/packages/fabro-api-client/src/models/run-projection.ts +++ b/lib/packages/fabro-api-client/src/models/run-projection.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { DiffSummary } from './diff-summary'; // May contain unused imports in some cases // @ts-ignore import type { PendingInterviewRecord } from './pending-interview-record'; @@ -57,6 +60,7 @@ export interface RunProjection { 'retro_response'?: string | null; 'sandbox'?: { [key: string]: any; } | null; 'final_patch'?: string | null; + 'diff_summary'?: DiffSummary | null; 'pull_request'?: { [key: string]: any; } | null; 'superseded_by'?: string | null; 'pending_interviews'?: { [key: string]: PendingInterviewRecord; }; diff --git a/lib/packages/fabro-api-client/src/models/run-summary.ts b/lib/packages/fabro-api-client/src/models/run-summary.ts index 290190f83..6e387ac06 100644 --- a/lib/packages/fabro-api-client/src/models/run-summary.ts +++ b/lib/packages/fabro-api-client/src/models/run-summary.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { DiffSummary } from './diff-summary'; // May contain unused imports in some cases // @ts-ignore import type { RepositoryReference } from './repository-reference'; @@ -46,6 +49,7 @@ export interface RunSummary { 'elapsed_secs'?: number | null; 'total_usd_micros'?: number | null; 'superseded_by'?: string | null; + 'diff_summary'?: DiffSummary | null; }