test: keep perf CLS setup scrolls outside measured windows

[Astro-Han]

Summary

Move setup-only timeline scroll alignment out of perf-probe measured windows and add a runtime guard so direct setup scrolls cannot be called while a perf measurement is active.

Why

session-scroll-reading was resetting the perf probe before a script-driven scrollTop = 0 alignment. Chromium does not mark that jump as recent user input, so the layout-shift observer counted the helper as CLS and produced false-positive perf reports.

Related Issue

Fixes #746

Human Review Status

Pending

Review Focus

Please check that measurePerfWindow() now defines the measured window cleanly, setTimelineScrollTopForSetup() remains setup-only, and session-timeline-recompute starts from a position that can actually move upward before measuring negative wheel input.

Risk Notes

session-timeline-recompute can still report high CLS from its real upward wheel path; this PR only removes script-driven setup scroll pollution from the measured window. Skipped conditional checklist items: no visible UI or copy changed; no platform, packaging, updater, signing, paths, shell, or permissions surface changed; no docs, release notes, dependencies, credentials, deletion behavior, generated content, or local file changes are relevant.

How To Verify

bun install --frozen-lockfile: ok
bun test --preload ./happydom.ts ./e2e/perf/*.unit.ts: 20 passed
bun run typecheck: ok
PLAYWRIGHT_PORT=3002 PLAYWRIGHT_WORKERS=1 PAWWORK_PERF_SCENARIOS=session-scroll-reading PAWWORK_PERF_BRANCH=i746-review PAWWORK_PERF_OUTPUT="/tmp/i746-session-scroll-reading-review-final.json" bun --cwd packages/app playwright test e2e/perf/perf-probe.spec.ts --project=chromium --grep "session-scroll-reading emits a 3-run JSON baseline" --reporter=line: 1 passed, output cls=0
PLAYWRIGHT_PORT=3002 PLAYWRIGHT_WORKERS=1 PAWWORK_PERF_PROFILE=low-end PAWWORK_PERF_SCENARIOS=session-timeline-recompute PAWWORK_PERF_BRANCH=i746-review PAWWORK_PERF_OUTPUT="/tmp/i746-session-timeline-recompute-review-final.json" bun --cwd packages/app playwright test e2e/perf/perf-probe.spec.ts --project=chromium --grep "session-timeline-recompute emits a 3-run low-end JSON baseline" --reporter=line: 1 passed
git diff --check: ok

Screenshots or Recordings

Not required: no visible UI or copy changes.

Checklist

• Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
• Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
• Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
• Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
• I linked the related issue, or stated in Summary why there is no issue.
• I described the review focus and any meaningful risks.
• I replaced the example block in How To Verify with the real verification steps and the key result for each.
• I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
• (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
• (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
• (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
• I reviewed the final diff for unrelated changes and suspicious dependency changes.
• I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a measured performance window lifecycle to prevent programmatic setup operations from being recorded as layout shift in perf probe measurements. A measuredPerfWindowDepth counter tracks active windows, guarded setup helpers separate timeline alignment from measurement, and specific scroll-reading scenarios are refactored to use the new bracketing mechanism. Validation tests enforce the invariants.

Changes

Measured Performance Window Guard System

Layer / File(s)	Summary
Measured window depth counter and guard enforcement packages/app/e2e/perf/perf-probe.spec.ts	Introduces module-scoped measuredPerfWindowDepth counter and implements setTimelineScrollTopForSetup that throws when invoked inside an active measured window, preventing programmatic scroll operations from being recorded as layout shift.
Measurement window wrapper and setup helpers packages/app/e2e/perf/perf-probe.spec.ts	Adds measurePerfWindow to bracket perf sampling (settle, reset probe, execute action, snapshot, decrement depth via finally) and settlePerfSetup for extra settling before setup/measurement phases.
Timeline hover interaction DOM measurement packages/app/e2e/perf/perf-probe.spec.ts	Refactors hoverTimelineScrollLane to compute scroll viewport bounding rectangle via page.evaluate and return coordinates instead of using Playwright locator bounding box.
Setup separation in timeline reveal scenarios packages/app/e2e/perf/perf-probe.spec.ts	Updates revealCachedSessionMessages and revealLongScrollWindow to use setTimelineScrollTopForSetup for timeline scroll positioning outside measured windows.
Scroll reading measurement refactoring packages/app/e2e/perf/perf-probe.spec.ts	Refactors session-scroll-reading, session-scroll-reading-long, and session-timeline-recompute scenarios to separate programmatic alignment (via setTimelineScrollTopForSetup) from measurement recording (via measurePerfWindow).
Validation test suite for measured window invariants packages/app/e2e/perf/perf-probe-window.unit.ts	Adds test utilities to read perf probe spec at runtime, extract measured window blocks, and validate that setup-only scroll operations are absent from measured windows, that measurePerfWindow properly manages measuredPerfWindowDepth, and that setTimelineScrollTopForSetup is guarded.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• Astro-Han/pawwork#607: Introduces initial perf-probe scenarios that this PR refactors with measured-window gating.
• Astro-Han/pawwork#664: Updates longScrollSeedText content used by session-scroll-reading-long, which this PR refactors to use measurePerfWindow and new setup helpers.
• Astro-Han/pawwork#635: Modifies perf-probe.spec.ts session scroll measurement flow; intersects with this PR's guard and setup refactoring.

Suggested labels

app

Poem

🐰 Scroll hops off the stage now,
Measurements take center frame,
Layout shifts fade to zero—
CLS wears truth like a name.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: keeping perf CLS (Cumulative Layout Shift) setup scrolls outside measured windows, which directly addresses the core problem in the changeset.
Linked Issues check	✅ Passed	The PR fully addresses issue #746's primary objective by moving programmatic scroll alignment outside measured windows and adding runtime guards to prevent setup scrolls during measurements. Secondary objective of auditing other scenarios (session-timeline-recompute) is also completed.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the perf-probe CLS false-positive issue: new unit tests validate measured-window boundaries, and perf-probe spec modifications implement the required measured-window guards and setup-scroll repositioning.
Description check	✅ Passed	The pull request description is comprehensive and complete. It includes all required sections: a clear summary of changes, explanation of the problem and goal, a linked issue reference, human review status, specific review focus, identified risk notes with conditional checklist items explained, detailed verification steps with actual test results, and confirmation of checklist completion.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/i746-perf-cls-window

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request introduces a structured mechanism for measuring performance windows in E2E tests, including a measurePerfWindow helper and a guard to prevent setup-only scroll jumps during active measurements. It also includes a new unit test for these constraints and improves the hoverTimelineScrollLane helper. A review comment identified a logic error in the session-timeline-recompute test, where scrolling up from the top of the container would likely cause an assertion failure.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/app/e2e/perf/perf-probe.spec.ts`:
- Around line 165-168: The fixed 250ms sleep in settlePerfSetup makes the test
timing-dependent; replace the waitForTimeout(250) with a measurable observable
wait (e.g., use page.waitForFunction) that waits for a stable condition before
opening the measured window. In settlePerfSetup (used by snapshotPerfProbe),
instead of sleeping, wait for a concrete signal such as document.readyState ===
'complete' and no new frames or long tasks for a short observation window (for
example poll performance.getEntriesByType('frame') or a custom window variable
and resolve when its count/lastTimestamp hasn't changed for ~200–300ms) so the
test only proceeds when the page has actually settled.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3dae00d1-2137-4b27-bbaf-0081c06f2ca1

📥 Commits

Reviewing files that changed from the base of the PR and between 9d8fd29 and 899bb25.

📒 Files selected for processing (2)

• packages/app/e2e/perf/perf-probe-window.unit.ts
• packages/app/e2e/perf/perf-probe.spec.ts

[github-actions]

Suggested priority: P3 (only low-risk paths changed (packages/app/e2e/perf/perf-probe-window.unit.ts, packages/app/e2e/perf/perf-probe.spec.ts)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

[github-actions]

Perf delta summary

Comparator: pass

Profile / Scenario	interaction median	interaction worst	long task max	tbt	frame gap p95	frame gap max	jank count	cls	status
default / homepage-cold	48 -> 24 (-24)	48 -> 32 (-16)	84 -> 68 (-16)	34 -> 18 (-16)	16.8 -> 33.3 (+16.5)	166.7 -> 133.4 (-33.3)	5 -> 3 (-2)	0 -> 0 (0)	pass
default / long-session-input-lag	48 -> 48 (0)	48 -> 48 (0)	0 -> 0 (0)	0 -> 0 (0)	16.7 -> 16.7 (0)	16.8 -> 16.8 (0)	0 -> 0 (0)	0 -> 0 (0)	pass
default / session-streaming-long	40 -> 40 (0)	72 -> 56 (-16)	0 -> 0 (0)	0 -> 0 (0)	16.8 -> 16.8 (0)	33.4 -> 33.3 (-0.1)	0 -> 0 (0)	0 -> 0 (0)	pass
default / tool-call-expand	24 -> 16 (-8)	40 -> 16 (-24)	0 -> 0 (0)	0 -> 0 (0)	16.7 -> 16.7 (0)	16.7 -> 16.7 (0)	0 -> 0 (0)	0 -> 0 (0)	pass
default / tool-default-open-heavy-bash	16 -> 16 (0)	24 -> 16 (-8)	93 -> 61 (-32)	66 -> 11 (-55)	50 -> 33.4 (-16.6)	116.7 -> 166.7 (+50)	3 -> 2 (-1)	0 -> 0 (0)	pass
default / terminal-side-panel-open	48 -> 48 (0)	48 -> 48 (0)	0 -> 0 (0)	0 -> 0 (0)	33.3 -> 33.4 (+0.1)	33.3 -> 33.4 (+0.1)	0 -> 0 (0)	0 -> 0 (0)	pass
default / session-scroll-reading	16 -> 16 (0)	16 -> 16 (0)	0 -> 0 (0)	0 -> 0 (0)	16.8 -> 16.7 (-0.1)	16.8 -> 16.7 (-0.1)	0 -> 0 (0)	0 -> 0 (0)	pass
low-end / session-scroll-reading-long	88 -> 0 (-88)	120 -> 0 (-120)	149 -> 81 (-68)	486 -> 199 (-287)	33.4 -> 33.4 (0)	166.6 -> 133.4 (-33.2)	19 -> 9 (-10)	0.011 -> 0.011 (0)	pass
low-end / session-timeline-recompute	32 -> 32 (0)	40 -> 32 (-8)	0 -> 0 (0)	0 -> 0 (0)	33.2 -> 33.3 (+0.1)	33.4 -> 33.4 (0)	0 -> 0 (0)	0.563 -> 1.075 (+0.512)	warn: cls_delta
low-end / concurrent-shimmer-extreme	0 -> 0 (0)	0 -> 0 (0)	0 -> 0 (0)	0 -> 0 (0)	16.7 -> 16.8 (+0.1)	16.8 -> 16.8 (0)	0 -> 0 (0)	0 -> 0 (0)	pass