fix(converter): warnings вместо тихой потери незнакомых нод/марок
Незнакомый тип ноды (default-ветка switch) молча схлопывался в свои дети, а незнакомая марка молча выбрасывалась — тихая потеря данных. Теперь сериализатор РЕПОРТИТ потерю: - по умолчанию поведение байт-в-байт прежнее (graceful degrade), но при переданном options.warnings в сток кладётся по одному сообщению на незамапленный тип (дедуп по типу) — потеря наблюдаема; - options.strict бросает ConverterLossError на ПЕРВОМ незнакомом типе (warning = ошибка). git-sync (lossless-путь) включает strict в stabilizePageBody: тип без серизализующей ветки падает громко на записи, а не пишет lossy .md. Валидный контент не затронут — у всех текущих типов схемы есть ветка. Покрытие: converter-loss-warnings.test.ts (нода/марка × strict/non-strict, дедуп, чистый контент) и strict-пин в git-sync stabilize.test.ts — всё через реальный конвертер. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -72,7 +72,13 @@ export async function stabilizePageFile(
|
||||
* keeps re-pulls of an unchanged page byte-identical (no churn, loop-guard).
|
||||
*/
|
||||
export async function stabilizePageBody(content: unknown): Promise<string> {
|
||||
const md1 = convertProseMirrorToMarkdown(content);
|
||||
// git-sync is the LOSSLESS mirror path, so run the serializer in `strict`
|
||||
// mode: a node/mark type the converter has no case for (e.g. one added to the
|
||||
// schema without a matching serializer arm) throws a ConverterLossError here
|
||||
// rather than silently degrading — surfacing the loss loudly at write time
|
||||
// instead of committing a lossy file. Valid content (every current schema type
|
||||
// has a case) is unaffected.
|
||||
const md1 = convertProseMirrorToMarkdown(content, { strict: true });
|
||||
const doc2 = await markdownToProseMirror(md1);
|
||||
return convertProseMirrorToMarkdown(doc2);
|
||||
return convertProseMirrorToMarkdown(doc2, { strict: true });
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import { stabilizePageFile, type PageMeta } from '../src/engine/stabilize.js';
|
||||
// global DOM via jsdom at module load time (required for @tiptap/html under Node).
|
||||
import { markdownToProseMirror } from '@docmost/prosemirror-markdown';
|
||||
import { parseDocmostMarkdown } from '@docmost/prosemirror-markdown';
|
||||
import { ConverterLossError } from '@docmost/prosemirror-markdown';
|
||||
|
||||
// stabilize.ts (SPEC §11 normalize-on-write) was 0% covered (only the gated e2e
|
||||
// touched it). stabilizePageFile is import-testable: build a small ProseMirror
|
||||
@@ -66,6 +67,23 @@ describe('stabilizePageFile — normalize-on-write fixpoint (SPEC §11)', () =>
|
||||
expect(body1).toContain('data-src="/d.drawio"');
|
||||
});
|
||||
|
||||
it('runs the serializer in STRICT mode — an unmappable node throws, not a lossy write (#493)', async () => {
|
||||
// git-sync is the lossless mirror path: a node type the converter has no
|
||||
// case for (here a fabricated one, standing in for a schema type added
|
||||
// without a matching serializer arm) must surface loudly at write time
|
||||
// rather than being silently flattened into a lossy .md file.
|
||||
const content = {
|
||||
type: 'doc',
|
||||
content: [
|
||||
{ type: 'paragraph', content: [{ type: 'text', text: 'ok' }] },
|
||||
{ type: 'quantumWidget', content: [{ type: 'text', text: 'lost?' }] },
|
||||
],
|
||||
};
|
||||
await expect(stabilizePageFile(content, meta)).rejects.toBeInstanceOf(
|
||||
ConverterLossError,
|
||||
);
|
||||
});
|
||||
|
||||
it('already-stable content is unchanged by the pass (idempotent)', async () => {
|
||||
// Plain prose is already a fixpoint; stabilizing it once and twice agree.
|
||||
const content = {
|
||||
|
||||
@@ -15,7 +15,10 @@ export {
|
||||
} from "./markdown-document.js";
|
||||
export type { DocmostMdMeta } from "./markdown-document.js";
|
||||
|
||||
export { convertProseMirrorToMarkdown } from "./markdown-converter.js";
|
||||
export {
|
||||
convertProseMirrorToMarkdown,
|
||||
ConverterLossError,
|
||||
} from "./markdown-converter.js";
|
||||
export type { ConvertProseMirrorToMarkdownOptions } from "./markdown-converter.js";
|
||||
|
||||
export {
|
||||
|
||||
@@ -33,6 +33,26 @@ import {
|
||||
*/
|
||||
const MAX_NODE_DEPTH = 400;
|
||||
|
||||
/**
|
||||
* Thrown by {@link convertProseMirrorToMarkdown} in `strict` mode when it hits a
|
||||
* node or mark type it has no lossless markdown form for (the serializer would
|
||||
* otherwise silently degrade it — drop an unknown mark, flatten an unknown node
|
||||
* to its children). Carries the offending kind/name so a caller (git-sync) can
|
||||
* surface exactly what would have been lost.
|
||||
*/
|
||||
export class ConverterLossError extends Error {
|
||||
readonly kind: "node" | "mark";
|
||||
readonly typeName: string;
|
||||
constructor(kind: "node" | "mark", typeName: string) {
|
||||
super(
|
||||
`convertProseMirrorToMarkdown: unknown ${kind} type "${typeName}" has no lossless markdown representation (strict mode)`,
|
||||
);
|
||||
this.name = "ConverterLossError";
|
||||
this.kind = kind;
|
||||
this.typeName = typeName;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for {@link convertProseMirrorToMarkdown}.
|
||||
*/
|
||||
@@ -46,6 +66,23 @@ export interface ConvertProseMirrorToMarkdownOptions {
|
||||
* path where resolved anchors MUST be preserved for round-tripping.
|
||||
*/
|
||||
dropResolvedCommentAnchors?: boolean;
|
||||
/**
|
||||
* Optional sink for LOSS warnings. When the serializer reaches a node or mark
|
||||
* type it has no dedicated case for, it degrades gracefully (flattens an
|
||||
* unknown node to its children, drops an unknown mark) — historically a SILENT
|
||||
* data loss. When this array is provided, one human-readable message per such
|
||||
* event is pushed here so the caller can observe (and log) what was degraded.
|
||||
* Not provided by default -> behavior is byte-identical to before for existing
|
||||
* callers.
|
||||
*/
|
||||
warnings?: string[];
|
||||
/**
|
||||
* When true, THROW a {@link ConverterLossError} on the FIRST unknown node/mark
|
||||
* instead of degrading silently — a warning becomes a hard error. Used by the
|
||||
* lossless git-sync export path and the converter tests, where an unmapped
|
||||
* type is a bug to surface, not data to quietly drop.
|
||||
*/
|
||||
strict?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -157,6 +194,26 @@ export function convertProseMirrorToMarkdown(
|
||||
// callers (mcp getPage / in-app AI chat) pass it true.
|
||||
const dropResolvedCommentAnchors = options.dropResolvedCommentAnchors === true;
|
||||
|
||||
// Loss reporting for node/mark types with no dedicated serializer case. In
|
||||
// `strict` mode the FIRST such type throws (git-sync, tests); otherwise the
|
||||
// serializer degrades gracefully (as it always has) but records one warning
|
||||
// per unmapped type into the optional sink so the loss is observable, not
|
||||
// silent. Deduped per type so a document with many unknown nodes of one type
|
||||
// produces one message.
|
||||
const strict = options.strict === true;
|
||||
const warningsSink = options.warnings;
|
||||
const seenLossTypes = new Set<string>();
|
||||
const warnLoss = (kind: "node" | "mark", typeName: string): void => {
|
||||
if (strict) throw new ConverterLossError(kind, typeName);
|
||||
if (!warningsSink) return;
|
||||
const key = `${kind}:${typeName}`;
|
||||
if (seenLossTypes.has(key)) return;
|
||||
seenLossTypes.add(key);
|
||||
warningsSink.push(
|
||||
`Unknown ${kind} type "${typeName}" has no lossless markdown form; it was degraded on export.`,
|
||||
);
|
||||
};
|
||||
|
||||
// Escape a value interpolated into an HTML double-quoted attribute value
|
||||
// (textAlign, colors, image src, math `text`, all data-* attrs, etc.). In the
|
||||
// ATTRIBUTE context only the quote that delimits the value and the ampersand
|
||||
@@ -646,6 +703,12 @@ export function convertProseMirrorToMarkdown(
|
||||
}
|
||||
break;
|
||||
}
|
||||
default:
|
||||
// Unknown mark: no dedicated case, so it has no markdown form and
|
||||
// is dropped from the run. Report the loss (throws in strict
|
||||
// mode) then leave the text unwrapped — the historical behavior.
|
||||
warnLoss("mark", String(mark.type));
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1224,7 +1287,11 @@ export function convertProseMirrorToMarkdown(
|
||||
}
|
||||
|
||||
default:
|
||||
// Fallback: process children
|
||||
// Unknown node type: no dedicated case, so the node's identity + attrs
|
||||
// have no lossless markdown form. Report the loss (throws in strict
|
||||
// mode) then degrade by flattening to its children — the historical
|
||||
// graceful fallback.
|
||||
warnLoss("node", String(type));
|
||||
return nodeContent.map(processNode).join("");
|
||||
}
|
||||
};
|
||||
@@ -1348,6 +1415,12 @@ export function convertProseMirrorToMarkdown(
|
||||
t = `<span data-comment-id="${escapeAttr(mark.attrs.commentId)}"${r}>${t}</span>`;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
// Unknown mark on the raw-HTML path: dropped (no HTML form). Report
|
||||
// the loss (throws in strict mode) — same policy as the markdown
|
||||
// path's marks loop above.
|
||||
warnLoss("mark", String(mark.type));
|
||||
break;
|
||||
}
|
||||
}
|
||||
return t;
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
convertProseMirrorToMarkdown,
|
||||
ConverterLossError,
|
||||
} from "../src/lib/markdown-converter.js";
|
||||
|
||||
/**
|
||||
* #493 commit 3 — a node/mark type the serializer has no dedicated case for used
|
||||
* to be degraded SILENTLY (an unknown node flattened to its children, an unknown
|
||||
* mark dropped from the run). The serializer now REPORTS the loss:
|
||||
* - default (non-strict): unchanged graceful degradation, but one warning per
|
||||
* unmapped type is pushed into an optional `warnings` sink so callers can
|
||||
* observe it;
|
||||
* - strict: the FIRST unmapped type throws a ConverterLossError (git-sync +
|
||||
* tests), turning a silent loss into a hard, surfaced error.
|
||||
*
|
||||
* Exercised through the REAL converter (no mock): the observable properties are
|
||||
* the emitted markdown, the warnings collected, and the thrown error.
|
||||
*/
|
||||
|
||||
const doc = (...nodes: any[]) => ({ type: "doc", content: nodes });
|
||||
|
||||
describe("converter loss reporting — unknown node types", () => {
|
||||
const unknownNode = doc({
|
||||
type: "quantumWidget",
|
||||
content: [{ type: "text", text: "inner text" }],
|
||||
});
|
||||
|
||||
it("degrades to children AND records a warning (non-strict, sink provided)", () => {
|
||||
const warnings: string[] = [];
|
||||
const md = convertProseMirrorToMarkdown(unknownNode, { warnings });
|
||||
// Graceful degrade: the child text still survives (historical behavior).
|
||||
expect(md).toContain("inner text");
|
||||
// The loss is now observable.
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toContain("quantumWidget");
|
||||
expect(warnings[0]).toContain("node");
|
||||
});
|
||||
|
||||
it("stays byte-identical for callers that pass no sink (zero behavior change)", () => {
|
||||
const withSink: string[] = [];
|
||||
const a = convertProseMirrorToMarkdown(unknownNode, { warnings: withSink });
|
||||
const b = convertProseMirrorToMarkdown(unknownNode);
|
||||
expect(b).toBe(a); // the sink does not alter the produced markdown
|
||||
});
|
||||
|
||||
it("throws ConverterLossError in strict mode", () => {
|
||||
try {
|
||||
convertProseMirrorToMarkdown(unknownNode, { strict: true });
|
||||
expect.unreachable("strict mode must throw on an unknown node");
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(ConverterLossError);
|
||||
expect((e as ConverterLossError).kind).toBe("node");
|
||||
expect((e as ConverterLossError).typeName).toBe("quantumWidget");
|
||||
}
|
||||
});
|
||||
|
||||
it("dedupes the warning per type (many unknown nodes -> one message)", () => {
|
||||
const warnings: string[] = [];
|
||||
convertProseMirrorToMarkdown(
|
||||
doc(
|
||||
{ type: "quantumWidget", content: [{ type: "text", text: "a" }] },
|
||||
{ type: "quantumWidget", content: [{ type: "text", text: "b" }] },
|
||||
),
|
||||
{ warnings },
|
||||
);
|
||||
expect(warnings).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("converter loss reporting — unknown mark types", () => {
|
||||
const unknownMark = doc({
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "glowing", marks: [{ type: "glow" }] }],
|
||||
});
|
||||
|
||||
it("drops the mark but keeps the text AND records a warning (non-strict)", () => {
|
||||
const warnings: string[] = [];
|
||||
const md = convertProseMirrorToMarkdown(unknownMark, { warnings });
|
||||
expect(md).toBe("glowing"); // text survives, mark silently had no form
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toContain("glow");
|
||||
expect(warnings[0]).toContain("mark");
|
||||
});
|
||||
|
||||
it("throws ConverterLossError in strict mode", () => {
|
||||
expect(() =>
|
||||
convertProseMirrorToMarkdown(unknownMark, { strict: true }),
|
||||
).toThrow(ConverterLossError);
|
||||
});
|
||||
});
|
||||
|
||||
describe("converter loss reporting — known content is never flagged", () => {
|
||||
it("a fully-mapped document produces no warnings and does not throw in strict mode", () => {
|
||||
const d = doc(
|
||||
{ type: "heading", attrs: { level: 2 }, content: [{ type: "text", text: "Title" }] },
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{ type: "text", text: "bold", marks: [{ type: "bold" }] },
|
||||
{ type: "text", text: " and " },
|
||||
{ type: "text", text: "link", marks: [{ type: "link", attrs: { href: "https://x.y" } }] },
|
||||
],
|
||||
},
|
||||
{ type: "bulletList", content: [{ type: "listItem", content: [{ type: "paragraph", content: [{ type: "text", text: "item" }] }] }] },
|
||||
);
|
||||
const warnings: string[] = [];
|
||||
const md = convertProseMirrorToMarkdown(d, { warnings, strict: true });
|
||||
expect(warnings).toEqual([]);
|
||||
expect(md).toContain("## Title");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user