From b9d61a3dbf2075a8fca30fb9c9ebc67a540950d8 Mon Sep 17 00:00:00 2001 From: Lucas Vogel Date: Thu, 1 Oct 2026 17:55:47 -0400 Subject: [PATCH 1/2] test: pin the inline wrappers, which shipped fixed but unprotected a3e3558 fixed two things and pinned one. Its tests cover the text-box half (w:txbxContent); the inline-wrapper half shipped with nothing asserting it: inline w:sdt no test w:fldSimple no test w:smartTag no test w:instrText no test (the exclusion) Only the BLOCK-LEVEL w:sdt route had a test, in ListStylesheetTests, and docs/limitations.md is explicit that the block form was never the broken one. So the fix was live, correct, and free to regress silently. This repo has already paid for that exact gap: the w:tab branch of SourceWords was deleted, 356 tests stayed green, and 382 phantom losses turned up in the next corpus run. Six tests, and they were proven able to fail. Reverting the select-list half of a3e3558 (back to "w:r | w:ins | w:hyperlink") fails exactly five of the six. FieldCode_IsNotEmitted survives the mutation, correctly: it guards an exclusion, which does not depend on that select list. A mutation that failed all six would have told me less, because it could not distinguish the two kinds of test. Why the inclusions are not a matter of taste: docmd:text-nodes walks the DESCENDANT axis, so TextCoverageReport already counts this text as words a reader can see. A transform that cannot reach it does not merely omit it -- it reports itself as lossy. The oracle had already cast the deciding vote. Verified end-to-end as well as in unit tests: the exact fixture docs/limitations.md says produces "Field:" and nothing else now yields all three wrapped strings, with the field code and the deleted text correctly absent, and --report clean at 0 words lost. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018wZEgtuzaZPswykiGEBxbz --- .../MarkdownStylesheetTests.cs | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/Docmd.Word.Tests/MarkdownStylesheetTests.cs b/tests/Docmd.Word.Tests/MarkdownStylesheetTests.cs index a33b48e..0adfa00 100644 --- a/tests/Docmd.Word.Tests/MarkdownStylesheetTests.cs +++ b/tests/Docmd.Word.Tests/MarkdownStylesheetTests.cs @@ -225,6 +225,103 @@ public async Task DeletedText_IsNotEmitted() markdown.Should().Be("The vent is compliant\n"); } + // --------------------------------------------------------------------------------------- + // Wrappers a reader never sees, holding text a reader does see. + // + // markdown.xslt descends through inline w:sdt, w:fldSimple and w:smartTag, and deliberately + // does NOT descend into w:instrText or w:del. That behaviour arrived as a fix and shipped + // with nothing pinning it: the block-level w:sdt route has a test, the inline route had + // none, and neither exclusion was asserted anywhere. This repo has already paid for that + // exact gap once - the w:tab branch of SourceWords was deleted, 356 tests stayed green, and + // 382 phantom losses appeared in the next corpus run. These tests close it. + // + // The oracle is why the inclusions are not optional. docmd:text-nodes walks the DESCENDANT + // axis, so TextCoverageReport already counts this text as words a reader can see; a + // transform that cannot reach it does not merely omit it, it reports itself as lossy. + // --------------------------------------------------------------------------------------- + + [Fact] + public async Task InlineContentControl_KeepsItsText() + { + // A content control that is a child of w:p, so its runs are NOT children of w:p. This + // is the form carrying the variable parts of a templated document - the customer name, + // the revision, the approval block - which are the fields most worth indexing. + var markdown = await ToMarkdownAsync( + """Inside control"""); + + markdown.Should().Be("Inside control\n"); + } + + [Fact] + public async Task InlineContentControl_AsAWholeParagraph_LeavesNoBlankLine() + { + // Two axes used to disagree about what counts as a paragraph's text. The empty-paragraph + // suppression rule asks docmd:visible-text (descendant axis) and so SAW this text and + // declined to suppress; the inline emitters used the child axis and could not reach it. + // The words were lost AND a stray blank line was left where they had been. Asserting the + // exact output pins both halves: fixing the blank line alone would have made the loss + // less visible rather than less real. + var markdown = await ToMarkdownAsync(""" + Before. + Controlled + After. + """); + + markdown.Should().Be("Before.\n\nControlled\n\nAfter.\n"); + } + + [Fact] + public async Task FieldResult_IsEmitted() + { + // w:fldSimple holds a cached result, which is what Word shows. Measured across the + // corpus the stylesheet was built against, those results were 103 DOCPROPERTY, 12 SEQ, + // a TITLE and an AUTHOR, with no PAGE and no TOC - every one of them content. + var markdown = await ToMarkdownAsync(""" + Title: Zoning Code + """); + + markdown.Should().Be("Title: Zoning Code\n"); + } + + [Fact] + public async Task SmartTag_KeepsItsWords() + { + // A smart tag is a recognition marker around ordinary words. It contributes nothing of + // its own and must contribute all of its children. + var markdown = await ToMarkdownAsync( + """Acme Corp filed."""); + + markdown.Should().Be("Acme Corp filed.\n"); + } + + [Fact] + public async Task FieldCode_IsNotEmitted() + { + // The other half of the whitelist, and the half that fails loudly if it ever breaks. + // w:instrText is the field CODE, not its result. Emitting it would put a raw field + // instruction into a document someone indexes, which is worse than omission because it + // invents content rather than missing it. + var markdown = await ToMarkdownAsync(""" + PAGE \* MERGEFORMATPage text. + """); + + markdown.Should().Be("Page text.\n"); + markdown.Should().NotContain("MERGEFORMAT"); + } + + [Fact] + public async Task NestedWrappers_AreAllTransparent() + { + // The templates recurse through each other, so a control inside a smart tag inside a + // control has to work. Real templated documents nest these routinely, and a whitelist + // that only handled one level would look correct on a simpler fixture. + var markdown = await ToMarkdownAsync(""" + Deeply nested + """); + + markdown.Should().Be("Deeply nested\n"); + } + [Fact] public async Task Heading_KeepsWordsApartAcrossATab() { From bda784ae8749b7d62382296d60938afa03f0a582 Mon Sep 17 00:00:00 2001 From: Lucas Vogel Date: Thu, 1 Oct 2026 22:02:20 -0400 Subject: [PATCH 2/2] docs: correct two limitations that were fixed, and file the three that are not docs/limitations.md carried "Status: open" on two defects that commit a3e3558 fixed on 2026-09-08. That file is the public accounting of what docmd cannot do, so it was telling prospective users docmd drops their content-control text and their text-box text. Both convert, verified end to end today. Text inside transparent wrappers fixed. The fixture this file says produces "Field:" and nothing else now yields every string, with the field code and the deleted text correctly absent and --report clean. Text inside a text box fixed. Content becomes blocks after its anchor paragraph; --report clean. Both sections now use the file's own convention for a closed item, carry the commit that fixed them, and name the tests that pin them. The wrapper entry also drops a prediction that turned out to be wrong. It said the policy for which fields carry content needed frequency data from a corpus audit first. It did not: docmd:text-nodes walks the descendant axis, so TextCoverageReport already counted that text as words a reader can see, which means a transform unable to reach it reported itself as lossy. The oracle had already decided. Recording that because the entry's reasoning is the kind that defers work indefinitely while waiting for data it does not need. Three defects ARE still real, reproduced today and now filed rather than left in prose: #37 a non-numeric w:ilvl aborts the conversion: exit 1, no output file, so in a batch the document is lost rather than degraded #38 four or more leading tabs silently turn a paragraph into a CommonMark indented code block #39 an interrupted ordered list restarts at 1. Each issue carries its fixture and its exact observed output. #38 and #39 both exit 0 and report no loss, which is what makes them worth filing: every word survives and only its meaning changes, so the text-coverage oracle cannot see them. Also corrected: this file listed --report among flags that "fail cleanly as not-yet-supported". It shipped in 0.2.3. --style-map is unblocked, phoenixmldb-xslt#12 being closed, and when it ships it should set an XsltTransformer.ResourcePolicy. Running a stylesheet somebody else wrote is untrusted code, and policy enforcement across reads, fetches and xsl:evaluate landed in Xslt 2.5.1 as GHSA-86rg-wxgp-9p5j. Noted where the flag is described rather than left to be rediscovered. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018wZEgtuzaZPswykiGEBxbz --- docs/deferred-work.md | 33 ++++++++-- docs/limitations.md | 138 +++++++++++++++++------------------------- 2 files changed, 84 insertions(+), 87 deletions(-) diff --git a/docs/deferred-work.md b/docs/deferred-work.md index aa7d312..e91e9f5 100644 --- a/docs/deferred-work.md +++ b/docs/deferred-work.md @@ -6,6 +6,9 @@ because later plans inherit them. Each says why it was deferred and what would c ## Known defects, ranked ### 1. `xs:integer` on a non-numeric `w:ilvl` raises a dynamic error + +**Filed as [#37](https://github.com/phoenixmldb/docmd/issues/37).** Reproduced 2026-10-01: it +exits 1 and writes no output file, so in a batch the document is lost rather than degraded. `src/Docmd.Word/Stylesheets/markdown.xslt` — `docmd:ilvl` and the `is-ordered` predicate. Identical failure mode to the duplicate-`w:numId` crash already fixed with positional `[1]` guards: @@ -17,6 +20,8 @@ duplicate-`numId` case (which merged or round-tripped documents produce routinel Closes with: a guarded cast, and a fixture asserting it degrades rather than throws. ### 2. Four or more leading `w:tab` render as an indented code block + +**Filed as [#38](https://github.com/phoenixmldb/docmd/issues/38).** Reproduced 2026-10-01. `markdown.xslt` emits one space per tab; the serialiser trims only *trailing* horizontal whitespace, never leading. Four leading spaces at the start of a block is CommonMark's indented code block, so a deeply tab-indented paragraph silently becomes code. @@ -26,9 +31,21 @@ raised at the merge gate. Closes with: a `TrimStart(' ', '\t')` on a paragraph's line — which would also tidy the harmless leading space an emphasis span can emit at a paragraph start. -### 3. Text inside inline content controls, fields and smart tags is dropped -See `docs/limitations.md` for the measured detail and the counts a corpus audit must produce. -Block-level `w:sdt` is *not* affected. Descending through transparent wrappers is a Plan 2 change. +### 3. ~~Text inside inline content controls, fields and smart tags is dropped~~ — FIXED + +Fixed by commit `a3e3558`; verified end-to-end and pinned by tests on 2026-10-01. The paragraph +templates descend through inline `w:sdt`, `w:fldSimple` and `w:smartTag`, and deliberately do not +descend into `w:instrText` or `w:del`. The stray blank line that accompanied the loss went with +it. See `docs/limitations.md` for the policy and its measured basis. + +This entry predicted the work needed frequency data from a corpus audit before the policy could +be chosen. It did not: the oracle had already decided it. `docmd:text-nodes` walks the descendant +axis, so `TextCoverageReport` already counted this text as words a reader can see — a transform +that could not reach it did not merely omit it, it reported itself as lossy. + +Worth recording what the fix left behind. `a3e3558` pinned only its text-box half; the wrapper +half shipped unprotected for three releases, and `ListStylesheetTests`' content-control test +covers the *block-level* form, which was never the broken one. Six tests now cover it. ### 4. Table cells lose inline formatting, including hyperlink URLs A decided product trade-off, not an oversight — for retrieval a URL is near-worthless and often @@ -44,6 +61,8 @@ already dead, while the anchor text survives. Recorded so the corpus audit can r - `--flavour commonmark` is parsed and plumbed, but the serialiser emits GFM tables regardless. - Ordered lists always restart at `1.`; a numbered procedure interrupted by a note paragraph renumbers from the top. `md:list/@start` exists in the vocabulary and is never emitted. + **Filed as [#39](https://github.com/phoenixmldb/docmd/issues/39)**, reproduced 2026-10-01. No + existing check catches it: the oracle counts words, and no word is missing. - Two media parts sharing a file name overwrite each other — `AssetRewriter` keys the output path on the file name alone. - ~~`HeadingAnnotator` walks `Descendants(w:p)`, which reaches paragraphs inside table cells... @@ -57,8 +76,12 @@ already dead, while the anchor text survives. Recorded so the corpus audit can r - A new `XsltTransformer` is constructed and the stylesheet recompiled per document. Correct, but it will dominate cost once batch mode exists. - `md:heading/@slug` is emitted and unused today. It is Plan 2's cross-link key, not dead weight. -- `docmd audit`, `-r`/`--recursive`, `--review`, `--style-map`, `--strict`, `--report` and - `--revisions` all currently fail cleanly as not-yet-supported. +- `docmd audit`, `-r`/`--recursive`, `--review`, `--style-map`, `--strict` and `--revisions` + all currently fail cleanly as not-yet-supported. (`--report` shipped in 0.2.3 and is no longer + on this list.) `--style-map` is unblocked: `phoenixmldb-xslt#12` is closed, so its workaround + can go. When it ships it should set an `XsltTransformer.ResourcePolicy` — running a stylesheet + somebody else wrote is untrusted code, and policy enforcement across reads, fetches and + `xsl:evaluate` landed in Xslt 2.5.1 (GHSA-86rg-wxgp-9p5j). ## xunit v3 4.0 needed a test-platform migration, not a version bump diff --git a/docs/limitations.md b/docs/limitations.md index e462d21..25e03d5 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -25,87 +25,58 @@ measurement changed. The oracle now counts `w:noBreakHyphen` and `w:sym`, which could not see, so it reports losses the old one was structurally unable to detect — see the `w:sym` section below for what that blindness cost. -## Text inside transparent wrappers is dropped +## ~~Text inside transparent wrappers is dropped~~ — fixed -**Status:** open. **Found:** whole-branch review of `feat/core-conversion`, 2026-09-04. +**Status:** fixed. **Found:** whole-branch review of `feat/core-conversion`, 2026-09-04. +**Fixed:** commit `a3e3558`, "read the wrappers that hide legitimate text". **Verified and +pinned:** 2026-10-01. -`markdown.xslt` emits inline content by selecting the *children* of a paragraph: - -```xml - -``` - -WordprocessingML has several elements that wrap runs without changing what the reader sees. -The child axis does not descend through them, and no template matches them, so every run -they contain contributes nothing at all: +WordprocessingML has elements that wrap runs without changing what a reader sees. The child axis +does not descend through them, so an earlier `markdown.xslt` — which selected +`w:r | w:ins | w:hyperlink` from a paragraph — lost every run inside one: | Element | What it wraps | What Word shows | |---|---|---| | `w:sdt` (**inline only** — a child of `w:p`) | a content control inside a paragraph | the control's current text | -| `w:fldSimple` | a field with its cached result | the result, e.g. a page number or a cross-reference | +| `w:fldSimple` | a field with its cached result | the result | | `w:smartTag` | a recognised entity | the words, unchanged | -**A block-level `w:sdt` is not affected.** One that wraps whole paragraphs sits in `w:body`, -where the built-in rules walk into it and the paragraphs inside reach the template rules -normally, so their text survives — pinned by -`ListStylesheetTests.NumberedParagraphInsideAContentControl_KeepsItsText`. Only the inline -form, where the control is a child of `w:p` and its runs are therefore not children of `w:p`, -is dropped. Scoping this row to the inline form matters: read as "inline or block" it -overstates the loss and would mis-size the work below. +A block-level `w:sdt` was never affected: one wrapping whole paragraphs sits in `w:body`, where +the built-in rules walk into it. -Measured against the stylesheet at the time of writing, this input (all three wrappers below -are the inline form — each is a child of `w:p`): +**What it does now.** The paragraph templates select all three wrappers, and a template for each +contributes its children; they recurse, so nesting works. This input, which previously produced +`Field:` and nothing else, now yields every string: ```xml Inside control -Field:7 +Field:CachedResult Acme Corp ``` -produces `Field:` and nothing else. "Inside control", "7" and "Acme Corp" are gone. - -This matters more than the element names suggest. Content controls are how templated -corporate documents carry their variable parts — the customer name, the revision, the -approval block — so the fields most worth indexing are exactly the ones most likely to be -inside one. - -### The related inconsistency: a stray blank line - -(Inline `w:sdt` again — a block-level one never reaches this rule as a paragraph's whole -content.) - -Two places in the stylesheet disagree about which axis counts as "the text of a paragraph". -The empty-paragraph suppression rule asks `docmd:visible-text`, which is `.//w:t` and -therefore *does* see inside a `w:sdt`: - -```xml - -``` - -So a paragraph whose only content is a `w:sdt` is not suppressed (it has visible text), then -emits an empty `md:para` (the inline emitters cannot reach that text), and the serialiser -turns that into a blank line. The paragraph's words are lost *and* a stray blank line is -left where they were. - -### Why it is not fixed here - -Descending through transparent wrappers is a real design decision, not a one-line patch: -`w:fldSimple` needs a policy for which fields have a meaningful cached result and which are -noise (`PAGE`, `DATE`), an inline `w:sdt` needs a decision about whether its `w:sdtPr` -placeholder text counts as content when the control is unfilled, and `w:hyperlink` already -sets a precedent for how a wrapper contributes its children. That work belongs with the plan -that also builds the corpus audit, so the choice can be made against measured frequencies. - -### What the audit must count - -- paragraphs containing at least one **inline** `w:sdt` (`w:p/w:sdt`), `w:fldSimple` or - `w:smartTag`, as a share of all paragraphs — block-level `w:sdt` is out of scope, since its - text already survives -- characters of `w:t` unreachable from the child axis, as a share of all `w:t` characters -- `w:fldSimple` occurrences by `w:instr` keyword, so the "which fields carry content" policy - is chosen from data -- paragraphs emitting an empty `md:para` despite having non-empty `docmd:visible-text` — the - stray-blank-line case above, which is a direct count of the inconsistency +**The whitelist stops where a reader stops.** `w:instrText` holds a field code rather than its +result, and `w:del` holds text the author removed under track changes. Neither is emitted: +putting `PAGE \* MERGEFORMAT` or a deleted price into a document someone indexes invents content, +which is worse than missing it. The policy was chosen from measurement, not taste — across the +corpus the stylesheet was built against, the cached results were 103 `DOCPROPERTY`, 12 `SEQ`, a +`TITLE` and an `AUTHOR`, with no `PAGE` and no `TOC`. Every one of them content. + +It remains a whitelist and will therefore always be incomplete, because OOXML keeps growing. The +text-coverage check is what makes that survivable: a document whose words go missing says so. + +**The stray blank line went with it.** Two axes used to disagree about a paragraph's text. The +empty-paragraph suppression rule asks `docmd:visible-text` (descendant axis) and so *saw* this +text and declined to suppress, while the inline emitters used the child axis and could not reach +it — so the words were lost *and* a blank line was left where they had been. Emitting the text +resolves both. Fixing the blank line alone would have made the loss less visible rather than +less real. + +**Pinned by** `MarkdownStylesheetTests`: `InlineContentControl_KeepsItsText`, +`InlineContentControl_AsAWholeParagraph_LeavesNoBlankLine`, `FieldResult_IsEmitted`, +`SmartTag_KeepsItsWords`, `FieldCode_IsNotEmitted` and `NestedWrappers_AreAllTransparent`. +`a3e3558` shipped the fix with tests for the text-box half only; the wrapper half then went +unpinned through every release from `v0.1.0` to `v0.2.4` — eight of them. Reverting the +select-list half of that commit fails five of the six. ## Conversion cost climbs faster than document size above ~4,000 paragraphs @@ -179,27 +150,28 @@ a sentence to attempt. Corpus effect at the time: it removed the single largest source of loss in the sample. -## Text inside a text box is dropped +## ~~Text inside a text box is dropped~~ — fixed -**Status:** open. **Found:** the text-preservation oracle's first corpus run, 2026-09-05. +**Status:** fixed. **Found:** the text-preservation oracle's first corpus run, 2026-09-05. +**Fixed:** commit `a3e3558`. **Verified:** 2026-10-01. `w:txbxContent` holds paragraphs, but it sits inside `w:p/w:r/w:pict` (or `mc:AlternateContent`), -so its paragraphs are not children of `w:body`. The `w:body` template groups over `*` — top-level -children only — and the inline templates select `w:r | w:ins | w:hyperlink` from a paragraph, so -nothing reaches into a text box. Every word inside one is lost. - -Measured on the 49-document sample: one document contained 236 `w:txbxContent` elements. Text -boxes are how pull quotes, callouts and diagram labels are authored, so the loss is concentrated -in exactly the summarising sentences a retrieval index would most want. docmd now counts them and -says so when a document loses words, rather than leaving the reader to wonder where the text -went. - -**What the audit must count:** `w:txbxContent` occurrences per document, and characters of `w:t` -inside them as a share of all `w:t` characters. +so its paragraphs are not children of `w:body`. Nothing reached into a text box and every word +inside one was lost. Measured on the 49-document sample, one document contained 236 +`w:txbxContent` elements — text boxes are how pull quotes, callouts and diagram labels are +authored, so the loss was concentrated in exactly the summarising sentences a retrieval index +most wants. + +Text-box content now becomes blocks after its anchor paragraph. A paragraph holding only a text +box does not also emit a blank paragraph, and a text box inside a table cell is not counted +twice. Pinned by `TextBoxContent_BecomesBlocksAfterItsAnchorParagraph`, +`AParagraphHoldingOnlyATextBox_DoesNotAlsoEmitABlankParagraph` and +`ATextBoxInsideATableCell_IsNotCountedTwice`. ## A paragraph beginning with four or more tabs becomes a code block -**Status:** open, parked deliberately at the end of the core-conversion wave. +**Status:** open — [#38](https://github.com/phoenixmldb/docmd/issues/38). Parked deliberately at +the end of the core-conversion wave; reproduced again on 2026-10-01. `w:tab` now emits one space (see the commit "Stop the stylesheet losing words"). A paragraph that opens with four or more consecutive tabs therefore starts its Markdown line with four or @@ -218,7 +190,9 @@ something different again). ## A non-numeric `w:ilvl` still raises a dynamic error -**Status:** open, pre-existing — not introduced by the core-conversion wave. +**Status:** open — [#37](https://github.com/phoenixmldb/docmd/issues/37). Pre-existing, not +introduced by the core-conversion wave; reproduced again on 2026-10-01, which confirmed it +exits 1 with no output file rather than degrading. `docmd:ilvl` does `xs:integer(($p/w:pPr/w:numPr/w:ilvl/@w:val, '0')[1])`. An attribute present but not a number (`w:val="one"`, or empty) is a cast failure, which propagates out of