Fix bundled note decisions around LaTeX environment edits

engine/src/core/changes.ts

@@ -2,6 +2,7 @@ import { Repo, within } from "./git.ts";
 import { groups, opcodes } from "./diff.ts";
 import { entrySpans } from "./bib.ts";
 import { textEdits } from "./text-edits.ts";
+import { groupEnvironmentEdits } from "./environment-edits.ts";
 import type { Scope, Unit, Op } from "../types.ts";
 export const EXCERPT = 6000;
 export function textOf(data: Buffer | null): string | null {
@@ -73,22 +74,10 @@ function unitsOf(
     };
     const l = offsets(left),
       r = offsets(right);
-    let fine = !whole && !file.endsWith(".bib") ? textEdits(a!, b!) : null;
-    // Environment boundary edits are reviewed together, including unchanged
-    // text between the opening and closing commands.
-    const boundary = (text: string) =>
-      /\\(?:begin|end)\b/.test(text) &&
-      !/^\\begin\{([^}]+)\}[^]*\\end\{\1\}$/.test(text);
-    if (
-      fine?.some(
-        ([, i, j, x, y]) =>
-          boundary(a!.slice(i, j)) || boundary(b!.slice(x, y)),
-      )
-    ) {
-      const first = fine[0],
-        last = fine.at(-1)!;
-      fine = [["replace", first[1], last[2], first[3], last[4]]];
-    }
+    const fine =
+      !whole && !file.endsWith(".bib")
+        ? groupEnvironmentEdits(a!, b!, textEdits(a!, b!))
+        : null;
     const lineAt = (offsets: number[], offset: number) => {
       let lo = 0,
         hi = offsets.length - 1;

engine/src/core/environment-edits.ts

@@ -0,0 +1,82 @@
+import type { Op } from "../types.ts";
+import { structuralSource } from "./tex-source.ts";
+
+/** Changing an environment's delimiters couples only the edits inside that
+ * environment. Unrelated prose elsewhere in the file remains independent. */
+export function groupEnvironmentEdits(
+  before: string,
+  after: string,
+  edits: Op[],
+): Op[] {
+  const ranges: [number, number][] = [];
+  const scan = (text: string, side: 1 | 3) => {
+    const stack: { name: string; start: number; end: number }[] = [];
+    const endSide = side === 1 ? 2 : 4;
+    const touches = (op: Op, start: number, end: number) =>
+      op[side] === op[endSide]
+        ? start < op[side] && op[side] < end
+        : op[side] < end && start < op[endSide];
+    for (const match of structuralSource(text).matchAll(
+      /\\(begin|end)\s*\{([^}]+)\}/g,
+    )) {
+      const token = {
+        name: match[2],
+        start: match.index,
+        end: match.index + match[0].length,
+      };
+      if (match[1] === "begin") {
+        stack.push(token);
+        continue;
+      }
+      const opening = stack.at(-1);
+      if (!opening || opening.name !== token.name) continue;
+      stack.pop();
+      if (
+        !edits.some(
+          (op) =>
+            touches(op, opening.start, opening.end) ||
+            touches(op, token.start, token.end),
+        )
+      )
+        continue;
+      const members = edits.flatMap((op, index) =>
+        touches(op, opening.start, token.end) ? [index] : [],
+      );
+      if (members.length > 1) ranges.push([members[0], members.at(-1)!]);
+    }
+  };
+  scan(before, 1);
+  scan(after, 3);
+  ranges.sort((a, b) => a[0] - b[0]);
+  const merged: [number, number][] = [];
+  for (const range of ranges) {
+    const previous = merged.at(-1);
+    if (previous && range[0] <= previous[1])
+      previous[1] = Math.max(previous[1], range[1]);
+    else merged.push([...range]);
+  }
+  const result: Op[] = [];
+  for (let index = 0, group = 0; index < edits.length; index++) {
+    const range = merged[group],
+      first = edits[index];
+    if (!range || range[0] !== index) {
+      result.push(first);
+      continue;
+    }
+    const last = edits[range[1]];
+    result.push([
+      first[1] === last[2]
+        ? "insert"
+        : first[3] === last[4]
+          ? "delete"
+          : "replace",
+      first[1],
+      last[2],
+      first[3],
+      last[4],
+    ]);
+    index = range[1];
+    group++;
+  }
+  return result;
+}

engine/tests/ts/fine-diff.test.ts

@@ -140,6 +140,54 @@ it("keeps multiline math and arguments together even across unchanged source lin
   ).toBe(next);
 });
 
+it("keeps a rewritten display's nested environment separate from surrounding prose", () => {
+  const old = 'A small example.\n\\[\\begin{aligned}x &= 1 \\\\ y &= 2\\end{aligned}\\]\nThe result is useful.\n';
+  const next = old.replace('small', 'short').replace('y &= 2', 'y &= 3').replace('useful', 'practical');
+  f.write(METHOD, old);
+  const before = f.repo.snapshot(['paper']);
+  f.write(METHOD, next);
+  const units = diffUnits(f.repo, before, f.repo.snapshot(['paper']));
+  expect(units).toHaveLength(3);
+  expect(units[1].old_text).toBe('\\[\\begin{aligned}x &= 1 \\\\ y &= 2\\end{aligned}\\]');
+  expect(applyUnits(Buffer.from(old), Buffer.from(next), [units[0]])?.toString()).toBe(old.replace('small', 'short'));
+  expect(applyUnits(Buffer.from(old), Buffer.from(next), [units[2]])?.toString()).toBe(old.replace('useful', 'practical'));
+});
+
+for (const [label, oldBody, newBody] of [
+  ['remove a list', '\\begin{enumerate}\n\\item A fast result.\n\\item A stable estimate.\n\\end{enumerate}', 'A fast result.\nA stable estimate.'],
+  ['add a list', 'A fast result.\nA stable estimate.', '\\begin{enumerate}\n\\item A fast result.\n\\item A stable estimate.\n\\end{enumerate}'],
+  ['rename a list', '\\begin{enumerate}\n\\item A fast result.\n\\item A stable estimate.\n\\end{enumerate}', '\\begin{itemize}\n\\item A fast result.\n\\item A robust estimate.\n\\end{itemize}'],
+  ['remove a nested list', '\\begin{quote}\n\\begin{itemize}\n\\item A fast result.\n\\end{itemize}\n\\end{quote}', 'A fast result.'],
+]) it(`groups only the affected environment when edits ${label}`, () => {
+  const old = `A small example.\n\n${oldBody}\n\nThe result is useful.\n`;
+  const next = old.replace('small', 'short').replace(oldBody, newBody).replace('useful', 'practical');
+  f.write(METHOD, old);
+  const before = f.repo.snapshot(['paper']);
+  f.write(METHOD, next);
+  const units = diffUnits(f.repo, before, f.repo.snapshot(['paper']));
+  expect(units).toHaveLength(3);
+  for (let mask = 0; mask < 8; mask++) {
+    const selected = units.filter((_, index) => mask & (1 << index));
+    let expected = old;
+    if (mask & 1) expected = expected.replace('small', 'short');
+    if (mask & 2) expected = expected.replace(oldBody, newBody);
+    if (mask & 4) expected = expected.replace('useful', 'practical');
+    expect(applyUnits(Buffer.from(old), Buffer.from(next), selected)?.toString()).toBe(expected);
+    expect(revertUnits(Buffer.from(old), Buffer.from(next), units.filter(unit => !selected.includes(unit)))?.toString()).toBe(expected);
+  }
+});
+
+it("does not couple prose edits because of delimiters inside comments or literal examples", () => {
+  const old = 'A small example.\n% \\begin{enumerate}\nA useful result.\n\\verb|\\end{enumerate}|\nA stable estimate.\n';
+  const next = old.replace('small', 'short').replace('% \\begin{enumerate}', '% \\begin{itemize}').replace('useful', 'practical').replace('stable', 'robust');
+  f.write(METHOD, old);
+  const before = f.repo.snapshot(['paper']);
+  f.write(METHOD, next);
+  const units = diffUnits(f.repo, before, f.repo.snapshot(['paper']));
+  expect(units).toHaveLength(4);
+  expect(applyUnits(Buffer.from(old), Buffer.from(next), [units[2]])?.toString()).toBe(old.replace('useful', 'practical'));
+});
+
 it("does not attach a comment to another change on the same source line", async () => {
   const old = "The method is fast and the result is useful.\n";
   f.write(METHOD, old);

scripts/check-note-option-preparation.mjs

@@ -17,8 +17,19 @@ f.write('paper/main.tex', String.raw`\documentclass{book}
 \end{document}
 `);
 f.write(METHOD, String.raw`\chapter{Method}
+A small example.
+
+\[\begin{array}{c}x = 1\end{array}\]
+
 The model is fast and the estimates are stable. The surrounding context remains here.
 
+\begin{enumerate}
+\item First check.
+\item Second check.
+\end{enumerate}
+
+The surrounding discussion stays unchanged. It separates the independent observations below.
+
 We evaluate the model on three datasets. This is a different note.
 `);
 f.write('paper/sections/results.tex', '\\chapter{Results}\n' + 'Unchanged results and their explanation.\n\n'.repeat(120));
@@ -30,6 +41,9 @@ const task = app.tasks.create('options', 'Prepared note options', [], ['paper'],
 const source = f.read(METHOD);
 f.edit(task, METHOD, 'fast', 'efficient');
 f.edit(task, METHOD, 'three', 'five');
+f.edit(task, METHOD, 'small', 'short');
+f.edit(task, METHOD, 'x = 1', 'x = 2');
+f.edit(task, METHOD, '\\begin{enumerate}\n\\item First check.\n\\item Second check.\n\\end{enumerate}', 'First check.\nSecond check.');
 const originalBuild = Builder.prototype.build, builds = [];
 Builder.prototype.build = function (rev, targets, seed, options) {
   builds.push({ rev, targets });
@@ -51,7 +65,10 @@ try {
   assert.ok(builds.every(build => JSON.stringify(build.targets) === '["sections/method"]'), 'No whole-document compilation for options');
   const first = results[0], scope = noteComparisonScope(first, [selected.id]);
   assert.ok(scope.sections.length || scope.loose.length);
-  assert.ok(scope.units.every(unit => unit.new_text !== 'five'), 'The other note’s separate group stays outside the drawer');
+  assert.ok(scope.units.every(unit => unit.new_text !== 'five'), 'The other note’s separate group stays outside the drawer: ' + JSON.stringify(scope.units.map(unit => [unit.id, unit.new_text])));
+  assert.ok(scope.units.every(unit => unit.new_text !== 'short'), 'The leading edit does not appear in every note’s alternatives');
+  assert.equal(results[0].units.find(unit => unit.id === selected.id).new_text, 'quick');
+  assert.equal(results[1].units.find(unit => unit.id === selected.id).new_text, 'computationally efficient');
   assert.ok(first.units.some(unit => unit.new_text === 'five'), 'The complete candidate still preserves the other edit');
   const paths = results.map(detail => path.join(app.state, 'cache', detail.files.replace('/option-files/', 'option-previews/'), 'preview.json'));
   assert.ok(paths.every(file => fs.existsSync(file)), 'Both manifests persist on disk');

scripts/check-note-results.mjs

@@ -44,6 +44,11 @@ const strip = text => {
   const src = 'data:image/svg+xml,' + encodeURIComponent(`<svg xmlns="http://www.w3.org/2000/svg" width="600" height="30"><rect width="600" height="30" fill="white"/><text x="40" y="20" font-size="12">${text.replaceAll('&', '&amp;').replaceAll('<', '&lt;')}</text></svg>`);
   return { src, clean: src, width: 600, height: 30, page: 1, label: '1', text: [{ text, chars: Array.from(text, (text, index) => ({ text, rect: [40 + index * 5, 8, 45 + index * 5, 22] })) }] };
 };
+const sharedStrip = text => {
+  const image = strip(text);
+  const src = 'data:image/svg+xml,' + encodeURIComponent(`<svg xmlns="http://www.w3.org/2000/svg" width="600" height="800"><rect width="600" height="800" fill="white"/><text x="40" y="20">Unrelated opening</text><text x="40" y="412" font-size="12">${text.replaceAll('&', '&amp;').replaceAll('<', '&lt;')}</text><text x="40" y="720">Unrelated ending</text></svg>`);
+  return { ...image, src, clean: src, height: 800, text: image.text.map(line => ({ ...line, chars: line.chars.map(char => ({ ...char, rect: [char.rect[0], char.rect[1] + 392, char.rect[2], char.rect[3] + 392] })) })), marks: [{ unit: 'c1', kind: 'added', rect: [40, 400, 300, 414] }, { unit: 'other', kind: 'added', rect: [40, 700, 300, 720] }] };
+};
 await page.route('**/api/**', async route => {
   const url = new URL(route.request().url());
   if (url.pathname.startsWith('/review/')) return route.fallback();
@@ -58,7 +63,8 @@ await page.route('**/api/**', async route => {
     assert.equal(url.searchParams.get('comparison'), comparisonVersion);
     const round = Number(url.searchParams.get('round')), option = url.searchParams.get('option');
     const candidate = round === 2 ? [...units[round].map(unit => ({ ...unit, new_text: option === 'brief' ? alternateSource : unit.new_text })), unit('other', 80, 'Three datasets.', 'Five datasets.')] : units[round];
-    return route.fulfill({ json: { ...rounds[round], units: candidate, task: task.name, before: 'before', after: `candidate-${option}-${comparisonVersion}`, files: '', warnings: [], loose: [...candidate.map(unit => ({ kind: 'change', units: [unit.id], before: [strip(unit.old_text)], after: [strip(unit.new_text)] })), ...Array.from({ length: 5 }, (_, i) => ({ kind: 'change', units: ['space-' + i], identical: true, before: [], after: [] }))] } });
+    const blocks = round === 2 ? [{ kind: 'change', units: candidate.map(unit => unit.id), before: [strip('Unrelated leading strip'), sharedStrip(candidate[0].old_text)], after: [strip('Unrelated leading strip'), sharedStrip(candidate[0].new_text)] }] : candidate.map(unit => ({ kind: 'change', units: [unit.id], before: [strip(unit.old_text)], after: [strip(unit.new_text)] }));
+    return route.fulfill({ json: { ...rounds[round], units: candidate, task: task.name, before: 'before', after: `candidate-${option}-${comparisonVersion}`, files: '', warnings: [], loose: [...blocks, ...Array.from({ length: 5 }, (_, i) => ({ kind: 'change', units: ['space-' + i], identical: true, before: [], after: [] }))] } });
   }
   if (url.pathname.endsWith('/note-options')) {
     const body = route.request().method() !== 'GET' ? route.request().postDataJSON() : null;
@@ -132,6 +138,8 @@ try {
   assert.equal(await controls().locator('.note-option-reason').textContent(), 'Preserve the author’s technical term.', 'The preferred proposal uses the AI-provided title and reason');
   assert.equal(await controls().locator('.note-comparison-side-label').count(), 2, 'Only this group has before/after labels');
   assert.equal(await controls().locator('.note-comparison-change[data-units="other"]').count(), 0, 'Other groups remain in the main review');
+  await expect(controls().locator('.note-comparison-crop').first()).toHaveAttribute('data-crop-top', '376');
+  assert.ok((await controls().locator('.note-comparison-crop').first().boundingBox()).height < 200, 'The shared PDF opens at this note instead of a page of unrelated edits');
   // A shared engine strip must follow live workspace language changes even
   // though this page has no viewer ?lang parameter or workspace reload.
   const firstStrip = controls().locator('.pdf-strip .pdf-text').first();

src/server/note-results.ts

@@ -57,11 +57,26 @@ function quoteSource(text: string, start: number) {
 function quotedUnit(note: NoteSummary, unit: NoteRound['units'][number]): boolean | null {
   if (!unit.text_ops || !unit.old_context || !note.quote.trim()) return null;
   const { text, start } = unit.old_context, { normalized, letters } = quoteSource(text, start);
-  const quote = normalize(note.quote), at = normalized.indexOf(quote);
+  const quote = normalize(note.quote);
   // Exact units must fail closed when a quote cannot be resolved. Falling back
   // to its source line would silently accept unrelated edits on that line.
-  if (!quote || at < 0 || normalized.indexOf(quote, at + 1) >= 0) return false;
-  const low = letters[at], last = letters[at + quote.length - 1], high = last + String.fromCodePoint(text.codePointAt(last - start)!).length;
+  if (!quote) return false;
+  let occurrences: { low: number; high: number }[] = [];
+  for (let at = normalized.indexOf(quote); at >= 0; at = normalized.indexOf(quote, at + 1)) {
+    const last = letters[at + quote.length - 1];
+    occurrences.push({ low: letters[at], high: last + String.fromCodePoint(text.codePointAt(last - start)!).length });
+  }
+  if (occurrences.length > 1) {
+    // A short selection can occur elsewhere in the context excerpt. Original
+    // manuscript lines disambiguate it, but an older review's lines cannot.
+    const anchor = noteAnchor(note), origin = unit.text_ops[0]?.[1];
+    if (anchor.round !== undefined || origin === undefined || origin < start || origin > start + text.length) return false;
+    const newlines = (a: number, b: number) => (text.slice(a - start, b - start).match(/\n/g) || []).length;
+    const lineAt = (offset: number) => unit.old_start + (offset < origin ? -newlines(offset, origin) : newlines(origin, offset));
+    occurrences = occurrences.filter(({ low, high }) => anchor.sources.some(place => place.file === unit.file && Math.max(place.lines[0], lineAt(low)) <= Math.min(place.lines[1], lineAt(high - 1))));
+  }
+  if (occurrences.length !== 1) return false;
+  const { low, high } = occurrences[0];
   return unit.text_ops.some(([, a, b]) => a === b ? low <= a && a <= high : a < high && b > low);
 }
 

src/web/note-comparison-scope.ts

@@ -1,11 +1,34 @@
-import type { Block, RoundDetail } from '../../engine/src/types.ts';
+import type { Block, RoundDetail, Strip } from '../../engine/src/types.ts';
 
-/** The drawer compares the current note's visual groups, not every change in
- * the round. Keep their native PDF context and neighboring edits within the
- * same crop, but only the note's own units receive highlights. */
+const focusedMarks = (image: Strip, focus: Set<string>) => (image.marks || []).filter(mark => mark.kind !== 'context' && focus.has(mark.unit));
+
+/** Keep complete nearby text lines around the note's marks. Coordinates stay
+ * in the original PDF strip so selection and highlighting remain aligned. */
+export function noteComparisonCrop(image: Strip, focus: string[]) {
+  const marks = focusedMarks(image, new Set(focus));
+  if (!marks.length) return { top: 0, height: image.height };
+  let top = Math.max(0, Math.min(...marks.map(mark => mark.rect[1])) - 24);
+  let bottom = Math.min(image.height, Math.max(...marks.map(mark => mark.rect[3])) + 24);
+  for (const line of image.text || []) {
+    if (!line.chars.length) continue;
+    const low = Math.min(...line.chars.map(char => char.rect[1])), high = Math.max(...line.chars.map(char => char.rect[3]));
+    if (low < bottom && high > top) { top = Math.max(0, Math.min(top, low)); bottom = Math.min(image.height, Math.max(bottom, high)); }
+  }
+  return { top, height: Math.max(1, bottom - top) };
+}
+
+/** Use attributed marks to focus a note inside a shared PDF group. Older
+ * comparisons without attribution retain their original groups and context. */
 export function noteComparisonScope(detail: RoundDetail, focus: string[]) {
   const focused = new Set(focus);
+  const all = [...detail.sections.flatMap(section => section.blocks), ...detail.loose];
+  const precise = all.some(block => block.kind === 'change' && block.units.some(id => focused.has(id)) && [...block.before, ...block.after].some(image => focusedMarks(image, focused).length));
   const select = (blocks: Block[]) => {
+    if (precise) return blocks.flatMap<Block>(block => {
+      if (block.kind !== 'change' || !block.units.some(id => focused.has(id))) return [];
+      const before = block.before.filter(image => focusedMarks(image, focused).length), after = block.after.filter(image => focusedMarks(image, focused).length);
+      return before.length || after.length ? [{ ...block, units: block.units.filter(id => focused.has(id)), before, after }] : [];
+    });
     const included = new Set<number>();
     blocks.forEach((block, index) => {
       if (block.kind !== 'change' || !block.units.some(id => focused.has(id))) return;
@@ -19,7 +42,7 @@ export function noteComparisonScope(detail: RoundDetail, focus: string[]) {
   };
   const groups = [...detail.sections.flatMap(section => section.blocks), ...detail.loose]
     .filter(block => block.kind === 'change' && block.units.some(id => focused.has(id)));
-  const sourceIds = new Set([...focus, ...groups.flatMap(block => block.kind === 'change' ? block.units : [])]);
+  const sourceIds = new Set([...focus, ...(precise ? [] : groups.flatMap(block => block.kind === 'change' ? block.units : []))]);
   return {
     sections: detail.sections.map(section => ({ ...section, blocks: select(section.blocks) })).filter(section => section.blocks.length),
     loose: select(detail.loose),