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 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() {