diff --git a/crates/hi-agent/src/agent/turn.rs b/crates/hi-agent/src/agent/turn.rs index caa30947..257756f6 100644 --- a/crates/hi-agent/src/agent/turn.rs +++ b/crates/hi-agent/src/agent/turn.rs @@ -33,9 +33,11 @@ use crate::steering::{ CONCRETE_REVIEW_NUDGE, EvidenceTracker, GAP_SEARCH_OVERCLAIM_NUDGE, IMPLEMENTATION_EMPTY_TUI_NUDGE, IMPLEMENTATION_NO_CHANGES_NUDGE, IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE, ImplementationIntent, ImplementationTracker, - MutationRecovery, POST_TOOL_EMPTY_RESPONSE_NUDGE, READ_AFTER_SEARCH_NUDGE, - READ_ONLY_SAFE_CONTEXT_WINDOW, REPEAT_NUDGE, REREAD_NUDGE, ReviewIntent, ReviewRepairMode, - SECURITY_BROAD_SEARCH_NUDGE, SECURITY_SCOPE_NUDGE, TOOL_PROTOCOL_RETRY_NUDGE, + BOOKKEEPING_REPOST_NUDGE, MutationRecovery, PLAN_REPOST_NUDGE, + POST_TOOL_EMPTY_RESPONSE_NUDGE, READ_AFTER_SEARCH_NUDGE, READ_ONLY_SAFE_CONTEXT_WINDOW, + REPEAT_NUDGE, REREAD_NUDGE, ReviewIntent, ReviewRepairMode, SECURITY_BROAD_SEARCH_NUDGE, + SECURITY_SCOPE_NUDGE, SKIPPED_BOOKKEEPING_REPOST_RESULT, SKIPPED_PLAN_REPOST_RESULT, + SKIPPED_REPEATED_CALL_RESULT, TOOL_PROTOCOL_RETRY_NUDGE, TOOL_PROTOCOL_TEXT_FALLBACK_NUDGE, ToolLoopGuardrail, WAIT_POLL_STATIC_NUDGE, active_read_only_inspection_cap, answer_says_insufficient_evidence, bash_call_waits, bash_command, bash_no_progress_signature, classify_bash_command, @@ -307,6 +309,26 @@ fn task_needs_repository_context(task: &str, contract: &TaskContract) -> bool { ".py", ".rs", ".ts", + // Comprehension/orientation markers. Omitting these caused a live + // regression: "what does this program do" matched no marker, so the + // turn ran with NO task context index — a repo-blind model has + // nothing to anchor on and (observed across two different models) + // falls back to re-posting its plan instead of exploring. Questions + // about "this program/project" are exactly the tasks that need the + // repository map most. + " program", + " project", + " codebase", + " architecture", + " explain", + " describe", + " overview", + " understand", + " summarize", + " what ", + " how ", + " where ", + " why ", ] .iter() .any(|marker| lower.contains(marker)) @@ -1226,6 +1248,21 @@ impl crate::Agent { let mut text_tool_fallback_next = false; let mut force_text_answer_next = false; let mut force_no_progress_final_answer_next = false; + // After a bookkeeping-repost nudge, withhold the bookkeeping tools + // (`update_plan`, `record_decision`) from the next request's tool + // list. A bookkeeping-fixated model (observed live) keeps re-posting + // meta-work through every nudge — and when only `update_plan` was + // withheld it slid to repeating `record_decision` instead. Clear + // feedback alone doesn't break the loop; removing the whole family + // for one round forces a tool that does real work. + let mut suppress_bookkeeping_tools_next = false; + // Consecutive rounds skipped by the repeat guard, driving recovery + // sampling: a model re-emitting the identical call each round is stuck + // in a token-level loop that only hotter sampling breaks. Resets as + // soon as the model issues a different round, so later rounds run at + // the configured sampling again (unlike the cumulative + // `repeat_nudges` budget, which never resets within a turn). + let mut repeat_sampling_rounds = 0u32; let mut tool_guardrail = ToolLoopGuardrail::default(); // Whether the turn ended because the model kept re-issuing the exact // same tool call through the whole repeat-nudge budget (drives the @@ -1291,11 +1328,23 @@ impl crate::Agent { // `empty_retries` resets on real output, so a normal round runs at // the configured sampling. Toggleable via HI_RECOVERY_SAMPLING for // A/B-ing on the eval harness. - let sampling_retries = empty_retries.max(retry_state.protocol_retries); - let sampling_budget = if retry_state.protocol_retries > empty_retries { - MAX_TOOL_PROTOCOL_RETRIES + let sampling_retries = empty_retries + .max(retry_state.protocol_retries) + .max(repeat_sampling_rounds); + let (sampling_mode, sampling_budget) = if repeat_sampling_rounds > 0 + && repeat_sampling_rounds >= empty_retries + && repeat_sampling_rounds >= retry_state.protocol_retries + { + // The model is deterministically re-emitting the same tool + // call round after round (observed live: four byte-identical + // `update_plan` calls despite nudges and withheld tools). + // Hotter sampling + a frequency penalty is what actually + // breaks a token-level loop; nudge text alone doesn't. + (StallMode::Repeat, self.config.max_repeat_nudges) + } else if retry_state.protocol_retries > empty_retries { + (StallMode::Empty, MAX_TOOL_PROTOCOL_RETRIES) } else { - self.config.max_empty_retries + (StallMode::Empty, self.config.max_empty_retries) }; let (temperature, top_p, frequency_penalty) = recovery_sampling( sampling_retries, @@ -1305,11 +1354,9 @@ impl crate::Agent { // Telemetry for the recovery-sampling A/B: emit a concise debug // line only when sampling is actually being changed (recovery on - // and this is a retry), so ordinary runs stay quiet. The empty - // path is the only mode that escalates sampling today; repeat and - // continue nudges re-run at the configured sampling. + // and this is a retry), so ordinary runs stay quiet. if let Some(line) = recovery_telemetry( - StallMode::Empty, + sampling_mode, sampling_retries, sampling_budget, temperature, @@ -1383,7 +1430,22 @@ impl crate::Agent { }; let requested_request_max_tokens = request_max_tokens_override.unwrap_or(self.config.max_tokens); - let request_tools = self.request_tools_for(tool_availability_mode); + let mut request_tools = self.request_tools_for(tool_availability_mode); + if suppress_bookkeeping_tools_next { + suppress_bookkeeping_tools_next = false; + // Only withhold when other tools remain — an empty tool + // list with tool_choice=required would be a provider error. + if request_tools + .iter() + .any(|tool| !hi_tools::is_coordination(&tool.name)) + { + request_tools = request_tools + .iter() + .filter(|tool| !hi_tools::is_coordination(&tool.name)) + .cloned() + .collect(); + } + } advertised_tool_names.extend(request_tools.iter().map(|tool| tool.name.clone())); let request_tool_schema_tokens = estimate_tool_schema_tokens(&request_tools); tool_schema_tokens = tool_schema_tokens.max(request_tool_schema_tokens); @@ -2016,20 +2078,47 @@ impl crate::Agent { let should_skip_for_repeat = is_repeat && (!no_new_after_mutation || repeat_budget_available); if should_skip_for_repeat { - // Record this round's assistant text (the model did emit - // something) before nudging, so the history stays coherent. - // We deliberately do NOT execute the repeated tool calls, so - // strip their `ToolCall` blocks from the recorded message: - // `push_assistant_text_only` is the intentional "calls - // skipped, not executed" path — leaving `tool_use` blocks - // without matching `tool_result` blocks puts the transcript - // in a state most providers reject on the next request. - self.messages - .push_assistant_text_only(std::mem::take(&mut completion.content)); + // We deliberately do NOT execute the repeated tool calls, + // but the calls stay in the transcript, each paired with a + // synthetic result that says why it was skipped. Stripping + // them (as this path once did) left the model's turn as a + // bare placeholder with no result for the call it just + // made — weak models concluded the tool layer was broken + // ("my tool calls aren't producing visible output") and + // gave up instead of correcting course. Pairing every + // skipped `tool_use` with a `tool_result` also keeps the + // transcript in the shape providers require. + let all_plan_reposts = + calls.iter().all(|(_, name, _)| name == "update_plan"); + let all_bookkeeping_reposts = calls + .iter() + .all(|(_, name, _)| hi_tools::is_coordination(name)); + let skip_results: Vec<(String, String)> = calls + .iter() + .map(|(id, name, _)| { + let note = if name == "update_plan" { + SKIPPED_PLAN_REPOST_RESULT + } else if hi_tools::is_coordination(name) { + SKIPPED_BOOKKEEPING_REPOST_RESULT + } else { + SKIPPED_REPEATED_CALL_RESULT + }; + (id.clone(), note.to_string()) + }) + .collect(); + self.messages.push_assistant_with_results( + std::mem::take(&mut completion.content), + skip_results, + ); if repeat_budget_available { repeat_nudges += 1; + repeat_sampling_rounds += 1; stalled_repeating = true; - let stall_reason = if stale_background_handle_call { + let stall_reason = if all_plan_reposts { + "unchanged plan repost" + } else if all_bookkeeping_reposts { + "repeated bookkeeping call" + } else if stale_background_handle_call { "stale background handle" } else if has_no_progress_bash { "semantic no-op bash command" @@ -2043,7 +2132,30 @@ impl crate::Agent { no_progress_signature_for_calls(&calls), ) && !no_new_after_mutation && implementation_intent.is_none(); - let nudge = if stale_background_handle_call { + let nudge = if all_bookkeeping_reposts { + if all_plan_reposts { + ui.nudge(&format!( + "the model re-posted an unchanged plan — withholding \ + bookkeeping tools for a round and nudging it to execute \ + the next step ({repeat_nudges}/{})", + self.config.max_repeat_nudges + )); + } else { + ui.nudge(&format!( + "the model repeated bookkeeping calls without real work — \ + withholding bookkeeping tools for a round \ + ({repeat_nudges}/{})", + self.config.max_repeat_nudges + )); + } + suppress_bookkeeping_tools_next = true; + force_tools_next = true; + if all_plan_reposts { + PLAN_REPOST_NUDGE.to_string() + } else { + BOOKKEEPING_REPOST_NUDGE.to_string() + } + } else if stale_background_handle_call { if has_background_output_poll { ui.nudge(&format!( "the model kept polling stale background process handles — \ @@ -2222,6 +2334,7 @@ If the task is already complete, stop and give your final recap." // waiting on external state is progress-neutral, not evidence // of a loop. stalled_repeating = false; + repeat_sampling_rounds = 0; prev_call_sig = Some(call_sig); prev_added_no_evidence = no_new_evidence && !has_wait_poll_bash; diff --git a/crates/hi-agent/src/heuristics.rs b/crates/hi-agent/src/heuristics.rs index 05e79826..382cc7ad 100644 --- a/crates/hi-agent/src/heuristics.rs +++ b/crates/hi-agent/src/heuristics.rs @@ -639,21 +639,22 @@ pub(crate) fn recovery_sampling( (Some(temperature), Some(0.95), Some(frequency_penalty)) } -/// Which stall mode fired and triggered recovery sampling. The retry counter -/// (`retries`) is shared across the empty-response path — repeat and continue -/// nudges don't currently escalate sampling, so they surface as `mode == …` with -/// `retries == 0` and produce no telemetry line (see `recovery_telemetry`). +/// Which stall mode fired and triggered recovery sampling. /// -/// `Repeat`/`Continue` are modeled but not yet constructed: the plan calls out a -/// separate experiment on whether they should escalate sampling too. They're +/// `Continue` is modeled but not yet constructed: the plan calls out a +/// separate experiment on whether it should escalate sampling too. It's /// kept here so the telemetry shape is fixed when that lands. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum StallMode { /// A content-less/garbled round (`EmptyCompletion`/`MalformedStream`, or no - /// text and no tool calls). The only mode recovery sampling escalates today. + /// text and no tool calls). Empty, - /// The model re-issued the previous round's exact tool calls. - #[allow(dead_code)] + /// The model re-issued the previous round's exact tool calls (or kept + /// cycling seen inspections) and the round was skipped by the repeat + /// guard. Escalates sampling on consecutive skipped rounds — a model + /// re-emitting the identical call is stuck in a token-level loop that + /// nudge text alone doesn't break (observed live: four byte-identical + /// `update_plan` calls in a row at the configured sampling). Repeat, /// The model announced a next step but emitted no tool call to perform it. #[allow(dead_code)] diff --git a/crates/hi-agent/src/steering/constants.rs b/crates/hi-agent/src/steering/constants.rs index df7e2a30..07796727 100644 --- a/crates/hi-agent/src/steering/constants.rs +++ b/crates/hi-agent/src/steering/constants.rs @@ -10,6 +10,51 @@ in the conversation above — running it again will only repeat the same result. now: make the edit it points to, move to the next step, or if the task is already complete, stop \ and give your final recap. Do not re-run the same command."; +/// Synthetic tool result recorded for a call the repeat guard skipped. The +/// skipped call stays in the transcript paired with this result (provider-safe) +/// so the model sees exactly what happened to the call it just made — stripping +/// the call left weak models convinced the tool layer was broken ("my tool +/// calls aren't producing visible output") and they gave up instead of +/// correcting course. +pub(crate) const SKIPPED_REPEATED_CALL_RESULT: &str = "[not executed: this call is identical to \ +the one you made last round. Its result is unchanged and already shown above — act on that result \ +instead of re-issuing the call.]"; + +/// Synthetic tool result for a repeated, unchanged `update_plan` call. Models +/// are told to keep re-posting the plan as statuses change, so an identical +/// re-post is a common weak-model stall: harmless bookkeeping, but zero +/// progress. Point the model at executing the plan instead. +pub(crate) const SKIPPED_PLAN_REPOST_RESULT: &str = "[not executed: this plan is already recorded \ +exactly as posted — re-posting an unchanged plan does nothing. Execute the plan's next step now \ +with your other tools; call update_plan again only when a step's status changes.]"; + +/// Synthetic tool result for a repeated, unchanged bookkeeping call other than +/// `update_plan` (today: `record_decision`). Same stall pattern as the plan +/// re-post: meta-work instead of work. +pub(crate) const SKIPPED_BOOKKEEPING_REPOST_RESULT: &str = "[not executed: this bookkeeping call \ +is already recorded from your previous identical call — recording it again does nothing. Do the \ +actual work now with your repository tools (read, list, grep, bash, edit).]"; + +/// Sent when the model re-posts an identical `update_plan` call instead of +/// working. The generic [`REPEAT_NUDGE`] ("you just ran that exact command… +/// act on its output") reads as nonsense for a bookkeeping call whose output +/// is a one-line ack, and confused models into believing their tools were +/// broken. This names the actual problem and the concrete next action. +pub(crate) const PLAN_REPOST_NUDGE: &str = "You re-posted the same plan without doing any work. \ +The plan is already recorded — do not call update_plan again until a step's status actually \ +changes; bookkeeping tools are unavailable for your next action. Execute the first incomplete \ +plan step now using your other tools (read, list, grep, bash, edit)."; + +/// Sent when the model repeats identical bookkeeping calls (`update_plan`, +/// `record_decision`) instead of working. Observed live: withholding only +/// `update_plan` made the model slide to `record_decision` and repeat that +/// instead — so the nudge (and the one-round tool withholding that accompanies +/// it) covers the whole bookkeeping family. +pub(crate) const BOOKKEEPING_REPOST_NUDGE: &str = "You repeated a bookkeeping call \ +(update_plan/record_decision) that was already recorded, without doing any work. Those records \ +are saved; bookkeeping tools are unavailable for your next action. Do the actual work now: \ +inspect files with read/list/grep, run a command with bash, or make an edit."; + pub(crate) const NO_EVIDENCE_REVIEW_NUDGE: &str = "This read-only review has no inspected evidence yet. \ Do not finalize. Use read-only inspection tools first, then answer from the inspected evidence. \ If inspection is impossible, explain which inspection failed and what remains unknown."; diff --git a/crates/hi-agent/src/tests/turn.rs b/crates/hi-agent/src/tests/turn.rs index dece4343..b8e534bb 100644 --- a/crates/hi-agent/src/tests/turn.rs +++ b/crates/hi-agent/src/tests/turn.rs @@ -417,6 +417,273 @@ async fn nudges_when_model_repeats_the_same_command() { assert!(ui.turn_end.is_some(), "turn completed"); } +#[tokio::test] +async fn repeated_plan_repost_gets_synthetic_result_and_plan_nudge() { + // Regression: a weak model re-posted an identical `update_plan` call right + // after its first one. The repeat guard used to strip the call from the + // transcript (leaving only the "Provider-invisible assistant content" + // placeholder) and send the generic "you just ran that exact command" + // nudge — the model concluded its tool calls weren't being executed and + // gave up without ever exploring. The skipped call must now stay in the + // transcript paired with a synthetic result that says why it was skipped, + // and the nudge must name the actual problem (unchanged plan re-post). + let plan_args = serde_json::json!({ + "steps": [{"title": "Explore project structure", "status": "active"}] + }) + .to_string(); + let plan_call = |id: &str| { + completion( + vec![Content::ToolCall { + id: id.into(), + name: "update_plan".into(), + arguments: plan_args.clone(), + }], + 1, + 1, + ) + }; + let responses = vec![ + plan_call("plan-1"), + plan_call("plan-2"), // byte-identical re-post → skipped with synthetic result + completion( + vec![Content::ToolCall { + id: "plan-3".into(), + name: "update_plan".into(), + arguments: serde_json::json!({ + "steps": [{"title": "Explore project structure", "status": "done"}] + }) + .to_string(), + }], + 1, + 1, + ), + completion(vec![Content::Text("It is a small CLI.".into())], 1, 1), + ]; + let mut agent = agent(responses, config()); + let mut ui = RecUi::default(); + agent.run_turn("check it", &mut ui).await.unwrap(); + + assert_eq!( + ui.statuses + .iter() + .filter(|s| s.contains("re-posted an unchanged plan")) + .count(), + 1, + "plan re-post gets its own nudge, got: {:?}", + ui.statuses + ); + let skipped_result = agent.messages().iter().find_map(|m| { + m.content.iter().find_map(|c| match c { + Content::ToolResult { call_id, output } if call_id == "plan-2" => Some(output.clone()), + _ => None, + }) + }); + let skipped_result = skipped_result.expect("skipped plan re-post has a synthetic tool result"); + assert!( + skipped_result.contains("not executed") && skipped_result.contains("update_plan"), + "synthetic result explains the skip: {skipped_result}" + ); + assert!( + !agent.messages().iter().any(|m| m + .text() + .contains("Provider-invisible assistant content")), + "skipped calls must not degrade to the provider-invisible placeholder" + ); + assert!( + ui.turn_end.is_some(), + "model recovered and finished the turn" + ); + agent.messages.validate_for_provider().unwrap(); +} + +#[tokio::test] +async fn plan_repost_nudge_withholds_update_plan_for_one_round() { + // After a plan-repost nudge, the next request must not offer the + // update_plan tool at all: the plan-fixated model observed live kept + // re-posting the plan through every nudge, so for one round it is forced + // to pick a tool that does real work. The round after that, update_plan + // is available again (legitimate status updates must still work). + let plan_args = serde_json::json!({ + "steps": [{"title": "Explore project structure", "status": "active"}] + }) + .to_string(); + let plan_call = |id: &str| { + completion( + vec![Content::ToolCall { + id: id.into(), + name: "update_plan".into(), + arguments: plan_args.clone(), + }], + 1, + 1, + ) + }; + let tool_names = std::sync::Arc::new(Mutex::new(Vec::new())); + let provider = RecordRequests { + responses: Mutex::new(vec![ + plan_call("plan-1"), + plan_call("plan-2"), // identical re-post → nudged, tool withheld + echo_call(), // real work in the withheld round + completion( + vec![Content::ToolCall { + id: "plan-3".into(), + name: "update_plan".into(), + arguments: serde_json::json!({ + "steps": [{"title": "Explore project structure", "status": "done"}] + }) + .to_string(), + }], + 1, + 1, + ), + completion(vec![Content::Text("It is a small CLI.".into())], 1, 1), + ]), + tool_names: tool_names.clone(), + modes: std::sync::Arc::new(Mutex::new(Vec::new())), + }; + let mut agent = Agent::new(std::sync::Arc::new(provider), config()).unwrap(); + let mut ui = RecUi::default(); + agent.run_turn("check it", &mut ui).await.unwrap(); + + let tool_names = tool_names.lock().unwrap(); + assert!( + tool_names.len() >= 4, + "expected at least four requests, got {}", + tool_names.len() + ); + assert!( + tool_names[1].iter().any(|name| name == "update_plan"), + "update_plan offered before the nudge: {:?}", + tool_names[1] + ); + assert!( + !tool_names[2] + .iter() + .any(|name| hi_tools::is_coordination(name)), + "all bookkeeping tools withheld for the round after the plan-repost nudge: {:?}", + tool_names[2] + ); + assert!( + tool_names[3].iter().any(|name| name == "update_plan"), + "update_plan restored after the withheld round: {:?}", + tool_names[3] + ); + agent.messages.validate_for_provider().unwrap(); +} + +#[tokio::test] +async fn comprehension_question_gets_repository_context() { + // Regression: "what does this program do" matched no marker in + // `task_needs_repository_context`, so the turn ran with NO task context + // index — and a repo-blind model (observed live with two different + // models) stalled re-posting its plan instead of exploring. Orientation + // questions about the program/project must carry the repository index. + let workspace = IsolatedWorkspace::new("comprehension-context"); + std::fs::create_dir_all(workspace.path("src")).unwrap(); + std::fs::write( + workspace.path("src/main.rs"), + "fn main() { println!(\"hi\"); }\n", + ) + .unwrap(); + let read_call = ProviderStep::Completion(completion( + vec![Content::ToolCall { + id: "r1".into(), + name: "read".into(), + arguments: serde_json::json!({"path": "src/main.rs"}).to_string(), + }], + 1, + 1, + )); + let answer = || { + ProviderStep::Completion(completion( + vec![Content::Text( + "src/main.rs is a small CLI that prints hi.".into(), + )], + 1, + 1, + )) + }; + let (mut agent, requests) = scripted_agent( + vec![read_call, answer(), answer(), answer(), answer()], + workspace.config(), + ); + let mut ui = RecUi::default(); + let _ = agent.run_turn("what does this program do", &mut ui).await; + + let requests = requests.lock().unwrap(); + let request_text = requests[0] + .iter() + .map(Message::text) + .collect::>() + .join("\n"); + assert!( + request_text.contains("# Task context index"), + "comprehension questions must carry the repository context index; \ + system prompt was: {}", + &request_text[..request_text.len().min(1500)] + ); + assert!( + request_text.contains("src/main.rs"), + "the index should surface repository files" + ); +} + +#[tokio::test] +async fn repeated_decision_repost_gets_bookkeeping_nudge() { + // The bookkeeping-repost handling covers the whole coordination family: + // when only update_plan was withheld, the plan-fixated model slid to + // repeating record_decision instead (observed live). A repeated identical + // record_decision gets the bookkeeping synthetic result and nudge. + let decision_args = serde_json::json!({ + "summary": "Explore repo first", + "rationale": "Need context", + "files": ["."] + }) + .to_string(); + let decision_call = |id: &str| { + completion( + vec![Content::ToolCall { + id: id.into(), + name: "record_decision".into(), + arguments: decision_args.clone(), + }], + 1, + 1, + ) + }; + let responses = vec![ + decision_call("dec-1"), + decision_call("dec-2"), // identical re-post → bookkeeping nudge + echo_call(), + completion(vec![Content::Text("It is a small CLI.".into())], 1, 1), + ]; + let mut agent = agent(responses, config()); + let mut ui = RecUi::default(); + agent.run_turn("check it", &mut ui).await.unwrap(); + + assert_eq!( + ui.statuses + .iter() + .filter(|s| s.contains("repeated bookkeeping calls")) + .count(), + 1, + "decision re-post gets the bookkeeping nudge, got: {:?}", + ui.statuses + ); + let skipped_result = agent.messages().iter().find_map(|m| { + m.content.iter().find_map(|c| match c { + Content::ToolResult { call_id, output } if call_id == "dec-2" => Some(output.clone()), + _ => None, + }) + }); + let skipped_result = skipped_result.expect("skipped decision re-post has a synthetic result"); + assert!( + skipped_result.contains("not executed") && skipped_result.contains("bookkeeping"), + "synthetic result explains the skip: {skipped_result}" + ); + agent.messages.validate_for_provider().unwrap(); +} + #[tokio::test] async fn wait_poll_with_changing_output_is_not_repeat_nudged() { // The model watches a slow external process by re-running the exact same diff --git a/crates/hi-agent/src/transcript.rs b/crates/hi-agent/src/transcript.rs index ae1c3662..50b4d479 100644 --- a/crates/hi-agent/src/transcript.rs +++ b/crates/hi-agent/src/transcript.rs @@ -410,12 +410,19 @@ impl Transcript { self.make_mut().push(Message::tool_result(call_id, output)); } - /// Record an assistant message whose tool calls were *deliberately not - /// executed* (the repeat guard: the model re-issued the exact same calls, - /// so re-running them would only reproduce the same output). The `ToolCall` - /// blocks are stripped from the recorded content so the transcript never - /// carries `tool_use` blocks without matching `tool_result`s. Text and - /// thinking blocks are kept. + /// Record an assistant message whose tool calls cannot be represented + /// (truncation: the calls are partial/malformed and were never executed; + /// inspection sprawl / unusable forced finals: the round is discarded). + /// The `ToolCall` blocks are stripped from the recorded content so the + /// transcript never carries `tool_use` blocks without matching + /// `tool_result`s. Text and thinking blocks are kept. + /// + /// Note: the repeat guard no longer uses this — a *skipped-but-valid* call + /// is recorded via [`push_assistant_with_results`] with a synthetic + /// "[not executed: …]" result, so the model can see what happened to the + /// call it just made instead of a bare placeholder. + /// + /// [`push_assistant_with_results`]: Self::push_assistant_with_results pub(crate) fn push_assistant_text_only(&mut self, content: Vec) { let mut text_only: Vec = content .into_iter() diff --git a/crates/hi-tools/src/lib.rs b/crates/hi-tools/src/lib.rs index 6706bcfd..c4a45a9b 100644 --- a/crates/hi-tools/src/lib.rs +++ b/crates/hi-tools/src/lib.rs @@ -38,8 +38,9 @@ pub use process::{AdoptableOutcome, ProcessExecution, ProcessRunner, RunningChil pub use tools::{ MINIMAL_TOOL_SPECS, PreparedMutation, TOOL_CATALOG, TOOL_SPECS, ToolCapability, ToolMetadata, commit_in, delegate_tool_spec, execute_in_runtime, execute_prepared_in_runtime, - execute_streaming_in_runtime, explore_tool_spec, fast_check_for, is_filesystem_mutating, - is_known_tool, is_read_only, prepare_mutation_in_with_state, prepare_verify_workdir, + execute_streaming_in_runtime, explore_tool_spec, fast_check_for, is_coordination, + is_filesystem_mutating, is_known_tool, is_read_only, prepare_mutation_in_with_state, + prepare_verify_workdir, run_check_in, run_fast_check_in, target_path, tool_metadata, working_tree_diff_in, working_tree_diff_plain_in, }; diff --git a/crates/hi-tools/src/tools.rs b/crates/hi-tools/src/tools.rs index 65c8d7db..b9be0787 100644 --- a/crates/hi-tools/src/tools.rs +++ b/crates/hi-tools/src/tools.rs @@ -1037,6 +1037,15 @@ pub fn is_filesystem_mutating(name: &str) -> bool { tool_metadata(name).is_some_and(|metadata| metadata.filesystem_mutating) } +/// Whether a tool is pure bookkeeping (`update_plan`, `record_decision`): +/// it records agent-side coordination state and does no work on the task +/// itself. The agent's steering uses this to spot rounds that only shuffle +/// bookkeeping — a weak-model stall pattern — and to withhold these tools for +/// a round when the model fixates on them. +pub fn is_coordination(name: &str) -> bool { + tool_metadata(name).is_some_and(|metadata| metadata.capability == ToolCapability::Coordination) +} + /// Best-effort extraction of the primary target path from a tool call's JSON /// arguments — the `path` field for read/write/edit/list, the `path`/`glob` for /// grep. Returns `None` for tools without a meaningful single path (e.g. diff --git a/crates/hi-tui/src/app/commands.rs b/crates/hi-tui/src/app/commands.rs index 09ed1a66..36863f0f 100644 --- a/crates/hi-tui/src/app/commands.rs +++ b/crates/hi-tui/src/app/commands.rs @@ -146,6 +146,11 @@ impl crate::App { KeyCode::Char('t') if ctrl => { self.show_reasoning = !self.show_reasoning; } + // Toggle full tool-output expansion: long blocks fold to a preview + // by default; Ctrl-O reveals every block's full body (and back). + KeyCode::Char('o') if ctrl => { + self.show_tool_output = !self.show_tool_output; + } KeyCode::Home => self.input.home(), KeyCode::End => self.input.end(), // `?` on an empty input line toggles a keybindings help overlay; @@ -191,7 +196,7 @@ impl crate::App { body.push_str("\n## transcript\n"); for entry in &self.transcript { match entry { - crate::TranscriptEntry::Line(_) => { + crate::TranscriptEntry::Line(_) | crate::TranscriptEntry::ToolOutput { .. } => { body.push_str(&entry.text()); body.push('\n'); } diff --git a/crates/hi-tui/src/app/lifecycle.rs b/crates/hi-tui/src/app/lifecycle.rs index c23b018a..f7b2634b 100644 --- a/crates/hi-tui/src/app/lifecycle.rs +++ b/crates/hi-tui/src/app/lifecycle.rs @@ -49,6 +49,7 @@ impl crate::App { reasoning_buffer: String::new(), reasoning_started: None, show_reasoning: false, + show_tool_output: false, code_lang: None, input: InputLine::default(), following: true, diff --git a/crates/hi-tui/src/app/render.rs b/crates/hi-tui/src/app/render.rs index b58cd492..8f4199eb 100644 --- a/crates/hi-tui/src/app/render.rs +++ b/crates/hi-tui/src/app/render.rs @@ -95,9 +95,10 @@ impl crate::App { let cycle = 2 * (n - 1).max(1); let step = self.spinner % cycle; let lit = if step < n { step } else { cycle - step }; - let gray = Style::default().fg(Color::DarkGray); + let th = crate::theme::theme(); + let gray = Style::default().fg(th.gray_dim); let lit_style = Style::default() - .fg(Color::White) + .fg(th.accent_running) .add_modifier(Modifier::BOLD); chars .iter() @@ -169,7 +170,7 @@ impl crate::App { /// the right edge. pub(crate) fn input_view(&self, width: u16) -> (Vec>, u16, u16) { const MAX_INPUT_ROWS: usize = 10; - const PREFIX: usize = 2; // "› " or " " + const PREFIX: usize = 2; // "❯ " or " " let text = self.input.text(); let before: String = text.chars().take(self.input.cursor()).collect(); let cursor_col_logical = before.chars().rev().take_while(|&c| c != '\n').count(); @@ -236,8 +237,15 @@ impl crate::App { )); } for (i, (chunk, cursor_here)) in wrapped[start..].iter().enumerate() { - let prefix = if i == 0 && !truncated { "› " } else { " " }; - lines.push(Line::from(format!("{prefix}{chunk}"))); + // `❯` on the first line (matching the transcript's prompt echo), + // aligned continuation on the rest. + let first = i == 0 && !truncated; + let prefix_span = if first { + Span::styled("❯ ", Style::default().fg(crate::theme::theme().accent_user)) + } else { + Span::raw(" ") + }; + lines.push(Line::from(vec![prefix_span, Span::raw(chunk.clone())])); if let Some(col) = cursor_here && !found_cursor { @@ -278,19 +286,20 @@ impl crate::App { .iter() .filter(|s| s.status == PlanStatus::Done) .count(); + let th = crate::theme::theme(); let mut out = vec![Line::styled( format!("plan · {done}/{total}"), Style::default() - .fg(Color::Cyan) + .fg(th.accent_plan) .add_modifier(Modifier::BOLD), )]; for s in self.plan.iter().take(max_steps) { let (glyph, glyph_style, title_style) = match s.status { - PlanStatus::Done => ('✓', Style::default().fg(Color::Green), dim()), + PlanStatus::Done => ('✓', Style::default().fg(th.accent_success), dim()), PlanStatus::Active => ( '▸', Style::default() - .fg(Color::Cyan) + .fg(th.accent_plan) .add_modifier(Modifier::BOLD), Style::default().add_modifier(Modifier::BOLD), ), @@ -327,23 +336,24 @@ impl crate::App { header.push_str(" · "); header.push_str(&goal.objective); } + let th = crate::theme::theme(); let mut out = vec![Line::styled( header, Style::default() - .fg(Color::Magenta) + .fg(th.accent_goal) .add_modifier(Modifier::BOLD), )]; for s in goal.sub_goals.iter().take(max_steps) { let (glyph, glyph_style, title_style) = match s.status { - hi_agent::GoalStatus::Done => ('✓', Style::default().fg(Color::Green), dim()), + hi_agent::GoalStatus::Done => ('✓', Style::default().fg(th.accent_success), dim()), hi_agent::GoalStatus::Active => ( '▸', Style::default() - .fg(Color::Magenta) + .fg(th.accent_goal) .add_modifier(Modifier::BOLD), Style::default().add_modifier(Modifier::BOLD), ), - hi_agent::GoalStatus::Failed => ('✗', Style::default().fg(Color::Red), dim()), + hi_agent::GoalStatus::Failed => ('✗', Style::default().fg(th.accent_error), dim()), hi_agent::GoalStatus::Pending => ('○', dim(), Style::default()), }; out.push(Line::from(vec![ @@ -467,10 +477,25 @@ impl crate::App { let rows = Layout::vertical([Constraint::Min(1), Constraint::Length(input_h)]).split(area); // --- Transcript --- - let title = format!(" hi · {} · {} ", self.provider, self.model); - // Right-aligned: durable navigation and context signals only. Detailed - // token usage lives in the observability panel. - let mut info_parts: Vec = Vec::new(); + let th = crate::theme::theme(); + // The title row is the app's status bar: product mark + provider/model + // on the left, goal/context chips on the right. `hi` reads as the brand + // mark (accent), the rest muted so it frames rather than shouts. + let title = Line::from(vec![ + Span::styled( + " hi ", + Style::default() + .fg(th.accent_assistant) + .add_modifier(Modifier::BOLD), + ), + Span::styled( + format!("· {} · {} ", self.provider, self.model), + Style::default().fg(th.text_secondary), + ), + ]); + // Right-aligned chips: durable navigation and context signals only. + // Detailed token usage lives in the observability panel. + let mut info_spans: Vec> = Vec::new(); if let Some(goal) = &self.goal { let total = goal.sub_goals.len(); if total > 0 { @@ -479,21 +504,30 @@ impl crate::App { .iter() .filter(|s| s.status == hi_agent::GoalStatus::Done) .count(); - if goal.paused { - info_parts.push(format!("goal {done}/{total} ⏸")); + let label = if goal.paused { + format!(" goal {done}/{total} ⏸ ") } else { - info_parts.push(format!("goal {done}/{total}")); - } + format!(" goal {done}/{total} ") + }; + info_spans.push(Span::styled(label, Style::default().fg(th.accent_goal))); } } if let Some(pct) = self.context_pct() { - info_parts.push(format!("{pct}% ctx")); + // The context chip warms as it fills: past 80% it's a warning color. + let color = if pct >= 80 { + th.warning + } else { + th.text_secondary + }; + if !info_spans.is_empty() { + info_spans.push(Span::styled("· ", Style::default().fg(th.gray_dim))); + } + info_spans.push(Span::styled( + format!("{pct}% ctx "), + Style::default().fg(color), + )); } - let info = if info_parts.is_empty() { - String::new() - } else { - format!(" {} ", info_parts.join(" · ")) - }; + let info = Line::from(info_spans).right_aligned(); let mut lines: Vec> = Vec::new(); // If older transcript lines have been trimmed, show a marker at the top // so the user knows earlier content scrolled off (it's still in the @@ -502,14 +536,14 @@ impl crate::App { lines.push(Line::styled( format!("↑ {} lines compacted (see session log)", self.trimmed), Style::default() - .fg(Color::DarkGray) + .fg(th.gray_dim) .add_modifier(Modifier::ITALIC), )); } lines.extend( self.transcript .iter() - .flat_map(|e| e.flatten(self.show_reasoning)), + .flat_map(|e| e.flatten(self.show_reasoning, self.show_tool_output)), ); if let Some((style, markdown, text)) = &self.pending { // Style the in-progress line live (headings, bold, code, …) so prose @@ -542,9 +576,9 @@ impl crate::App { let mut block = Block::bordered() .border_type(BorderType::Rounded) - .border_style(dim()) + .border_style(Style::default().fg(th.prompt_border)) .title(title) - .title_top(Line::from(info).right_aligned()); + .title_top(info); // While scrolled up, a bottom-right hint shows how much is below — new // lines that arrived since you left the bottom, else how far down it is. if !self.following { @@ -558,7 +592,7 @@ impl crate::App { Line::from(Span::styled( label, Style::default() - .fg(Color::Cyan) + .fg(th.selection) .add_modifier(Modifier::BOLD), )) .right_aligned(), @@ -806,9 +840,9 @@ impl crate::App { let input_block = Block::bordered() .border_type(BorderType::Rounded) .border_style(if self.working { - Style::default().fg(Color::Cyan) + Style::default().fg(crate::theme::theme().prompt_border_active) } else { - dim() + Style::default().fg(crate::theme::theme().prompt_border) }); let mut ilines: Vec = Vec::new(); @@ -996,6 +1030,7 @@ impl crate::App { ), ("Ctrl-D", "toggle the working-tree diff panel"), ("Ctrl-T", "toggle reasoning (thinking) display"), + ("Ctrl-O", "expand/collapse long tool output"), ("Ctrl-?", "toggle agent observability panel"), ("Ctrl-R", "fuzzy-search input history"), ("PageUp/PageDown", "scroll the transcript"), @@ -1053,11 +1088,10 @@ impl crate::App { ); let mut lead: Vec> = Vec::with_capacity(if is_working_wave { 8 } else { 1 }); + let running = crate::theme::theme().accent_running; lead.push(Span::styled( format!("{frame_ch} "), - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), + Style::default().fg(running).add_modifier(Modifier::BOLD), )); if is_working_wave { lead.extend(self.working_spans()); @@ -1066,17 +1100,13 @@ impl crate::App { if let Some(rest) = activity.strip_prefix("Working") { lead.push(Span::styled( rest.to_string(), - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), + Style::default().fg(running).add_modifier(Modifier::BOLD), )); } } else { lead.push(Span::styled( activity, - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), + Style::default().fg(running).add_modifier(Modifier::BOLD), )); } lead.push(Span::styled(stats.to_string(), Style::default())); @@ -1087,7 +1117,7 @@ impl crate::App { for line in &self.tool_stream_tail { ilines.push(Line::styled( format!(" │ {}", clip_reason(line)), - Style::default().fg(Color::DarkGray), + Style::default().fg(crate::theme::theme().gray_dim), )); } } else { diff --git a/crates/hi-tui/src/app/run.rs b/crates/hi-tui/src/app/run.rs index ec8b24d9..df69dfc7 100644 --- a/crates/hi-tui/src/app/run.rs +++ b/crates/hi-tui/src/app/run.rs @@ -166,7 +166,7 @@ pub async fn run( } app.push(Line::styled( format!( - "Enter to send · Alt-Enter for a newline · Ctrl-C interrupts/double exits · Ctrl-T shows reasoning · Ctrl-D toggles diff · /help for all commands{ctx}.", + "Enter to send · Alt-Enter for a newline · Ctrl-C interrupts/double exits · Ctrl-T shows reasoning · Ctrl-O expands tool output · Ctrl-D toggles diff · /help for all commands{ctx}.", ), dim(), )); @@ -1584,9 +1584,10 @@ pub async fn run( // Mirrors the main turn path so cancellation, // background-process cleanup, and session-state // rewind are handled identically. - app.push(Line::styled( - format!("› {run_line}"), - Style::default().fg(Color::Blue), + app.push(ratatui::text::Line::styled( + format!("❯ {run_line}"), + ratatui::style::Style::default() + .fg(crate::theme::theme().accent_user), )); app.set_working(true); app.follow(); @@ -2106,9 +2107,9 @@ pub async fn run( }; // --- Turn phase: run the agent behind a channel, staying responsive. --- - app.push(Line::styled( - format!("› {run_line}"), - Style::default().fg(Color::Blue), + app.push(ratatui::text::Line::styled( + format!("❯ {run_line}"), + ratatui::style::Style::default().fg(crate::theme::theme().accent_user), )); app.set_working(true); app.follow(); diff --git a/crates/hi-tui/src/app/transcript.rs b/crates/hi-tui/src/app/transcript.rs index e878d24d..59797d16 100644 --- a/crates/hi-tui/src/app/transcript.rs +++ b/crates/hi-tui/src/app/transcript.rs @@ -5,16 +5,45 @@ use std::time::{Duration, Instant}; use ansi_to_tui::IntoText; use hi_agent::ui::tool_label; use hi_agent::{ReviewStatus, TurnOutcome, TurnStatus, TurnStopReason, VerificationStatus}; -use ratatui::style::{Color, Style}; -use ratatui::text::{Line, Text}; +use ratatui::style::{Modifier, Style}; +use ratatui::text::{Line, Span, Text}; use crate::event::UiEvent; -use crate::render::{diff_lines, dim, looks_like_diff, markdown_line}; +use crate::render::{accent_line, diff_lines, dim, gutter, looks_like_diff, markdown_line}; +use crate::theme::theme; use crate::util::fmt_rate_limits; use crate::{ ExploreRun, MAX_EVENT_LOG, MAX_TRANSCRIPT_LINES, TranscriptEntry, TurnEventKind, TurnState, }; +/// Build a tool-call header line: `┃ ◆ verb rest` — the accent gutter and `◆` +/// bullet in the tool color, the leading verb bold, the rest in secondary text. +/// This is the block signature that marks agent machinery at a glance. +fn tool_header(label: &str) -> Line<'static> { + let t = theme(); + let (verb, rest) = match label.split_once(' ') { + Some((v, r)) => (v, r), + None => (label, ""), + }; + let mut spans = vec![ + gutter(t.accent_tool), + Span::styled("◆ ", Style::default().fg(t.accent_tool)), + Span::styled( + verb.to_string(), + Style::default() + .fg(t.text_secondary) + .add_modifier(Modifier::BOLD), + ), + ]; + if !rest.is_empty() { + spans.push(Span::styled( + format!(" {rest}"), + Style::default().fg(t.text_secondary), + )); + } + Line::from(spans) +} + impl crate::App { pub(crate) fn push(&mut self, line: Line<'static>) { self.transcript.push(TranscriptEntry::Line(line)); @@ -47,7 +76,11 @@ impl crate::App { self.status = format!("done · {detail}"); self.last_turn_state = TurnState::Done(detail.clone()); self.last_error = None; - self.push(Line::styled(format!("✓ done · {detail}"), dim())); + self.push(accent_line( + theme().accent_success, + format!("✓ done · {detail}"), + dim(), + )); } OutcomeState::Warning => { let label = match outcome.status { @@ -58,27 +91,30 @@ impl crate::App { self.status = format!("warning · {label}"); self.last_turn_state = TurnState::Warning(label.clone()); self.last_error = Some(label.clone()); - self.push(Line::styled( + self.push(accent_line( + theme().warning, format!("⚠ {label}"), - Style::default().fg(Color::Yellow), + Style::default().fg(theme().warning), )); } OutcomeState::Failed => { self.status = format!("failed · {detail}"); self.last_turn_state = TurnState::Failed(detail.clone()); self.last_error = Some(detail.clone()); - self.push(Line::styled( + self.push(accent_line( + theme().accent_error, format!("✗ failed · {detail}"), - Style::default().fg(Color::Red), + Style::default().fg(theme().accent_error), )); } OutcomeState::Cancelled => { self.status = "cancelled".to_string(); self.last_turn_state = TurnState::Cancelled; self.last_error = None; - self.push(Line::styled( + self.push(accent_line( + theme().warning, "⚠ cancelled", - Style::default().fg(Color::Yellow), + Style::default().fg(theme().warning), )); } } @@ -97,18 +133,20 @@ impl crate::App { let limits = fmt_rate_limits(self.rate_limits) .map(|limits| format!("\n {limits}")) .unwrap_or_default(); - self.push(Line::styled( + self.push(accent_line( + theme().accent_error, format!("✗ failed · {kind}: {error}{guidance_line}{limits}"), - Style::default().fg(Color::Red), + Style::default().fg(theme().accent_error), )); self.follow(); } pub(crate) fn note_backend_waiting(&mut self, idle: Duration, threshold: Duration) { let _ = (idle, threshold); - self.push(Line::styled( + self.push(accent_line( + theme().warning, "⚠ Still thinking. Ctrl-C cancels; keep waiting to continue.", - Style::default().fg(Color::Yellow), + Style::default().fg(theme().warning), )); self.follow(); } @@ -319,10 +357,7 @@ impl crate::App { } else { // A non-explore tool breaks any active explore run. self.explore_run = None; - self.push(Line::styled( - format!("⏺ {label}"), - Style::default().fg(Color::Cyan), - )); + self.push(tool_header(&label)); } } UiEvent::ToolResult { name, result } => { @@ -348,13 +383,24 @@ impl crate::App { self.event_log.push(format!("status {text}")); self.last_turn_event = Some(TurnEventKind::Status); self.flush_pending(); - self.push(Line::styled(text, Style::default().fg(Color::Blue))); + // The status stream is informational — a muted gutter + muted + // text so it reads as agent chatter, not as the user's own words + // (which historically shared this color). + self.push(accent_line( + theme().gray_dim, + text, + Style::default().fg(theme().status), + )); } UiEvent::CheckpointWarning { text } => { self.event_log.push("checkpoint integrity warning".into()); self.checkpoint_warning = Some(text.clone()); self.flush_pending(); - self.push(Line::styled(text, Style::default().fg(Color::Yellow))); + self.push(accent_line( + theme().warning, + text, + Style::default().fg(theme().warning), + )); } // Plan updates replace the pinned checklist in place — no transcript // line, so progress reads as one updating block rather than a scroll. @@ -389,7 +435,11 @@ impl crate::App { // This callback is a usage summary, not a completion result. // The typed `TurnOutcome` returned after final workspace // reconciliation decides Done/Warning/Failed/Cancelled. - self.push(Line::styled(format!("usage · {summary}"), dim())); + self.push(accent_line( + theme().gray_dim, + format!("usage · {summary}"), + dim(), + )); // No follow(): respect a reader who scrolled up — the "↓ N new" // hint tells them the summary landed below. } @@ -411,9 +461,10 @@ impl crate::App { let label = if files.len() == 1 { "file" } else { "files" }; let list = files.join(", "); let clipped = hi_agent::ui::clip(&list, 200); - self.push(Line::styled( + self.push(accent_line( + theme().accent_success, format!("✎ {} {} changed: {}", files.len(), label, clipped), - Style::default().fg(Color::Green), + Style::default().fg(theme().accent_success), )); self.follow(); } @@ -460,7 +511,7 @@ impl crate::App { if n > 0 { run.all_empty = false; } - let line = self.render_explore_run(&header); + let line = tool_header(&self.render_explore_run(&header)); self.replace_last_line(line); return; } @@ -473,52 +524,56 @@ impl crate::App { all_empty: n == 0, line_pos: self.trimmed + self.transcript.len() as u64, }); - let line = self.render_explore_run(&header); - self.push(Line::styled(line, Style::default().fg(Color::Cyan))); + let line = tool_header(&self.render_explore_run(&header)); + self.push(line); return; } // A non-explore result breaks any active explore run. self.explore_run = None; - // Enough to show a small edit's diff with its context inline; larger - // results truncate with a footer (use `/diff` for the full diff). - const MAX: usize = 16; if result.trim().is_empty() { - self.push(Line::styled(" (no output)", dim())); + self.push(accent_line(theme().gray_dim, "(no output)", dim())); return; } - let body: String = result.lines().take(MAX).collect::>().join("\n"); - let lines: Vec> = if !body.contains('\u{1b}') && looks_like_diff(result) { - diff_lines(&body) + // Keep the *entire* output — it becomes a foldable ToolOutput block that + // shows a preview by default and expands on Ctrl-O. (The old path hard- + // truncated at 16 lines and discarded the rest.) + let lines: Vec> = if !result.contains('\u{1b}') && looks_like_diff(result) { + diff_lines(result) } else { // ANSI (already-colored) or non-diff text: parse escapes as before. - body.into_text() - .unwrap_or_else(|_| Text::from(body.clone())) + result + .into_text() + .unwrap_or_else(|_| Text::from(result.to_string())) .lines }; - for mut line in lines { - line.spans.insert(0, " ".into()); - self.transcript.push(TranscriptEntry::Line(line)); - } - let extra = result.lines().count().saturating_sub(MAX); - if extra > 0 { - self.push(Line::styled(format!(" … {extra} more lines"), dim())); - } + // Sit tool output under a dim continuation gutter so it reads as the + // body of the tool block above it, not free-floating text. + let body: Vec> = lines + .into_iter() + .map(|mut line| { + line.spans.insert(0, gutter(theme().gray_dim)); + line + }) + .collect(); + self.transcript.push(TranscriptEntry::ToolOutput { body }); + self.cap_transcript(); } - /// Render the current explore run as a single transcript line. A run of one - /// shows the per-call label and line count (`⏺ read src/a.rs · 113 lines`); - /// a run of many collapses to a summary (`⏺ read 6 files · 743 lines`). + /// Render the current explore run as a single transcript label (no bullet — + /// the caller wraps it with [`tool_header`]). A run of one shows the per-call + /// label and line count (`read src/a.rs · 113 lines`); a run of many collapses + /// to a summary (`read 6 files · 743 lines`). fn render_explore_run(&self, header: &str) -> String { let run = match &self.explore_run { Some(r) => r, - None => return format!("⏺ {header}"), + None => return header.to_string(), }; if run.count <= 1 { if run.all_empty { - format!("⏺ {header} · (no output)") + format!("{header} · (no output)") } else { let s = if run.lines == 1 { "" } else { "s" }; - format!("⏺ {header} · {} line{}", run.lines, s) + format!("{header} · {} line{}", run.lines, s) } } else { // Multi-call summary: drop the per-file label, show counts. @@ -527,11 +582,11 @@ impl crate::App { _ => "calls", }; if run.all_empty { - format!("⏺ {} {} {} · (no output)", run.tool, run.count, noun) + format!("{} {} {} · (no output)", run.tool, run.count, noun) } else { let s = if run.lines == 1 { "" } else { "s" }; format!( - "⏺ {} {} {} · {} line{}", + "{} {} {} · {} line{}", run.tool, run.count, noun, run.lines, s ) } @@ -541,9 +596,9 @@ impl crate::App { /// Replace the last transcript line in place (used to update a merged /// explore-run line as more results fold in). No-op if the transcript is /// empty or the last entry isn't a plain line. - fn replace_last_line(&mut self, text: String) { - if let Some(TranscriptEntry::Line(line)) = self.transcript.last_mut() { - *line = Line::styled(text, Style::default().fg(Color::Cyan)); + fn replace_last_line(&mut self, line: Line<'static>) { + if let Some(TranscriptEntry::Line(slot)) = self.transcript.last_mut() { + *slot = line; } } } diff --git a/crates/hi-tui/src/lib.rs b/crates/hi-tui/src/lib.rs index 6bbbfe56..bf703e88 100644 --- a/crates/hi-tui/src/lib.rs +++ b/crates/hi-tui/src/lib.rs @@ -23,6 +23,7 @@ mod model_picker; mod provider_form; mod render; mod sync_tui; +mod theme; mod util; mod watch; @@ -376,8 +377,20 @@ pub(crate) enum TranscriptEntry { text: String, elapsed: Duration, }, + /// A tool's (non-explore) output as a foldable block: the full body is + /// retained, but only a preview shows by default when it's long, with the + /// remainder revealed by `Ctrl-O` (or per the global `show_tool_output`). + /// Keeps a burst of shell output from burying the conversation while never + /// discarding it (the old path hard-truncated at 16 lines). + ToolOutput { + /// The already-styled body lines (gutter + diff/ANSI coloring applied). + body: Vec>, + }, } +/// How many lines of a long tool-output block show before it folds to a preview. +pub(crate) const TOOL_OUTPUT_PREVIEW_LINES: usize = 16; + /// A run of consecutive same-tool exploration results (read/list/grep) being /// collapsed into one transcript line, so a burst of reads renders as /// `⏺ read 6 files · 743 lines` instead of six separate lines. @@ -399,10 +412,15 @@ pub(crate) struct ExploreRun { } impl TranscriptEntry { - /// Flatten this entry into display lines under the current `show_reasoning` - /// setting. A collapsed reasoning block is one dim summary line; expanded, - /// it's the full text indented and dimmed. - pub(crate) fn flatten(&self, show_reasoning: bool) -> Vec> { + /// Flatten this entry into display lines under the current fold settings. + /// A collapsed reasoning block is one dim summary line; a long tool-output + /// block shows a preview plus a fold footer unless `show_tool_output` is on. + pub(crate) fn flatten( + &self, + show_reasoning: bool, + show_tool_output: bool, + ) -> Vec> { + let th = crate::theme::theme(); match self { TranscriptEntry::Line(line) => vec![line.clone()], TranscriptEntry::Reasoning { text, elapsed } => { @@ -415,31 +433,54 @@ impl TranscriptEntry { if show_reasoning { let mut lines = vec![Line::styled( format!("⏺ thought for {label} (Ctrl-T to collapse)"), - Style::default().fg(Color::DarkGray), + Style::default().fg(th.accent_thinking), )]; for line in text.lines() { lines.push(Line::styled( format!(" {line}"), - Style::default().fg(Color::DarkGray), + Style::default().fg(th.gray_dim), )); } lines } else { vec![Line::styled( format!("⏺ thought for {label} (Ctrl-T to expand)",), - Style::default().fg(Color::DarkGray), + Style::default().fg(th.accent_thinking), )] } } + TranscriptEntry::ToolOutput { body } => { + // Short output, or the global expand toggle, shows in full; + // otherwise a preview + a fold footer naming what's hidden. + if show_tool_output || body.len() <= TOOL_OUTPUT_PREVIEW_LINES { + body.clone() + } else { + let hidden = body.len() - TOOL_OUTPUT_PREVIEW_LINES; + let mut lines: Vec> = body[..TOOL_OUTPUT_PREVIEW_LINES].to_vec(); + lines.push(Line::from(vec![ + Span::styled("┃ ", Style::default().fg(th.gray_dim)), + Span::styled( + format!("… +{hidden} more lines · Ctrl-O to expand"), + Style::default() + .fg(th.gray_dim) + .add_modifier(Modifier::ITALIC), + ), + ])); + lines + } + } } } - /// The plain text of this entry, for /copy and /export (always includes - /// reasoning text regardless of collapse state). + /// The plain text of this entry, for /copy and /export (always the full + /// content regardless of collapse state). pub(crate) fn text(&self) -> String { match self { TranscriptEntry::Line(line) => line_text(line), TranscriptEntry::Reasoning { text, .. } => text.clone(), + TranscriptEntry::ToolOutput { body } => { + body.iter().map(line_text).collect::>().join("\n") + } } } } @@ -484,6 +525,10 @@ pub(crate) struct App { /// reasoning is collapsed to a one-line "thought for Ns" summary; Ctrl-T /// toggles this to show/hide the full thinking text. pub(crate) show_reasoning: bool, + /// Whether long tool-output blocks are expanded in full. Off by default — + /// output beyond [`TOOL_OUTPUT_PREVIEW_LINES`] folds to a preview; Ctrl-O + /// toggles this to reveal every block's full body. + pub(crate) show_tool_output: bool, /// The language of the ``` fence the streamed assistant text is currently /// inside (empty string if the fence gave none); `None` when not in a fence. /// Carries across streamed lines so code interiors highlight consistently. diff --git a/crates/hi-tui/src/render.rs b/crates/hi-tui/src/render.rs index ecccfc79..ebe9254f 100644 --- a/crates/hi-tui/src/render.rs +++ b/crates/hi-tui/src/render.rs @@ -4,6 +4,8 @@ use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; +use crate::theme::theme; + pub(crate) fn dim() -> Style { Style::default().add_modifier(Modifier::DIM) } @@ -12,9 +14,29 @@ pub(crate) fn line_text(line: &Line) -> String { line.spans.iter().map(|s| s.content.as_ref()).collect() } +/// A left accent-gutter span (`┃ `) in a role color — the block-accent bar that +/// marks agent "machinery" lines (tool calls, status, errors) as distinct from +/// the user's prompts and the assistant's prose (which stay flush-left). +pub(crate) fn gutter(color: Color) -> Span<'static> { + Span::styled("┃ ", Style::default().fg(color)) +} + +/// Build an accent-gutter line: the role-colored `┃ ` bar followed by `content` +/// styled with `content_style`. +pub(crate) fn accent_line( + color: Color, + content: impl Into, + content_style: Style, +) -> Line<'static> { + Line::from(vec![ + gutter(color), + Span::styled(content.into(), content_style), + ]) +} + /// The style for code — inline spans and fenced blocks. fn code_style() -> Style { - Style::default().fg(Color::Cyan) + Style::default().fg(theme().code) } /// Whether `s` looks like unified-diff output (a hunk header, a git header, or @@ -44,11 +66,11 @@ pub(crate) fn diff_lines(body: &str) -> Vec> { (Style::default().add_modifier(Modifier::BOLD), None, false) } else if line.starts_with("@@") { new_line = parse_hunk_new_start(line); - (Style::default().fg(Color::Cyan), None, false) + (Style::default().fg(theme().diff_hunk), None, false) } else if line.starts_with('+') { - (Style::default().fg(Color::Green), new_line, true) + (Style::default().fg(theme().diff_add), new_line, true) } else if line.starts_with('-') { - (Style::default().fg(Color::Red), None, false) + (Style::default().fg(theme().diff_del), None, false) } else { (dim(), new_line, true) }; @@ -132,7 +154,7 @@ fn highlight_strings(line: &str) -> Vec> { spans.push(Span::raw(std::mem::take(&mut plain))); } let s: String = chars[i..=j].iter().collect(); - spans.push(Span::styled(s, Style::default().fg(Color::Green))); + spans.push(Span::styled(s, Style::default().fg(theme().diff_add))); i = j + 1; continue; } @@ -480,7 +502,7 @@ mod tests { assert!( line.spans .iter() - .any(|s| s.content == "Vec" && s.style.fg == Some(Color::Cyan)), + .any(|s| s.content == "Vec" && s.style.fg == Some(crate::theme::theme().code)), "`code` styled" ); // A bare underscore in an identifier must not start italics. @@ -510,7 +532,8 @@ mod tests { assert!( s.spans .iter() - .any(|sp| sp.content == "\"hi\"" && sp.style.fg == Some(Color::Green)), + .any(|sp| sp.content == "\"hi\"" + && sp.style.fg == Some(crate::theme::theme().diff_add)), "string greened: {:?}", s.spans .iter() diff --git a/crates/hi-tui/src/tests.rs b/crates/hi-tui/src/tests.rs index cf907a6b..964db960 100644 --- a/crates/hi-tui/src/tests.rs +++ b/crates/hi-tui/src/tests.rs @@ -550,14 +550,14 @@ fn working_wave_sweeps_one_lit_letter_at_a_time() { assert_eq!(spans.len(), n, "one span per letter at tick {tick}"); let lit_count = spans .iter() - .filter(|s| s.style.fg == Some(Color::White)) + .filter(|s| s.style.fg == Some(crate::theme::theme().accent_running)) .count(); assert_eq!(lit_count, 1, "exactly one lit letter at tick {tick}"); // The lit index matches the forward/back sweep. let expected_lit = if tick < n { tick } else { cycle - tick }; assert_eq!( spans[expected_lit].style.fg, - Some(Color::White), + Some(crate::theme::theme().accent_running), "lit index {expected_lit} at tick {tick}" ); } @@ -586,7 +586,7 @@ fn renders_tool_call_diff_and_spinner() { let screen = dump(&term); // The header reads as "edit ", not a raw JSON dump. - assert!(screen.contains("⏺ edit src/cli.rs"), "readable tool header"); + assert!(screen.contains("◆ edit src/cli.rs"), "readable tool header"); assert!( !screen.contains("old_string"), "header must not dump JSON args" @@ -618,7 +618,7 @@ fn colorizes_plain_diff_tool_output() { let colored: Vec<(String, Option)> = app .transcript .iter() - .flat_map(|e| e.flatten(false)) + .flat_map(|e| e.flatten(false, true)) .map(|l| { let text: String = l.spans.iter().map(|s| s.content.as_ref()).collect(); (text, l.spans.last().map(|s| s.style.fg).unwrap_or(None)) @@ -627,19 +627,19 @@ fn colorizes_plain_diff_tool_output() { assert!( colored .iter() - .any(|(t, fg)| t.contains("+new") && *fg == Some(Color::Green)), + .any(|(t, fg)| t.contains("+new") && *fg == Some(crate::theme::theme().diff_add)), "added line is green: {colored:?}" ); assert!( colored .iter() - .any(|(t, fg)| t.contains("-old") && *fg == Some(Color::Red)), + .any(|(t, fg)| t.contains("-old") && *fg == Some(crate::theme::theme().diff_del)), "removed line is red" ); assert!( colored .iter() - .any(|(t, fg)| t.contains("@@") && *fg == Some(Color::Cyan)), + .any(|(t, fg)| t.contains("@@") && *fg == Some(crate::theme::theme().diff_hunk)), "hunk header is cyan" ); } @@ -654,7 +654,7 @@ fn non_diff_tool_output_is_not_colorized() { let any_red = app .transcript .iter() - .flat_map(|e| e.flatten(false)) + .flat_map(|e| e.flatten(false, true)) .any(|l| l.spans.last().map(|s| s.style.fg) == Some(Some(Color::Red))); assert!(!any_red, "a plain list must not be colorized as a diff"); } @@ -940,7 +940,7 @@ fn startup_notice_does_not_clip_input_line() { "notice shown:\n{screen}" ); // The input prompt must still be visible inside the box (not clipped). - assert!(screen.contains('›'), "input prompt visible:\n{screen}"); + assert!(screen.contains('❯'), "input prompt visible:\n{screen}"); // The input box's bottom border closes cleanly (the transcript block // also has a `╰`, so check the last one — the input box is at the bottom). let bottom: Vec<&str> = screen.lines().filter(|l| l.contains('╰')).collect(); @@ -958,7 +958,7 @@ fn startup_notice_does_not_clip_input_line() { .unwrap(); let above_border: String = rows[..border_row_idx].join("\n"); assert!( - above_border.contains("model metadata not loaded") && above_border.contains('›'), + above_border.contains("model metadata not loaded") && above_border.contains('❯'), "notice + prompt above the border:\n{screen}" ); } @@ -977,7 +977,7 @@ fn quit_notice_renders_and_does_not_clip_input() { screen.contains("Press Ctrl-C again to exit"), "quit notice shown:\n{screen}" ); - assert!(screen.contains('›'), "input prompt visible:\n{screen}"); + assert!(screen.contains('❯'), "input prompt visible:\n{screen}"); // The input box bottom border closes cleanly (not overlapping content). let bottom: Vec<&str> = screen.lines().filter(|l| l.contains('╰')).collect(); let input_box_border = bottom.last().expect("input box bottom border"); @@ -1627,7 +1627,7 @@ fn explore_tools_collapse_header_and_line_count_into_one_line() { let lines: Vec = app.transcript.iter().map(TranscriptEntry::text).collect(); // No header emitted yet — it waits for the result. assert!( - !lines.iter().any(|l| l.contains("⏺ read")), + !lines.iter().any(|l| l.contains("◆ read")), "no deferred header before result: {lines:?}" ); app.apply(UiEvent::ToolResult { @@ -1637,11 +1637,13 @@ fn explore_tools_collapse_header_and_line_count_into_one_line() { let lines: Vec = app.transcript.iter().map(TranscriptEntry::text).collect(); // Exactly one line, combining the header and the count. assert!( - lines.iter().any(|l| l == "⏺ read src/main.rs · 3 lines"), + lines + .iter() + .any(|l| l.contains("◆ read src/main.rs · 3 lines")), "collapsed read line: {lines:?}" ); assert_eq!( - lines.iter().filter(|l| l.contains("⏺ read")).count(), + lines.iter().filter(|l| l.contains("◆ read")).count(), 1, "exactly one read header line: {lines:?}" ); @@ -1657,7 +1659,7 @@ fn explore_tools_collapse_header_and_line_count_into_one_line() { }); let lines: Vec = app.transcript.iter().map(TranscriptEntry::text).collect(); assert!( - lines.iter().any(|l| l == "⏺ grep foo · (no output)"), + lines.iter().any(|l| l.contains("◆ grep foo · (no output)")), "collapsed grep empty line: {lines:?}" ); } @@ -1693,12 +1695,12 @@ fn consecutive_same_tool_explore_results_merge_into_one_line() { let lines: Vec = app.transcript.iter().map(TranscriptEntry::text).collect(); // Exactly one read line, summarizing all three. assert_eq!( - lines.iter().filter(|l| l.contains("⏺ read")).count(), + lines.iter().filter(|l| l.contains("◆ read")).count(), 1, "one merged read line: {lines:?}" ); assert!( - lines.iter().any(|l| l == "⏺ read 3 files · 6 lines"), + lines.iter().any(|l| l.contains("◆ read 3 files · 6 lines")), "merged summary: {lines:?}" ); @@ -1722,12 +1724,12 @@ fn consecutive_same_tool_explore_results_merge_into_one_line() { let lines: Vec = app.transcript.iter().map(TranscriptEntry::text).collect(); // Now two read lines: the merged 3-file run and a fresh single read. assert_eq!( - lines.iter().filter(|l| l.contains("⏺ read")).count(), + lines.iter().filter(|l| l.contains("◆ read")).count(), 2, "run broken by edit: {lines:?}" ); assert!( - lines.iter().any(|l| l == "⏺ read d.rs · 2 lines"), + lines.iter().any(|l| l.contains("◆ read d.rs · 2 lines")), "fresh read after break: {lines:?}" ); } @@ -1796,7 +1798,7 @@ fn renders_multiline_input() { term.draw(|f| app.render(f)).unwrap(); let screen = dump(&term); assert!( - screen.contains("› first"), + screen.contains("❯ first"), "first line with prompt: {screen}" ); assert!(screen.contains("second"), "second line: {screen}"); @@ -2370,3 +2372,142 @@ fn uievent_serializes_and_deserializes_roundtrip() { "turn_error must not use kind for the error type (conflicts with tag): {error_json}" ); } + +/// A visual smoke of the Phase-1 transcript grammar: prints the rendered screen +/// (run with `--nocapture`) and asserts the block-accent markers are present. +#[test] +fn phase1_visual_grammar_smoke() { + let mut app = test_app("pipe", "glm-5.2"); + app.push(ratatui::text::Line::styled( + "❯ port the parser to the new API", + ratatui::style::Style::default().fg(crate::theme::theme().accent_user), + )); + app.apply(UiEvent::ToolCall { + name: "read".into(), + arguments: "{\"path\":\"src/parser.rs\"}".into(), + }); + app.apply(UiEvent::ToolResult { + name: "read".into(), + result: "a\nb\nc\n".into(), + }); + app.apply(UiEvent::ToolCall { + name: "bash".into(), + arguments: "{\"command\":\"cargo test\"}".into(), + }); + app.apply(UiEvent::ToolResult { + name: "bash".into(), + result: "running 3 tests\ntest result: ok".into(), + }); + app.apply(UiEvent::Status { + text: "🔍 skeptic approved — advancing".into(), + }); + app.apply(UiEvent::Text { + text: "Done. The parser now uses the new API.\n".into(), + }); + app.apply(UiEvent::AssistantEnd); + app.apply(UiEvent::ChangedFiles { + files: vec!["src/parser.rs".into()], + }); + + // Exercise the chrome: context chip + input. + app.context_used = 42000; + app.context_window = Some(128000); + let mut term = Terminal::new(TestBackend::new(72, 22)).unwrap(); + term.draw(|f| app.render(f)).unwrap(); + let screen = dump(&term); + println!("\n{screen}"); + + assert!(screen.contains("❯ port the parser"), "user prompt band"); + assert!( + screen.contains("◆ read src/parser.rs"), + "read header with ◆" + ); + assert!(screen.contains("◆ bash"), "bash header with ◆"); + assert!(screen.contains("skeptic approved"), "status line present"); + assert!(screen.contains("┃"), "accent gutter present"); + assert!(screen.contains("✎ 1 file changed"), "changed-files line"); +} + +#[test] +fn long_tool_output_folds_to_preview_and_expands_on_ctrl_o() { + let mut app = test_app("pipe", "glm-5.2"); + // 40 lines of bash output — well over the preview cap. + let output = (0..40) + .map(|i| format!("line {i}")) + .collect::>() + .join("\n"); + app.apply(UiEvent::ToolCall { + name: "bash".into(), + arguments: "{\"command\":\"seq 40\"}".into(), + }); + app.apply(UiEvent::ToolResult { + name: "bash".into(), + result: output, + }); + + // Collapsed (default): preview lines + a fold footer, not all 40. + let collapsed: Vec = app + .transcript + .iter() + .flat_map(|e| e.flatten(false, false)) + .map(|l| crate::render::line_text(&l)) + .collect(); + assert!( + collapsed.iter().any(|l| l.contains("line 0")), + "preview shows the head: {collapsed:?}" + ); + assert!( + !collapsed.iter().any(|l| l.contains("line 39")), + "the tail is folded away when collapsed" + ); + assert!( + collapsed + .iter() + .any(|l| l.contains("more lines · Ctrl-O to expand")), + "a fold footer names the hidden lines: {collapsed:?}" + ); + + // Expanded (Ctrl-O / show_tool_output): the full body, no footer. + let expanded: Vec = app + .transcript + .iter() + .flat_map(|e| e.flatten(false, true)) + .map(|l| crate::render::line_text(&l)) + .collect(); + assert!( + expanded.iter().any(|l| l.contains("line 39")), + "expanded shows the whole output" + ); + assert!( + !expanded.iter().any(|l| l.contains("Ctrl-O to expand")), + "no fold footer when expanded" + ); + + // Short output (≤ preview) is never folded — no regression from the old + // inline behavior. + let mut app2 = test_app("pipe", "glm-5.2"); + app2.apply(UiEvent::ToolResult { + name: "bash".into(), + result: "just one line".into(), + }); + let short: Vec = app2 + .transcript + .iter() + .flat_map(|e| e.flatten(false, false)) + .map(|l| crate::render::line_text(&l)) + .collect(); + assert!(short.iter().any(|l| l.contains("just one line"))); + assert!( + !short.iter().any(|l| l.contains("Ctrl-O")), + "short output isn't folded" + ); + + // Full text (for /copy and /export) always has everything, regardless of fold. + let full = app + .transcript + .iter() + .map(TranscriptEntry::text) + .collect::>() + .join("\n"); + assert!(full.contains("line 39"), "copy/export keeps the full body"); +} diff --git a/crates/hi-tui/src/theme.rs b/crates/hi-tui/src/theme.rs new file mode 100644 index 00000000..a4740b39 --- /dev/null +++ b/crates/hi-tui/src/theme.rs @@ -0,0 +1,299 @@ +//! The TUI color theme: a named slot vocabulary so every rendered color maps to +//! a *role* (user, tool, error, running, …) rather than a hardcoded ANSI name. +//! +//! Modeled on grok-build's theme system. Two built-in palettes: +//! - `dark` — a GrokNight-style truecolor palette (neutral dark base, magenta +//! assistant accent) shown on terminals that support 24-bit color. +//! - `ansi` — named ANSI colors that respect the user's own terminal theme; +//! this reproduces hi's historical look and is the fallback on terminals +//! without truecolor. +//! +//! Selection: `HI_THEME` (`dark` | `light` | `ansi` | `auto`, default `auto`). +//! `auto` picks `dark` on a truecolor terminal, `ansi` otherwise — so the +//! designed palette shows where it renders faithfully and never regresses a +//! basic terminal. A single global [`theme()`] accessor is read on every +//! render; [`set_theme`] switches at runtime (e.g. a future `/theme`). + +use std::sync::{OnceLock, RwLock}; + +use ratatui::style::Color; + +/// Every color role the TUI draws. One field per semantic slot; renderers ask +/// for a role, never a raw `Color`, so the whole look restyles from one place. +/// +/// The full palette is defined up front; call sites migrate onto it in phases +/// (transcript first, then chrome), so some slots have no reader yet. +#[allow(dead_code)] +#[derive(Clone, Copy, Debug)] +pub struct Theme { + // Accents — the left gutter bar and headers take their color from the block + // role, so a glance at the bar tells you what a block is. + pub accent_user: Color, + pub accent_assistant: Color, + pub accent_thinking: Color, + pub accent_tool: Color, + pub accent_system: Color, + pub accent_error: Color, + pub accent_success: Color, + pub accent_running: Color, + pub accent_skill: Color, + pub accent_plan: Color, + pub accent_goal: Color, + pub accent_verify: Color, + + // Text. + pub text_primary: Color, + pub text_secondary: Color, + pub gray_dim: Color, + pub gray: Color, + pub gray_bright: Color, + + // Semantic. + pub warning: Color, + pub path: Color, + pub command: Color, + pub code: Color, + pub link: Color, + /// The `ui.status` stream ("🔍 skeptic approved") — informational, so it is + /// muted rather than competing with the user's own prompt echo. + pub status: Color, + + // Diffs. + pub diff_add: Color, + pub diff_del: Color, + pub diff_hunk: Color, + pub diff_context: Color, + pub diff_gutter: Color, + + // Chrome. + pub selection: Color, + pub prompt_border: Color, + pub prompt_border_active: Color, + /// A subtle band behind a user prompt block (truecolor only; `Reset` on + /// ansi so nothing paints a background the terminal theme won't match). + pub band_user: Color, + /// A sunken panel behind expanded tool output (truecolor only). + pub panel: Color, +} + +impl Theme { + /// GrokNight-style truecolor dark palette. Neutral gray base, magenta + /// assistant/thinking accent, blue system, standard green/red/yellow. + pub const fn dark() -> Self { + Self { + accent_user: Color::Rgb(0xc8, 0xc8, 0xc8), + accent_assistant: Color::Rgb(0xbb, 0x9a, 0xf7), + accent_thinking: Color::Rgb(0x9d, 0x7c, 0xd8), + accent_tool: Color::Rgb(0x78, 0x78, 0x78), + accent_system: Color::Rgb(0x7a, 0xa2, 0xf7), + accent_error: Color::Rgb(0xf7, 0x76, 0x8e), + accent_success: Color::Rgb(0x9e, 0xce, 0x6a), + accent_running: Color::Rgb(0xbb, 0x9a, 0xf7), + accent_skill: Color::Rgb(0x7a, 0xa2, 0xf7), + accent_plan: Color::Rgb(0x7d, 0xcf, 0xff), + accent_goal: Color::Rgb(0xbb, 0x9a, 0xf7), + accent_verify: Color::Rgb(0x7d, 0xcf, 0xff), + text_primary: Color::Rgb(0xc0, 0xca, 0xf5), + text_secondary: Color::Rgb(0x9a, 0xa5, 0xce), + gray_dim: Color::Rgb(0x56, 0x5f, 0x89), + gray: Color::Rgb(0x78, 0x7c, 0x99), + gray_bright: Color::Rgb(0xa9, 0xb1, 0xd6), + warning: Color::Rgb(0xe0, 0xaf, 0x68), + path: Color::Rgb(0xff, 0x9e, 0x64), + command: Color::Rgb(0x7d, 0xcf, 0xff), + code: Color::Rgb(0x7d, 0xcf, 0xff), + link: Color::Rgb(0x7a, 0xa2, 0xf7), + status: Color::Rgb(0x9a, 0xa5, 0xce), + diff_add: Color::Rgb(0x9e, 0xce, 0x6a), + diff_del: Color::Rgb(0xf7, 0x76, 0x8e), + diff_hunk: Color::Rgb(0x7d, 0xcf, 0xff), + diff_context: Color::Rgb(0x78, 0x7c, 0x99), + diff_gutter: Color::Rgb(0x56, 0x5f, 0x89), + selection: Color::Rgb(0x7d, 0xcf, 0xff), + prompt_border: Color::Rgb(0x56, 0x5f, 0x89), + prompt_border_active: Color::Rgb(0x7d, 0xcf, 0xff), + band_user: Color::Rgb(0x1f, 0x23, 0x35), + panel: Color::Rgb(0x1a, 0x1b, 0x26), + } + } + + /// A light truecolor palette for bright terminal backgrounds. + pub const fn light() -> Self { + Self { + accent_user: Color::Rgb(0x34, 0x3b, 0x58), + accent_assistant: Color::Rgb(0x7a, 0x4c, 0xc9), + accent_thinking: Color::Rgb(0x8a, 0x5c, 0xd9), + accent_tool: Color::Rgb(0x8c, 0x8c, 0x8c), + accent_system: Color::Rgb(0x2e, 0x5c, 0xc9), + accent_error: Color::Rgb(0xc0, 0x36, 0x4e), + accent_success: Color::Rgb(0x38, 0x7a, 0x2c), + accent_running: Color::Rgb(0x7a, 0x4c, 0xc9), + accent_skill: Color::Rgb(0x2e, 0x5c, 0xc9), + accent_plan: Color::Rgb(0x16, 0x7a, 0xa6), + accent_goal: Color::Rgb(0x7a, 0x4c, 0xc9), + accent_verify: Color::Rgb(0x16, 0x7a, 0xa6), + text_primary: Color::Rgb(0x2a, 0x2e, 0x3a), + text_secondary: Color::Rgb(0x50, 0x56, 0x6a), + gray_dim: Color::Rgb(0x9a, 0xa0, 0xb0), + gray: Color::Rgb(0x70, 0x76, 0x88), + gray_bright: Color::Rgb(0x40, 0x46, 0x58), + warning: Color::Rgb(0xa6, 0x6a, 0x00), + path: Color::Rgb(0xc0, 0x54, 0x1a), + command: Color::Rgb(0x16, 0x6a, 0xa6), + code: Color::Rgb(0x16, 0x6a, 0xa6), + link: Color::Rgb(0x2e, 0x5c, 0xc9), + status: Color::Rgb(0x50, 0x56, 0x6a), + diff_add: Color::Rgb(0x38, 0x7a, 0x2c), + diff_del: Color::Rgb(0xc0, 0x36, 0x4e), + diff_hunk: Color::Rgb(0x16, 0x6a, 0xa6), + diff_context: Color::Rgb(0x70, 0x76, 0x88), + diff_gutter: Color::Rgb(0x9a, 0xa0, 0xb0), + selection: Color::Rgb(0x16, 0x6a, 0xa6), + prompt_border: Color::Rgb(0x9a, 0xa0, 0xb0), + prompt_border_active: Color::Rgb(0x16, 0x6a, 0xa6), + band_user: Color::Rgb(0xec, 0xee, 0xf5), + panel: Color::Rgb(0xf0, 0xf2, 0xf8), + } + } + + /// Named-ANSI palette: reproduces hi's historical look and respects the + /// user's own terminal colors. Backgrounds are `Reset` so nothing paints a + /// band a terminal theme won't match. This is the non-truecolor fallback. + pub const fn ansi() -> Self { + Self { + accent_user: Color::Blue, + accent_assistant: Color::Magenta, + accent_thinking: Color::DarkGray, + accent_tool: Color::Cyan, + accent_system: Color::Blue, + accent_error: Color::Red, + accent_success: Color::Green, + accent_running: Color::Cyan, + accent_skill: Color::Blue, + accent_plan: Color::Cyan, + accent_goal: Color::Magenta, + accent_verify: Color::Cyan, + text_primary: Color::Reset, + text_secondary: Color::Gray, + gray_dim: Color::DarkGray, + gray: Color::Gray, + gray_bright: Color::White, + warning: Color::Yellow, + path: Color::Cyan, + command: Color::Cyan, + code: Color::Cyan, + link: Color::Blue, + status: Color::Blue, + diff_add: Color::Green, + diff_del: Color::Red, + diff_hunk: Color::Cyan, + diff_context: Color::DarkGray, + diff_gutter: Color::DarkGray, + selection: Color::Cyan, + prompt_border: Color::DarkGray, + prompt_border_active: Color::Cyan, + band_user: Color::Reset, + panel: Color::Reset, + } + } + + /// Whether this theme paints real backgrounds (truecolor) or leaves them at + /// the terminal default (ansi). Renderers use this to skip band/panel fills + /// that would look wrong against an unknown terminal background. + #[allow(dead_code)] + pub fn paints_backgrounds(&self) -> bool { + !matches!(self.band_user, Color::Reset) + } +} + +/// Resolve the active theme from `HI_THEME` and terminal capability. +fn resolve_from_env() -> Theme { + match std::env::var("HI_THEME").ok().as_deref().map(str::trim) { + Some("dark") => Theme::dark(), + Some("light") => Theme::light(), + Some("ansi") | Some("none") => Theme::ansi(), + // `auto` (and unset): the designed palette on a truecolor terminal, + // the terminal-respecting ANSI look otherwise. + _ => { + if terminal_supports_truecolor() { + Theme::dark() + } else { + Theme::ansi() + } + } + } +} + +/// Best-effort truecolor detection. `COLORTERM=truecolor|24bit` is the standard +/// signal; a few terminals advertise via `TERM`. Conservative: unknown → false +/// (fall back to the terminal-respecting ANSI palette). +fn terminal_supports_truecolor() -> bool { + if let Ok(colorterm) = std::env::var("COLORTERM") { + let c = colorterm.to_ascii_lowercase(); + if c.contains("truecolor") || c.contains("24bit") { + return true; + } + } + if let Ok(term) = std::env::var("TERM") { + let t = term.to_ascii_lowercase(); + if t.contains("truecolor") || t.contains("24bit") || t == "xterm-kitty" { + return true; + } + } + // Modern terminal emulators that always support truecolor. + matches!( + std::env::var("TERM_PROGRAM").ok().as_deref(), + Some("iTerm.app") | Some("WezTerm") | Some("ghostty") | Some("vscode") + ) +} + +static THEME: OnceLock> = OnceLock::new(); + +fn cell() -> &'static RwLock { + THEME.get_or_init(|| RwLock::new(resolve_from_env())) +} + +/// The active theme. Cheap `Copy`; read freely on every render. +pub fn theme() -> Theme { + *cell().read().unwrap() +} + +/// Switch the active theme at runtime (wired to a future `/theme` command). +#[allow(dead_code)] +pub fn set_theme(theme: Theme) { + *cell().write().unwrap() = theme; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ansi_theme_leaves_backgrounds_at_terminal_default() { + let t = Theme::ansi(); + assert!(!t.paints_backgrounds()); + assert_eq!(t.band_user, Color::Reset); + assert_eq!(t.panel, Color::Reset); + } + + #[test] + fn truecolor_themes_paint_backgrounds() { + assert!(Theme::dark().paints_backgrounds()); + assert!(Theme::light().paints_backgrounds()); + } + + #[test] + fn every_role_is_distinct_enough_in_dark() { + // The three most-overloaded historical roles (user, tool, status) must + // not collapse to the same color in the designed palette. + let t = Theme::dark(); + assert_ne!(t.accent_user, t.accent_tool); + assert_ne!(t.accent_user, t.status); + assert_ne!(t.accent_tool, t.accent_goal); + } + + // Note: the runtime `set_theme`/`theme` global is deliberately not unit- + // tested here — mutating the process-wide theme would race color-asserting + // render tests running in parallel. The pure `Theme` constructors above + // cover the palette; the global is exercised by the app at startup. +} diff --git a/scripts/file_size_baseline.txt b/scripts/file_size_baseline.txt index 4f71aae0..17480da5 100644 --- a/scripts/file_size_baseline.txt +++ b/scripts/file_size_baseline.txt @@ -28,12 +28,12 @@ 3102 crates/hi-tools/src/tools.rs 1489 crates/hi-tools/src/transaction.rs 1301 crates/hi-tools/src/web.rs -858 crates/hi-tui/src/app/commands.rs -1169 crates/hi-tui/src/app/render.rs +863 crates/hi-tui/src/app/commands.rs +1199 crates/hi-tui/src/app/render.rs 2562 crates/hi-tui/src/app/run.rs 1758 crates/hi-tui/src/dashboard.rs 2527 crates/hi-tui/src/loops.rs -2372 crates/hi-tui/src/tests.rs +2513 crates/hi-tui/src/tests.rs 934 crates/hi-tui/src/watch.rs 1016 crates/hi-agent/src/goal.rs 1476 crates/hi-agent/src/tests/goal.rs