Skip to content

feat(parser): capture DOCX headers/footers into structured config + raw sidecar#483

Merged
thewrz merged 21 commits into
mainfrom
feat/issue-306
Jul 14, 2026
Merged

feat(parser): capture DOCX headers/footers into structured config + raw sidecar#483
thewrz merged 21 commits into
mainfrom
feat/issue-306

Conversation

@thewrz

@thewrz thewrz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Why

Ingress needs to learn real firm/client header/footer layouts from uploaded DOCX files. Recognizable content should become editable structured config; unsupported OOXML should be retained in a raw sidecar with warnings, not silently discarded.

Design: docs/superpowers/specs/2026-06-26-header-footer-fidelity-design.md
ADR: docs/adr/039-header-footer-fidelity.md, docs/adr/068-header-footer-capture-mapping.md

What

Parses word/header*.xml / word/footer*.xml, their relationships, and section-level references (default/first/even, first-page and even/odd settings) into the #208 composition model:

  • Recognized literals, known Word fields (PAGE, NUMPAGES, DATE, etc.), simple rule lines, and common section-number/title fields map to field references, not copied text.
  • Unsupported image/table/raw OOXML content is preserved in a raw.unmodeled sidecar rather than discarded, each entry tagged with a kind (including a new inactiveVariant case for an unpromoted first/even reference whose content is still captured) and paired with a header-footer-content-skipped parse warning.
  • SpecTree.headerFooter round-trips through the parse-worker boundary and is documented in openapi.yaml.
  • New DOCX_HEADER_FOOTER_XML_INVALID error code for genuinely malformed header/footer XML — capture itself never throws for document-content reasons (see design decision 8).

New modules: header-footer-parts.ts (JSZip glob-read I/O), header-footer-relationships.ts, header-footer-field-recognition.ts, header-footer-region.ts, header-footer.ts (orchestrator). Prerequisite extractions to stay under the 400-line file cap: core-metadata.ts, source-detection.ts, and an ast/schemas.tsspec-tree-schemas.ts split.

Design decisions

Corrections made to the pre-spike design based on concrete spike evidence (each independently reproduced, not taken on the spike's word alone — re-confirmed via wc -l/grep at the top of this pass):

  1. index.ts line budget: the original design assumed ONE extraction (detectSource/detectArticleIlvl) bought enough headroom. Spike proved this false — index.ts lands at 448/400 lines after that single extraction plus the header-footer wiring. Added a SECOND prerequisite extraction (core-metadata.ts) AND moved the JSZip glob-read I/O itself into a dedicated header-footer-parts.ts helper rather than inlining it in extractEntries, both to satisfy the file-level 400-line cap and the function-level 50-line cap (extractEntries alone hit 52 lines as originally sketched). Chose extraction over trimming comments/whitespace since code.md requires "extract first," not squeeze.

  2. WARNING_SUGGESTIONS exhaustive map (src/parser/text/index.ts) was entirely absent from the original design's todos despite being a hard TypeScript compile error the moment the new ParseWarningType literal is added. Added as its own line item rather than folding into the schema task, since it's a distinct file/module.

  3. header-footer-schemas.ts (feat(ast): header/footer variants + page-number policy + raw sidecar #302's shipped file) — the original design explicitly forbade touching it. Spike hit a real TS2375/TS2322 type error proving that constraint unworkable: a plain detail: unknown interface isn't structurally assignable into z.json()'s inferred type even via an open .catchall(). Resolved by adding a small, additive, non-breaking field (HeaderFooterUnmodeledEntrySchema + unmodeled on the raw sidecar) rather than working around it with unsafe casts (as unknown as, forbidden by this repo's TS strictness) or abandoning content-preservation (which would violate acceptance criterion 3). Verified no circular-import risk: header-footer-schemas.ts already owns its own local JsonValue const, independent of the ast/schemas.ts / spec-tree-schemas.ts split.

  4. resolveReferenceTargets's Map-keyed-by-target-path return shape was replaced with an array-of-resolved-pairs shape, closing a real (if narrow) correctness gap the pre-spike design didn't consider: two different reference slots resolving to the same physical part would silently collide in a Map. An array with a linear (variant, region) filter is simpler and removes the invariant dependency on target-path uniqueness entirely, at negligible cost given at most ~6 references per document.

  5. Added a 6th unmodeled 'kind': 'inactiveVariant', because the pre-spike design's own prose already required this exact case (an unpromoted first reference still gets its content captured into raw.unmodeled + warning) but its enum had nowhere honest to put it — 'unresolvedReference' would misstate that the reference didn't resolve, when it did (it's the toggle that's off). This is a precision fix, not a scope change.

  6. w:tbl detection: corrected from "anywhere in the paragraph" (pre-spike design's imprecise phrasing) to "the part's root-level direct children" — an OOXML structural fact (tables cannot nest inside w:p) the spike verified by direct implementation, not merely inferred. Captured as a positive fixed fact for the real build and for ADR-068, not left as a probable bug for a future PR to discover.

  7. Internal complexity/line-cap decomposition (collapseComplexFields, buildRegionFromParagraph/captureRegion helpers, parseSectionHeaderFooterInfo, captureHeaderFooter's variant logic) is now named explicitly in the interfaces section as private helper functions each public function must be decomposed into, rather than left as an implicit "the implementer will figure it out" gap — the spike quantified exactly which functions and by how much they exceeded eslint's complexity 10 / sonarjs/cognitive-complexity 10 / max-lines-per-function 50 caps, so the real build budgets for it from the start instead of discovering it at pnpm lint time.

  8. .parse()-failure error-attribution ambiguity (spike finding feat(db): migration runner and core schema #7, a real risk the spike flagged but did not resolve) is now resolved explicitly: guarantee JSON-safety at every unmodeled-detail construction site via the existing compact() helper (already used elsewhere in the module) BEFORE building the composition object, so the final HeaderFooterCompositionSchema.parse() call is expected to always succeed on any real document input; if it nonetheless throws, that is treated as an uncaught internal defect (a bug the test suite should have caught), never remapped to the document-blaming DOCX_HEADER_FOOTER_XML_INVALID code. This keeps captureHeaderFooter's "total, never throws (for document-content reasons)" contract honest rather than silently laundering a capture-code bug as a source-document problem.

  9. Everything the spike explicitly confirmed as correct in the original design (407/392-line prerequisite facts, single openapi.yaml enum site, schemas.ts split boundary cohesion, RunProperties field mapping, HeaderFooterRegionSchema's single-paragraph ceiling, async Promise.all JSZip glob-read pattern, zero MCP contract-map.ts changes, zero regression across the 55 pre-existing index.test.ts tests) is carried forward unchanged — re-litigating settled, verified facts wastes the next agent's budget.

Testing

  • Unit tests pass (pnpm test — 159 files / 2143 tests)
  • Integration tests pass (pnpm test:integration — 130 files / 1321 tests, 10 pre-existing skips)
  • pnpm build (tsc) clean
  • pnpm lint (eslint + tsc --noEmit + prettier --check) clean
  • Manual verification: upload a DOCX fixture with default/first/even headers and footers and confirm the resulting SpecTree.headerFooter composition and raw.unmodeled sidecar
  • CI green

🤖 Co-authored by Claude Sonnet 5. Closes #306

Summary by CodeRabbit

  • New Features
    • DOCX parsing now captures header and footer content (default, first-page, and even-page variants) when present.
    • Parse output can include optional header/footer composition data (e.g., modeled fields, styles, borders, and tab-separated layout).
  • Bug Fixes
    • Introduced a dedicated parse warning when header/footer content is skipped (header-footer-content-skipped) and preserved unmodeled or inactive content for review instead of dropping it.
    • Added clearer handling for malformed DOCX header/footer XML with a specific parser error code.
  • Documentation
    • Added an ADR detailing the header/footer capture mapping behavior and limitations.

thewrz and others added 18 commits July 13, 2026 20:36
Prerequisite for #306's header/footer capture work: NodeTypeSchema through
SpecTreeSchema (~280 lines) move out of schemas.ts into a new
spec-tree-schemas.ts, giving the module headroom for the headerFooter field
and ParseWarningType literal that later tasks in this slice add. Mirrors
commit da3e149 (#474)'s style-schemas.ts split mechanics: barrel and
importers re-pointed, zero behavior change. Pinned by a line-budget test
mirroring router-header-footer.test.ts's eslint-semantics line counter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…detection.ts

Prerequisite, zero-behavior-change extraction for #306: DOCX
header/footer capture needs to land in src/parser/docx/index.ts, which
has no headroom left under the repo's 400-line hard cap. Pulling the
source-fingerprint helpers into their own module is the first of two
planned extractions that buy that room.

Adds a line-budget test (mirroring src/ast/schemas-line-budget.test.ts)
that pins index.ts and source-detection.ts under the cap, so a future
regression names the exact file instead of surfacing only at lint time.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Second of two zero-behavior-change prerequisite extractions for #306
(header/footer capture): the docProps/core.xml parsing helper and its
XMLParser instance move out of index.ts into their own module, dropping
index.ts from 364 to 323 lines and completing the headroom the header/footer
wiring task needs. Extends the line-budget test (mirroring the
source-detection.ts precedent) to pin core-metadata.ts under the 400-line cap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the spike-corrected design for #306's DOCX header/footer
capture pipeline before implementation lands: single-trailing-sectPr
scope, resolveReferenceTargets as variant/region pairs (not a
target-path-keyed map), the inactiveVariant vs unresolvedReference
distinction, root-level w:tbl detection, the tab-boundary KNOWN
AMBIGUITY split, core.xml-literal-only field recognition, the
compact()-json-safe raw sidecar, and the additive
HeaderFooterUnmodeledEntrySchema edit to header-footer-schemas.ts
(#302) that the original task design assumed was unnecessary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ar (#306)

Header/footer capture (#306) needs somewhere to preserve content it can't
map into a typed field — the existing raw.warnings only holds message
strings, not the content itself. Adds an additive, backward-compatible
HeaderFooterUnmodeledEntrySchema (variant/region/kind/detail, six kinds
incl. `inactiveVariant` for a resolved-but-toggled-off page variant) and
a raw.unmodeled array field, per ADR-068. Every pre-#306 composition
value continues to parse unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ree.headerFooter

Wires the #306 header/footer capture output into the shared AST shapes so
the (still-unbuilt) DOCX capture orchestrator has a typed field to populate:
a new ParseWarningType literal mirroring the table-content-skipped (#293)
precedent, and an exactOptional SpecTree.headerFooter (parse-output only,
no DB/REST/MCP persistence in this slice — ADR-068). Also extends the
WARNING_SUGGESTIONS exhaustive map and the pinned ParseWarningType snapshot
in parser/docx/types.test.ts, both required by this addition.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…footer parser types (#306)

Reserves a ParserErrorCode for malformed-but-present word/settings.xml,
document.xml.rels, or header*/footer*.xml, strictly scoped to source-XML
parse failures (never an internal capture-code shape defect — see
error.test.ts and ADR-068). Adds the parser-local header/footer types
(HeaderFooterReference, RelationshipMap, SectionHeaderFooterInfo,
DocumentSettingsInfo, ResolvedHeaderFooterReference,
HeaderFooterUnmodeledEntry) that header-footer-relationships.ts,
header-footer-region.ts, and header-footer.ts will build on.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reads word/document.xml.rels, the trailing body-level w:sectPr
(header/footer references, titlePg, pgNumStart, additional-section-break
detection), and word/settings.xml (evenAndOddHeaders), per ADR-068.
resolveReferenceTargets returns an array of (reference, target) pairs
rather than a Map keyed by target path, so two reference slots resolving
to the same physical part are never collided into one entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds OOXML complex-field collapsing (w:fldChar begin/separate/end),
PAGE/DATE field-code recognition, literal spec section-number/title
matching against core.xml identity, and RunProperties ->
HeaderFooterVisualStyle mapping (ADR-068). Pins the "literal match
only, never a guessed field reference" invariant with dedicated
substring/partial-match regression tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Captures a single header/footer OOXML part into at most one
HeaderFooterRegion ({left, center, right} cells split on tab
boundaries, plus a rule-line border passthrough), preserving every
unsupported/unrecognized content item (tables, images, extra
paragraphs, unrecognized field codes, >2-tab overflow) as a stamped,
JSON-safe unmodeled entry instead of silently dropping it (ADR-068,
acceptance criteria 3/4).

Pins the two ADR-068 invariants directly: a part contributes at most
one captured region (a second content-bearing paragraph is unmodeled,
never merged), and w:tbl detection scans only the part's root-level
direct children, never a paragraph's run sequence (OOXML tables cannot
nest inside w:p). The 3+ tab-stop case is pinned as a KNOWN AMBIGUITY
test per CLAUDE.md's OOXML ambiguity rule.

Exports header-footer-field-recognition.ts's extractTextLikeValue for
reuse in cell-text extraction, avoiding a third copy of the same
fast-xml-parser text-node unwrapping helper.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Isolates the JSZip glob-read I/O for word/header*.xml and
word/footer*.xml parts into its own module, discovering parts by
regex (no fixed-name assumption — Word numbers header/footer parts
arbitrarily and non-sequentially) and returning them keyed by full
zip path, matching resolveReferenceTargets' resolved target shape.

Keeps the coming extractEntries wiring in index.ts under eslint's
50-line function cap (ADR-068 spike finding #1) by moving the
multi-part-read Promise.all logic out of index.ts entirely. Also
adds the module to line-budget.test.ts's per-file 400-line invariant
list, alongside header-footer-region.ts (missing from an earlier
commit).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires relationship resolution (header-footer-relationships.ts) and
per-part content capture (header-footer-region.ts) into
captureHeaderFooter, a single entry point that resolves each
default/first/even header/footer variant, gates first/even on the
section's own titlePg/evenAndOddHeaders toggle, and preserves every
unresolved/inactive/unmodeled item in raw.unmodeled with exactly one
aggregate ParseWarning at the tree level (ADR-068). The composition is
built as a loosely-typed intermediate and validated once via
HeaderFooterCompositionSchema.parse() so a captured internal defect is
never misattributed to the source document.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires captureHeaderFooter (ADR-068) into the DOCX pipeline: extractEntries
now glob-reads word/header*.xml/word/footer*.xml (header-footer-parts.ts)
alongside word/settings.xml and word/_rels/document.xml.rels, and
runPipeline calls captureHeaderFooter with meta.section/meta.title (core.xml
literal identity, never a content-inference guess) after tree/meta resolve.
hf.warnings compose into the aggregate warnings array (mirrors the existing
table-content-skipped pattern) and hf.composition lands on SpecTree.headerFooter
only when non-empty, matching the hiddenTables conditional-spread idiom.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r boundary (#306)

Zod's default 'strip unknown keys' behavior would otherwise silently drop
every captureHeaderFooter() result at the Piscina cross-thread boundary
before it reaches the API caller or DB — the same failure class fixed for
hiddenTables in #293. Adds a regression test pinning the round-trip and
declares headerFooter on workerOutputSchema.tree.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…erFooter (#306)

Wires the two remaining contract-surface gaps for header/footer capture
(src/parser/docx/header-footer.ts): the ParseWarning.type enum gains
header-footer-content-skipped alongside table-content-skipped, and
SpecTree gains a headerFooter field referencing the existing
HeaderFooterComposition component (ADR-040/#302), sibling to
hiddenTables. Pinned with a structural unit test against the raw
(un-dereferenced) spec so the enum literal can't silently duplicate
across $ref sites or go undocumented.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…er capture (#306)

The three existing header/footer wiring tests in index.test.ts covered
literal text and the inactiveVariant toggle, but never exercised an
actual unsupported content type through the full parseDocx pipeline —
leaving acceptance criteria 3 ("unsupported image/table content is
preserved in raw sidecar and warned") and 4 ("no unsupported H/F
content is silently discarded") unpinned at the module boundary, even
though header-footer-region.ts already covers them in isolation.

Adds an end-to-end test with a header part containing a drawing run
and a root-level w:tbl: asserts no fabricated (empty) variant region
is produced, both items land in raw.unmodeled, raw.warnings has one
line per unmodeled item, and the aggregate
header-footer-content-skipped warning surfaces on the tree.

No DB/REST/MCP persistence of SpecTree.headerFooter exists in this
slice — confirmed by inspection: insertTree (paragraphs.ts) and
upsertParsedSpecRow (specs.ts) only touch tree.parts/section/title,
mirroring the pre-existing hiddenTables precedent. Not asserted here
directly since src/parser/docx/ tests must not cross into src/db/
(module-boundary rule).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#306)

The 16 preceding commits implemented DOCX header/footer capture end to
end, but the design's own invariant — "src/mcp/contract-map.ts requires
zero new entries" — was only verified by inspection and by the generic
INV-1/INV-2/INV-2b tests, which already cover it incidentally rather
than pinning it for this feature specifically.

Adds a regression test driving the parse_document MCP tool with a real
header/footer DOCX (via dolanmiu/docx's Header/Footer builders, not
synthetic XML) and asserting the response never exposes headerFooter —
it stays parse-output only, per ADR-068 and ADR-064's persistence
scope. Verified RED-then-GREEN by temporarily adding
`response['headerFooter'] = tree.headerFooter` to buildParseResponse:
the new test failed for the right reason, then passed once reverted —
no production code needed changing, confirming the contract-map
mapping ('post /parse' -> 'parse_document') already covers this
without modification.

Full suite green: 159/159 unit test files (2117 tests),
130/130 non-skipped integration test files (1321 tests).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…OCX corpus (#306)

Task 19 asked to verify captureHeaderFooter is a clean no-op on the
real corpus via pnpm fixture:snapshot/fixture:diff, but the FixtureRecord
snapshot shape doesn't track headerFooter at all (only parts/noteLeaks/
refs/render/error), so a snapshot diff alone can't pin this feature's
own contract — it can only prove rendering is unaffected, which by
inspection it already provably is (renderMarkdown never reads
tree.headerFooter).

Adds a standing corpus-sweep regression test (matching the existing
"every real spec parses to 3 parts" pattern in this file): every real
DOCX in docs/references either has no headerFooter, or one that
round-trips through HeaderFooterCompositionSchema without throwing.
Verified RED-then-GREEN against the local corpus (39 real DOCX,
gitignored/not committed) by temporarily injecting an invalid
`kind` into captureHeaderFooter's unmodeled array: all 39 new tests
failed with a ZodError at the exact buildComposition .parse() boundary,
then passed again once reverted.

Also ran pnpm fixture:snapshot/fixture:diff A/B (this branch vs. the
d5f7797 merge-base, corpus copied into an isolated worktree): 0/705
fixtures changed (39 DOCX + 666 .SEC), confirming zero regression to
existing render/parts/notes/refs output anywhere in the corpus.

Surfaced a pre-existing, unrelated failure while the corpus was
present locally: paring-fixes.integration.test.ts's note-banner count
assertion fails identically on the d5f7797 merge-base (verified via
the same isolated worktree), already tracked by #468/#471 — left
untouched here per the single-demonstrable-change rule.

Full suite green with the corpus absent (its normal, CI-matching
state): 159/159 unit test files (2117 tests), 130/130 non-skipped
integration test files (1321 tests).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

DOCX parsing now captures header/footer content into structured composition data, preserves unsupported content in a raw sidecar, emits parse warnings, and exposes the result through validated AST and worker outputs.

Changes

Header/footer capture

Layer / File(s) Summary
Contracts and capture rules
docs/adr/..., openapi.yaml, src/ast/*
Adds ADR-068 rules, raw unmodeled-entry schemas, SpecTree.headerFooter, and the new parse-warning contract.
DOCX inputs and metadata
src/parser/docx/core-metadata.ts, src/parser/docx/header-footer-*.ts, src/parser/docx/types.ts
Reads metadata, relationships, settings, section references, and header/footer XML parts.
Field and region modeling
src/parser/docx/header-footer-field-recognition.ts, src/parser/docx/header-footer-region.ts
Recognizes fields, maps styles, splits paragraph cells, and preserves unsupported content.
Composition orchestration
src/parser/docx/header-footer.ts
Builds variants, preserves unresolved/inactive/duplicate content, validates compositions, and aggregates warnings.
Pipeline wiring and validation
src/parser/docx/index.ts, src/lib/parse-worker.ts, src/parser/docx/*test.ts, src/api/*, src/mcp/*
Connects capture to parsing and worker boundaries and adds schema, integration, corpus, contract, and line-budget tests.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

  • #306 — Directly implements the DOCX header/footer structured capture and raw-sidecar objectives.
  • #301 — Parent issue for the DOCX header/footer ingress work.
  • #484 — Concerns the region capture and rule-line behavior implemented here.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #306 by parsing H/F parts, relationships, section refs, first/even settings, structured mapping, raw preservation, warnings, and v2 output.
Out of Scope Changes check ✅ Passed The added docs, schema updates, tests, and supporting parser modules all align with the header/footer capture feature.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: DOCX header/footer capture into structured config with a raw sidecar.

Comment @coderabbitai help to get the list of available commands.

thewrz and others added 2 commits July 13, 2026 23:38
…, pgNumStart drop

Addresses 7 code-review findings on the #306 header/footer capture
(ADR-068):

- matchKnownSectionField no longer matches literal header/footer text
  against core-metadata.ts's 'unknown' fallback sentinel — a document
  with missing/corrupt docProps/core.xml could otherwise have ordinary
  text literally reading "unknown" fabricated into a sectionNumber/
  sectionTitle field reference. The sentinel is now a shared export
  (UNKNOWN_SECTION_IDENTITY) instead of a duplicated literal.
- parseSectionHeaderFooterInfo/parseDocumentSettings now read
  w:titlePg/w:evenAndOddHeaders as proper CT_OnOff toggles (@w:val),
  matching this codebase's established convention (resolver.ts's
  toggle(), comments.ts's isStrikeOn()) instead of presence-only checks
  — an explicit <w:titlePg w:val="0"/> now correctly reads as off.
- sectionInfo.pgNumStart (w:pgNumType/@w:start) is no longer silently
  discarded: it's preserved under raw.pgNumStart with a raw.warnings
  line. It is not promoted to composition.pageNumbering.startAt because
  PageNumberingSchema.mode is required and is a cross-document policy
  decision this single-document capture cannot infer (ADR-068 updated
  to record this correction).
- Pinned the previously-untested boundary invariant that buildComposition's
  final HeaderFooterCompositionSchema.parse() call is never remapped to
  DOCX_HEADER_FOOTER_XML_INVALID; buildComposition is now exported so the
  invariant can be tested directly, matching error.test.ts's existing
  cross-reference to this file.
- Strengthened the malformed-settings.xml propagation test to assert
  ParserError/code, not just the message substring, matching its sibling
  tests in header-footer-relationships.test.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#306)

Addresses 6 code-review findings on the #306 header/footer capture
(ADR-068):

- header-footer-region.ts's runsOf() only scanned a paragraph's direct
  w:r children, so header/footer content wrapped in w:hyperlink,
  tracked-change wrappers (w:ins/w:del), or a w:sdt content control was
  invisible to capture and silently dropped with no raw.unmodeled entry
  and no warning. It now reuses document.ts's collectRuns (exported for
  this purpose), the same deep-scan traversal already used for ordinary
  body paragraphs.
- header-footer.ts's findResolvedRef used Array.find(), so a second
  w:headerReference/w:footerReference resolving to the same
  (variant, region) slot as an already-resolved one had its target
  silently discarded. findResolvedRefs now returns every match; a new
  buildRegionSlot helper captures the first and folds any additional
  resolved reference in as a duplicate unmodeled entry (this also
  de-duplicates buildVariantForKind's previously-repeated header/footer
  wiring).
- toHeaderFooterVisualStyle() was implemented and unit-tested but never
  called, so run-level formatting (bold/italic/color/font/caps) on
  captured header/footer text was always silently lost.
  header-footer-region.ts now maps the first styled run in each cell
  onto HeaderFooterCell.style (a documented per-cell simplification —
  HeaderFooterCellSchema models one style per cell, not one per run).
- index.ts's no-core.xml fallback hardcoded the literal 'unknown'
  instead of importing core-metadata.ts's exported
  UNKNOWN_SECTION_IDENTITY sentinel.
- corpus-parts.integration.test.ts's header/footer sweep re-asserted
  HeaderFooterCompositionSchema.parse(tree.headerFooter).not.toThrow(),
  which can never independently fail (tree.headerFooter was already
  validated once by that exact schema inside buildComposition). Replaced
  with a genuinely independent, corpus-wide check: the cross-field
  invariant between raw.warnings and the tree-level aggregate
  'header-footer-content-skipped' warning, which the schema itself
  cannot verify.
- header-footer.test.ts's "never throws for document-content reasons"
  block only fed malformed document.xml/rels/settings.xml through
  captureHeaderFooter, never a malformed header/footer PART itself —
  added a test pinning that captureRegion's XML-parse throw propagates
  unchanged through buildVariant at the orchestrator boundary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@thewrz

thewrz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot added style-fidelity:2 Style fidelity WT-2: effective-style cascade resolver (shipped, PR #148) style-fidelity:3 Style fidelity WT-3: template import from DOCX + consensus derivation labels Jul 14, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/mcp/contract.integration.test.ts`:
- Around line 219-220: Remove the Record<string, unknown> cast from the payload
assignment in the contract integration test and assert the absence of
headerFooter directly on the unknown value returned by parsePayload(result).
Preserve the existing Object.keys and negative containment assertion without
introducing another cast.

In `@src/parser/docx/core-metadata.ts`:
- Around line 29-47: Update the metadata parsing flow around coreParser.parse in
the try block to call XMLValidator.validate(xml) before parsing. When validation
fails, use the existing warning/error handling for unreadable core metadata and
avoid returning the unknown fallback; only invoke parse after validation
succeeds.

In `@src/parser/docx/header-footer-region.ts`:
- Around line 291-317: Update assignSegmentsToCells so the overflow warning is
emitted only when at least one segment after index 2 contains content, rather
than whenever segments.length exceeds 3. Preserve the existing unmodeled warning
details and cell assignment behavior for genuine overflow while ignoring empty
trailing segments.

In `@src/parser/docx/header-footer.ts`:
- Around line 11-13: Update the ParseWarning type import in header-footer.ts to
come from the ../../ast/index.js barrel, matching HeaderFooterCompositionSchema
and HeaderFooterComposition, and remove the direct ../../ast/types.js import.

In `@src/parser/docx/index.test.ts`:
- Around line 984-991: Declare the missing OOXML namespace prefixes in both test
fixtures: update docWithSectPr in src/parser/docx/index.test.ts at lines 984-991
to add the r namespace on the document root, and update the header fixture at
lines 1066-1070 to add the wp namespace on the header root. No other fixture
behavior should change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d3ca6e84-fe45-4e0f-bd07-c5bd394f4ac8

📥 Commits

Reviewing files that changed from the base of the PR and between d5f7797 and a07ea02.

📒 Files selected for processing (36)
  • docs/adr/068-header-footer-capture-mapping.md
  • openapi.yaml
  • src/api/header-footer-warning-openapi.test.ts
  • src/ast/header-footer-schemas.test.ts
  • src/ast/header-footer-schemas.ts
  • src/ast/index.ts
  • src/ast/schemas-line-budget.test.ts
  • src/ast/schemas.test.ts
  • src/ast/schemas.ts
  • src/ast/spec-tree-schemas.ts
  • src/ast/types.ts
  • src/lib/parse-worker.test.ts
  • src/lib/parse-worker.ts
  • src/mcp/contract.integration.test.ts
  • src/parser/docx/core-metadata.ts
  • src/parser/docx/corpus-parts.integration.test.ts
  • src/parser/docx/document.ts
  • src/parser/docx/header-footer-field-recognition.test.ts
  • src/parser/docx/header-footer-field-recognition.ts
  • src/parser/docx/header-footer-parts.test.ts
  • src/parser/docx/header-footer-parts.ts
  • src/parser/docx/header-footer-region.test.ts
  • src/parser/docx/header-footer-region.ts
  • src/parser/docx/header-footer-relationships.test.ts
  • src/parser/docx/header-footer-relationships.ts
  • src/parser/docx/header-footer.test.ts
  • src/parser/docx/header-footer.ts
  • src/parser/docx/index.test.ts
  • src/parser/docx/index.ts
  • src/parser/docx/line-budget.test.ts
  • src/parser/docx/source-detection.ts
  • src/parser/docx/types.test.ts
  • src/parser/docx/types.ts
  • src/parser/error.test.ts
  • src/parser/error.ts
  • src/parser/text/index.ts

Comment thread src/mcp/contract.integration.test.ts Outdated
Comment thread src/parser/docx/core-metadata.ts
Comment thread src/parser/docx/header-footer-region.ts
Comment thread src/parser/docx/header-footer.ts Outdated
Comment thread src/parser/docx/index.test.ts
…CodeRabbit)

Address the 5 CodeRabbit findings on PR #483:

- core-metadata: fast-xml-parser's parse() is lenient and accepts malformed
  markup (unclosed/mismatched tags) without throwing, so a truncated core.xml
  could silently scrape a section value from a corrupt file with no warning.
  Validate with XMLValidator before parsing and route invalid XML through the
  same core-metadata-unreadable path as an outright throw. Extract the shared
  unreadable fallback to avoid duplicating the warning. Regression test pins the
  unclosed-root case.
- header-footer-region: the "extra content folded into right" overflow warning
  fired on segments.length > 3 alone, so a bare trailing tab (empty 4th segment)
  produced a spurious warning. Gate it on whether an overflow segment actually
  carries content. Regression test pins the bare-trailing-tab case.
- header-footer: import ParseWarning from the ../../ast/index.js barrel, matching
  the file's other ast imports and the documented module-boundary rule.
- contract test: drop the Record<string, unknown> cast; assert absence of
  headerFooter directly on the unknown parsePayload result via toHaveProperty.
- index.test fixtures: declare the xmlns:r (document root) and xmlns:wp (header
  root) namespace prefixes the fixtures reference, so they are OOXML-well-formed.

Co-Authored-By: Claude <noreply@anthropic.com>
@thewrz

thewrz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Second adversarial review (GPT-5.5, xhigh) — dispositions

Ran a second independent reviewer over the branch diff. Three findings; dispositions below.

[P1] Deprecated XMLValidator fails enforced lint — HANDLED (a39b3b9).
fast-xml-parser's XMLValidator is flagged sonarjs/deprecation (relocated to a separate fast-xml-validator package in 5.x). It still ships and works in the pinned version, so rather than take on a whole new dependency (added attack surface) for one core.xml validity check, it is used with a scoped, justified eslint-disable-next-line sonarjs/deprecation on the import and call site. pnpm lint passes locally.

[P2] w:fldSimple fields captured as literals, not field references — DECLINED here, filed #485.
Confirmed: collectRuns flattens the inner cached-result run and drops the w:fldSimple wrapper + w:instr, so a simple PAGE/DATE field is captured as a literal (and unrecognized simple fields get no unmodeled warning). The cached text is preserved (not lost), so this is a field-recognition gap, not data loss. Full simple-field recognition is a feature addition beyond this PR's complex-field scope and warrants its own tests — tracked in #485.

[P2] Standalone border-only rule-line paragraph silently dropped — DECLINED here, filed #484.
Confirmed: a rule line authored as an empty paragraph with w:pBdr and no runs is not content-bearing, so captureRuleLine is never reached and it is dropped with no raw.unmodeled entry — an ADR-068 criterion-4 gap for that shape. A faithful fix promotes the standalone rule line into the region model (changing the "first content-bearing paragraph" rule-line rule) and warrants a short ADR-068 addendum + regression tests, so it is tracked in #484 rather than expanding this PR.

Both P2s are genuine but edge-case OOXML shapes (Word overwhelmingly authors border-on-text and complex fields); keeping them out of this already-large, CI-green PR follows the one-demonstrable-change / separate-issue conventions.

@thewrz

thewrz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(parser): capture DOCX headers/footers into structured config + raw sidecar

1 participant