Skip to content

fix(tui): stop overdraw across Setup Report, /diff overlay, slash palette, outro#779

Merged
kelsonpw merged 1 commit into
mainfrom
fix/tui-region-overdraw
May 14, 2026
Merged

fix(tui): stop overdraw across Setup Report, /diff overlay, slash palette, outro#779
kelsonpw merged 1 commit into
mainfrom
fix/tui-region-overdraw

Conversation

@kelsonpw
Copy link
Copy Markdown
Member

@kelsonpw kelsonpw commented May 14, 2026

Summary

Five user-reported overdraw bugs against the post-run Outro screen
share two root causes. This PR fixes both and adds regression tests.

The bugs (from live screenshots)

  • Bug A[O] Open in browser · [Esc] Back hint strip rendering INSIDE the Setup Report's events table header row
  • Bug B/diff slash command output rendering on top of the visible Setup Report content
  • Bug C — Slash command palette description column collides with the next row's description (/login tRe-authenticateter region (US or EU))
  • Bug D — DiffViewer summary file list rows mashed onto a single line (NEW report.md · +78/-0AddToLibrary.tsx · +2/-0…)
  • Bug E — Outro success bullets visually overlap (tracking plang Yarn V1)
  • Bug F — Picker rows merging with each other and the dashboard URL ([3] Exit setup reportmplitude.com))

Root causes

  1. ReportViewer claimed height={visibleLines} but rendered visibleLines + 1 children when paginated. The scroll-hint row wasn't budgeted into visibleLines, so the extra child overflowed downward and overdrew the next sibling (the [O] hint strip in showReport; in worst cases the table header row when the report itself was tall). Same mechanism drives Bug B: when the /diff feedback panel mounts below the content area, the report viewer doesn't shrink — the bottom row overdraws.

    Fix: subtract one row from the body budget when the scroll hint will be shown, AND add overflow="hidden" as defense in depth.

  2. Multiple row-flex <Box> rows composed several <Text> siblings with no width budget. On narrow terminals Yoga had no rule to share width between siblings; descriptions/paths wrapped onto a 2nd line and visually overlapped neighbouring rows. This affected DiffViewer summary rows, SlashCommandInput palette rows, and Outro success change bullets.

    Fix: render each row as a single <Text wrap="truncate-end"> with inline child <Text> elements for per-segment color. One row = one terminal line, regardless of viewport width.

Also adds overflow="hidden" to OutroScreen's outer Box on both the success/error/cancel branch and the showReport branch so a tall body can't push picker rows out of bounds (Bug F).

src/utils/wizard-abort.ts unchanged.

Files changed

  • src/ui/tui/primitives/ReportViewer.tsx — budget the scroll hint inside visibleLines; add overflow="hidden"
  • src/ui/tui/components/DiffViewer.tsx — one <Text> per summary row
  • src/ui/tui/primitives/SlashCommandInput.tsx — one <Text> per palette row
  • src/ui/tui/screens/OutroScreen.tsxoverflow="hidden" on outer Boxes; wrap="truncate-end" on change bullets
  • src/ui/tui/primitives/__tests__/ReportViewer.overdraw.test.tsx — new regression test
  • src/ui/tui/screens/__tests__/OutroScreen.overdraw.test.tsx — new regression tests

Test plan

  • pnpm tsc --noEmit clean
  • pnpm lint clean (prettier + eslint)
  • pnpm test — 4238 / 4238 pass
  • pnpm exec vitest run --pool=forks --maxWorkers=1 src/ui/tui/ — 1002 / 1002 pass
  • Existing OutroScreen snapshots still match
  • New regression tests for ReportViewer paginated overflow (2) and OutroScreen file-row / bullet-row uniqueness (4)
  • Manual verification on a real terminal against pnpm try:prod --install-dir=~/excalidraw to confirm Bug A/B/C/D/E/F do not reproduce

🤖 Generated with Claude Code


Note

Medium Risk
Adjusts TUI layout/scrolling calculations and overflow behavior; while covered by new regression tests, it could subtly affect rendering and scroll offsets across different terminal sizes.

Overview
Prevents multiple TUI regions from overwriting each other on narrow/short terminals by budgeting ReportViewer height correctly (reserving a row for the scroll hint) and adding overflow="hidden" to clip any excess rendering.

Reworks DiffViewer summary rows and SlashCommandInput palette rows to render each entry as a single Text with wrap="truncate-end", avoiding multi-Text row-flex layouts that previously caused adjacent rows to mash together; similarly applies wrap="truncate-end" to Outro change bullets.

Adds regression tests for ReportViewer pagination/scroll-hint overflow and for Outro diff/bullet row integrity to prevent future overdraw regressions.

Reviewed by Cursor Bugbot for commit bc69a67. Bugbot is set up for automated code reviews on this repo. Configure here.

…ette, outro

Five user-reported overdraw bugs in the post-run Outro screen all
share two root causes:

1. **`ReportViewer` claimed `height={visibleLines}` but rendered
   `visibleLines + 1` children when paginated** — the scroll-hint row
   wasn't budgeted, so it overflowed downward into the next sibling.
   In the showReport sub-view of OutroScreen, that sibling is the
   "[O] Open in browser · [Esc] Back" hint strip — which is exactly
   the user's "[O] hint overlapping the events table header" bug
   (Bug A). The same overflow drives the `/diff` overlay overlapping
   the Setup Report body (Bug B), because the feedback panel below
   the content area triggers the same too-many-children path.

   Fix: reserve one row of the budget for the scroll hint when
   paginated, AND add `overflow="hidden"` as defense in depth.

2. **Multiple row-flex `<Box>`es composed several `<Text>` siblings
   with no width budget, so on narrow terminals Yoga had no rule to
   share width and rows mashed onto a single terminal line.** This
   affected:
     - `DiffViewer` summary rows ("NEW report.md · +78/-0AddTo
       Library.tsx · +2/-0…" — Bug D)
     - `SlashCommandInput` palette rows ("/login        tRe-
       authenticateter region (US or EU)" — Bug C)
     - Outro success change bullets ("tracking plang Yarn V1" —
       Bug E)

   Fix: render each row as a single `<Text wrap="truncate-end">`
   with inline child `<Text>`s for per-segment color. One row =
   one terminal line, regardless of viewport width.

Also adds `overflow="hidden"` to OutroScreen's outer Box on both
the success/error/cancel and showReport branches so a tall body
can't push the PickerMenu off the bottom and overlap with
ConsoleView chrome below (Bug F: "[3] Exit setup reportmplitude.com)"
— picker rows visually merging with each other and with the
dashboard URL).

Regression tests in
`src/ui/tui/primitives/__tests__/ReportViewer.overdraw.test.tsx` pin
the body+hint budget contract, and
`src/ui/tui/screens/__tests__/OutroScreen.overdraw.test.tsx` pins
the "one file per rendered line" and "one bullet per rendered line"
invariants for DiffViewer summary and Outro success bullets.

`wizard-abort.ts` unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kelsonpw kelsonpw requested a review from a team as a code owner May 14, 2026 19:57
@kelsonpw kelsonpw merged commit eab7fad into main May 14, 2026
11 checks passed
kelsonpw added a commit that referenced this pull request May 22, 2026
What landed
-----------
- New `src/ui/tui/components/DiffTab.tsx` — persistent tab body in
  RunScreen. File list (newest-first, deduped by path) with a colored
  unified diff for the selected file rendered below. `↑↓` / `j/k` move
  selection; `PgUp/PgDn` scroll the diff body; `←→` still switch tabs.
- `src/ui/tui/screens/RunScreen.tsx` — wired the Diff tab between
  Events and Logs.

What was removed
----------------
- `/diff` slash command registration in `src/ui/tui/console-commands.ts`
  (and the `parseDiffSlashInput` helper).
- `/diff` dispatch case + its summary/detail branches in
  `src/ui/tui/components/ConsoleView.tsx`.
- (kept) `DiffViewer` primitive — now consumed by the new tab and the
  existing outro "what changed" section.

Diff tab behavior
-----------------
The tab subscribes narrowly to `fileWritesTotal` (a monotonic counter
that climbs on every agent file write) and re-walks the canonical
`FileChangeLedger` only when that counter advances. The DiffViewer
primitive already windows the patch body to `maxLines` (default 30)
around `scrollOffset`, so large diffs render at a constant ~30-line
cost per frame. Outer Box uses `overflow="hidden"` (defense against
the #779 overdraw bug). Empty state copy: "no file changes yet — the
agent hasn't edited anything." Discoverability hints in
`FileWritesPanel` and `DiffViewer` summary mode now point to the
Diff tab instead of the removed slash command.

Tests
-----
New: `src/ui/tui/screens/__tests__/RunScreen.diffTab.test.tsx`
(6 tests + 2 snapshots at 80/60 cols) covers tab visibility, tab
ordering (Progress → Events → Diff → Logs), empty state, and
content rendering with seeded ledger entries.
Updated: `console-commands.test.ts` — replaced the old `/diff` registry
assertions with a "removal" describe block that asserts `/diff` is
absent and `isKnownCommand('/diff')` is false (the registry-doesn't-
contain-/diff acceptance check). `DiffViewer.test.tsx` and
`FileWritesPanel.test.tsx` updated for the new hint copy.

Verification
------------
- `pnpm tsc --noEmit && pnpm lint && pnpm test` — all green (4470 tests)
- `src/utils/wizard-abort.ts` untouched
- ← → tab switching still works
- /diff is gone from the slash palette (registry + isKnownCommand
  assertions)

Deferred
--------
The end-of-run "all changes summary" overlay that PR #599 also added
is intentionally untouched — this PR only addresses the slash command
→ tab migration. No docs/skill prompts referenced `/diff`.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant