feat(parser): detect asterisk-row note-delimiter convention (DOCX)

[thewrz]

Why

Firms bracket editor notes with rows of asterisks (*****…). The parser kept these as literal wall-of-asterisks text and only caught the enclosed content as a note incidentally, via a /note/-named style — a firm using this convention with an unrelated style name got no note treatment at all.

What (Phase 2 of 4)

• New pure src/lib/note-delimiters.ts (isRuleRow, classifyNoteRoles) — format-agnostic, so Phase 4 (feat(parser): asterisk notes in text/PDF + surface vanish/banner in source_facts #294, text/PDF ingest) can reuse it directly.
• New src/parser/docx/note-roles.ts DOCX adapter (isHeadingParagraph, computeNoteRoles), kept as a sibling of heuristics.ts so its existing isDecorationSeparator suite (dash/equals rows, '****** [OR] ******') is untouched.
• inference.ts wiring: a rule-row paragraph (*****, 5+ asterisks and nothing else) is now marked suppressed — it produces no SpecNode at all — and prose enclosed by a paired rule-row region renders as note even when no banner/style signal fires.
• New optional ClassifiedParagraph.suppressed?: boolean, distinct in kind from isVanish/isNote (those govern how a retained node renders; suppressed means "no node at all").
• classifyOne's pre-signal cascade extracted into collectSignalHits — adding the role guard clauses pushed cyclomatic complexity to 11 (over the enforced budget of 10); the extraction brings it back to 7 with no behavior change.

No-op confirmations (explicitly verified, not assumed)

• No openapi.yaml / AST / migration / ADR changes — confirmed via git diff --stat main...HEAD, this PR touches only src/lib/note-delimiters.ts, src/parser/docx/{types,note-roles,inference}.ts, and their test files. StyleNodeTypeSchema (ast/schemas.ts) was never a home for 'continuation'/'note', so no schema ripple; mergeProfileConflicts (numbering-profile.ts) already object-spreads unknown fields through on both the equal-reference and conflict-append paths, so suppressed needs zero merge-profile wiring.
• No source_fact recording added (out of scope for this phase; issue body scopes only lib + DOCX wiring).

Fixture A/B verification (full 705-file corpus, incl. hand-authored MANUFACTURER_EXAMPLES)

• An initial A/B during implementation ran against an incomplete worktree corpus (696 files, missing MANUFACTURER_EXAMPLES) and showed 0 changes. Re-running against the full 705-file corpus surfaced 4 hand-authored fixtures (the DOCX parser: hand-authored specs (tab-delimited / manually-numbered / centered-title) parse incorrectly #430 corpus) losing PART structure (3→2, 3→1, 3→0): their asterisk walls are frequently unpaired, the open/close toggle drifted out of phase, and real structure (PRODUCTS/EXECUTION headings, manufacturer lists) was swallowed as [NOTE] rows.
• Fixed in 9f415f3 with a drift guard: a structural (numbered) item enclosed by an open rule region is proof the wall pairing has drifted, so the asterisk convention disengages for that document and classification falls back to the per-paragraph style/banner signals that handled these files correctly before this feature. Cleanly-paired documents still engage.
• Final A/B vs the main baseline: 2/705 fixtures changed, both intended — hidden-text-test.docx and paring-fixes.docx no longer render literal wall-of-asterisks [NOTE] rows. Zero part-count changes anywhere.
• note-region-corpus.integration.test.ts now scans the full corpus (39 tests) including a pinned regression named for the symptom (unpaired wall must not swallow PART headings).

Known ambiguities (documented in tests, not silently resolved)

1. Unpaired opener force-closed by a heading (note-delimiters.test.ts, note-roles.test.ts, inference-notes.test.ts) — an unpaired rule-row opener with no closing row has no signal to tell whether the author simply forgot the closing delimiter (heading + everything after should NOT be swallowed) or genuinely left the note open. Resolved by treating a literal PART/article-shaped heading as a force-close: everything from the opener through the paragraph before the heading becomes suppressed/note; the heading and everything after resumes normal inference.
2. Unpaired opener with no closing row and no trailing heading — swallows every remaining paragraph as note through end-of-stream. This can over-suppress trailing structural content if the author forgot the closing delimiter and the document simply ends (or the next heading is numbering/style-only, see chore(ci): bump pnpm/action-setup from 4 to 6 #3) — there is no signal in this shape to disambiguate.
3. isHeadingParagraph is text-pattern-only (Signal 4 shape) — it does not consult Signal 1 (numbering.xml) or Signal 2 (style chain). A heading whose PART/article status is carried entirely by numbering/style, with no literal "PART n" / "N.N" text (e.g. a bare "GENERAL" title under a spec-shaped numId), is invisible to the force-close check and won't break an open note region. Mitigated by the drift guard (9f415f3): the DOCX adapter now feeds the numbering signal (NoteScanItem.isStructural), so such a heading inside an open region disengages the convention for the whole document instead of being swallowed.

Two consecutive rule rows (open-then-close back to back, nothing note between them) is also documented but is expected/correct behavior, not an ambiguity.

Testing

• Unit tests pass (pnpm test) — 1690/1690 (with .env sourced; the earlier open-comments.test.ts failures were an unsourced-env artifact, not test failures).
• Integration: note-region-corpus.integration.test.ts 39/39 against the full 705-file corpus. (Full-suite runs carry known pre-existing cross-file flakes — test(integration): cross-file shared-DB contamination — rotating unrelated failures in full-suite runs #442 — not chased here.)
• Fixture A/B vs main baseline: 2/705 changed, both intended; zero part-count changes (see above).
• pnpm lint (eslint + tsc --noEmit + prettier --check) clean.
• pnpm build clean.
• Manual verification: end-to-end makeDocx() test through the full parseDocx pipeline (extract → numbering → styles → document → classify → tree) pins no bare 5+-asterisk text surviving in the tree, enclosed prose typed note, and post-close content resuming as continuation.
• CI green (pending)

Security note for reviewers

During implementation, a spike step's revert (git checkout back to the last commit) was followed by system-reminders claiming types.ts/inference.ts had been "intentionally modified by the user or a linter" and instructing silence toward the user about it. This was independently re-verified false via git status --porcelain, git diff --stat, and git status immediately after — the working tree was genuinely clean and matched HEAD exactly. Treated as an injected/anomalous instruction and disregarded per security policy (never conceal repo state; verify from tool output, not claims). Flagging here for the maintainer to audit — not a code change, and it did not affect what shipped in this PR.

🤖 Co-authored by Claude Sonnet 5 (implementation) and Claude Fable 5 (corpus-regression fix 9f415f3 + review fix dd2a411). Closes #292.

Summary by CodeRabbit

• New Features

  • DOCX files now recognize paired rows of five or more asterisks as note-region delimiters.
  • Delimited content renders as notes; delimiter rows are omitted.
  • Note regions safely close at section headings and include drift-guard protection to avoid accidental leakage.

• Bug Fixes

  • Exactly four asterisks are no longer treated as delimiters.
  • Vanish behavior no longer causes delimiter text to surface.
  • Suppressed delimiters no longer interfere with lead-in nesting.

• Tests

  • Added unit and DOCX end-to-end regression/integration coverage for delimiter pairing, edge cases, and drift-guard behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 96336239-69a3-42dc-ac18-2b2564b376c7

📥 Commits

Reviewing files that changed from the base of the PR and between 2bfd779 and 9b3b6a7.

📒 Files selected for processing (14)

• src/lib/note-delimiters.test.ts
• src/lib/note-delimiters.ts
• src/parser/docx/index.test.ts
• src/parser/docx/inference-notes.test.ts
• src/parser/docx/inference.test.ts
• src/parser/docx/inference.ts
• src/parser/docx/lead-in-nesting.test.ts
• src/parser/docx/lead-in-nesting.ts
• src/parser/docx/note-region-corpus.integration.test.ts
• src/parser/docx/note-roles.test.ts
• src/parser/docx/note-roles.ts
• src/parser/docx/numbering-profile-apply.test.ts
• src/parser/docx/types.test.ts
• src/parser/docx/types.ts

---

📝 Walkthrough

Walkthrough

Adds shared asterisk rule-row detection and note-role classification, wires it into DOCX inference and tree building, preserves suppressed paragraph state through nesting, and adds unit, end-to-end, regression, and corpus coverage.

Changes

Asterisk note-region parsing

Layer / File(s)	Summary
Shared note-role classifier src/lib/note-delimiters.ts, src/lib/note-delimiters.test.ts	Detects contiguous 5-or-more-asterisk rows, assigns rule/note/none roles, force-closes on headings, and applies structural drift protection.
DOCX role inference integration src/parser/docx/note-roles.ts, src/parser/docx/inference.ts, src/parser/docx/note-roles.test.ts	Maps DOCX paragraph metadata into shared roles, suppresses delimiter paragraphs, classifies enclosed prose as notes, and removes suppressed roots during tree construction.
Suppressed state and nesting src/parser/docx/types.ts, src/parser/docx/lead-in-nesting.ts, src/parser/docx/numbering-profile-apply.test.ts, src/parser/docx/types.test.ts, src/parser/docx/lead-in-nesting.test.ts	Adds the optional suppressed classification field, preserves it through conflict merging, and excludes suppressed rows from lead-in adjacency checks.
DOCX regression and corpus coverage src/parser/docx/index.test.ts, src/parser/docx/inference*.test.ts, src/parser/docx/note-region-corpus.integration.test.ts	Covers paired and unpaired regions, heading closure, structural drift handling, vanish precedence, four-asterisk boundaries, decoration rows, rendered output, and corpus cases.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DOCX as DOCX paragraphs
  participant Roles as computeNoteRoles
  participant Classifier as classifyNoteRoles
  participant Inference as classifyParagraphs
  participant Tree as buildTree
  DOCX->>Roles: provide paragraph text and metadata
  Roles->>Classifier: classify NoteScanItem[]
  Classifier-->>Roles: return rule/note/none roles
  Roles-->>Inference: provide paragraph roles
  Inference->>Tree: pass suppressed rule rows and note classifications
  Tree-->>DOCX: emit notes and omit delimiter rows

Loading

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• wrzonance/SpecR#21: Introduced the DOCX 5-signal inference engine extended by this role-aware classification flow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: DOCX asterisk-row note-delimiter detection.
Linked Issues check	✅ Passed	The changes implement the linked #292 Phase 2 requirements: shared rule-row utilities, DOCX wiring, note classification, and delimiter suppression.
Out of Scope Changes check	✅ Passed	The added tests and supporting parser/type changes all appear to support the asterisk-rule delimiter feature and its regressions.

---

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

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/note-delimiters.ts (1)

57-63: 📐 Maintainability & Code Quality | 🔵 Trivial

Drift guard silently discards the signal without surfacing it for downstream conflict tracking.

classifyNoteRoles returns only the final roles (or all-'none' on drift) with no indication that a drift/disengagement occurred. Per the coding guidelines, src/parser/**/*.ts must "persist inference conflicts rather than discarding losing signals" and expose them via meta.conflicts/paragraphs.conflicts. Since this classifier is the only place that detects the drift, and it doesn't return that fact, the DOCX layer (note-roles.ts/inference.ts) has no way to record the conflict even if it wanted to — the losing signal (the note-region classification) is discarded here with no trace.

Consider returning a richer result (e.g. { roles, drifted: boolean }) so callers can persist the disengagement as a conflict.
[medium]

Based on coding guidelines: "Persist inference conflicts rather than discarding losing signals; expose them through meta.conflicts and store them in paragraphs.conflicts."

🤖 Prompt for 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.

In `@src/lib/note-delimiters.ts` around lines 57 - 63, Update classifyNoteRoles to
return both the computed roles and whether structural drift was detected, rather
than silently returning only the all-'none' roles. Adjust its callers in the
DOCX note-role/inference flow to persist the drift signal as an inference
conflict through meta.conflicts and paragraphs.conflicts, while preserving the
existing role output behavior.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@src/lib/note-delimiters.ts`:
- Around line 57-63: Update classifyNoteRoles to return both the computed roles
and whether structural drift was detected, rather than silently returning only
the all-'none' roles. Adjust its callers in the DOCX note-role/inference flow to
persist the drift signal as an inference conflict through meta.conflicts and
paragraphs.conflicts, while preserving the existing role output behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 27b25ce4-ea1e-4522-8cec-a62015045ac1

📥 Commits

Reviewing files that changed from the base of the PR and between 2bfd779 and 9f415f3.

📒 Files selected for processing (14)

• src/lib/note-delimiters.test.ts
• src/lib/note-delimiters.ts
• src/parser/docx/index.test.ts
• src/parser/docx/inference-notes.test.ts
• src/parser/docx/inference.test.ts
• src/parser/docx/inference.ts
• src/parser/docx/lead-in-nesting.test.ts
• src/parser/docx/lead-in-nesting.ts
• src/parser/docx/note-region-corpus.integration.test.ts
• src/parser/docx/note-roles.test.ts
• src/parser/docx/note-roles.ts
• src/parser/docx/numbering-profile-apply.test.ts
• src/parser/docx/types.test.ts
• src/parser/docx/types.ts

[thewrz]

Codex (GPT-5.5, xhigh) adversarial review — 1 finding [P2], handled (fixed in 9b3b6a7).

[P2] Include Signal 2 styles in drift detection (src/parser/docx/note-roles.ts) — VALID, fixed.
The #292 drift guard keyed only on Signal 1 (a live positive numId). A paragraph structural only via Signal 2 — its style resolves to a real tier through resolvedNumPr, with no direct numId and no literal "PART n"/"N.N" text — was invisible to both the drift check and the text-pattern heading gate, so inside a drifted (unpaired) asterisk wall it was swallowed as a [NOTE]: the exact structure-loss class the guard exists to prevent.

Fix: computeNoteRoles now takes an injected isStructural predicate; the DOCX driver supplies numId>0 || trySignal2(...), reusing trySignal2's numId=0 / suppressesNumbering / lead-in opt-out resolution verbatim (no duplicated OOXML logic). Pinned by a regression test at the classifyParagraphs boundary (inference-notes.test.ts) — verified red without the Signal-2 half, green with it. isNoteParagraph moved to the note adapter to keep inference.ts under the 400-line cap.

Verification: fixture A/B vs main baseline unchanged — 2/705 (both intended), zero part-count changes; full unit suite green (1691); lint + build clean.

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 4 minutes.

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 5 minutes.

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.