Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/review-desktop-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,3 @@ artifacts/devbox/
scripts/acceptance/evidence/

packages/review/tutorial/pins.json
packages/review/bin/
2 changes: 1 addition & 1 deletion apps/review-desktop/code-oss/build/darwin/sign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,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 } };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ 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) };
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -270,8 +270,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 ? [diff.lhs.root] : undefined, state);
const rhs = knownRegions(diff.rhs ? [diff.rhs.root] : undefined, 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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,34 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { StructuralDiffSession } from './reviewStructuralDiffSession.js';
import type { StructuralEvent } from '../common/reviewStructuralDiff.js';
import { STRUCTURAL_WIRE_VERSION, type StructuralEvent } from '../common/reviewStructuralDiff.js';

const file = { rhs: { path: 'a.ts', oid: 'abc', mode: '100644' } };
const start: StructuralEvent = { type: 'start', version: 4, lhs: { type: 'empty_tree' }, rhs: { type: 'revision', rev: 'head' }, files: [{ file, status: 'added' }] };
const result: StructuralEvent = { type: 'file', file, diff: { type: 'text', structural_changes: { base: [], head: [[0, 1]] }, rhs: { text: 'a', regions: [{ kind: 'leaf', id: 1, fold_state_id: 2, alignment_id: 1, start: { line: 0, column: 0 }, end: { line: 0, column: 1 }, visibility: { collapsed: true } }] }, stats: { textual: { added: 1, removed: 0 }, visible: { added: 0, removed: 0 } } } };
const start: StructuralEvent = { type: 'start', version: STRUCTURAL_WIRE_VERSION, lhs: { type: 'empty_tree' }, rhs: { type: 'revision', rev: 'head' }, files: [{ file, status: 'added' }] };
const result: StructuralEvent = { type: 'file', file, diff: { type: 'text', structural_changes: { base: [], head: [[0, 1]] }, rhs: { text: 'a', root: { kind: 'leaf', id: 1, fold_state_id: 2, alignment_id: 1, start: { line: 0, column: 0 }, end: { line: 0, column: 1 }, visibility: { collapsed: true } } }, stats: { textual: { added: 1, removed: 0 }, visible: { added: 0, removed: 0 } } } };
const complete: StructuralEvent = { type: 'complete', succeeded: 1, failed: 0 };

test('file results supply root visibility and summary labels before comparison completion', async () => {
let release!: () => void;
const gate = new Promise<void>(resolve => release = resolve);
const labeled = structuredClone(result);
if (labeled.type !== 'file' || labeled.diff?.type !== 'text') throw new Error('Invalid fixture');
labeled.diff.rhs!.root.visibility = { collapsed: true, label: 'Generated file' };
const session = new StructuralDiffSession({ async *streamComparison() { yield start; yield labeled; await gate; yield complete; } });
const label = session.regionLabel('a.ts', 1);
const task = session.start();
try {
const fileResult = await session.fileResult('a.ts');
assert.equal(fileResult.hidden, 'Generated file');
assert.equal(label.get(), 'Generated file');
assert.equal(session.complete, false);
session.setRegionCollapsed('a.ts', 2, false);
release(); await task;
assert.equal(session.isRegionCollapsed('a.ts', 2), false);
assert.equal(session.error, undefined);
} finally { release(); session.dispose(); await task; }
});

test('views detach and reattach without restarting a comparison or losing folds', async () => {
let requests = 0;
const session = new StructuralDiffSession({ async *streamComparison() { requests++; yield start; yield result; yield complete; } });
Expand Down Expand Up @@ -61,117 +82,6 @@ test('truncation and missing manifest results are visible to later subscribers',
}
});

test('deferred annotations retain source identity, counts and user fold choices', async () => {
let release!: () => void;
const gate = new Promise<void>(resolve => release = resolve);
const session = new StructuralDiffSession({
async *streamComparison() {
yield start; yield structuredClone(result);
await gate;
yield { type: 'annotations', file, annotations: [{ region_id: 1, label: 'summarized' }] } as StructuralEvent;
yield complete;
}
});
const task = session.start();
while (!session.getTextDiff('a.ts')) await new Promise(resolve => setTimeout(resolve, 0));
try {
const initial = session.getTextDiff('a.ts')!;
assert.equal(session.complete, false);
session.setRegionCollapsed('a.ts', 2, false);
release(); await task;
assert.equal(session.getTextDiff('a.ts'), initial);
assert.equal(initial.rhs!.regions![0].visibility?.label, 'summarized');
assert.equal(initial.rhs!.text, 'a');
assert.deepEqual(initial.structural_changes, { base: [], head: [[0, 1]] });
assert.equal(session.isRegionCollapsed('a.ts', 2), false);
assert.equal(session.error, undefined);
} finally { release(); session.dispose(); }
});

test('invalid annotation batches are atomic and enrichment failure keeps the initial diff usable', async () => {
for (const invalid of [false, true]) {
const session = new StructuralDiffSession({
async *streamComparison() {
yield start; yield structuredClone(result);
yield {
type: 'annotations', file,
annotations: invalid ? [{ region_id: 1, label: 'must not apply' }, { region_id: 99, label: 'bad' }] : [],
error: invalid ? undefined : { code: 'enrichment_failed', message: 'offline' },
} as StructuralEvent;
yield complete;
}
});
try {
await session.start();
assert.equal(session.getTextDiff('a.ts')!.rhs!.regions![0].visibility?.label, undefined);
assert.equal(session.isRegionCollapsed('a.ts', 2), true);
if (invalid) assert.match(session.error!, /Unknown annotation region/);
else {
assert.equal(session.getFileResult('a.ts')?.annotationError, 'offline');
assert.equal(session.error, undefined);
}
} finally { session.dispose(); }
}
});

test('streamed labels update existing bands without invalidating either file provider', async () => {
const { StructuralDiffProvider } = await import('./reviewStructuralDiff.js');
const { URI } = await import('../../base/common/uri.js');
const { CancellationToken } = await import('../../base/common/cancellation.js');
const other = { rhs: { path: 'b.ts', oid: 'def', mode: '100644' } };
let release!: () => void;
const gate = new Promise<void>(resolve => release = resolve);
const session = new StructuralDiffSession({
async *streamComparison() {
yield { ...start, files: [...start.files, { file: other, status: 'added' as const }] };
yield structuredClone(result);
yield { ...structuredClone(result), file: other };
await gate;
yield { type: 'annotations', file, annotations: [{ region_id: 1, label: 'first' }] } as StructuralEvent;
yield { type: 'annotations', file, annotations: [{ region_id: 1, label: '// pseudocode\nfinal summary' }] } as StructuralEvent;
yield { ...complete, succeeded: 2 };
}
});
const task = session.start();
const ready = new Promise<void>(resolve => {
const sub = session.onDidChange(change => { if (change.files.has('b.ts')) { sub.dispose(); resolve(); } });
});
try {
await ready;
const models = (path: string) => [
{ uri: URI.parse(`test:/base/${path}`), getLinesContent: () => [''] },
{ uri: URI.parse(`test:/head/${path}`), getLinesContent: () => ['a'] },
] as const;
const a = models('a.ts'), b = models('b.ts');
const pairs = new Map([[`${a[0].uri}\n${a[1].uri}`, 'a.ts'], [`${b[0].uri}\n${b[1].uri}`, 'b.ts']]);
const pa = new StructuralDiffProvider(session, pairs, new Set());
const pb = new StructuralDiffProvider(session, pairs, new Set());
const compute = (provider: typeof pa, pair: ReturnType<typeof models>) => provider.computeDiff(
pair[0] as Parameters<typeof provider.computeDiff>[0], pair[1] as Parameters<typeof provider.computeDiff>[1],
{ ignoreTrimWhitespace: false, maxComputationTimeMs: 0, computeMoves: false }, CancellationToken.None,
);
const diff = await compute(pa, a);
await compute(pb, b);
let invalidationsA = 0, invalidationsB = 0, labelBatches = 0;
const subscriptions = [pa.onDidChange(() => invalidationsA++), pb.onDidChange(() => invalidationsB++),
session.onDidChange(change => { if (change.labels.has('a.ts')) labelBatches++; })];
const band = diff.contextGaps![0];
release(); await task;
assert.equal(band.labelObservable!.get(), '// pseudocode\nfinal summary');
assert.equal(invalidationsA, 0);
assert.equal(invalidationsB, 0);
assert.equal(labelBatches, 1, 'a burst publishes only the latest presentation state');
const folded = new Promise<void>(resolve => {
const sub = pa.onDidChange(() => { sub.dispose(); resolve(); });
});
session.setRegionCollapsed('a.ts', 2, false);
await folded;
assert.equal(invalidationsA, 1);
assert.equal(invalidationsB, 0, 'folding a file does not invalidate unrelated editors');
subscriptions.forEach(sub => sub.dispose());
} finally { release(); session.dispose(); await task; }
});

test('structural editor models retain diffr bytes across a live file save', async () => {
const { createStructuralDiffEditors, StructuralDiffProvider } = await import('./reviewStructuralDiff.js');
const { URI } = await import('../../base/common/uri.js');
Expand All @@ -188,7 +98,7 @@ test('structural editor models retain diffr bytes across a live file save', asyn
yield { ...start, files: [{ file: changedFile, status: 'modified' }] } as StructuralEvent;
await gate;
yield { type: 'file', file: changedFile, diff: {
type: 'text', lhs: { text: 'base\n' }, rhs: { text: 'saved A\n' },
type: 'text', lhs: { text: 'base\n', root: {kind:'leaf',id:3,fold_state_id:3,alignment_id:1,start:{line:0,column:0},end:{line:1,column:0}} }, rhs: { text: 'saved A\n', root: {kind:'leaf',id:4,fold_state_id:4,alignment_id:1,start:{line:0,column:0},end:{line:1,column:0}} },
structural_changes: { base: [[0, 1]], head: [[0, 1]] },
stats: { textual: { added: 1, removed: 1 }, visible: { added: 1, removed: 1 } },
} } as StructuralEvent;
Expand Down
Loading