vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

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
Conversation 1 Commits 7 Files Changed 37 +6077 -75
Ghost commented 2026-06-27 06:37:01 +03:00
Copy Link

Closes #228.

Сноски стали авторскими-инлайн: агент даёт только ГДЕ (anchor) + ЧТО (текст), а нумерация и список внизу выводятся детерминированно на сервере — у агента нет доступа к footnotesList, рассинхрон невозможен. + дедуп по содержимому (одинаковый текст → один номер/определение, несколько ссылок).

  • Канонизатор footnote-canonicalize.ts (editor-ext, pure) + зеркало в packages/mcp (mcp намеренно развязан с editor-ext-барелем): нумерация по порядку первой ссылки, один definition на id в порядке ссылок, сироты дропаются, идемпотентно. Golden-тест: стабильные состояния live-плагина — байт-в-байт no-op канонизатора.
  • insert_footnote tool (pageId/anchorText/text), дедуп по footnoteContentKey, атомарно.
  • Авто-канонизация на ВСЕХ write-путях (markdown-import — чинит «вразнобой», updatePageJson, после каждого docmost_transform).

Тесты: editor-ext 157, mcp 325 (вкл. e2e markdown-import ordering fix). tsc (server+client) 0. Deferred (Phase 4 по issue): normalize_footnotes для литералов в уже-сохранённом JSON, git-sync footnote support.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

Closes #228. Сноски стали **авторскими-инлайн**: агент даёт только ГДЕ (anchor) + ЧТО (текст), а нумерация и список внизу выводятся **детерминированно на сервере** — у агента нет доступа к `footnotesList`, рассинхрон невозможен. + дедуп по содержимому (одинаковый текст → один номер/определение, несколько ссылок). - Канонизатор `footnote-canonicalize.ts` (editor-ext, pure) + зеркало в `packages/mcp` (mcp намеренно развязан с editor-ext-барелем): нумерация по порядку первой ссылки, один definition на id в порядке ссылок, сироты дропаются, идемпотентно. Golden-тест: стабильные состояния live-плагина — байт-в-байт no-op канонизатора. - `insert_footnote` tool (`pageId/anchorText/text`), дедуп по `footnoteContentKey`, атомарно. - Авто-канонизация на ВСЕХ write-путях (markdown-import — чинит «вразнобой», updatePageJson, после каждого docmost_transform). Тесты: editor-ext 157, mcp 325 (вкл. e2e markdown-import ordering fix). tsc (server+client) 0. Deferred (Phase 4 по issue): `normalize_footnotes` для литералов в уже-сохранённом JSON, git-sync footnote support. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-06-27 06:37:01 +03:00
feat(footnotes): inline authoring + deterministic server-side canonicalization 30cb9d293c 
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>
Ghost added 1 commit 2026-06-27 17:11:00 +03:00
fix(footnotes): canonicalize footnotes on server import + markdown paste (#228) fa929c9e86 
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>
Ghost added 1 commit 2026-06-27 20:23:40 +03:00
fix(footnotes): address PR #232 review — fragment-safe canonicalization, plugin placement parity, dead-code removal (#228) 07ebd8c63e 
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>
Ghost added 1 commit 2026-06-27 21:41:34 +03:00
fix(footnotes): re-review #232 — refuse footnoteRef into codeBlock/definition, deep-strip nested lists, docs + cross-copy guard (#228) a77a0bc92b 
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>
Ghost added 1 commit 2026-06-27 22:17:38 +03:00
fix(footnotes): don't canonicalize comment bodies (data loss); canonicalize only page write paths (#228) 3fd66b4245 
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>
Ghost added 1 commit 2026-06-27 23:40:51 +03:00
fix(footnotes): guard insert against nested/bare definitions, skip definitions-only paste, doc + reorder fixes (#228) 9c1f952b2f 
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>
vvzvlad commented 2026-06-28 01:10:35 +03:00
Owner
Copy Link

Code review — author-inline footnotes + детерминированная серверная канонизация (#228)

Вердикт: Approve with comments (одобрить с замечаниями). База — develop (merge-base 904f7b4), 35 файлов, +5584/−72. Полный обзор по 8 аспектам (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture). Блокирующих находок нет, критических нет.

Код продуман и тяжело документирован, инвариант сносок энфорсится во всех full-document write-путях; рефакторинг insertMarkerAfter → insertNodesAfterAnchor побайтово сохраняет старое поведение; канонизатор — строгий no-op для контента без сносок; закоммиченный packages/mcp/build/*.js побайтово совпадает со свежим tsc; счётчик тулов 38→39 корректен. Прежде чем мержить, стоит закрыть один узкий дефект (ниже) — он не блокирует, но это реальная регрессия контента.

Ревью охватывает только diff PR против develop и то, чего он касается. Сгенерированные packages/mcp/build/*.js (намеренно закоммиченный бандл — проверено, в синхроне с src/) и тест-фикстуры footnote-corpus.{ts,mjs} глубоко не ревьюились как продакшен-логика.

Must fix before merge

  • [warning][stability] Удаляйте «осиротевший» footnoteDefinition вне footnotesList, иначе rebuild его дублирует — packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts:157-161 (и зеркало packages/mcp/src/lib/footnote-canonicalize.ts:~157).
    В шаге 7 stripFootnotesListsDeep удаляет только узлы footnotesList, но collectDefinitions собирает определения рекурсивно на любой глубине. Если определение лежит «голым» вне списка (например, вложено в callout), оригинал остаётся на месте, а копия попадает в пересобранный нижний список — определение дублируется (две ноды с одним id, идемпотентно, само не чинится). Воспроизведено эмпирически на собранном канонизаторе; PMNode.fromJSON такой документ принимает. Достижимо только через raw-JSON write-пути (update_page_json / docmost_transform), где агент вручную авторит footnoteDefinition вне списка: схема callout — block+, а footnoteDefinition не входит в группу block, поэтому обычный редактор/импорт/вставка такую форму не порождают. Предусловие надуманное → non-blocking, но это реальная порча контента, которую вносит именно это изменение.
    Fix: после stripFootnotesListsDeep(out) в шаге 7 (в обеих копиях) дополнительно вырезать любые уцелевшие footnoteDefinition на всех глубинах (компаньон stripFootnoteDefinitionsDeep), и добавить корпус-кейс с голым определением во callout, чтобы запинить поведение в обоих зеркалах.

  • [suggestion][simplification] Уберите недостижимую проверку top-level footnoteReference в canonicalizePastedFootnotes — apps/client/src/features/editor/extensions/markdown-clipboard.ts:184-190.
    Строка if (node.type.name === FOOTNOTE_REFERENCE_NAME) hasReference = true; внутри top-level forEach никогда не срабатывает: функция выходит раньше, если openStart/openEnd !== 0, поэтому верхние дети slice — целые блоки, а footnoteReference — inline, atom, ловится только последующим node.descendants(...). Редундантная защитная строка, внесённая этим diff.
    Fix: удалить её, оставив скан descendants и top-level проверку hasFootnotesList.

Test coverage

Покрытие нового кода в целом очень хорошее: canonicalizeFootnotes (порядок по ссылкам, first-wins дедуп, дроп сирот, идемпотентность, fast-path «уже канонично», strip-all при отсутствии ссылок, вложенные ссылки, размещение перед хвостовым пустым параграфом) закрыт общим golden-корпусом в обоих пакетах + golden-parity с live-плагином; insertInlineFootnote закрыт исчерпывающе (дедуп по контенту, новый id, anchor-not-found, forbidBlockTypes/codeBlock, skipSubtreeTypes/refusal, граница beforeBlock); клиентская обёртка insertFootnote — через сим mutatePage; markdownToProseMirrorCanonical vs markdownToProseMirror (тела комментариев НЕ канонизируются) — обе ветки; page.service replace-канонизирует / append-prepend нет; паритет editor-ext↔mcp. Конкретные пробелы (все non-blocking):

  • Нет теста на «голый» footnoteDefinition вне footnotesList — та же ветка, что и warning выше; сейчас она и не тестируется, и ведёт себя неверно. Добавить кейс при починке.
  • Биндинги update_page_json и copy_page_content в MCP не покрыты (packages/mcp/src/client.ts:1354 и copyPageContent): однострочные идемпотентные вызовы уже хорошо протестированной чистой функции, но серверная сторона свои аналоги пинит фокус-тестами — стоит добавить симметричные на MCP-стороне через сим mutatePage.
  • Zip-импорт FileImportTaskService не покрыт (apps/server/src/integrations/import/services/file-import-task.service.ts:496-511): тот же canonicalizeFootnotes-биндинг, что и в import.service (он покрыт), но у FileImportTaskService спеки нет вовсе.

Примечание: новые серверные *.spec.ts — Jest с настроенным moduleNameMapper (@docmost/db/*), не vitest; инфраструктура тестов этим PR не менялась.

Architecture & design

2. ~8 разбросанных вызовов canonicalizeFootnotes, гарантируемых только комментарием-«enforcement rule» + различием replace-vs-fragment. Единый choke-point невозможен начисто: пути персистят через три несвязанных слоя (REST PageService→collab gateway; два импорт-сервиса в обход PageService через pageRepo.insertPage; MCP-клиент через collab-WebSocket mutatePageContent, который зовётся из ~12 мест, большинство — fragment-операции, где канонизация запрещена). PR уже сделал правильную частичную консолидацию (markdownToProseMirrorCanonical). Остаточный риск — будущий новый full-doc путь, который забудет вызов (тихий сбой: сноски не по порядку).
статус-кво + регресс-тест на каждую известную full-doc точку входа* (effort: s). Pros: превращает «помни правило» в проверяемое свойство. Cons: новый путь всё ещё полагается на чтение комментария.

## Code review — author-inline footnotes + детерминированная серверная канонизация (#228) **Вердикт: Approve with comments (одобрить с замечаниями).** База — `develop` (merge-base `904f7b4`), 35 файлов, +5584/−72. Полный обзор по 8 аспектам (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture). Блокирующих находок нет, критических нет. Код продуман и тяжело документирован, инвариант сносок энфорсится во всех full-document write-путях; рефакторинг `insertMarkerAfter` → `insertNodesAfterAnchor` побайтово сохраняет старое поведение; канонизатор — строгий no-op для контента без сносок; закоммиченный `packages/mcp/build/*.js` побайтово совпадает со свежим `tsc`; счётчик тулов 38→39 корректен. Прежде чем мержить, стоит закрыть один узкий дефект (ниже) — он не блокирует, но это реальная регрессия контента. > Ревью охватывает только diff PR против `develop` и то, чего он касается. Сгенерированные `packages/mcp/build/*.js` (намеренно закоммиченный бандл — проверено, в синхроне с `src/`) и тест-фикстуры `footnote-corpus.{ts,mjs}` глубоко не ревьюились как продакшен-логика. ### Must fix before merge - **[warning][stability] Удаляйте «осиротевший» `footnoteDefinition` вне `footnotesList`, иначе rebuild его дублирует** — `packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts:157-161` (и зеркало `packages/mcp/src/lib/footnote-canonicalize.ts:~157`). В шаге 7 `stripFootnotesListsDeep` удаляет только узлы `footnotesList`, но `collectDefinitions` собирает определения рекурсивно на любой глубине. Если определение лежит «голым» вне списка (например, вложено в `callout`), оригинал остаётся на месте, а копия попадает в пересобранный нижний список — определение дублируется (две ноды с одним `id`, идемпотентно, само не чинится). Воспроизведено эмпирически на собранном канонизаторе; `PMNode.fromJSON` такой документ принимает. Достижимо **только** через raw-JSON write-пути (`update_page_json` / `docmost_transform`), где агент вручную авторит `footnoteDefinition` вне списка: схема `callout` — `block+`, а `footnoteDefinition` не входит в группу `block`, поэтому обычный редактор/импорт/вставка такую форму не порождают. Предусловие надуманное → non-blocking, но это реальная порча контента, которую вносит именно это изменение. **Fix:** после `stripFootnotesListsDeep(out)` в шаге 7 (в обеих копиях) дополнительно вырезать любые уцелевшие `footnoteDefinition` на всех глубинах (компаньон `stripFootnoteDefinitionsDeep`), и добавить корпус-кейс с голым определением во `callout`, чтобы запинить поведение в обоих зеркалах. - **[suggestion][simplification] Уберите недостижимую проверку top-level `footnoteReference` в `canonicalizePastedFootnotes`** — `apps/client/src/features/editor/extensions/markdown-clipboard.ts:184-190`. Строка `if (node.type.name === FOOTNOTE_REFERENCE_NAME) hasReference = true;` внутри top-level `forEach` никогда не срабатывает: функция выходит раньше, если `openStart/openEnd !== 0`, поэтому верхние дети slice — целые блоки, а `footnoteReference` — `inline, atom`, ловится только последующим `node.descendants(...)`. Редундантная защитная строка, внесённая этим diff. **Fix:** удалить её, оставив скан `descendants` и top-level проверку `hasFootnotesList`. ### Test coverage Покрытие нового кода в целом очень хорошее: `canonicalizeFootnotes` (порядок по ссылкам, first-wins дедуп, дроп сирот, идемпотентность, fast-path «уже канонично», strip-all при отсутствии ссылок, вложенные ссылки, размещение перед хвостовым пустым параграфом) закрыт общим golden-корпусом в обоих пакетах + golden-parity с live-плагином; `insertInlineFootnote` закрыт исчерпывающе (дедуп по контенту, новый id, anchor-not-found, `forbidBlockTypes`/codeBlock, `skipSubtreeTypes`/refusal, граница `beforeBlock`); клиентская обёртка `insertFootnote` — через сим `mutatePage`; `markdownToProseMirrorCanonical` vs `markdownToProseMirror` (тела комментариев НЕ канонизируются) — обе ветки; `page.service` replace-канонизирует / append-prepend нет; паритет editor-ext↔mcp. Конкретные пробелы (все non-blocking): - **Нет теста на «голый» `footnoteDefinition` вне `footnotesList`** — та же ветка, что и warning выше; сейчас она и не тестируется, и ведёт себя неверно. Добавить кейс при починке. - **Биндинги `update_page_json` и `copy_page_content` в MCP не покрыты** (`packages/mcp/src/client.ts:1354` и `copyPageContent`): однострочные идемпотентные вызовы уже хорошо протестированной чистой функции, но серверная сторона свои аналоги пинит фокус-тестами — стоит добавить симметричные на MCP-стороне через сим `mutatePage`. - **Zip-импорт `FileImportTaskService` не покрыт** (`apps/server/src/integrations/import/services/file-import-task.service.ts:496-511`): тот же `canonicalizeFootnotes`-биндинг, что и в `import.service` (он покрыт), но у `FileImportTaskService` спеки нет вовсе. > Примечание: новые серверные `*.spec.ts` — Jest с настроенным `moduleNameMapper` (`@docmost/db/*`), не vitest; инфраструктура тестов этим PR не менялась. ### Architecture & design **2. ~8 разбросанных вызовов `canonicalizeFootnotes`, гарантируемых только комментарием-«enforcement rule» + различием replace-vs-fragment.** Единый choke-point невозможен начисто: пути персистят через три несвязанных слоя (REST `PageService`→collab gateway; два импорт-сервиса в обход `PageService` через `pageRepo.insertPage`; MCP-клиент через collab-WebSocket `mutatePageContent`, который зовётся из ~12 мест, большинство — fragment-операции, где канонизация запрещена). PR уже сделал правильную частичную консолидацию (`markdownToProseMirrorCanonical`). Остаточный риск — будущий новый full-doc путь, который забудет вызов (тихий сбой: сноски не по порядку). статус-кво + регресс-тест на каждую известную full-doc точку входа* (effort: s). Pros: превращает «помни правило» в проверяемое свойство. Cons: новый путь всё ещё полагается на чтение комментария.
Ghost added 1 commit 2026-06-28 01:39:48 +03:00
fix(footnotes): strip bare definitions on rebuild; MCP full-doc + zip-import canonicalize tests (#228) c4ed4a4855 
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>
vvzvlad merged commit c5109aa2a3 into develop 2026-06-28 02:23:28 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#232
Delete Branch "feat/228-inline-footnotes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?