Repository navigation
feat(review): persist viewed-file progress across review sessions - #1632
Merged
backnotprop merged 5 commits intoSep 29, 2026
Merged
Conversation
extractReviewPatch ran one git hash-object per dirty working-tree file, sequentially, plus a rev-parse --show-toplevel per untracked file. That put ~9 ms per file on startup, every /api/diff/switch and every Refresh (200 dirty files: 102 ms -> 1.8 s; 3300 files: 0.5 s -> 29 s). Identities are now resolved once per snapshot: every regular file goes through ONE git hash-object --stdin-paths call from the repo root (so clean filters and attributes resolve exactly as before), symlinks keep a --stdin call each, and untracked files are resolved together after their --no-index diffs instead of one by one. The size cap, the gitlink skip and the check against the displayed index line are unchanged. Paths the line protocol cannot carry verbatim (line breaks, CR, a leading quote) get no identity rather than a wrong one; a path that vanishes mid-batch leaves the files after it without an identity.
…IEW_PROGRESS
Viewed-file progress writes repo file paths in plain text under
review-progress/ in the data dir, and nothing prunes it, so it gets the same
kind of opt-out as the other persistence features: PLANNOTATOR_REVIEW_PROGRESS=0
(also false/off/disabled) or { "reviewProgress": false } in config.json. The
env var wins, an empty or unrecognized env value counts as unset, and the
default stays enabled (resolveReviewProgress in packages/shared/config.ts).
When off, both review servers capture no progress snapshot, so
/api/review-progress answers { available: false } without reading or
writing records, and the client falls back to draft-based viewed tracking
exactly as it does for unsupported modes. Documented in the AGENTS.md
environment table, the marketing environment-variable and code-review pages,
and the privacy page's choices list.
Both servers captured the progress snapshot at declaration and again from the initial fingerprint capture whenever the caller supplied no fingerprint, running the identity lookups twice at startup. Capture it once: through the fingerprint capture, or directly when a fingerprint was supplied.
- Chunk progress writes at 1000 changes per POST (the server refuses more than 5000), and drop edits the server rejects with 400 instead of retrying them on every later save and flush. - Re-arm the failure toast after a successful load or save, so a new failure later in the session is reported instead of swallowed for the page lifetime. - markFilesViewed (auto-view) persists only files it actually changes, not every already-viewed path it is handed again. - Keep persistViewedFiles false until the diff snapshot is known: before that the progress status reads 'unsupported', and the true -> false flip once loading began could flash the draft banner for a viewed-only draft. The hook's default for hosts is unchanged.
Owner
|
Thanks @oorestisime, this is a careful piece of work: the snapshot-bound writes, the conservative identity check against the patch's index lines, and Bun/Pi parity were all solid. Before merging we pushed a few commits on top:
It'll ship in the next release. |
YeKc1M
pushed a commit
to YeKc1M/plannotator
that referenced
this pull request
Oct 1, 2026
…cknotprop#1632) * feat(review): persist viewed-file progress across review sessions * perf(review): hash worktree file identities in one batch per snapshot extractReviewPatch ran one git hash-object per dirty working-tree file, sequentially, plus a rev-parse --show-toplevel per untracked file. That put ~9 ms per file on startup, every /api/diff/switch and every Refresh (200 dirty files: 102 ms -> 1.8 s; 3300 files: 0.5 s -> 29 s). Identities are now resolved once per snapshot: every regular file goes through ONE git hash-object --stdin-paths call from the repo root (so clean filters and attributes resolve exactly as before), symlinks keep a --stdin call each, and untracked files are resolved together after their --no-index diffs instead of one by one. The size cap, the gitlink skip and the check against the displayed index line are unchanged. Paths the line protocol cannot carry verbatim (line breaks, CR, a leading quote) get no identity rather than a wrong one; a path that vanishes mid-batch leaves the files after it without an identity. * feat(review): opt out of durable viewed progress with PLANNOTATOR_REVIEW_PROGRESS Viewed-file progress writes repo file paths in plain text under review-progress/ in the data dir, and nothing prunes it, so it gets the same kind of opt-out as the other persistence features: PLANNOTATOR_REVIEW_PROGRESS=0 (also false/off/disabled) or { "reviewProgress": false } in config.json. The env var wins, an empty or unrecognized env value counts as unset, and the default stays enabled (resolveReviewProgress in packages/shared/config.ts). When off, both review servers capture no progress snapshot, so /api/review-progress answers { available: false } without reading or writing records, and the client falls back to draft-based viewed tracking exactly as it does for unsupported modes. Documented in the AGENTS.md environment table, the marketing environment-variable and code-review pages, and the privacy page's choices list. * fix(review): capture the startup progress snapshot once Both servers captured the progress snapshot at declaration and again from the initial fingerprint capture whenever the caller supplied no fingerprint, running the identity lookups twice at startup. Capture it once: through the fingerprint capture, or directly when a fingerprint was supplied. * fix(review): harden the viewed-progress client - Chunk progress writes at 1000 changes per POST (the server refuses more than 5000), and drop edits the server rejects with 400 instead of retrying them on every later save and flush. - Re-arm the failure toast after a successful load or save, so a new failure later in the session is reported instead of swallowed for the page lifetime. - markFilesViewed (auto-view) persists only files it actually changes, not every already-viewed path it is handed again. - Keep persistViewedFiles false until the diff snapshot is known: before that the progress status reads 'unsupported', and the true -> false flip once loading began could flash the draft banner for a viewed-only draft. The hook's default for hosts is unchanged. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Viewed-file progress now survives feedback submission, server restarts, and reopening local Git or PR reviews, independently of Auto-mark viewed. Unchanged files retain their marks; files whose reviewed content changed become unviewed.
Progress is stored separately from annotation drafts in atomic per-file records, with shared storage/validation and matching Bun and Pi endpoints. Local Git scopes follow the worktree, branch, and comparison selection; PR scopes follow the PR and layer/full-stack selection. Workspace, other VCS providers, and piped patches keep draft-based tracking.
Review focus
review-core.tscaptures full object IDs, paths, and modes in a sidecar without changing rendered patch bytes or existing draft keys. Worktree hashes are bounded and checked against the displayed patch; files without reliable identities are conservatively excluded from restoration.useReviewProgressoverlays clicks made during loading, orders snapshot-bound saves, preserves explicit unchecks, and flushes before switches and decisions. Both requests and the total flush have five-second limits; failures retain pending edits for retry and failed loads keep draft fallback available.persistViewedFilesseam, defaulting to existing behavior. PR draft recovery and progress restoration remain independent.Browser verification covered reload and a fresh server session after editing one of two viewed files: only the unchanged file stayed viewed, and hide-viewed filtering matched. Startup overhead from per-file worktree hashing on large changesets has not yet been measured.
Fixes #1136