feat(tui): show tool and MCP schema costs (#5603)

[Hmbown]

Rebase of @wuisabel-gif's #5603 onto current main after #5604. Authorship preserved (Isabel Wu). The original fork PR conflicted on CHANGELOG after #5604 merged; this keeps both changelog bullets.

Original: #5603

Verified locally: cargo test -p codewhale-tui --lib 'tui::context_inspector' --locked → 14 passed.

No-Issue: Landing the already-reviewed #5603 slice; the display-only #5553 work is not closed by this rebase PR.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

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

---

Review: feat(tui): show tool and MCP schema costs (#5611)

• Gather context (diff, changed files, CLAUDE.md/AGENTS.md rules)
• Understand the request (code review of PR feat(tui): show tool and MCP schema costs (#5603) #5611)
• Review crates/tui/src/tui/context_inspector.rs changes
• Review CHANGELOG updates
• Check for provider/model/route safety issues
• Check for correctness bugs, reuse/simplification, tests
• Post final review

Summary

This PR (a rebase of #5603 onto current main after #5604, authorship preserved for Isabel Wu) adds a bounded schema-cost breakdown to the context inspector: total estimated tokens for the built-in tool catalog, sorted per-tool rows (capped at MAX_SCHEMA_COST_ROWS = 24), and a per-MCP-server rollup matched against app.mcp_snapshot. Estimation is serde_json::to_string(tool).chars().count() / 4 (rounded up), consistent with the len / 4 heuristic already used elsewhere in compaction.rs.

Findings

1. Duplicate/misleading section header (correctness — posted as inline comment on context_inspector.rs:575)
The new section reuses MessageId::CtxInspRecentTools ("Recent Tools") for its own header, and build_context_inspector_text calls push_tool_schema_costs immediately after push_tools (context_inspector.rs:231) with no blank-line separator — unlike every other section in that function, which is preceded by writeln!(out). The rendered output ends up with two consecutive sections both headed "Recent Tools", one being tool-activity history and the other being the new schema-cost breakdown, run together with no visual break. Given the PR's purpose is exactly to surface this cost breakdown legibly, this undercuts it. Suggested fix: add a distinct MessageId (e.g. CtxInspToolSchemaCosts) with translations in every crates/tui/locales/*.json, and add the missing writeln!(out) before the call.

2. Provider/model/route safety — N/A. This change is purely a debug/inspector text view; it doesn't touch provider selection, model routing, or prompt/tool-catalog assembly sent to the model (the CHANGELOG entry correctly notes "without changing prompt assembly or cache behavior").

3. Reuse/simplification — Good reuse: tool_schema_tokens piggybacks on the existing Tool serde representation (matching the actual wire fields in crates/core/src/request.rs) rather than reimplementing a schema walker, and MCP tool classification correctly reuses McpPool::is_mcp_tool to avoid double-counting MCP-prefixed tools in the "built-in" bucket. No dead code or needless cloning spotted.

4. Tests — The new inspector_reports_catalog_and_mcp_schema_costs_with_bounded_rows test covers the bounded-row truncation (MAX_SCHEMA_COST_ROWS), sorting by descending cost, matched vs. announced MCP tool counts, and an empty-server case. Verified the McpServerSnapshot/McpManagerSnapshot/McpDiscoveredItem field usage in the test matches the current struct definitions in crates/tui/src/mcp.rs, so the reported local pass (cargo test -p codewhale-tui --lib 'tui::context_inspector' --locked → 14 passed) is consistent with what's in the diff. I was not able to independently re-run cargo test/cargo build in this environment (Bash tool calls for cargo ... required approval that wasn't available non-interactively) — if you'd like me to independently verify, the --allowedTools for this job would need to permit cargo test/cargo build.

5. Security — No secret handling, shell/exec, or input-validation surface touched.

Verdict

One real (non-blocking, cosmetic-but-user-facing) issue: the duplicated "Recent Tools" header / missing separator between the two sections. Everything else — the estimation approach, MCP-vs-built-in classification, row bounding, and test coverage — looks correct and consistent with existing codebase conventions.
· branch grok/take-5603-20260825

[claude]

The new Tool Schema Costs section reuses MessageId::CtxInspRecentTools ("Recent Tools") as its own header, and push_tool_schema_costs is called right after push_tools at crates/tui/src/tui/context_inspector.rs:231 with no writeln!(out) separator (every other section in build_context_inspector_text gets a blank line before it, e.g. lines 224-230).

The result: the rendered text shows two back-to-back sections both titled "Recent Tools", with the schema-cost breakdown appearing to be a continuation of the tool-activity list rather than its own section. Given the whole point of this PR is a clearly labeled schema-cost view, this undermines it.

Fix: add a new MessageId (e.g. CtxInspToolSchemaCosts) with an entry in every crates/tui/locales/*.json file, and insert a blank-line writeln!(out) before the push_tool_schema_costs call at line 231, matching the spacing convention used for the other sections.

Fix this →

[Hmbown]

Both red checks here are inherited from main, not caused by this PR

I dug into the two failing checks. Neither is this PR's fault — this branch only touches CHANGELOG.md, crates/tui/CHANGELOG.md, and crates/tui/src/tui/context_inspector.rs.

1. Version drift — main itself fails this gate right now. Reproduced locally against origin/main (6110159):

$ ./scripts/release/check-feature-release-notes.sh v0.9.10 origin/main
::error::Feature commit 6110159bd0bb references #5604, but no release-note receipt exists in CHANGELOG.md docs/CHANGELOG_ARCHIVE.md.
  feat(tui): make Fleet roster editing discoverable (#5604)

The Fleet-roster changelog bullet credits (#5589) but the commit that landed it is #5604, so check 12 in scripts/release/check-versions.sh has no receipt to find. Every open PR inherits this.

2. Test (windows-latest) — the same two tests fail, byte-for-byte, on this PR, on #5609, and on any PR branched off current main:

FAIL codewhale-tui tools::shell::tests::readonly_operands_are_workspace_bounded_and_symlink_aware
  panicked at crates\tui\src\tools\shell\tests.rs:964:6:
  absolute in-workspace operand: PermissionDenied { message: "[shell.readonly.operand.shape] Read-only Scout
  shell operands must stay inside the workspace; refused \"\\?C:UsersrunneradminAppDataLocalTemp.tmpr1CWRBinside.txt\"" }

FAIL codewhale-tui tools::subagent::tests::read_only_inspection_roles_execute_pwd_and_absolute_git_log
  panicked at crates\tui\src\tools\subagent\tests.rs:7526:13:
  Scout posture must admit git -C \\?\C:\Users\runneradmin\AppData\Local\Temp\.tmpVIvyua log --oneline -20

Note the mangled operand: \\?C:UsersrunneradminAppData.... shell_words::split is a POSIX splitter and eats the backslashes of a Windows path before operand classification ever sees it. #5606 stripped the \\?\ verbatim prefix at classification time, but the word-split damage happens earlier, so the fix was incomplete.

Both are already fixed by #5610 (@aboimpinto), which is green on Version drift and Test (windows-latest). Merging #5610 first should turn this PR green with no changes here.

I deliberately did not duplicate the #5604 changelog receipt onto this branch — it would collide on the same line as #5610's fix and take credit for their work.

[Hmbown]

Closing as already integrated: every hunk of this display-only #5553 slice is byte-identical on the v0.9.12 integration branch — push_tool_schema_costs/tool_schema_tokens at crates/tui/src/tui/context_inspector.rs:648-706 wired into build_context_inspector_text, MAX_SCHEMA_COST_ROWS/SCHEMA_TOKEN_DIVISOR constants, the inspector_reports_catalog_and_mcp_schema_costs_with_bounded_rows test, and both changelog bullets (CHANGELOG.md:302-304, crates/tui/CHANGELOG.md:270-272). No action needed from this PR; the rebase goal is already met.

[Hmbown]

Re-landing the underlying #5603 work on current main as #5653.

This grok rebase, like the original #5603, was closed as already integrated, but the schema-cost inspector never reached origin/main. Isabel's authorship is preserved.

[Hmbown]

The #5603 work this rebased is now on main via #5653 (d4d9a567d).