Skip to content

Keep tour steps and code peeks clipped to their pinned lines - #627

Open
jykim256 wants to merge 2 commits into
devdotfast:mainfrom
jykim256:fix/exact-document-lens
Open

jykim256 wants to merge 2 commits into
devdotfast:mainfrom
jykim256:fix/exact-document-lens

Conversation

@jykim256

@jykim256 jykim256 commented Sep 27, 2026 •

Copy link
Copy Markdown

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

image

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 Preserve diffr context when applying review lenses #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

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>
@thesiti92
thesiti92 requested a review from sidkmenon September 28, 2026 17:37
@milanb17
milanb17 self-requested a review September 30, 2026 17:47
@milanb17 milanb17 self-assigned this Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants