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; }