feat(tui): compact and hidden presets for the bottom chrome (#5950) - #5973
feat(tui): compact and hidden presets for the bottom chrome (#5950)#5973Hmbown wants to merge 3 commits into
Conversation
The second half of #5950; the /statusline composition half landed as #5962. These presets only decide how much of each of the two rows under the composer paints: [tui].posture_bar = full | compact | hidden (default full) [tui].metrics_line = full | compact | hidden (default full) Both also settable live with /config posture_bar compact; --save writes the [tui] key, a session-only set says so, and an unknown preset names the three and changes nothing. compact is not a second renderer: each row keeps its existing shed ladder and the preset starts it at a fixed rung, so what compact keeps is exactly what a narrow row keeps (posture bar starts render_tideline_footer at COMPACT_SHED; metrics line starts shed_pass with every segment at or above SHED_BEFORE_HELP). hidden gives the row to the transcript, resolving info_height/footer_height to 0 exactly as mini mode already did. Honesty in what the rows claim (respecting #5578): a route that cannot prove its effective reasoning tier states no effort field instead of a placeholder that could never resolve (App::provable_reasoning_effort_label is the single gate); the cost segment is omitted only where the route itself cannot be priced (UsageChip::Unknown on BillingPresentation::Unknown) — cost: unknown stays wherever a price could exist. Absence in an older config.toml means full; an unknown preset is refused at parse time rather than guessed. Gates (run by the agent that authored this slice, on the pre-rebase tree; CARGO_PROFILE_DEV_DEBUG=0 to pin own artifacts in the shared target dir): - cargo fmt --all: clean - cargo clippy --workspace --all-targets --all-features --locked -- -D warnings (standing allowances): clean - targeted (config, tui::infoline, tui::phase_strip, tui::ui::frame): 1014 passed / 0 failed / 0 ignored - full cargo test -p codewhale-tui --lib --locked: 11857 passed / 0 failed / 13 ignored Rebased onto main 9c66003 by the operator; CI is the gate for the rebased tree. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_da6e0645-9b61-4ae8-b42a-1ee938c5fd47) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @Hmbown's task in 2m 37s —— View job Review: compact/hidden presets for the bottom chrome (#5950)
SummaryClean, well-scoped implementation. No correctness bugs found.
Minor (non-blocking) nit
Good documentation throughout (module docs, CHANGELOG in both root and |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| crate::route_billing::UsageChip::PricedSubtotal { .. } | ||
| | crate::route_billing::UsageChip::Unknown => { | ||
| UsageChip::Money(amount) => Some(amount.clone()), | ||
| UsageChip::Unknown if app.billing_presentation == BillingPresentation::Unknown => None, |
There was a problem hiding this comment.
🟡 Earlier unknown costs disappear
After an unpriced metered turn, switching to an unclassified route makes session_cost_label hide the session-wide unknown cost. Earlier billable usage remains unpriced.
Prompt for agents
Preserve the cumulative session-cost contract when deciding whether to omit an Unknown chip. session_cost_label currently checks only app.billing_presentation, which describes the active route, while App::cumulative_usage_chip returns Unknown for historical unpriced or legacy turns across the whole session. Distinguish an empty/current unpriceable route from unknown coverage already accumulated on earlier priceable routes, and omit only the former. Add a route-switch regression case: record an unpriced metered turn, switch billing_presentation to Unknown, and verify the metrics line still reports unknown cumulative cost.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - The bottom chrome no longer prints facts it cannot stand behind. A route | ||
| that cannot prove its effective reasoning tier — typically a custom | ||
| OpenAI-compatible gateway with no endpoint receipt — states no effort | ||
| field in the metrics line rather than the placeholder | ||
| `high→effective unavailable`; `/status` still tells the full story. The | ||
| cost segment is omitted only where the route itself cannot be priced | ||
| (`cost: unknown` stays on metered routes that merely lack a reading this | ||
| session, per #5578) (#5950). |
There was a problem hiding this comment.
🔍 Remove branch changelog edits
The contribution rules reserve both changelogs for batched updates on main. This PR modifies both files and creates avoidable merge conflicts.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🟢 Approval recommended
The change is well-scoped to TUI/config behavior with strong targeted tests, and the only issue found is a small doc-comment mismatch.
Pull request overview
Adds configurable size presets for the TUI’s two bottom-chrome rows (posture bar + metrics line), enabling compact/hidden modes without changing /statusline composition, and tightens the “honesty” rules so the chrome avoids rendering facts it cannot substantiate.
Changes:
- Introduces
[tui].posture_barand[tui].metrics_linepresets (full|compact|hidden) with live toggling via/config …and optional persistence via--save. - Implements
compactby starting existing shed ladders at a later rung (no second renderer), and implementshiddenby zeroing row height to return space to the transcript. - Omits unprovable reasoning-effort fields and omits the cost segment only for unpriceable routes, while keeping other surfaces’ reporting intact.
File summaries
| File | Description |
|---|---|
| docs/zh_hans/GUIDE.md | Documents the new bottom-chrome row presets in Simplified Chinese. |
| docs/GUIDE.md | Documents the new bottom-chrome row presets and their semantics. |
| crates/tui/src/tui/ui/tests.rs | Updates UI test fixtures to include the new config fields. |
| crates/tui/src/tui/ui/frame/one_owner_tests.rs | Adds frame-level tests for compact/hidden row preset behavior. |
| crates/tui/src/tui/ui/frame.rs | Applies compact/hidden presets in layout/render; adds “honesty” tests for effort/cost display. |
| crates/tui/src/tui/phase_strip/tideline_tests.rs | Adds posture-bar compact-mode tests to ensure only “posture” facts remain. |
| crates/tui/src/tui/phase_strip.rs | Omits unprovable effort in route identity; adds compact start rung support to TidelineFooter shedding. |
| crates/tui/src/tui/infoline/tests.rs | Adds compact-mode tests to ensure telemetry/help drop before width shedding. |
| crates/tui/src/tui/infoline.rs | Adds compact-mode support to the infoline shed pass and documentation. |
| crates/tui/src/tui/app/init.rs | Loads posture_bar / metrics_line from config with Full defaults when absent. |
| crates/tui/src/tui/app.rs | Adds posture_bar / metrics_line fields and the provable-effort gate helper. |
| crates/tui/src/lib.rs | Updates embedded config/test fixtures to include the new TUI config fields. |
| crates/tui/src/config/tests.rs | Adds parsing tests for the new ChromeRowPreset and backward compatibility (absent => full). |
| crates/tui/src/config.rs | Adds TuiConfig fields and defines ChromeRowPreset (`full |
| crates/tui/src/commands/groups/config/config.rs | Adds `/config posture_bar |
| crates/tui/CHANGELOG.md | Documents the new presets and the bottom-chrome “honesty” changes at the crate level. |
| config.example.toml | Documents the new [tui] keys and their allowed values/usages. |
| CHANGELOG.md | Documents the new presets and “honesty” changes in the repo-level changelog. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// The session cost as the metrics line prints it — the same price string | ||
| /// `/cost`, the roster's right column and the price widget print | ||
| /// (SHELL-DESIGN-20260901 §2.11 item 5). Empty until the session has a | ||
| /// priced or counted turn. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a6288f531
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| crate::route_billing::UsageChip::PricedSubtotal { .. } | ||
| | crate::route_billing::UsageChip::Unknown => { | ||
| UsageChip::Money(amount) => Some(amount.clone()), | ||
| UsageChip::Unknown if app.billing_presentation == BillingPresentation::Unknown => None, |
There was a problem hiding this comment.
When a paid route uses an unrecognized gateway or lacks enough credential/product evidence, BillingPresentation::Unknown means its billing basis is unknown, not that it cannot incur a charge. In that scenario cumulative_usage_chip() deliberately returns UsageChip::Unknown, but this arm removes cost: unknown from the metrics line entirely, hiding the only persistent indication that session cost is indeterminate. Keep the unknown chip unless there is affirmative Local or subscription evidence.
AGENTS.md reference: crates/tui/AGENTS.md:L8-L11
Useful? React with 👍 / 👎.
| return CommandResult::error(format!( | ||
| "{row_key} must be one of: {}", | ||
| crate::config::ChromeRowPreset::SETTINGS.join(", ") |
There was a problem hiding this comment.
For every non-English UI locale, invalid preset feedback—and the success/session-only responses later in this branch—remains hard-coded English. These user-visible command messages should use tr(locale, MessageId::...), leaving only setting names and command tokens composed in code.
AGENTS.md reference: crates/tui/AGENTS.md:L25-L26
Useful? React with 👍 / 👎.
| if row_key == "posture_bar" { | ||
| app.posture_bar = preset; | ||
| } else { | ||
| app.metrics_line = preset; | ||
| } |
There was a problem hiding this comment.
When /config posture_bar ... --save or /config metrics_line ... --save targets an unwritable or malformed config file, these assignments occur before persist_table_string_key can fail. The command therefore reports an error while silently applying the requested preset for the current session. Persist first and update the app only on success, or restore the previous value on failure.
Useful? React with 👍 / 👎.
| // rather than `high→effective unavailable` (#5950): a placeholder that | ||
| // can never resolve is noise, not a reading. First-party routes keep | ||
| // their tier, `auto: tier` and `req→eff` labels. | ||
| let effort = app.provable_reasoning_effort_label().unwrap_or_default(); |
There was a problem hiding this comment.
For a custom or otherwise unprovable route, this now converts the missing effort label to an empty string, but the existing candidate builder only includes the provider when effort is nonempty. Consequently even a wide metrics line changes from my-gateway · vendor-model-x · … to only vendor-model-x; the requested omission of the unverifiable effort field accidentally removes the still-known provider identity as well. Add a provider-plus-model candidate for this case.
Useful? React with 👍 / 👎.
| return match persist_table_string_key( | ||
| app.config_path.as_deref(), | ||
| "tui", | ||
| row_key, | ||
| value, |
There was a problem hiding this comment.
When Codewhale is launched with --profile work and that profile defines any [profiles.work.tui] table, merge_config selects the profile's entire TuiConfig over the base table. This call nevertheless writes the preset to the base [tui] table and reports success, so the live change works but the profile shadows it on the next launch and the supposedly saved preset reverts. Write through the active profile path or reject the save with an explicit scope message.
Useful? React with 👍 / 👎.
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
Adds tui.posture_bar and tui.metrics_line presets (full/compact/hidden), live /config control, and honesty gating for effort and cost labels. The implementation reuses existing shed ladders rather than adding a second renderer, and tests cover config parsing, live mutation, compact shedding, frame layout, and the new effort/cost honesty rules.
Findings
- [INFO] Only metrics_line --save persistence is covered; posture_bar save path lacks direct test (
crates/tui/src/commands/groups/config/config.rs:4876)
The new config test exercises live/config posture_bar compactand savingmetrics_line HIDDEN --save, but never savesposture_barwith--save. The two branches share a helper, but a regression in the posture_bar persist path or key name would not be caught. - [INFO] Case sensitivity differs between /config and config.toml (
crates/tui/src/config.rs:1883)
ChromeRowPreset::from_settingtrims and lowercases, so/config posture_bar COMPACTworks. However the serde-derivedDeserializewithrename_all = "snake_case"only accepts the exact lowercase values, so a hand-editedposture_bar = "COMPACT"in config.toml is rejected. Consider adding serde aliases/custom deserialization or explicitly documenting that TOML values must be lowercase.
Assessment
The PR is well-structured, heavily tested, and the behavior matches the stated design. No correctness defects were found in the changed paths; only minor test and documentation gaps are noted.
Advisory review by Codewhale (codewhale review --pr 5973 --post, head 8a6288f5312219e87af4d05e67918d2dc929f86f). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| } | ||
|
|
||
| /// The bottom-chrome row presets (#5950) apply on the next frame and | ||
| /// `--save` writes the `[tui]` key; an unknown preset names the three. |
There was a problem hiding this comment.
[INFO] Only metrics_line --save persistence is covered; posture_bar save path lacks direct test
The new config test exercises live /config posture_bar compact and saving metrics_line HIDDEN --save, but never saves posture_bar with --save. The two branches share a helper, but a regression in the posture_bar persist path or key name would not be caught.
| /// rows back to the transcript without touching `status_items`. | ||
| /// | ||
| /// `compact` is not a second renderer: it starts the row's existing shed | ||
| /// ladder at a fixed rung and lets width shed the rest, so what it keeps is |
There was a problem hiding this comment.
[INFO] Case sensitivity differs between /config and config.toml
ChromeRowPreset::from_setting trims and lowercases, so /config posture_bar COMPACT works. However the serde-derived Deserialize with rename_all = "snake_case" only accepts the exact lowercase values, so a hand-edited posture_bar = "COMPACT" in config.toml is rejected. Consider adding serde aliases/custom deserialization or explicitly documenting that TOML values must be lowercase.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0955a1de-3e5b-4c1d-8f67-6a5ae9082b1b) |
|
Claude finished @Hmbown's task in 3m 28s —— View job Review: compact/hidden presets for the bottom chrome (#5950)
SummaryOverall this is a clean, well-scoped feature (
Other bot findings I checked and downgraded:
Provider/model safety (EPIC #2608): no findings. Reuse/simplification: good — Tests: strong breadth (config round-trip, |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| crate::route_billing::UsageChip::PricedSubtotal { .. } | ||
| | crate::route_billing::UsageChip::Unknown => { | ||
| UsageChip::Money(amount) => Some(amount.clone()), | ||
| UsageChip::Unknown if app.billing_presentation == BillingPresentation::Unknown => None, |
There was a problem hiding this comment.
Confirmed bug: session-wide cost: unknown can be hidden by a later route switch.
session_cost_label gates omission on app.billing_presentation, but billing_presentation reflects only the currently active route (set in apply.rs/session_state.rs/provider_routes.rs on every route switch), not why cumulative_usage_chip() returned Unknown.
Trace: App::cumulative_usage_chip (crates/tui/src/tui/app.rs:2720) checks session-wide counters (cost_coverage_unknown_legacy, cost_unpriced_turns) before falling back to the route-based usage_chip(self.billing_presentation, ...). So a session that accrued a genuine unpriced turn on a Metered route (session.cost_unpriced_turns > 0) keeps returning UsageChip::Unknown even after the user switches to an unrelated BillingPresentation::Unknown route later. At that point this new guard hides the label — even though the Unknown chip is reporting real, unrelated session history, not something about the currently-unclassified route.
This PR's own test (cost_is_omitted_only_where_the_route_cannot_be_priced, added just below) demonstrates the same session state (cost_coverage_unknown_legacy = true) producing "cost: unknown" under Metered and "" under Unknown — i.e. the label depends on the current route even though the underlying unknown-coverage fact hasn't changed.
Fix direction: only suppress when the session itself has no priced/unpriced/legacy coverage at all (i.e., cumulative_usage_chip() reached Unknown via the final route-based fallback, not the earlier session-counter branches) — e.g. have cumulative_usage_chip distinguish "no session coverage, route itself unpriceable" from "session has real unknown coverage" as separate UsageChip states, or have session_cost_label check the session counters directly instead of the current route's billing_presentation.
| // rather than `high→effective unavailable` (#5950): a placeholder that | ||
| // can never resolve is noise, not a reading. First-party routes keep | ||
| // their tier, `auto: tier` and `req→eff` labels. | ||
| let effort = app.provable_reasoning_effort_label().unwrap_or_default(); |
There was a problem hiding this comment.
Confirmed bug: provider identity is silently dropped, not just the effort field.
route_identity_fields gates both the [Provider, Model, Effort] candidate (line 67: !provider.is_empty() && !effort.is_empty()) and the [Model, Effort] candidate (line 76: !effort.is_empty()) on effort being non-empty. Before this PR, effort came from reasoning_effort_display_label(), which was always non-empty (even the "unavailable" case produced high→effective unavailable), so these gates were effectively always true whenever there was width for them.
Now that provable_reasoning_effort_label() can legitimately return None → effort == "" for an unprovable route, both gates fail and the code falls straight through to candidates.push(vec![field(RouteFieldKind::Model, model)]) — dropping the provider field entirely, even at ShellTier::Wide with a large budget where provider · model would easily fit. The PR intends to omit only the effort field ("no effort field... First-party routes keep their tier"), not the provider.
The added test unprovable_effort_states_no_field_instead_of_a_placeholder (crates/tui/src/tui/ui/frame.rs) sets app.set_provider_identity(ApiProvider::Custom, "my-gateway") but never asserts "my-gateway" appears in the row — this gap is why the regression wasn't caught.
Fix direction: add a [Provider, Model] candidate (no effort field) for tier != ShellTier::Compact && !provider.is_empty() && effort.is_empty(), so the provider survives when only the effort is unprovable.
…ake; passes on re-roll) Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1cd0fc17-041a-41d9-8b32-2807c7ff68de) |
|
Claude finished @Hmbown's task in 1m 56s —— View job Review: compact/hidden presets for the bottom chrome (#5950)
SummaryRe-checked Correctness bugs
Both were already flagged inline by Devin, Codex, and a prior Claude pass on this same commit — confirming again because they remain unfixed and are the only correctness-blocking findings. Provider/model/route safety (EPIC #2608)No issues. Reuse / simplificationGood. Non-blocking, previously noted by other reviewers, not re-litigating:
TestsStrong breadth otherwise (config round-trip, |
There was a problem hiding this comment.
Codewhale review
Adds tui.posture_bar and tui.metrics_line presets (full | compact | hidden), wired through config parsing, app init, /config live updates, and frame rendering. Also changes effort and cost display so unprovable/unpriceable facts are omitted instead of shown as placeholders. The implementation is well-structured and the targeted tests cover the main behavior.
Findings
- [INFO] Compact metrics balance retention lacks a direct regression test (
crates/tui/src/tui/infoline/tests.rs)
The compact metrics path drops segments withshed_priority >= SHED_BEFORE_HELPand the PR states that the balance segment should survive compact mode. The new infoline compact test only asserts route, context, and cost are kept;work_segments()appears to omit balance, so a misclassifiedshed_priorityfor the Balance segment could go unnoticed.
Assessment
No blocking correctness issues found. The change is localized, follows the existing shed-ladder design, and includes broad tests. The main gap is a small test-coverage hole for the compact metrics balance claim.
Advisory review by Codewhale (codewhale review --pr 5973 --post, head edfb317414ba99148135043c0847ec9f10378032). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
Closes #5950
The second half of #5950 — the
/statuslinecomposition half landed as #5962.What this adds
[tui].posture_barand[tui].metrics_lineeach acceptfull | compact | hidden(defaultfull), also settable live via/config posture_bar compact(--savewrites the[tui]key; a session-only set says so; an unknown preset names the three and changes nothing; absence in an older config.toml meansfull).compact is not a second renderer. Each row keeps its existing shed ladder; the preset only starts it at a fixed rung — what compact keeps is exactly what a narrow row keeps:
render_tideline_footerstarts atCOMPACT_SHED— clocks, hint and live counts go first; the permission chip, mode chip and cap warning stay.shed_passstarts with every segment at or aboveSHED_BEFORE_HELP— route, context reading, cost and balance stay; telemetry goes.hidden gives the row to the transcript (
info_height/footer_height→ 0, exactly as mini mode already did); no other row moves, so the composer is never displaced.Honesty (#5578): a route that cannot prove its effective reasoning tier states no effort field instead of a placeholder that could never resolve (
App::provable_reasoning_effort_label()is the single gate); the cost segment is omitted only where the route itself cannot be priced (UsageChip::UnknownonBillingPresentation::Unknown) —cost: unknownstays wherever a price could exist, because there the words are a reading, not noise.Gates
cargo fmt --all: cleancargo clippy --workspace --all-targets --all-features --locked -- -D warnings(standing allowances): cleancargo test -p codewhale-tui --lib --locked: 11857 passed / 0 failed / 13 ignoredAuthored and gated by an agent session (CARGO_PROFILE_DEV_DEBUG=0 to pin own artifacts in the shared target dir); rebased onto current main (
9c66003ff) and pushed by the operator. CI is the gate for the rebased tree.This slice was carried to PR by the takeover session after its authoring agent terminated post-commit.
Note
Low Risk
Changes are confined to TUI layout presets and display-only metrics text, with broad unit coverage and no auth or billing execution paths touched.
Overview
Adds
[tui].posture_barand[tui].metrics_linepresets (full|compact|hidden, defaultfull), wired fromconfig.toml, app init, and live/config … --save.hiddensets the posture/metrics row heights to zero so the transcript keeps those lines;compactdoes not add a second renderer—it starts each row’s existing shed ladder one rung in (posture: permission/mode/cap warning only; metrics: route, context, cost/balance without telemetry or the help hint).The bottom chrome also stops stating unprovable facts: routes that cannot prove an effective reasoning tier omit the effort field in the metrics route segment (via
provable_reasoning_effort_label) instead ofhigh→effective unavailable, while/statusstill shows the full label. Session cost is dropped from the line only whenBillingPresentation::Unknown;cost: unknownremains on metered routes that simply lack a session reading.Docs/changelog and targeted tests cover config parsing, infoline/phase_strip shedding, composed frame layout, and the effort/cost honesty rules.
Reviewed by Cursor Bugbot for commit edfb317. Bugbot is set up for automated code reviews on this repo. Configure here.