Skip to content

feat(tui): the work surface sits under the composer by default - #5809

Merged
Hmbown merged 2 commits into
mainfrom
feat/work-surface-bottom-20260901
Sep 2, 2026
Merged

Hmbown merged 2 commits into
mainfrom
feat/work-surface-bottom-20260901

Conversation

@Hmbown

@Hmbown Hmbown commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

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 bottom is accepted; top, left, right, off still 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::Bottom is 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_placement changes from left to bottom. 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 Top because they assert the old geometry; Bottom has its own coverage (bottom_placement_draws_the_work_strip_under_the_composer; rail_panels_render_in_all_placements now includes Bottom).

Evidence.

scripts/dev-test.sh tui  (cargo test -p codewhale-tui --lib)
test result: FAILED. 11848 passed; 3 failed; 13 ignored; 0 measured; 0 filtered out; finished in 271.13s
  # 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 diff and pass in isolation:
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 11861 filtered out; finished in 2.68s
focused rerun after the last edits:
test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 11854 filtered out; finished in 0.60s
cargo clippy --workspace --all-targets -- -D warnings (CI's -A flags): clean

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. New WorkSurfacePlacement::Bottom shares top-strip auto-fit and row rendering via is_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 bottom is valid; top / left / right / off unchanged. 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_placement default moves from left to bottom; unknown placement normalizes to bottom. Legacy UI tests pin Top geometry; 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.

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>
Copilot AI lite review requested due to automatic review settings September 2, 2026 01:04
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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

claude Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 4m 9s —— View job


Review: work surface moves under the composer by default (#5809)

  • Gather context (read changed files; diffed against origin/main was unavailable — shallow checkout has no common ancestor locally, so review is based on full-file reads of all 15 changed files)
  • Understand request (code review of this PR)
  • Correctness/bugs
  • Provider/model/route safety — N/A, this is a TUI layout PR with no model/provider routing code
  • Reuse/simplification
  • Test coverage
  • Security — N/A
  • Post review

Summary

Clean, well-documented placement/default change. WorkSurfacePlacement::Bottom, is_strip(), settings migration/normalization, /rail and /config wiring, the hotbar toggle, and the divider drag-delta inversion in work_surface/input.rs are all correct and consistent with each other. However, I found a critical layout bug in the new frame body-chunk indexing.

Critical: chat/workflow/preview/indicator render into the wrong slots when the strip is Bottom

crates/tui/src/tui/ui/frame.rs (around lines 1226–1260) builds two different constraints vectors depending on strip_below:

  • Bottom (now the default): [Chat(0), Workflow(1), Preview(2), Indicator(3), PluginCta(4), Composer(5), Strip(6), Footer(7)]
  • Top/other: [Strip(0), Chat(1), Workflow(2), Preview(3), Indicator(4), PluginCta(5), Composer(6), Footer(7)]

strip_slot, composer_slot, and footer_slot (and the derived plugin_cta_slot = composer_slot - 1) are correctly parameterized per branch. But the rest of the render body still uses hardcoded literal indices that only match the Top/other ordering:

  • body_chunks[1] for work_chat_area/split_chat (line ~1268–1274) — for Bottom this is actually the Workflow panel slot, not Chat (Min(1), which sits at index 0 and is never read).
  • body_chunks[2] for the workflow panel render (line ~1365) — for Bottom this is actually the Preview slot.
  • body_chunks[3] for the pending-input preview render (line ~1385) — for Bottom this is actually the Indicator slot.
  • body_chunks[4] for the background-work indicator render (line ~1392) — for Bottom this is actually the PluginCta slot.
  • The post-render paint_matching calls at body_chunks[2]/body_chunks[3] (lines ~1512–1513) have the same mismatch.

Net effect for the new default (Bottom): the transcript (ChatWidget) is rendered into body_chunks[1], which is Constraint::Length(workflow_panel_height) — normally 0 since no workflow panel is active — while the large Min(1) area actually reserved for chat at index 0 is never painted into. The workflow panel, pending-input preview, and background-work chip are similarly rendered one slot off. This would visibly break (or zero-height) the main conversation view under the new default placement.

Why this wasn't caught:

  • bottom_placement_draws_the_work_strip_under_the_composer (crates/tui/src/tui/ui/tests.rs:4412) only asserts the strip's y relative to the composer's bottom()/y — both of which come from the correctly-parameterized strip_slot/composer_slot, so it passes regardless of this bug.
  • rail_panels_render_in_all_placements (crates/tui/src/tui/work_surface/mod.rs:1879) calls work_surface::render directly, not the full-frame ui::frame::render, so it never exercises body_chunks at all.
  • Nearly every other UI test explicitly pins app.work_surface.placement = WorkSurfacePlacement::Top in its create_test_app() helper specifically to preserve "legacy strip geometry" — which incidentally routes around the new Bottom branch entirely, so the mismatched indices are never exercised by the existing suite.

Suggested fix: compute chat_slot/workflow_slot/preview_slot/indicator_slot the same way strip_slot/composer_slot/footer_slot are already computed (as part of the same tuple/struct per branch), and replace every hardcoded body_chunks[1..4] literal with the corresponding named slot. A render-level test asserting last_composer_area sits below a non-trivial chat_area/transcript rect under the default (Bottom) placement would have caught this.

Fix this →

Everything else looks solid

  • WorkSurfacePlacement::is_strip(), effective_placement fallback, top_cap/ambient_cap, and project_visible bucketing are unchanged in spirit — only Bottom is added alongside Top, consistently.
  • render/mod.rs's body_area/divider split for Bottom (divider at area.top(), body starts one row below) is the correct mirror of Top's split.
  • work_surface/input.rs's mouse divider hit-test and inverted drag delta for Bottom are correct (resize_anchor_row - mouse.row vs Top's mouse.row - resize_anchor_row).
  • settings.rs migration/normalization is thorough: normalize_work_surface_placement defaults unknown values to "bottom", work_surface_placement_explicit correctly gates the sidebar→rail migration, and there's direct round-trip test coverage (work_surface_placement_persists_all_placements_with_bottom_default, settings_load_migrates_unreadable_top_work_surface_height).
  • /rail, /config work_surface_placement, the hotbar SidebarToggle action, and Ctrl+Alt+0 all consistently restore Bottom (not the old Left/Top) when toggled back on, with matching tests.
  • No provider/model routing code touched; no security-relevant surface changed.

Branch: feat/work-surface-bottom-20260901

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Not approved: Cursor Bugbot was present but skipped (usage limit), so the automated review signal is incomplete. Human review is needed; one reviewer was assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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.

🟡 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::Bottom as a strip placement (shared strip behavior via is_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.

Comment on lines +1226 to +1244
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 {
Comment on lines 1779 to 1782
(
"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>
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not approved: Cursor Bugbot was present but skipped (usage limit), so the automated review signal is incomplete. Human review is needed; one reviewer was assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@claude

claude Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

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


Review: work surface moves under the composer by default (#5809)

  • Gather context (PR body, prior bot reviews, changed files, new commit since last pass)
  • Understand request (code review of current PR state)
  • Correctness/bugs
  • Provider/model/route safety — N/A, TUI layout PR, no provider/model routing code
  • Reuse/simplification
  • Test coverage
  • Security — N/A
  • Post review

The previously-flagged critical bug is fixed

Commit b5077aa ("the bottom strip keeps every other frame slot in place") already fixes the body_chunks index mismatch that the earlier review and Copilot both flagged: frame.rs now uses one 9-slot layout for both placements (chat/workflow/preview/indicator/plugin-cta/composer/footer always at the same index; only the strip's slot — 0 for Top, 7 for Bottom — and its height move), and strip_slot/plugin_cta_slot/composer_slot/footer_slot are consistent literals matching that layout. Verified by re-reading every body_chunks[N] use in crates/tui/src/tui/ui/frame.rs — all now point at the right slot in both placements.

New finding: narrow terminals can still silently demote Bottom to Top

WorkSurfacePlacement::effective_placement() (crates/tui/src/tui/work_surface/render/layout.rs:17-32) is correctly scoped: only Left/Right fall back to Top under SIDE_RAIL_MIN_HOST_WIDTH (72 cols); Bottom is exempt, matching the PR's stated goal ("narrow terminals keep Bottom").

But split_chat (layout.rs:203-259) re-derives effective_placement and has its own, separate narrow-width fallback that is not scoped to side rails:

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 Top/Off, a configured Bottom placement falls through into the side-rail width arithmetic. With the default side_width (30) and the caller's min_chat_width floor (rail_min_chat_width(false) == 0, then .max(SIDE_RAIL_MIN_CHAT_WIDTH) = 40 inside split_chat), rail_width < 26 whenever area.width < 66 — so any terminal narrower than ~66 columns with visible work rows flips effective_placement from Bottom back to Top, even though height() (called earlier in frame.rs) already committed to Bottom and reserved the strip's row budget at the bottom slot.

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:

  • work_surface::render (render/mod.rs:55,61-72) reads effective_placement to choose the body/divider split direction. With it wrongly Top, the divider paints at area.bottom()-1 (the edge next to the footer) instead of area.top() (the edge next to the composer), and the content rows shift by one row inside a rect that frame.rs still positioned as the bottom strip slot — a visibly broken divider/row placement, not just a mislabeled status readout.
  • work_surface::input.rs:162 also reads effective_placement for the divider hit-test and drag-delta inversion, so mouse resize would use the wrong (Top) drag direction in the same narrow-width condition.

Suggested fix: widen the early-return guard in split_chat to match is_strip():

if placement.is_strip() || placement == WorkSurfacePlacement::Off {
    return (area, None);
}

so Bottom never enters the side-rail width computation at all — it doesn't reserve columns, so that arithmetic was never meant to apply to it.

Test gap: none of the existing tests catch this. empty_work_reserves_no_side_rail and side_rail_yields_the_columns_the_idle_ocean_needs (work_surface/mod.rs, ui/tests.rs) only exercise Left/Right at narrow widths; rail_panels_render_in_all_placements covers Bottom but only at a 100-column area. A regression test calling split_chat with placement = Bottom, populated rows, and area.width in the 40–65 range — asserting (chat, rail) == (area, None) and effective_placement() == Bottom — would have caught this.

Fix this →

Minor: settings help text still doesn't list off

Copilot's earlier comment on settings.rs:1781 is still unaddressed — the /config help string reads "Ocean Tasks/To-do/Workers rail placement: bottom (default)/top/left/right", omitting off even though it's a valid, documented value elsewhere. Cosmetic, low priority.

Everything else holds up

  • WorkSurfacePlacement::is_strip(), the Bottom divider mirror in render/mod.rs (top edge vs. Top's bottom edge), and the inverted drag delta in input.rs are correct in isolation.
  • Settings default/migration/normalization (bottom default, unknown values normalize to bottom), /rail, /config work_surface_placement, the hotbar toggle, and Ctrl+Alt+0 all consistently restore Bottom.
  • The new regression test bottom_placement_keeps_the_stage_and_queued_preview_at_twelve_rows is well-targeted at the bug it closes (PTY-observed at 40×12).
  • No provider/model routing code touched.
    · Branch: feat/work-surface-bottom-20260901

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 reads Merged 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add an explicit match arm for "bottom" so valid placements are enumerated and unknown values are clearly the fallback.

Suggested change
"top" => Self::Top,
"top" => Self::Top,
"bottom" => Self::Bottom,

@Hmbown
Hmbown merged commit f77ff18 into main Sep 2, 2026
30 of 33 checks passed
@Hmbown
Hmbown deleted the feat/work-surface-bottom-20260901 branch September 2, 2026 01:47
Garfield1985 pushed a commit to Garfield1985/DeepSeek-TUI that referenced this pull request Sep 5, 2026
…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
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