Skip to content

tui: quiet transcript noise from agent checks and unknown-tool calls (dogfood A5) - #4231

Merged
Hmbown merged 1 commit into
mainfrom
codex/v0868-fix-4112
Jul 8, 2026
Merged

tui: quiet transcript noise from agent checks and unknown-tool calls (dogfood A5)#4231
Hmbown merged 1 commit into
mainfrom
codex/v0868-fix-4112

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

Transcript lane (#4112), dogfood finding A5 on #4092. Three cuts at the reported noise:

  1. Agent inspection cells stop impersonating spawns. agent peek/status/wait calls rendered the same compact delegate done · agent_x line as a completed launch — in a fan-out session every poll read as another finished delegate. tool_routing now stamps the non-default action at the front of the args summary and render_agent_compact labels checks checking/checked and joins waiting/waited.
  2. Inspection cells stay compact in Transcript mode (the pager/Ctrl-O surface) instead of dumping a full session projection per check. Spawns keep the full block for session replay.
  3. Unknown-tool failures collapse to one line. The observed tool issue · item / name: item / args: status: pending / result: … Did you mean: note, tts? card becomes a single header carrying the catalog error. For the checklist mis-call shape (item/todo/… with per-entry calls) the error now names the actual fix — one checklist_write call with an items array — instead of a misleading fuzzy match.

Pairs with #4229 (which removes the polling that generates most of these cells in the first place).

Testing

  • 5 new render tests + 1 catalog-message test
  • tui::history::tests: 101 pass; missing_tool: 4 pass; tool_routing: 5 pass
  • fmt + CI-flag clippy clean

Dogfood finding A5 (#4092), transcript lane #4112. Three cuts:

- agent peek/status/wait cells no longer render the spawn's
  'delegate done · agent_x' line. tool_routing stamps the non-default
  action at the front of the args summary, and render_agent_compact
  labels checks as 'checking/checked' and joins as 'waiting/waited'.
  During a fan-out session every status check previously read as yet
  another completed delegate — that was most of the reported spam.

- Inspection cells stay compact in Transcript mode too (the Ctrl-O /
  pager surface), instead of dumping a full session projection per
  check. Spawn cells keep the full block for session replay.

- A call to a nonexistent tool collapses to one header line carrying
  the catalog error, instead of a four-line name/args/result card. And
  for the observed checklist mis-call shape (model emitting each list
  entry as a tool named 'item'/'todo'/...), the catalog error now says
  the actual fix — one checklist_write call with an items array —
  instead of the misleading fuzzy match "Did you mean: note, tts?".

Tests: five new render tests + one catalog-message test; history
module 101 passed, engine missing_tool 4 passed, tool_routing 5 passed.
Copilot AI review requested due to automatic review settings July 8, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 58s —— View job


Reviewing PR #4231

Summary

Solid, well-scoped fix for the dogfood A5 noise, with good test coverage of the observed shapes. No provider/model/route-safety concerns — this diff doesn't touch prefix/model-inference logic at all. One correctness-adjacent issue and one duplication issue worth addressing before merge; the rest are minor.

Findings

1. wait/join/await/block branch is currently dead code (moderate)
crates/tui/src/tui/history/agent_activity.rs:29-32,64 adds an AgentCompactAction::Wait case matching action strings "wait" | "join" | "await" | "block", and crates/tui/src/tui/history/tests.rs:2830-2842 (agent_wait_renders_waited_label) asserts on it. But the real agent tool's schema enum is ["start", "status", "peek", "cancel"], and parse_agent_tool_action (crates/tui/src/tools/subagent/mod.rs:3575-3587) rejects anything else with Invalid agent action '{other}'. There is no way for a model to actually produce an action: wait cell today — I couldn't find a Wait variant on AgentToolAction, nor a settled field anywhere outside this new test. So this code (and its test) validates a shape that can't occur in production yet. If this is meant to pair with the batch-wait feature referenced by #4229, it'd help to say so in the PR description or gate it until that lands; otherwise it's speculative code that should be dropped.

2. Agent-action taxonomy is now duplicated in three places (reuse)
The "which action string means what" mapping is hand-maintained independently in:

  • parse_agent_tool_action — canonical (crates/tui/src/tools/subagent/mod.rs:3579-3583)
  • the spawn-vs-non-default check in handle_tool_call_startedaction != "start" && action != "spawn" && action != "run" (crates/tui/src/tui/tool_routing.rs:264-267)
  • agent_inspection_action's Check/Wait groups (crates/tui/src/tui/history/agent_activity.rs:58-66)

If a future change adds a new synonym to the real parser (e.g. an "info" alias for status) without updating the other two call sites, a peek-like call silently renders as "delegate done" again — reintroducing the exact bug this PR fixes. Worth extracting one shared classifier both the routing stamp and the compact renderer call instead of three string lists that must stay in sync by convention.

3. Minor: dead already_leads guard
crates/tui/src/tui/tool_routing.rs:262-263 checks whether summarize_tool_args already produced a string starting with "action:" — it never does, since summarize_tool_args (crates/tui/src/tui/history/tool_output.rs) has no action key handling. Harmless, but the guard can never trigger.

4. Minor: substring match for unknown-tool detection
The new collapse in crates/tui/src/tui/history.rs:1341-1355 detects unknown-tool failures via output.contains("is not available in the current tool catalog"). It's gated on ToolStatus::Failed so risk is low, but it's a string-content check rather than a typed error variant — a coincidental match in unrelated failure output would misroute. Not blocking, just noting for awareness.

Good choices

  • Stamping the action into input_summary rather than adding a schema field is a minimal, low-risk way to thread this through existing generic-cell plumbing.
  • Transcript mode correctly keeps the full block for spawns (session replay) while collapsing inspections — agent_spawn_keeps_full_block_in_transcript_mode test covers that boundary well.
  • Checklist mis-call redirect (tool_catalog.rs:660-675) names the actual fix instead of fuzzy-matching to unrelated tools like note/tts — directly addresses the reported confusing error.

Not verified

Repo-provided cargo test/cargo build/cargo fmt invocations required approval that wasn't granted in this run, so I could not independently execute the test suite or confirm clippy/fmt cleanliness. Please re-run cargo test -p codewhale-tui --bin codewhale-tui --locked if --allowedTools can be widened for a future pass — the PR description's reported counts (101/4/5 passing) look consistent with the diff by inspection.

@Hmbown
Hmbown merged commit ad9af14 into main Jul 8, 2026
19 checks passed
@Hmbown
Hmbown deleted the codex/v0868-fix-4112 branch July 24, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants