diff --git a/REFERENCE.md b/REFERENCE.md index e65ffa8..7680ddc 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -24,7 +24,7 @@ docx find FILE --batch queries.jsonl [--json] # many independent text/forma docx wc FILE [LOCATOR] [--accepted | --baseline | --current] [--json] docx outline FILE [--style-prefix S] [--json] docx diff FILE --against SRC [--from LOC] [--to LOC] [--comments] [--json] # what changed vs another version -docx validate FILE [--json] # ECMA-376 schema check, per WML part (exit 0 = clean) +docx validate FILE [--json] # ECMA-376 schema check per WML part + cross-part comment consistency (exit 0 = clean) docx raw get FILE --at LOCATOR [--json] # a block's exact XML — the read half of the raw patch loop docx styles FILE [--used] [--at STYLEID] [--json] # the style catalog (not in the body) — what --style NAMEs exist docx styles --catalog [--json] # built-in styles you can apply on demand (Title, Heading1–9, Quote, …), no FILE needed @@ -344,7 +344,7 @@ docx raw edit FILE --at LOCATOR --find S --with S # patch it in place, one gate docx raw replace FILE --at LOCATOR --xml '…' # swap it wholesale (also --batch) docx raw insert FILE --after pN --xml '…' # splice new block XML (or a ) docx raw part list|get|add|replace|edit FILE … # OPC parts, addressed by --name -docx validate FILE [--json] # schema-check against ECMA-376 (transitional) +docx validate FILE [--json] # schema-check against ECMA-376 (transitional) + cross-part consistency ``` For constructs no modeled verb covers — drop caps, TOC field codes, embedded diff --git a/skills/docx-cli/references/commands.md b/skills/docx-cli/references/commands.md index ab62a51..38f97de 100644 --- a/skills/docx-cli/references/commands.md +++ b/skills/docx-cli/references/commands.md @@ -21,7 +21,7 @@ docx info skill # this skill, regenerated from the binary | `outline FILE` | Headings as a locator tree. | | `render FILE` | Render each page to PNG/JPG via Word or LibreOffice — for verifying LAYOUT only. | | `styles FILE` | List/describe styles (`--used`, `--at ID`). | -| `validate FILE` | Schema-check every WML part against the bundled ECMA-376 transitional XSDs (exit 0 = clean). | +| `validate FILE` | Schema-check every WML part against the bundled ECMA-376 transitional XSDs, plus cross-part comment consistency (exit 0 = clean). | | `info ` | `schema` / `locators` / `skill` — reference material, no FILE. | ## Mutate (overwrite FILE in place; `-o PATH` writes a copy; `--dry-run` previews) diff --git a/src/cli/CLAUDE.md b/src/cli/CLAUDE.md index fa3ad50..a82588c 100644 --- a/src/cli/CLAUDE.md +++ b/src/cli/CLAUDE.md @@ -43,7 +43,10 @@ a fragment whose roots are `` goes to the rels part (`cli/raw/rels.ts`, no placement flags), an `--at rIdN`/`rels` locator likewise; everything else is a body block. `docx validate` is a thin shell over the same engine on a bare `Pkg` (no `Document` — it validates parts, -so building the body AST would be pure waste). +so building the body AST would be pure waste), plus the cross-part +`checkPackageConsistency` audit (`@core/raw/package-consistency`) over the +same parsed parts; its findings file under the part they concern, tagged +`kind: "consistency"`, and count toward exit 1. ## `info` subcommands & the generated Agent Skill diff --git a/src/cli/validate/index.ts b/src/cli/validate/index.ts index dffdce9..1058d00 100644 --- a/src/cli/validate/index.ts +++ b/src/cli/validate/index.ts @@ -1,6 +1,11 @@ import type { Pkg } from "@core/ast/document/package"; -import type { XmlNode } from "@core/parser"; import { + type ConsistencyIssue, + checkPackageConsistency, + type PackagePart, +} from "@core/raw/package-consistency"; +import { + type ValidationIssue, validateWmlXml, validationXmlFor, WML_ROOT_TAGS, @@ -27,6 +32,14 @@ itself writes) after markup-compatibility preprocessing (mc:Ignorable extension markup is skipped, exactly as a consumer would). Exit 0 = clean; exit 1 = errors, listed per part. +Also checks the cross-part invariants a per-part schema can't see — each one +schema-valid but repaired or dropped by Word on open: + - every comment has its entry in commentsExtended.xml / commentsIds.xml + (when that part is present) + - every commentRangeStart has its commentRangeEnd and commentReference, + and every commentReference names a comment that exists +These list under the part they concern, tagged "(consistency)". + Useful before/after hand-made XML changes (docx raw, or edits made outside docx-cli) and as a "will Word open this?" first check — note Word TOLERATES some schema violations, so a finding here is "invalid OOXML," not always @@ -49,7 +62,8 @@ export async function run(args: string[]): Promise { const pkg = await openPkgOrFail(filePath); if (typeof pkg === "number") return pkg; - const parts = await collectWmlParts(pkg); + const xmlParts = await collectXmlParts(pkg); + const parts = xmlParts.filter((part) => WML_ROOT_TAGS.has(part.root.tag)); if (isStrictProfile(parts)) { const note = @@ -62,10 +76,13 @@ export async function run(args: string[]): Promise { return EXIT.OK; } - const results = parts.map((part) => ({ - name: part.name, - issues: validateWmlXml(validationXmlFor(part.root)), - })); + const results: { name: string; issues: ReportedIssue[] }[] = parts.map( + (part) => ({ + name: part.name, + issues: validateWmlXml(validationXmlFor(part.root)), + }), + ); + mergeConsistencyIssues(results, checkPackageConsistency(xmlParts)); const errorCount = results.reduce((sum, part) => sum + part.issues.length, 0); if (values.json) { @@ -95,7 +112,7 @@ export async function run(args: string[]): Promise { ); for (const issue of part.issues) { lines.push( - ` ${issue.message}${issue.line !== undefined ? ` (line ${issue.line})` : ""}`, + ` ${issue.message}${issue.line !== undefined ? ` (line ${issue.line})` : ""}${issue.kind ? ` (${issue.kind})` : ""}`, ); } } @@ -103,12 +120,11 @@ export async function run(args: string[]): Promise { return EXIT.GENERAL_ERROR; } -/** Every package part whose root is a WML root, parsed once — the parse - * feeds both the strict-profile sniff and validation. Sorted with +/** Every XML package part, parsed once — the parse feeds the strict-profile + * sniff, schema validation (WML roots only), and the consistency audit (which + * also reads non-WML side-parts like commentsIds.xml). Sorted with * word/document.xml first (the part a reader looks for), the rest by name. */ -async function collectWmlParts( - pkg: Pkg, -): Promise<{ name: string; root: XmlNode }[]> { +async function collectXmlParts(pkg: Pkg): Promise { const names = pkg .listParts() .filter((name) => name.endsWith(".xml")) @@ -117,19 +133,35 @@ async function collectWmlParts( if (b === "word/document.xml") return 1; return a.localeCompare(b); }); - const parts: { name: string; root: XmlNode }[] = []; + const parts: PackagePart[] = []; for (const name of names) { const tree = await pkg.readPart(name); const root = tree?.find((node) => !node.isText && node.tag !== "?xml"); - if (!root || !WML_ROOT_TAGS.has(root.tag)) continue; - parts.push({ name, root }); + if (root) parts.push({ name, root }); } return parts; } +/** File each consistency finding under the part it concerns — appended to + * that part's schema entry, or a new entry for a part the schema pass skips + * (commentsIds.xml has no wml.xsd root). */ +function mergeConsistencyIssues( + results: { name: string; issues: ReportedIssue[] }[], + issues: ConsistencyIssue[], +): void { + for (const issue of issues) { + let entry = results.find((result) => result.name === issue.part); + if (!entry) { + entry = { name: issue.part, issues: [] }; + results.push(entry); + } + entry.issues.push({ message: issue.message, kind: "consistency" }); + } +} + /** ISO Strict re-namespaces every part (`http://purl.oclc.org/ooxml/…`), so the * transitional schemas see unknown content and every finding would be noise. */ -function isStrictProfile(parts: { name: string; root: XmlNode }[]): boolean { +function isStrictProfile(parts: PackagePart[]): boolean { const documentPart = parts.find((part) => part.name === "word/document.xml"); return ( documentPart?.root @@ -137,3 +169,5 @@ function isStrictProfile(parts: { name: string; root: XmlNode }[]): boolean { ?.startsWith("http://purl.oclc.org/ooxml") ?? false ); } + +type ReportedIssue = ValidationIssue & { kind?: "consistency" }; diff --git a/src/core/raw/CLAUDE.md b/src/core/raw/CLAUDE.md index cc37d06..ea6fef4 100644 --- a/src/core/raw/CLAUDE.md +++ b/src/core/raw/CLAUDE.md @@ -97,6 +97,19 @@ mutation is in-memory until `cli/raw/commit.ts` saves after the last gate. WASM at import, so the CLI imports it LAZILY (`cli/raw/commit.ts` dynamic-imports it only when the gate will actually run) — keep it out of static import chains that non-validating paths load. +- [package-consistency.ts](package-consistency.ts) — the cross-part audit + `docx validate` runs beside the schema pass (`checkPackageConsistency` over + the same parsed parts): invariants that are schema-valid per part but that + Word repairs or drops on open. Today: every comment keyed (by its LAST + paragraph's `w14:paraId`) in each PRESENT side-part (`commentsExtended.xml`, + `commentsIds.xml` — an absent one is legal), and comment marker balance + across every story part (range start↔end, range→reference, + reference→existing comment). Ids compare as SETS (an `mc:AlternateContent` + story repeats its markers under Choice and Fallback). It reports only what + Word actually repairs — orphan side-part entries and marker-less comments are + deliberately silent, and the "every fixture validates clean" test is the + false-positive pin. Pure (no WASM), not a raw gate: a raw edit is still + judged on schema errors only. ## Schema provenance diff --git a/src/core/raw/package-consistency.ts b/src/core/raw/package-consistency.ts new file mode 100644 index 0000000..133b115 --- /dev/null +++ b/src/core/raw/package-consistency.ts @@ -0,0 +1,188 @@ +import type { XmlNode } from "../parser"; + +/** Package-level consistency audit — the cross-part invariants no per-part + * schema can see, run by `docx validate` beside the XSD check. Every finding + * is something a schema-valid package can still get wrong and that Word + * repairs (or silently drops) on open: + * + * - **Comment side-parts.** `commentsExtended.xml` (`w15:commentEx`) and + * `commentsIds.xml` (`w16cid:commentId`) key each comment by the + * `w14:paraId` of its LAST paragraph. When a side-part is present, a + * comment with no entry in it is a Word repair trigger (issue #33's + * "unreadable content"). An absent side-part is legal — older producers + * write none — so the check is per present part. + * - **Comment marker balance** across every story part (body, headers, + * footers, notes): a `commentRangeStart` needs its `commentRangeEnd` and a + * `commentReference`, and a reference must name a comment that exists. + * + * Ids are compared as SETS: `mc:AlternateContent` writes the same text-box + * story under both Choice and Fallback, so a marker appearing twice is + * legal. Orphan side-part entries and comments with no body markers are not + * reported — Word drops those quietly, so flagging them would be noise. + * + * The glossary document (`word/glossary/…`, building blocks) is a separate + * story set with its own comment parts, so it's excluded rather than mixed + * into the main document's id spaces. */ +export function checkPackageConsistency( + packageParts: PackagePart[], +): ConsistencyIssue[] { + const parts = packageParts.filter( + (part) => !part.name.startsWith("word/glossary/"), + ); + const comments = collectComments(parts); + return [ + ...checkCommentSideParts(parts, comments), + ...checkCommentMarkers(parts, comments), + ]; +} + +const COMMENTS_ROOT = "w:comments"; + +/** The comment side-parts keyed by paraId: root tag → entry tag + key attr. */ +const SIDE_PARTS = [ + { + root: "w15:commentsEx", + entry: "w15:commentEx", + paraIdAttribute: "w15:paraId", + }, + { + root: "w16cid:commentsIds", + entry: "w16cid:commentId", + paraIdAttribute: "w16cid:paraId", + }, +] as const; + +const STORY_ROOTS = new Set([ + "w:document", + "w:hdr", + "w:ftr", + "w:footnotes", + "w:endnotes", +]); + +function collectComments(parts: PackagePart[]): CommentsSet { + const commentsPart = parts.find((part) => part.root.tag === COMMENTS_ROOT); + if (!commentsPart) return undefined; + const entries: CommentEntry[] = []; + for (const comment of commentsPart.root.findChildren("w:comment")) { + const id = comment.getAttribute("w:id"); + if (id === undefined) continue; + // A comment's threading identity is its LAST `` (see CommentsView). + const paraId = comment + .findChildren("w:p") + .at(-1) + ?.getAttribute("w14:paraId"); + entries.push({ id, ...(paraId ? { paraId } : {}) }); + } + return { part: commentsPart.name, entries }; +} + +function checkCommentSideParts( + parts: PackagePart[], + comments: CommentsSet, +): ConsistencyIssue[] { + if (!comments) return []; + const issues: ConsistencyIssue[] = []; + for (const sidePart of SIDE_PARTS) { + const part = parts.find( + (candidate) => candidate.root.tag === sidePart.root, + ); + if (!part) continue; + const keyed = new Set( + part.root + .findChildren(sidePart.entry) + .map((entry) => entry.getAttribute(sidePart.paraIdAttribute)), + ); + for (const comment of comments.entries) { + if (comment.paraId && keyed.has(comment.paraId)) continue; + issues.push({ + part: part.name, + message: comment.paraId + ? `comment c${comment.id} (paraId ${comment.paraId}) has no entry in ${part.name}` + : `comment c${comment.id} has no w14:paraId, so it has no entry in ${part.name}`, + }); + } + } + return issues; +} + +function checkCommentMarkers( + parts: PackagePart[], + comments: CommentsSet, +): ConsistencyIssue[] { + const markers = { + "w:commentRangeStart": new Map(), + "w:commentRangeEnd": new Map(), + "w:commentReference": new Map(), + }; + for (const part of parts) { + if (!STORY_ROOTS.has(part.root.tag)) continue; + collectMarkers(part.root, part.name, markers); + } + const starts = markers["w:commentRangeStart"]; + const ends = markers["w:commentRangeEnd"]; + const references = markers["w:commentReference"]; + const commentIds = new Set(comments?.entries.map((entry) => entry.id)); + const commentsPartName = comments?.part ?? "word/comments.xml"; + + const issues: ConsistencyIssue[] = []; + for (const [id, part] of starts) { + if (!ends.has(id)) { + issues.push({ + part, + message: `commentRangeStart ${id} has no matching commentRangeEnd`, + }); + } + } + for (const [id, part] of ends) { + if (!starts.has(id)) { + issues.push({ + part, + message: `commentRangeEnd ${id} has no matching commentRangeStart`, + }); + } + } + for (const [id, part] of new Map([...ends, ...starts])) { + if (!references.has(id)) { + issues.push({ + part, + message: `comment range ${id} has no matching commentReference`, + }); + } + } + for (const [id, part] of references) { + if (!commentIds.has(id)) { + issues.push({ + part, + message: `commentReference ${id} points at no comment in ${commentsPartName}`, + }); + } + } + return issues; +} + +/** Record the first part each comment marker id appears in, per marker tag. */ +function collectMarkers( + node: XmlNode, + partName: string, + markers: Record>, +): void { + for (const child of node.children) { + if (child.isText) continue; + const seen = markers[child.tag]; + if (seen) { + const id = child.getAttribute("w:id"); + if (id !== undefined && !seen.has(id)) seen.set(id, partName); + continue; + } + collectMarkers(child, partName, markers); + } +} + +export type PackagePart = { name: string; root: XmlNode }; + +export type ConsistencyIssue = { part: string; message: string }; + +type CommentEntry = { id: string; paraId?: string }; + +type CommentsSet = { part: string; entries: CommentEntry[] } | undefined; diff --git a/tests/cli/validate.test.ts b/tests/cli/validate.test.ts index a4688a3..3560871 100644 --- a/tests/cli/validate.test.ts +++ b/tests/cli/validate.test.ts @@ -91,6 +91,109 @@ describe("every fixture validates clean", () => { }); }); +describe("cross-part consistency (issue #34)", () => { + // Schema-valid packages Word still repairs on open: the per-part XSD check + // can't see a comment missing from its paraId-keyed side-part, or comment + // markers that don't pair up. `validate` reports them beside schema errors. + let commented: string; + + beforeEach(async () => { + commented = join(workspace, "commented.docx"); + await runCli("create", commented, "--text", "hello world"); + await runCli("comments", "add", commented, "--at", "p0", "--text", "n"); + }); + + async function patchPart( + name: string, + patch: (xml: string) => string, + ): Promise { + const pkg = await Pkg.open(commented); + pkg.writeText(name, patch(await pkg.readText(name))); + await pkg.save(); + } + + test("a consistent commented document validates clean", async () => { + const result = await runCli("validate", commented); + expect(result.exitCode).toBe(0); + expect(result.stdout).toMatch(/^valid/); + }); + + test("a comment missing from a present commentsIds.xml is an error", async () => { + const pkg = await Pkg.open(commented); + pkg.writeText( + "word/commentsIds.xml", + '', + ); + await pkg.save(); + const result = await runCli("validate", commented); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain("word/commentsIds.xml: 1 error"); + expect(result.stdout).toMatch( + /comment c0 \(paraId [0-9A-F]{8}\) has no entry in word\/commentsIds\.xml \(consistency\)/, + ); + }); + + test("a present commentsIds.xml that keys the comment passes", async () => { + const comments = await (await Pkg.open(commented)).readText( + "word/comments.xml", + ); + const paraId = comments.match(/w14:paraId="([0-9A-F]{8})"/)?.[1]; + expect(paraId).toBeDefined(); + const pkg = await Pkg.open(commented); + pkg.writeText( + "word/commentsIds.xml", + ``, + ); + await pkg.save(); + expect((await runCli("validate", commented)).exitCode).toBe(0); + }); + + test("a comment range whose reference run was deleted is an error", async () => { + await patchPart("word/document.xml", (xml) => + xml.replace( + /<\/w:rPr><\/w:r>/, + "", + ), + ); + const result = await runCli("validate", commented); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain( + "comment range 0 has no matching commentReference (consistency)", + ); + }); + + test("an unpaired commentRangeStart is an error", async () => { + await patchPart("word/document.xml", (xml) => + xml.replace('', ""), + ); + const result = await runCli("validate", commented); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain( + "commentRangeStart 0 has no matching commentRangeEnd", + ); + }); + + test("a commentReference to a missing comment is an error, filed under its story part", async () => { + await patchPart("word/comments.xml", (xml) => + xml.replace(//, ""), + ); + const result = await runCli("validate", commented, "--json"); + expect(result.exitCode).toBe(1); + const parsed = result.parsed as { + valid: boolean; + parts: { part: string; issues: { message: string; kind?: string }[] }[]; + }; + expect(parsed.valid).toBe(false); + const documentPart = parsed.parts.find( + (part) => part.part === "word/document.xml", + ); + expect(documentPart?.issues).toContainEqual({ + message: "commentReference 0 points at no comment in word/comments.xml", + kind: "consistency", + }); + }); +}); + describe("legacy schema noise self-heals on any save", () => { test("a misnamed and an undeclared w14 comments part are repaired by an unrelated edit", async () => { // Documents produced by earlier docx-cli versions (and some Word files)