Stabilize TUI refresh, scrolling, responsive layout, and dashboard data density - #863
Stabilize TUI refresh, scrolling, responsive layout, and dashboard data density#863ihearttokyo wants to merge 3 commits into
Conversation
Keep background refreshes from blanking or replacing the active Optimize view, and enforce a one-minute minimum refresh interval.\n\nRework the dashboard into a stable 3/2/1-column flow with left-aligned bars, justified metric columns, readable project headings, and a ten-row Daily Activity viewport.\n\nSynchronize resize state before Ink paints so breakpoint transitions do not leave stale frames, preserve content beyond 256 terminal columns, and cap the dashboard at the current data's renderable width. Add focused regression coverage and a submission statement documenting live Ghostty validation.
Submission note — rebuilt August 1
What this work fixesCodeBurn's dashboard had four connected failure modes. A background result could replace state after the user entered Optimize; Ink could paint once at the previous terminal width; the alternate screen removed terminal scrollback while the app remained taller than its viewport; and narrow rows let labels displace metric headings or values. The repair keeps those decisions in the existing dashboard state and rendering path. It adds no dependency or second layout system. Behavior contract
Verification
The full repository run produced 2,507 passing, 2 failing, and 5 skipped tests, plus 26 missing- Reviewer focusThe highest-value review is the interaction among shared metric sizing, calculated Daily Activity page size, and existing scroll state. No refresh should change view or position, no supported width should lose a metric, each resize should preserve panel order on its first frame, and Daily Activity navigation should use the page size visible on screen. |
Pin the interactive dashboard to a terminal-sized viewport and add line, page, home, and end navigation without sacrificing the alternate-screen resize protections. Preserve the viewport offset across background refreshes and ordinary rerenders while resetting cleanly for a new view or period. Give daily-history paging its own Space binding, render full model costs whenever the panel can hold them, and spell out the project session heading. Extend the responsive dashboard regressions across one-, two-, and three-column viewports and update the submission evidence.
Follow-up implementation and validation update — August 1
Latest implementation
TDDRGR and post-implementation roundsThe new two-column regression first failed with 10 dates where 14 were required. One shared page-size calculation made it green; the refactor then reused the existing project limit and Activity aggregation so counting cannot drift from rendering. Two dedicated bug-fix rounds followed. Each ended with the relevant 70/70 regression matrix and a real Ghostty user path:
No additional defect appeared in either round. Correctness review was clean, and Ponytail review found no smaller useful design. Evidence and limits
This follow-up is now part of PR #863. The push retriggered GitHub checks; the local 70-test matrix and production builds were rerun immediately before commit. |
Keep every dashboard metric visible with intrinsic column widths and one cell of separation, allowing bars and project labels to yield space before headings or values disappear. Shorten project paths in meaningful stages so the project title remains recognizable for as long as possible. Derive Daily Activity's page size from the sibling panels in the active responsive row: ten dates in one column, the visible project count in two, and the greater project or activity count in three. Reuse that calculation for rendering, cursor bounds, paging, and status text so the viewport cannot drift from its navigation contract. Cover the behavior with live Ink regressions, path-shortening contracts, the 70-test dashboard/model/overview matrix, and the rebuilt submission record.
ozymandiashh
left a comment
There was a problem hiding this comment.
Thanks for this. The scrolling, refresh-race and adaptive-layout work is careful, and I traced a fair bit of it by hand: the generation-counter and pending-ref coordination in reloadData, the viewRef stale-closure avoidance, the shared getDailyActivityPageSize / getProjectBreakdownRowLimit calculation that fixes the drift you describe, and the shortProject truncation staging. All of it holds up, and the test coverage for the stated behaviour is genuinely strong.
One thing should block, though, because it is invisible to CI and to this PR's own tests by construction.
The resize handler reintroduces the Windows/ConPTY hang from #195
src/dashboard.tsx:1504
process.stdout.write('\u001B[?2026h\u001B[2J\u001B[H')That writes Begin-Synchronized-Update directly to stdout. The repo already has a guard for exactly this sequence, added by c511627 to close #195, whose commit message states that ConPTY "does not implement this protocol and buffers indefinitely, causing the dashboard to hang with no output":
// src/ink-win.ts:11
if (chunk === BSU || chunk === ESU) return trueThe filter is an exact string comparison. The write above is BSU concatenated with clear-screen and cursor-home, so it is a different string, chunk === BSU is false, and the raw BSU passes straight through to the terminal on Windows. Every resize would then trigger the failure mode that guard exists to prevent.
There is also no matching End-Synchronized-Update anywhere in the diff. I grepped the branch: \u001B[?2026l appears zero times. The region is left open and only closes because Ink's next internal frame happens to emit its own correctly paired bsu/esu.
Neither the Windows package-build check nor tests/dashboard.test.ts can catch this. The tests drive PassThrough streams and assert on stripAnsied substrings, so they cannot observe ConPTY buffering or cursor desync.
Ink 7 already listens for resize on stdout and performs a layout recalc plus a full unthrottled re-render (ink.js:261-272), wrapped in its own correct bsu/esu pairing, so the manual write may simply be unnecessary. If a hard clear really is needed, gating it behind process.platform !== 'win32' would at least match the reasoning already encoded in ink-win.ts.
Smaller things, none blocking
-
The
isHeavyPeriodauto-refresh gate is gone. The previous code didif (!dayDate && isHeavyPeriod(period)) return, skipping auto-refresh on 30days/month/all/lifetime.src/dashboard.tsx:1270-1276no longer has it, so background reparses now fire on heavy tabs.getDashboardScanRangebounds the scan whenscrollableHistoryis set, so this may well be fine, but it is a behaviour change the description does not mention and it would be good to hear it was deliberate. -
Aggregation runs inside render.
dailyHistoryPageSize,dailyHistoryRowCountandmodelCountare computed in theInteractiveDashboardbody (src/dashboard.tsx:1132-1144) rather than inuseMemo, so they recompute on every paging keystroke. SeparatelyaggregateModelTotals(projects)runs twice per data-changing render, once inside the memoizedgetDashboardMaxWidthand again unmemoized inModelBreakdown. -
ScrollableViewport'suseLayoutEffecthas no dependency array (src/dashboard.tsx:1062-1067), someasureElementruns after every render including every scroll tick, even though content height does not change while scrolling. -
StatusBar scrolls away. It is the last child of
content, which is entirely insideScrollableViewport(:1431-1439), so the keybinding hints disappear when you scroll down on a tall dashboard. Pinning it outside the scroll region would keep them visible. -
--refreshhelp text does not mention the new 60s floor.src/main.ts:767,1197,1215still say "Auto-refresh interval in seconds (0 to disable)", so--refresh 10silently becomes 60 with only the README explaining why. -
src/compare.tsxstill caps at 2 columns (:214-215,257), so pressingcon a 135+ column terminal visibly narrows from the new 3-column layout. Pre-existing duplication rather than something this PR introduced, but the gap is wider now.
Everything else checked out on my side: tsc --noEmit is clean, and tests/dashboard.test.ts passes in full. The other failures I saw in full-suite runs (cli-emitters, cli-status-menubar, cache-refresh-lock) reproduce on unmodified main in my environment and are not attributable to this branch.
Happy to re-review once the resize write is sorted.
Summary
Why the dashboard failed
Three independent behaviors combined into the visible failures. A background result could replace state after the user had entered Optimize. Ink could paint once with the previous terminal width before React received a resize. Because the alternate screen removed terminal scrollback, content taller than the viewport became unreachable. At narrow widths, the shared row renderer also gave labels enough space to displace metric headings or values.
The repair keeps view, width, viewport, and row-density decisions inside the existing dashboard state and rendering path. It adds no dependency or parallel layout system.
User-visible behavior
Stable refresh and navigation
--refresh 0remains fully static.Responsive dashboard
Complete, compact data rows
Tok/sand every other metric column always render. Unavailable values display-instead of removing a column.~marker, render in full whenever the panel can hold them.session.Adaptive Daily Activity history
MAX(10, visible By Project rows).MAX(10, visible By Project rows, visible By Activity rows).j/k, Space paging,g/G, final-page clamping, and theShowing X-Y of Zstatus.TDDRGR and bug-fix rounds
The adaptive-row regression first failed for the intended behavioral reason: a two-column lifetime view with 14 visible projects rendered 10 dates instead of 14. The smallest production change introduced one shared page-size calculation. After the test passed, existing project-row limits and Activity aggregation were reused rather than duplicated, and the focused tests remained green.
Two post-implementation bug-fix rounds then exercised independent real user paths. After each round, the relevant 70-test regression matrix and live Ghostty path were rerun:
1-14, Space advanced to15-28, andgreturned to1-14. Accessibility bounds confirmed the entire app frame when a native Ghostty layer capture omitted window chrome; the misleading partial captures were discarded.Correctness review was clean. Ponytail review found the implementation already lean and did not recommend another abstraction.
Validation
git diff --check.dist/main.js,dist/cli.js, and dashboard HTML hashes match the repository build.In the full repository run, 2,507 tests passed, 2 failed, and 5 were skipped, with 26 missing-
jsdomenvironment errors. Both failures are pre-existing Copilot durable-cache assertions intests/parser.test.ts; they do not exercise this dashboard work. A durable-total assertion that failed in an earlier run passed in the final run.Native Shift-Space could not be distinguished from Space in synthesized terminal input because both arrive as the same byte. Reverse page-cursor behavior remains covered deterministically, and
gwas validated in Ghostty as the reliable first-page return path.Reviewer focus
The highest-value review is the interaction among the shared row renderer, the calculated Daily Activity page size, and the existing scroll state. The acceptance criteria are that no view or scroll position changes because of background refresh, no metric disappears at supported widths, each resize immediately preserves panel order, and Daily Activity navigation uses the same page size shown on screen.