From 3b13724fed38dec681c85d5bff8429c830999c00 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Tue, 2 Jun 2026 09:28:28 -0400 Subject: [PATCH] Wire Auto Review lifecycle store updates --- code-rs/core/src/review_store.rs | 12 +- code-rs/exec/Cargo.toml | 2 +- code-rs/exec/src/lib.rs | 280 ++++++++++++++++++++++ code-rs/tui/src/chatwidget.rs | 400 ++++++++++++++++++++++++++++++- 4 files changed, 689 insertions(+), 5 deletions(-) diff --git a/code-rs/core/src/review_store.rs b/code-rs/core/src/review_store.rs index aadf5542474..ca9b99f3dc2 100644 --- a/code-rs/core/src/review_store.rs +++ b/code-rs/core/src/review_store.rs @@ -14,6 +14,8 @@ const RUNS_FILENAME: &str = "runs.json"; const OUTPUTS_DIR: &str = "outputs"; const SCHEMA_VERSION: u32 = 1; const DEFAULT_MAX_RUNS: usize = 500; +const MAX_FINDING_DIGESTS: usize = 25; +const MAX_FINDING_DIGEST_TITLE_CHARS: usize = 160; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -135,6 +137,8 @@ pub struct AutoReviewRun { pub finding_count: usize, #[serde(default)] pub finding_digests: Vec, + #[serde(default)] + pub omitted_finding_digest_count: usize, #[serde(skip_serializing_if = "Option::is_none")] pub summary_digest: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -183,6 +187,7 @@ impl AutoReviewRun { token_count: None, finding_count: 0, finding_digests: Vec::new(), + omitted_finding_digest_count: 0, summary_digest: None, superseded_by: None, cancel_reason: None, @@ -349,6 +354,10 @@ impl AutoReviewRunStore { run.output_path = Some(output_path.clone()); run.finding_count = output.findings.len(); run.finding_digests = finding_digests(output); + run.omitted_finding_digest_count = output + .findings + .len() + .saturating_sub(run.finding_digests.len()); run.summary_digest = summarize(&output.overall_explanation, 240); self.save()?; Ok(output_path) @@ -447,11 +456,12 @@ fn finding_digests(output: &ReviewOutputEvent) -> Vec { output .findings .iter() + .take(MAX_FINDING_DIGESTS) .enumerate() .map(|(idx, finding)| AutoReviewFindingDigest { finding_id: format!("f{}", idx + 1), priority: finding.priority, - title: finding.title.clone(), + title: summarize(&finding.title, MAX_FINDING_DIGEST_TITLE_CHARS).unwrap_or_default(), path: Some(finding.code_location.absolute_file_path.clone()), line_start: Some(finding.code_location.line_range.start), line_end: Some(finding.code_location.line_range.end), diff --git a/code-rs/exec/Cargo.toml b/code-rs/exec/Cargo.toml index a9b71a96d2a..59002d838a5 100644 --- a/code-rs/exec/Cargo.toml +++ b/code-rs/exec/Cargo.toml @@ -48,6 +48,7 @@ tokio = { workspace = true, features = [ tracing = { workspace = true, features = ["log"] } tracing-subscriber = { workspace = true, features = ["env-filter"] } once_cell = { workspace = true } +uuid = { workspace = true, features = ["v4"] } [target.'cfg(unix)'.dependencies] libc = "0.2" @@ -56,4 +57,3 @@ libc = "0.2" [dev-dependencies] filetime = { workspace = true } tempfile = { workspace = true } -uuid = { version = "1", features = ["v4"] } diff --git a/code-rs/exec/src/lib.rs b/code-rs/exec/src/lib.rs index 4dc0619bb25..b0748f66cb3 100644 --- a/code-rs/exec/src/lib.rs +++ b/code-rs/exec/src/lib.rs @@ -42,6 +42,13 @@ use code_core::protocol::Op; use code_core::protocol::ReviewOutputEvent; use code_core::protocol::ReviewRequest; use code_core::protocol::TaskCompleteEvent; +use code_core::review_store::{ + AutoReviewFreshness, + AutoReviewRun, + AutoReviewRunSource, + AutoReviewRunStatus, + AutoReviewRunStore, +}; use code_protocol::models::ContentItem; use code_protocol::models::ResponseItem; use code_protocol::protocol::SessionSource; @@ -55,12 +62,14 @@ use code_git_tooling::CreateGhostCommitOptions; use code_git_tooling::create_ghost_commit; use serde_json::Value; use std::collections::HashSet; +use std::collections::HashMap; use std::backtrace::Backtrace; use std::io::IsTerminal; use std::io::Read; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::Once; +use std::time::{SystemTime, UNIX_EPOCH}; use supports_color::Stream; use tokio::time::{Duration, Instant}; use tracing::debug; @@ -99,6 +108,7 @@ use code_core::timeboxed_exec_guidance::{ /// How long exec waits after task completion before sending Shutdown when Auto Review /// may be about to start. Guarded so sub-agents are not delayed. const AUTO_REVIEW_SHUTDOWN_GRACE_MS: u64 = 1_500; +const AUTO_REVIEW_STALE_SECS: u64 = 5 * 60; const REVIEW_SCOPE_MAX_LISTED_PATHS: usize = 120; const REVIEW_SCOPE_EXCLUDED_PREFIXES: [&str; 5] = [ "codex-rs/", @@ -1700,16 +1710,19 @@ struct AutoReviewCompletion { struct AutoReviewTracker { running: HashSet, processed: HashSet, + run_ids: HashMap, git_root: PathBuf, } impl AutoReviewTracker { fn new(cwd: &Path) -> Self { let git_root = get_git_repo_root(cwd).unwrap_or_else(|| cwd.to_path_buf()); + reconcile_auto_review_store_after_restart(&git_root); Self { running: HashSet::new(), processed: HashSet::new(), + run_ids: HashMap::new(), git_root, } } @@ -1723,6 +1736,15 @@ impl AutoReviewTracker { } let status = agent.status.to_ascii_lowercase(); + let run_id = if let Some(run_id) = self.run_ids.get(&agent.id).copied() { + run_id + } else { + let run_id = find_existing_auto_review_run_id(&self.git_root, &agent.id) + .unwrap_or_else(uuid::Uuid::new_v4); + self.run_ids.insert(agent.id.clone(), run_id); + run_id + }; + self.record_agent_status(run_id, agent, status.as_str()); if status == "pending" || status == "running" { self.running.insert(agent.id.clone()); continue; @@ -1759,11 +1781,171 @@ impl AutoReviewTracker { completions } + fn record_agent_status( + &self, + run_id: uuid::Uuid, + agent: &code_core::protocol::AgentInfo, + status: &str, + ) { + let now = unix_now_secs(); + let mut store = match AutoReviewRunStore::open(&self.git_root) { + Ok(store) => store, + Err(err) => { + tracing::warn!(?err, run_id = %run_id, "failed to open exec auto-review run store"); + return; + } + }; + + let durable_status = match status { + "completed" => AutoReviewRunStatus::Completed, + "failed" => AutoReviewRunStatus::Failed, + "cancelled" => AutoReviewRunStatus::Cancelled, + "running" => AutoReviewRunStatus::Reviewing, + _ => AutoReviewRunStatus::Pending, + }; + + let mut run = store + .get(run_id) + .cloned() + .unwrap_or_else(|| AutoReviewRun::new(run_id, AutoReviewRunSource::Exec, now)); + run.source = AutoReviewRunSource::Exec; + run.agent_id = Some(agent.id.clone()); + run.batch_id = agent.batch_id.clone().or_else(|| run.batch_id.clone()); + run.branch = agent.batch_id.clone().or_else(|| run.branch.clone()); + run.snapshot_commit = agent.worktree_base.clone().or_else(|| run.snapshot_commit.clone()); + run.owner_session_id = agent + .owner_session_id + .as_deref() + .and_then(|id| uuid::Uuid::parse_str(id).ok()) + .or(run.owner_session_id); + run.model = agent.model.clone().or_else(|| run.model.clone()); + run.token_count = agent.token_count.or(run.token_count); + run.freshness = if durable_status.is_terminal() { + AutoReviewFreshness::Current + } else if agent + .seconds_since_last_activity + .is_some_and(|seconds| seconds >= AUTO_REVIEW_STALE_SECS) + { + AutoReviewFreshness::Inactive + } else { + AutoReviewFreshness::Current + }; + run.mark_status(durable_status, now); + if !durable_status.is_terminal() || agent.last_activity_at.is_some() { + run.mark_activity(now); + } + if let Some(error) = agent.error.as_deref() { + run.error_summary = Some(summarize_auto_review_error(error)); + } + if durable_status.is_terminal() + && let Some(raw_result) = agent.result.as_deref() + { + let summary = parse_auto_review_summary(raw_result); + if summary.has_findings { + run.finding_count = summary.findings.max(1); + } + run.summary_digest = summary + .summary + .as_deref() + .map(|summary| summarize_auto_review_text(summary, 240)) + .or(run.summary_digest); + } + + if let Err(err) = store.upsert(run) { + tracing::warn!(?err, run_id = %run_id, "failed to persist exec auto-review run"); + return; + } + + if durable_status == AutoReviewRunStatus::Completed + && let Some(output) = parse_auto_review_output(agent.result.as_deref()) + { + match AutoReviewRunStore::open(&self.git_root) { + Ok(mut store) => { + if let Err(err) = store.write_output(run_id, &output) { + tracing::warn!(?err, run_id = %run_id, "failed to persist exec auto-review output"); + } + } + Err(err) => { + tracing::warn!(?err, run_id = %run_id, "failed to open exec auto-review store for output"); + } + } + } + } + fn is_running(&self) -> bool { !self.running.is_empty() } } +fn unix_now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_secs()) + .unwrap_or(0) +} + +fn reconcile_auto_review_store_after_restart(git_root: &Path) { + let now = unix_now_secs(); + match AutoReviewRunStore::open(git_root) { + Ok(mut store) => { + if let Err(err) = store.reconcile_orphaned_in_flight(std::iter::empty::<&str>(), now) { + tracing::warn!(?err, "failed to reconcile exec auto-review run store after restart"); + } + } + Err(err) => { + tracing::warn!(?err, "failed to open exec auto-review run store for restart reconciliation"); + } + } +} + +fn find_existing_auto_review_run_id(git_root: &Path, agent_id: &str) -> Option { + let store = AutoReviewRunStore::open(git_root).ok()?; + store + .runs() + .filter(|run| run.agent_id.as_deref() == Some(agent_id)) + .max_by(|left, right| { + let left_in_flight = left.status.is_in_flight(); + let right_in_flight = right.status.is_in_flight(); + left_in_flight + .cmp(&right_in_flight) + .then_with(|| left.updated_at.cmp(&right.updated_at)) + .then_with(|| left.created_at.cmp(&right.created_at)) + .then_with(|| left.run_id.cmp(&right.run_id)) + }) + .map(|run| run.run_id) +} + +fn summarize_auto_review_error(error: &str) -> String { + let trimmed = error.trim(); + if trimmed.is_empty() { + return "Auto review failed before producing findings.".to_string(); + } + trimmed + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or("Auto review failed before producing findings.") + .chars() + .take(240) + .collect() +} + +fn summarize_auto_review_text(text: &str, max_chars: usize) -> String { + let trimmed = text.trim(); + let mut chars = trimmed.chars(); + let mut out = String::new(); + for _ in 0..max_chars { + let Some(ch) = chars.next() else { + return out; + }; + out.push(ch); + } + if chars.next().is_some() { + out.push('…'); + } + out +} + fn emit_auto_review_completion(completion: &AutoReviewCompletion) { let branch = completion.branch.as_deref().unwrap_or("auto-review"); @@ -1885,6 +2067,104 @@ fn parse_auto_review_summary(raw: &str) -> AutoReviewSummary { } } +fn parse_auto_review_output(raw: Option<&str>) -> Option { + #[derive(serde::Deserialize)] + struct MultiRunReview { + #[serde(flatten)] + latest: ReviewOutputEvent, + #[serde(default)] + runs: Vec, + } + + let text = raw?.trim(); + if text.is_empty() { + return None; + } + + if let Ok(wrapper) = serde_json::from_str::(text) { + return wrapper.runs.last().cloned().or(Some(wrapper.latest)); + } + if let Ok(output) = serde_json::from_str::(text) { + return Some(output); + } + if let Some(output) = extract_review_output_from_mixed_text(text) { + return Some(output); + } + if let Some(start) = text.find("```") + && let Some((body, _)) = text[start + 3..].split_once("```") + { + let candidate = body.trim_start_matches("json").trim(); + if let Ok(output) = serde_json::from_str::(candidate) { + return Some(output); + } + } + None +} + +fn extract_review_output_from_mixed_text(text: &str) -> Option { + #[derive(serde::Deserialize)] + struct MultiRunReview { + #[serde(flatten)] + latest: ReviewOutputEvent, + #[serde(default)] + runs: Vec, + } + + let mut brace_depth = 0usize; + let mut in_string = false; + let mut escaped = false; + let mut object_start = None; + let mut best_match: Option = None; + + for (idx, ch) in text.char_indices() { + if in_string { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + in_string = false; + } + continue; + } + + match ch { + '"' => in_string = true, + '{' => { + if brace_depth == 0 { + object_start = Some(idx); + } + brace_depth = brace_depth.saturating_add(1); + } + '}' => { + if brace_depth == 0 { + continue; + } + brace_depth -= 1; + if brace_depth == 0 + && let Some(start) = object_start.take() + { + let candidate = &text[start..=idx]; + if let Ok(wrapper) = serde_json::from_str::(candidate) { + best_match = wrapper.runs.last().cloned().or(Some(wrapper.latest)); + continue; + } + if let Ok(output) = serde_json::from_str::(candidate) { + best_match = Some(output); + } + } + } + _ => {} + } + } + + best_match +} + fn summary_from_runs(outputs: &[ReviewOutputEvent]) -> AutoReviewSummary { if outputs.is_empty() { return AutoReviewSummary::default(); diff --git a/code-rs/tui/src/chatwidget.rs b/code-rs/tui/src/chatwidget.rs index 0a2929d3060..c2a64732c94 100644 --- a/code-rs/tui/src/chatwidget.rs +++ b/code-rs/tui/src/chatwidget.rs @@ -201,6 +201,13 @@ use code_core::protocol::TaskCompleteEvent; use code_core::protocol::TokenUsage; use code_core::protocol::TurnDiffEvent; use code_core::protocol::ViewImageToolCallEvent; +use code_core::review_store::{ + AutoReviewFreshness, + AutoReviewRun, + AutoReviewRunSource, + AutoReviewRunStatus, + AutoReviewRunStore, +}; use code_core::review_coord::{bump_snapshot_epoch_for, try_acquire_lock, ReviewGuard}; use code_core::ConversationManager; use code_core::codex::compact::COMPACTION_CHECKPOINT_MESSAGE; @@ -1421,6 +1428,20 @@ impl BackgroundReviewState { } } +struct AutoReviewStoreCompletion<'a> { + run_id: Uuid, + branch: &'a str, + worktree_path: &'a std::path::Path, + has_findings: bool, + findings: usize, + summary: Option<&'a str>, + error: Option<&'a str>, + agent_id: Option<&'a str>, + snapshot: Option<&'a str>, + owner_session_id: Option, + output: Option<&'a ReviewOutputEvent>, +} + #[derive(Clone, Debug)] struct PendingAutoReviewRange { base: GhostCommit, @@ -1497,6 +1518,13 @@ fn auto_review_running_detail( Some(parts.join(", ")) } +fn unix_now_secs() -> u64 { + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|duration| duration.as_secs()) + .unwrap_or(0) +} + const SKIP_REVIEW_PROGRESS_SENTINEL: &str = "Another review is already running; skipping this /review."; const AUTO_REVIEW_SHARED_WORKTREE: &str = "auto-review"; const AUTO_REVIEW_FALLBACK_PREFIX: &str = "auto-review-"; @@ -3115,7 +3143,7 @@ struct AgentInfo { last_progress: Option, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] enum AgentStatus { Pending, Running, @@ -7485,6 +7513,7 @@ impl ChatWidget<'_> { resume_placeholder_visible: false, resume_picker_loading: false, }; + new_widget.reconcile_auto_review_store_after_restart(); new_widget.load_auto_review_baseline_marker(); new_widget.spawn_conversation_runtime(config.clone(), auth_manager.clone(), code_op_rx); if let Ok(Some(active_id)) = auth_accounts::get_active_account_id(&config.code_home) { @@ -40393,6 +40422,13 @@ impl ChatWidget<'_> { return; } + if let Some(run_id) = self.background_review_run_id { + self.mark_auto_review_run_lost( + run_id, + "auto_review_inactive_in_tui", + state.agent_id.as_deref(), + ); + } let stale = self.background_review.take(); self.background_review_guard = None; self.background_review_run_id = None; @@ -40422,6 +40458,7 @@ impl ChatWidget<'_> { ); let owner_session_id = self.session_id.unwrap_or_else(Uuid::new_v4); let run_id = Uuid::new_v4(); + self.record_auto_review_launch_in_store(run_id, owner_session_id, base_snapshot.as_ref()); self.background_review_run_id = Some(run_id); self.background_review = Some(BackgroundReviewState { worktree_path: std::path::PathBuf::new(), @@ -40523,6 +40560,25 @@ impl ChatWidget<'_> { if let Some(progress) = agent.last_progress.as_deref() { if progress.contains(SKIP_REVIEW_PROGRESS_SENTINEL) { + if let Some(run_id) = self.background_review_run_id { + let owner_session_id = agent + .owner_session_id + .as_deref() + .and_then(|id| Uuid::parse_str(id).ok()); + self.record_auto_review_completion_in_store(AutoReviewStoreCompletion { + run_id, + branch: agent.batch_id.as_deref().unwrap_or_default(), + worktree_path: Path::new(""), + has_findings: false, + findings: 0, + summary: Some("Auto review skipped: another auto review is already running."), + error: None, + agent_id: Some(agent.id.as_str()), + snapshot: agent.worktree_base.as_deref(), + owner_session_id, + output: None, + }); + } // Treat skipped review as benign: clear indicator and state, do not surface. self.clear_auto_review_indicator(); self.background_review = None; @@ -40545,6 +40601,10 @@ impl ChatWidget<'_> { } } + if let Some(run_id) = self.background_review_run_id { + self.record_auto_review_agent_status_in_store(run_id, agent, status); + } + if matches!(status, AgentStatus::Running | AgentStatus::Pending) { let findings = self.auto_review_status.as_ref().and_then(|s| s.findings); let detail = auto_review_running_detail( @@ -40620,6 +40680,7 @@ impl ChatWidget<'_> { }; let (has_findings, findings, summary) = Self::parse_agent_review_result(agent.result.as_deref()); + let output = Self::parse_agent_review_output(agent.result.as_deref()); let owner_session_id = agent .owner_session_id .as_deref() @@ -40652,6 +40713,21 @@ impl ChatWidget<'_> { } self.remember_processed_auto_review_agent(&agent.id); + if let Some(run_id) = self.background_review_run_id { + self.record_auto_review_completion_in_store(AutoReviewStoreCompletion { + run_id, + branch: &branch, + worktree_path: &worktree_path, + has_findings, + findings, + summary: summary.as_deref(), + error: agent.error.as_deref(), + agent_id: Some(agent.id.as_str()), + snapshot: snapshot.as_deref(), + owner_session_id, + output: output.as_ref(), + }); + } self.on_background_review_finished( worktree_path, branch, @@ -40825,6 +40901,303 @@ impl ChatWidget<'_> { best_match } + fn parse_agent_review_output(raw: Option<&str>) -> Option { + #[derive(serde::Deserialize)] + struct MultiRunReview { + #[serde(flatten)] + latest: ReviewOutputEvent, + #[serde(default)] + runs: Vec, + } + + let text = raw?.trim(); + if text.is_empty() { + return None; + } + + if let Ok(wrapper) = serde_json::from_str::(text) { + return wrapper.runs.last().cloned().or(Some(wrapper.latest)); + } + if let Ok(output) = serde_json::from_str::(text) { + return Some(output); + } + if let Some(output) = Self::extract_review_output_from_mixed_text(text) { + return Some(output); + } + if let Some(start) = text.find("```") + && let Some((body, _)) = text[start + 3..].split_once("```") + { + let candidate = body.trim_start_matches("json").trim(); + if let Ok(output) = serde_json::from_str::(candidate) { + return Some(output); + } + } + None + } + + fn extract_review_output_from_mixed_text(text: &str) -> Option { + #[derive(serde::Deserialize)] + struct MultiRunReview { + #[serde(flatten)] + latest: ReviewOutputEvent, + #[serde(default)] + runs: Vec, + } + + let mut brace_depth = 0usize; + let mut in_string = false; + let mut escaped = false; + let mut object_start = None; + let mut best_match: Option = None; + + for (idx, ch) in text.char_indices() { + if in_string { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + in_string = false; + } + continue; + } + + match ch { + '"' => in_string = true, + '{' => { + if brace_depth == 0 { + object_start = Some(idx); + } + brace_depth = brace_depth.saturating_add(1); + } + '}' => { + if brace_depth == 0 { + continue; + } + brace_depth -= 1; + if brace_depth == 0 + && let Some(start) = object_start.take() + { + let candidate = &text[start..=idx]; + if let Ok(wrapper) = serde_json::from_str::(candidate) { + best_match = wrapper.runs.last().cloned().or(Some(wrapper.latest)); + continue; + } + if let Ok(output) = serde_json::from_str::(candidate) { + best_match = Some(output); + } + } + } + _ => {} + } + } + + best_match + } + + fn durable_auto_review_status(status: AgentStatus) -> AutoReviewRunStatus { + match status { + AgentStatus::Pending => AutoReviewRunStatus::Pending, + AgentStatus::Running => AutoReviewRunStatus::Reviewing, + AgentStatus::Completed => AutoReviewRunStatus::Completed, + AgentStatus::Failed => AutoReviewRunStatus::Failed, + AgentStatus::Cancelled => AutoReviewRunStatus::Cancelled, + } + } + + fn update_auto_review_store(&self, run_id: Uuid, update: F) + where + F: FnOnce(&mut AutoReviewRun, u64), + { + let now = unix_now_secs(); + let mut store = match AutoReviewRunStore::open(&self.config.cwd) { + Ok(store) => store, + Err(err) => { + tracing::warn!(?err, run_id = %run_id, "failed to open auto-review run store"); + return; + } + }; + + let mut run = store + .get(run_id) + .cloned() + .unwrap_or_else(|| AutoReviewRun::new(run_id, AutoReviewRunSource::Tui, now)); + update(&mut run, now); + if let Err(err) = store.upsert(run) { + tracing::warn!(?err, run_id = %run_id, "failed to persist auto-review run"); + } + } + + fn reconcile_auto_review_store_after_restart(&self) { + let now = unix_now_secs(); + match AutoReviewRunStore::open(&self.config.cwd) { + Ok(mut store) => { + if let Err(err) = store.reconcile_orphaned_in_flight(std::iter::empty::<&str>(), now) { + tracing::warn!(?err, "failed to reconcile auto-review run store after restart"); + } + } + Err(err) => { + tracing::warn!(?err, "failed to open auto-review run store for restart reconciliation"); + } + } + } + + fn mark_auto_review_run_lost( + &self, + run_id: Uuid, + reason: &str, + agent_id: Option<&str>, + ) { + self.update_auto_review_store(run_id, |run, now| { + run.agent_id = agent_id.map(str::to_string).or_else(|| run.agent_id.clone()); + run.freshness = AutoReviewFreshness::Lost; + run.cancel_reason = Some(reason.to_string()); + run.mark_status(AutoReviewRunStatus::Lost, now); + }); + } + + fn record_auto_review_launch_in_store( + &self, + run_id: Uuid, + owner_session_id: Uuid, + base_snapshot: Option<&GhostCommit>, + ) { + self.update_auto_review_store(run_id, |run, now| { + run.source = AutoReviewRunSource::Tui; + run.owner_session_id = Some(owner_session_id); + run.base_commit = base_snapshot.map(|base| base.id().to_string()); + run.model = Some(self.config.auto_review_model.clone()); + run.reasoning_effort = Some(self.config.auto_review_model_reasoning_effort.to_string()); + run.freshness = AutoReviewFreshness::Current; + run.mark_status(AutoReviewRunStatus::Snapshotting, now); + run.mark_activity(now); + }); + } + + fn record_auto_review_started_in_store( + &self, + run_id: Uuid, + worktree_path: &std::path::Path, + branch: &str, + agent_id: Option<&str>, + snapshot: Option<&str>, + owner_session_id: Option, + ) { + self.update_auto_review_store(run_id, |run, now| { + run.source = AutoReviewRunSource::Tui; + if let Some(owner_session_id) = owner_session_id { + run.owner_session_id = Some(owner_session_id); + } + run.agent_id = agent_id.map(str::to_string).or_else(|| run.agent_id.clone()); + run.batch_id = (!branch.trim().is_empty()).then(|| branch.to_string()).or_else(|| run.batch_id.clone()); + run.branch = (!branch.trim().is_empty()).then(|| branch.to_string()).or_else(|| run.branch.clone()); + if !worktree_path.as_os_str().is_empty() { + run.worktree_path = Some(worktree_path.to_path_buf()); + } + run.snapshot_commit = snapshot.map(str::to_string).or_else(|| run.snapshot_commit.clone()); + run.model = Some(self.config.auto_review_model.clone()); + run.reasoning_effort = Some(self.config.auto_review_model_reasoning_effort.to_string()); + run.freshness = AutoReviewFreshness::Current; + run.mark_status(AutoReviewRunStatus::Reviewing, now); + run.mark_activity(now); + }); + } + + fn record_auto_review_agent_status_in_store( + &self, + run_id: Uuid, + agent: &code_core::protocol::AgentInfo, + status: AgentStatus, + ) { + let durable_status = Self::durable_auto_review_status(status); + self.update_auto_review_store(run_id, |run, now| { + run.source = AutoReviewRunSource::Tui; + run.agent_id = Some(agent.id.clone()); + run.batch_id = agent.batch_id.clone().or_else(|| run.batch_id.clone()); + run.branch = agent.batch_id.clone().or_else(|| run.branch.clone()); + run.snapshot_commit = agent.worktree_base.clone().or_else(|| run.snapshot_commit.clone()); + run.owner_session_id = agent + .owner_session_id + .as_deref() + .and_then(|id| Uuid::parse_str(id).ok()) + .or(run.owner_session_id); + run.model = agent.model.clone().or_else(|| run.model.clone()); + run.token_count = agent.token_count.or(run.token_count); + run.freshness = if durable_status.is_terminal() { + AutoReviewFreshness::Current + } else if agent + .seconds_since_last_activity + .is_some_and(|seconds| seconds >= AUTO_REVIEW_STALE_SECS) + { + AutoReviewFreshness::Inactive + } else { + AutoReviewFreshness::Current + }; + run.mark_status(durable_status, now); + if !durable_status.is_terminal() || agent.last_activity_at.is_some() { + run.mark_activity(now); + } + if let Some(error) = agent.error.as_deref() { + let classification = Self::classify_auto_review_error(error); + run.error_class = Some(classification.failure_class.to_string()); + run.error_summary = Some(Self::summarize_auto_review_error(error)); + } + }); + } + + fn record_auto_review_completion_in_store(&self, completion: AutoReviewStoreCompletion<'_>) { + self.update_auto_review_store(completion.run_id, |run, now| { + run.source = AutoReviewRunSource::Tui; + run.owner_session_id = completion.owner_session_id.or(run.owner_session_id); + run.agent_id = completion.agent_id.map(str::to_string).or_else(|| run.agent_id.clone()); + run.batch_id = (!completion.branch.trim().is_empty()) + .then(|| completion.branch.to_string()) + .or_else(|| run.batch_id.clone()); + run.branch = (!completion.branch.trim().is_empty()) + .then(|| completion.branch.to_string()) + .or_else(|| run.branch.clone()); + if !completion.worktree_path.as_os_str().is_empty() { + run.worktree_path = Some(completion.worktree_path.to_path_buf()); + } + run.snapshot_commit = completion + .snapshot + .map(str::to_string) + .or_else(|| run.snapshot_commit.clone()); + run.finding_count = completion.findings; + run.summary_digest = completion + .summary + .map(|summary| Self::summarize_plain_review_text(summary, 240)); + run.freshness = AutoReviewFreshness::Current; + if let Some(error) = completion.error { + let classification = Self::classify_auto_review_error(error); + run.error_class = Some(classification.failure_class.to_string()); + run.error_summary = Some(Self::summarize_auto_review_error(error)); + run.mark_status(AutoReviewRunStatus::Failed, now); + } else if completion.snapshot.is_none() && !completion.has_findings { + run.mark_status(AutoReviewRunStatus::Skipped, now); + } else { + run.mark_status(AutoReviewRunStatus::Completed, now); + } + }); + + if let Some(output) = completion.output { + match AutoReviewRunStore::open(&self.config.cwd) { + Ok(mut store) => { + if let Err(err) = store.write_output(completion.run_id, output) { + tracing::warn!(?err, run_id = %completion.run_id, "failed to persist auto-review output"); + } + } + Err(err) => { + tracing::warn!(?err, run_id = %completion.run_id, "failed to open auto-review run store for output"); + } + } + } + } + fn summarize_plain_review_text(text: &str, max_chars: usize) -> String { let mut line = text .lines() @@ -41612,11 +41985,19 @@ impl ChatWidget<'_> { if let Some(state) = self.background_review.as_mut() { state.worktree_path = worktree_path.clone(); state.branch = branch.clone(); - state.agent_id = agent_id; - state.snapshot = snapshot; + state.agent_id = agent_id.clone(); + state.snapshot = snapshot.clone(); state.owner_session_id = owner_session_id; state.last_seen = Instant::now(); } + self.record_auto_review_started_in_store( + run_id, + &worktree_path, + &branch, + agent_id.as_deref(), + snapshot.as_deref(), + owner_session_id, + ); let detail = auto_review_running_detail(self.background_review.as_ref(), None, None); if let Some(status) = self.auto_review_status.clone() { if status.status == AutoReviewIndicatorStatus::Running { @@ -41790,6 +42171,19 @@ impl ChatWidget<'_> { resolved_worktree_path.display().to_string() }; let errored = error.is_some(); + self.record_auto_review_completion_in_store(AutoReviewStoreCompletion { + run_id, + branch: &resolved_branch, + worktree_path: &resolved_worktree_path, + has_findings, + findings: effective_findings, + summary: summary.as_deref(), + error: error.as_deref(), + agent_id: agent_id.as_deref(), + snapshot: inflight_snapshot.as_deref(), + owner_session_id, + output: None, + }); let notice_line = has_findings.then(|| { Self::auto_review_notice_line( &worktree_label,