feat(review): mark files viewed as you scroll past them

[backnotprop]

Files check themselves off as you read a review. One invariant drives both surfaces: a file becomes viewed when the reviewer moves on from it after its content was actually on screen long enough to have been read. Arriving at a file never marks it; leaving it downward does.

On by default, with two off switches and a one time notice the first time it fires.

What shipped, rule by rule

Rule 1, all files surface: pass below plus dwell. A file marks when the tracked reading file moves to a later file AND the passed file's successor has reached the viewport top (it genuinely scrolled out above, not merely lost the active race) AND it has accumulated at least AUTO_VIEW_DWELL_MS (1000ms) as the reading file this snapshot. Dwell is cumulative across visits, so comparing two files back and forth still accrues, while a momentum flick to the bottom marks nothing. Collapsed cards accrue no dwell and never emit. At the bottom of the container the currently active last file counts as passed once its dwell is met, because it can never scroll out above.

Implementation note beyond the design: the pass is judged on a candidate that survives across scroll ticks. The active file rule switches at scrollTop + 50 while a pass demands the successor actually reach the top, so a single scroll frame can land inside that gap. Judging only on the transition tick skipped that file for the rest of the session, which the browser smoke run surfaced (5 of 6 files marked on a full read through). With the candidate it is 6 of 6.

Rule 2, single file panel: navigate away plus dwell. Opening a file never marks it. Moving the panel off file A, to another file or to nothing, marks A if it was the panel's file for the same dwell. Keyboard file navigation drives the same panel switches, so keyboard only parity is automatic.

Rule 3, manual un-view suppresses. Un-viewing a file, by v, the header button, or a tree row, adds it to a suppression set that auto-view never re-marks. Marking it viewed by hand clears the suppression. The set rides the code review draft as an additive optional autoViewSuppressed?: string[]. Deliberate edge kept from the design: a draft whose only state is suppression is still empty and is still tombstoned, so #948's clear everything semantics are untouched.

Rule 4, scope. Inert while the Guided Review takeover is open and while a commit:<sha> detour is active. Dwell stops accruing rather than banking, so leaving a detour does not immediately mark the next file passed. It does run in PR mode, workspace, GitButler, and jj. PR marks batch into ONE /api/pr-viewed POST (about 2s window); the checkmarks themselves apply immediately, only the remote sync waits.

Rule 5, changed files un-view on refresh. Shipped, not severed. When a refresh or switch applies a new patch and a viewed file's per path patch text changed, it loses its checkmark, because a check on code the agent just rewrote is misleading. Gated on the setting: with auto-view off, today's manual semantics stay byte identical. Files absent from the new patch keep their viewed state. No platform sync, since GitHub applies the same rule to its own viewed state server side.

Rule 6, Guided Review independence. No link, as designed. Guide section reviewed state is untouched.

Rule 7, submission. No change. Nothing gates submit on viewed count and this does not introduce a gate.

Setting and notice

reviewAutoViewed, boolean, default true, cookie only (plannotator-review-auto-viewed), declared beside reviewShowViewedControls in the settings registry. Two switches:

• Settings > Git > Viewed files > Auto-mark viewed
• Auto-mark viewed row in the file list gear popover ("Check files off as you scroll past")

The first time auto-view actually marks something, a toast says so and offers Turn off, which flips the setting and confirms. The toast is deferred, not lost, behind the guide takeover or any first run dialog: the file still marks and the next auto-view retries the toast, so it can never stack on the startup dialog chain. The seen marker is a versioned cookie, and it is also consumed by an explicit toggle from either switch, because someone holding the switch has demonstrably found it.

Footprint

The monolith stayed small. New logic lives in its own files; AllFilesCodeView gained one optional emission callback on the rAF path it already runs, and App.tsx gained the hook mount, the suppression feed, and the toast.

File	Added	Removed
packages/review-editor/utils/autoViewed.ts (new, pure)	292	0
packages/review-editor/hooks/useAutoViewed.ts (new)	226	0
packages/review-editor/utils/autoViewedNotice.ts (new)	31	0
packages/review-editor/App.tsx	139	12
packages/review-editor/components/AllFilesCodeView.tsx	76	7
packages/review-editor/components/PanelChrome.tsx	36	0
packages/ui/config/reviewView.ts	37	0
packages/ui/hooks/useCodeAnnotationDraft.ts	19	2
packages/ui/components/Settings.tsx	16	1
packages/ui/config/settings.ts	15	0
packages/review-editor/components/{FileTree,SectionsPanel}.tsx	12	0
packages/review-editor/dock/{ReviewStateContext,panels/ReviewAllFilesDiffPanel}.tsx	5	1
packages/ui/config/index.ts	3	0
Tests (4 new files plus 2 extended)	949	0
Docs, CI registration	21	0

Every new prop is optional and defaults to today's behavior, per the published package rule. @plannotator/ui changes are additive: one registry entry, one optional draft field, one Settings row, and the notice gate beside the setting (it lives in ui/config/reviewView.ts because both setting writers must consume it). No server changes in either runtime; Pi and OpenCode get the feature through the built HTML.

Tests

Every test names the regression it guards.

packages/review-editor/utils/autoViewed.test.ts (14, pure, no DOM, no timers): flick through five files marks nothing; read through marks and an upward move never does; cumulative dwell across two 600ms visits; collapsed cards accrue nothing; enabled: false produces zero marks from input that otherwise marks; un-view then re-pass is suppressed and a manual re-view clears it; snapshot reset clears dwell but keeps suppression; a suspended stretch accrues nothing; navigate away marks only after the floor; the batcher sends one call with every path and drops a pending batch on dispose; Rule 5 un-views only the changed file, un-views nothing when the setting is off, and keeps a file missing from the new patch.

packages/review-editor/components/AllFilesCodeView.autoViewed.test.tsx (10, DOM): the emission contract against a fake CodeView geometry. Downward pass emits once the successor reaches the top; a predecessor still on screen emits nothing; the threshold gap resolves on a later tick; a candidate abandoned by scrolling back up is dropped; upward never emits; collapsed never emits, including at the bottom; the reading file report carries the collapsed flag; without the handler nothing changes.

packages/review-editor/hooks/useAutoViewed.test.tsx (7, DOM): three files read through produce ONE platform sync carrying all three while the checkmarks appear immediately; the off switch severs the pipeline; suppression reaches the core; a suspended session marks nothing and banks nothing; the single file panel marks on navigate away and never on open; a new snapshot clears dwell; a file parked at the bottom matures without further scrolling.

packages/review-editor/utils/autoViewedNotice.test.ts (5): needed once then never; a stale version re-opens the gate; the toast's Turn off action actually turns the setting off (the copy is not snapshotted, the action's effect is); an explicit toggle consumes the gate; the cookie round trips and the default is on.

Extended: packages/ui/codeAnnotationDraftPersistence.test.tsx (suppression round trips, a draft without the field restores empty, and suppression alone does not keep an empty draft alive) and packages/review-editor/components/PanelChrome.test.tsx (the gear row appears only when a handler is supplied, and toggles).

All storage in these tests goes through a memory backend or a temp PLANNOTATOR_DATA_DIR; nothing touches the real ~/.plannotator, and nothing mutates env or globals at module scope.

Results

• bun test: 4045 pass, 12 fail. The same 12 fail on the base commit (79cb0168), which had 4020 pass, 12 fail. They are the sem and but binaries being absent locally.
• Full DOM lane as CI runs it, plus the two new files: 524 pass, 0 fail.
• bun run typecheck (all projects including tsconfig.strict-consumer.json): clean.
• packages/review-editor/tsconfig.json is not a CI target and reports 164 pre-existing errors on both base and branch; the branch adds only the 4 Cannot find module 'bun:test' lines every test file in that project already produces.
• The two new DOM files are registered in .github/workflows/test.yml beside the existing entries.

Bundle: apps/review/dist/index.html 17,571,622 to 17,580,152 bytes, +8,530 (+0.049%).

Browser smoke

Compiled binary, sandboxed PLANNOTATOR_DATA_DIR under a scratch dir, fixed port, PLANNOTATOR_BROWSER=/usr/bin/true, against a real 7 file diff (6 source files plus a bun.lock that Plannotator classifies as generated and seeds collapsed). Driven with Playwright.

FLICK-THROUGH: 0/7 -> 1/7 viewed: ["src/mod1.ts"] (of 7 files flicked past)
  the one mark is the file that had been on screen since page load; the six
  files flicked past at 40ms per step marked nothing
TOAST: "Files are marked viewed as you scroll / Scroll past a file or move on
  to the next and it's checked off. Turn this off in Settings -> Git, or from
  the gear above the file list. / Turn off"
TURN OFF -> cookie: plannotator-review-auto-viewed=false
READ-THROUGH WHILE OFF: 1/7 -> 1/7
GEAR ROW present: 1  aria-checked: false  (re-enabled from the popover)
PACED READ-THROUGH: 6/7   viewed: mod1..mod6
bun.lock (generated, collapsed) auto-viewed? false
UN-VIEW src/mod1.ts -> counter 5/7, draft autoViewSuppressed: ["src/mod1.ts"]
SUPPRESSION after another read + pass: 5/7 -> 5/7

Rule 5 was driven separately by rewriting one viewed file on disk and clicking the staleness Refresh:

setting = true  : 2/7 -> 1/7  (only the rewritten file lost its check)
setting = false : 2/7 -> 2/7  (byte identical to today)

Screenshots from the run, on the maintainer's machine at
/private/tmp/claude-501/-Users-ramos-plannotator-plannotator/34d56d5a-33a8-4878-92b2-3f4318214ae2/scratchpad/shots/:

• 04-first-time-toast.png the toast with Turn off, counter at 1/7, src/mod1.ts checked in the tree
• 06-gear-popover.png the gear popover with the third row Auto-mark viewed / Check files off as you scroll past, off, and the confirmation toast
• 10-settings-git.png Settings > Git > Viewed files > Auto-mark viewed, on by default
• 07-read-through.png, 08-unviewed.png, 09-suppression-held.png, 11-rule5.png the remaining steps

Notes and deliberate limits

• The dwell constant and the roughly 2s PR batch window are named starting values, not contracts.
• Auto marks look identical to manual marks. The suppression rule makes the distinction behaviorally unnecessary.
• A very short file read at normal scroll speed can accrue under 1000ms and not mark. Accepted for v1, as designed; the constant is central if reports come in.
• Demo and standalone mode work with no special casing: the core runs in session, drafts and PR sync are simply absent.

AI-assisted (Claude) under maintainer direction.

[backnotprop]

Review findings addressed in bad3c256. The pass-candidate mechanism is unchanged.

1. HIGH: Rule 5 fired on every applied diff switch

Correct and worse than it looked: the Commits rail auto-opens HEAD on entry, so merely clicking the Commits segment applied a commit:<sha> diff whose per-path text differs from the review diff, and every viewed file that commit touched lost its checkmark. Same mechanism on a base switch and on the hide-whitespace toggle, whose patches differ by design.

The apply path now calls resolveDiffSwitchUnviews, which answers "is this even a moment where a patch delta means the content changed?" before the delta logic runs. The pure resolveContentChangedUnviews is untouched.

Two layers, because the discriminator alone is not enough. The hide-whitespace toggle also re-fetches the same selection with the same base, so identity does not separate it from a refresh. The caller therefore has to opt in with contentRefresh: true, which only handleRefreshStaleDiff and handleFetchBase do, and the resolver re-checks identity anyway: same requested/active diff type, same base, and never a commit-family type on either side of the transition. A future caller that passes the flag on a diff-changing switch still cannot un-view anything.

Call-site scope is now pinned by tests, since that is where the guarantee actually lives:

• autoViewed.test.ts names the transitions: staleness refresh with changed content un-views the changed file; a commit-detour entry un-views nothing (including when it wrongly opts in); the hide-whitespace toggle un-views nothing; a diff-type or base switch un-views nothing even opted in; the off switch still severs an otherwise-qualifying refresh.
• autoViewedCallSites.test.ts (new, source-level, same idiom as webmcp/iframeIsolation.test.ts) asserts exactly two call sites opt in, both with preserveFile: true; the hide-whitespace effect does not; no commit-family call does; and the apply path calls the scoped resolver rather than the raw one. Both of its Rule 5 assertions fail against the previous commit's App.tsx, which is the check that was missing.

Note on handleFetchBase: it is one of the two opted-in callers. It re-fetches the same selection and same base after git fetch advances the remote tracking ref, which is the "the content moved under me" case, not a change of what is being compared. Say the word if you would rather it stayed out.

2. MEDIUM: at-bottom fired on the mount tick

Confirmed. reportVisibleFile's at-bottom branch is true from the first report when the diff fits the viewport, and that first report is the mount seed, so the visible file marked itself about a second later with no interaction at all and fired the first-time toast at a motionless page. Exactly the thing the contract forbids.

The branch now requires hasScrolledRef, set by a real onScroll and reset with the rest of the per-file-set state on fileSetKey. Only the at-bottom branch is gated: the pass-candidate path cannot fire on the mount seed anyway, because there is no previous item to have moved on from.

New emission test: mount with a viewport-fitting diff, wait past the dwell floor, assert nothing emits; then one real scroll and assert the at-bottom mark proceeds. It fails against the previous commit.

3. LOW: staging did not clear suppression

handleFileViewedFromStage now routes through applyAutoViewSuppression(path, true), the same clearing v, the header button and the tree row get. Pinned in the source-level test as "every path that marks a file viewed clears its suppression".

4. LOW: dwell accrued while disabled

AutoViewedTracker.commit now consults enabled. activeSince still advances, so enabling starts a fresh clock rather than replaying the disabled stretch. New test: accrue a full dwell while off, enable, pass, assert no mark; then assert genuinely fresh dwell does mark. It fails against the previous commit.

Not touched, per instruction: tree-click forward-jump marking, the reviewShowViewedControls interplay, and the two nits.

Verification

• bun test: 4056 pass, 12 fail. The same 12 as the base commit (sem and but absent locally); the failing test names are identical.
• Full DOM lane as CI runs it, in one shared process: 527 pass, 0 fail.
• bun run typecheck: clean. packages/review-editor/tsconfig.json (not a CI target) goes 168 to 172, and all four new lines are the bun:test / node-types / import.meta errors every test file in that project already produces.
• Each new or extended test was run against the previous commit's code and fails there, then passes after.
• Bundle: apps/review/dist/index.html 17,580,152 to 17,580,756 bytes, +604 for this round (+9,134 against base main, +0.052%).

Browser re-smoke of the two user-visible fixes, on a 2 file branch diff where the commit under the detour touches one of the viewed files:

A  viewport-fitting diff, no interaction, after 6s: 0/2 viewed
A  first-time toast shown? false
B  marked 2 by hand -> 2/2
B  inside the Commits detour, the rendered file header still reads "Mark as not viewed"
B  after exiting the detour: 2/2

Screenshots: 12-fit-no-automark.png, 13-commit-detour-keeps-viewed.png, 14-detour-exit.png beside the earlier ones in the scratch shots/ directory named in the PR body.

AI-assisted (Claude) under maintainer direction.