feat(tui): Phase 1+2 — theme layer, block-accent grammar, foldable tool output

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;
 

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)]

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.";