From c5ea582c7a8e860361b576ca69a91d0774858ff3 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Thu, 15 Oct 2026 11:30:02 +0700 Subject: [PATCH] refactor(review): prepare / judge / finalize with the LLM as the seam (#649) review::prepare builds a judge::Packet, a Judge returns findings (HttpJudge = today's LLM call, NoJudge = deterministic only), and review::review_with merges, guards paths, post-processes and decides should_block. The pipeline now has end-to-end tests with a fake judge. Spinners move out of the llm module into progress::Spinner. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: ajianaz --- CHANGELOG.md | 2 + src/commands/scan.rs | 9 +- src/engine/judge.rs | 107 ++++++++++++++++ src/engine/llm/mod.rs | 69 ++-------- src/engine/mod.rs | 1 + src/engine/review.rs | 285 +++++++++++++++++++++++++++++++++--------- src/progress.rs | 34 +++++ 7 files changed, 386 insertions(+), 121 deletions(-) create mode 100644 src/engine/judge.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cce3b7c..f33880f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Review is now prepare / judge / finalize with the LLM as the seam (#649).** `review::prepare` turns a diff into a `judge::Packet` (judge-visible text, assembled context, system prompt override, `valid_files`, diff chunks, deterministic findings); a `judge::Judge` returns findings (`HttpJudge` = today's LLM call, `NoJudge` = deterministic only); `review::review_with` merges, guards file paths, post-processes and decides `should_block`. Output of `cora review` is unchanged. The review pipeline now has an end-to-end test with a fake judge (no network), and the `llm` module no longer prints: spinners moved to `progress::Spinner` and `llm::review_diff` / `llm::scan_files` take an events observer like the stream variant. + - **One `finalize` module owns the result tail of review, scan, commit and MCP (#648).** `engine::finalize` (`shape`, `shape_issues`, `blocks`, `exit_code`, `save_debt`, `save_history`) replaces six hand-copied versions of severity filtering, `should_block`, quality gate, exit code, debt snapshot and review history. Behavior of `cora review` (single and chunked) and `cora scan` is unchanged. Fixed along the way: the MCP `cora.review_diff` tool now honors its `min_severity` parameter and evaluates the quality gate on the reported findings; `cora commit` now records its review in the review history (`command = "commit"`). ## [0.18.0] - 2026-10-09 diff --git a/src/commands/scan.rs b/src/commands/scan.rs index b2d9352..3fe674c 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -179,7 +179,8 @@ pub async fn execute_scan( eprintln!(" Reviewing{batch_label}…"); - match crate::engine::llm::scan_files( + let spinner = crate::progress::Spinner::new("Scanning files…", false); + let scanned = crate::engine::llm::scan_files( llm_config, &files_content, &effective_focus, @@ -187,9 +188,11 @@ pub async fn execute_scan( &config.response_format, None, brain_ctx.as_deref(), + &spinner, ) - .await - { + .await; + spinner.finish(); + match scanned { Ok((issues, _summary, tokens)) => { all_issues.extend(issues); total_tokens = match (total_tokens, tokens) { diff --git a/src/engine/judge.rs b/src/engine/judge.rs new file mode 100644 index 0000000..f18242e --- /dev/null +++ b/src/engine/judge.rs @@ -0,0 +1,107 @@ +//! The seam between *preparing* a review and *judging* it (#649). +//! +//! A review is three steps: +//! +//! 1. [`review::prepare`](crate::engine::review::prepare) turns a diff into a +//! [`Packet`]: everything a judge needs (the text it should see, the +//! assembled context, the system prompt override) plus everything the +//! finishing step needs (`valid_files`, the diff chunks, the deterministic +//! findings). +//! 2. A [`Judge`] reads the packet and returns findings. This is the seam. +//! 3. [`review::review_with`](crate::engine::review::review_with) merges the +//! deterministic findings, guards the judge's file paths, post-processes, +//! and decides `should_block`. +//! +//! Adapters at the seam: [`HttpJudge`] (an OpenAI-compatible LLM, today's +//! behavior), [`NoJudge`] (deterministic checks only, no LLM), and, later, an +//! agent that judges for itself. A fake `Judge` makes the whole pipeline +//! testable without a network. + +use crate::config::schema::Config; +use crate::engine::deterministic::DeterministicReport; +use crate::engine::diff_parser::FileChunk; +use crate::engine::llm; +use crate::engine::types::{LLMConfig, ReviewResponse}; +use crate::error::CoraError; + +/// Everything about one review that does not depend on who judges it. +#[derive(Debug)] +pub struct Packet { + /// The diff text a judge sees (comment-sanitized when configured). + pub diff_text: String, + /// Assembled prompt context (static analysis, deterministic findings, + /// cross-file, language, profile, memory, brain), if any. + pub context: Option, + /// Resolved system prompt override, if any. + pub system_prompt: Option, + /// File paths present in the diff; a judge may only report on these. + pub valid_files: Vec, + /// Parsed diff of the ORIGINAL (unsanitized) text. + pub chunks: Vec, + /// Findings of every deterministic scanner family. + pub deterministic: DeterministicReport, +} + +/// Something that turns a [`Packet`] into findings. +pub trait Judge { + async fn judge(&self, config: &Config, packet: &Packet) -> Result; +} + +/// Judge backed by an OpenAI-compatible LLM. +pub struct HttpJudge<'a> { + pub llm_config: &'a LLMConfig, + /// Stream tokens to stdout as they arrive. + pub stream: bool, + /// No spinner (ignored when streaming). + pub quiet: bool, +} + +impl Judge for HttpJudge<'_> { + async fn judge(&self, config: &Config, packet: &Packet) -> Result { + if self.stream { + return llm::review_diff_stream( + self.llm_config, + &packet.diff_text, + &config.focus, + &config.rules, + &config.response_format, + packet.system_prompt.as_deref(), + packet.context.as_deref(), + &crate::progress::StdoutStream, + ) + .await + .inspect(|_| println!()); // trailing newline after streamed output + } + let spinner = crate::progress::Spinner::new("Reviewing diff…", self.quiet); + let result = llm::review_diff( + self.llm_config, + &packet.diff_text, + &config.focus, + &config.rules, + &config.response_format, + packet.system_prompt.as_deref(), + packet.context.as_deref(), + &spinner, + ) + .await; + spinner.finish(); + result + } +} + +/// Judge that finds nothing: the review is the deterministic findings only. +// Selected by `cora review --no-llm` / a missing API key (#646). +#[allow(dead_code)] +pub struct NoJudge; + +impl Judge for NoJudge { + async fn judge(&self, _config: &Config, _packet: &Packet) -> Result { + Ok(ReviewResponse { + issues: Vec::new(), + summary: "Deterministic checks only (no LLM judge).".to_string(), + tokens_used: None, + should_block: false, + dropped_findings: 0, + }) + } +} diff --git a/src/engine/llm/mod.rs b/src/engine/llm/mod.rs index 7dd4797..016f5cc 100644 --- a/src/engine/llm/mod.rs +++ b/src/engine/llm/mod.rs @@ -20,8 +20,6 @@ mod prompts; mod repair; mod transport; -use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle}; - use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, TokenUsage}; use crate::error::CoraError; @@ -56,42 +54,6 @@ pub trait LlmEvents: Sync { pub struct NoEvents; impl LlmEvents for NoEvents {} -/// Spinner-backed events for interactive non-streaming runs. -struct SpinnerEvents(ProgressBar); - -impl LlmEvents for SpinnerEvents { - fn status(&self, msg: &str) { - self.0.set_message(msg.to_string()); - } -} - -/// Create an animated spinner for LLM operations. -/// -/// Automatically hidden when stderr is not a TTY (piped/redirected), -/// preventing ANSI pollution in captured output. -fn create_spinner(message: &str) -> ProgressBar { - let spinner = ProgressBar::new_spinner(); - // Hide spinner when stderr is not a terminal (piped/redirected) - if !atty_check() { - spinner.set_draw_target(ProgressDrawTarget::hidden()); - return spinner; - } - spinner.enable_steady_tick(std::time::Duration::from_millis(80)); - spinner.set_style( - ProgressStyle::with_template("{spinner:.cyan} {msg}") - .expect("valid spinner template") - .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈ "), - ); - spinner.set_message(message.to_string()); - spinner -} - -/// Check if stderr is connected to a TTY. -fn atty_check() -> bool { - use std::io::IsTerminal; - std::io::stderr().is_terminal() -} - /// Raw chat completion — returns the raw string response. /// Used by commit message generation and other non-review tasks. /// @@ -182,6 +144,8 @@ async fn review_with( } /// Review a diff using the LLM. Returns a `ReviewResponse`. +/// +/// Status text goes to `events`; the caller decides whether that is a spinner. #[allow(clippy::too_many_arguments)] pub async fn review_diff( llm_config: &LLMConfig, @@ -190,23 +154,18 @@ pub async fn review_diff( rules: &[String], response_format: &str, system_prompt_override: Option<&str>, - quiet: bool, static_context: Option<&str>, + events: &dyn LlmEvents, ) -> std::result::Result { - let events = SpinnerEvents(if quiet { - ProgressBar::hidden() - } else { - create_spinner("Reviewing diff…") - }); let transport = HttpTransport { config: llm_config, response_format, stream: false, - events: &events, + events, }; - let result = review_with( + review_with( &transport, - &events, + events, llm_config.max_tokens, diff, focus, @@ -214,9 +173,7 @@ pub async fn review_diff( system_prompt_override, static_context, ) - .await; - events.0.finish_and_clear(); - result + .await } /// Review a diff using the LLM with streaming. Returns a `ReviewResponse`. @@ -297,17 +254,17 @@ pub async fn scan_files( response_format: &str, system_prompt_override: Option<&str>, brain_context: Option<&str>, + events: &dyn LlmEvents, ) -> std::result::Result<(Vec, Option, Option), CoraError> { - let events = SpinnerEvents(create_spinner("Scanning files…")); let transport = HttpTransport { config: llm_config, response_format, stream: false, - events: &events, + events, }; - let result = scan_with( + scan_with( &transport, - &events, + events, llm_config.max_tokens, files_content, focus, @@ -315,9 +272,7 @@ pub async fn scan_files( system_prompt_override, brain_context, ) - .await; - events.0.finish_and_clear(); - result + .await } #[cfg(test)] diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 123574f..f76854c 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -11,6 +11,7 @@ pub mod finalize; pub mod index_bridge; pub mod index_scanner; pub mod inline_suppress; +pub mod judge; pub mod language_analyzer; pub mod llm; pub mod markdown; diff --git a/src/engine/review.rs b/src/engine/review.rs index eae1248..73f982d 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -2,6 +2,8 @@ use crate::error::CoraError; use tracing::{debug, instrument}; use crate::config::schema::Config; +use crate::engine::index_bridge::IndexBridge; +use crate::engine::judge::{HttpJudge, Judge, Packet}; use crate::engine::llm; use crate::engine::postprocess::{Source, postprocess_report}; use crate::engine::types::{LLMConfig, ReviewResponse}; @@ -116,11 +118,54 @@ async fn review_diff_inner( } } + // Index bridge: one project handle, rooted via resolve_project_root, shared + // by every index-backed step so a run from a subdirectory agrees with + // `cora index` (#566). + let index_bridge = crate::engine::index_bridge::IndexBridge::open_cwd(); + + let packet = prepare(config, diff, memory_context, &index_bridge); + let judge = HttpJudge { + llm_config, + stream, + quiet, + }; + let response = review_with(&judge, config, packet).await?; + + // Save fully-processed response to cache (after filtering) + if use_cache { + if let Err(e) = crate::engine::cache::save_cached_review( + diff, + &llm_config.model, + llm_config.temperature, + &response, + &llm_config.provider, + &llm_config.base_url, + ) { + debug!("failed to save review to cache: {}", e); + } + } + + Ok(response) +} + +/// Step 1 of a review: turn `diff` into a [`Packet`]. +/// +/// Runs every deterministic check, assembles the prompt context, and records +/// what the finishing step needs. Deterministic scanners always operate on the +/// ORIGINAL unsanitized diff; only the text a judge sees is sanitized (ALIBI +/// defense, arXiv:2607.24964). No judge is involved and nothing touches the +/// network. +pub fn prepare( + config: &Config, + diff: &str, + memory_context: Option<&str>, + index_bridge: &IndexBridge, +) -> Packet { // Extract valid file paths for post-parse filtering let valid_files = llm::extract_file_paths_from_diff(diff); // Resolve custom system prompt for review - let review_prompt = resolve_system_prompt( + let system_prompt = resolve_system_prompt( config.review_system_prompt_override.as_deref(), config.review_system_prompt_file.as_deref(), ); @@ -129,11 +174,8 @@ async fn review_diff_inner( let static_context = crate::engine::static_analysis::collect_static_context(diff, &config.static_analysis); - // Parse diff and run rule engine. Deterministic scanners (rules, secrets, - // security) always operate on the ORIGINAL unsanitized diff — only the - // LLM sees sanitized text (ALIBI defense, arXiv:2607.24964). let diff_chunks = crate::engine::diff_parser::parse_diff(diff); - let review_diff_text: std::borrow::Cow<'_, str> = if config.sanitize_comments { + let diff_text = if config.sanitize_comments { let mut sanitized_chunks = crate::engine::diff_parser::parse_diff(diff); let full_report = crate::engine::comment_sanitizer::sanitize_chunks(&mut sanitized_chunks); let rendered = crate::engine::comment_sanitizer::render_sanitized_diff(&sanitized_chunks); @@ -143,18 +185,14 @@ async fn review_diff_inner( "ALIBI comment defense applied" ); if rendered.is_empty() { - std::borrow::Cow::Borrowed(diff) + diff.to_string() } else { - std::borrow::Cow::Owned(rendered) + rendered } } else { - std::borrow::Cow::Borrowed(diff) + diff.to_string() }; - // Index bridge: one project handle, rooted via resolve_project_root, shared - // by every index-backed step so a run from a subdirectory agrees with - // `cora index` (#566). - let index_bridge = crate::engine::index_bridge::IndexBridge::open_cwd(); let project_root = if index_bridge.root().as_os_str().is_empty() { std::env::current_dir().unwrap_or_default() } else { @@ -164,7 +202,7 @@ async fn review_diff_inner( // All deterministic checks (rules, secrets, security, index scans) on the // ORIGINAL diff — no LLM involved. Context = static analysis + claim // warning + one block per scanner family. - let deterministic = crate::engine::deterministic::run(&diff_chunks, config, &index_bridge); + let deterministic = crate::engine::deterministic::run(&diff_chunks, config, index_bridge); if !deterministic.claims.suspicious_claims.is_empty() && !config.sanitize_comments { debug!( claims = deterministic.claims.suspicious_claims.len(), @@ -230,11 +268,11 @@ async fn review_diff_inner( // ── Brain enrichment phase (Tier 1) ────────────────────────────────── // When use_brain is enabled and an index exists, enrich the review prompt // with impact analysis, affected tests, and semantic pattern search. - let final_context = if config.context_chain.use_brain { + let context = if config.context_chain.use_brain { match build_brain_context( &diff_chunks, config.context_chain.impact_depth, - &index_bridge, + index_bridge, ) { Some(brain_ctx) if !brain_ctx.is_empty() => { debug!( @@ -252,38 +290,41 @@ async fn review_diff_inner( } } else { final_context - }; // but preserve deterministic rule findings even on LLM failure - let llm_result: Result = if stream { - llm::review_diff_stream( - llm_config, - &review_diff_text, - &config.focus, - &config.rules, - &config.response_format, - review_prompt.as_deref(), - final_context.as_deref(), - &crate::progress::StdoutStream, - ) - .await - .inspect(|_| println!()) // trailing newline after streamed output - } else { - llm::review_diff( - llm_config, - &review_diff_text, - &config.focus, - &config.rules, - &config.response_format, - review_prompt.as_deref(), - quiet, - final_context.as_deref(), - ) - .await }; - let mut response = match llm_result { + Packet { + diff_text, + context, + system_prompt, + valid_files, + chunks: diff_chunks, + deterministic, + } +} + +/// Steps 2 and 3 of a review: ask `judge` for findings, then merge the +/// deterministic findings, guard file paths, post-process, and decide +/// `should_block`. +/// +/// If the judge fails but the deterministic scanners found something, that is +/// returned instead of an error (it must not be silently swallowed). +pub async fn review_with( + judge: &J, + config: &Config, + packet: Packet, +) -> std::result::Result { + let judged = judge.judge(config, &packet).await; + let Packet { + valid_files, + chunks, + deterministic, + .. + } = packet; + + let mut response = match judged { Ok(resp) => resp, Err(e) => { - // LLM failed — return deterministic findings only (don't silently swallow them) + // Judge failed — return deterministic findings only if !deterministic.is_empty() { let n_rules = deterministic.rules.len(); let n_secrets = deterministic.secrets.len(); @@ -311,7 +352,7 @@ async fn review_diff_inner( should_block: false, dropped_findings: 0, }; - let pp = postprocess_report(fallback.issues, &Source::Diff(&diff_chunks), config); + let pp = postprocess_report(fallback.issues, &Source::Diff(&chunks), config); fallback.issues = pp.issues; fallback.dropped_findings = pp.dropped; fallback.should_block = crate::engine::finalize::blocks( @@ -324,7 +365,7 @@ async fn review_diff_inner( } }; - // Merge deterministic findings (rules, secrets, security, index) with LLM issues + // Merge deterministic findings (rules, secrets, security, index) with the judge's issues response.issues = deterministic.merge_into(response.issues); // Filter out issues with invalid file paths (hallucination guard) @@ -346,11 +387,10 @@ async fn review_diff_inner( // Shared post-processing (LLM secret FP cross-check, Markdown code blocks, // ignore.rules, inline `cora-ignore:`, context-line filter), see // `engine::postprocess`. - let pp = postprocess_report(response.issues, &Source::Diff(&diff_chunks), config); + let pp = postprocess_report(response.issues, &Source::Diff(&chunks), config); response.issues = pp.issues; response.dropped_findings = pp.dropped; - // Calculate should_block based on min_severity response.should_block = crate::engine::finalize::blocks(&response.issues, config.hook.min_severity_level()); @@ -360,20 +400,6 @@ async fn review_diff_inner( "review complete" ); - // Save fully-processed response to cache (after filtering) - if use_cache { - if let Err(e) = crate::engine::cache::save_cached_review( - diff, - &llm_config.model, - llm_config.temperature, - &response, - &llm_config.provider, - &llm_config.base_url, - ) { - debug!("failed to save review to cache: {}", e); - } - } - Ok(response) } @@ -662,4 +688,141 @@ mod tests { "system_prompt_file outside project root should be rejected" ); } + + // ─── #649: the judge seam ─── + + use crate::engine::judge::NoJudge; + use crate::engine::types::{ReviewIssue, Severity}; + + /// Test judge: returns canned issues or a canned error. + struct FakeJudge(Result, String>); + + impl Judge for FakeJudge { + async fn judge( + &self, + _config: &Config, + _packet: &Packet, + ) -> std::result::Result { + match &self.0 { + Ok(issues) => Ok(ReviewResponse { + issues: issues.clone(), + summary: "fake".to_string(), + tokens_used: None, + should_block: false, + dropped_findings: 0, + }), + Err(msg) => Err(CoraError::LlmParse(msg.clone())), + } + } + } + + const SECRET_DIFF: &str = "diff --git a/app.py b/app.py\n\ +new file mode 100644\n\ +--- /dev/null\n\ ++++ b/app.py\n\ +@@ -0,0 +1,2 @@\n\ ++import os\n\ ++password = \"hunter2hunter2\"\n"; + + const CLEAN_DIFF: &str = "diff --git a/app.py b/app.py\n\ +new file mode 100644\n\ +--- /dev/null\n\ ++++ b/app.py\n\ +@@ -0,0 +1,2 @@\n\ ++def add(a, b):\n\ ++ return a + b\n"; + + fn packet(diff: &str, config: &Config) -> Packet { + prepare(config, diff, None, &IndexBridge::unavailable()) + } + + fn judged(file: &str, line: u32, severity: Severity) -> ReviewIssue { + ReviewIssue::new(file, Some(line), severity, "judge finding") + .with_type("bug") + .with_body("body") + } + + fn run( + judge: &J, + config: &Config, + packet: Packet, + ) -> std::result::Result { + tokio::runtime::Builder::new_current_thread() + .build() + .expect("runtime") + .block_on(review_with(judge, config, packet)) + } + + #[test] + fn prepare_records_valid_files_and_deterministic_findings() { + let config = Config::default(); + let p = packet(SECRET_DIFF, &config); + assert_eq!(p.valid_files, vec!["app.py".to_string()]); + assert!(!p.deterministic.is_empty(), "secret should be detected"); + assert!(p.diff_text.contains("hunter2hunter2")); + } + + #[test] + fn judge_findings_are_merged_with_deterministic_ones() { + let config = Config::default(); + let judge = FakeJudge(Ok(vec![judged("app.py", 1, Severity::Minor)])); + let resp = run(&judge, &config, packet(SECRET_DIFF, &config)).expect("review"); + assert!(resp.issues.iter().any(|i| i.title == "judge finding")); + assert!( + resp.issues.iter().any(|i| i.rule_id.is_some()), + "deterministic finding must survive the merge" + ); + } + + #[test] + fn findings_for_files_outside_the_diff_are_dropped() { + let config = Config::default(); + let judge = FakeJudge(Ok(vec![judged( + "src/not_in_diff.rs", + 3, + Severity::Critical, + )])); + let resp = run(&judge, &config, packet(CLEAN_DIFF, &config)).expect("review"); + assert!(resp.issues.is_empty()); + assert!(!resp.should_block); + } + + #[test] + fn should_block_follows_hook_min_severity() { + let mut config = Config::default(); + config.hook.min_severity = "major".to_string(); + let minor = FakeJudge(Ok(vec![judged("app.py", 1, Severity::Minor)])); + let resp = run(&minor, &config, packet(CLEAN_DIFF, &config)).expect("review"); + assert!(!resp.should_block); + + let major = FakeJudge(Ok(vec![judged("app.py", 1, Severity::Major)])); + let resp = run(&major, &config, packet(CLEAN_DIFF, &config)).expect("review"); + assert!(resp.should_block); + } + + #[test] + fn judge_failure_keeps_deterministic_findings() { + let config = Config::default(); + let judge = FakeJudge(Err("boom".to_string())); + let resp = run(&judge, &config, packet(SECRET_DIFF, &config)).expect("fallback"); + assert!(resp.summary.contains("LLM review failed")); + assert!(resp.issues.iter().any(|i| i.rule_id.is_some())); + } + + #[test] + fn judge_failure_without_deterministic_findings_is_an_error() { + let config = Config::default(); + let judge = FakeJudge(Err("boom".to_string())); + assert!(run(&judge, &config, packet(CLEAN_DIFF, &config)).is_err()); + } + + #[test] + fn no_judge_returns_deterministic_findings_only() { + let config = Config::default(); + let resp = run(&NoJudge, &config, packet(SECRET_DIFF, &config)).expect("review"); + assert!(!resp.issues.is_empty()); + assert!(resp.issues.iter().all(|i| i.rule_id.is_some())); + assert!(resp.summary.contains("Deterministic")); + assert!(resp.tokens_used.is_none()); + } } diff --git a/src/progress.rs b/src/progress.rs index a8e57df..bebb2bb 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -203,6 +203,40 @@ pub fn diff_stats(diff: &str) -> (usize, usize) { (files_changed, lines_changed) } +/// Spinner-backed [`LlmEvents`](crate::engine::llm::LlmEvents) for interactive +/// non-streaming runs. Hidden when `quiet` or when stderr is not a TTY, so +/// piped output stays free of ANSI. +pub struct Spinner(indicatif::ProgressBar); + +impl Spinner { + pub fn new(message: &str, quiet: bool) -> Self { + use std::io::IsTerminal; + if quiet || !std::io::stderr().is_terminal() { + return Self(indicatif::ProgressBar::hidden()); + } + let spinner = indicatif::ProgressBar::new_spinner(); + spinner.enable_steady_tick(std::time::Duration::from_millis(80)); + spinner.set_style( + indicatif::ProgressStyle::with_template("{spinner:.cyan} {msg}") + .expect("valid spinner template") + .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈ "), + ); + spinner.set_message(message.to_string()); + Self(spinner) + } + + /// Remove the spinner from the terminal. + pub fn finish(&self) { + self.0.finish_and_clear(); + } +} + +impl crate::engine::llm::LlmEvents for Spinner { + fn status(&self, msg: &str) { + self.0.set_message(msg.to_string()); + } +} + /// [`LlmEvents`](crate::engine::llm::LlmEvents) that streams model output to /// stdout as it arrives. The LLM layer itself never prints; commands that want /// live token output (`review --stream`, `commit --stream`) pass this in.