fix(parser): nested text-box run-vanish leak (#641); refute object-table rule-row leak (#633)

[thewrz]

Why

Both issues are the same class of defect in body-objects.ts's captured-object text walk (collectText/extractBlobText), and both surface through hidden-text.integration.test.ts — investigated together, as directed.

• fix(parser): a hidden text box nested inside a visible text box leaks its interior text into objectText #641: a hidden text box nested inside a visible text box (or a plain vanish run sitting beside a visible run in the same interior paragraph) leaked its hidden text into the captured objectText, because the walk had no depth limit and no visibility check. This is the same privacy-class bug as fix(parser): body text-box visibility is decided per host-paragraph, not per text box (mixed visible/hidden boxes leak or suppress objectText) #515/ADR-087, one level deeper — the box-level hiddenFlags/hiddenSubtrees correlation never sees a nested boundary, so nothing catches it.
• test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633: hidden-text.integration.test.ts was failing against hidden-text-test.docx because 4 objectText nodes contain bare asterisk-rule text. Investigating this (not blindly porting inference.ts's rule-row suppression) revealed it is not a parser defect — note-region-corpus.integration.test.ts already has a deliberate, pre-existing, ADR-072-decision-14-backed test proving this SAME fixture's body table should render those SAME 4 asterisk-rule cells verbatim (captured object content is a faithful, out-of-band mirror of the source, never re-run through paragraph-tier suppression). The #294 test's assertion simply predates that decision and was never reconciled with it.

What

• body-objects.ts: collectText now skips any w:r flagged by a new presence-only hasRunVanish check, at whatever depth it's encountered — closing both the nested-text-box leak and the same-paragraph mixed visible/vanish-run leak with one mechanism, entirely inside the tree already being walked (no extension of the box-level correlation machinery, avoiding the fix(parser): decide body text-box visibility per box, not per host paragraph #636 near-miss's correlation-desync risk one level deeper).
• hidden-text.integration.test.ts: the "no bare-asterisk node anywhere" assertion is narrowed to exclude objectText nodes, with a comment citing ADR-072 decision 14 and the sibling test that already pins the verbatim behavior. This is the actual fix for test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633 — a test-scoping correction, not a behavior change.
• body-objects.test.ts: new regression tests for fix(parser): a hidden text box nested inside a visible text box leaks its interior text into objectText #641 (visible-outer/hidden-nested, same-paragraph mixed visible/vanish runs, hidden-outer/visible-nested pinning existing behavior, and a byte-identical round-trip assertion comparing real serialized bytes) and for test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633 (positive pins that a rule-row-only interior paragraph in both a text box and a table surfaces verbatim, replacing an earlier, incorrect suppression-based test).
• docs/adr/092-object-text-walk-run-and-paragraph-suppression.md: records both decisions — the fix(parser): a hidden text box nested inside a visible text box leaks its interior text into objectText #641 fix and the test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633 refutation, including why the first (incorrect) approach was tried and reverted.

Investigation notes (both premises proven empirically, per sprint policy)

#641 — reproduced on an unmodified checkout, then fixed:

before: interiorTexts = [{ text: "Outer visibleNESTED SECRET" }]
after:  interiorTexts = [{ text: "Outer visible" }]

#633 — reproduced on an unmodified checkout; the "fix" that broke an existing test:

before (unmodified): 4 leaked objectText nodes, each 40+ literal '*' characters

Implementing direction (a) from the issue (route object capture through isRuleRow) immediately turned note-region-corpus.integration.test.ts's "DOCX object-table verbatim rendering — hidden-text-test.docx (#300, ADR-072)" test red (expected 0 to be 4) — that test already pins this exact fixture's table rendering those exact 4 rule rows verbatim, by design. Reverted the code-level suppression; resolved via direction (b) instead (narrow the #294 test).

Related case named in #641 ("likely same fix"), confirmed and closed by the same mechanism: a single interior paragraph mixing one visible run and one w:vanish run (no nesting at all) leaked the vanish run's text the same way — pinned by its own regression test.

Mutation verification

• hasRunVanish (fix(parser): a hidden text box nested inside a visible text box leaks its interior text into objectText #641): temporarily forced to always return false → exactly the 2 vanish-specific unit tests failed ('a visible outer text box exposes only its own text…', 'a single interior paragraph mixing…'); nothing else broke. Reverted.
• hidden-text.integration.test.ts's narrowed assertion (test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633): temporarily disabled inference.ts's classifyOne role === 'rule' branch (ADR-086's own paragraph-tier suppression) → the narrowed assertion still failed (expected true to be false), proving it still catches genuine paragraph-tier regressions and isn't vacuously excluding too much by dropping objectText. Reverted.

Fixture corpus revalidation

pnpm fixture:snapshot / pnpm fixture:diff over the full 705-fixture corpus, comparing an unmodified checkout against this branch's final code: 0/705 fixtures changed. (No fixture in the corpus exercises the #641 nested-text-box shape, and #633 ended up as a test-only change — so this is the expected result, not a false negative; logged in the ADR.) Same single pre-existing parse-error fixture (11_53_00nle.docx) before and after — not a regression.

LOC note

This PR grows slightly past the file it touches most (body-objects.test.ts) by the sprint's own accounting rule (fixtures/tests count differently, but flagging per policy): the diff is concentrated in one parser module + its test file + one test-scoping fix + one ADR, all one coherent investigation per the sprint's "one PR, one question" directive.

Draft-phase review round (adversarial + independent verification)

Three further defects were found and fixed on this branch after the initial
implementation. All three are latent — no corpus fixture reaches any of
them — and full-corpus revalidation stayed at 0/705 changed throughout.

1. w:vanish was read as element presence, not as the ST_OnOff toggle it
   is (82a05540). Per ECMA-376 §17.3.2.45 an explicit w:val of
   0/false/off switches the toggle OFF — a visible run. Because the
   new collectText branch DROPS text, reading presence as "hidden" would
   silently delete visible spec text. The ADR had dismissed this as
   "vanishingly rare"; it is not — two real CPI fixtures carry 15
   <w:vanish w:val="0"/> runs between them, several text-bearing
   (CPI_COMMUNICATIONS_RACK_MOUNTED_POWER_PROTECTION_CSIMFS.docx holds
   "Select voltage/phase; breaker number, " and "rating" in exactly that
   shape). Same fix applied to document.ts's runIsVanish /
   paragraphMarkVanish, which had the identical bug (8e083953,
   fix(cross):).

2. The object EDIT walk was not updated to match the capture walk
   (039e1f77) — found by the Codex adversarial review. Dropping vanish runs
   changed what objectText means, but object-blob-edit.ts's
   rewriteFirstText still assumed "all text", despite a docstring promising
   it mirrors collectText. On a paragraph whose hidden run precedes its
   visible one, an edit landed in the hidden run while the visible run
   was blanked — the edit disappears from the document and real text is
   destroyed. Fixed by exporting hasRunVanish and reusing it, so the two
   walks cannot drift apart again.

Every gate added or changed here is mutation-verified in both directions,
so none of them can silently degrade into a no-op:

Gate	Mutant	Result
narrowed #294 assertion	disable classifyOne's role === 'rule' branch	red (expected true to be false) — still catches the paragraph-tier class
hasRunVanish (object walk)	presence-only	red — 2 survival tests
hasRunVanish (object walk)	suppress nothing	red — 5 tests incl. the ON-side
runIsVanish / paragraphMarkVanish	presence-only	red — 2 survival tests
runIsVanish / paragraphMarkVanish	never vanish	red — 8 tests incl. the ON-side
edit-walk vanish skip	skip removed	red — both ordering tests

Known divergence, deliberately NOT fixed here

merge/extract.ts's visibleText still reads hidden runs: for an
SDT-anchored cell paragraph mixing a hidden and a visible run it extracts
"HIDDEN SECRETvisible text" where the AST now stores "visible text", so an
untouched round-tripped DOCX can report as modified. The obvious remedy —
make visibleText skip vanish runs — is wrong as stated: it is merge's
general paragraph walk, and the paragraph tier deliberately KEEPS hidden-run
text in a mixed paragraph (document.ts's extractParagraphText, pinned by
a KNOWN AMBIGUITY test). A blanket skip would create the mirror divergence
for every ordinary paragraph. A correct fix must scope visibility to object
interiors only — a merge-engine semantics change affecting every table-cell
paragraph, owed its own ADR and PR. Recorded in ADR-092 rather than left to
be rediscovered; latent today (0/705).

Testing

• pnpm lint (eslint + tsc --noEmit + prettier --check)
• pnpm build
• pnpm test — 252 files / 3563 tests passed
• pnpm test:integration — 169 files passed, 1 skipped / 2053 tests passed, 2 skipped (full suite, DB-backed)
• Target tests specifically: hidden-text.integration.test.ts and note-region-corpus.integration.test.ts both green
• pnpm fixture:snapshot / pnpm fixture:diff — 0/705 changed
• Mutation verification on both new/changed gates (see above)
• Manual verification: N/A (parser-only change, no UI surface)

🤖 Co-authored by Claude Sonnet 5. Closes #641. Closes #633.

Summary by CodeRabbit

• New Features

  • Improved handling of hidden document text during text extraction and editing.
  • Hidden runs are now consistently excluded, while explicitly visible content remains available.
  • Preserved verbatim content in text boxes and tables, including rule-row text.

• Bug Fixes

  • Corrected visibility handling for hidden-text toggle values.
  • Prevented edits from modifying hidden text.

• Documentation

  • Added an architectural decision record documenting visibility behavior and limitations.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds toggle-aware w:vanish handling, skips hidden runs during object-text extraction and rewriting, preserves verbatim object content, and adds regression coverage for nested text boxes, tables, paragraph visibility, and text editing.

Changes

DOCX visibility and object text

Layer / File(s)	Summary
Toggle-aware vanish semantics src/parser/docx/body-objects.ts, src/parser/docx/document.ts, src/parser/docx/*test.ts	w:vanish treats explicit 0, false, and off values as visible for runs and paragraph marks.
Nested object text capture src/parser/docx/body-objects.ts, src/parser/docx/body-objects.test.ts, src/parser/docx/hidden-text.integration.test.ts, src/parser/docx/note-region-corpus.integration.test.ts, src/parser/docx/body-order.ts, docs/adr/092-object-text-walk-run-and-paragraph-suppression.md	Object text traversal skips vanished runs at nested depths. Visible text, nested OOXML, and decorative rule rows remain preserved as specified.
Visible object text rewriting src/parser/docx/object-blob-edit.ts, src/parser/docx/object-blob-edit.test.ts, docs/adr/092-object-text-walk-run-and-paragraph-suppression.md	rewriteFirstText skips vanished runs and edits the first visible text leaf without changing hidden content.

Sequence Diagram(s)

sequenceDiagram
  participant DOCX as DOCX object content
  participant collectText
  participant hasRunVanish
  participant objectText
  DOCX->>collectText: Traverse nested text-box or table XML
  collectText->>hasRunVanish: Check each run's w:vanish value
  hasRunVanish-->>collectText: Return visible or hidden state
  collectText->>objectText: Add visible w:t content

Loading

Possibly related issues

• fix(parser): a hidden text box nested inside a visible text box leaks its interior text into objectText #641 — Adds the nested hidden-run extraction fix described by this issue.
• test(parser): hidden-text-test.docx leaks a bare-asterisk objectText node (corpus-gated — does not reach CI) #633 — Narrows the asterisk-wall assertion to exclude verbatim objectText.
• feat(parser/generator): represent + round-trip visible tables in specs (AST model, inference exclusion, render) #300 — Shares DOCX table and text-box object capture and verbatim text handling.

Possibly related PRs

• wrzonance/SpecR#513 — Directly relates to the existing object-text extraction and editing paths extended here.
• wrzonance/SpecR#521 — Directly relates to object-blob-edit.ts handling of object text.
• wrzonance/SpecR#636 — Directly relates to text-box visibility handling and nested hidden-run extraction.

🚥 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 identifies the nested text-box hidden-run fix and the object-table rule-row finding addressed by the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

[thewrz]

This was written agentically; verify its assertions and edit accordingly:

Adversarial review — Codex gpt-5.6-sol, effort xhigh (run once, on the final draft diff)

1 finding: 1 P1. Confirmed and fixed, with its suggested remedy partially declined on evidence.

[P1] Object-text readers were left out of sync with the new hidden-run filtering — src/parser/docx/body-objects.ts:217

When a captured table/text-box paragraph contains both visible and vanished runs, this skip stores only visible text in CapturedObjectText while leaving every hidden w:t under the same SDT anchor. merge/extract.ts::visibleText still reads those hidden leaves, so an unchanged generated DOCX is reported as modified; object-blob-edit.ts::rewriteFirstText likewise rewrites every leaf, so if the hidden run comes first an edit is placed into hidden content and the visible run is blanked. Apply the same visibility rule to the merge and edit walks, including unanchored vanished table paragraphs.

Both halves verified empirically before acting — not accepted on the reviewer's word.

Half A — object-blob-edit.ts::rewriteFirstText: CONFIRMED, FIXED in 039e1f77.
Correct and serious. rewriteFirstText's own docstring already promised it mirrors collectText ("an edit reaches the text wherever capture read it from"); this PR silently made that false. On an interior paragraph whose hidden run precedes its visible one, the edit landed in the hidden run — invisible in Word — while the visible run was blanked, destroying real text. Fixed by exporting hasRunVanish from body-objects.ts and reusing it, rather than writing a second copy free to drift. Skipped vanish runs are left untouched rather than blanked (ADR-072's no-silent-loss posture), which also keeps post-edit visible text exactly equal to the new text. Three regression tests cover hidden-first, hidden-last, and the w:vanish w:val="0" visible case; mutation-verified — removing the skip fails both ordering tests.

Half B — merge/extract.ts::visibleText: CONFIRMED as a divergence, but the suggested remedy DECLINED as written.
The divergence is real and I reproduced it: for an SDT-anchored cell paragraph mixing a hidden and a visible run, the merge extractor returns "HIDDEN SECRETvisible text" while the AST now stores "visible text", so an untouched round-trip can report as modified.

The proposed fix — "apply the same visibility rule to the merge walk" — would introduce a worse bug than it removes. visibleText is merge's general paragraph walk, not an object-specific one, and the paragraph tier deliberately KEEPS hidden-run text in a mixed paragraph (document.ts's extractParagraphText; pinned by document.test.ts's "does NOT mark vanish when only some runs are hidden" KNOWN AMBIGUITY test). A blanket vanish skip in visibleText would therefore create the exact mirror divergence for every ordinary body paragraph in every document.

A correct fix must scope visibility to object interiors only, which means threading a new mode through walkBlocks / visitParagraph / visibleText and changing merge's notion of text for every table-cell paragraph — a merge-engine semantics change that belongs in its own ADR and PR rather than inside a parser leak fix. It is latent today: no corpus fixture puts a mixed visible/hidden run pair inside a captured object (0/705 on pnpm fixture:diff). Recorded in ADR-092's Decision section so it cannot be silently rediscovered, and escalated for a human call.

Independently found and fixed in the same round (not from Codex)

• 82a05540 — w:vanish was read as element presence rather than as the ST_OnOff toggle ECMA-376 §17.3.2.45 defines. An explicit w:val="0" means the toggle is OFF (a visible run), and since collectText's new branch drops text, the presence-only read would silently delete visible spec text. ADR-092 had called this "vanishingly rare" — it is not: two real CPI fixtures carry 15 such runs between them, several text-bearing.
• 8e083953 (fix(cross):) — the identical presence-vs-toggle bug in document.ts's runIsVanish / paragraphMarkVanish. Measured before changing: 0/705 fixtures move, because those CPI paragraphs already resolve hidden via their CMT paragraph style. Latent landmine removed, not a live regression fixed.

Verification

pnpm lint clean · pnpm test 252 files / 3563 passed · target integration suites (hidden-text + note-region-corpus) 40 passed · pnpm fixture:snapshot/fixture:diff 0/705 changed for main → final, three-part structure and note-leak counts unchanged, same single pre-existing parse-error fixture. Every gate added or changed is mutation-verified in both directions (see the PR body's table), including the narrowed #294 assertion, which still goes red when classifyOne's role === 'rule' branch is disabled — so narrowing it to exclude objectText did not make it vacuous.

🤖 Co-authored by Claude Opus 5 (1M context).

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/parser/docx/document.ts (1)

133-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve explicit OFF when resolving inherited vanish.

A direct w:vanish w:val="0" does not stop the fallback style checks. runIsVanish continues to w:rStyle, and resolveParagraphVanish continues to vanishStyleIds. A direct OFF value can therefore still suppress text from an enabled character or paragraph style.

Return a tri-state value: absent, ON, or OFF. Consult inherited styles only when the local value is absent.

• src/parser/docx/document.ts#L133-L145: make explicit OFF terminal before checking vanishCharStyleIds.
• src/parser/docx/document.ts#L187-L202: make explicit paragraph-mark OFF terminal before checking vanishStyleIds.
• src/parser/docx/document.test.ts#L211-L243: add named regressions for direct-OFF character-style and paragraph-style overrides.
• docs/adr/092-object-text-walk-run-and-paragraph-suppression.md#L119-L137: revise the decision text to match the tested precedence.

As per coding guidelines, add a regression test for every bug fix named after the observed symptom.

🤖 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/parser/docx/document.ts` around lines 133 - 145, Update runIsVanish and
resolveParagraphVanish in src/parser/docx/document.ts (lines 133-145 and
187-202) to resolve local vanish values as absent, ON, or OFF, stopping
inheritance when an explicit OFF is present before checking character or
paragraph style IDs. Add named regressions in src/parser/docx/document.test.ts
(lines 211-243) for direct-OFF character-style and paragraph-style overrides,
and revise docs/adr/092-object-text-walk-run-and-paragraph-suppression.md (lines
119-137) to document the tested precedence.

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.

Inline comments:
In `@src/parser/docx/body-objects.ts`:
- Around line 187-192: Resolve character-style vanish through
StyleMap.vanishCharStyleIds before object capture and rewrite: update
hasRunVanish and the collectText path in src/parser/docx/body-objects.ts (lines
187-192 and 216-225), and the rewriteFirstText flow in
src/parser/docx/object-blob-edit.ts (lines 196-202), to recognize runs whose
w:rStyle applies vanish. Add capture regressions in
src/parser/docx/body-objects.test.ts (lines 1038-1076) and rewrite regressions
in src/parser/docx/object-blob-edit.test.ts (lines 277-339). Document the
behavior and updated handling in
docs/adr/092-object-text-walk-run-and-paragraph-suppression.md (lines 139-153).

---

Outside diff comments:
In `@src/parser/docx/document.ts`:
- Around line 133-145: Update runIsVanish and resolveParagraphVanish in
src/parser/docx/document.ts (lines 133-145 and 187-202) to resolve local vanish
values as absent, ON, or OFF, stopping inheritance when an explicit OFF is
present before checking character or paragraph style IDs. Add named regressions
in src/parser/docx/document.test.ts (lines 211-243) for direct-OFF
character-style and paragraph-style overrides, and revise
docs/adr/092-object-text-walk-run-and-paragraph-suppression.md (lines 119-137)
to document the tested precedence.

🪄 Autofix

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: 08432a20-0523-4cea-94ef-05bb796c729c

📥 Commits

Reviewing files that changed from the base of the PR and between 35935f1 and 006f80f.

📒 Files selected for processing (10)

• docs/adr/092-object-text-walk-run-and-paragraph-suppression.md
• src/parser/docx/body-objects.test.ts
• src/parser/docx/body-objects.ts
• src/parser/docx/body-order.ts
• src/parser/docx/document.test.ts
• src/parser/docx/document.ts
• src/parser/docx/hidden-text.integration.test.ts
• src/parser/docx/note-region-corpus.integration.test.ts
• src/parser/docx/object-blob-edit.test.ts
• src/parser/docx/object-blob-edit.ts