fix(parser): parse hand-authored DOCX specs (tab delimiters, centered titles, manual pr-labels)#432
Conversation
Hand-authored specs delimit a typed outline number from its heading with a
TAB, not a space ("1.1<tab>SUMMARY", "A.<tab>General", "1.<tab>Authority").
The parser's authoritative text extractor (source-facts.ts collectInline)
dropped <w:tab/> entirely, de-spacing the number into the title
("1.1SUMMARY"). That defeated every Signal-4 text pattern — all require \s
after the number — so the heading fell through to a wrong signal (indent /
continuation) and its outline label never stripped. The same drop silently
CONCATENATED words split across a tab in body prose ("wireless<tab>signals"
-> "wirelesssignals"), a content-fidelity loss.
Emit '\t' for a content <w:tab/>, skipping property subtrees
(w:pPr > w:tabs > w:tab tab-STOP definitions) so no phantom tab is injected.
This mirrors the merge module (src/merge/extract.ts), which already
normalizes w:tab -> '\t' the same way; the parser was simply missing it.
Corpus A/B (scripts/fixture-ab.ts, 705 fixtures): 28 fixtures change (23
ARCAT + 4 CPI + the failing example), ALL tab-preservation improvements;
0 structural regressions (part-count, note-leak, parse-error, presence all
unchanged). Full unit suite green (1559); gold gate green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dent
A centered title carries a large SYMMETRIC w:ind (centering padding to push
the text into the middle of the text column), not outline depth. Signal 5
(indentation) read the section header "SECTION 26 0513.01" (w:jc=center,
w:ind left=3859) as 3859/576 ≈ 7 → pr6, and the section title as pr5 —
spurious deep-pr roots. Those junk roots inflated the part ordinal, so the
first real PART (GENERAL) rendered as "PART 3" and its articles as "3.1 /
3.2 …", which in turn blocked the author-typed "1.1"/"1.2" outline label
from stripping (the computed label never matched the typed one).
Capture w:jc on DocxParagraph and skip Signal 5 for center/right/end
alignment — a centered/right-aligned paragraph is never a hierarchy node,
so its leftIndent is positioning, not nesting. Left/start/both/distribute
(normal flow) still use indentation.
With this, the section header/title fall to preamble continuations (matching
the well-styled sibling spec), GENERAL/PRODUCTS become PART 1/2, and the
article labels strip cleanly ("### 1.1 SUMMARY").
Corpus A/B: 1/705 fixtures change (the target file), 0 structural
regressions. Unit suite green (1563); lint green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eral")
The article label-strip already removed an author-typed number when it equals
the article's render-derived CSI label. Hand-authored specs type the SAME kind
of label one tier down — a manufacturer/list item "A. General Cable", "1.
Authority …" — which the renderer's own getLabel re-prepends, doubling it to
"A. A. General Cable". Single-token pr labels were never stripped (only
multi-dot outlines were, inline), so the duplicate leaked.
Generalize the label-strip post-pass from articles to all tiers: track every
Signal-4 (manual text-outline) article/pr node, and recurse the assembled tree
mirroring the renderer's ordinal walk (renderChildren), stripping a node's
typed label only when it equals the label getLabel would prepend at its
position. Parts remain handled inline (planPartStrip). The equality gate keeps
the strip conservative — a coincidental value/content ("A. Datum reference
frame", a mis-numbered item) is left verbatim — and it fires only on manual
outlines, never a Word/style-numbered node whose visible text is content.
Corpus A/B: 2/705 fixtures change (CPI_BUSBAR + the failing example), every
change a doubled-label removal; 0 structural regressions. Unit suite green
(1567); lint green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughDOCX parsing now preserves tab delimiters, extracts paragraph alignment, excludes positional alignment from indentation inference, and removes matching manually typed outline labels from eligible inferred nodes. ChangesDOCX outline parsing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DOCX as DOCX XML
participant Extractor as source-facts extractor
participant Parser as paragraph parser
participant Inference as outline inference
participant Tree as parsed tree
DOCX->>Extractor: Supply paragraph text, w:tab, and property subtrees
Extractor->>Parser: Return text with tab delimiters
DOCX->>Parser: Supply w:jc alignment
Parser->>Inference: Provide paragraph text, indent, and jc
Inference->>Tree: Classify nodes and strip matching manual labels
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codex adversarial review (PR #432): the Signal-5 centered-paragraph guard read w:jc only from the paragraph's direct pPr. Word commonly stores a title's alignment in its paragraph STYLE, so a styled centered section title (only <w:pStyle> + <w:ind>, no direct <w:jc>) still resolved jc=undefined and its left indent was misread as outline depth — the exact regression the guard was meant to close, left open for the style-based path. Resolve effective alignment through the basedOn chain, mirroring resolvedNumPr: buildStyleMap now computes resolvedJc, and resolveJustification prefers the direct pPr w:jc, then falls back to the style's resolved alignment. No corpus fixture exercises this path (0/705 change) — it is a robustness completion of the direct-w:jc fix, verified by unit tests for direct/inherited/ absent style alignment and the precedence of a direct override. Unit 1572 + parser-integration 152 green; lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex (GPT-5.5, xhigh) adversarial review — 1 finding, addressed[P2] Resolve style-inherited alignment for Signal 5 — Verdict: valid. Fixed in
CodeRabbit reviewed the prior commits with no actionable comments. It will re-review this push automatically. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Codex closing-gate review (PR #432): the generalized pr-label strip routed pr nodes through planLabelStrip, whose (?=[A-Z]) lookahead is an ARTICLE guard — it tells a heading ("1.2 RELATED SECTIONS") from a decimal value ("1.1 inches"). Pr items are different: a pr node is classified Signal-4 BECAUSE its text opens with a list marker ("A.", "1.", "a."), and the label-equality gate already confirms that marker is this node's label. But pr content is arbitrary prose that commonly starts lowercase or numeric ("a. install anchors", "1. acceptance criteria"), so the uppercase guard wrongly left those matching labels un-stripped — the renderer then re-prepended them, doubling the label. Add a requireUpper flag to planLabelStrip (default true — articles unchanged); pr-tier stripNodeLabel passes false, so pr labels strip on label-equality alone, requiring only that content follows. The article decimal-prose guard is intact. Corpus A/B: 0/705 fixtures change — a robustness completion (no current fixture pairs a matching pr label with lowercase content); unit tests cover the pr lowercase/numeric strip, the intact article guard, and the wrong-label/empty safety cases. Unit suite green; lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex closing-gate review (round 2) — 1 finding, addressed[P2] Use pr-specific stripping for lowercase pr labels — the generalized pr-label strip (commit Verdict: valid. Fixed in
|
Codex final closing-gate review (round 3) — clean ✅
Review loop converged. Summary of this PR's review:
Ready for merge. |
Why
Two near-identical medium-voltage cable specs parsed completely differently:
more-parsing-examples-works.docx(semantic paragraph styles) parsed clean, while its twinmore-parsing-examples-fails.docx— hand-authored with outline numbers typed as literal text, tab-delimited, under a centered header/title — parsed into a garbled tree (section header/title became## PART 1/2, realGENERALbecamePART 3, articles rendered### 3.1 1.1⇥SUMMARY, manufacturer lists doubled labelsA. A. …).Three distinct, pre-existing defects broke the text/indent inference path that hand-authored specs depend on.
What
Three focused parser fixes (one commit each), each gated on the 705-fixture A/B snapshot harness:
<w:tab/>in DOCX text extraction (source-facts.ts). The authoritative text path dropped tabs, de-spacing1.1<tab>SUMMARY→1.1SUMMARY(defeating every Signal-4 pattern, which all require\safter the number) and silently concatenating words split across a tab in body prose (wireless<tab>signals→wirelesssignals) corpus-wide. Mirrorssrc/merge/extract.ts, which already normalizedw:tab → \t.w:ind left=3859) was read as≈7 → pr6, creating junk roots that inflated the part ordinal and blocked article-label stripping. Capturesw:jc; skips indentation for center/right/end.A./1.only when it equals the node's computed CSI label. RemovesA. A. General Cabledoubling.Result: the failing file now renders a well-formed CSI spec (PART 1/2/3 = GENERAL/PRODUCTS/EXECUTION, articles 1.1–1.4 / 2.1–2.8 / 3.1–3.5), matching the shape of its well-authored twin. The twin's output is byte-identical (no regression).
Corpus A/B (
pnpm fixture:snapshot/fixture:diff, 705 fixtures)Out of scope
Remaining pr-tier nesting ambiguities in the failing file (a Word-numbered
":"-terminated lead-in with a manual restarting sub-list landing as siblings) need text-based lead-in nesting — tracked in #431.Testing
w:jcextraction, Signal-5 center/right/justified, pr-label strip + safety)🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #430.
Summary by CodeRabbit
New Features
Bug Fixes