gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-10-26 22:37:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(collab): a multi-block patch into an open editor leaves the unchanged blocks between edits untouched (PLAN-3578)

Browse Source 
Retro CONVE-735 finding on #2056. planPatch replaced ONE span from the
first changed block to the last, so an unchanged block between two edits
was re-copied from the reparsed markdown, and what alignment treats as
equivalent (a link's title, bold/italic on whitespace) was lost.

planPatch now returns hunks: one per changed run of top-level blocks (the
gaps in an LCS of equal blocks, A vs B), each mapped to its aligned live
blocks under the existing rules. patchTransaction applies them last to
first in ONE transaction; applyPatch and ItemDetail's applier both use it.

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
xarmian
2026-10-11 21:13:18 +00:00
parent ccd915f1cc
commit b994d368ee
3 changed files with 135 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/collab/patchApply.svelte.test.ts
+44
View File
@@ -45,6 +45,50 @@ describe('PLAN-3578 step 2: narrowed apply', () => {
expect(ed.state.doc.textBetween(sel - 2, sel + 3)).toBe('gamma');
});
// Retro CONVE-735 finding on #2056: edits in separate blocks replaced the
// whole span between them, re-copying the unchanged middle blocks from the
// reparse and losing what alignment treats as equivalent (a link title).
it('leaves an unchanged block between two edited blocks untouched', () => {
const ed = createHeadlessEditor();
const s = ed.schema;
const titled = s.marks.link.create({ href: 'https://example.com', title: 'Keep me' });
const doc = s.node('doc', null, [
s.node('paragraph', null, [s.text('alpha')]),
s.node('paragraph', null, [s.text('see '), s.text('the site', [titled])]),
s.node('paragraph', null, [s.text('gamma')]),
]);
ed.view.dispatch(ed.state.tr.replaceWith(0, ed.state.doc.content.size, doc.content));
// The canonical form drops the title, as a flush may: the middle block
// differs from its reparse only by it, which alignment allows.
const dropTitle: PatchContext = { toCanonical: (md) => md.replace(/ "Keep me"/, ''), toEditor: (md) => md };
const middle = ed.state.doc.child(1);
const res = applyPatch(ed, [{ old: 'alpha', new: 'ALPHA' }, { old: 'gamma', new: 'GAMMA' }], dropTitle);
expect(res.ok).toBe(true);
expect(blocks(ed.state.doc).map((b) => b.textContent)).toEqual(['ALPHA', 'see the site', 'GAMMA']);
expect(ed.state.doc.child(1)).toBe(middle);
let title: unknown;
ed.state.doc.child(1).descendants((n) => {
const m = n.marks.find((mk) => mk.type.name === 'link');
if (m) title = m.attrs.title;
});
expect(title).toBe('Keep me');
});
it('applies an insertion and a later edit as separate runs, in one transaction', () => {
const ed = editorWith('one\n\ntwo\n\nthree\n\nfour');
const kept = [ed.state.doc.child(1), ed.state.doc.child(2)];
let dispatches = 0;
ed.on('transaction', ({ transaction }) => {
if (transaction.docChanged) dispatches++;
});
const res = applyPatch(ed, [{ old: 'one', new: 'zero\n\none' }, { old: 'four', new: 'FOUR' }], ctx);
expect(res.ok).toBe(true);
expect(dispatches).toBe(1);
expect(blocks(ed.state.doc).map((b) => b.textContent)).toEqual(['zero', 'one', 'two', 'three', 'FOUR']);
expect(ed.state.doc.child(2)).toBe(kept[0]);
expect(ed.state.doc.child(3)).toBe(kept[1]);
});
it('inserts a new block between unchanged neighbours', () => {
const ed = editorWith('first\n\nlast');
const first = ed.state.doc.child(0);
web/src/lib/collab/patchApply.ts
+88 -42
View File
@@ -16,11 +16,13 @@
// whitespace (invisible); a trailing empty live paragraph is ignored. A
// changed A block that aligns to no live block refuses unstable_roundtrip:
// replacing it would change something the agent never saw.
// 4. The changed top-level blocks (A vs B) replace exactly their aligned live
// blocks in one transaction. Never a whole-document replace.
// 4. Each changed run of top-level blocks (A vs B, gaps in an LCS of equal
// blocks) replaces exactly its aligned live blocks; all runs land in one
// transaction. Unchanged blocks between runs are never touched, and it is
// never a whole-document replace.
import type { Editor } from '@tiptap/core';
import { createDocument } from '@tiptap/core';
import { EditorState, type Plugin } from '@tiptap/pm/state';
import { EditorState, type Plugin, type Transaction } from '@tiptap/pm/state';
import type { Node as PMNode } from '@tiptap/pm/model';
export interface PatchEdit {
@@ -179,12 +181,57 @@ export interface PatchContext {
toEditor(stored: string): string;
}
/** One changed run of top-level blocks: live [from, to) becomes slice. */
export interface PatchHunk {
from: number;
to: number;
slice: PMNode[];
}
/**
* The changed runs of top-level blocks, A vs B: the gaps between an LCS of
* equal blocks. Each run is replaced on its own, so an unchanged block between
* two edits is never touched (retro CONVE-735 finding on #2056: one span from
* the first to the last change re-copied the blocks in between from the
* reparse, losing what alignment treats as equivalent, such as a link title).
* Block granularity on purpose; a character-level diff misplaces a block
* insertion that repeats its neighbour's text.
*/
function changedRuns(A: PMNode, B: PMNode): { aFrom: number; aTo: number; bFrom: number; bTo: number }[] {
const M = A.childCount;
const N = B.childCount;
const dp: number[][] = Array.from({ length: M + 1 }, () => new Array(N + 1).fill(0));
for (let i = M - 1; i >= 0; i--)
for (let j = N - 1; j >= 0; j--) dp[i][j] = A.child(i).eq(B.child(j)) ? dp[i + 1][j + 1] + 1 : Math.max(dp[i + 1][j], dp[i][j + 1]);
const runs: { aFrom: number; aTo: number; bFrom: number; bTo: number }[] = [];
let i = 0;
let j = 0;
let open: { aFrom: number; bFrom: number } | null = null;
const close = () => {
if (open) runs.push({ aFrom: open.aFrom, aTo: i, bFrom: open.bFrom, bTo: j });
open = null;
};
while (i < M || j < N) {
if (i < M && j < N && A.child(i).eq(B.child(j)) && dp[i][j] === dp[i + 1][j + 1] + 1) {
close();
i++;
j++;
continue;
}
if (!open) open = { aFrom: i, bFrom: j };
if (j >= N || (i < M && dp[i + 1][j] >= dp[i][j + 1])) i++;
else j++;
}
close();
return runs;
}
/** Plans the narrowed replace without touching the editor. */
export function planPatch(
editor: Editor,
edits: PatchEdit[],
ctx: PatchContext,
): { ok: true; before: string; after: string; from: number; to: number; slice: PMNode[] } | PatchRefusal {
): { ok: true; before: string; after: string; hunks: PatchHunk[] } | PatchRefusal {
const storage = (editor.storage as unknown as { markdown: { serializer: { serialize(d: PMNode): string } } }).markdown;
const live = editor.state.doc;
const before = ctx.toCanonical(storage.serializer.serialize(live));
@@ -195,58 +242,57 @@ export function planPatch(
const B = parseDetached(editor, ctx.toEditor(after));
if (!A || !B) return { ok: false, reason: 'parse_refused' };
// The changed block range, A vs B: the blocks between their longest
// common prefix and suffix of equal top-level blocks. Block granularity on
// purpose; a character-level diff (findDiffStart/End) misplaces a block
// insertion that repeats its neighbour's text.
const M = A.childCount;
const N = B.childCount;
let pre = 0;
while (pre < M && pre < N && A.child(pre).eq(B.child(pre))) pre++;
let suf = 0;
while (suf < M - pre && suf < N - pre && A.child(M - 1 - suf).eq(B.child(N - 1 - suf))) suf++;
const aFrom = pre;
const aTo = M - suf;
const bFrom = pre;
const bTo = N - suf;
if (aFrom === aTo && bFrom === bTo) return { ok: true, before, after, from: 0, to: 0, slice: [] }; // no visible change
const runs = changedRuns(A, B);
if (runs.length === 0) return { ok: true, before, after, hunks: [] }; // no visible change
const map = align(live, A);
let lFrom: number;
let lTo: number;
if (aTo > aFrom) {
const ls: number[] = [];
for (let j = aFrom; j < aTo; j++) {
const l = map.get(j);
if (l === undefined) return { ok: false, reason: 'unstable_roundtrip', line: lineOfBlock(A, j, before) };
ls.push(l);
}
for (let k = 1; k < ls.length; k++)
if (ls[k] !== ls[k - 1] + 1) return { ok: false, reason: 'unstable_roundtrip', line: lineOfBlock(A, aFrom + k, before) };
lFrom = ls[0];
lTo = ls[ls.length - 1] + 1;
} else {
// Pure insertion before A[aFrom]: anchor on its neighbours.
if (aFrom > 0) {
const hunks: PatchHunk[] = [];
for (const { aFrom, aTo, bFrom, bTo } of runs) {
let lFrom: number;
let lTo: number;
if (aTo > aFrom) {
const ls: number[] = [];
for (let j = aFrom; j < aTo; j++) {
const l = map.get(j);
if (l === undefined) return { ok: false, reason: 'unstable_roundtrip', line: lineOfBlock(A, j, before) };
ls.push(l);
}
for (let k = 1; k < ls.length; k++)
if (ls[k] !== ls[k - 1] + 1) return { ok: false, reason: 'unstable_roundtrip', line: lineOfBlock(A, aFrom + k, before) };
lFrom = ls[0];
lTo = ls[ls.length - 1] + 1;
} else if (aFrom > 0) {
// Pure insertion before A[aFrom]: anchor on the block before it.
const prev = map.get(aFrom - 1);
if (prev === undefined) return { ok: false, reason: 'unstable_roundtrip', line: lineOfBlock(A, aFrom - 1, before) };
lFrom = lTo = prev + 1;
} else {
lFrom = lTo = 0;
}
const slice: PMNode[] = [];
for (let j = bFrom; j < bTo; j++) slice.push(B.child(j));
hunks.push({ from: blockStart(live, lFrom), to: blockStart(live, lTo), slice });
}
const slice: PMNode[] = [];
for (let j = bFrom; j < bTo; j++) slice.push(B.child(j));
return { ok: true, before, after, from: blockStart(live, lFrom), to: blockStart(live, lTo), slice };
return { ok: true, before, after, hunks };
}
/**
* The plan's hunks as ONE transaction on state, or null when nothing changes.
* Applied last to first, so each hunk's positions (computed on the live doc
* the plan read) are still valid when it lands.
*/
export function patchTransaction(state: EditorState, hunks: PatchHunk[]): Transaction | null {
if (hunks.length === 0) return null;
const tr = state.tr;
for (let k = hunks.length - 1; k >= 0; k--) tr.replaceWith(hunks[k].from, hunks[k].to, hunks[k].slice);
return tr;
}
/** Plans and applies the patch to the live editor in ONE transaction. */
export function applyPatch(editor: Editor, edits: PatchEdit[], ctx: PatchContext): PatchApplied | PatchRefusal {
const plan = planPatch(editor, edits, ctx);
if (!plan.ok) return plan;
if (plan.from !== plan.to || plan.slice.length > 0) {
const tr = editor.state.tr.replaceWith(plan.from, plan.to, plan.slice);
editor.view.dispatch(tr);
}
const tr = patchTransaction(editor.state, plan.hunks);
if (tr) editor.view.dispatch(tr);
return { ok: true, before: plan.before, after: plan.after };
}
web/src/lib/components/items/ItemDetail.svelte
+3 -4
View File
@@ -73,7 +73,7 @@
import { beforeNavigate } from '$app/navigation';
import { relativeTime, wikiLinksToMarkdown, markdownToWikiLinks, cleanBrokenLinks, unescapeDocLinks } from '$lib/utils/markdown';
import { toEditorMarkdown } from '$lib/utils/markdown';
import { planPatch } from '$lib/collab/patchApply';
import { planPatch, patchTransaction } from '$lib/collab/patchApply';
import { canonicalEditorMarkdown } from '$lib/collab/canonicalMarkdown';
import { toastStore } from '$lib/stores/toast.svelte';
import { confirmDialog } from '$lib/stores/confirmDialog.svelte';
@@ -2834,9 +2834,8 @@
const plan = planPatch(editorInstance, edits, ctx);
if (!plan.ok) return plan;
openBracket();
if (plan.from !== plan.to || plan.slice.length > 0) {
editorInstance.view.dispatch(editorInstance.state.tr.replaceWith(plan.from, plan.to, plan.slice));
}
const tr = patchTransaction(editorInstance.state, plan.hunks);
if (tr) editorInstance.view.dispatch(tr);
toastStore.show('External edit applied', 'info');
return { ok: true, before: plan.before, after: plan.after };
} catch (err) {