improvement: update diffr to 0.1.14

.github/workflows/review-desktop-ci.yml

@@ -161,7 +161,6 @@ jobs:
 
       - name: Bundled diffr integration
         run: |
-          pnpm --filter @dev.fast/whiteboard ensure:diffr --required
           pnpm --filter @dev.fast/whiteboard exec vitest run --config vitest.integration.config.ts
 
       - name: Upload browser test diagnostics

.gitignore

@@ -91,4 +91,3 @@ artifacts/devbox/
 scripts/acceptance/evidence/
 
 packages/review/tutorial/pins.json
-packages/review/bin/

apps/review-desktop/code-oss/build/darwin/sign.ts

@@ -19,7 +19,7 @@ function getElectronVersion(): string {
 }
 
 function getEntitlementsForFile(filePath: string): string {
-	if (filePath.endsWith('/Contents/Resources/app/review-runtime/bin/diffr')) {
+	if (/\/Contents\/Resources\/app\/review-runtime\/node_modules\/@dev\.fast\/diffr-darwin-(arm64|x64)\/diffr$/.test(filePath)) {
 		return path.join(entitlementsDir, 'diffr.plist');
 	}
 	if (filePath.includes(' Helper (GPU).app')) {

apps/review-desktop/code-oss/src/vs/editor/browser/widget/diffEditor/diffEditorViewModel.ts

@@ -689,7 +689,7 @@ class SuppliedContextGap extends UnchangedRegion {
 	constructor(private readonly gap: NonNullable<IDocumentDiff['contextGaps']>[number]) {
 		super(gap.originalStart, gap.modifiedStart, Math.max(gap.originalCount, gap.modifiedCount), 0, 0);
 	}
-	override readLabel(reader: IReader | undefined): string | undefined { return this.gap.labelObservable?.read(reader) ?? this.gap.label; }
+	override readLabel(_reader: IReader | undefined): string | undefined { return this.gap.label; }
 	override get foldStateId(): number | undefined { return this.gap.foldStateId; }
 	override get owner(): 'base' | 'head' | 'both' { return this.gap.owner ?? 'both'; }
 	override get change(): 'unchanged' | 'inserted' | 'removed' | 'modified' { return this.gap.change ?? 'unchanged'; }

apps/review-desktop/code-oss/src/vs/editor/common/diff/documentDiffProvider.ts

@@ -3,7 +3,6 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import type { IObservable } from "../../../base/common/observable.js";
 import { CancellationToken } from '../../../base/common/cancellation.js';
 import { IRange } from '../core/range.js';
 import { Event } from '../../../base/common/event.js';
@@ -67,8 +66,6 @@ export interface IDocumentContextGap {
 	readonly originalCount: number;
 	readonly modifiedCount: number;
 	readonly label?: string;
-	/** Presentation updates do not invalidate the diff or its fold state. */
-	readonly labelObservable?: IObservable<string | undefined>;
 	readonly foldStateId?: number;
 	/** Where the fold control originates; independent of the enclosed code's change status. */
 	readonly owner?: 'base' | 'head' | 'both';

apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.test.ts

@@ -7,8 +7,6 @@ import assert from "node:assert/strict";
 import test from "node:test";
 import { projectInlineSourceAlignment, projectSplitSourceAlignment } from "../../editor/common/diff/sourceLineAlignment.js";
 import {
-	collapsedRegions,
-	hiddenLinesOf,
 	structuralContextGaps,
 	bandDetail,
 	structuralHighlights,
@@ -30,10 +28,10 @@ function leaf(id: number, start: number, end: number, extra: Partial<StructuralL
 }
 function fold(id: number, children: StructuralRegion[], tags: string[] = ["context:body"]): StructuralFold {
 	const first = children[0], last = children[children.length - 1];
-	return { id, fold_state_id: id, kind: "fold", start: first.start, end: last.end, tags, children };
+	return { id, fold_state_id: id, kind: "fold", indent: first.start, start: first.start, end: last.end, tags, children };
 }
 function text(lines: string[], regions: StructuralRegion[]) {
-	return { text: lines.join("\n") + "\n", regions };
+	return { text: lines.join("\n") + "\n", root: {kind: "fold" as const, id: 1000, fold_state_id: 1000, start: {line: 0, column: 0}, end: {line: lines.length, column: 0}, indent: {line: 0, column: 0}, children: regions} };
 }
 const stats = { textual: { added: 0, removed: 0 }, visible: { added: 0, removed: 0 } };
 
@@ -156,18 +154,6 @@ test("Tree-sitter byte offsets convert to Monaco UTF-16 columns", () => {
 });
 
 
-test("a collapsed region hides every line it covers, fold or leaf; nested collapses are subsumed", () => {
-	const collapsed = { collapsed: true, label: "x" };
-	const inner = fold(3, [leaf(4, 3, 4), leaf(5, 4, 6)]);
-	inner.visibility = collapsed;
-	const body = fold(2, [leaf(6, 2, 3), inner, leaf(7, 6, 8)]);
-	body.visibility = collapsed;
-	assert.deepEqual(hiddenLinesOf(body), { start: 2, end: 8 });
-	assert.deepEqual(hiddenLinesOf(leaf(1, 0, 2, { visibility: collapsed })), { start: 0, end: 2 });
-	assert.deepEqual(collapsedRegions([leaf(1, 0, 2, { visibility: collapsed }), body], (id) => id === 1 || id === 2 || id === 3).map((r) => r.fold_state_id), [1, 2]);
-	assert.deepEqual(collapsedRegions([leaf(1, 0, 2), body], (id) => id === 3).map((r) => r.fold_state_id), [3]);
-});
-
 test("every collapsed region becomes a labelled band: paired across sides, or one-sided at the aligned line", () => {
 	const pseudocode = "// pseudocode\nif not key: return None\nreturn call(key)";
 	const gapL = leaf(1, 0, 40, { visibility: { collapsed: true, label: "40 unchanged lines" } });
@@ -188,10 +174,10 @@ test("every collapsed region becomes a labelled band: paired across sides, or on
 	};
 	const gaps = structuralContextGaps(diff, (id) => id === 1 || id === 2 || id === 5);
 	assert.deepEqual(gaps, [
-		{ originalStart: 1, originalCount: 40, modifiedStart: 1, modifiedCount: 40, label: "40 unchanged lines", owner: "both", change: "unchanged", collapsed: true, foldStateId: 1, regionIds: [11, 1], breadcrumbs: true },
+		{ originalStart: 1, originalCount: 40, modifiedStart: 1, modifiedCount: 40, label: "40 unchanged lines", owner: "both", change: "unchanged", collapsed: true, foldStateId: 1, breadcrumbs: true },
 		// The removed body hides lines 42..60 on the left; its rows precede the added body's, so it anchors before them on the right.
-		{ originalStart: 42, originalCount: 19, modifiedStart: 41, modifiedCount: 0, label: "19 lines removed", owner: "base", change: "removed", collapsed: true, foldStateId: 2, regionIds: [2], breadcrumbs: true },
-		{ originalStart: 61, originalCount: 0, modifiedStart: 42, modifiedCount: 29, label: pseudocode, owner: "head", change: "inserted", collapsed: true, foldStateId: 5, regionIds: [5], breadcrumbs: true },
+		{ originalStart: 42, originalCount: 19, modifiedStart: 41, modifiedCount: 0, label: "19 lines removed", owner: "base", change: "removed", collapsed: true, foldStateId: 2, breadcrumbs: true },
+		{ originalStart: 61, originalCount: 0, modifiedStart: 42, modifiedCount: 29, label: pseudocode, owner: "head", change: "inserted", collapsed: true, foldStateId: 5, breadcrumbs: true },
 	]);
 	// A region the reader revealed stays a band, marked open, so the editor keeps a fold control on it.
 	const revealed = structuralContextGaps(diff, (id) => id === 2 || id === 5, (id) => (id === 1 ? false : id === 2 || id === 5 ? true : undefined));
@@ -222,7 +208,7 @@ test("a one-sided band also hides the opposite lines the zip aligned with it", (
 	const [gap] = structuralContextGaps(diff, (id) => id === 2);
 	assert.deepEqual(gap, {
 		originalStart: 2, originalCount: 5, modifiedStart: 2, modifiedCount: 5,
-		label: "5 unchanged lines", owner: "base", change: "unchanged", collapsed: true, foldStateId: 2, regionIds: [2], breadcrumbs: true,
+		label: "5 unchanged lines", owner: "base", change: "unchanged", collapsed: true, foldStateId: 2, breadcrumbs: true,
 	});
 });
 
@@ -242,7 +228,7 @@ test("a one-sided band starts after the row before its fold and hides only the o
 	]);
 	assert.deepEqual(structuralContextGaps(inserted, (id) => id === 20), [{
 		originalStart: 3, originalCount: 2, modifiedStart: 4, modifiedCount: 6,
-		label: "// pseudocode\nreturn rows", owner: "head", change: "inserted", collapsed: true, foldStateId: 20, regionIds: [20], breadcrumbs: true,
+		label: "// pseudocode\nreturn rows", owner: "head", change: "inserted", collapsed: true, foldStateId: 20, breadcrumbs: true,
 	}]);
 	// The mirror: a left-only function whose rows are all filler on the right, followed by a right-only line.
 	const removed = fold(20, [leaf(22, 3, 6)], ["deleted-bodies:function"]);
@@ -256,7 +242,7 @@ test("a one-sided band starts after the row before its fold and hides only the o
 	assert.deepEqual(structuralRows(deleted).slice(0, 8), [[0, 0], [1, 1], [2, null], [3, null], [4, null], [5, null], [null, 2], [6, 3]]);
 	assert.deepEqual(structuralContextGaps(deleted, (id) => id === 20), [{
 		originalStart: 4, originalCount: 3, modifiedStart: 3, modifiedCount: 0,
-		label: "3 lines removed", owner: "base", change: "removed", collapsed: true, foldStateId: 20, regionIds: [20], breadcrumbs: true,
+		label: "3 lines removed", owner: "base", change: "removed", collapsed: true, foldStateId: 20, breadcrumbs: true,
 	}]);
 });
 
@@ -277,7 +263,7 @@ test("alignment and fold state are separate: the zip follows one, collapse follo
 	assert.deepEqual(gaps.map((g) => [g.change, g.foldStateId]).sort(), [["inserted", 7], ["unchanged", 7]]);
 	assert.deepEqual(gaps.find((g) => g.change === "unchanged"), {
 		originalStart: 2, originalCount: 3, modifiedStart: 4, modifiedCount: 3,
-		label: "3 hidden lines", owner: "both", change: "unchanged", collapsed: true, foldStateId: 7, regionIds: [5, 2], breadcrumbs: true,
+		label: "3 hidden lines", owner: "both", change: "unchanged", collapsed: true, foldStateId: 7, breadcrumbs: true,
 	});
 	// With fold state 7 open, neither the docstring nor the function is a band.
 	assert.deepEqual(structuralContextGaps(diff, () => false), []);

apps/review-desktop/code-oss/src/vs/review/common/reviewStructuralDiff.ts

@@ -79,19 +79,19 @@ export function structuralContextScopes(diff: StructuralTextDiff) {
 			}
 			region.children.forEach(visit);
 		};
-		source?.regions?.forEach(visit);
+		if (source) visit(source.root);
 		return result;
 	};
 	return { original: scopes(diff.lhs), modified: scopes(diff.rhs) };
 }
 
-export function structuralLeaves(regions: readonly StructuralRegion[] | undefined): StructuralLeaf[] {
+function structuralLeaves(root: StructuralRegion): StructuralLeaf[] {
 	const leaves: StructuralLeaf[] = [];
 	const walk = (region: StructuralRegion) => {
 		if (region.kind === "leaf") leaves.push(region);
 		else for (const child of region.children) walk(child);
 	};
-	for (const region of regions ?? []) walk(region);
+	walk(root);
 	return leaves;
 }
 
@@ -120,7 +120,7 @@ export function structuralHighlights(diff: StructuralTextDiff) {
 			for (let line = start; line < end; line++) changedLines.push(line + 1);
 		}
 		const spans = [];
-		for (const leaf of structuralLeaves(source.regions)) {
+		for (const leaf of structuralLeaves(source.root)) {
 			for (const span of leaf.changed ?? []) {
 				spans.push({
 					startLineNumber: span.line + 1,
@@ -177,8 +177,6 @@ export interface StructuralGap {
 	collapsed: boolean;
 	/** The fold-state id of the region(s) this band hides; toggling the band toggles it. */
 	foldStateId: number;
-	/** Label precedence: head first, then base. */
-	regionIds: readonly number[];
 	/** False for a bundled docstring: its band shows the bare count, no symbol names. */
 	breadcrumbs: boolean;
 }
@@ -195,37 +193,20 @@ export function bandDetail(label: string): string {
 	return body.join("\n");
 }
 
-/**
- * The lines a collapsed region hides on its side, zero-based half-open: every
- * line it covers, fold or leaf. A fold covers its body alone — the line that
- * opens the construct is the last line of the leaf before it — so a band
- * hides exactly the fold's range, and the signature above it stays a row.
- */
-export function hiddenLinesOf(region: StructuralRegion): { start: number; end: number } {
-	return regionLines(region);
-}
-
-/** Collapsed regions of one side, outermost first; a collapsed descendant of a collapsed region is subsumed. `isCollapsed` answers for a fold-state id. */
-export function collapsedRegions(
-	regions: readonly StructuralRegion[] | undefined,
-	isCollapsed: (foldStateId: number) => boolean,
-): StructuralRegion[] {
-	return knownRegions(regions, (id) => (isCollapsed(id) ? true : undefined)).map((r) => r.region);
-}
-
 /**
  * Regions of one side the state knows about, outermost first: collapsed ones
  * and ones a reader revealed. A collapsed region subsumes its descendants; a
  * revealed one still lists them, since a child may be collapsed on its own.
  */
-export function knownRegions(
-	regions: readonly StructuralRegion[] | undefined,
+function knownRegions(
+	root: StructuralRegion | undefined,
 	state: (foldStateId: number) => boolean | undefined,
 ): { region: StructuralRegion; collapsed: boolean }[] {
 	const result: { region: StructuralRegion; collapsed: boolean }[] = [];
 	const walk = (region: StructuralRegion) => {
 		const known = state(region.fold_state_id);
-		const hides = hiddenLinesOf(region).end > hiddenLinesOf(region).start;
+		const { start, end } = regionLines(region);
+		const hides = end > start;
 		if (known === true) {
 			if (hides) result.push({ region, collapsed: true });
 			return;
@@ -234,7 +215,7 @@ export function knownRegions(
 		if (known === false && hides && region.visibility?.collapsed === true) result.push({ region, collapsed: false });
 		if (region.kind === "fold") for (const child of region.children) walk(child);
 	};
-	for (const region of regions ?? []) walk(region);
+	if (root) walk(root);
 	return result;
 }
 
@@ -270,8 +251,8 @@ export function structuralContextGaps(
 		for (let index = first; index <= last; index++) if (rows[index][other] !== null) count++;
 		return { start: before + 2, count };
 	};
-	const lhs = knownRegions(diff.lhs?.regions, state);
-	const rhs = knownRegions(diff.rhs?.regions, state);
+	const lhs = knownRegions(diff.lhs?.root, state);
+	const rhs = knownRegions(diff.rhs?.root, state);
 	// Leaves pair by alignment; folds pair by fold state, the only identity they share across sides.
 	const usedRhs = new Set<StructuralRegion>();
 	const pairs = (left: StructuralRegion, right: StructuralRegion) => {
@@ -282,18 +263,18 @@ export function structuralContextGaps(
 		rhs.find(({ region: right }) => !usedRhs.has(right) && pairs(left, right));
 	const gaps: StructuralGap[] = [];
 	for (const { region: left, collapsed } of lhs) {
-		const hidden = hiddenLinesOf(left);
+		const hidden = regionLines(left);
 		const partner = counterpart(left);
 		if (partner) {
 			usedRhs.add(partner.region);
-			const right = hiddenLinesOf(partner.region);
+			const right = regionLines(partner.region);
 			gaps.push({
 				originalStart: hidden.start + 1, originalCount: hidden.end - hidden.start,
 				modifiedStart: right.start + 1, modifiedCount: right.end - right.start,
 				label: partner.region.visibility?.label || left.visibility?.label || "",
 				owner: "both", change: "unchanged",
 				collapsed: collapsed && partner.collapsed,
-				foldStateId: left.fold_state_id, regionIds: [partner.region.id, left.id],
+				foldStateId: left.fold_state_id,
 				breadcrumbs: !isDocstring(left) && !isDocstring(partner.region),
 			});
 			continue;
@@ -302,18 +283,18 @@ export function structuralContextGaps(
 		gaps.push({
 			originalStart: hidden.start + 1, originalCount: hidden.end - hidden.start,
 			modifiedStart: opposite.start, modifiedCount: opposite.count,
-			label: left.visibility?.label || "", owner: "base", change: "unchanged", collapsed, foldStateId: left.fold_state_id, regionIds: [left.id],
+			label: left.visibility?.label || "", owner: "base", change: "unchanged", collapsed, foldStateId: left.fold_state_id,
 			breadcrumbs: !isDocstring(left),
 		});
 	}
 	for (const { region: right, collapsed } of rhs) {
 		if (usedRhs.has(right)) continue;
-		const hidden = hiddenLinesOf(right);
+		const hidden = regionLines(right);
 		const opposite = oppositeSpan(1, hidden);
 		gaps.push({
 			originalStart: opposite.start, originalCount: opposite.count,
 			modifiedStart: hidden.start + 1, modifiedCount: hidden.end - hidden.start,
-			label: right.visibility?.label || "", owner: "head", change: "unchanged", collapsed, foldStateId: right.fold_state_id, regionIds: [right.id],
+			label: right.visibility?.label || "", owner: "head", change: "unchanged", collapsed, foldStateId: right.fold_state_id,
 			breadcrumbs: !isDocstring(right),
 		});
 	}

apps/review-desktop/code-oss/src/vs/review/contrib/extensions/reviewCuratedExtensions.contribution.ts

@@ -30,9 +30,6 @@ import { IProgressService, ProgressLocation } from '../../../platform/progress/c
 import { IQuickInputService, type IQuickPickItem } from '../../../platform/quickinput/common/quickInput.js';
 import { Registry } from '../../../platform/registry/common/platform.js';
 import { IStorageService, StorageScope, StorageTarget } from '../../../platform/storage/common/storage.js';
-import { IReviewDesktopConnectionService } from '../../services/reviewDesktopConnectionService.js';
-import { IEditorService } from '../../../workbench/services/editor/common/editorService.js';
-import { ReviewCanvasEditorPane } from '../../browser/parts/canvas/reviewCanvasPart.js';
 import { LifecyclePhase } from '../../../workbench/services/lifecycle/common/lifecycle.js';
 import {
 	Extensions as WorkbenchExtensions,
@@ -740,22 +737,3 @@ Registry.as<IWorkbenchContributionsRegistry>(WorkbenchExtensions.Workbench).regi
 	OptionalExtensionPinUpgrades,
 	LifecyclePhase.Eventually
 );
-
-class InstallExtraDiffLanguagesAction extends Action2 {
-	constructor() {
-		super({ id: 'review.installExtraDiffLanguages', title: localize2('review.diff.installLanguages', "Install extra diffr languages"), f1: true });
-	}
-	override async run(accessor: ServicesAccessor): Promise<void> {
-		const connection = accessor.get(IReviewDesktopConnectionService);
-		const progress = accessor.get(IProgressService);
-		const editors = accessor.get(IEditorService);
-		await progress.withProgress({ location: ProgressLocation.Notification, title: localize('review.diff.installingLanguages', "Installing extra diffr languages..."), delay: 0 }, async () => {
-			const { serverUrl, token } = await connection.getConnection();
-			const response = await fetch(`${serverUrl}/diffr-languages/install`, { method: 'POST', headers: { 'x-review-token': token } });
-			if (!response.ok) throw new Error((await response.json()).error);
-			const pane = editors.activeEditorPane;
-			if (pane instanceof ReviewCanvasEditorPane) await pane.refreshComparison();
-		});
-	}
-}
-registerAction2(InstallExtraDiffLanguagesAction);

apps/review-desktop/code-oss/src/vs/review/services/reviewDiffViewService.ts

@@ -37,7 +37,7 @@ import { ReviewFilesDiffView, ReviewFilesEditorInput, type ReviewFilesEditorEntr
 import { ReviewEmbeddedEditors } from "./reviewEmbeddedEditors.js";
 
 import { createStructuralDiffEditors } from "./reviewStructuralDiff.js";
-import { StructuralDiffSession } from "./reviewStructuralDiffSession.js";
+import { StructuralDiffSession, type StructuralSessionChange } from "./reviewStructuralDiffSession.js";
 import type { StructuralDiffStream } from "./reviewStructuralDiffClient.js";
 
 export interface ReviewDiffViewSource {
@@ -360,17 +360,12 @@ class DiffViewController extends Disposable implements ReviewDiffViewHandle {
 
 	private observeSession(session: StructuralDiffSession, entries: readonly ReviewFilesEditorEntry[], view: ReviewFilesDiffView, store: DisposableStore, structuralEnabled: boolean): void {
 		const rendered = new Set<string>();
-		const annotationWarnings = new Set<string>();
-		const renderCurrentState = (change?: import("./reviewStructuralDiffSession.js").StructuralSessionChange) => {
+		const renderCurrentState = (change?: StructuralSessionChange) => {
 			if (this.disposed) return;
 			for (const entry of entries) {
 				const path = entry.file.path;
-				if (change && !change.files.has(path) && !change.labels.has(path)) continue;
+				if (change && !change.files.has(path)) continue;
 				const result = entry.file.status === "unchanged" ? {} : session.getFileResult(path);
-				if (result?.annotationError && !annotationWarnings.has(path)) {
-					annotationWarnings.add(path);
-					this._onDidError.fire(`Summary unavailable for ${path}: ${result.annotationError}`);
-				}
 				if (!result || rendered.has(path)) continue;
 				rendered.add(path);
 				if (structuralEnabled && result.hidden !== undefined) view.hideFile(path, result.hidden);