Skip to content

refactor(eve): derive trace viewer geometry once - #1468

Merged
chadhietala merged 1 commit into
mainfrom
simplify-commit-review-trace-viewer
Jul 31, 2026
Merged

refactor(eve): derive trace viewer geometry once#1468
chadhietala merged 1 commit into
mainfrom
simplify-commit-review-trace-viewer

Conversation

@chadhietala

@chadhietala chadhietala commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Found

  • Card-line geometry (each card's lines plus one separator) is hand-rolled in four places in trace-viewer-state.ts, two of them byte-identical, and they had split two-and-two on the stale-measurement default — so a click could map to a different card than the scroll math assumed.
  • The nine-key "reset the view" literal is spelled out three times; feat(eve): select and copy text in the trace viewer #1446 had to add textSelection to two of them.
  • In trace-view.ts, padBody is the identity function at both call sites, center duplicates renderEmptyState, and joinPanels' separator parameter cannot vary.

Did
One cardLineOffsets prefix-sum feeds all four sites, one clearedTraceView() factory serves the three resets, padBody is gone and the parameter dropped. −81/+78.

Validated
1010 CLI unit tests and tui-traces (all 9 smoke scenarios: expand/collapse, click, drag-copy, trace switching, empty state) pass. No test file touched. tsc --noEmit, lint, fmt, guard:invariants clean.

The conversation viewer's line layout — each card occupies its rendered
lines plus one separator row — was hand-rolled in four places: the scroll
offset of the selected card, the row-to-card lookup for clicks, and two
byte-identical total-line loops (#1446 added one that #1403's wheel
handler already had). They had also drifted on the edge case where the
measured line counts lag the card list: two loops walked the counts array
and two walked the cards with a one-line default.

All four now come from `cardLineOffsets`, which walks the cards and keeps
the default, so a frame measured before a repaint maps rows to cards the
same way everywhere.

The nine-key "reset the view" literal was spelled out in `applyTraceList`,
`switchTrace`, and `createTraceViewerState`, and had drifted too:
`textSelection` was added to two of them by #1446. One
`clearedTraceView()` factory now serves all three. The pruned-trace branch
keeps its own key list, since it deliberately preserves expansion state
and selection.

In the renderer, `renderEmptyState` and `center` both built a body-height
array and centred content in it, so `center` was `renderEmptyState` with
one line; both already returned exactly `bodyRows` entries, which made
`padBody` the identity function at both of its call sites. `center` now
takes the lines to centre and `padBody` is gone. `joinPanels` loses its
`separator` parameter, which had one caller passing a constant the
function's own width math already assumed.

Also types the session's `#metrics` field as the `TraceViewerKeyEnvironment`
it restates field-for-field and passes straight to the reducer.

Signed-off-by: Chad Hietala <chadhietala@gmail.com>
@chadhietala chadhietala added the simplify-devbox Simplification found by the automated devbox code review label Jul 31, 2026
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
eve-docs Ready Ready Preview Jul 31, 2026 3:06pm
eve-docs-4759 Ready Ready Preview, v0 Jul 31, 2026 3:06pm

@chadhietala
chadhietala merged commit f7ba3b3 into main Jul 31, 2026
89 of 90 checks passed
@chadhietala
chadhietala deleted the simplify-commit-review-trace-viewer branch July 31, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

simplify-devbox Simplification found by the automated devbox code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants