diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts index 4470324ed..2c6efa5c4 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts @@ -24,6 +24,20 @@ test('structural folds within the lens survive, overlapping folds do not', () => assert.ok(!gaps.includes(outside)); }); +test('an exact document lens (tour step, code peek) clips an added file to its +/-3 window; without it, #502 fills the whole open run', () => { + const diff = { + ...plain, + sourceLineAlignment: Array.from({ length: 300 }, (_, i) => [null, i] as const), + contextScopes: { original: [], modified: [[0, 300] as const] }, + }; + const ranges = [{ side: 'head' as const, file: 'new.ts', fromLine: 80, toLine: 87 }]; + // #502 behavior (Diff tab): the added file is one open run, so contextScopes fills it whole. + assert.deepEqual(lensContextGaps(diff, 0, 300, ranges), []); + // exact document lens: withLens strips contextScopes before calling lensContextGaps. + const clipped = lensContextGaps({ ...diff, contextScopes: undefined }, 0, 300, ranges); + assert.deepEqual(clipped.map(gap => [gap.modifiedStart, gap.modifiedCount]), [[1, 76], [91, 210]]); +}); + test('viewed folds never hide an unread counterpart, and do not overlap structural folds', async () => { const { viewedContextGaps } = await import('./reviewLens.js'); const range = (side: 'base' | 'head', fromLine: number, toLine: number) => ({ side, file: 'a.ts', fromLine, toLine }); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts index b27f49d51..b50e5bcf3 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts @@ -287,7 +287,7 @@ export class ReviewApiSourceService extends Disposable implements IReviewApiSour const current = spec.pins ? reviewSourceAnchor(view(), spec.pins) : reviewSourceComparison(view()); const lens: ReviewDiffLens = { id: "document:" + JSON.stringify([spec.path, spec.ranges, spec.pins]), title: spec.path, - reviewId: current.reviewId, version: current.version, + reviewId: current.reviewId, version: current.version, exact: true, ranges: spec.ranges.map(range => ({ file: spec.path, side: range.side ?? spec.side, fromLine: range.startLine, toLine: range.endLine })), }; return { lens, source: makeSource(() => current) }; diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts index 8cc8a33d2..1b73282bc 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts @@ -59,7 +59,10 @@ export function withLens(instantiation: IInstantiationService, entries: readonly }) ? { ...gap, collapsed: false } : gap) }; const section = progress()?.sections?.find(section => section.id === entry.sectionId); - return lens && !lens.wholeFiles ? { ...diff, contextGaps: lensContextGaps(diff, original.getLineCount(), modified.getLineCount(), lensRanges(section ? { ...lens, ranges: section.sources } : lens, entry)) } : diff; + // An exact lens (a tour step, a code peek) is authored to show one chunk; + // diffr's structural scope expansion is right for the Diff tab, not here. + const scopedDiff = lens?.exact ? { ...diff, contextScopes: undefined } : diff; + return lens && !lens.wholeFiles ? { ...diff, contextGaps: lensContextGaps(scopedDiff, original.getLineCount(), modified.getLineCount(), lensRanges(section ? { ...lens, ranges: section.sources } : lens, entry)) } : diff; }, }; }, diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index 67e486393..b1a5478b2 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -199,6 +199,8 @@ export interface ReviewInlineEditorFactory { export interface ReviewDiffLens { /** Filter files while retaining ordinary diff context/folding within them. */ wholeFiles?: boolean; + /** Show only the pinned range, ignoring diffr's structural scope expansion. */ + exact?: boolean; id: string; title: string; reviewId: string; diff --git a/packages/review/instructions/authoring.md b/packages/review/instructions/authoring.md index 438b9e7b5..593eb0094 100644 --- a/packages/review/instructions/authoring.md +++ b/packages/review/instructions/authoring.md @@ -34,3 +34,4 @@ follow these first five steps in order, with no tool calls beyond what they need - keep whiteboards short and sweet when possible (esp. for small changes.) feel free to omit sections. - when something (a phrase in the prose, diagram node, etc.) describes actual code in the codebase, always default to attaching/hyperlink code. - Link repository code as `[label](review-source:head/src/file.ts#L10-L24)`; use `base` for old code. Use repository-relative paths and verified line numbers. +- point each code reference (diagram step, call-stack frame, `code_peek`) at the smallest range that shows the claim, usually 3-15 lines: the call, the branch, the assignment. not the whole function. tour steps and peeks show only that range.