Repository navigation
Preserve diffr context when applying review lenses - #502
Merged
Merged
Conversation
Agent-Session: b320a5ae-50a4-4472-a324-515d964c5f5a Agent-Session: 01a0cf8e-5b2b-7253-9d6c-2d77ac6485b8
sidkmenon
added this pull request to stack #505
September 23, 2026 19:13
sidkmenon
marked this pull request as ready for review
September 23, 2026 19:21
thesiti92
approved these changes
Sep 23, 2026
ketan0
pushed a commit
to jykim256/whiteboard
that referenced
this pull request
Oct 9, 2026
Tour steps and code peeks build a "document:" lens that goes through lensContextGaps(). Since devdotfast#502, when diffr supplies contextScopes, any unfolded run of lines that touches the lens range is shown whole. An added file is one unfolded run, so the card shows the entire file instead of the pinned lines. Add ReviewDiffLens.exact, set it on the document lens, and have withLens() drop contextScopes before computing gaps for an exact lens. Diff tab lenses never set exact, so devdotfast#502's behavior there is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: 01a0d12e-51ca-7230-9991-0ce7a73018b2
ketan0
added a commit
to jykim256/whiteboard
that referenced
this pull request
Oct 9, 2026
Since devdotfast#502, a lens with diffr's contextScopes shows every open run (rows outside a collapsed band) that holds a pinned line or the first or last line of a scope enclosing one. diffr collapses nothing in an added file, so its one open run is the whole file, and a peek pinned to a few lines showed all of it. Replace the `exact` lens flag with a narrower rule in lensContextGaps: an open run made only of added lines shows only the pinned lines, three lines around them, and the first and last lines of their enclosing scopes. diffr never collapses inside added code, so no band of its own bounds such a run; a scope the reader folded only splits it. Runs with any base-side line are unchanged, so modified files keep diffr's context, and peeks and tour steps keep diffr's scopes instead of dropping them. The rule also applies to Diff-tab section lenses on added files, which now get the same narrow view. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: 01a0d12e-51ca-7230-9991-0ce7a73018b2
ketan0
added a commit
that referenced
this pull request
Oct 9, 2026
### Problem When you step through a sequence diagram's tour, or open a code peek, the code card is supposed to show just that step's line range, like `workflow_evaluation.py:199-204`. Instead it shows the whole file for a newly added file, and the whole diffr hunk region for a modified file. This broke in #502, "Preserve diffr context when applying review lenses." That PR made `lensContextGaps()` honor diffr's structural scope info (`diff.contextScopes`). That's the right call for the Diff tab, where you want a lens to keep the syntactic context around the code it highlights. But tour steps and code peeks use a different kind of lens. They build a "document" lens with one exact chunk in mind, and that lens goes through the same `lensContextGaps()` function. The bug comes from the run-fill rule. When `contextScopes` is set, `lensContextGaps()` shows every unfolded run of lines that touches the lens range. For an added file, diffr folds nothing, so the whole file is one run. One small lens range anywhere in that file makes the whole file visible. ### Fix Added a new `exact` flag to `ReviewDiffLens`. The document lens (used by tour steps and code peeks) sets `exact: true`. When `withLens()` sees a lens with `exact: true`, it strips `contextScopes` off the diff before calling `lensContextGaps()`. Without `contextScopes`, only the old +/-3 line window around the pinned range applies, same as before #502. Lenses used by the Diff tab never set `exact`, so nothing changes there. #502's scope-preserving behavior is still in effect for normal diff review. ### Authoring guidance - `instructions/authoring.md` now tells authors to pin the smallest range that shows the claim (usually 3-15 lines), since tour steps and peeks show only that range. ### Reviewer notes This touches the vendored Code - OSS fork (`apps/review-desktop/code-oss/src/vs/review/**`) and adds an optional field to `ReviewDiffLens` in `review-protocol`. No stored-data or telemetry changes. ### Testing <img width="3417" height="1410" alt="image" src="https://github.com/user-attachments/assets/10b86464-e206-4c93-9176-09cce80cc7b9" /> Before left, After right Added a test in `reviewLens.test.ts` that builds an added-file diff (300 lines, all inserted) with a scope covering the whole file, and a lens range of lines 80-87. It checks two things: - Without stripping `contextScopes` (today's #502 behavior), the whole file is visible — no gaps at all. This reproduces the bug. - With `contextScopes` stripped (what `exact: true` triggers), only the +/-3 window around lines 80-87 is visible. This is the fix. Ran the full test suite for `apps/review-desktop`: - `code-oss/src/vs/review/**/*.test.ts` (204 tests via `node --test`) — all pass. - `scripts/**/*.test.mjs` (120 tests) — 1 pre-existing failure (`curated-extensions.test.mjs`, "extracts nested Windows executables from a VSIX archive"), confirmed failing the same way on `main` before this change, unrelated to this fix (missing native module for VSIX extraction). I did not run `typecheck-client` (`tsgo` over the whole code-oss client), because it needs the separate Electron-ABI `npm install` in `code-oss/`. The touched files compile under `tsx` in the test runs above. CI should cover the full typecheck. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: ketan0 <ketanjayagrawal@gmail.com>
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.
Applying a review lens could hide enclosing function context intentionally retained by diffr. Carry syntax scope boundaries through the structural provider and retain the visible context surrounding selected code, without expanding collapsed provider regions.
Reapplies #495 after the rollback using existing Review paths and imports.
Validation: all 23 existing lens and structural-diff tests passed.