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

test(footnotes): cover footnoteWarnings import plumbing + doc fixes (#169 second review)

Browse Source 
Follow-up to the merged #166/#169. Addresses the second review pass (comment
1227):

- footnoteWarnings plumbing: extract a single `footnoteWarningsField(markdown)`
  helper (footnote-analyze) and use it at all three call sites (create_page,
  update_page, import_page_markdown) so the field is attached identically.
- New unit test footnote-warnings-import.test.mjs pins the contract that was
  uncovered: the field is present on problems / omitted on clean input, and the
  IMPORT path analyzes the BODY after the docmost:meta / docmost:comments blocks
  (a footnote-like token inside those JSON blocks must NOT warn; a real body
  marker must). Tested via the same pure composition the importer uses
  (footnoteWarningsField(parseDocmostMarkdown(full).body)) — no collab socket
  needed; a regression that analyzed fullMarkdown or skipped the body split would
  now go red.
- footnote.marked.ts: correct the stale module header — it claimed "only
  definitions that have a matching reference are emitted", which was never true
  (orphan defs are emitted; the editor sync plugin reconciles). Now describes
  first-wins + reuse + sync reconciliation.
- derive-id golden test: rename the describe from "(cross-package drift guard)"
  to "(deterministic-scheme pin)" — there is no second package to drift against.

editor-ext 129, MCP 304 (+3), client+server tsc clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-24 16:44:53 +03:00
parent b9056e2bee
commit 0e8af13122
7 changed files with 109 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
packages/mcp/build/client.js
View File

@@ -9,7 +9,7 @@ import WebSocket from "ws";
import { convertProseMirrorToMarkdown } from "./lib/markdown-converter.js";
import { updatePageContentRealtime, replacePageContent, markdownToProseMirror, mutatePageContent, buildCollabWsUrl, assertYjsEncodable, } from "./lib/collaboration.js";
import { docmostExtensions } from "./lib/docmost-schema.js";
import { analyzeFootnotes } from "./lib/footnote-analyze.js";
import { footnoteWarningsField } from "./lib/footnote-analyze.js";
import { buildPageTree } from "./lib/tree.js";
import { serializeDocmostMarkdown, parseDocmostMarkdown, } from "./lib/markdown-document.js";
import { replaceNodeById, deleteNodeById, insertNodeRelative, buildOutline, getNodeByRef, readTable, insertTableRow, deleteTableRow, updateTableCell, } from "./lib/node-ops.js";
@@ -820,8 +820,7 @@ export class DocmostClient {
const page = await this.getPage(newPageId);
// Surface non-fatal footnote problems (dangling refs, empty/duplicate
// definitions, markers in tables) so the agent can fix its markup (#166).
const { warnings } = analyzeFootnotes(content);
return warnings.length > 0 ? { ...page, footnoteWarnings: warnings } : page;
return { ...page, ...footnoteWarningsField(content) };
}
/**
* Update a page's content from markdown and optionally its title.
@@ -851,7 +850,6 @@ export class DocmostClient {
}
throw new Error(`Failed to update page content: ${error.message}`);
}
const { warnings } = analyzeFootnotes(content);
return {
success: true,
modified: true,
@@ -859,7 +857,7 @@ export class DocmostClient {
pageId: pageId,
verify: mutation.verify,
// Non-fatal footnote diagnostics (#166); omitted when there are none.
...(warnings.length > 0 ? { footnoteWarnings: warnings } : {}),
...footnoteWarningsField(content),
};
}
/**
@@ -1129,11 +1127,11 @@ export class DocmostClient {
if (meta?.pageId && meta.pageId !== pageId) {
result.warning = `File was exported from page ${meta.pageId} but is being imported into ${pageId}.`;
}
// Non-fatal footnote diagnostics (#166), analyzed on the body (definitions
// and references live there, not in the front-matter/comments sections).
const { warnings } = analyzeFootnotes(body);
if (warnings.length > 0)
result.footnoteWarnings = warnings;
// Non-fatal footnote diagnostics (#166), analyzed on the BODY (the part after
// the docmost:meta / docmost:comments blocks) — so a `[^x]`-like token inside
// those JSON blocks never produces a false warning, while real markers in the
// body do. `body` comes from parseDocmostMarkdown(fullMarkdown) above.
Object.assign(result, footnoteWarningsField(body));
return result;
}
/**

10
packages/mcp/build/lib/footnote-analyze.js
View File

@@ -89,3 +89,13 @@ export function analyzeFootnotes(markdown) {
warnings,
};
}
/**
* The optional `footnoteWarnings` field for a page-write tool result: present
* (with the warning lines) only when `markdown` has footnote problems, omitted
* otherwise. One helper so all three call sites (create/update/import) attach the
* field identically. Spread into the result: `{ ...result, ...footnoteWarningsField(text) }`.
*/
export function footnoteWarningsField(markdown) {
const { warnings } = analyzeFootnotes(markdown);
return warnings.length > 0 ? { footnoteWarnings: warnings } : {};
}

17
packages/mcp/src/client.ts
View File

@@ -23,7 +23,7 @@ import {
MutationResult,
} from "./lib/collaboration.js";
import { docmostExtensions } from "./lib/docmost-schema.js";
import { analyzeFootnotes } from "./lib/footnote-analyze.js";
import { footnoteWarningsField } from "./lib/footnote-analyze.js";
import { buildPageTree } from "./lib/tree.js";
import {
serializeDocmostMarkdown,
@@ -1058,8 +1058,7 @@ export class DocmostClient {
const page = await this.getPage(newPageId);
// Surface non-fatal footnote problems (dangling refs, empty/duplicate
// definitions, markers in tables) so the agent can fix its markup (#166).
const { warnings } = analyzeFootnotes(content);
return warnings.length > 0 ? { ...page, footnoteWarnings: warnings } : page;
return { ...page, ...footnoteWarningsField(content) };
}
/**
@@ -1100,7 +1099,6 @@ export class DocmostClient {
throw new Error(`Failed to update page content: ${error.message}`);
}
const { warnings } = analyzeFootnotes(content);
return {
success: true,
modified: true,
@@ -1108,7 +1106,7 @@ export class DocmostClient {
pageId: pageId,
verify: mutation.verify,
// Non-fatal footnote diagnostics (#166); omitted when there are none.
...(warnings.length > 0 ? { footnoteWarnings: warnings } : {}),
...footnoteWarningsField(content),
};
}
@@ -1424,10 +1422,11 @@ export class DocmostClient {
if (meta?.pageId && meta.pageId !== pageId) {
result.warning = `File was exported from page ${meta.pageId} but is being imported into ${pageId}.`;
}
// Non-fatal footnote diagnostics (#166), analyzed on the body (definitions
// and references live there, not in the front-matter/comments sections).
const { warnings } = analyzeFootnotes(body);
if (warnings.length > 0) result.footnoteWarnings = warnings;
// Non-fatal footnote diagnostics (#166), analyzed on the BODY (the part after
// the docmost:meta / docmost:comments blocks) — so a `[^x]`-like token inside
// those JSON blocks never produces a false warning, while real markers in the
// body do. `body` comes from parseDocmostMarkdown(fullMarkdown) above.
Object.assign(result, footnoteWarningsField(body));
return result;
}

13
packages/mcp/src/lib/footnote-analyze.ts
View File

@@ -114,3 +114,16 @@ export function analyzeFootnotes(markdown: string): FootnoteDiagnostics {
warnings,
};
}
/**
* The optional `footnoteWarnings` field for a page-write tool result: present
* (with the warning lines) only when `markdown` has footnote problems, omitted
* otherwise. One helper so all three call sites (create/update/import) attach the
* field identically. Spread into the result: `{ ...result, ...footnoteWarningsField(text) }`.
*/
export function footnoteWarningsField(markdown: string): {
footnoteWarnings?: string[];
} {
const { warnings } = analyzeFootnotes(markdown);
return warnings.length > 0 ? { footnoteWarnings: warnings } : {};
}

63
packages/mcp/test/unit/footnote-warnings-import.test.mjs Normal file
View File

@@ -0,0 +1,63 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import {
analyzeFootnotes,
footnoteWarningsField,
} from "../../build/lib/footnote-analyze.js";
import {
serializeDocmostMarkdown,
parseDocmostMarkdown,
} from "../../build/lib/markdown-document.js";
// Pins the footnoteWarnings PLUMBING contract (#169 review): the field is
// present only on problems and omitted on clean input, AND `import_page_markdown`
// analyzes the BODY (after the docmost:meta / docmost:comments blocks) — so a
// footnote-like token inside those JSON blocks never warns, while a real marker
// in the body does. importPageMarkdown does exactly
// `footnoteWarningsField(parseDocmostMarkdown(full).body)` over a collab socket
// this harness does not stand up, so we test the same pure composition directly.
test("footnoteWarningsField is present on problems and omitted on clean input", () => {
const problem = footnoteWarningsField("See[^missing].\n\n[^a]: defined");
assert.ok(Array.isArray(problem.footnoteWarnings));
assert.match(problem.footnoteWarnings.join("\n"), /no matching definition/);
const clean = footnoteWarningsField("A[^a] and reuse[^a].\n\n[^a]: fine");
assert.deepEqual(clean, {}); // no key at all on clean input
});
test("import analyzes the BODY only — tokens inside meta/comments never warn", () => {
// meta + comments JSON carry `[^metaonly]` / `[^commentonly]`-looking text; the
// BODY has a genuinely dangling `[^bodyref]`.
const full = serializeDocmostMarkdown(
{ pageId: "p1", note: "front-matter mentions [^metaonly] in text" },
"Body with a dangling[^bodyref] marker.",
[{ id: "c1", content: "a comment that says [^commentonly]" }],
);
const { body } = parseDocmostMarkdown(full);
// Sanity: the meta/comments markers are NOT in the parsed body.
assert.ok(!body.includes("[^metaonly]"));
assert.ok(!body.includes("[^commentonly]"));
const field = footnoteWarningsField(body);
const joined = (field.footnoteWarnings ?? []).join("\n");
// ONLY the body's dangling reference is flagged.
assert.match(joined, /\[\^bodyref\]/);
assert.ok(!joined.includes("metaonly"));
assert.ok(!joined.includes("commentonly"));
// Cross-check against analyzeFootnotes directly (same composition the importer uses).
assert.deepEqual(analyzeFootnotes(body).danglingReferences, ["bodyref"]);
});
test("import on a clean body yields no footnoteWarnings field", () => {
const full = serializeDocmostMarkdown(
{ pageId: "p1" },
"Clean body[^a] reusing[^a].\n\n[^a]: ok",
[],
);
const { body } = parseDocmostMarkdown(full);
assert.deepEqual(footnoteWarningsField(body), {});
});