fix(footnotes): re-review #232 — refuse footnoteRef into codeBlock/definition, deep-strip nested lists, docs + cross-copy guard (#228)
Must-fix: - REAL BUG: insertInlineFootnote could splice a footnoteReference (inline atom) into a codeBlock or an existing footnoteDefinition, persisting a schema-invalid doc (insert_footnote skips validateDocStructure). Now the search is bounded to the BODY (before the first footnotesList) and the insertNodesAfterAnchor core refuses textblocks that can't hold the atom (codeBlock); when the only match is in such a place the insert returns inserted:false and the write aborts cleanly. Reachable via docmost_transform too. Added codeBlock / definition / fall-through tests. - Fixed the deepEqualJson doc comment in both copies: arrays are order-SENSITIVE (correctness depends on it), only object keys are order-insensitive. - README.ru.md MCP tool count 38 -> 39 (lines 36/47/63), matching README.md/AGENTS. - CHANGELOG [Unreleased] Added entry for insert_footnote + server-side footnote canonicalization on non-editor write paths (#228). Suggestions: - canonicalize step 5/7 now strips footnotesList at ANY depth (both copies), so a schema-valid list nested in a callout/blockquote can't leave duplicate defs. - Exclude the test-only footnote-corpus.ts fixture from the editor-ext build (tsconfig), so it no longer ships in dist/. - Removed the duplicate manual canonicalize cases from the MCP unit test (the shared corpus covers them via full deepEqual); kept idempotence + immutability. - insertInlineFootnote dedup key now keys off the inline array directly (footnoteContentKey({ content: inline })) instead of a throwaway node. Tests / architecture: - New client-wrapper test (#9): overrides a small mutatePage seam to assert the not-found path throws and persists NOTHING, and the success path shapes footnoteId/reused/message/verify and writes the right content. Fixed the misleading comment in footnote-write.test.mjs. - B: cross-copy corpus parity guard test (loads both corpora, asserts deep-equal) so a typo in one copy can't pass both suites green. - A: declined — the full-vs-fragment decision lives at the call site, so a prepareDocForPersist wrapper would be a bare alias for canonicalizeFootnotes; kept the existing per-call-site comments instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -119,11 +119,9 @@ export function canonicalizeFootnotes<T = any>(doc: T): T {
|
||||
}
|
||||
}
|
||||
|
||||
// 5) No references -> there must be NO list at all.
|
||||
// 5) No references -> there must be NO list at all (at any depth).
|
||||
if (referenceIds.length === 0) {
|
||||
out.content = out.content.filter(
|
||||
(n: any) => !(n && n.type === FOOTNOTES_LIST_NAME),
|
||||
);
|
||||
stripFootnotesListsDeep(out);
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -147,13 +145,15 @@ export function canonicalizeFootnotes<T = any>(doc: T): T {
|
||||
return out;
|
||||
}
|
||||
|
||||
// 7) Otherwise rebuild: strip every footnotesList and re-insert exactly one
|
||||
// after the last meaningful (non-empty paragraph) block, so it coexists with
|
||||
// a trailing-node empty paragraph. This both repairs a non-canonical doc and
|
||||
// (in the import case) physically reorders the list into reference order.
|
||||
const top: any[] = out.content.filter(
|
||||
(n: any) => !(n && n.type === FOOTNOTES_LIST_NAME),
|
||||
);
|
||||
// 7) Otherwise rebuild: strip every footnotesList at ANY depth (collectDefinitions
|
||||
// gathers defs recursively, so a list nested in a callout/blockquote would
|
||||
// otherwise have its defs copied into the new list while the original
|
||||
// survives — duplicates) and re-insert exactly one after the last meaningful
|
||||
// (non-empty paragraph) top-level block, so it coexists with a trailing-node
|
||||
// empty paragraph. This both repairs a non-canonical doc and (in the import
|
||||
// case) physically reorders the list into reference order.
|
||||
stripFootnotesListsDeep(out);
|
||||
const top: any[] = out.content;
|
||||
let insertAt = top.length;
|
||||
while (insertAt > 0 && isEmptyParagraph(top[insertAt - 1])) insertAt--;
|
||||
top.splice(insertAt, 0, { type: FOOTNOTES_LIST_NAME, content: orderedDefs });
|
||||
@@ -161,10 +161,20 @@ export function canonicalizeFootnotes<T = any>(doc: T): T {
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Remove every `footnotesList` node at ANY depth (mutates the given clone). */
|
||||
function stripFootnotesListsDeep(node: any): void {
|
||||
if (!node || typeof node !== 'object' || !Array.isArray(node.content)) return;
|
||||
node.content = node.content.filter(
|
||||
(c: any) => !(c && c.type === FOOTNOTES_LIST_NAME),
|
||||
);
|
||||
for (const child of node.content) stripFootnotesListsDeep(child);
|
||||
}
|
||||
|
||||
/**
|
||||
* Order-insensitive deep equality over plain JSON (objects/arrays/primitives).
|
||||
* Used to detect an already-canonical footnotesList so its physical position is
|
||||
* preserved (placement parity with the live plugin).
|
||||
* Deep equality over plain JSON: arrays are compared POSITIONALLY
|
||||
* (order-SENSITIVE), object keys order-insensitively. The array order-sensitivity
|
||||
* is required for correctness here — a reordered `footnotesList.content` must
|
||||
* compare UNEQUAL so the canonical rebuild fires instead of leaving it in place.
|
||||
*/
|
||||
function deepEqualJson(a: any, b: any): boolean {
|
||||
if (a === b) return true;
|
||||
|
||||
Reference in New Issue
Block a user