refactor(ui): one test file per screen, and a document under its ceiling by bmethod · Pull Request #43 · vanillagreencom/vsys

Brad (bmethod) · 2026-09-10T11:36:32Z

Phase 5 of #24: shape. One test file per screen, and a document under its ceiling. Stacked on #42.

Most of phase 5 landed inside phase 6 — the column helper replaced the padEnd arithmetic, gap dots draw quietly, the footer reads configured keys. What is left is the piece phase 5 named first, plus what a validation note surfaced.

One test file per screen, beside the screen

src/ui/App.test.tsx was 1741 lines holding every screen's rendered tests. It now keeps only what is about the shell — tabs, pinning, startup, the help overlay, the footer, the narrow layout — and each screen's tests sit beside the screen. src/ui/agent.tsx had no test file at all; it has one now.

The mounted app and the selected-row reader moved to src/test/harness.tsx, beside the fixture, because every screen's file mounts the whole shell. Seven copies of a mount helper is how seven screens' tests drift apart.

290 tests before the split, 290 after. A split that drops a test looks identical to one that does not unless someone counts.

The replay of this commit is where it earned its keep

This commit was written before ten rounds of review added tests to the file it empties, so 33 of main's tests existed on neither side of its renames. Taking its version whole would have deleted them and left the suite green. Each was placed in the file its screen now owns: main held 104 UI tests, the tree holds 169, none lost.

Then a second check, which the count cannot do. For the 104 tests present on both sides, the bodies were compared. Five differed, and only two failed. The three that passed are the interesting ones:

the footer test reverting to its narrow pre-widening form — the version that checked one screen while being named for every screen;
a selection test dropping from 49 lines to 27, a row kind gone;
a change-row test that stopped asserting its own identity.

All five carry main's body. A weaker assertion that still passes is the loss nothing reports.

A document nine bytes under its ceiling

docs/architecture/ui.md was 16383 of 16384 bytes. The next documentation edit anywhere near it would have failed a gate that had nothing to do with that edit, and whoever hit it would have spent their round on it.

Its invariants are now docs/architecture/ui-invariants.md, linked from it: 9938 and 9473 bytes, each well clear. No ceiling raised.

The split invalidated 21 references naming the shell test file, and the reference gate would not have caught one of them — it checks that a path resolves, and src/ui/App.test.tsx still resolves. Each was retargeted against the file that now holds its claim. That blind spot is filed upstream as kendex#2502.

What the remaining phase 5 items turned out to be

Two of three were done but unguarded, which is not done:

Gap dots did draw quietly, but only the arithmetic was tested, not the colour. The renderer could have handed gaps the metric's hue and nothing would have failed. There is a rendered guard on the spans now.
Every key hint did read its configured binding, but nothing would have failed if one were written as fixed text. A guard rebinds three keys and reads them back from the footer and the help panel.

git grep "padEnd(" src/ui finds one hit, inside the column helper.

Also here

A row kept on screen with its detail under it. The Settings scroll was double-scrolling — scrollChildIntoView did its job and the arrow key then reached the scrollbox as well, because Settings is the only screen that passes focus to it. From the eighth row down, the selected row was never on screen. Found by logging the child and viewport geometry, not by reasoning: the row sat exactly at the viewport top after the scroll and five rows above it by the next keypress.

paddingLeft moves nothing on a text element — not the first line, not the wrapped ones. A Detail box now carries the settings help sentence, the capability diagnostic, the unreadable-source list and the Storage scrub report.

Proof

358 tests, 0 fail.

The figure this line carried until now was 344, and it was wrong twice over. It was stale, because four rounds of review have added tests since. It was also one too high when it was written: a duplicate test entered during the cut, so the split boundary was 343. A by-name body comparison cannot see that — two identical tests collapse to one key on both sides and read as no loss — and only the suite count carries the evidence. The two checks are complements, not alternatives.

Nine plants across the two original commits, each reddening its own test. Four rounds since have added four more tests to the Settings scroll effect:

three controls that fail against the mode this pull request shipped first — a capability's reason below the fold, the same on Enter, and a setting's help sentence;
one guard on the unreadable-sources list that cannot fail against any version this path has had, measured across fifteen viewport and error-count combinations rather than argued. It fails at once if that path is ever sent down the row-only branch, which is what it is for.

Part of #24.

https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

chatgpt-codex-connector · 2026-09-10T11:36:36Z

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.

The shell's test file held every screen's rendered tests, 1741 lines of them. Each screen's tests now sit beside the screen they drive, and `App.test.tsx` keeps only what is about the shell: the tabs, pinning, startup, the help overlay, the footer and the narrow layout. The mounted app and the selected-row reader move to `src/test/harness.tsx`, beside the fixture, because every screen's file mounts the whole shell. No test changed and none was removed: 290 before, 290 after, with the two new guards below making 292. `docs/architecture/ui.md` was one byte under its class limit, so its invariants move to `docs/architecture/ui-invariants.md` and it links to them. Every test reference the split invalidated now names the file that holds the claim, in that document and in the plan coverage table. Two properties phase 5 states were true but unguarded, so a change could have taken either away in silence. A chart's gap columns are now asserted quiet and dim on the rendered screen rather than only in the arithmetic that splits them, and a key hint is asserted to follow a changed binding in the footer and in the help panel alike. Claude-Session: https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

Walking down Settings lost the row the reader was standing on: from the eighth row to the end of the list it was never on screen. The arrow moved the selection and the focused scrollbox moved the viewport as well, so the two scrolls added up and the row slid past the top edge. The arrow now moves the selection alone. Opening an editor or a pick list moves its row near the top instead, because a block that has just grown has no laid-out height to scroll into view, and an editor drawn below the bottom edge is one the reader cannot use. A detail line under a row was not indented at all. `paddingLeft` on a text element moves nothing: not the first line, not the wrapped ones. `Detail` puts the indent on a box, the way Home already did, and the settings help sentence, the missing-capability diagnostic, the unreadable-source list and the Storage scrub report all read as belonging to the row above them. Claude-Session: https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

Copilot

🟡 Changes recommended

The screen-test split and documentation reference migration remain incomplete, and the shell suite contains a duplicate test.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This Phase 5 refactor reorganizes UI tests by screen, centralizes the rendering harness, improves detail-row layout, and splits UI architecture documentation.

Changes:

Moves rendered tests into screen-specific suites with a shared harness.
Adds reusable detail indentation and Settings scrolling fixes.
Splits UI invariants into a separate document and updates references.

File summaries

File	Description
`src/ui/App.test.tsx`	Reduces shell tests; incomplete moves and duplication flagged.
`src/ui/widgets.tsx`	Adds the reusable `Detail` component.
`src/ui/widgets.test.tsx`	Guards sparkline gap styling.
`src/ui/timeline-screen.test.tsx`	Consolidates Timeline tests.
`src/ui/timeline-screen.test.ts`	Removes the former unit-test file.
`src/ui/storage-screen.tsx`	Uses `Detail` for scrub reports.
`src/ui/storage-screen.test.tsx`	Consolidates Storage tests.
`src/ui/storage-screen.test.ts`	Removes the former unit-test file.
`src/ui/settings-screen.tsx`	Fixes scrolling and detail layout.
`src/ui/settings-screen.test.tsx`	Consolidates Settings tests.
`src/ui/settings-screen.test.ts`	Removes the former unit-test file.
`src/ui/home.test.tsx`	Consolidates Home tests.
`src/ui/home.test.ts`	Removes the former unit-test file.
`src/ui/agents.test.tsx`	Consolidates Agents tests.
`src/ui/agents.test.ts`	Removes the former unit-test file.
`src/ui/agent.test.tsx`	Adds Agent-detail coverage.
`src/test/harness.tsx`	Centralizes mounted-app test utilities.
`docs/architecture/ui.md`	Links to extracted UI invariants.
`docs/architecture/ui-invariants.md`	Holds UI invariants and test ownership.
`docs/architecture/settings.md`	Retargets Settings verification references.
`docs/architecture/plan-coverage.md`	Updates per-screen test paths.

Review details

Files reviewed: 20/21 changed files
Comments generated: 3
Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Seven rendered screen tests still sat in `App.test.tsx`, which is this pull request's own criterion unmet: Resources' tile widths, the Agents column digit, Home's memory-reclaim card and copy notice, the Storage and Resources target handling, and the Builds pool and linkers cells. Each now sits beside its screen. `App.test.tsx` keeps nine, all of them about the shell. One test was an exact duplicate of another, byte for byte. It added no coverage and it counted once in every suite figure this branch has reported since the split introduced it. The reference migration was incomplete in a way nothing could report. The gate checks that a path resolves, and `src/ui/App.test.tsx` resolves, so a claim kept naming it after its test moved elsewhere. Eight claims across five documents now name the file that holds them, including two naming `.test.ts` files the split had already deleted. Claude-Session: https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

Copilot

🟡 Changes recommended

Settings scrolling, cross-screen test placement, and the documentation split still contain unresolved structural defects.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

src/ui/settings-screen.tsx:111

sourcesOpen remains true after the selection leaves the sources row, so every subsequent arrow takes the “opened” branch and scrolls each selected setting near the top even though the open block is elsewhere. This makes the list jump on every keypress; scope this state to the currently selected sources item.
docs/architecture/ui-invariants.md:3
This new architecture topic lacks the Covers: declaration used to map source changes to their documentation; every sibling topic provides it on line 3. Without it, UI changes will not identify this extracted invariant file as covering them.
docs/architecture/ui-invariants.md:41
The short-Timeline assertion moved to timeline-screen.test.tsx (the viewport test at lines 69-101); App.test.tsx now checks only the 80-column shell/Agents half. Retarget both clauses so the verification map does not send maintainers to the wrong test.
docs/architecture/ui.md:44
The extraction is incomplete: ui.md:46-48 still leaves three invariant bullets below this pointer, while the first two are also copied into ui-invariants.md:42-43 and the third is absent there. Consolidate the remaining bullets into the new file so each invariant has one owner and this pointer is accurate.

Files reviewed: 26/27 changed files
Comments generated: 3
Review effort level: Balanced

Copilot

🟡 Changes recommended

Selected setting help can remain below the viewport because the new scrolling mode does not recognize that detail block.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 26/27 changed files
Comments generated: 1
Review effort level: Balanced

Naming the openers missed a kind twice. A capability shows its reason as soon as it is selected; a setting shows its help sentence. Both were drawn under the row, neither was named, and both left their sentence below the fold at the bottom of a short list. A third clause would have missed the next one. Every entry is a block with its row at the top, so whether anything is drawn under the selected row is a property of the block rather than of a list of kinds. The effect reads it and `opened` is gone. It reads the block's child count rather than its height. The detail is in the tree as soon as it is rendered and its height is not known until the layout after that, so a measurement in rows would decide one pass too late — which the capability control caught when this was written the other way. The second pass stays, for the arithmetic that reads positions the opening has not been laid out into yet. The picker branch is untouched. `openCap` leaves the dependency array as a value and comes back as a re-run trigger with `editing` and `sourcesOpen`, named as triggers the way the column count already is. Last round I called it insurance against a mode that stops reading it. This is that mode. Claude-Session: https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

Copilot

🟢 Approval recommended

The changes are coherent and no blocking defect was found; one nonblocking regression-test gap remains.

Review details

Files reviewed: 26/27 changed files
Comments generated: 1
Review effort level: Balanced

The comment above the effect described the mechanism this commit tried and rejected: the block being taller than its row. The code asks the block whether it has a second child, and the comment inside says why height decides one pass too late. The top comment now says what the code does. The sources list gets the control it had no reason to need. It cannot fail against any version this path has had — `sourcesOpen` was named from the first one, and the last entry is shown or cut identically before and after the rewrite, in all fifteen viewport and error-count combinations I measured. It fails at once if the path is ever sent down the row-only branch, which is what three defects on this effect in one evening make worth holding. Its fixture is chosen rather than assumed. Four unreadable sources is enough that the opened list does not fit under the row where it sits, and few enough that all of it fits once the row moves up. A list taller than the viewport is shown by no scroll position, and a list that fits either way cannot tell one scroll from another. Claude-Session: https://claude.ai/code/session_01B4rKWUEy6WF5MRxtBEwFdB

Copilot

🟢 Approval recommended

The screen-test split, behavior guards, shared harness, and documentation references are consistent with the stated scope.

Review details

Files reviewed: 26/27 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🟢 Approval recommended

The implementation is coherent and well-covered; only the PR description’s stale proof count should be refreshed.

Review details

Files reviewed: 26/27 changed files
Comments generated: 1
Review effort level: Balanced

Brad (bmethod) mentioned this pull request Sep 10, 2026

feat(ui): see and reach the agent's terminal #44

Closed

Brad (bmethod) force-pushed the issue-24-phase-4 branch from 12649ef to d9b3b1c Compare September 10, 2026 11:46

Brad (bmethod) force-pushed the issue-24-phase-5 branch from 57e7f0f to 8636260 Compare September 10, 2026 11:48

Brad (bmethod) force-pushed the issue-24-phase-4 branch from d9b3b1c to 97e3759 Compare September 10, 2026 11:58

Brad (bmethod) force-pushed the issue-24-phase-5 branch from 8636260 to 13e71a4 Compare September 10, 2026 11:59

Brad (bmethod) force-pushed the issue-24-phase-4 branch from 97e3759 to bfeb0b4 Compare September 10, 2026 12:12

Brad (bmethod) force-pushed the issue-24-phase-5 branch from 13e71a4 to caaaaf8 Compare September 10, 2026 12:13

Brad (bmethod) force-pushed the issue-24-phase-4 branch from bfeb0b4 to 3f771e2 Compare September 10, 2026 12:20

Brad (bmethod) force-pushed the issue-24-phase-5 branch 3 times, most recently from 6e93a80 to 853edf6 Compare September 10, 2026 13:15

Base automatically changed from issue-24-phase-4 to main September 10, 2026 13:22

Brad (bmethod) added 2 commits September 10, 2026 06:23

Brad (bmethod) force-pushed the issue-24-phase-5 branch from 853edf6 to d90c5f3 Compare September 10, 2026 13:23

Copilot AI balanced review requested due to automatic review settings September 10, 2026 13:23

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 13:24 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/ui/App.test.tsx Outdated

Comment thread src/ui/App.test.tsx Outdated

Comment thread docs/architecture/settings.md

Copilot AI review requested due to automatic review settings September 10, 2026 13:35

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 13:35 View session

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Brad (bmethod) mentioned this pull request Sep 10, 2026

A move-triggered reference check: which tests left which file, and which documents still name it #48

Closed

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/ui/home.test.tsx Outdated

Comment thread src/ui/settings-screen.test.tsx Outdated

Comment thread src/ui/storage-screen.test.tsx Outdated

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Copilot AI review requested due to automatic review settings September 10, 2026 14:12

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 14:13 View session

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/ui/settings-screen.tsx Outdated

Brad (bmethod) mentioned this pull request Sep 10, 2026

review-gate: findings suppressed into the review body create no thread, so the gate passes blind vanillagreencom/kendex#2519

Closed

Copilot AI review requested due to automatic review settings September 10, 2026 14:26

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 14:26 View session

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/ui/settings-screen.tsx

Copilot AI review requested due to automatic review settings September 10, 2026 14:35

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 14:35 View session

Brad (bmethod) force-pushed the issue-24-phase-5 branch from c462779 to 420f655 Compare September 10, 2026 14:36

Brad (bmethod) closed this Sep 10, 2026

Brad (bmethod) reopened this Sep 10, 2026

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 10, 2026 14:39

Copilot started reviewing on behalf of Brad (bmethod) September 10, 2026 14:40 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/ui/settings-screen.test.tsx

Brad (bmethod) added this pull request to the merge queue Sep 10, 2026

Merged via the queue into main with commit c93602c Sep 10, 2026
26 checks passed

Brad (bmethod) deleted the issue-24-phase-5 branch September 10, 2026 14:46

Navigation Menu

Uh oh!

refactor(ui): one test file per screen, and a document under its ceiling - #43

refactor(ui): one test file per screen, and a document under its ceiling#43
Brad (bmethod) merged 9 commits into
mainfrom
issue-24-phase-5

Brad (bmethod) commented Sep 10, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Brad (bmethod) commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

One test file per screen, beside the screen

The replay of this commit is where it earned its keep

A document nine bytes under its ceiling

What the remaining phase 5 items turned out to be

Also here

Proof

Uh oh!

chatgpt-codex-connector Bot commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Brad (bmethod) commented Sep 10, 2026 •

edited

Loading