Conversation
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
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 samelensContextGaps()function.The bug comes from the run-fill rule. When
contextScopesis 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
exactflag toReviewDiffLens. The document lens (used by tour steps and code peeks) setsexact: true. WhenwithLens()sees a lens withexact: true, it stripscontextScopesoff the diff before callinglensContextGaps(). WithoutcontextScopes, 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.mdnow 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 toReviewDiffLensinreview-protocol. No stored-data or telemetry changes.Testing
Before left, After right
Added a test in
reviewLens.test.tsthat 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:contextScopes(today's Preserve diffr context when applying review lenses #502 behavior), the whole file is visible — no gaps at all. This reproduces the bug.contextScopesstripped (whatexact: truetriggers), 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 vianode --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 onmainbefore this change, unrelated to this fix (missing native module for VSIX extraction).I did not run
typecheck-client(tsgoover the whole code-oss client), because it needs the separate Electron-ABInpm installincode-oss/. The touched files compile undertsxin the test runs above. CI should cover the full typecheck.🤖 Generated with Claude Code