Validate portable evidence and exercise direct Review submission

bindings/node/schema.ts

@@ -0,0 +1,66 @@
+// Shared validation for the serialized search contract; no native binding import.
+import { z } from "zod";
+import type { RegionData, SearchResultData, SourceData } from "./types.ts";
+
+const coordinate = z.number().int().nonnegative();
+const position = z.strictObject({ line: coordinate, column: coordinate });
+const span = z.strictObject({ line: coordinate, start_column: coordinate, end_column: coordinate });
+const regionFields = {
+  id: coordinate,
+  fold_state_id: coordinate,
+  start: position,
+  end: position,
+  tags: z.array(z.string()).optional(),
+  visibility: z.strictObject({ collapsed: z.boolean().optional(), label: z.string().optional() }).optional(),
+};
+export const regionDataSchema: z.ZodType<RegionData> = z.lazy(() => z.discriminatedUnion("kind", [
+  z.strictObject({ ...regionFields, kind: z.literal("leaf"), alignment_id: coordinate,
+    changed: z.array(span).optional(), search_highlights: z.array(span).optional() }),
+  z.strictObject({ ...regionFields, kind: z.literal("fold"), children: z.array(regionDataSchema) }),
+]));
+const fileRef = z.strictObject({ path: z.string(), oid: z.string(), mode: z.string() });
+const source = z.strictObject({ text: z.string(),
+  syntax: z.array(span.extend({ capture: z.string() })).optional(), regions: z.array(regionDataSchema) });
+const worktree = z.strictObject({ commitId: z.string(), path: z.string() });
+const common = {
+  display: z.enum(["both", "lhs", "rhs"]),
+  scope: z.strictObject({ repo: z.string(), baseWorktree: worktree, headWorktree: worktree }),
+};
+export const searchResultDataSchema: z.ZodType<SearchResultData> = z.union([
+  z.strictObject({ ...common, file: z.strictObject({lhs: fileRef, rhs: fileRef}), sources: z.strictObject({same: source}) }),
+  z.strictObject({ ...common, file: z.strictObject({lhs: fileRef, rhs: fileRef}), sources: z.strictObject({lhs: source, rhs: source}) }),
+  z.strictObject({ ...common, file: z.strictObject({lhs: fileRef}), sources: z.strictObject({lhs: source}) }),
+  z.strictObject({ ...common, file: z.strictObject({rhs: fileRef}), sources: z.strictObject({rhs: source}) }),
+]).superRefine((result, context) => {
+  const fail = (message: string) => context.addIssue({code: "custom", message});
+  if (result.display !== "both" && !(result.display in result.file)) fail("Display requests an absent side");
+  if ("same" in result.sources && "lhs" in result.file && "rhs" in result.file && result.file.lhs.oid !== result.file.rhs.oid) fail("Shared content requires identical blob identities");
+  for (const file of Object.values(result.file)) {
+    if (!file.path || file.path.startsWith("/") || file.path.includes("\\") || file.path.split("/").some(part => part === ".." || part === ".") || /[\x00-\x1f]/.test(file.path)) fail("File paths must be repository-relative");
+    if (!/^(?:[a-f0-9]{40}|[a-f0-9]{64})$/.test(file.oid) || !["100644", "100755"].includes(file.mode)) fail("Expected regular Git blob identity");
+  }
+  const compare = (a: {line: number; column: number}, b: {line: number; column: number}) => a.line - b.line || a.column - b.column;
+  for (const source of Object.values(result.sources) as SourceData[]) {
+    const lines = source.text.split("\n");
+    const bytes = lines.map(line => {
+      const boundaries = new Set([0]); let length = 0;
+      for (const char of line) { length += new TextEncoder().encode(char).length; boundaries.add(length); }
+      return boundaries;
+    });
+    const position = (pos: {line: number; column: number}) => bytes[pos.line]?.has(pos.column) === true;
+    const walk = (regions: RegionData[], parent?: RegionData) => {
+      for (const region of regions) {
+        if (!position(region.start) || !position(region.end) || compare(region.start, region.end) > 0) fail("Region is outside source bounds");
+        if (parent && (compare(region.start, parent.start) < 0 || compare(region.end, parent.end) > 0)) fail("Child region is outside its parent");
+        if (region.kind === "fold") walk(region.children, region);
+        else for (const span of [...(region.changed ?? []), ...(region.search_highlights ?? [])]) {
+          const start = {line: span.line, column: span.start_column}, end = {line: span.line, column: span.end_column};
+          if (!position(start) || !position(end) || compare(start, end) > 0 || compare(start, region.start) < 0 || compare(end, region.end) > 0) fail("Highlight is outside its leaf or splits a UTF-8 character");
+        }
+      }
+    };
+    walk(source.regions);
+    for (const span of source.syntax ?? []) if (!position({line: span.line, column: span.start_column}) || !position({line: span.line, column: span.end_column}) || span.start_column > span.end_column) fail("Syntax span is outside source bounds");
+  }
+});
+export type { RegionData, SourceData, SearchResultData } from "./types.ts";

docs/review-evidence-plan.md

@@ -0,0 +1,55 @@
+# Direct diffr evidence in Review
+
+## Agreed behavior
+
+An AI model retrieves hits, calls the diffr JS API (hydrate, candidate/side selection, postprocess, optional Jev), inspects pretty output, then submits the structured selected results through Review's existing MCP/HTTP/code-mode authoring operations. Review validates, persists, and renders the supplied evidence. It does not rerun the search or require a separate resource upload.
+
+Source locations remain `{side,file,fromLine,toLine}`. Displayable code evidence becomes `Source | SearchResultData`. A Source displays its explicit side/ranges; a diffr result displays exactly its supplied source pairing. Head-only evidence must remain head-only even for modified files. Focus/navigation coordinates do not add a display side.
+
+Diffr's wire fields retain their existing names. Review consumes shared method-free transport types and a runtime schema; its separate structural declarations must not drift from diffr. Search highlights and fold visibility are rendered from the supplied trees, without another context-expansion or opposite-side inference pass.
+
+## Contracts to scaffold, then implement
+
+```diff
++export type CodeEvidence = Source | SearchResultData;
++export const codeEvidenceSchema = z.union([sourceSchema, searchResultDataSchema]);
+
+ // Code peek, sequence step, frame, and code-backed operation fields:
+-source: sourceSchema
++source: codeEvidenceSchema
+
+ // File lens targets:
++{kind: "results", results: SearchResultData[]}
+
+ // Native inline editor content:
+-path: string;
+-side: ReviewDiffSide;
+-ranges: readonly ReviewInlineEditorRange[];
++content:
++  | {kind: "source"; path: string; side: ReviewDiffSide; ranges: readonly ReviewInlineEditorRange[]}
++  | {kind: "diffr"; result: SearchResultData};
+```
+
+Existing multiple-range, selection, count, event, and presentation behavior must be accounted for in implementation. Use existing Source locations for navigation within evidence, not a new focus/storage abstraction.
+
+## Stored state
+
+Review persists submitted results inline in its existing versioned document JSON (`versions.snapshot`). Existing Source blocks remain valid. No SQL migration, new table, resource-upload requirement, or retained-result wrapper. Save only authored evidence, not every candidate. Inline duplication is acceptable initially.
+
+Diffr's StoredDiff and Store backend schemas are unchanged: they cache query-independent computed trees, never selected highlights or processed visibility. A Review must reopen with its authored evidence even if that cache is cleared.
+
+Validate receiving repository/commit compatibility, file identities and source text, source pairing, and region/span bounds. Treat submitted worktree paths as metadata, never as permission or instructions to open paths. On repin/worktree refresh, incompatible results must be stale or require replacement; never reinterpret their trees against new pins. Sharing/version restore must carry the evidence intact.
+
+## Implementation and verification
+
+1. Commit this plan in both repos. Amend each plan commit with API and stored-JSON contract scaffolding; incomplete builds are permitted for this layer.
+2. Create a separate implementation change on top in each repo. Do not amend behavior into the contract layer.
+3. Implement shared diffr serialization/schema, Review validation/persistence/transport, and native rendering with exact side preservation and highlight/fold support.
+4. Test meaningful behavior: round trips, invalid/pin-mismatched evidence, existing Source input, left/right/paired/unchanged results, nested highlights/folds, reopen with diffr cache unavailable, repin handling, and lenses/other evidence-bearing blocks.
+5. Build a Review dev app. Query a subset of the implementation's own pinned diff via real diffr JS calls. Submit the selected structured evidence through the real Review authoring API, reopen the document, and inspect it in the app. Include a head-only result from a modified file and verify no base-side rows are manufactured.
+
+## Change control
+
+The user requires a stop and explicit flag before any API/data-flow deviation or additional storage schema is introduced. The contract changes listed here are the agreed baseline. Do not silently add an upload/resource model, a server-side search flow, storage tables, or new result wrappers. Record any necessary deviation and obtain the user's direction before dependent implementation.
+
+Implemented with Codex assistance.

tests/code-mode/api.test.ts

@@ -0,0 +1,166 @@
+import { afterAll, beforeAll, expect, test } from "bun:test";
+import * as diffr from "diffr/api";
+import { searchResultDataSchema } from "diffr/schema";
+import { execFileSync } from "node:child_process";
+import { createFixture, grep } from "./fixture";
+
+let fixture: Awaited<ReturnType<typeof createFixture>>;
+let hits: diffr.Hit[];
+beforeAll(async () => {
+  fixture = await createFixture();
+  hits = await grep(fixture.scope, "search_token");
+});
+afterAll(async () => { await fixture?.cleanup(); });
+
+test("reject stale text, invalid lines, paths outside the scope, and wrong pins", async () => {
+  const { scope } = fixture;
+  const hit = hits[0];
+  await expect(diffr.hydrate(scope, [{ ...hit, lines: [{ line: 1, text: "stale" }] }]))
+    .rejects.toThrow("hit text differs");
+  await expect(diffr.hydrate(scope, [{ ...hit, lines: [{ line: 0, text: "" }] }]))
+    .rejects.toThrow("1-based");
+  await expect(diffr.hydrate(scope, [{ ...hit, lines: [{ line: 999, text: "" }] }]))
+    .rejects.toThrow("out of bounds");
+  await expect(diffr.hydrate(scope, [{ ...hit, file: import.meta.path }]))
+    .rejects.toThrow("outside scoped worktrees");
+  await expect(diffr.hydrate({ ...scope, headWorktree: {
+    ...scope.headWorktree, commitId: scope.baseWorktree.commitId,
+  } }, [])).rejects.toThrow("worktree HEAD");
+});
+
+test("selection coalesces repeated candidates without highlighting the counterpart", async () => {
+  const { scope } = fixture;
+  const hit = hits.find(hit => hit.file.endsWith("head/retry.js") && hit.lines[0].line === 3)!;
+  const hydrated = await diffr.hydrate(scope, [hit, hit]);
+  const selected = hydrated[0];
+  expect(selected.sources.rhs!.regions.some(region => region.hasHighlights())).toBe(true);
+  expect(selected.sources.rhs!.regions.some(region => region.hasChangedHighlights())).toBe(false);
+  const [result] = await diffr.postprocess(scope, hydrated);
+  const spans = (source: diffr.Source) => {
+    const out: diffr.Span[] = [];
+    function visit(regions: diffr.Region[]) {
+      for (const region of regions) {
+        if (region.kind === "fold") visit(region.children);
+        else out.push(...region.search_highlights ?? []);
+      }
+    }
+    visit(source.regions);
+    return out;
+  };
+  expect(spans(result.sources.lhs!)).toEqual([]);
+  expect(spans(result.sources.rhs!)).toEqual([{
+    line: 2, start_column: 0, end_column: Buffer.byteLength(hit.lines[0].text),
+  }]);
+});
+
+test("expanding a fold updates printing locally without rerunning plugins", async () => {
+  const { scope } = fixture;
+  const hydrated = await diffr.hydrate(scope, hits);
+  const [first, second] = await Promise.all([
+    diffr.postprocess(scope, hydrated), diffr.postprocess(scope, hydrated),
+  ]);
+  const result = first.find(result => result.file.rhs?.path === "retry.js")!;
+  const other = second.find(result => result.file.rhs?.path === "retry.js")!;
+  const before = other.toString();
+  const id = Number(result.toString().match(/fold_state_id=(\d+)/)![1]);
+  function flatten(regions: diffr.Region[]): diffr.Region[] {
+    return regions.flatMap(region => [region,
+      ...(region.kind === "fold" ? flatten(region.children) : [])]);
+  }
+  const outer = flatten(result.sources.rhs!.regions).find(region => region.fold_state_id === id)!;
+  if (outer.kind !== "fold") throw new Error("Expected the try body fold");
+  const child = flatten(outer.children).find(region => region.fold_state_id !== id)!;
+  // Explicitly collapse a child: context need not create a collapsed child.
+  result.setCollapsed(child.fold_state_id, true);
+  result.setCollapsed(id, false);
+  expect(result.toString()).not.toContain(`fold_state_id=${id}]`);
+  expect(result.toString()).toContain(`fold_state_id=${child.fold_state_id}]`);
+  expect(result.toString()).not.toContain("return response;");
+  result.setCollapsed(child.fold_state_id, false);
+  expect(result.toString()).toContain("return response;");
+  expect(result.toString()).toContain("const response = request();");
+  expect(other.toString()).toBe(before);
+  expect(() => result.setCollapsed(999999, false)).toThrow("No fold_state_id");
+});
+
+test("postprocessing accepts plugin settings and a deliberate side-only view", async () => {
+  const { scope } = fixture;
+  const [result] = await diffr.hydrate(scope, hits.filter(hit => hit.file.endsWith("head/retry.js")));
+  result.display = "rhs";
+  const [processed] = await diffr.postprocess(scope, [result], { plugins: { order: [] } });
+  expect(processed.sources.lhs).toBeDefined();
+  expect(processed.display).toBe("rhs");
+  expect(processed.toString()).toContain("const response = request();");
+  expect(processed.toString()).not.toContain("collapsed");
+});
+
+test("Git rename correspondence pairs different base and head paths", async () => {
+  const renamed = await createFixture();
+  try {
+    const { scope } = renamed;
+    const git = (...args: string[]) => execFileSync("git", [
+      "-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid",
+      "-c", "commit.gpgSign=false", "-c", "core.hooksPath=/dev/null", ...args,
+    ], { cwd: scope.repo, encoding: "utf8" }).trim();
+    git("mv", "retry.js", "renamed.js");
+    git("commit", "--quiet", "-m", "Rename fixture");
+    scope.headWorktree.commitId = git("rev-parse", "HEAD");
+    git("-C", scope.headWorktree.path, "checkout", "--quiet", "--detach", scope.headWorktree.commitId);
+    const results = await diffr.hydrate(scope, await grep(scope, "search_token"));
+    const result = results.find(result => result.file.rhs?.path === "renamed.js")!;
+    expect(result.file.lhs?.path).toBe("retry.js");
+    expect(result.display).toBe("both");
+  } finally { await renamed.cleanup(); }
+});
+
+
+test("wire schema accepts real hydrated and processed results and rejects corrupt evidence", async () => {
+  const hydrated = await diffr.hydrate(fixture.scope, hits);
+  const processed = await diffr.postprocess(fixture.scope, hydrated);
+  for (const result of [...hydrated, ...processed]) {
+    const wire = JSON.parse(JSON.stringify(result));
+    expect(searchResultDataSchema.parse(wire)).toEqual(wire);
+  }
+  const wire = JSON.parse(JSON.stringify(processed.find(result => result.file.rhs?.path === "retry.js")));
+  delete wire.file.lhs;
+  expect(searchResultDataSchema.safeParse(wire).success).toBe(false);
+  wire.file = {rhs: processed.find(result => result.file.rhs?.path === "retry.js")!.file.rhs};
+  wire.sources = {rhs: wire.sources.rhs};
+  wire.display = "rhs";
+  expect(searchResultDataSchema.safeParse(wire).success).toBe(true);
+  wire.sources.rhs.regions[0].end.line = 999999;
+  expect(searchResultDataSchema.safeParse(wire).success).toBe(false);
+});
+
+
+test("shared unchanged trees survive hydration, processing and every display choice", async () => {
+  const [result] = await diffr.hydrate(fixture.scope, hits.filter(hit => hit.file.endsWith("/unchanged.js")));
+  expect(result.sources.same).toBeDefined();
+  expect(result.sources.lhs).toBeUndefined();
+  expect(result.sources.rhs).toBeUndefined();
+  expect(result.file.lhs).toBeDefined();
+  expect(result.file.rhs).toBeDefined();
+  const [processed] = await diffr.postprocess(fixture.scope, [result]);
+  expect(processed.sources.same).toBeDefined();
+  for (const display of ["both", "lhs", "rhs"] as const) {
+    processed.display = display;
+    expect(searchResultDataSchema.safeParse(processed.toJSON()).success).toBe(true);
+    expect(processed.toString()).toContain("search_token: wholly unchanged file");
+  }
+});
+
+test("postprocessing rejects truncated comparisons and rewritten change classification", async () => {
+  const [result] = await diffr.hydrate(fixture.scope, hits.filter(hit => hit.file.endsWith("head/retry.js")));
+  const original = result.sources.rhs!.regions;
+  result.sources.rhs!.regions = [];
+  await expect(diffr.postprocess(fixture.scope, [result])).rejects.toThrow("differs from index");
+  result.sources.rhs!.regions = original;
+  const walk = (regions: diffr.Region[]) => {
+    for (const region of regions) {
+      if (region.kind === "fold") walk(region.children);
+      else region.changed = [];
+    }
+  };
+  walk(original);
+  await expect(diffr.postprocess(fixture.scope, [result])).rejects.toThrow("differs from index");
+});