diff --git a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts index 523f3313c..afaf2b2c7 100644 --- a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts +++ b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts @@ -189,6 +189,7 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject< return { ...options, ...optionsOverride?.get(), + ...(options.forceInline ? { renderSideBySide: false } : {}), scrollBeyondLastLine: false, hideUnchangedRegions: options.hideUnchangedRegions ?? { enabled: true, diff --git a/apps/review-desktop/code-oss/src/vs/editor/common/config/editorOptions.ts b/apps/review-desktop/code-oss/src/vs/editor/common/config/editorOptions.ts index 6a7973ad8..b30661ab2 100644 --- a/apps/review-desktop/code-oss/src/vs/editor/common/config/editorOptions.ts +++ b/apps/review-desktop/code-oss/src/vs/editor/common/config/editorOptions.ts @@ -1015,6 +1015,8 @@ export interface IDiffEditorBaseOptions { * Configuration options for the diff editor. */ export interface IDiffEditorOptions extends IEditorOptions, IDiffEditorBaseOptions { + /** Keep an explicitly one-sided item inline even when its multi-diff container uses split layout. @internal */ + forceInline?: boolean; } /** diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/media/review.css b/apps/review-desktop/code-oss/src/vs/review/browser/media/review.css index 03fcaee15..2e5403f59 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/media/review.css +++ b/apps/review-desktop/code-oss/src/vs/review/browser/media/review.css @@ -1204,3 +1204,9 @@ body .review-diff-group-toggle { display:flex; align-items:center; gap:8px; min-width:0; border:0; padding:0; color:inherit; background:transparent; cursor:pointer; text-align:left; } .review-diff-group-toggle:focus-visible { outline:1px solid var(--vscode-focusBorder); } + +/* Search evidence is distinct from inserted/deleted-line coloring. */ +.review-search-highlight { + background-color: rgba(65, 135, 245, 0.30); + outline: 1px solid rgba(65, 135, 245, 0.65); +} diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.ts index c59c126a9..a2bd1d383 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.ts @@ -6,7 +6,7 @@ import type { IDocumentDiff, IDocumentContextGap } from '../../editor/common/dif import type { ReviewDiffLens } from './reviewProtocol.js'; /** Project pinned ranges onto the current diff's correspondence, never onto another revision. */ -export function lensContextGaps(diff: IDocumentDiff, originalCount: number, modifiedCount: number, ranges: ReviewDiffLens['ranges']): IDocumentContextGap[] { +export function lensContextGaps(diff: IDocumentDiff, originalCount: number, modifiedCount: number, ranges: Extract['ranges']): IDocumentContextGap[] { const rows = alignmentRows(diff, originalCount, modifiedCount); const visible = rows.map(row => ranges.some(range => { const line = row[range.side === 'base' ? 0 : 1]; @@ -48,9 +48,9 @@ function alignmentRows(diff: IDocumentDiff, originalCount: number, modifiedCount } /** A paired row folds only when none of its changed lines remain unread. */ -export function viewedContextGaps(diff: IDocumentDiff, originalCount: number, modifiedCount: number, viewed: ReviewDiffLens['ranges'], changed: ReviewDiffLens['ranges']): IDocumentContextGap[] { +export function viewedContextGaps(diff: IDocumentDiff, originalCount: number, modifiedCount: number, viewed: Extract['ranges'], changed: Extract['ranges']): IDocumentContextGap[] { const rows = alignmentRows(diff, originalCount, modifiedCount); - const contains = (ranges: ReviewDiffLens['ranges'], side: 0 | 1, line: number) => ranges.some(range => range.side === (side === 0 ? 'base' : 'head') && line + 1 >= range.fromLine && line + 1 <= range.toLine); + const contains = (ranges: Extract['ranges'], side: 0 | 1, line: number) => ranges.some(range => range.side === (side === 0 ? 'base' : 'head') && line + 1 >= range.fromLine && line + 1 <= range.toLine); const hidden = rows.map(row => row.some((line, side) => line !== null && contains(viewed, side as 0 | 1, line)) && row.every((line, side) => line === null || !contains(changed, side as 0 | 1, line) || contains(viewed, side as 0 | 1, line))); const gaps: IDocumentContextGap[] = []; let left = 1, right = 1; diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.test.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.test.ts index a7b786b00..8278b5923 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.test.ts @@ -5,11 +5,11 @@ import { lensContextGaps } from './reviewLens.js'; import type { ReviewDiffFileWire, ReviewDiffLens } from './reviewProtocol.js'; const files: ReviewDiffFileWire[] = [{ path: 'renamed.ts', previousPath: 'old.ts', status: 'renamed', additions: 2, deletions: 1 }]; -const lens: ReviewDiffLens = { id: 'lens', title: 'Context', reviewId: 'review', version: 1, ranges: [ +const lens: ReviewDiffLens = { id: 'lens', title: 'Context', reviewId: 'review', version: 1, targets: [{ kind: 'ranges', ranges: [ { file: 'old.ts', side: 'base', fromLine: 2, toLine: 5 }, { file: 'context.ts', side: 'base', fromLine: 10, toLine: 12 }, { file: 'context.ts', side: 'head', fromLine: 10, toLine: 12 }, -] }; +] }] }; test('diagram references add each unchanged file once, preserving rename identity', () => { const result = lensFiles(files, lens); @@ -23,6 +23,6 @@ test('clearing the lens and whole-file glob lenses do not introduce context file }); test('an identical file exposes the referenced slice with three surrounding lines', () => { - const gaps = lensContextGaps({ changes: [], moves: [], identical: true, quitEarly: false }, 30, 30, lens.ranges.filter(range => range.file === 'context.ts')); + const gaps = lensContextGaps({ changes: [], moves: [], identical: true, quitEarly: false }, 30, 30, lens.targets.flatMap(target => target.kind === 'ranges' ? target.ranges : []).filter(range => range.file === 'context.ts')); assert.deepEqual(gaps.map(gap => [gap.originalStart, gap.originalCount, gap.modifiedStart, gap.modifiedCount]), [[1, 6, 1, 6], [16, 15, 16, 15]]); }); diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.ts index 8fd4a8578..c36124047 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewLensFiles.ts @@ -5,7 +5,7 @@ export function lensFiles(files: readonly ReviewDiffFileWire[], lens?: ReviewDif if (!lens || lens.wholeFiles) return files; const known = new Set(files.flatMap(file => [file.path, ...(file.previousPath ? [file.previousPath] : [])])); const context: ReviewDiffFileWire[] = []; - for (const range of lens.ranges) { + for (const range of lens.targets.flatMap(target => target.kind === "ranges" ? [...target.ranges] : [])) { if (known.has(range.file)) continue; known.add(range.file); context.push({ path: range.file, status: 'unchanged', additions: 0, deletions: 0 }); diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.test.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.test.ts new file mode 100644 index 000000000..d86de5785 --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.test.ts @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import type { SearchResultData, SourceData } from "./reviewProtocol.js"; +import { evidenceRows } from "./reviewSearchEvidence.js"; + +const source = (text: string): SourceData => ({ + text, + regions: [ + { + kind: "leaf", + id: 1, + fold_state_id: 1, + alignment_id: 1, + start: { line: 0, column: 0 }, + end: { line: 1, column: 0 }, + search_highlights: [{ line: 0, start_column: 2, end_column: 6 }], + }, + { + kind: "fold", + id: 2, + fold_state_id: 2, + start: { line: 1, column: 0 }, + end: { line: 2, column: 0 }, + visibility: { collapsed: true }, + children: [ + { + kind: "leaf", + id: 3, + fold_state_id: 3, + alignment_id: 3, + start: { line: 1, column: 0 }, + end: { line: 2, column: 0 }, + }, + ], + }, + ], +}); +function result(): SearchResultData { + return { + display: "rhs", + scope: { + repo: "/unused", + baseWorktree: { commitId: "base", path: "/unused/base" }, + headWorktree: { commitId: "head", path: "/unused/head" }, + }, + file: { rhs: { path: "example.ts", oid: "a".repeat(40), mode: "100644" } }, + sources: { rhs: source("Γ©πŸ”Ž match\nhidden\n") }, + }; +} + +test("head-only evidence preserves its lines, UTF-8 highlights, and nested folded context", () => { + const evidence = result(); + const rows = evidenceRows(evidence); + assert.ok(rows.every((row) => row.baseLine === undefined)); + assert.equal(rows[0].headLine, 1); + assert.deepEqual(rows[0].highlights, [{ startColumn: 2, endColumn: 4 }]); + assert.ok(!rows.some((row) => row.content === "hidden")); + const fold = evidence.sources.rhs!.regions[1]; + fold.visibility = { collapsed: false }; + const expanded = evidenceRows(evidence); + assert.equal(expanded[1].content, "hidden"); + assert.equal(expanded[1].headLine, 2); + assert.equal(expanded[1].fold?.id, 2); +}); + +test("paired changed lines remain separate while unchanged aligned lines merge", () => { + const evidence = result(); + evidence.display = "both"; + evidence.file = { lhs: evidence.file.rhs!, rhs: evidence.file.rhs! }; + evidence.sources = { + lhs: source("Γ©πŸ”Ž match\nhidden\n"), + rhs: evidence.sources.rhs!, + }; + assert.equal(evidenceRows(evidence)[0].baseLine, 1); + assert.equal(evidenceRows(evidence)[0].headLine, 1); + const leaf = evidence.sources.lhs!.regions[0]; + if (leaf.kind !== "leaf") throw new Error("Expected leaf"); + leaf.changed = [{ line: 0, start_column: 0, end_column: 2 }]; + const rows = evidenceRows(evidence); + assert.equal(rows[0].kind, "deleted"); + assert.equal(rows[0].headLine, undefined); + assert.equal(rows[1].baseLine, undefined); +}); + + +test("display hides the counterpart without losing it, and shared sources keep folds", () => { + const single = result(); + const shared: SearchResultData = { + scope: single.scope, display: "rhs", + file: {lhs: single.file.rhs!, rhs: single.file.rhs!}, + sources: {same: single.sources.rhs!}, + }; + assert.ok(evidenceRows(shared).every(row => row.baseLine === undefined)); + assert.equal(evidenceRows(shared)[1].fold?.collapsed, true); + shared.display = "both"; + assert.equal(evidenceRows(shared)[0].baseLine, 1); + assert.equal(evidenceRows(shared)[0].headLine, 1); + assert.equal(evidenceRows(shared)[1].fold?.collapsed, true); + const paired: SearchResultData = {...shared, display: "lhs", sources: { + lhs: source("before\nhidden\n"), rhs: source("after\nhidden\n"), + }}; + assert.ok(evidenceRows(paired).every(row => row.headLine === undefined)); + assert.equal(evidenceRows(paired)[0].content, "before"); + paired.display = "rhs"; + assert.equal(evidenceRows(paired)[0].content, "after"); +}); diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.ts new file mode 100644 index 000000000..8d48b89ea --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewSearchEvidence.ts @@ -0,0 +1,157 @@ +import type { + RegionData, + SearchResultData, + SourceData, + ReviewInlineEditorSpec, +} from "./reviewProtocol.js"; +import type { ReviewUnifiedDiffRow } from "./reviewUnifiedDiff.js"; + +/** Rendering selection never removes sources from retained evidence. */ +export function displayedSources(result: SearchResultData) { + return { + lhs: result.display !== "rhs" ? result.sources.same ?? result.sources.lhs : undefined, + rhs: result.display !== "lhs" ? result.sources.same ?? result.sources.rhs : undefined, + }; +} + +export interface EvidenceRow extends ReviewUnifiedDiffRow { + highlights: { startColumn: number; endColumn: number }[]; + fold?: { id: number; collapsed: boolean }; +} +interface Item { + key: string; + line: number; + text: string; + changed: boolean; + highlights: EvidenceRow["highlights"]; + fold?: EvidenceRow["fold"]; +} + +function items(source: SourceData): Item[] { + const lines = source.text.split("\n").map((line) => line.replace(/\r$/, "")); + const output: Item[] = []; + const column = (text: string, byte: number) => + new TextDecoder().decode(new TextEncoder().encode(text).slice(0, byte)) + .length + 1; + const walk = (regions: RegionData[]) => { + for (const region of regions) { + const first = output.length; + const end = region.end.line + Number(region.end.column > 0); + if (region.visibility?.collapsed) { + output.push({ + key: `fold:${region.fold_state_id}`, + line: region.start.line + 1, + text: `… ${region.visibility.label || `${region.start.line + 1}–${end}`} …`, + changed: false, + highlights: [], + fold: { id: region.fold_state_id, collapsed: true }, + }); + } else if (region.kind === "fold") walk(region.children); + else + for (let line = region.start.line; line < end; line++) { + const text = lines[line] ?? ""; + output.push({ + key: `line:${region.alignment_id}:${line - region.start.line}`, + line: line + 1, + text, + changed: (region.changed ?? []).some((span) => span.line === line), + highlights: (region.search_highlights ?? []) + .filter((span) => span.line === line) + .map((span) => ({ + startColumn: column(text, span.start_column), + endColumn: column(text, span.end_column), + })), + }); + } + if ( + output[first] && + !output[first].fold && + (region.kind === "fold" || region.visibility?.label) + ) + output[first].fold = { id: region.fold_state_id, collapsed: false }; + } + }; + walk(source.regions); + return output; +} + +/** Use supplied structural alignment, visibility and highlights; never compute a new diff. */ +export function evidenceRows(result: SearchResultData): EvidenceRow[] { + const sources = displayedSources(result); + const lhs = sources.lhs ? items(sources.lhs) : []; + const rhs = sources.rhs ? items(sources.rhs) : []; + const rows: EvidenceRow[] = []; + const append = (left?: Item, right?: Item) => { + if ( + left && + right && + (left.changed || right.changed || left.text !== right.text) && + !left.fold?.collapsed && + !right.fold?.collapsed + ) { + append(left); + append(undefined, right); + return; + } + const item = right ?? left!; + rows.push({ + lineNumber: rows.length + 1, + content: item.text, + kind: item.changed ? (right ? "added" : "deleted") : "unchanged", + baseLine: left?.line, + headLine: right?.line, + authorSide: right ? "head" : "base", + authorLine: item.line, + highlights: [...(left?.highlights ?? []), ...(right?.highlights ?? [])], + fold: item.fold ?? left?.fold, + }); + }; + let right = 0; + for (const left of lhs) { + const match = rhs.findIndex( + (item, index) => index >= right && item.key === left.key, + ); + if (match < 0) append(left); + else { + while (right < match) append(undefined, rhs[right++]); + append(left, rhs[right++]); + } + } + while (right < rhs.length) append(undefined, rhs[right++]); + return rows; +} + +export function evidenceCoordinates( + content: ReviewInlineEditorSpec["content"], +) { + if (content.kind === "source") return content; + const result = content.result; + const sources = displayedSources(result); + const side = sources.rhs ? ("head" as const) : ("base" as const); + const file = side === "head" ? result.file.rhs! : result.file.lhs!; + const source = sources.rhs ?? sources.lhs!; + const highlighted = evidenceRows(result) + .filter( + (row) => + row.highlights.length && + (side === "head" + ? row.headLine !== undefined + : row.baseLine !== undefined), + ) + .map((row) => ({ + startLine: (side === "head" ? row.headLine : row.baseLine)!, + endLine: (side === "head" ? row.headLine : row.baseLine)!, + })); + if (highlighted.length) return { path: file.path, side, ranges: highlighted }; + return { + path: file.path, + side, + ranges: source.regions.map((region) => ({ + startLine: region.start.line + 1, + endLine: Math.max( + region.start.line + 1, + region.end.line + Number(region.end.column > 0), + ), + })), + }; +} diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.test.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.test.ts index 016d601c9..5eb5a9f1e 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.test.ts @@ -10,6 +10,8 @@ import { collapsedRegions, hiddenLinesOf, structuralContextGaps, + searchStructuralDiff, + structuralSearchHighlights, bandDetail, structuralCountsTooltip, structuralInitialCounts, @@ -325,3 +327,19 @@ test("a fold pairs through fold state, and a docstring fold only with a docstrin const unpaired: StructuralTextDiff = { ...diff, rhs: text(lines(4), [other]) }; assert.deepEqual(structuralContextGaps(unpaired, (id) => id === 9 || id === 10).map((g) => g.kind).sort(), ["inserted", "removed", "removed"]); }); + + +test("head-only search evidence retains native pseudocode bands and UTF-16 highlight positions", () => { + const body = fold(4, [leaf(5, 1, 3, { changed: [{ line: 1, start_column: 0, end_column: 6 }] })]); + body.visibility = { collapsed: true, label: "// pseudocode\nreturn cached value" }; + const source = text(["πŸ˜€match", "hidden", "return"], [leaf(1, 0, 1, { search_highlights: [{ line: 0, start_column: 4, end_column: 9 }] }), body]); + const diff = searchStructuralDiff({ display: "rhs", scope: { repo: "/unused", baseWorktree: { commitId: "base", path: "/unused" }, headWorktree: { commitId: "head", path: "/unused" } }, file: { rhs: { path: "x.ts", oid: "a".repeat(40), mode: "100644" } }, sources: { rhs: source } }); + assert.ok(structuralRows(diff).every(([left]) => left === null)); + assert.deepEqual(structuralSearchHighlights(diff.rhs), [{ startLineNumber: 1, endLineNumber: 1, startColumn: 3, endColumn: 8 }]); + const [band] = structuralContextGaps(diff, id => id === 4); + assert.equal(band.originalCount, 0); + assert.equal(band.modifiedCount, 2); + assert.equal(bandDetail(band.label), "return cached value"); + assert.equal(structuralInitialCounts(diff).visible.added, 0); + assert.equal(structuralInitialCounts(diff).textual.added, 1); +}); diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.ts index ce92a6445..c177b6f2c 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.ts @@ -1,3 +1,5 @@ +import { displayedSources } from "./reviewSearchEvidence.js"; +import type { RegionData } from "./reviewProtocol.js"; /*--------------------------------------------------------------------------------------------- * Copyright (c) dev.fast. All rights reserved. * Licensed under the MIT License. See LICENSE in the repository root for license information. @@ -41,24 +43,9 @@ export interface StructuralVisibility { * side sharing it. Leaves tile the file in order; a fold's range is the hull * of its children. Tags are `:`. */ -export type StructuralRegion = StructuralLeaf | StructuralFold; -interface StructuralRegionBase { - id: number; - fold_state_id: number; - start: StructuralPos; - end: StructuralPos; - tags?: string[]; - visibility?: StructuralVisibility; -} -export interface StructuralLeaf extends StructuralRegionBase { - kind: "leaf"; - alignment_id: number; - changed?: StructuralSpan[]; -} -export interface StructuralFold extends StructuralRegionBase { - kind: "fold"; - children: StructuralRegion[]; -} +export type StructuralRegion = RegionData; +export type StructuralLeaf = Extract; +export type StructuralFold = Extract; export interface StructuralSyntaxSpan extends StructuralSpan { capture: string; } @@ -397,3 +384,31 @@ export function structuralCountsTooltip(counts: StructuralFileCounts): string { if (counts.fallback) rows.push(`line diff: ${counts.fallback.code}`); return rows.join("\n"); } + +/** Retained search trees enter the same fold/alignment renderer as streamed diffs. */ +export function searchStructuralDiff(result: import("./reviewProtocol.js").SearchResultData): StructuralTextDiff { + const count = (source: StructuralSource | undefined, visible: boolean) => { + const lines = new Set(); + const visit = (region: StructuralRegion) => { + if (visible && region.visibility?.collapsed) return; + if (region.kind === "fold") region.children.forEach(visit); + else for (const span of region.changed ?? []) lines.add(span.line); + }; + source?.regions?.forEach(visit); + return lines.size; + }; + const sources = displayedSources(result); + return { type: "text", ...sources, stats: { + textual: { added: count(sources.rhs, false), removed: count(sources.lhs, false) }, + visible: { added: count(sources.rhs, true), removed: count(sources.lhs, true) }, + } }; +} + +export function structuralSearchHighlights(source: StructuralSource | undefined) { + const lines = source?.text.replace(/\r\n/g, "\n").split("\n") ?? []; + return structuralLeaves(source?.regions).flatMap(leaf => (leaf.search_highlights ?? []).map(span => ({ + startLineNumber: span.line + 1, endLineNumber: span.line + 1, + startColumn: utf16Column(lines[span.line], span.start_column), + endColumn: utf16Column(lines[span.line], span.end_column), + }))); +} diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.test.ts index d6e038200..0904ad121 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.test.ts @@ -87,12 +87,13 @@ test("a native peek reads the pinned version through the authenticated API, not } as never, {} as never, ); - canvas.inlineEditors.create({ + canvas.inlineEditors.create({content: {kind: "source", path: "src/[route].ts", side: "base", ranges: [{ startLine: 2, endLine: 2 }], - } as never); + }} as never); version = 4; + assert.equal(await source.diff(), undefined); const snippet = await source.snippet(); assert.equal(models.get(snippet.target.resource.toString())?.text, "old first line\nold second line"); snippet.dispose(); @@ -171,11 +172,11 @@ test("unavailable pinned files report the API error instead of falling back to d } as never, {} as never, ); - canvas.inlineEditors.create({ + canvas.inlineEditors.create({content: {kind: "source", path: "missing.ts", side: "head", ranges: [{ startLine: 1, endLine: 1 }], - } as never); + }} as never); await assert.rejects(source.snippet(), /unavailable at the pinned commit/); assert.equal(disposed(), 0); }); 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 e0056509b..56322989b 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 @@ -1,3 +1,4 @@ +import { displayedSources, evidenceCoordinates } from "../common/reviewSearchEvidence.js"; import { lensFiles } from "../common/reviewLensFiles.js"; /*--------------------------------------------------------------------------------------------- * Copyright (c) dev.fast. All rights reserved. @@ -22,8 +23,8 @@ import { reviewPeekLineMappings, } from "../common/reviewPeek.js"; import type { - ReviewDiffSide, ReviewInlineEditorRange, + ReviewInlineEditorSpec, ReviewDiffFileWire, ReviewInlineEditorFactory, ReviewDiffViewFactory, @@ -444,15 +445,22 @@ export class ReviewApiSourceService } return list; }; - const source = ( - file: string, - side: ReviewDiffSide, - ranges: readonly ReviewInlineEditorRange[], - ): ReviewInlineSource => { + const source = (content: ReviewInlineEditorSpec["content"]): ReviewInlineSource => { + const {path: file, side, ranges} = evidenceCoordinates(content); const target = { view: view(), file, side }; return { snippet: () => this.snippet(target, ranges), - diff: () => this.peekDiff(target, ranges, files(target.view)), + diff: async () => { + if (content.kind === "source") return content.ranges.some(range => range.side && range.side !== content.side) ? this.peekDiff(target, ranges, files(target.view)) : undefined; + const result = content.result; + const path = (result.file.rhs ?? result.file.lhs!).path; + return { + original: apiSourceUri({...target, side: "base", file: result.file.lhs?.path ?? path}), + modified: apiSourceUri({...target, side: "head", file: result.file.rhs?.path ?? path}), + diffFile: {path, previousPath: result.file.lhs?.path, status: result.sources.same ? "unchanged" : result.file.lhs ? result.file.rhs ? "modified" : "deleted" : "added", additions: 0, deletions: 0}, + mappings: [], windows: () => ({original: [], modified: []}), + }; + }, }; }; const diffSource: ReviewDiffViewSource = { @@ -480,7 +488,7 @@ export class ReviewApiSourceService ? `commit=${encodeURIComponent(current.commit)}` : undefined, }), - entries: await Promise.all( + entries: [...await Promise.all( entries.map(async (file) => { const original = file.status === "added" @@ -505,16 +513,26 @@ export class ReviewApiSourceService goToFileResource: (modified ?? original)!, }; }), - ), + ), ...(lens && !lens.wholeFiles ? lens.targets.flatMap(target => target.kind === "results" ? target.results : []) : []).map((result, index) => { + const path = (result.file.rhs ?? result.file.lhs!).path; + const resource = (side: "base" | "head", file: string) => apiSourceUri({ view: current, side, file }).with({ fragment: `evidence-${index}` }); + const original = displayedSources(result).lhs && resource("base", result.file.lhs!.path); + const modified = displayedSources(result).rhs && resource("head", result.file.rhs!.path); + return { + evidence: result, + file: { path, previousPath: result.file.lhs?.path, status: entries.find(file => file.path === path || file.previousPath === path)?.status ?? (result.sources.same ? "unchanged" as const : result.file.lhs ? result.file.rhs ? "modified" as const : "deleted" as const : "added" as const), additions: 0, deletions: 0 }, + original, modified, goToFileResource: (modified ?? original)!, + }; + })], }; }, }; return { inlineEditors: { create: (spec) => - inline.create(spec, source(spec.path, spec.side, spec.ranges)), + inline.create(spec, source(spec.content)), find: (spec, query) => - inline.find(spec, query, source(spec.path, spec.side, spec.ranges)), + inline.find(spec, query, source(spec.content)), } satisfies ReviewInlineEditorFactory, diffView: { create: (spec) => diff.create(spec, diffSource), diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewCodeResourceService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCodeResourceService.ts index 189dd4586..c606f46f8 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewCodeResourceService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewCodeResourceService.ts @@ -1,3 +1,5 @@ +import { displayedSources, evidenceRows } from "../common/reviewSearchEvidence.js"; +import type { SearchResultData } from "../common/reviewProtocol.js"; /*--------------------------------------------------------------------------------------------- * Copyright (c) dev.fast. All rights reserved. * Licensed under the MIT License. See LICENSE in the repository root for license information. @@ -88,6 +90,7 @@ export const IReviewCodeResourceService = createDecorator; + unifiedResource(resource: URI): ReviewUnifiedResourceInfo | undefined; reset(): void; } @@ -190,6 +194,27 @@ export class ReviewCodeResourceService extends Disposable implements IReviewCode }; } + acquireEvidence(result: SearchResultData, target: ReviewCodeDiffTarget): ReviewUnifiedCodeModelReference { + const rows = evidenceRows(result); + const path = (result.file.rhs ?? result.file.lhs!).path; + const resource = URI.from({scheme: REVIEW_UNIFIED_SCHEME, path: `/${path}`, query: `evidence=${Date.now()}-${Math.random()}`}); + const model = this.modelService.createModel(rows.map(row => row.content).join('\n'), this.languageService.createByFilepathOrFirstLine(URI.file(path)), resource); + const entry: ReviewUnifiedResourceEntry = { + model, rows, originalLineCount: displayedSources(result).lhs?.text.split('\n').length ?? 0, + modifiedLineCount: displayedSources(result).rhs?.text.split('\n').length ?? 0, references: 1, + info: { original: target.original, modified: target.modified, path, diffFile: target.diffFile, rows, + targetForRange: (startLine, endLine) => { + if (rows.slice(startLine - 1, endLine).some(row => row.fold?.collapsed)) return null; + const selected = reviewUnifiedTargetForRange(path, rows, startLine, endLine); + return selected ? {...selected, path: result.file[selected.side === "base" ? "lhs" : "rhs"]!.path} : null; + } }, + dispose: () => model.dispose(), + }; + this.unifiedResources.set(resource.toString(), entry); + const windows = [{startLine: 1, endLine: rows.length, lineCount: rows.length, visibleLineCount: rows.length, height: rows.length * 20}]; + return {model, target, rows, windows, ranges: [], dispose: () => {this.unifiedResources.delete(resource.toString()); model.dispose();}}; + } + unifiedResource(resource: URI): ReviewUnifiedResourceInfo | undefined { return this.unifiedResources.get(resource.toString())?.info; } diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDiffViewService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDiffViewService.ts index 6cb3c3321..c4d2944dd 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDiffViewService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDiffViewService.ts @@ -1,5 +1,5 @@ import type { ReviewDiffProgress } from "../common/reviewProtocol.js"; -import { lensRanges, withLens } from "./reviewLens.js"; +import { selectLensEntries, withLens } from "./reviewLens.js"; /*--------------------------------------------------------------------------------------------- * Copyright (c) dev.fast. All rights reserved. * Licensed under the MIT License. See LICENSE in the repository root for license information. @@ -97,9 +97,9 @@ class DiffViewHandle extends Disposable implements ReviewDiffViewHandle { private progress: ReviewDiffProgress | undefined; private readonly progressChanged = this._register(new Emitter()); private pendingSectionId: string | undefined; - private pendingSource: ReviewDiffLens['ranges'][number] | undefined; + private pendingSource: Extract['ranges'][number] | undefined; setProgress(progress: ReviewDiffProgress): void { this.progress = progress; this.view?.setProgress(progress); this.progressChanged.fire(); } - revealSource(source: ReviewDiffLens['ranges'][number], sectionId?: string): void { this.pendingSource = source; this.pendingSectionId = sectionId; this.view?.revealSource(source, sectionId); } + revealSource(source: Extract['ranges'][number], sectionId?: string): void { this.pendingSource = source; this.pendingSectionId = sectionId; this.view?.revealSource(source, sectionId); } private viewStateKey: string | undefined; private adoptedEditors: readonly ICodeEditor[] = []; private disposed = false; @@ -142,7 +142,8 @@ class DiffViewHandle extends Disposable implements ReviewDiffViewHandle { try { const data = await this.source.load(this.spec.scope, this.spec.lens); const { sourceUri, entries } = data; - const structuralEnabled = this.instantiationService.invokeFunction( + const selected = selectLensEntries(entries, this.spec.lens, this.progress?.sections); + const structuralEnabled = selected.some(entry => entry.evidence) || this.instantiationService.invokeFunction( (a) => a .get(IConfigurationService) @@ -155,29 +156,24 @@ class DiffViewHandle extends Disposable implements ReviewDiffViewHandle { const structural = structuralEnabled ? await prepareStructuralReview( this.instantiationService, - entries, + selected, store, data.structuralDiff!, ) : { instantiation: this.instantiationService, - entries, + entries: selected, enabled: false, load: undefined, onDidChangeCounts: undefined, }; if (this.disposed) return; const lens = this.spec.lens; - const sections = this.progress?.sections; - const selected = lens && sections?.length ? sections.flatMap(section => { - const matches = structural.entries.filter(entry => lensRanges({ ...lens, ranges: section.sources }, entry).length > 0); - return matches.map((entry, index) => ({ ...entry, sectionId: section.id, sectionStart: index === 0, original: entry.original?.with({ fragment: section.id }), modified: entry.modified?.with({ fragment: section.id }) })); - }) : lens ? structural.entries.filter(entry => lensRanges(lens, entry).length > 0) : structural.entries; - const selectedPaths = new Set(selected.map(entry => entry.file.path)); - const instantiation = withLens(structural.instantiation, selected, lens, store, () => this.progress, this.progressChanged.event); + const selectedPaths = new Set(structural.entries.map(entry => entry.file.path)); + const instantiation = withLens(structural.instantiation, structural.entries, lens, store, () => this.progress, this.progressChanged.event); // The input owns the text-model references its view model resolves, so // this handle disposes it alongside the view. - const input = store.add(instantiation.createInstance(ReviewFilesEditorInput, sourceUri, selected, + const input = store.add(instantiation.createInstance(ReviewFilesEditorInput, sourceUri, structural.entries, structural.enabled, !!lens || !!this.spec.onToggleViewed)); const view = store.add( instantiation.createInstance( @@ -192,7 +188,7 @@ class DiffViewHandle extends Disposable implements ReviewDiffViewHandle { ); this.view = view; if (this.progress) view.setProgress(this.progress); - if (structural.enabled) view.startLoading(selected); + if (structural.enabled) view.startLoading(structural.entries); store.add(view.onDidChangeActiveControl(() => this.bindActiveControl(view))); // A saved whole-list offset cannot be restored into a partial streamed list. await view.setInput(input, structural.enabled ? undefined : this.viewStates.get(this.viewStateKey), diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index 9d44aac75..1e180a99c 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts @@ -51,6 +51,7 @@ const REVIEW_FILES_DIFF_EDITOR_OPTIONS = { } satisfies IDiffEditorOptions; export interface ReviewFilesEditorEntry { + readonly evidence?: import("../common/reviewProtocol.js").SearchResultData; readonly sectionId?: string; readonly sectionStart?: boolean; readonly file: ReviewDiffFileWire; @@ -96,7 +97,8 @@ export class ReviewFilesEditorInput extends MultiDiffEditorInput { (structural || lens) ? { ...REVIEW_FILES_DIFF_EDITOR_OPTIONS, - hideOriginalLineNumbers: entry.file.status === "added", + hideOriginalLineNumbers: entry.original?.scheme === "review-structural-empty" || entry.file.status === "added", + ...((entry.evidence || lens) && (entry.original?.scheme === "review-structural-empty" || entry.modified?.scheme === "review-structural-empty") ? { forceInline: true } : {}), hideUnchangedRegions: { enabled: true, minimumLineCount: 1, @@ -184,7 +186,7 @@ export class ReviewFilesDiffView extends Disposable { private readonly hiddenApplied = new Set(); private pendingPath: string | undefined; private pendingSectionId: string | undefined; - private pendingSource: ReviewDiffLens["ranges"][number] | undefined; + private pendingSource: Extract["ranges"][number] | undefined; private progress: ReviewDiffProgress | undefined; private readonly viewedApplied = new Map(); private readonly streamStatus: HTMLElement; @@ -540,8 +542,8 @@ export class ReviewFilesDiffView extends Disposable { this.viewedApplied.set(key, file.state); } } - revealSource(source: ReviewDiffLens['ranges'][number], sectionId?: string): void { - const entry = this.input?.entries.find(entry => (!sectionId || entry.sectionId === sectionId) && (!entry.sectionId || this.progress?.sections?.find(section => section.id === entry.sectionId)?.sources.some(range => range.file === source.file && range.side === source.side && range.fromLine <= source.fromLine && range.toLine >= source.fromLine)) && source.file === (source.side === 'base' ? entry.file.previousPath ?? entry.file.path : entry.file.path)); + revealSource(source: Extract['ranges'][number], sectionId?: string): void { + const entry = this.input?.entries.find(entry => (!sectionId || entry.sectionId === sectionId) && (source.side === "base" ? entry.original?.scheme !== "review-structural-empty" : entry.modified?.scheme !== "review-structural-empty") && (!entry.sectionId || this.progress?.sections?.find(section => section.id === entry.sectionId)?.sources.some(range => range.file === source.file && range.side === source.side && range.fromLine <= source.fromLine && range.toLine >= source.fromLine)) && source.file === (source.side === 'base' ? entry.file.previousPath ?? entry.file.path : entry.file.path)); if (!entry) return; if (entry.sectionId && this.collapsedSections.delete(entry.sectionId)) this.headerFactory.refreshHeaders(); this.pendingSource = source; this.pendingSectionId = sectionId; diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewInlineEditorService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewInlineEditorService.ts index d37ceeb25..6be403e52 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewInlineEditorService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewInlineEditorService.ts @@ -1,3 +1,4 @@ +import { displayedSources, evidenceCoordinates, type EvidenceRow } from "../common/reviewSearchEvidence.js"; /*--------------------------------------------------------------------------------------------- * Copyright (c) dev.fast. All rights reserved. * Licensed under the MIT License. See LICENSE in the repository root for license information. @@ -8,7 +9,7 @@ import { Disposable, DisposableStore, type IDisposable } from "../../base/common import { autorun, observableValue } from "../../base/common/observable.js"; import { URI } from "../../base/common/uri.js"; import type { IEditorConstructionOptions } from "../../editor/browser/config/editorConfiguration.js"; -import type { ICodeEditor } from "../../editor/browser/editorBrowser.js"; +import { MouseTargetType, type ICodeEditor } from "../../editor/browser/editorBrowser.js"; import { EditorExtensionsRegistry } from "../../editor/browser/editorExtensions.js"; import { ICodeEditorService } from "../../editor/browser/services/codeEditorService.js"; import { CodeEditorWidget } from "../../editor/browser/widget/codeEditor/codeEditorWidget.js"; @@ -78,11 +79,13 @@ export interface ReviewInlineSource { /** Pinned API sources feed the unified diff builder. */ async function acquireUnifiedFromSource( resources: IReviewCodeResourceService, - spec: Pick, + spec: ReviewInlineFindSpec, source: ReviewInlineSource, ) { const target = await source.diff(); - return target && resources.acquireUnifiedDiffForTarget(spec.path, spec.side, spec.ranges, target); + if (!target) return undefined; + if (spec.content.kind === "diffr") return resources.acquireEvidence(spec.content.result, target); + return resources.acquireUnifiedDiffForTarget(spec.content.path, spec.content.side, spec.content.ranges, target); } export class ReviewInlineEditorService extends Disposable implements ICompositeCodeEditor { @@ -362,6 +365,8 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle return this._height; } + private get coordinates() { return evidenceCoordinates(this.spec.content); } + get hasWidget(): boolean { return this.editor !== undefined; } @@ -383,15 +388,15 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle private readonly source: ReviewInlineSource, ) { super(); - if (spec.ranges.length === 0) { + if (evidenceCoordinates(spec.content).ranges.length === 0) { throw new Error("Inline editor requires at least one range."); } this.active = spec.active; - this.expandedHeight = estimatedHeight(spec.ranges, spec.heightMode); + this.expandedHeight = estimatedHeight(evidenceCoordinates(spec.content).ranges, spec.heightMode); this._height = this.expandedHeight; spec.container.classList.add("review-inline-code-editor"); - spec.container.dataset["reviewInlineEditorPath"] = spec.path; - spec.container.dataset["reviewInlineEditorSide"] = spec.side; + spec.container.dataset["reviewInlineEditorPath"] = evidenceCoordinates(spec.content).path; + spec.container.dataset["reviewInlineEditorSide"] = evidenceCoordinates(spec.content).side; const document = spec.container.ownerDocument; const headerHost = document.createElement("div"); headerHost.className = "review-inline-editor-header-host monaco-component multiDiffEditor"; @@ -419,8 +424,8 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle this.body.className = "review-inline-editor-body"; spec.container.append(headerHost, this.body); this.setHeader( - URI.from({ scheme: "file", path: `/${this.spec.path}` }), - URI.from({ scheme: "file", path: `/${this.spec.path}` }), + URI.from({ scheme: "file", path: `/${this.coordinates.path}` }), + URI.from({ scheme: "file", path: `/${this.coordinates.path}` }), ); this._register( autorun((reader) => { @@ -556,14 +561,14 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle private initializeUnifiedEditor(reference: ReviewUnifiedCodeModelReference): void { const labelUris = reviewMultiDiffLabelUris(reference.target.diffFile); this.setHeader( - reference.target.original, - reference.target.modified, + this.spec.content.kind === "diffr" && !displayedSources(this.spec.content.result).lhs ? undefined : reference.target.original, + this.spec.content.kind === "diffr" && !displayedSources(this.spec.content.result).rhs ? undefined : reference.target.modified, labelUris.original, labelUris.modified, reviewCodePeekRangeCounts( reference.target.diffFile.patch, - this.spec.countRanges ?? this.spec.ranges, - this.spec.side, + this.spec.countRanges ?? this.coordinates.ranges, + this.coordinates.side, ), ); this.unifiedModelReference = reference; @@ -587,7 +592,36 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle this.editorStore.add(editor); editor.setModel(reference.model); this.diffDecoration = editor.createDecorationsCollection(); - this.diffDecoration.set(reviewUnifiedDiffDecorations(reference.rows)); + this.diffDecoration.set([ + ...reviewUnifiedDiffDecorations(reference.rows), + ...(this.spec.content.kind === "diffr" ? (reference.rows as EvidenceRow[]).flatMap(row => [ + ...row.highlights.map(span => ({range: new Range(row.lineNumber, span.startColumn, row.lineNumber, Math.max(span.startColumn + 1, span.endColumn)), options: { description: 'Review search highlight', inlineClassName: "review-search-highlight" }})), + ...(row.fold ? [{range: new Range(row.lineNumber, 1, row.lineNumber, 1), options: {description: "Review evidence fold", glyphMarginClassName: row.fold.collapsed ? "codicon codicon-chevron-right" : "codicon codicon-chevron-down", glyphMarginHoverMessage: {value: row.fold.collapsed ? "Expand region" : "Collapse region"}}}] : []), + ]) : []), + ]); + if (this.spec.content.kind === "diffr") { + editor.updateOptions({glyphMargin: true}); + this.editorStore.add(editor.onMouseDown(event => { + const line = event.target.position?.lineNumber; + const row = line ? (reference.rows as EvidenceRow[])[line - 1] : undefined; + if (!row?.fold || event.target.type !== MouseTargetType.GUTTER_GLYPH_MARGIN || this.spec.content.kind !== "diffr") return; + const result = structuredClone(this.spec.content.result); + const toggle = (regions: import("../common/reviewProtocol.js").RegionData[]) => {for (const region of regions) { + if (region.fold_state_id === row.fold!.id) region.visibility = {...region.visibility, collapsed: !row.fold!.collapsed}; + if (region.kind === "fold") toggle(region.children); + }}; + for (const source of Object.values(result.sources)) toggle(source.regions); + this.spec.content = {kind: "diffr", result}; + this.clearFind(); + this.editorStore.clear(); + this.decoration = undefined; + this.diffDecoration = undefined; + this.unifiedModelReference = undefined; + this.editor = undefined; + this.body.replaceChildren(); + void this.initialize(); + })); + } this.bindFocus(editor); this.trackScroll( () => editor.getScrollTop(), @@ -768,16 +802,18 @@ class InlineEditorHandle extends Disposable implements ReviewInlineEditorHandle } private ranges(): Range[] { + if (this.spec.content.kind === "diffr") return (this.unifiedModelReference?.rows as EvidenceRow[] ?? []).flatMap(row => row.highlights.map(span => new Range(row.lineNumber, span.startColumn, row.lineNumber, Math.max(span.startColumn + 1, span.endColumn)))); + if (this.unifiedModelReference) { return this.unifiedModelReference.ranges.map( (range) => new Range(range.startLine, 1, range.endLine, Number.MAX_SAFE_INTEGER), ); } - return this.spec.ranges.map((range) => new Range(range.startLine, 1, range.endLine, Number.MAX_SAFE_INTEGER)); + return this.coordinates.ranges.map((range) => new Range(range.startLine, 1, range.endLine, Number.MAX_SAFE_INTEGER)); } private primaryRange(): Range { - return this.ranges()[0]!; + return this.ranges()[0] ?? new Range(1, 1, 1, 1); } private setHeight(height: number): void { diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.test.ts new file mode 100644 index 000000000..daeacdaa0 --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.test.ts @@ -0,0 +1,28 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { URI } from '../../base/common/uri.js'; +import { selectLensEntries } from './reviewLens.js'; +import type { ReviewDiffLens, ReviewDiffProgress, SearchResultData } from '../common/reviewProtocol.js'; +import type { ReviewFilesEditorEntry } from './reviewFilesDiffView.js'; + +test('sections select their retained result after JSON transport without sharing editor identity', () => { + const result: SearchResultData = { + display: 'rhs', scope: { repo: '/repo', baseWorktree: { path: '/base', commitId: 'base' }, headWorktree: { path: '/head', commitId: 'head' } }, + file: { rhs: { path: 'same.ts', oid: 'a'.repeat(40), mode: '100644' } }, + sources: { rhs: { text: 'selected', regions: [] } }, + }; + const original = URI.parse('review-api-source://review/same.ts?side=base'); + const modified = URI.parse('review-api-source://review/same.ts?side=head'); + const entries: ReviewFilesEditorEntry[] = [{ file: { path: 'same.ts', status: 'modified', additions: 1, deletions: 1 }, original: undefined, modified, goToFileResource: modified, evidence: result }]; + const lens: ReviewDiffLens = { id: 'lens', reviewId: 'review', title: 'Results', version: 1, targets: [{ kind: 'results', results: [result] }] }; + const sections: ReviewDiffProgress['sections'] = ['first', 'second'].map(id => ({ id, label: id, sources: [], state: 'unread', total: { additions: 1, deletions: 0 }, remaining: { additions: 1, deletions: 0 }, targets: JSON.parse(JSON.stringify(lens.targets)) })); + const selected = selectLensEntries(entries, lens, sections); + assert.equal(selected.length, 2); + assert.ok(selected.every(entry => entry.original === undefined)); + assert.notEqual(selected[0].modified!.toString(), selected[1].modified!.toString()); + assert.deepEqual(selected.map(entry => entry.sectionId), ['first', 'second']); + + const ranged = selectLensEntries([{ ...entries[0], evidence: undefined, original }], { ...lens, targets: [{ kind: 'ranges', ranges: [{ file: 'same.ts', side: 'base', fromLine: 1, toLine: 1 }] }] }, undefined); + assert.equal(ranged[0].modified, undefined); + assert.equal(ranged[0].goToFileResource.toString(), original.toString()); +}); 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 681c211e7..fadd05a55 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 @@ -11,8 +11,31 @@ import { lensContextGaps, viewedContextGaps } from '../common/reviewLens.js'; import type { ReviewDiffLens, ReviewDiffProgress } from '../common/reviewProtocol.js'; import type { ReviewFilesEditorEntry } from './reviewFilesDiffView.js'; -export function lensRanges(lens: ReviewDiffLens, entry: ReviewFilesEditorEntry): ReviewDiffLens['ranges'] { - return lens.ranges.filter(range => range.file === (range.side === 'base' ? entry.file.previousPath ?? entry.file.path : entry.file.path)); +export function lensRanges(lens: ReviewDiffLens, entry: ReviewFilesEditorEntry): Extract['ranges'] { + return lens.targets.flatMap(target => target.kind === "ranges" ? [...target.ranges] : []).filter(range => range.file === (range.side === 'base' ? entry.file.previousPath ?? entry.file.path : entry.file.path)); +} + +/** Preserve each result's identity even when several sections show the same file. */ +export function selectLensEntries(entries: readonly ReviewFilesEditorEntry[], lens: ReviewDiffLens | undefined, sections: ReviewDiffProgress["sections"]): readonly ReviewFilesEditorEntry[] { + if (!lens) return entries; + const select = (targets: ReviewDiffLens["targets"]) => { + const results = new Set(targets.flatMap(target => target.kind === "results" ? target.results.map(result => JSON.stringify(result)) : [])); + return entries.flatMap(entry => { + if (entry.evidence) return results.has(JSON.stringify(entry.evidence)) ? [entry] : []; + const ranges = lensRanges({ ...lens, targets }, entry); + if (!ranges.length) return []; + if (lens.wholeFiles) return [entry]; + const original = ranges.some(range => range.side === "base") ? entry.original : undefined; + const modified = ranges.some(range => range.side === "head") ? entry.modified : undefined; + return [{ ...entry, original, modified, goToFileResource: (modified ?? original)! }]; + }); + }; + if (!sections?.length) return select(lens.targets); + return sections.flatMap(section => select(section.targets).map((entry, index) => ({ + ...entry, sectionId: section.id, sectionStart: index === 0, + original: entry.original?.with({ fragment: `${entry.original.fragment}/${section.id}` }), + modified: entry.modified?.with({ fragment: `${entry.modified.fragment}/${section.id}` }), + }))); } export function withLens(instantiation: IInstantiationService, entries: readonly ReviewFilesEditorEntry[], lens: ReviewDiffLens | undefined, lifetime: DisposableStore, progress: () => ReviewDiffProgress | undefined, onProgress: Event): IInstantiationService { @@ -27,7 +50,7 @@ export function withLens(instantiation: IInstantiationService, entries: readonly async computeDiff(original, modified, options, token) { let diff = await provider.computeDiff(original, modified, options, token); const entry = entries.find(entry => entry.original?.toString() === original.uri.toString() || entry.modified?.toString() === modified.uri.toString()); - if (!entry) return diff; + if (!entry || entry.evidence) return diff; const file = progress()?.files.find(file => file.path === entry.file.path); if (file && (!lens || lens.wholeFiles) && !diff.contextGaps) diff = { ...diff, contextGaps: lensContextGaps(diff, original.getLineCount(), modified.getLineCount(), file.changedRanges).map(gap => ({ ...gap, label: 'Unchanged' })) }; if (file) diff = { ...diff, contextGaps: viewedContextGaps(diff, original.getLineCount(), modified.getLineCount(), file.viewedRanges, file.changedRanges) }; @@ -37,7 +60,7 @@ export function withLens(instantiation: IInstantiationService, entries: readonly return count > 0 && range.fromLine < start + count && range.toLine >= start; }) ? { ...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; + return lens && !lens.wholeFiles ? { ...diff, contextGaps: lensContextGaps(diff, original.getLineCount(), modified.getLineCount(), lensRanges(section ? { ...lens, targets: [{kind: "ranges", ranges: section.sources}] } : lens, entry)) } : diff; }, }; }, diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewStructuralDiff.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewStructuralDiff.ts index b47894033..f1a63623c 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewStructuralDiff.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewStructuralDiff.ts @@ -20,6 +20,8 @@ import { DetailedLineRangeMapping } from "../../editor/common/diff/rangeMapping. import { autorun, type IObservable } from "../../base/common/observable.js"; import type { UnchangedRegion } from "../../editor/browser/widget/diffEditor/diffEditorViewModel.js"; import { + searchStructuralDiff, + structuralSearchHighlights, structuralContextGaps, structuralFilePath, structuralInitialCounts, @@ -59,16 +61,17 @@ export async function prepareStructuralReview( /** Fires when a file's visible counts change: on arrival and on every fold toggle. */ onDidChangeCounts: Event<{ path: string; counts: StructuralFileCounts }>; }> { - const unchanged = new Set(entries.filter(entry => entry.file.status === "unchanged").map(entry => entry.file.path)); + const unchanged = new Set(entries.filter(entry => !entry.evidence && entry.file.status === "unchanged").map(entry => entry.file.path)); const files = new Map(); const binary = new Set(); - /** Collapse state by `${path}:${fold_state_id}`, seeded from the wire's initial visibility. A fold-state id spans sides. */ + /** Collapse state by editor pair and fold_state_id, seeded from the supplied visibility. A fold-state id spans sides. */ const collapsed = new Map(); const collapseKey = (path: string, foldStateId: number) => `${path}:${foldStateId}`; const countsChanged = lifetime.add(new Emitter<{ path: string; counts: StructuralFileCounts }>()); // Counts are the wire's, read once when a file arrives; folding never changes them. const emitCounts = (path: string) => { - const diff = files.get(path); + const entry = resolvedEntries.find(entry => !entry.evidence && entry.file.path === path); + const diff = entry && files.get(keyOf(entry)); if (!diff) return; countsChanged.fire({ path, counts: structuralInitialCounts(diff) }); }; @@ -85,13 +88,18 @@ export async function prepareStructuralReview( original: entry.original ?? URI.from({ scheme: "review-structural-empty", path: "/base/" + entry.file.path, query: entry.modified!.toString() }), modified: entry.modified ?? URI.from({ scheme: "review-structural-empty", path: "/head/" + entry.file.path, query: entry.original!.toString() }), })); - const pairs = new Map(resolvedEntries.map(e => [e.original!.toString() + "\n" + e.modified!.toString(), e.file.path])); - async function accept(event: Extract): Promise<{ path: string; stats?: StructuralLineCounts }> { + const keyOf = (entry: ReviewFilesEditorEntry) => entry.original!.toString() + "\n" + entry.modified!.toString(); + async function accept(event: Extract, supplied?: (typeof resolvedEntries)[number]): Promise<{ path: string; stats?: StructuralLineCounts }> { const path = structuralFilePath(event.file); - const entry = resolvedEntries.find(e => e.file.path === path); + const entry = supplied ?? resolvedEntries.find(e => !e.evidence && e.file.path === path); if (!entry) throw new Error(`diffr returned an unexpected file: ${path}`); if (event.error) throw new Error(event.error.message); - const diff = event.diff!; + const key = keyOf(entry); + const payload = event.diff!; + const diff = payload.type === "text" ? { ...payload, + lhs: entry.original.scheme === "review-structural-empty" ? undefined : payload.lhs, + rhs: entry.modified.scheme === "review-structural-empty" ? undefined : payload.rhs, + } : payload; if (diff.type === "text") { const sides: [typeof entry.original, StructuralSource | undefined][] = [ [entry.original, diff.lhs], @@ -109,24 +117,46 @@ export async function prepareStructuralReview( if (lifetime.isDisposed) throw new CancellationError(); if (diff.type === "text") { structuralInitialCounts(diff); - files.set(path, diff); + files.set(key, diff); for (const source of [diff.lhs, diff.rhs]) { const seed = (region: StructuralRegion) => { - const key = collapseKey(path, region.fold_state_id); - if (!collapsed.has(key)) collapsed.set(key, region.visibility?.collapsed === true); + const foldKey = collapseKey(key, region.fold_state_id); + if (!collapsed.has(foldKey)) collapsed.set(foldKey, region.visibility?.collapsed === true); if (region.kind === "fold") region.children.forEach(seed); }; (source?.regions ?? []).forEach(seed); } - } else binary.add(path); - emitCounts(path); + } else binary.add(key); + if (!entry.evidence) emitCounts(path); return { path, stats: diff.type === "text" ? diff.stats.textual : undefined }; } async function load( onFile: (path: string, outcome: StructuralFileOutcome) => void, ): Promise { + const retained = new Map(); + for (const entry of resolvedEntries.filter(entry => entry.evidence)) { + try { + const accepted = await accept({ type: "file", file: entry.evidence!.file, diff: searchStructuralDiff(entry.evidence!) }, entry); + if (!retained.has(accepted.path)) retained.set(accepted.path, {}); + } catch (error) { + retained.set(entry.file.path, { error: error instanceof Error ? error.message : String(error) }); + } + } + for (const [path, outcome] of retained) if (!entries.some(entry => !entry.evidence && entry.file.path === path && !unchanged.has(path))) onFile(path, outcome); + for (const entry of resolvedEntries.filter(entry => !entry.evidence && unchanged.has(entry.file.path))) { + const read = async (uri: URI): Promise => { + if (uri.scheme === "review-structural-empty") return undefined; + const reference = lifetime.add(await resolver.createModelReference(uri)); + const text = reference.object.textEditorModel.getValue(); + const lines = text.split("\n"); + return { text, regions: [{ kind: "leaf", id: 0, fold_state_id: 0, alignment_id: 0, start: { line: 0, column: 0 }, end: { line: lines.length - 1, column: new TextEncoder().encode(lines.at(-1)!).length } }] }; + }; + const lhs = await read(entry.original), rhs = await read(entry.modified); + if (lhs && rhs && lhs.text !== rhs.text) throw new Error("Referenced context file changed; reload the review."); + files.set(keyOf(entry), { type: "text", lhs, rhs, stats: { textual: { added: 0, removed: 0 }, visible: { added: 0, removed: 0 } } }); + } for (const path of unchanged) onFile(path, {}); - if (unchanged.size === entries.length) return; + if (entries.every(entry => entry.evidence || unchanged.has(entry.file.path))) return; const abort = new AbortController(); lifetime.add(toDisposable(() => abort.abort())); const response = await request(abort.signal); @@ -146,12 +176,14 @@ export async function prepareStructuralReview( started = true; } else if (event.type === "file") { const path = structuralFilePath(event.file); + if (!entries.some(entry => !entry.evidence && entry.file.path === path && !unchanged.has(path))) return; seen.add(path); try { - const accepted = await accept(event); - onFile(accepted.path, { - stats: accepted.stats, - hidden: event.visibility?.collapsed ? event.visibility.label || "Hidden by default" : undefined, + const accepted = await Promise.all(resolvedEntries.filter(entry => !entry.evidence && entry.file.path === path).map(entry => accept(event, entry))); + onFile(path, { + ...retained.get(path), + stats: accepted[0]?.stats, + hidden: !retained.has(path) && event.visibility?.collapsed ? event.visibility.label || "Hidden by default" : undefined, }); } catch (error) { if (lifetime.isDisposed) throw error; @@ -175,7 +207,7 @@ export async function prepareStructuralReview( } await line(buffer); if (!complete) throw new Error("diffr stream ended before completion."); - for (const entry of entries) if (!seen.has(entry.file.path) && !unchanged.has(entry.file.path)) + for (const entry of entries) if (!entry.evidence && !seen.has(entry.file.path) && !unchanged.has(entry.file.path)) onFile(entry.file.path, { error: "diffr did not supply a result for this file." }); } finally { await reader.cancel().catch(() => {}); @@ -192,15 +224,11 @@ export async function prepareStructuralReview( onDidChange: providerChanged.event, async computeDiff(original, modified, _options, token) { if (token.isCancellationRequested) throw new CancellationError(); - const path = pairs.get(original.uri.with({ fragment: "" }).toString() + "\n" + modified.uri.with({ fragment: "" }).toString()); - if (path !== undefined && unchanged.has(path)) { - if (original.getValue() !== modified.getValue()) throw new Error("Referenced context file changed; reload the review."); - return { changes: [], moves: [], identical: true, quitEarly: false }; - } - if (path !== undefined && binary.has(path)) { + const path = original.uri.toString() + "\n" + modified.uri.toString(); + if (binary.has(path)) { return { changes: [], moves: [], identical: false, quitEarly: false, changeHighlights: { original: [], modified: [] } }; } - const diff = path === undefined ? undefined : files.get(path); + const diff = files.get(path); if (!diff) throw new Error("diffr did not supply a result for this file."); const left = (diff.lhs?.text ?? "").replace(/\r\n/g, "\n"); const right = (diff.rhs?.text ?? "").replace(/\r\n/g, "\n"); @@ -248,8 +276,8 @@ export async function prepareStructuralReview( // Every collapsed region is a hidden-region band, labelled by the wire. contextGaps: structuralContextGaps( diff, - (id) => collapsed.get(collapseKey(path!, id)) === true, - (id) => collapsed.get(collapseKey(path!, id)), + (id) => collapsed.get(collapseKey(path, id)) === true, + (id) => collapsed.get(collapseKey(path, id)), ), changeHighlights: highlights, }; @@ -260,7 +288,7 @@ export async function prepareStructuralReview( const child = lifetime.add( instantiation.createChild(new ServiceCollection([IDiffProviderFactoryService, factory])), ); - attachStructuralEditors(instantiation, resolvedEntries, files, lifetime, { + attachStructuralEditors(instantiation, files, lifetime, { get: (path, id) => collapsed.get(collapseKey(path, id)), set: (path, id, value) => { if (collapsed.get(collapseKey(path, id)) === value) return; @@ -278,25 +306,29 @@ export async function prepareStructuralReview( */ function attachStructuralEditors( instantiation: IInstantiationService, - entries: readonly ReviewFilesEditorEntry[], files: Map, lifetime: DisposableStore, collapsed: CollapseState, ): void { const editors = instantiation.invokeFunction((a) => a.get(ICodeEditorService)); - const pairs = new Map(entries.map((e) => [e.original!.toString() + "\n" + e.modified!.toString(), e.file.path])); function watch(editor: IDiffEditor) { const store = lifetime.add(new DisposableStore()); store.add(editor.onDidDispose(() => store.dispose())); const widget = editor as unknown as { unchangedRegions?: IObservable }; if (!widget.unchangedRegions) return; + const decorations = [editor.getOriginalEditor().createDecorationsCollection(), editor.getModifiedEditor().createDecorationsCollection()]; + store.add(toDisposable(() => decorations.forEach(decoration => decoration.clear()))); let revealed = new Set(); store.add( autorun((reader) => { const model = editor.getModel(); - const path = model && pairs.get(model.original.uri.with({ fragment: "" }).toString() + "\n" + model.modified.uri.with({ fragment: "" }).toString()); + const path = model && model.original.uri.toString() + "\n" + model.modified.uri.toString(); const regions = widget.unchangedRegions!.read(reader); - if (!path || !files.has(path)) return; + if (!path || !files.has(path)) { decorations.forEach(decoration => decoration.clear()); return; } + const diff = files.get(path)!; + [diff.lhs, diff.rhs].forEach((source, side) => decorations[side].set(structuralSearchHighlights(source).map(range => ({ + range, options: { description: "Review search highlight", inlineClassName: "review-search-highlight" }, + })))); const gaps = structuralContextGaps(files.get(path)!, (id) => collapsed.get(path, id) === true, (id) => collapsed.get(path, id)); const next = new Set(); const gapOf = (region: UnchangedRegion) => @@ -325,7 +357,7 @@ function attachStructuralEditors( for (const editor of editors.listDiffEditors()) watch(editor); } -/** Collapse state per file, keyed by fold-state id. */ +/** Collapse state per editor pair, keyed by fold-state id. */ interface CollapseState { get(path: string, foldStateId: number): boolean | undefined; set(path: string, foldStateId: number, value: boolean): void; diff --git a/docs/plans/diffr-evidence-verification.md b/docs/plans/diffr-evidence-verification.md new file mode 100644 index 000000000..1d481a0c9 --- /dev/null +++ b/docs/plans/diffr-evidence-verification.md @@ -0,0 +1,75 @@ +# Direct diffr evidence: verification + +Review's contract scaffold is `bb635df4`; implementation starts with `2a7b8dfb`. Diffr's contract scaffold is `a4f76a4ad`; implementation starts with `71d8db355`. + +## Automated checks + +- 101 focused Review tests pass, including persistence/reopen, invalid pins/text/blob rejection, stale evidence after repin, mixed glob/result targets, authoring API behavior, and lens UI lifecycle. +- 132 native tests pass, including UTF-8 highlight conversion, one-sided rows, nested fold expansion, paired changed lines, pinned source reads, and separate evidence/navigation/viewed actions for two sections of the same file. The desktop script tests also pass. +- All 9 diffr code-mode tests pass. Existing pretty-output snapshots are unchanged; the new schema test round-trips real hydrated and postprocessed results. +- Review, native, and diffr TypeScript checks pass. The dev build succeeds. + +## Real API-to-app run + +`diffr-search/tests/code-mode/review.live.ts` queried `Diffr evidence text differs` in this implementation's `local-data.ts`, comparing `e54665e3` with `90d7288c`. It used the real diffr JS binding, hydrated and postprocessed the query, and submitted structured results through Review's HTTP authoring API. It also selected a base-side result and a wholly unchanged `AGENTS.md` result. + +The saved review includes a head-only excerpt, head-only and paired lenses, a mixed file-glob/result lens, an unchanged-file excerpt, and two diagram steps with distinct side selections on the same file. The script removed the query worktrees before reading back and opening the saved document. + +The completed dev Review is `cc4f1cc1-1fbf-40d1-b42c-df4dafece6e6`, version 7. It remains open in the isolated `/tmp/review-evidence-dev` app instance. Local artifacts are in `/tmp/review-evidence-e2e-final/` (`evidence.json`, `pretty.txt`, `review.json`), with no server credentials. + +Computer-use verification confirmed: + +- Head-only code displays with search highlighting; Open File selects its matched line 739 in pinned source. +- The mixed lens retains both the glob-selected `source.ts` and the supplied search results, including unchanged `AGENTS.md`. +- The diagram has separate head-only and paired sections; selecting the paired step navigates to its section. +- Marking the head-only section viewed marks 76 added lines and leaves all five base deletions unread. The paired section changes to +0/-5. Viewed state was reset afterward. +- Earlier checks confirmed local nested fold toggling, find over visible evidence, and paired deleted/added rows. + +The larger two-result sequence initially hit the old 1 MiB authoring limit. The command endpoint now permits 8 MiB, matching resource uploads; the unchanged live test then passed. No evidence was truncated or replaced with ranges to pass the test. + +## Contracts and remaining environment limitation + +The approved `targets` ADT now flows through `DiagramLens` and native `ReviewDiffSection`. `sources` remains a coverage/navigation projection. No SQL schema, computed-diff cache schema, upload requirement, or evidence wire-field rename was introduced. The shared diffr contract is installed from a pinned Git revision. + +The dev app reports a language-environment preparation failure on both tested revisions: its setup runs the review-protocol build before the `@dev.fast/json` and trace-protocol declaration outputs exist. Pinned-source opening and line selection were verified despite this warning; LSP definition navigation is not claimed as verified. + +## Review order + +- diffr: #6 contracts/output β†’ #7 plugins β†’ #20 Rust storage β†’ #8 search/bindings β†’ #10 Jev β†’ [#13 Review evidence](https://github.com/devdotfast/diffr/pull/13) +- Review: [#369 contracts](https://github.com/devdotfast/review/pull/369) β†’ [#381 validation/storage](https://github.com/devdotfast/review/pull/381) β†’ [#370 rendering](https://github.com/devdotfast/review/pull/370) + +Both are draft stacks. History now groups the final contracts, consumers, and implementations by system boundary; superseded approaches and follow-up corrections are folded into their owning changes. The rewritten diffr tip has the exact original Git tree; Review application code is unchanged, with its dependency repinned to that rewritten tip. The verification identities below record the original runs. + +## Structural Diff renderer correction + +The initial integration routed explicit lenses through an inline-editor container. That preserved evidence coordinates but bypassed the established structural Diff UI; the initial E2E did not establish UI parity. This path, its bespoke toolbar/file list, and its implementation-coupled DOM test have been removed. + +Both retained results and ordinary range lenses now use ReviewFilesDiffView and the existing structural provider. Retained trees seed the existing alignment, fold bands, labels/pseudocode, and collapse controls. Search decorations are blue. Model-pair identity separates two same-file results/sections. Explicit side selection is preserved, with a native internal forceInline option preventing an empty counterpart pane. No public evidence DTO or stored schema changed. + +Validation: 133 native tests and native typechecking pass; the dev build succeeds. The new checks cover head-only pseudocode bands and UTF-16 search spans, JSON-transported section targets, and one-sided range navigation. Visual checks against the saved real search results confirmed the native file header, fold expansion, blue highlight, full-width head-only layout, mixed targets (three files), and navigation between the head-only and paired diagram sections. Pseudocode label conversion is covered by tests; the saved query's labels are unchanged-line summaries. + + +## Complete comparisons and shared unchanged sources + +Diffr now retains complete comparison trees through hydration and postprocessing. +The public Comparison union couples file references to sources, stores unchanged +content once as `sources.same`, and uses `display` solely for presentation. Review +validates both retained sides and upgrades older saved `kind` payloads on read. +It keeps using the existing structural Diff component; no replacement view was added. + +Validated against diffr `9516f12f4` and Review `b1c608ee`: 262 Rust library tests, +11 JS integration tests (unchanged pretty-output snapshot), 68 focused Review tests, +20 native presentation tests, and TypeScript checks. The dev build succeeds. + +The live query selected evidence from Review's own `e54665e3..b1c608ee` diff, +submitted head-only, paired, shared-unchanged and base-only displays, removed the +query worktrees, and verified the saved evidence round trip. Review +`0aabeb9f-e593-4b98-bf8c-dad4dbad66f4`, version 7, remains in the isolated dev app. +Artifacts: `/tmp/review-evidence-comparison/{evidence.json,pretty.txt,review.json}`. +Computer-use checks confirmed the full-width head-only structural view, blue +matched-line highlight, native fold expansion revealing retained lines 9–24, +and shared unchanged AGENTS.md displayed in the mixed-results lens. + +The separate language-environment warning remains. A normal whole-file entry in +the mixed lens also reported a busy pinned checkout during preparation; retained +search evidence displayed successfully. This check does not establish LSP readiness. diff --git a/packages/review/app/src/CodePeek.tsx b/packages/review/app/src/CodePeek.tsx index e7605088b..fc46476cc 100644 --- a/packages/review/app/src/CodePeek.tsx +++ b/packages/review/app/src/CodePeek.tsx @@ -6,7 +6,12 @@ import type { import { useMemo, useRef } from "react"; import type { ReviewComponentProps } from "../../src/review-document-data"; -import { type Source, codePeekSource } from "../../src/source"; +import { + type Source, + type CodeEvidence, + evidenceLocation, + codePeekSource, +} from "../../src/source"; import { useReviewSession } from "./host/review-session"; import { InlineCodeEditor } from "./InlineCodeEditor"; @@ -104,7 +109,7 @@ export function CodePeekCard({ heightMode = "capped", onNativeFocus, }: { - source: Source; + source: CodeEvidence; active?: boolean; heightMode?: ReviewInlineEditorHeightMode; onNativeFocus?: () => void; @@ -121,7 +126,8 @@ export function CodePeekCard({ @@ -138,7 +144,8 @@ export function CodePeekCard({ ); } -export function codePeekSubject(source: Source): CodePeekSubject { +export function codePeekSubject(evidence: CodeEvidence): CodePeekSubject { + const source = evidenceLocation(evidence); return { title: codePeekRangeTitle(source.file, source.fromLine, source.toLine), file: source.file, diff --git a/packages/review/app/src/DiffView.tsx b/packages/review/app/src/DiffView.tsx index 01841c691..f4cc6a1ed 100644 --- a/packages/review/app/src/DiffView.tsx +++ b/packages/review/app/src/DiffView.tsx @@ -6,6 +6,7 @@ import type { } from "@dev.fast/review-protocol"; import { useLayoutEffect, useMemo, useRef, useState } from "react"; +import { evidenceSources } from "../../src/source"; import type { Source } from "../../src/source"; import { type CoverageProgress, @@ -85,7 +86,14 @@ export function ReviewDiffView({ scope }: { scope?: ReviewCommitScope }) { ? { files: lenses.progress.files.map((file) => ({ path: file.path, - ...coverageProgress([file], lens.ranges), + ...coverageProgress( + [file], + lens.targets.flatMap((target) => + target.kind === "ranges" + ? [...target.ranges] + : target.results.flatMap(evidenceSources), + ), + ), viewedRanges: coverageSources(file), changedRanges: coverageSources(file, file.changed), unfoldRanges: lenses.unfoldRanges.filter( @@ -126,7 +134,11 @@ export function ReviewDiffView({ scope }: { scope?: ReviewCommitScope }) { const sources = scoped ? (sectionId ? sections.find((section) => section.id === sectionId)?.sources - : lens?.ranges + : lens?.targets.flatMap((target) => + target.kind === "ranges" + ? [...target.ranges] + : target.results.flatMap(evidenceSources), + ) )?.filter( (source) => source.file === @@ -244,16 +256,22 @@ export function ReviewDiffView({ scope }: { scope?: ReviewCommitScope }) { {lenses.progress ? lens ? new Set( - lens.ranges.map( - (source) => - lenses.progress!.files.find( - (file) => - source.file === - (source.side === "base" - ? (file.previousPath ?? file.path) - : file.path), - )?.path ?? source.file, - ), + lens.targets + .flatMap((target) => + target.kind === "ranges" + ? [...target.ranges] + : target.results.flatMap(evidenceSources), + ) + .map( + (source) => + lenses.progress!.files.find( + (file) => + source.file === + (source.side === "base" + ? (file.previousPath ?? file.path) + : file.path), + )?.path ?? source.file, + ), ).size : lenses.progress.files.length : "…"} diff --git a/packages/review/app/src/InlineCodeEditor.tsx b/packages/review/app/src/InlineCodeEditor.tsx index 675e0af14..1322a4aa1 100644 --- a/packages/review/app/src/InlineCodeEditor.tsx +++ b/packages/review/app/src/InlineCodeEditor.tsx @@ -7,6 +7,7 @@ import type { } from "@dev.fast/review-protocol"; import { useCallback, useLayoutEffect, useRef, useState } from "react"; +import type { CodeEvidence } from "../../src/source"; import { useReviewSession } from "./host/review-session"; import { useReviewFindRegistration } from "./review-find"; import { emitReviewInteraction } from "./review-interaction-event"; @@ -19,6 +20,7 @@ const INLINE_HEADER_HEIGHT = 40; export function InlineCodeEditor({ path, + evidence, title, description, side, @@ -31,6 +33,7 @@ export function InlineCodeEditor({ collapsed = false, }: { path: string; + evidence?: CodeEvidence; title: string; description?: string; side: ReviewDiffSide; @@ -43,6 +46,12 @@ export function InlineCodeEditor({ collapsed?: boolean; }) { const session = useReviewSession(); + const content = + evidence && "display" in evidence + ? { kind: "diffr" as const, result: evidence } + : { kind: "source" as const, path, side, ranges }; + const evidenceKey = + evidence && "display" in evidence ? JSON.stringify(evidence) : ""; const [container, setContainer] = useState(null); const handleNavigation = useCallback( @@ -110,9 +119,9 @@ export function InlineCodeEditor({ if (handle) return handle.setFindQuery(query); - return inlineEditorFactory.find({ path, side, ranges }, query); + return inlineEditorFactory.find({ content }, query); }, - [inlineEditorFactory, path, rangesKey, side], + [inlineEditorFactory, path, rangesKey, side, evidenceKey], ); const revealFindMatch = useCallback( @@ -188,11 +197,9 @@ export function InlineCodeEditor({ try { handle = inlineEditorFactory.create({ container, - path, + content, title, description, - side, - ranges, heightMode, active, @@ -242,6 +249,7 @@ export function InlineCodeEditor({ inlineEditorSessionId, path, rangesKey, + evidenceKey, shouldMount, side, title, diff --git a/packages/review/app/src/authored-code-surface.tsx b/packages/review/app/src/authored-code-surface.tsx index 480b00e4e..e66d1721a 100644 --- a/packages/review/app/src/authored-code-surface.tsx +++ b/packages/review/app/src/authored-code-surface.tsx @@ -1,5 +1,6 @@ import type { ReactElement } from "react"; +import { evidenceLocation } from "../../src/source"; import type { PeekAnchor } from "./review-panel-model"; /** @@ -15,7 +16,7 @@ export function AuthoredCodeSurface({ code: string; language?: string; }): ReactElement { - const firstLine = anchor.peek?.fromLine ?? 1; + const firstLine = anchor.peek ? evidenceLocation(anchor.peek).fromLine : 1; return (
diff --git a/packages/review/app/src/call-stack-diff.tsx b/packages/review/app/src/call-stack-diff.tsx index b58c5fa24..69f549da2 100644 --- a/packages/review/app/src/call-stack-diff.tsx +++ b/packages/review/app/src/call-stack-diff.tsx @@ -4,6 +4,7 @@ import { } from "../../src/call-stack-diff"; import { frameIdentity, frameName } from "../../src/call-stack-frames"; import type { Frame } from "../../src/review-api/document"; +import { evidenceLocation } from "../../src/source"; import { useReviewSession } from "./host/review-session"; import { useReviewPanel } from "./review-panel"; import type { PeekAnchor } from "./review-panel-model"; @@ -59,7 +60,7 @@ export function CallStackDiff({ title, base, head }: CallStackDiffProps) { role="listitem" className={`call-stack-row call-stack-${row.change}`} data-review-anchor-id={frame.id ?? frameIdentity(frame)} - title={`${rowTooltip(frame, parent)} β€” ${frame.source.file}:${frame.source.fromLine}`} + title={`${rowTooltip(frame, parent)} β€” ${evidenceLocation(frame.source).file}:${evidenceLocation(frame.source).fromLine}`} onClick={() => { captureUiEvent(session, "peek_opened", { via: "call_stack_frame", @@ -83,7 +84,10 @@ export function CallStackDiff({ title, base, head }: CallStackDiffProps) { ) : null} - {locationLabel(frame.source.file, frame.source.fromLine)} + {locationLabel( + evidenceLocation(frame.source).file, + evidenceLocation(frame.source).fromLine, + )} ); diff --git a/packages/review/app/src/call-tree.ts b/packages/review/app/src/call-tree.ts index 457428ec3..19345b4fd 100644 --- a/packages/review/app/src/call-tree.ts +++ b/packages/review/app/src/call-tree.ts @@ -1,10 +1,18 @@ +import type { ReviewDiffLensTarget } from "@dev.fast/review-protocol"; + import type { CallStackDiffBlock } from "../../src/review-api/blocks/call_stack_diff"; +import { + evidenceLocation, + evidenceSources, + evidenceTargets, +} from "../../src/source"; import type { Source } from "../../src/source"; export interface CallTreeStop { id: string; label: string; sources: Source[]; + targets: ReviewDiffLensTarget[]; parentId?: string; callSite?: Source; via?: string; @@ -27,19 +35,28 @@ export function callTreeStops(block: CallStackDiffBlock): CallTreeStop[] { const id = `${block.id}:${frame.key ?? `${side}:${index}`}`; const existing = nodes.get(id); - if (existing) + const evidence = [ + frame.source, + ...(frame.contextSources ?? []), + ...(frame.callSite ? [frame.callSite] : []), + ]; + if (existing) { + existing.targets.push(...evidenceTargets(evidence)); existing.sources.push( - frame.source, - ...(frame.contextSources ?? []), + ...evidenceSources(frame.source), + ...(frame.contextSources ?? []).flatMap(evidenceSources), ...(frame.callSite ? [frame.callSite] : []), ); - else + } else nodes.set(id, { id, - label: frame.label ?? frame.source.file.split("/").pop()!, + label: + frame.label ?? + evidenceLocation(frame.source).file.split("/").pop()!, + targets: evidenceTargets(evidence), sources: [ - frame.source, - ...(frame.contextSources ?? []), + ...evidenceSources(frame.source), + ...(frame.contextSources ?? []).flatMap(evidenceSources), ...(frame.callSite ? [frame.callSite] : []), ], parentId: diff --git a/packages/review/app/src/database-lens.tsx b/packages/review/app/src/database-lens.tsx index 5d6e056fe..d7ef2b148 100644 --- a/packages/review/app/src/database-lens.tsx +++ b/packages/review/app/src/database-lens.tsx @@ -15,7 +15,7 @@ import type { DatabaseOperation, DatabaseStore, } from "../../src/review-api/document"; -import type { Source } from "../../src/source"; +import type { CodeEvidence as Source } from "../../src/source"; import { DiagramTourOverlay, useDiagramTourShell } from "./diagram-tour"; import { useReviewSession } from "./host/review-session"; import type { GuidedTour, PeekAnchor } from "./review-panel-model"; diff --git a/packages/review/app/src/diff-sections.ts b/packages/review/app/src/diff-sections.ts index 34f0339eb..e571c90a2 100644 --- a/packages/review/app/src/diff-sections.ts +++ b/packages/review/app/src/diff-sections.ts @@ -1,5 +1,11 @@ +import type { ReviewDiffLensTarget } from "@dev.fast/review-protocol"; + import type { Block } from "../../src/review-api/document"; -import type { Source } from "../../src/source"; +import { + evidenceSources, + evidenceTargets, + type Source, +} from "../../src/source"; import { callTreeStops } from "./call-tree"; /** The same scope drives diagram navigation, diff boundaries and viewed actions. */ @@ -7,6 +13,7 @@ export interface DiffSection { id: string; label: string; sources: Source[]; + targets: ReviewDiffLensTarget[]; } export function diffSections(block: Block | undefined): DiffSection[] { @@ -19,7 +26,8 @@ export function diffSections(block: Block | undefined): DiffSection[] { { id: step.id ?? `${block.id}:${index}`, label: step.label, - sources: [step.source], + sources: evidenceSources(step.source), + targets: evidenceTargets([step.source]), }, ] : [], @@ -32,7 +40,8 @@ export function diffSections(block: Block | undefined): DiffSection[] { useCase.operations.map((operation, index) => ({ id: operation.id ?? `${useCase.id}:${index}`, label: operation.label, - sources: [operation.source], + sources: evidenceSources(operation.source), + targets: evidenceTargets([operation.source]), })), ); diff --git a/packages/review/app/src/fixture-review-bridge.ts b/packages/review/app/src/fixture-review-bridge.ts index f9d5df883..6bdec5f7e 100644 --- a/packages/review/app/src/fixture-review-bridge.ts +++ b/packages/review/app/src/fixture-review-bridge.ts @@ -85,7 +85,11 @@ export function fixtureReviewBridge(api: FixtureReviewApi): ReviewCanvasBridge { create: (spec) => { const editor = document.createElement("div"); editor.className = "fixture-inline-editor"; - editor.dataset.path = spec.path; + editor.dataset.path = + spec.content.kind === "source" + ? spec.content.path + : (spec.content.result.file.rhs ?? spec.content.result.file.lhs!) + .path; spec.container.appendChild(editor); return { diff --git a/packages/review/app/src/lens-diagram.tsx b/packages/review/app/src/lens-diagram.tsx index d579978a8..e29611d6b 100644 --- a/packages/review/app/src/lens-diagram.tsx +++ b/packages/review/app/src/lens-diagram.tsx @@ -1,5 +1,10 @@ import type { Block } from "../../src/review-api/document"; -import type { Source } from "../../src/source"; +import { + evidenceSources, + evidenceLocation, + type CodeEvidence, + type Source, +} from "../../src/source"; import { LensCallTree } from "./lens-call-tree"; import { ElementCounts } from "./lens-counts"; import { useReviewLenses } from "./review-lenses"; @@ -7,15 +12,18 @@ import { useReviewLenses } from "./review-lenses"; /** Compact diagrams are navigation: clicking evidence scrolls, never changes scope. */ export function LensDiagram({ block, - onReveal, + onReveal: reveal, }: { block: Block; onReveal(source: Source, sectionId?: string): void; }) { const lenses = useReviewLenses()!; + const onReveal = (evidence: CodeEvidence, sectionId?: string) => + reveal(evidenceLocation(evidence), sectionId); + const stats = (sources: CodeEvidence[]) => + lenses.stats(sources.flatMap(evidenceSources)); - const viewed = (sources: Source[]) => - lenses.stats(sources).state === "viewed"; + const viewed = (sources: CodeEvidence[]) => stats(sources).state === "viewed"; if (block.type === "sequence") { const actors = Object.entries(block.actors); @@ -72,7 +80,7 @@ export function LensDiagram({ {step.label} {source - ? ` Β· Total +${lenses.stats([source]).total.additions} βˆ’${lenses.stats([source]).total.deletions}` + ? ` Β· Total +${stats([source]).total.additions} βˆ’${stats([source]).total.deletions}` : ""} - + )}