Skip to content

Item 3 follow-up: land the scroll/virtualization benchmark (recovers a commit orphaned by #115's merge) - #178

Merged
WilfordGrimley merged 2 commits into
masterfrom
claude/proposal-h-item3-benchmark-followup-04bam2
Jul 19, 2026
Merged

Item 3 follow-up: land the scroll/virtualization benchmark (recovers a commit orphaned by #115's merge)#178
WilfordGrimley merged 2 commits into
masterfrom
claude/proposal-h-item3-benchmark-followup-04bam2

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

This is a self-caught mistake, being fixed transparently. PR #115 (Item 3: flat scroll + virtualization) merged at commit 9b613f42. Afterward, in the same session, I ran the owner-requested benchmark and pushed a follow-up commit (a8cd3bc8, committing frontend/tests/perf/display-scroll.bench.spec.ts + playwright.perf.config.ts) to that same branch — but the PR had already merged, so that push landed on a now-closed branch and never reached master. The PR body and my own relay report both describe the benchmark script as "now committed," which was true of the branch but not of master — the follow-up commit was silently orphaned. Confirmed via git show origin/master:frontend/playwright.perf.config.ts failing before this PR.

Per this repo's own documented policy for exactly this situation (CLAUDE.md: "a merged pull request is finished — it cannot track new work and must not be reused"), this PR recovers that orphaned commit as a fresh branch off current master (git cherry-pick a8cd3bc8, clean, no conflicts) rather than trying to reopen or stack onto #115.

What this actually adds (unchanged from the original description)

The manual scroll/virtualization benchmark that gated Item 3's sheet-level-vs-row-granular decision is now committed under frontend/tests/perf/display-scroll.bench.spec.ts, runnable via npx playwright test --config=playwright.perf.config.ts. playwright.config.ts now testIgnores tests/perf/** so it's invisible to both a plain npx playwright test and CI's sharded run. Intended to give the upcoming pane migration (which restructures this exact surface) a number to compare against instead of a feeling.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate. N/A beyond the benchmark script itself — no other test coverage changes needed for this recovery.
  • I have manually tested my changes as follows, re-run fresh on this branch (not trusted from the original, now-orphaned commit):
    • npx tsc --noEmit clean
    • npx jest — 43/43 suites, 402/402 tests passing
    • npx playwright test --list (default config) confirms tests/perf/ is invisible; npx playwright test --config=playwright.perf.config.ts --list shows exactly the 1 benchmark test
    • npx playwright test --config=playwright.perf.config.ts — reproduces the original benchmark numbers (120 cards/15 sheets, ~59.8fps, peak heap ~258MB, 16/120 max mounted <img>s, 0 long tasks)
    • NEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED=true npx next build — flag-on production build succeeds
  • I have updated any relevant documentation or created new documentation where appropriate. N/A — same reasoning as the original PR Proposal H, item 3: flat vertical scroll + sheet-level virtualization #115 description.

Why this happened / what I'm changing about my own process

I checked #115's merge status before building on it (this session's own established branch-hygiene lesson), but didn't re-check it between the PR merging and my own follow-up push to the same branch later in the same session — the check needs to happen at push time for every push to a branch, not just once at the start of a task. Flagging this explicitly rather than quietly folding it into a routine-looking PR.


Generated by Claude Code

… script

Per owner follow-up on #115: the manual scroll fps/heap/jank benchmark
that gated the sheet-level-vs-row-granular virtualization decision is
now under frontend/tests/perf/, runnable via
`npx playwright test --config=playwright.perf.config.ts`. The main
playwright.config.ts testIgnores tests/perf/ so it never runs in CI or
a plain `npx playwright test`. Ahead of the pane migration, which
restructures this exact surface, this turns "did scrolling regress?"
into a number to compare against, not a feeling.

Verified: `npx playwright test --config=playwright.perf.config.ts`
reproduces the PR's original results (120 cards/15 sheets, ~59fps,
peak heap ~257MB, 16/120 max simultaneously-mounted <img> tags, 0 long
tasks); `npx playwright test --list` (default config) does not surface
the perf spec.
@WilfordGrimley
WilfordGrimley force-pushed the claude/proposal-h-item3-benchmark-followup-04bam2 branch from 6b3246a to d654f66 Compare July 19, 2026 20:13
@WilfordGrimley
WilfordGrimley merged commit e00ed56 into master Jul 19, 2026
6 checks passed
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