feat(footnotes): author-inline footnotes + deterministic server canonicalization (#228) #232

Merged
vvzvlad merged 7 commits from feat/228-inline-footnotes into develop 2026-06-28 02:23:28 +03:00

7 Commits

Author SHA1 Message Date
a
c4ed4a4855 fix(footnotes): strip bare definitions on rebuild; MCP full-doc + zip-import canonicalize tests (#228)
Review #6 (approve-with-comments) follow-ups:
1. canonicalize step 7 now strips bare footnoteDefinitions at ANY depth
   (stripFootnoteDefinitionsDeep), not just footnotesList, in BOTH copies. A
   definition hand-authored outside a list (e.g. nested in a callout via a
   raw-JSON write path) was left in place while a copy was also added to the
   rebuilt list -> duplicate, idempotent, self-perpetuating. Runs only in the
   rebuild path (after the lists are stripped); the fast-path / placement-keep
   branch is untouched. Added a shared-corpus case (bare def nested in a callout)
   to pin it in both mirrors.
2. markdown-clipboard: removed the dead top-level footnoteReference check in
   canonicalizePastedFootnotes (an inline atom is never a top-level slice child;
   only the descendants scan can find it).

Test coverage:
4. New MCP binding tests (full-doc-write-canonicalize.test.mjs): update_page_json
   and copy_page_content canonicalize the persisted full doc, asserted via a new
   `replacePage` seam (symmetric to the existing `mutatePage` seam) so no live
   collab socket is needed. Routed both writers through the seam.
5. New server spec (file-import-task.service.footnote-canonicalize.spec.ts): the
   zip-import path (processGenericImport) canonicalizes footnotes — real
   markdown->HTML->JSON via a real ImportService over a temp-dir .md file, DB trx
   stubbed to capture the persisted page content. FileImportTaskService had no
   spec before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-28 01:39:25 +03:00
a
9c1f952b2f fix(footnotes): guard insert against nested/bare definitions, skip definitions-only paste, doc + reorder fixes (#228)
Must-fix:
- insertInlineFootnote could glue a footnoteReference inside an EXISTING
  definition (nested footnotesList, or a bare footnoteDefinition with no list
  wrapper), which canonicalize then dropped as an orphan — silently losing the
  definition's prose. Now: (a) the body/notes boundary is computed from the first
  top-level block that IS or CONTAINS (recursively) a footnotesList/
  footnoteDefinition, not just a top-level list; and (b) the insertNodesAfterAnchor
  core skips footnotesList/footnoteDefinition subtrees entirely (skipSubtreeTypes),
  so an anchor whose only match is inside a definition -> inserted:false (clean
  abort, no write). Added tests: nested-definition, bare-definition, and
  body-before-nested-list-still-inserts.
- editor-ext footnote-canonicalize header listed `markdownToProseMirror` among the
  canonicalizing MCP paths; it is the NON-canonicalizing primitive. Replaced with
  `markdownToProseMirrorCanonical` (+ note that the plain primitive is for comment
  bodies) and added copy_page_content.
- Client paste: canonicalizePastedFootnotes now skips a definitions-ONLY paste
  (no footnoteReference anywhere) — canonicalizing it would strip the
  reference-less list and yield an EMPTY paste. Added a test.

Suggestions:
- docmost_transform now runs validateDocStructure/validateDocUrls on the RAW
  transform output BEFORE canonicalizeFootnotes (mirrors updatePageJson), so a
  too-deep doc gives the intended max-depth error instead of a stack overflow.
- docmost_transform tool description now states the RESULT is footnote-canonical
  (dryRun diff may show tidy-ups; idempotent after first run).
- insertFootnote: dropped the dead `result ? … : undefined` ternaries and the
  `as any` casts (result is always set by the time we return; the not-found path
  throws and aborts mutatePage). `const r = result!;`.

Tests / architecture:
- Added a LIVE-plugin golden case: the real footnoteSyncPlugin leaves a list with
  non-empty content after it in place, and canonicalize agrees (placement parity
  is now a driven property, not a hand-set expected).
- Added generateFootnoteId uuidv7 shape + uniqueness test.
- Item 9: added the ENFORCEMENT-RULE comments at the server parseProsemirrorContent
  and the MCP canonicalizer header (any NEW full-doc persist path MUST canonicalize;
  fragments/append/prepend and comment bodies MUST NOT). Kept per-call-site over a
  brittle grep CI test (the replace-vs-fragment + comment-vs-page nuance makes a
  single wrapper unsafe).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 23:40:28 +03:00
a
3fd66b4245 fix(footnotes): don't canonicalize comment bodies (data loss); canonicalize only page write paths (#228)
Must-fix (REAL DATA LOSS):
- markdownToProseMirror is reused for COMMENT bodies (createComment/updateComment).
  It unconditionally canonicalized, so a comment carrying a standalone footnote
  definition ([^1]: text with no matching reference) had its whole footnotesList
  stripped (referenceIds.length===0 -> stripFootnotesListsDeep) — the text
  vanished. Fix: markdownToProseMirror no longer canonicalizes (content-preserving
  primitive); a new markdownToProseMirrorCanonical wraps it for the PAGE write
  paths (markdown import via importPageMarkdown, update_page markdown via
  updatePageContentRealtime). Comment callers keep the non-canonicalizing
  primitive. Updated the now-false header comment and added create/update-comment
  inline notes. Added collaboration tests: comment path PRESERVES a reference-less
  definition; page path still drops it AND still reorders real footnotes. Updated
  the page-import canonicalization test to use the canonical variant.

Suggestions / architecture:
- #2: collapsed transforms.footnoteDefinition onto the shared
  makeFootnoteDefinition factory (adds only the inner paragraph block id); kept
  the dependency direction transforms -> footnote-authoring (no circular import,
  mirror stays pure).
- #3: confirmed docmost_transform auto-canonicalization is documented (inline
  comment, tool description, CHANGELOG) — no code change.
- #4: copyPageContent is a FULL-document write (replacePageContent of a
  type:"doc"); added a defensive canonicalizeFootnotes pass (no-op on
  already-canonical source).
- CHANGELOG entry refined to list the FULL-document write paths (incl.
  copy_page_content) and to state canonicalization is NOT applied to comment
  bodies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 22:17:15 +03:00
a
a77a0bc92b 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>
2026-06-27 21:41:10 +03:00
a
07ebd8c63e fix(footnotes): address PR #232 review — fragment-safe canonicalization, plugin placement parity, dead-code removal (#228)
Must-fix:
- Move canonicalizeFootnotes OUT of parseProsemirrorContent. It now runs only
  on FULL writes (createPage, updatePageContent operation==='replace'), never on
  an append/prepend fragment (a fragment would lose definition-only footnotes or
  synthesize a bogus empty list). Add a server binding spec.
- Match the live plugin's list PLACEMENT: a single already-canonical
  footnotesList is left exactly where it sits (the plugin never repositions a
  sole correct list), so the first write no longer reorders content that follows
  the list. Applied to BOTH the editor-ext copy and the MCP mirror; pinned by a
  shared golden corpus case with content after the list.
- Fix MCP tool count 38 -> 39 (README x3, AGENTS.md) and the transformJs param
  help (add canonicalizeFootnotes/insertInlineFootnote).

Simplifications:
- Remove the dead duplicate re-id mechanism (deriveFootnoteId/suffix/occurrence)
  from the PURE canonicalizer in both copies — references are never renamed, so
  the derived ids were never requested; first-wins-drop is the real behaviour.
  This also makes the editor-ext footnote-util note about "no cross-package copy"
  true again.
- Remove the sentinel round-trip in insertInlineFootnote: a generalized
  insertNodesAfterAnchor core inserts the footnoteReference node directly.
- Drop the redundant per-definition deep clone in step 4 (shallow id-normalizing
  copy; out is already deep-cloned).

Docs / architecture:
- Correct the editor-ext copy's "It exists because…" header to its real
  consumers (server import, page.service create/update, client paste).
- Note markdownToProseMirror reuse for create/update comment in collaboration.ts.
- A: shared golden JSON corpus exercised by BOTH the editor-ext copy and the MCP
  mirror (footnote-corpus.ts / .mjs) so "the two copies behave identically" is
  checkable.
- C: split the MCP canonicalizer into a pure mirror + footnote-authoring.ts.
- B: import services persist via a different path, so left one-line consolidation
  comments at the call sites rather than folding (does not fall out cleanly).

Tests: insertFootnote wrapper guards + docmost_transform dryRun auto-canonicalize
(MCP mock), page.service create/update + append/prepend binding (server jest),
shared corpus incl. nested-container reference.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 20:23:16 +03:00
a
fa929c9e86 fix(footnotes): canonicalize footnotes on server import + markdown paste (#228)
The footnote canonicalizer was wired into the MCP and editor-ext write paths
but NOT into the server's user-facing markdown/HTML import paths, so importing
or pasting markdown with out-of-order, reused, or orphan footnotes did not
canonicalize -- the exact trigger bug #228 fixes was still reproduced on
import. markdownToHtml -> htmlToJson builds ProseMirror JSON directly and never
runs the editor's footnoteSyncPlugin, and that plugin does not reorder an
existing list, so the stored footnotes kept the source's physical definition
order, retained orphans, and did not collapse reused references.

Wire canonicalizeFootnotes (already exported from @docmost/editor-ext) into
every server markdown/HTML -> page-JSON seam, before persisting:
  - ImportService.importPage (REST single-file .md/.html import)
  - FileImportTaskService (zip import worker)
  - PageService.parseProsemirrorContent (API createPage / updatePageContent)

Also hook the client markdown paste: handlePaste applies a manual transaction
(returns true), bypassing transformPasted/footnoteSyncPlugin, so a pasted
out-of-order markdown footnote block would persist out of order.
canonicalizePastedFootnotes reorders a self-contained pasted block (one that
carries its own footnotesList) to reference order, deduped and orphan-free; it
is deliberately scoped to whole-block pastes so a reference-only paste that
reuses a footnote already defined in the target doc is left untouched.

canonicalizeFootnotes is pure, idempotent and shape-safe (a doc with no
footnotes is unchanged), so it is safe on every write path.

Residual: when a pasted block merges into a doc that already has footnotes,
ordering relative to the pre-existing footnotes is still governed by the live
sync plugin (which does not reorder across the boundary).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 17:10:41 +03:00
claude code agent 227
30cb9d293c feat(footnotes): inline authoring + deterministic server-side canonicalization
Make footnotes author-inline: the agent/tool inserts a footnote at its point
of use (anchor + text) and the numbering plus the bottom list are DERIVED
deterministically server-side. The agent has no access to footnotesList and
cannot desync — out-of-order lists, orphan definitions, and raw trailing
[^id] blocks become structurally impossible.

editor-ext:
- canonicalizeFootnotes(docJSON) -> docJSON: a pure, EditorView-free port of
  footnoteSyncPlugin's end-state. Distinct reference ids in document order are
  the source of truth; exactly one trailing footnotesList holds one definition
  per referenced id in reference order (reusing the existing node or
  synthesizing an empty one); orphans dropped; duplicate definitions resolved
  deterministically (first wins, never lost); idempotent.
- Unit tests + a golden parity suite: on every editor-reachable steady state
  the live footnoteSyncPlugin's JSON is a canonicalize no-op (byte-for-byte
  parity), and the canonicalizer additionally repairs the out-of-order list a
  non-editor write produces.

mcp:
- footnote-canonicalize.ts: behavioural mirror of the editor-ext canonicalizer
  (the MCP package is intentionally decoupled from the editor barrel, like
  footnote-lex/docmost-schema), plus footnoteContentKey for content dedup.
- Auto-canonicalize on EVERY write path: markdownToProseMirror (fixes import
  ordering), update_page_json, and after every docmost_transform. Idempotent,
  so it is a no-op when footnotes are already canonical.
- insert_footnote tool + insertInlineFootnote: anchor + markdown text -> a
  mark-safe footnoteReference and a content-dedup'd definition; the list and
  numbering are derived. Same-content footnotes reuse one number/definition.
- canonicalizeFootnotes + insertInlineFootnote exposed as docmost_transform
  sandbox helpers.

Tests: editor-ext 157 green; MCP 325 green; server + client tsc clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 06:35:25 +03:00