Compare commits

..

2 Commits

Author SHA1 Message Date
agent_coder db2ed95ea6 fix(mcp): не нормализовать текст под маркой code в сносках (порча code-литералов)
Ревью #422: безусловная нормализация переписывала в ASCII и текст inline-code
внутри сносок (кавычки/тире/спецпробелы) — для code-литерала это не типографика,
а изменение смысла (эмпирически на raw-JSON путях: code-нода "a—b «x»" -> "a-b").
normalizeDefinitionText теперь пропускает текст-ноды с маркой code (verbatim), и
краевой trim определения тоже не трогает крайние code-ноды. footnoteMergeKey
читает сырой текст code-нод -> сноски, различающиеся только глифами в коде, НЕ
сливаются, а форки в прозе по-прежнему сливаются. Тесты: code verbatim +
непослияние по code-глифам.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 23:46:00 +03:00
agent_coder 845ef9af3d feat(mcp): детерминированная нормализация текста сносок + склейка форков по нормализованному тексту
Новый чистый модуль footnote-normalize-merge.ts (normalizeAndMergeFootnotes):
нормализует текст определений сносок (типографские кавычки->ASCII, тире->'-',
NBSP/спецпробелы->пробел, схлопывание пробелов, trim) и сливает определения с
совпавшим нормализованным текстом, перевешивая ссылки на канонический id;
дубли-сироты добивает canonicalizeFootnotes. Ключ слияния attrs-aware
(footnoteMergeKey/stableAttrs) — сноски с одинаковым текстом, но разными attrs
марок (напр. link.href) НЕ сливаются (защита от потери target). Пасс вызывается
строго ПЕРЕД canonicalizeFootnotes на 5 write-путях MCP (markdown-импорт,
updatePageJson, copyPageContent, docmost_transform, insertInlineFootnote).
Глиф-карты продублированы из comment-anchor.ts (там private+завязаны на golden).
Идемпотентен, чистый (deep-clone), scope строго внутри footnoteDefinition.

closes #419

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 23:24:11 +03:00
8 changed files with 657 additions and 242 deletions
@@ -148,53 +148,6 @@ describe('assistantParts', () => {
expect(toolPart).not.toHaveProperty('output');
});
it('replays the REAL error text for a THROWN tool (tool-error part)', () => {
const steps = [
{
text: '',
toolCalls: [
{ toolCallId: 'c7', toolName: 'editPageText', input: { id: 'p1' } },
],
// A thrown tool is a `tool-error` content part; toolResults holds only
// successes and stays empty for this call.
toolResults: [],
content: [
{
type: 'tool-error',
toolCallId: 'c7',
toolName: 'editPageText',
input: { id: 'p1' },
error: new Error('page is locked'),
},
],
},
];
const parts = assistantParts(steps, '') as AnyPart[];
const toolPart = parts.find((p) => p.type === 'tool-editPageText');
expect(toolPart).toBeDefined();
expect(toolPart!.state).toBe('output-error');
// The REAL error is replayed, NOT the 'Tool call did not complete.' placeholder.
expect(toolPart!.errorText).toBe('page is locked');
expect(toolPart).not.toHaveProperty('output');
});
it('keeps the placeholder ONLY for a call with neither result nor tool-error', () => {
const steps = [
{
text: '',
toolCalls: [
{ toolCallId: 'c8', toolName: 'insertNode', input: { node: {} } },
],
toolResults: [],
content: [], // aborted mid-step: no result AND no tool-error
},
];
const parts = assistantParts(steps, '') as AnyPart[];
const toolPart = parts.find((p) => p.type === 'tool-insertNode');
expect(toolPart!.state).toBe('output-error');
expect(toolPart!.errorText).toBe('Tool call did not complete.');
});
it('skips malformed tool-calls (missing toolName or toolCallId)', () => {
const steps = [
{
@@ -242,45 +195,6 @@ describe('serializeSteps', () => {
expect(trace[0]).toEqual({ toolName: 'getPage', input: { id: 'p1' } });
expect(trace[1]).toEqual({ toolName: 'getPage', output: { title: 'T' } });
});
it('records a THROWN tool failure (tool-error part) with its error message', () => {
const trace = serializeSteps([
{
toolCalls: [{ toolName: 'editPageText', input: { id: 'p1' } }],
toolResults: [],
content: [
{
type: 'tool-error',
toolName: 'editPageText',
error: new Error('page is locked'),
},
],
},
]) as Array<Record<string, unknown>>;
// The call element is followed by a paired error element (mirroring how a
// successful result is appended), so the failure survives in the trace.
expect(trace).toHaveLength(2);
expect(trace[0]).toEqual({ toolName: 'editPageText', input: { id: 'p1' } });
expect(trace[1]).toEqual({
toolName: 'editPageText',
error: 'page is locked',
});
});
it('truncates a very long tool-error message to the tool-output limit', () => {
const long = 'x'.repeat(5000);
const trace = serializeSteps([
{
toolCalls: [{ toolName: 'editPageText', input: {} }],
toolResults: [],
content: [{ type: 'tool-error', toolName: 'editPageText', error: long }],
},
]) as Array<Record<string, unknown>>;
const errorText = trace[1].error as string;
// Truncated (not the full 5000 chars) and carries the omission marker.
expect(errorText.length).toBeLessThan(long.length);
expect(errorText).toContain('chars omitted');
});
});
describe('rowToUiMessage', () => {
@@ -1637,17 +1637,6 @@ type StepLike = {
toolName?: string;
output?: unknown;
}>;
// ai@6.0.134: a tool that THREW surfaces as a `tool-error` content part
// ({ type:'tool-error', toolCallId, toolName, input, error }), NOT as a
// `toolResults` entry (which holds only successes). Read from here so failed
// calls are persisted with their real error instead of being dropped.
content?: ReadonlyArray<{
type?: string;
toolCallId?: string;
toolName?: string;
input?: unknown;
error?: unknown;
}>;
};
/**
@@ -1750,26 +1739,6 @@ function compactValue(value: unknown, depth: number): unknown {
return value;
}
/**
* Extract a bounded string message from a `tool-error` part's `error` field for
* persistence and history replay. The field may be an `Error`, a string, or an
* arbitrary object, so pull a message robustly. The result is passed through
* `compactValue` so a very long error honors the SAME truncation limits the file
* already applies to tool outputs (no new limit is introduced here).
*/
function normalizeToolError(error: unknown): string {
const message =
error instanceof Error
? error.message
: typeof error === 'string'
? error
: error != null &&
typeof (error as { message?: unknown }).message === 'string'
? (error as { message: string }).message
: String(error);
return compactValue(message, 0) as string;
}
/**
* Rebuild the FULL UIMessage `parts` for an assistant turn from the SDK steps,
* so multi-turn history replays prior tool-calls/results to the model (not just
@@ -1802,14 +1771,6 @@ export function assistantParts(
for (const r of step.toolResults ?? []) {
if (r.toolCallId) resultsById.set(r.toolCallId, r.output);
}
// Index this step's THROWN tool failures (ai@6 `tool-error` content parts)
// by tool call id, so a call that failed replays with its real error text.
const errorsById = new Map<string, unknown>();
for (const part of step.content ?? []) {
if (part.type === 'tool-error' && part.toolCallId) {
errorsById.set(part.toolCallId, part.error);
}
}
for (const call of step.toolCalls ?? []) {
if (!call.toolName || !call.toolCallId) continue;
const hasResult = resultsById.has(call.toolCallId);
@@ -1822,21 +1783,9 @@ export function assistantParts(
input: call.input,
output: compactToolOutput(resultsById.get(call.toolCallId)),
});
} else if (errorsById.has(call.toolCallId)) {
// The tool THREW: replay the REAL error so the model on the next turn
// knows WHY the call failed (and does not blindly repeat it). An
// output-error round-trips through convertToModelMessages as a balanced
// tool-call + tool-result, keeping the rebuilt history valid.
parts.push({
type: `tool-${call.toolName}`,
toolCallId: call.toolCallId,
state: 'output-error',
input: call.input,
errorText: normalizeToolError(errorsById.get(call.toolCallId)),
});
} else {
// No paired result AND no tool-error (e.g. aborted mid-step). Persisting
// a bare tool-call (input-available) would replay as an unpaired call and
// No paired result (e.g. aborted mid-step). Persisting a bare
// tool-call (input-available) would replay as an unpaired call and
// throw MissingToolResultsError on the next turn (convertToModelMessages
// emits no tool-result for it). Emit a SYNTHETIC paired result instead:
// an output-error round-trips through convertToModelMessages as a
@@ -2072,19 +2021,10 @@ export function serializeSteps(
steps: ReadonlyArray<{
toolCalls?: ReadonlyArray<{ toolName?: string; input?: unknown }>;
toolResults?: ReadonlyArray<{ toolName?: string; output?: unknown }>;
content?: ReadonlyArray<{
type?: string;
toolName?: string;
error?: unknown;
}>;
}>,
): unknown {
const calls: Array<{
toolName?: string;
input?: unknown;
output?: unknown;
error?: string;
}> = [];
const calls: Array<{ toolName?: string; input?: unknown; output?: unknown }> =
[];
for (const step of steps ?? []) {
for (const call of step.toolCalls ?? []) {
calls.push({ toolName: call.toolName, input: call.input });
@@ -2092,18 +2032,6 @@ export function serializeSteps(
for (const r of step.toolResults ?? []) {
calls.push({ toolName: r.toolName, output: compactToolOutput(r.output) });
}
// ai@6 surfaces a THROWN tool failure as a `tool-error` content part, NOT as
// a `toolResults` entry. Record it as its own paired element (mirroring how a
// successful result is appended) so the failure and its reason survive in the
// trace instead of leaving an orphaned call with no result.
for (const part of step.content ?? []) {
if (part.type === 'tool-error') {
calls.push({
toolName: part.toolName,
error: normalizeToolError(part.error),
});
}
}
}
return calls.length > 0 ? calls : null;
}
+39 -77
View File
@@ -13,14 +13,12 @@ Read the **Gotchas** section before you trust any error count.
- Agent chats live in Postgres, DB `docmost`, tables `ai_chat_*`.
- Each tool invocation is stored as **two** array elements (a `tool-call` part and
a `tool-result` part), so naive counting double-counts.
- **A tool that *throws* writes no result part.** Since the #407 fix its error is
persisted as a dedicated `{toolName, error}` element in `tool_calls` (queryable +
replayed to the model). **Rows written before #407 still drop it** — the error is
nowhere in the DB and shows only in the live UI. So `isError` / `success=false`
scans under-report by design, and pre-#407 thrown errors are invisible.
- To find where agents fail: (1) soft-failure markers in `tool_calls`, (2) the new
`error` field for thrown errors (new rows) / the orphan-gap proxy (old rows),
(3) server logs / the live UI for full stack traces beyond the truncated message.
- **A tool that *throws* writes no result part at all.** Its error text is nowhere
in the DB — not in `tool_calls`, `content`, or `metadata`. It is shown live in
the UI only. So `isError` / `success=false` scans under-report by design.
- To find where agents fail you need **three** sources: (1) soft-failure markers in
`tool_calls`, (2) the orphan-gap proxy for thrown errors, (3) server logs / the
live UI for the actual error text.
## Where the data lives
@@ -63,17 +61,13 @@ index 0: { "toolName": "getPage", "input": { "pageId": "…" } } ← tool-ca
index 1: { "toolName": "getPage", "output": { … } } ← tool-result (has output, NO input)
```
The keys that appear on an element are `toolName`, `input`, `output`, and — for a
**thrown** failure on rows written after the #407 fix — `error` (the tool's error
message; see the "Hard failures" section below). There is no `state`, no `errorText`,
no `type`. On pre-#407 rows a thrown failure has NO paired result element at all
(silent orphan). Consequences:
The **only** keys that ever appear on an element are `toolName`, `input`, `output`.
There is no `state`, no `errorText`, no `type`. Consequences:
1. **Real invocation count = elements that have `output` or `error`.** Counting every
element double-counts (you get ~2× and a spurious "~50% of every tool has no output").
2. **Pairing:** a call = a `tool-call` part followed by its result part. A success
carries `output`; a thrown failure (post-#407) carries `error` instead. Both carry
`toolName`, so you can group by tool on either.
1. **Real invocation count = elements that have `output`.** Counting every element
double-counts (you get ~2× and a spurious "~50% of every tool has no output").
2. **Pairing:** a successful call = a `tool-call` part followed by its `tool-result`
part. Both carry `toolName`, so you can group by tool on either.
## The two classes of failure (and which the DB can see)
@@ -91,37 +85,25 @@ These are visible in the `tool-result` `output`. The marker differs per tool:
Note `editPageText` returns `failed: []` on success — filtering on the *presence*
of the key gives false positives; filter on **non-empty**.
### 2. Hard failures — tool THREW → NOW PERSISTED (since the #407 fix)
### 2. Hard failures — tool THREW → NOT PERSISTED (the trap)
When a tool throws (the classic one is `patchNode` / `insertNode` / `tableUpdateCell`
`Failed to encode document to Yjs (fromJSON): Unknown node type: undefined`), the
runtime still writes **no `tool-result` part** — the failure is an ai@6 `tool-error`
content part instead. **Since the #407 fix, that error is persisted**: `serializeSteps`
appends a dedicated element `{toolName, error: "<message>"}` right after the failed
call, mirroring how a successful `{toolName, output}` element is appended. So a thrown
error now leaves a queryable `error` field carrying its (truncated) reason, and the
same real text is replayed to the model on the next turn (an `output-error` part with
the real `errorText`, no longer the `'Tool call did not complete.'` placeholder).
runtime writes **no `tool-result` part**. The orphaned `tool-call` part stays, but
the error text is **nowhere in the DB**. It is streamed to the UI live and (until
rotation) to server logs — that is it.
**Cutover caveat — old rows keep the old blind shape.** Rows written **before** this
change have the two-part shape (`call` + `output` only) and simply **drop** thrown
errors, leaving a silent **orphan** (a `call` with no `output` *and* no `error`). Rows
written **after** the fix additionally carry the `error` element. So:
So any query like `count(*) FILTER (WHERE output.success = false)` will happily
return **0** for `patchNode` even when the chat is visibly full of red failures.
That is survivorship bias, not reliability.
- **New rows:** query the `error` field directly (see the hard-error query below) — no
orphan heuristic needed for thrown failures.
- **Old rows (pre-#407):** the only DB-side proxy is still an **orphan**: a `tool-call`
part with no matching `tool-result` *and* no `error`. Orphans also appear when a run
is **aborted** mid-flight (server restart), so a high-volume tool (`createComment`,
`searchInPage`, `Search_web_search`) shows orphans from aborts, not real errors on
old rows. Treat the orphan gap as an *upper bound*, and cross-check the tool: a gap on
a structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
A note on the aborted-call fallback: a call with **neither** a result **nor** a
`tool-error` (genuinely interrupted mid-step) still replays with the
`'Tool call did not complete.'` placeholder and persists as an orphan — that path is
unchanged, and is distinct from a real thrown error, which now carries `error`.
The only DB-side proxy for a thrown error is an **orphan**: a `tool-call` part with
no matching `tool-result`. Caveat: orphans also appear when a run is **aborted**
mid-flight (server restart), so a high-volume tool (`createComment`, `searchInPage`,
`Search_web_search`) shows orphans from aborts, not from real errors. Treat the
orphan gap as an *upper bound* on hard errors, and cross-check the tool: a gap on a
structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
### 3. Run-level failures → `ai_chat_runs`
@@ -182,28 +164,14 @@ WHERE jsonb_typeof(o->'failed') = 'array'
GROUP BY 1 ORDER BY 2 DESC;
```
**Hard errors — persisted `error` field per tool (NEW rows, since #407)** — thrown
tool failures now carry their real reason, so query them directly:
```sql
SELECT elem->>'toolName' AS tool, count(*) AS thrown_errors,
min(elem->>'error') AS sample_error
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
WHERE jsonb_typeof(m.tool_calls) = 'array' AND elem ? 'error'
GROUP BY 1 ORDER BY 2 DESC;
```
**Hard-error proxy for OLD rows (pre-#407) — orphan gap per tool, WITH a spread column**
(call parts minus result parts, plus how many distinct chats the gap is spread across).
This covers rows written before thrown errors were persisted; on new rows a thrown
failure now has its own `error` element (use the query above) and an orphan means only
a genuinely aborted mid-step call:
**Hard-error proxy — orphan gap per tool, WITH a spread column** (call parts minus
result parts, plus how many distinct chats the gap is spread across):
```sql
WITH parts AS (
SELECT m.chat_id, elem->>'toolName' AS tool,
(elem ? 'input' AND NOT (elem ? 'output')) AS is_call,
(elem ? 'output' OR elem ? 'error') AS is_result
(elem ? 'output') AS is_result
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
WHERE jsonb_typeof(m.tool_calls) = 'array' AND m.role = 'assistant'
),
@@ -220,12 +188,8 @@ HAVING sum(gap) FILTER (WHERE gap > 0) > 0
ORDER BY missing_results DESC;
```
The `is_result` predicate counts an `error` element as a paired result too, so on new
rows a persisted thrown error no longer inflates the orphan gap; a remaining gap is an
aborted/interrupted call.
**On OLD rows, `missing_results` mixes thrown errors AND aborted/interrupted runs — you
cannot split them from `output` alone** (a positional "what follows the orphan" heuristic
**`missing_results` mixes thrown errors AND aborted/interrupted runs — you cannot
split them from `output` alone** (a positional "what follows the orphan" heuristic
breaks on parallel tool batches, which persist as `call,call,…,result,result`). Use
`chats_spread` to disambiguate:
@@ -280,20 +244,18 @@ docker compose -p gitmost logs -f --tail=100 # whole stack
```
Logging is `json-file`, `max-size=10m max-file=5` → ~50 MB retained, then rotated,
and **wiped on container recreate**. Since the #407 fix, thrown-tool error text is
**persisted in the `error` field** of `tool_calls` (see the hard-error query above), so
you no longer depend on live logs for it. Logs/live UI remain useful for **pre-#407
rows** (whose thrown errors were dropped) and for full stack traces beyond the
truncated stored message. A per-tool `tool_calls_total{tool,status}` metric to
VictoriaMetrics is still a possible future add for aggregate dashboards.
and **wiped on container recreate**. So thrown-tool error text is only reliably
caught **in real time** (or in the live chat UI, which renders the failed part with
its message). There is no durable, queryable store of hard tool errors today — if you
need one, that is a feature to add (persist `output-error` parts, or emit a
`tool_calls_total{tool,status}` metric to VictoriaMetrics).
## Gotchas checklist
- [ ] Counting every `tool_calls` element → **overcount**. Count `output` elements; add `error` elements for thrown failures (new rows), but don't count both as invocations.
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors are a separate `error` element (new rows) or dropped entirely (pre-#407 rows).
- [ ] Thrown errors persist only on rows written **after the #407 fix** — pre-#407 rows still drop them (orphan only). Mind the cutover when trending over time.
- [ ] Counting every `tool_calls` element → **overcount**. Count elements with `output`.
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors aren't persisted.
- [ ] `editPageText.failed` is `[]` on success — test for **non-empty**, not presence.
- [ ] Orphan gap on OLD rows mixes thrown errors **and** aborted runs — split by tool. On NEW rows a thrown error is its own `error` element, so a gap ≈ aborted call.
- [ ] Orphan gap mixes thrown errors **and** aborted runs — split by tool before concluding.
- [ ] `aborted` runs = server restarts, `failed` runs = provider overload — not agent mistakes.
- [ ] Never dump a raw `tool_calls` cell — it can be hundreds of KB.
- [ ] Logs are ephemeral (≤50 MB, wiped on recreate) — grab hard-error text live.
+7 -2
View File
@@ -75,6 +75,7 @@ import {
canonicalizeFootnotes,
insertInlineFootnote,
} from "./lib/transforms.js";
import { normalizeAndMergeFootnotes } from "./lib/footnote-normalize-merge.js";
import vm from "node:vm";
// Supported image types, kept as two lookup tables so both a local file
@@ -1694,6 +1695,8 @@ export class DocmostClient {
// leave footnotes out of order, orphaned, or in multiple lists — the bottom
// list + numbering are always derived from reference order. No-op when the
// footnotes are already canonical.
// #419: normalize + merge glyph-forked definitions before canonicalizing.
doc = normalizeAndMergeFootnotes(doc);
doc = canonicalizeFootnotes(doc);
// Write the BODY first, then the title (#159 split-brain): a failed body
@@ -1958,7 +1961,8 @@ export class DocmostClient {
// footnotes before copying — a no-op on already-canonical source content, but
// it guarantees a copy can never propagate a non-canonical footnote topology
// to the target (parity with the other full-doc write paths).
const canonical = canonicalizeFootnotes(content);
// #419: normalize + merge glyph-forked definitions before canonicalizing.
const canonical = canonicalizeFootnotes(normalizeAndMergeFootnotes(content));
const collabToken = await this.getCollabTokenWithReauth();
// Open the TARGET collab doc by its canonical UUID, never the slugId (#260).
@@ -3667,7 +3671,8 @@ export class DocmostClient {
// path can leave footnotes out of order / orphaned / in a raw `[^id]`
// block. In a dryRun preview this may surface footnote edits the script
// author did not write (the canonicalizer tidied them) — that is expected.
const result = canonicalizeFootnotes(raw);
// #419: normalize + merge glyph-forked definitions before canonicalizing.
const result = canonicalizeFootnotes(normalizeAndMergeFootnotes(raw));
newDoc = result;
return result;
};
+7 -1
View File
@@ -16,6 +16,7 @@ import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
import { withPageLock } from "./page-lock.js";
import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js";
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
import { summarizeChange, VerifyReport } from "./diff.js";
export { markdownToProseMirror };
@@ -82,7 +83,12 @@ global.WebSocket = WebSocket;
export async function markdownToProseMirrorCanonical(
markdownContent: string,
): Promise<any> {
return canonicalizeFootnotes(await markdownToProseMirror(markdownContent));
// #419: normalize + merge glyph-forked footnote definitions BEFORE
// canonicalizing, so the canonicalizer re-hangs references and drops the
// now-orphaned duplicate definitions.
return canonicalizeFootnotes(
normalizeAndMergeFootnotes(await markdownToProseMirror(markdownContent)),
);
}
/**
@@ -0,0 +1,280 @@
/**
* Deterministic server-side NORMALIZATION + MERGE of footnote DEFINITIONS
* (MCP, PURE).
*
* Problem (#419): footnotes with the same meaning but different GLYPHS
* typographic quotes («»/) vs ASCII "…", em/en-dash vs `-`, non-breaking
* space vs normal space, differing space counts are not recognized as equal
* and "fork": two definitions appear where the author meant one. The existing
* de-dup paths miss this: `footnoteContentKey` (footnote-authoring.ts) only
* collapses ASCII whitespace (quotes/dashes/NBSP untouched), and
* `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have
* different ids), so neither glues the forks together.
*
* This pass fixes that DETERMINISTICALLY on the MCP write-paths (an LLM
* instruction gives no glue guarantee). It:
* 1. Normalizes the TEXT of every `footnoteDefinition`'s text nodes IN PLACE
* (typographic quotes -> ASCII "/', dashes -> `-`, NBSP & friends ->
* normal space, whitespace runs collapsed, whole-definition edges
* trimmed) unconditionally, for ALL definitions, KEEPING their marks.
* 2. Computes a MERGE KEY per definition (normalized text + an ATTRS-AWARE
* inline-mark signature, via the local `footnoteMergeKey`), so notes that
* read the same but differ in formatting (bold vs plain) OR in a mark
* attribute (a `link` with a different `href`, differing `code`/`highlight`
* attrs) are NOT merged. See `footnoteMergeKey` for why this diverges from
* the shared type-only `footnoteContentKey`.
* 3. Maps every duplicate definition id to the FIRST (document-order)
* definition's id and re-hangs `footnoteReference` nodes onto it.
*
* Duplicate definitions keep their original ids but now have NO references, so
* the canonicalizer that runs immediately after this pass removes them as
* orphans and derives the single tail list + numbering. This pass therefore
* MUST run BEFORE `canonicalizeFootnotes(doc)` at every write-path call-site
* (see the enforcement rule in `footnote-canonicalize.ts`).
*
* Accepted tradeoff: the exact typographic glyphs of the SURVIVING footnote are
* rewritten to ASCII, in exchange for a GUARANTEED merge. Scope is strictly
* INSIDE `footnoteDefinition` body text (normal paragraphs) is never touched.
*
* Pure: deep-clones its input, deterministic, idempotent (a re-run is a no-op
* text is already normalized and references already point at the canonical id,
* so no spurious mutations / git-sync churn).
*/
const FOOTNOTE_DEFINITION_NAME = "footnoteDefinition";
const FOOTNOTE_REFERENCE_NAME = "footnoteReference";
/**
* Typographic glyph maps. DUPLICATED from `comment-anchor.ts` (the source of
* truth, `normalizeForMatch`) on purpose: those constants are private there and
* bound to that module's anchor-matching golden tests, so extracting them would
* risk changing anchor behaviour. Keeping a local copy makes this pass fully
* self-contained. If the anchor maps grow, mirror the change here.
*/
/** Typographic double-quote variants mapped to ASCII `"`. */
const DOUBLE_QUOTES = "«»„“”‟〝〞"";
/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
const SINGLE_QUOTES = "‘’‚‛";
/** Dash variants mapped to ASCII `-`. */
const DASHES = "–—―−‐‑‒";
function cloneJson<T>(v: T): T {
if (typeof structuredClone === "function") return structuredClone(v);
return JSON.parse(JSON.stringify(v)) as T;
}
/**
* True for any character we collapse/replace with a single normal space.
* Mirrors `comment-anchor.ts`'s `isWhitespaceChar`: ASCII whitespace (`\s`
* covers tab/newline) plus the non-breaking / special spaces listed explicitly
* for determinism across engines.
*/
function isWhitespaceChar(ch: string): boolean {
return (
/\s/.test(ch) ||
ch === " " || // no-break space
ch === " " || // figure space
ch === " " || // narrow no-break space
ch === " " || // thin space
ch === " " || // hair space
ch === " " || // en space
ch === " " // em space
);
}
/**
* Map typographic quotes/dashes to ASCII and collapse every whitespace run
* (including NBSP & friends) to a SINGLE normal space. Does NOT trim the
* whole-definition edge trim is applied separately so inter-node spacing across
* a multi-text-node definition is preserved.
*/
function normalizeAndCollapse(s: string): string {
let out = "";
let i = 0;
while (i < s.length) {
const ch = s[i];
if (isWhitespaceChar(ch)) {
while (i < s.length && isWhitespaceChar(s[i])) i++;
out += " ";
continue;
}
let mapped = ch;
if (DOUBLE_QUOTES.indexOf(ch) !== -1) mapped = '"';
else if (SINGLE_QUOTES.indexOf(ch) !== -1) mapped = "'";
else if (DASHES.indexOf(ch) !== -1) mapped = "-";
out += mapped;
i++;
}
return out;
}
/** Collect every text node inside `def`, in document order (deep). */
function collectTextNodes(node: any, out: any[]): void {
if (!node || typeof node !== "object") return;
if (node.type === "text" && typeof node.text === "string") out.push(node);
if (Array.isArray(node.content)) {
for (const child of node.content) collectTextNodes(child, out);
}
}
/** Collect every `footnoteDefinition` node in document order (deep). */
function collectDefinitions(node: any, out: any[]): void {
if (!node || typeof node !== "object") return;
if (node.type === FOOTNOTE_DEFINITION_NAME) out.push(node);
if (Array.isArray(node.content)) {
for (const child of node.content) collectDefinitions(child, out);
}
}
/**
* Normalize the text of one definition's text nodes IN PLACE: map glyphs +
* collapse whitespace on every node (marks untouched), then trim the leading
* edge of the first text node and the trailing edge of the last so the
* definition as a whole is trimmed WITHOUT dropping the spacing between two
* adjacent text nodes. The edge trims are guarded so an all-whitespace edge
* node is never emptied into a schema-invalid empty text node.
*/
function normalizeDefinitionText(def: any): void {
const textNodes: any[] = [];
collectTextNodes(def, textNodes);
for (const t of textNodes) {
// Skip text carrying a `code` mark: inline code is a verbatim literal, not
// prose typography. Rewriting quotes/dashes/special-spaces there would
// corrupt the literal's meaning (a string literal, an em-dash flag, i18n).
// Leaving it untouched also makes it contribute its RAW text to
// `footnoteMergeKey`, so two notes differing only by glyphs inside code
// stay distinct (while prose glyph-forks still merge). See #419.
if ((t.marks || []).some((m: any) => m?.type === "code")) continue;
t.text = normalizeAndCollapse(t.text);
}
if (textNodes.length === 0) return;
const hasCodeMark = (t: any): boolean =>
(t.marks || []).some((m: any) => m?.type === "code");
const first = textNodes[0];
if (!hasCodeMark(first)) {
const startTrimmed = first.text.replace(/^ +/, "");
if (startTrimmed !== "") first.text = startTrimmed;
}
const last = textNodes[textNodes.length - 1];
if (!hasCodeMark(last)) {
const endTrimmed = last.text.replace(/ +$/, "");
if (endTrimmed !== "") last.text = endTrimmed;
}
}
/** Rewrite `footnoteReference` ids IN PLACE using `defIdToCanon` (deep). */
function rehangReferences(
node: any,
defIdToCanon: Map<string, string>,
): void {
if (!node || typeof node !== "object") return;
if (node.type === FOOTNOTE_REFERENCE_NAME) {
const id = node?.attrs?.id;
if (typeof id === "string") {
const canon = defIdToCanon.get(id);
if (canon && canon !== id) node.attrs.id = canon;
}
}
if (Array.isArray(node.content)) {
for (const child of node.content) rehangReferences(child, defIdToCanon);
}
}
/**
* Stable, order-independent serialization of a mark's `attrs`: sort keys so the
* same attrs always yield the same string regardless of authoring order. Empty /
* missing attrs -> "" (so an attr-less mark keys identically to a type-only mark
* signature, preserving bold-vs-plain parity).
*/
function stableAttrs(attrs: any): string {
if (!attrs || typeof attrs !== "object") return "";
const sorted: Record<string, any> = {};
for (const k of Object.keys(attrs).sort()) sorted[k] = attrs[k];
return JSON.stringify(sorted);
}
/**
* ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from
* the shared `footnoteContentKey` (footnote-authoring.ts): that key's mark
* signature is TYPE-ONLY (`m.type`), so two definitions with identical visible
* text but marks differing only in ATTRIBUTES most importantly a `link` with a
* different `href` (footnotes are usually citations/links), also `code` /
* `highlight` with differing attrs collapse to the SAME key and get merged;
* one definition then loses its references and the canonicalizer deletes it as an
* orphan, silently dropping a distinct link target (data loss, #419).
*
* This key folds each mark's `attrs` (stable, sorted-key serialization) into the
* signature, so different-href / different-attr notes stay separate. We do NOT
* change `footnoteContentKey` itself: it is shared with the live
* `insertInlineFootnote` / `commentsToFootnotes` dedup and altering it there
* would change their behaviour out of scope here.
*
* The TEXT portion mirrors `footnoteContentKey` exactly (per text node
* `text + mark-signature`, concatenated, whitespace-collapsed, trimmed) over the
* already-in-place-normalized text, so empty text still yields "" (empties never
* collapse) and merge parity with the rest of the pass is preserved.
*/
function footnoteMergeKey(defNode: any): string {
const parts: string[] = [];
const visit = (n: any): void => {
if (!n || typeof n !== "object") return;
if (n.type === "text" && typeof n.text === "string") {
const marks = Array.isArray(n.marks)
? n.marks
.filter((m: any) => m && m.type)
.map((m: any) => `${m.type}${stableAttrs(m.attrs)}`)
.sort()
.join(",")
: "";
parts.push(`${n.text}${marks}`);
}
if (Array.isArray(n.content)) for (const c of n.content) visit(c);
};
visit(defNode);
return parts
.join("")
.replace(/[ \t\r\n]+/g, " ")
.trim();
}
/**
* Normalize footnote-definition text and merge definitions whose normalized
* text (+ mark signature) matches. See the file header for the full contract.
* Pure (deep-clones input, deterministic, idempotent). Intended to run
* immediately BEFORE `canonicalizeFootnotes(doc)`.
*/
export function normalizeAndMergeFootnotes<T = any>(doc: T): T {
if (doc == null || typeof doc !== "object") return doc;
const out = cloneJson(doc) as any;
// 1) All definitions in document order; normalize each one's text in place.
const defNodes: any[] = [];
collectDefinitions(out, defNodes);
for (const def of defNodes) normalizeDefinitionText(def);
// 2) Merge key per definition (normalized text + inline-mark signature). The
// first definition in document order per key wins; later ones map onto it.
// Empty-text definitions (key === "") are NOT merged — otherwise every
// empty footnote would collapse into one (parity with insertInlineFootnote).
const keyToCanon = new Map<string, string>();
const defIdToCanon = new Map<string, string>();
for (const def of defNodes) {
const id = def?.attrs?.id;
if (typeof id !== "string" || id === "") continue;
const key = footnoteMergeKey(def);
if (key === "") continue;
const canon = keyToCanon.get(key);
if (canon === undefined) {
keyToCanon.set(key, id);
} else if (canon !== id) {
defIdToCanon.set(id, canon);
}
}
// 3) Re-hang references from duplicate ids onto the canonical id. Duplicate
// definitions keep their ids but now have no references -> the following
// canonicalizer pass drops them as orphans.
if (defIdToCanon.size > 0) rehangReferences(out, defIdToCanon);
return out;
}
+3
View File
@@ -16,6 +16,7 @@
import { blockPlainText } from "./node-ops.js";
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
import {
footnoteContentKey,
makeFootnoteDefinition,
@@ -766,6 +767,8 @@ export function insertInlineFootnote(
appendDefinition(working, makeFootnoteDefinition(footnoteId, inline));
}
// #419: normalize + merge glyph-forked definitions before canonicalizing.
working = normalizeAndMergeFootnotes(working);
// Derive numbering + the single bottom list deterministically.
working = canonicalizeFootnotes(working);
return { doc: working, inserted: true, footnoteId, reused };
@@ -0,0 +1,317 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { normalizeAndMergeFootnotes } from "../../build/lib/footnote-normalize-merge.js";
import { canonicalizeFootnotes } from "../../build/lib/footnote-canonicalize.js";
function findAll(node, type, acc = []) {
if (!node || typeof node !== "object") return acc;
if (node.type === type) acc.push(node);
if (Array.isArray(node.content)) {
for (const c of node.content) findAll(c, type, acc);
}
return acc;
}
const defs = (doc) => findAll(doc, "footnoteDefinition");
const defIds = (doc) => defs(doc).map((d) => d.attrs.id);
const refIds = (doc) => findAll(doc, "footnoteReference").map((r) => r.attrs.id);
const defText = (d) =>
findAll(d, "text")
.map((t) => t.text)
.join("");
const ref = (id) => ({ type: "footnoteReference", attrs: { id } });
const para = (...inline) => ({ type: "paragraph", content: inline });
const txt = (text, marks) =>
marks ? { type: "text", text, marks } : { type: "text", text };
const def = (id, ...inline) => ({
type: "footnoteDefinition",
attrs: { id },
content: [para(...inline)],
});
const list = (...defs) => ({ type: "footnotesList", content: defs });
const doc = (...content) => ({ type: "doc", content });
// --- Normalization + merge of glyph forks ----------------------------------
test("typographic double quotes «…» vs \"…\" merge into one", () => {
const d = doc(
para(txt("a"), ref("A"), txt(" b"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const out = normalizeAndMergeFootnotes(d);
// Both references now point at the first definition's id.
assert.deepEqual(refIds(out), ["A", "A"]);
// Surviving text is ASCII-normalized.
assert.equal(defText(defs(out)[0]), '"word"');
// Duplicate def kept its id (canonicalizer removes it as an orphan later).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A"]);
assert.equal(findAll(canon, "footnotesList").length, 1);
});
test("em/en dash and hyphen merge", () => {
const d = doc(
para(ref("A"), ref("B"), ref("C")),
list(
def("A", txt("see — here")),
def("B", txt("see – here")),
def("C", txt("see - here")),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A", "A"]);
assert.equal(defText(defs(out)[0]), "see - here");
});
test("NBSP and extra spaces merge with normal spacing", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("foo bar")), // NBSP
def("B", txt("foo bar")), // collapsed spaces
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), "foo bar");
});
test("same text but different styling (bold vs plain) does NOT merge", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("word", [{ type: "bold" }])),
def("B", txt("word")),
),
);
const out = normalizeAndMergeFootnotes(d);
// No re-hang: references keep their own ids.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Marks preserved on the surviving text node.
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
{ type: "bold" },
]);
});
test("same text but a link mark with different href does NOT merge (data-loss guard)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
def("B", txt("source", [{ type: "link", attrs: { href: "https://b.example/2" } }])),
),
);
const out = normalizeAndMergeFootnotes(d);
// No re-hang: each reference keeps its own definition (distinct link target).
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Both distinct hrefs survive.
assert.deepEqual(
defs(out).map((dn) => dn.content[0].content[0].marks[0].attrs.href),
["https://a.example/1", "https://b.example/2"],
);
// Canonicalize keeps both as two tail entries (neither is an orphan).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A", "B"]);
assert.deepEqual(refIds(canon), ["A", "B"]);
});
test("same text and SAME link href still merges (attrs-aware key doesn't over-separate)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
def("B", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A"]);
assert.equal(findAll(canon, "footnotesList").length, 1);
});
test("marks are kept on merged (surviving) definition text", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("«x»", [{ type: "italic" }])),
def("B", txt("«x»", [{ type: "italic" }])),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
{ type: "italic" },
]);
assert.equal(defText(defs(out)[0]), '"x"');
});
// --- Inline code is verbatim (not typography) ------------------------------
test("text inside a code mark is left verbatim; prose in the same def is normalized", () => {
const d = doc(
para(ref("A")),
list({
type: "footnoteDefinition",
attrs: { id: "A" },
content: [
para(
txt("a—b «x»", [{ type: "code" }]),
txt(" prose «y» — z"),
),
],
}),
);
const out = normalizeAndMergeFootnotes(d);
const nodes = findAll(defs(out)[0], "text");
// Code node: byte-for-byte unchanged (typography preserved).
assert.equal(nodes[0].text, "a—b «x»");
// Prose node: dashes/quotes normalized to ASCII.
assert.equal(nodes[1].text, ' prose "y" - z');
});
test("two notes differing ONLY by glyphs inside a code mark do NOT merge", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("«x»", [{ type: "code" }]), txt(" same prose «q»")),
def("B", txt('"x"', [{ type: "code" }]), txt(" same prose «q»")),
),
);
const out = normalizeAndMergeFootnotes(d);
// Prose is identical after normalization, but the code literals differ raw
// -> the merge key diverges -> both definitions survive, no re-hang.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Each code literal stays verbatim.
assert.equal(defs(out)[0].content[0].content[0].text, "«x»");
assert.equal(defs(out)[1].content[0].content[0].text, '"x"');
// Both survive canonicalization (neither is an orphan).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A", "B"]);
assert.deepEqual(refIds(canon), ["A", "B"]);
});
// --- Composition with the canonicalizer ------------------------------------
test("pass + canonicalize: single tail list and sequential numbering", () => {
const d = doc(
para(txt("intro "), ref("A"), txt(" middle "), ref("B")),
list(def("A", txt("«note»")), def("B", txt('"note"'))),
);
const canon = canonicalizeFootnotes(normalizeAndMergeFootnotes(d));
assert.equal(findAll(canon, "footnotesList").length, 1);
assert.deepEqual(defIds(canon), ["A"]);
assert.deepEqual(refIds(canon), ["A", "A"]);
});
// --- Idempotency -----------------------------------------------------------
test("idempotent: a second run is a no-op", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const once = normalizeAndMergeFootnotes(d);
const twice = normalizeAndMergeFootnotes(once);
assert.deepEqual(twice, once);
});
test("input document is not mutated (pure)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const snapshot = JSON.parse(JSON.stringify(d));
normalizeAndMergeFootnotes(d);
assert.deepEqual(d, snapshot);
});
// --- Nested definitions ----------------------------------------------------
test("definitions nested in a callout are normalized and merged", () => {
const callout = (...content) => ({
type: "callout",
attrs: { type: "info" },
content,
});
const d = doc(
para(ref("A"), ref("B")),
callout(list(def("A", txt("«c»")), def("B", txt('"c"')))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), '"c"');
});
// --- Empty footnotes -------------------------------------------------------
test("empty footnotes do NOT collapse into each other", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("")), { type: "footnoteDefinition", attrs: { id: "B" }, content: [{ type: "paragraph" }] }),
);
const out = normalizeAndMergeFootnotes(d);
// Both empty definitions keep distinct ids; references unchanged.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
});
// --- Body text left untouched ----------------------------------------------
test("body text (outside footnotes) is NOT normalized", () => {
const d = doc(
para(txt("body «quoted» — dash"), ref("A")),
list(def("A", txt("«note»"))),
);
const out = normalizeAndMergeFootnotes(d);
// Body paragraph keeps its typographic glyphs verbatim.
assert.equal(out.content[0].content[0].text, "body «quoted» — dash");
// Footnote text IS normalized.
assert.equal(defText(defs(out)[0]), '"note"');
});
// --- Multi-paragraph structure preserved -----------------------------------
test("multi-paragraph definition: text normalized, structure preserved", () => {
const d = doc(
para(ref("A")),
list({
type: "footnoteDefinition",
attrs: { id: "A" },
content: [para(txt("«p1»")), para(txt("p2 — end"))],
}),
);
const out = normalizeAndMergeFootnotes(d);
const def0 = defs(out)[0];
assert.equal(def0.content.length, 2);
assert.equal(def0.content[0].content[0].text, '"p1"');
assert.equal(def0.content[1].content[0].text, "p2 - end");
});
// --- Multi-reference footnote not broken -----------------------------------
test("one id shared by multiple references is preserved", () => {
const d = doc(
para(ref("A"), txt(" x "), ref("A")),
list(def("A", txt("note"))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.deepEqual(defIds(out), ["A"]);
});
// --- Whole-definition edge trim --------------------------------------------
test("leading/trailing whitespace is trimmed for the merge and stored text", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt(" hello ")), def("B", txt("hello"))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), "hello");
});