Repository navigation
feat(tui): the work surface sits under the composer by default - #5809
Conversation
Round 3 of the shell design (codewhale-ops/design/SHELL-DESIGN-20260901.md
§2.0): the bar's information — posture, agent roster, to-do — lives UNDER
the composer, so scrolling up reads as intentional history and nothing
paints above the stage by default.
- `WorkSurfacePlacement::Bottom`: a strip placement that shares Top's
auto-fit height math (`is_strip()`) and row machinery; only its slot in
the frame differs (after the composer, before the footer). Its divider
is the strip's top edge, so a drag on it inverts Top's delta; the body
starts one row below the divider so row hitboxes stay honest.
- `bottom` is the default (`work_surface_placement`, `WorkSurfaceState`,
`/rail on`, the rail hotbar toggle and Ctrl+Alt+0 all restore it) and
the fallback for unknown values. Narrow terminals keep Bottom; only the
side rails still fall back to Top below the side-rail width.
- `/config work_surface_placement bottom` is accepted; the error text
lists the five placements.
Upgrade note: the default for `work_surface_placement` changes from
`left` to `bottom`. Users who set it explicitly are untouched.
Legacy ui/sidebar/mouse test fixtures pin `Top` because they assert the
old strip-above-transcript geometry; the Bottom default has its own
coverage (`bottom_placement_draws_the_work_strip_under_the_composer`,
`rail_panels_render_in_all_placements` now includes Bottom).
No goldens changed: the startup goldens carry no work rows, so the strip
is zero-height in both placements.
Evidence:
scripts/dev-test.sh tui (cargo test -p codewhale-tui --lib):
test result: FAILED. 11848 passed; 3 failed; 13 ignored
(the 3 — runtime_api::omitted_runtime_models_use_the_active_provider_default,
runtime_threads::approval_required_awaits_external_decision_allow,
sandbox::seatbelt::file_provider_synthetic_operations_are_independent_under_seatbelt —
are unrelated to this change and pass in isolation:
test result: ok. 3 passed; 0 failed)
focused rerun after the last edits:
test result: ok. 11 passed; 0 failed; 0 ignored
cargo clippy --workspace --all-targets -D warnings (CI allows): clean
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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_1422a1c1-840d-4721-b46f-ea0d5cdc0df6) |
|
Claude finished @Hmbown's task in 4m 9s —— View job Review: work surface moves under the composer by default (#5809)
SummaryClean, well-documented placement/default change. Critical: chat/workflow/preview/indicator render into the wrong slots when the strip is Bottom
Net effect for the new default (Bottom): the transcript ( Why this wasn't caught:
Suggested fix: compute Everything else looks solid
|
There was a problem hiding this comment.
🟡 Changes recommended
The Bottom layout reorders body_chunks but leaves downstream fixed indices in frame.rs, which can render chat/workflow/preview/indicator into incorrect slots under Bottom placement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the TUI “work surface” (tasks/agents/to-do rail) so it defaults to a bottom strip under the composer, while preserving existing placement options and updating commands/settings/tests accordingly.
Changes:
- Introduces
WorkSurfacePlacement::Bottomas a strip placement (shared strip behavior viais_strip()), and makes it the default. - Reworks layout/rendering, mouse resizing, and commands/hotbar toggles so “rail on/show” restores the bottom placement and Bottom renders as a top-edge divider strip.
- Updates settings normalization/defaults and extends test coverage for Bottom placement geometry and command behavior.
File summaries
| File | Description |
|---|---|
| crates/tui/src/tui/work_surface/render/mod.rs | Adds Bottom body/divider split and treats Bottom like Top via is_strip() for strip-only rendering policies. |
| crates/tui/src/tui/work_surface/render/layout.rs | Adjusts effective placement fallback so only side rails fall back on narrow terminals; treats Bottom as a strip. |
| crates/tui/src/tui/work_surface/model.rs | Adds Bottom variant, is_strip(), updates parsing/defaults, and extends strip-only projection behavior. |
| crates/tui/src/tui/work_surface/mod.rs | Extends placement coverage in work-surface tests to include Bottom. |
| crates/tui/src/tui/work_surface/input.rs | Adds Bottom divider hitbox and inverted drag behavior for resizing the bottom strip. |
| crates/tui/src/tui/ui/tests.rs | Pins legacy geometry tests to Top and adds a Bottom-vs-Top composer/strip geometry test; updates Ctrl+Alt+0 expectation to Bottom. |
| crates/tui/src/tui/ui/frame.rs | Reorders vertical layout slots to place the strip below the composer when configured Bottom. |
| crates/tui/src/tui/ui/apply.rs | Changes Ctrl+Alt+0 restore target from Top to Bottom and updates status message. |
| crates/tui/src/tui/ui.rs | Pins provider-key validation test app geometry to Top to avoid bottom-default changes affecting legacy assertions. |
| crates/tui/src/tui/sidebar.rs | Pins sidebar test app geometry to Top for legacy geometry expectations. |
| crates/tui/src/tui/mouse_ui.rs | Pins mouse UI test app geometry to Top for legacy geometry expectations. |
| crates/tui/src/tui/hotbar/actions.rs | Updates rail toggle action to restore Bottom (default) and adjusts tests/messages. |
| crates/tui/src/settings.rs | Changes default placement to bottom, updates normalization/validation/help text, and updates persistence/migration tests. |
| crates/tui/src/commands/mod.rs | Updates command test expectation so “on” restores Bottom. |
| crates/tui/src/commands/groups/config/config.rs | Updates /rail parsing/status messaging and tests to accept/restore Bottom. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let strip_below = | ||
| app.work_surface.placement == crate::tui::work_surface::WorkSurfacePlacement::Bottom; | ||
| let (constraints, strip_slot, composer_slot, footer_slot) = if strip_below { | ||
| ( | ||
| vec![ | ||
| Constraint::Min(1), // Chat area | ||
| Constraint::Length(workflow_panel_height), // Workflow panel (#4121) | ||
| Constraint::Length(preview_height), // Pending input preview (0 if empty) | ||
| Constraint::Length(indicator_height), // Background-work chip (#5286, 0 if idle) | ||
| Constraint::Length(plugin_cta_height), // Live plugin CTA (0 unless matched) | ||
| Constraint::Length(composer_height), // Composer | ||
| Constraint::Length(top_work_strip_height), // Tasks + To-do below composer | ||
| Constraint::Length(footer_height), // Merged Tideline footer (slots 6+8) | ||
| ], | ||
| 6usize, | ||
| 5usize, | ||
| 7usize, | ||
| ) | ||
| } else { |
| ( | ||
| "work_surface_placement", | ||
| "Ocean Tasks/To-do/Workers rail placement: top/left/right", | ||
| "Ocean Tasks/To-do/Workers rail placement: bottom (default)/top/left/right", | ||
| ), |
Buildkite 1535 on #5809: two cucumber PTY tests against the real binary failed under the new Bottom default — at 40x12 a queued message had no `Queued #1:` preview, and the plugin acceptance stage stayed blank. Cause: the Bottom layout dropped the strip's leading slot and re-ordered the constraint list, but the stage, workflow panel, pending-input preview and background chip are addressed by fixed slot index (`body_chunks[1..4]`), so under Bottom they drew into the wrong, zero-height slots. Fix: one nine-slot layout in both placements. The strip owns a slot at each end (above the stage for `top`, under the composer for `bottom`) and only one has height, so every other slot keeps its index. The dual constraint list is gone. Test: `bottom_placement_keeps_the_stage_and_queued_preview_at_twelve_rows` renders a 40x12 frame under Bottom with a transcript row and a queued message and asserts both paint. Evidence: cargo test -p codewhale-tui --all-features --locked --test cucumber -- \ active_composer_pointer_submit_queues_without_provider \ plugin_toml_binary_lifecycle_skill_and_stdio_mcp_acceptance test result: ok. 2 passed; 0 failed; 0 ignored (before the fix, the pointer test failed exactly as on Buildkite: "40x12: queued preview ... not visible after pointer submit") cargo test -p codewhale-tui --lib -- work_surface tui::ui::tests test result: ok. 808 passed; 0 failed; 0 ignored cargo test -p codewhale-tui --lib -- bottom_placement test result: ok. 3 passed; 0 failed; 0 ignored cargo clippy -p codewhale-tui --all-targets -D warnings (CI allows): clean 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_595c0d9f-f0ce-4b25-9396-39c8b85a7432) |
|
Claude finished @Hmbown's task in 3m 38s —— View job Review: work surface moves under the composer by default (#5809)
The previously-flagged critical bug is fixedCommit New finding: narrow terminals can still silently demote Bottom to Top
But let placement = effective_placement(app.work_surface.placement, area.width);
app.work_surface.effective_placement = placement;
if placement == WorkSurfacePlacement::Top || placement == WorkSurfacePlacement::Off {
return (area, None); // Bottom does NOT take this early return
}
...
let rail_width = app.work_surface.side_width.clamp(SIDE_WIDTH_MIN, SIDE_WIDTH_MAX)
.min(area.width.saturating_sub(min_chat_width));
if rail_width < model::SIDE_WIDTH_MIN {
app.work_surface.effective_placement = WorkSurfacePlacement::Top; // clobbers Bottom too
collapse_strip(app);
return (area, None);
}Because the early-return guard checks only This directly contradicts the PR's own stated behavior ("Narrow terminals no longer force bottom → top—only left/right still fall back to a top strip when too narrow") and re-introduces a version of the bug the other fix just closed, one layer down:
Suggested fix: widen the early-return guard in if placement.is_strip() || placement == WorkSurfacePlacement::Off {
return (area, None);
}so Test gap: none of the existing tests catch this. Minor: settings help text still doesn't list
|
There was a problem hiding this comment.
Codewhale review
PR #5809 adds a WorkSurfacePlacement::Bottom variant, makes it the default, updates the TUI frame layout to place the work surface strip below the composer, and adjusts rail/config/hotbar/input/render paths accordingly. Test coverage includes new bottom-placement ordering and 40x12 regression tests.
Findings
- [INFO] WorkSurfacePlacement::parse does not explicitly match "bottom" (
crates/tui/src/tui/work_surface/model.rs:102)
The match arm for "bottom" is missing; the string falls through to_ => Self::Bottom. This works for now, but any unrecognized value silently becomes Bottom. Adding an explicit arm improves clarity and prevents accidental silent acceptance of typos. - [INFO] Footer slot comment is inaccurate after layout change (
crates/tui/src/tui/ui/frame.rs)
The comment on the footer constraint still readsMerged Tideline footer (slots 6+8), but the footer now occupies slot 8 only after inserting the bottom strip slot. The comment should be updated to avoid confusion.
Suggestions
-
crates/tui/src/tui/work_surface/model.rs:102— Add an explicit match arm for "bottom" so valid placements are enumerated and unknown values are clearly the fallback."top" => Self::Top, "bottom" => Self::Bottom,
Assessment
The PR is well-structured and consistently updates all relevant code paths. Functional behavior appears correct, with dedicated tests for the new bottom placement and the 40x12 regression. Only minor clarity improvements are suggested.
Advisory review by Codewhale (codewhale review --pr 5809 --post, head b5077aa844a644702232bbdb370ea2d06830aaf3). 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.
| #[must_use] | ||
| pub fn parse(value: &str) -> Self { | ||
| match value.trim().to_ascii_lowercase().as_str() { | ||
| "top" => Self::Top, |
There was a problem hiding this comment.
[INFO] WorkSurfacePlacement::parse does not explicitly match "bottom"
The match arm for "bottom" is missing; the string falls through to _ => Self::Bottom. This works for now, but any unrecognized value silently becomes Bottom. Adding an explicit arm improves clarity and prevents accidental silent acceptance of typos.
| #[must_use] | ||
| pub fn parse(value: &str) -> Self { | ||
| match value.trim().to_ascii_lowercase().as_str() { | ||
| "top" => Self::Top, |
There was a problem hiding this comment.
Add an explicit match arm for "bottom" so valid placements are enumerated and unknown values are clearly the fallback.
| "top" => Self::Top, | |
| "top" => Self::Top, | |
| "bottom" => Self::Bottom, |
…umber hang, mobile smoke, release receipts) - Windows warnings (§5.1): restructure link and error bindings in document.rs and runtime_api/tests.rs so variables exist only on targets that use them. Note: Windows CI cannot be run locally on this host. - macOS PTY acceptance (§5.2): in plugin_e2e_acceptance, iterate over all marker match indices in review_confirmation_in_text rather than only the first, preventing false stalls on the composer's typed command echo. Full 31/31 cucumber tests passed. - Mobile smoke (§5.3): update scripts/mobile-smoke.sh Test Group 3 to assert loopback security warning, matching the loopback-only hardening from e80be8c. 9/9 passed. - Release-note receipts (§5.4): cite genuine issues (codewhale-hq#5533, codewhale-hq#5831, codewhale-hq#5825, codewhale-hq#5829, codewhale-hq#5801, codewhale-hq#5815, codewhale-hq#5826, codewhale-hq#5286, codewhale-hq#5809, codewhale-hq#5778, codewhale-hq#5725, codewhale-hq#5701, codewhale-hq#5712, codewhale-hq#5747, codewhale-hq#5782, codewhale-hq#4394) in CHANGELOG.md; filter commit-body scrape noise (#000, codewhale-hq#1, codewhale-hq#24, codewhale-hq#26, codewhale-hq#1834, #142352) in check-feature-release-notes.sh. check-versions passes in strict release mode (71 receipts checked). Evidence: - cargo nextest run --workspace --all-features --locked: 14229 passed; 0 failed; 15 skipped - cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: clean (0 warnings) - cargo test --workspace --all-features --doc --locked: 3 passed; 0 failed; 8 ignored - cargo test -p codewhale-tui --test cucumber --features long-running-tests --locked: 31 passed; 0 failed; 0 ignored - scripts/mobile-smoke.sh: 9 passed; 0 failed - scripts/release/check-versions.sh --require-dated-release: Feature release-note receipts OK (71 linked issues); Version state OK - Python policy gates: check-provider-registry, check-command-crate-boundaries, check-dead-code-budget, check-runtime-contract-budget, check-readme-translations, check-tui-locale-parity all PASS


What changed for the user. The tasks / agents / to-do strip now sits under the composer by default instead of above the transcript (or as a left rail).
/rail on, the rail hotbar toggle and Ctrl+Alt+0 restore that bottom placement;/config work_surface_placement bottomis accepted;top,left,right,offstill work as before.Why. Round 3 of the shell design (
codewhale-ops/design/SHELL-DESIGN-20260901.md§2.0): the bar's information — posture, agent roster, to-do — lives under the composer so scrolling up reads as intentional history and nothing paints above the stage in the default working shell. This slice is only the placement + default; the launch header and the braille mark are separate slices.How.
WorkSurfacePlacement::Bottomis a strip placement sharing Top's auto-fit height math (is_strip()) and row machinery; only its frame slot differs (after the composer, before the footer). Its divider is the strip's top edge (drag delta inverted vs Top) and the body starts one row below it so row hitboxes stay honest. Narrow terminals keep Bottom; only the side rails still fall back to Top under the side-rail width.Upgrade note. The default for
work_surface_placementchanges fromlefttobottom. Users who set it explicitly are untouched.Goldens. None re-blessed: the startup goldens carry no work rows, so the strip is zero-height in both placements. Legacy ui/sidebar/mouse test fixtures pin
Topbecause they assert the old geometry; Bottom has its own coverage (bottom_placement_draws_the_work_strip_under_the_composer;rail_panels_render_in_all_placementsnow includes Bottom).Evidence.
Note
Medium Risk
Touches core TUI frame layout and persisted defaults, so wrong slot indexing could break transcript/composer rendering; scope is mostly placement and settings with broad test updates.
Overview
Round 3 shell change: the tasks/agents rail defaults to
bottom(under the composer, above the footer) instead of a left rail or top strip. NewWorkSurfacePlacement::Bottomshares top-strip auto-fit and row rendering viais_strip(); only frame slot order and divider edge differ.User-facing behavior:
/rail on, sidebar hotbar toggle, and Ctrl+Alt+0 restore bottom placement./config work_surface_placement bottomis valid;top/left/right/offunchanged. Narrow terminals keep bottom; only side rails still fall back to top when too narrow.Layout fix: vertical body constraints gain a slot below the composer so stage, preview, and queued message rows keep non-zero height at small sizes (regression #5809). Bottom-strip resize uses inverted drag delta on the strip’s top divider.
Defaults & tests: fresh
work_surface_placementdefault moves fromlefttobottom; unknown placement normalizes to bottom. Legacy UI tests pinTopgeometry; new tests assert strip vs composer ordering and 40×12 queued preview under bottom placement.Reviewed by Cursor Bugbot for commit b5077aa. Bugbot is set up for automated code reviews on this repo. Configure here.