Skip to content

fix(docx-core): resolve run formatting from document defaults - #759

Merged
stevenobiajulu merged 2 commits into
mainfrom
753-resolve-doc-defaults-formatting-20260729
Oct 4, 2026
Merged

stevenobiajulu merged 2 commits into
mainfrom
753-resolve-doc-defaults-formatting-20260729

Conversation

@stevenobiajulu

@stevenobiajulu stevenobiajulu commented Jul 29, 2026 •

Copy link
Copy Markdown
Member

Summary

extractEffectiveRunFormatting now reads w:docDefaults/w:rPrDefault/w:rPr as the lowest-precedence run-property layer (#753).

This PR was rebuilt from scratch on top of #758 (6e60dcf). The July draft was based on old main and reverted RunFormatting to non-null types. The rebuild keeps #758's nullable contract and turns its detection-only StylesModel.docDefaultsRPr probe into a real resolution layer.

What changes

  • Ordinary properties (underline, highlight, colour, font, size): the nearest declaration among direct rPr, the rStyle chain, paragraph-mark rPr and the pStyle chain wins. If none of those declares the property, the docDefaults value is used. If docDefaults doesn't declare it either, the result is the OOXML default (false / 'auto'), or null for fontName / fontSizePt, which have no OOXML default.
  • Toggles (the ten docx-core: run-formatting resolver implements a nearest-declaration cascade, not OOXML toggle-property evaluation #737 properties): a 'default' step seeds the walk with the docDefaults value. Style-level on inverts the value, style-level off preserves it, and direct formatting is absolute. The seed itself is not absolute (see below).
  • Theme fonts in docDefaults (w:rFonts w:asciiTheme="minorHAnsi", which Word writes in nearly every document) resolve through the theme part. Without a theme part they stay null (fix(docx-core): expose unresolved run formatting #758 contract (b)).
  • Unread layers: docDefaults is no longer one of them. Table styles still are. For a run inside a table, a property that nothing above docDefaults declares is null when a table style declares a value different from the base (the docDefaults value or the OOXML default). A toggle is null when a table style turns it on and no direct layer set it.
  • Comment / footnote tagged_text (full mode) reports colour, size and font only where a layer above docDefaults declares them (extractAnnotationRunFormatting). Toggles, underline and highlight resolve through docDefaults as everywhere else. Without this, every annotation run in a document whose docDefaults declares a font carried face="...", and importDocxToMarkdoc rejected all ILPA footnotes with ANNOTATION_IMPORT_UNSUPPORTED. A first version suppressed values equal to the docDefaults; peer review showed that this drops a direct colour restating the default over a named character style, which markdoc then re-imports as the style's colour. That version was replaced.
  • Table-style probe compares hex colours and font names case-insensitively (abcdef restates ABCDEF).
  • Consumers: the fix(docx-core): expose unresolved run formatting #758 rules that drop a property when any member leaves it unresolved (the modal baseline in formatting_tags.ts and the convention vote in formatting_convention.ts) now work for docDefaults-only documents with no further change. Tests cover the convention vote counting docDefaults italic and a run-in header that is bold only through docDefaults.

Toggle semantics

Document defaults are the absolute base state. ECMA-376 5th ed. Part 1 §§17.7.3 and 17.7.5.1 define the inheritance and default layers. Microsoft [MS-OI29500] §17.7.3 describes document defaults as the fallback and base for toggle evaluation. [MS-OE376] §2.7.7 documents a paragraph-style reset deviation, but no deviation that makes docDefaults another toggling style level. This matches the Word oracle rows in #753 (<toggle>.single.docDefaults.on, <toggle>.docDefaultsOnly, <toggle>.crossLevel.threeOn = on), which are pinned in styles-doc-defaults.test.ts.

Corpus measurement (54 repo .docx, one is an intentionally corrupt fixture; 41,919 text runs in word/document.xml, theme supplied)

main @ 6e60dcf this PR
fontName: null 41,425 (98.8%) 0
fontSizePt: null 28,035 (66.9%) 237 (0.57%)
fontSizePt: 0 / fontName: "" 0 / 0 0 / 0

The remaining 237 null sizes come from 7 documents whose docDefaults omit w:sz and where no consulted layer declares one. Most of them are NVCA COI source (162) and ILPA (33 + 33).

Tests

  • New styles-doc-defaults.test.ts: every property from docDefaults only; precedence under direct formatting and paragraph styles; toggle seed and parity (one, two and three levels on; style off); theme font in docDefaults with and without a theme; table-style overrides vs restating the docDefaults value; the read_file baseline (no size="0", only the outlier tagged).
  • styles-unresolved.test.ts: docDefaults expectations flip from null to resolved values (allowed by docx-core: run-formatting resolver ignores w:docDefaults, so most runs resolve fontName to an empty string #753's acceptance criteria).
  • The null-handling tests for the heading detectors and the convention vote now use table styles, the remaining unread layer. New positive tests cover docDefaults resolution.
  • footnotes_structured.test.ts: annotation tags omit the inherited default font and keep the style size, an explicit Arial run, and a direct restatement of the default font over an Arial style.
  • docx-markdoc annotation-roundtrip.test.ts: for both comments and footnotes, a direct colour/size restating the docDefaults over a named style survives import, edit and compile.

Validation

npm run build, lint:workspaces, check:cycles, check:spec-coverage, check:conformance-citations, check:conformance-doc, check:conformance-explorer, check:allure-labels, check:allure-quality, check:allure-filenames, check:tool-docs, test:docx-formatting-loss, and the docx-core, docx-mcp and docx-markdoc suites all pass.

Fixes #753

@vercel

vercel Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
site Ready Ready Preview Oct 4, 2026 1:17am UTC

Request Review

@github-actions github-actions Bot added the fix PR type: bug fix label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

stevenobiajulu added a commit that referenced this pull request Oct 3, 2026
extractEffectiveRunFormatting returned '', 0 and false both for properties
declared nowhere and for properties declared only in layers it does not
read (w:docDefaults, table styles), so callers could not tell a known
default from missing resolver coverage.

RunFormatting fields are now nullable, and null means only "unresolved":
a non-default declaration exists in an unread layer (docDefaults, or a
table style for a run inside a table), or the property has no OOXML
default and nothing declares it (fontName, fontSizePt). A property
declared nowhere resolves to its OOXML default (false, highlightVal
false, colorHex 'auto'). A toggle set by a direct layer is absolute and
stays resolved; one reached only through style-level parity over an
unread base is unresolved. StylesModel gains optional docDefaultsRPr and
tableStyleRPrs probes (same docDefaultsRPr field #759 will resolve from).

Consumers:
- formatting tags: unresolved values form their own modal bucket (so
  tag output for resolved values is unchanged) and are never emitted;
  an unresolved size no longer produces <font size="0">.
- heading detectors: only a resolved true counts as bold/underline.
- formatting-convention check: tuples carry null; a divergence is only
  reported on a member resolved on both sides.
- formatting-loss script: unresolved projects as 'unresolved'.

Fixes #752
stevenobiajulu added a commit that referenced this pull request Oct 3, 2026
extractEffectiveRunFormatting returned '', 0 and false both for properties
declared nowhere and for properties declared only in layers it does not
read (w:docDefaults, table styles), so callers could not tell a known
default from missing resolver coverage.

RunFormatting fields are now nullable, and null means only "unresolved":
a non-default declaration exists in an unread layer (docDefaults, or a
table style for a run inside a table), or the property has no OOXML
default and nothing declares it (fontName, fontSizePt). A property
declared nowhere resolves to its OOXML default (false, highlightVal
false, colorHex 'auto'). A toggle set by a direct layer is absolute and
stays resolved; one reached only through style-level parity over an
unread base is unresolved. StylesModel gains optional docDefaultsRPr and
tableStyleRPrs probes (same docDefaultsRPr field #759 will resolve from).

Consumers:
- formatting tags: unresolved values form their own modal bucket (so
  tag output for resolved values is unchanged) and are never emitted;
  an unresolved size no longer produces <font size="0">.
- heading detectors: only a resolved true counts as bold/underline.
- formatting-convention check: tuples carry null; a divergence is only
  reported on a member resolved on both sides.
- formatting-loss script: unresolved projects as 'unresolved'.

Fixes #752
stevenobiajulu added a commit that referenced this pull request Oct 4, 2026
extractEffectiveRunFormatting returned '', 0 and false both for properties
declared nowhere and for properties declared only in layers it does not
read (w:docDefaults, table styles), so callers could not tell a known
default from missing resolver coverage.

RunFormatting fields are now nullable, and null means only "unresolved":
a non-default declaration exists in an unread layer (docDefaults, or a
table style for a run inside a table), or the property has no OOXML
default and nothing declares it (fontName, fontSizePt). A property
declared nowhere resolves to its OOXML default (false, highlightVal
false, colorHex 'auto'). A toggle set by a direct layer is absolute and
stays resolved; one reached only through style-level parity over an
unread base is unresolved. StylesModel gains optional docDefaultsRPr and
tableStyleRPrs probes (same docDefaultsRPr field #759 will resolve from).

Consumers:
- formatting tags: unresolved values form their own modal bucket (so
  tag output for resolved values is unchanged) and are never emitted;
  an unresolved size no longer produces <font size="0">.
- heading detectors: only a resolved true counts as bold/underline.
- formatting-convention check: tuples carry null; a divergence is only
  reported on a member resolved on both sides.
- formatting-loss script: unresolved projects as 'unresolved'.

Fixes #752
stevenobiajulu added a commit that referenced this pull request Oct 4, 2026
* fix(docx-core): expose unresolved run formatting

extractEffectiveRunFormatting returned '', 0 and false both for properties
declared nowhere and for properties declared only in layers it does not
read (w:docDefaults, table styles), so callers could not tell a known
default from missing resolver coverage.

RunFormatting fields are now nullable, and null means only "unresolved":
a non-default declaration exists in an unread layer (docDefaults, or a
table style for a run inside a table), or the property has no OOXML
default and nothing declares it (fontName, fontSizePt). A property
declared nowhere resolves to its OOXML default (false, highlightVal
false, colorHex 'auto'). A toggle set by a direct layer is absolute and
stays resolved; one reached only through style-level parity over an
unread base is unresolved. StylesModel gains optional docDefaultsRPr and
tableStyleRPrs probes (same docDefaultsRPr field #759 will resolve from).

Consumers:
- formatting tags: unresolved values form their own modal bucket (so
  tag output for resolved values is unchanged) and are never emitted;
  an unresolved size no longer produces <font size="0">.
- heading detectors: only a resolved true counts as bold/underline.
- formatting-convention check: tuples carry null; a divergence is only
  reported on a member resolved on both sides.
- formatting-loss script: unresolved projects as 'unresolved'.

Fixes #752

* fix(docx-core): address #758 review on unresolved formatting

- An unresolvable theme colour/font reference is unresolved (null) and
  stops inheritance, instead of reading as 'auto' or showing a lower
  layer through.
- Modal b/i/u baseline and the convention vote drop a member that any
  run/instance leaves unresolved, so uncertainty in one member cannot
  split the tuple and change tags or erase a convention on another.
- CHANGELOG enumerates the intentional output changes (explicit auto
  colour over Hyperlink style; unknown-norm tagging).

* fix(scripts): project unresolved formatting as null in the loss check

A string sentinel could collide with a real font literally named
'unresolved', hiding a dropped font declaration from D1. null never
occurs as a resolved value. (#758 review)
extractEffectiveRunFormatting now reads w:docDefaults/w:rPrDefault/w:rPr
as the lowest-precedence run-property layer (#753). Rebuilt on top of the
nullable RunFormatting contract from #758:

- StylesModel.docDefaultsRPr becomes a resolution layer: the last source
  for ordinary properties, and a 'default' toggle step that seeds the
  starting value before style parity and absolute direct formatting
  (MS-OI29500 note on 17.7.3; MS-OE376 2.7.7 documents no deviation that
  makes docDefaults another style level).
- docDefaults leaves the unread-layer list. Table styles remain unread:
  for a run in a table, a property nothing above docDefaults declares is
  null when a table style declares a value different from the base.
- fontName/fontSizePt resolve from docDefaults when declared; a theme
  font there resolves through the theme part and is null without one.
- Comment and footnote tagged_text use the document defaults as their
  font baseline, so the inherited default font is not emitted as face=
  on every run (which also broke docx-markdoc annotation import).

Fixes #753
@stevenobiajulu
stevenobiajulu force-pushed the 753-resolve-doc-defaults-formatting-20260729 branch from c10e0c7 to d78ab6b Compare October 4, 2026 01:02
@stevenobiajulu
stevenobiajulu marked this pull request as ready for review October 4, 2026 01:04
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-04T01:08:20.181673Z d78ab6b Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@usejunior-llm-gate

usejunior-llm-gate Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

LLM gate (advisory)

All evaluated rules passed - 10 pass, 0 warn, 0 error, 6 skipped, 16 total

Findings

None.

All 16 rules (10 evaluated, 6 skipped)
Rule Verdict Detail
read_file response metadata parity SKIPPED paths not touched by this PR
Live DOM namespace-safe OOXML writes PASS The PR touches packages/docx-core/src/primitives/comments.ts at lines 15 and 1494 but only modifies extraction/read logic to use extractAnnotationRunFormatting. It does not write any OOXML elements or prefixed attributes, so the namespace-aware write requirements are not triggered.
Complex-field revisions preserve complete accept/reject state machines PASS The PR does not touch field atomization, validateFieldStructure, w:fldChar, w:instrText, w:delInstrText, or collapsed-field comparison logic; it instead focuses on resolving w:docDefaults in effective run formatting.
Field validation per story, not global SKIPPED paths not touched by this PR
Revision IDs seeded from all revision-bearing side parts SKIPPED paths not touched by this PR
Accept/reject sweep side parts and caches PASS The PR does not touch DocxDocument.acceptChanges, DocxDocument.rejectChanges, REVISION_STORY_PART_PATHS, accept_changes, reject_changes, or side-part revision markup, as it is focused on resolving document-default run properties (#753).
DocumentViewNode.heading stays canonical SKIPPED paths not touched by this PR
AI-author parity across entry points SKIPPED paths not touched by this PR
Property-change wrapper discipline SKIPPED paths not touched by this PR
SUPPORT.md Table A drift vs. implementation PASS The PR does not modify OOXML revision emission behavior or touch SUPPORT.md; it only resolves document-default formatting during run property extraction.
Table A / Table B boundary on side-part revisions PASS The PR touches packages/docx-core/src/primitives/comments.ts:1491 and packages/docx-core/src/primitives/footnotes.ts:463 to update run formatting extraction, but it does not add or change any tracked-change revision markup or logic.
Canonical-emission surface completeness PASS The PR does not add or change any tracked-edit surface in primitives or tools, so the precondition is not met. It only resolves and tests w:docDefaults for effective run formatting in packages/docx-core/src/primitives/styles.ts.
Unit-test quality (avoid tautological / change-detector tests) PASS The PR adds and updates unit and integration tests (e.g. in packages/docx-core/src/primitives/styles-doc-defaults.test.ts:74) to verify docDefaults resolution (#753). All assertions are independent, use hardcoded values derived from first principles, make concrete semantic claims, and do not mock the system under test.
Re-derived facts vs canonical sources PASS The PR does not introduce any duplicate fact-derivations or logic re-computations; instead, the new helper extractAnnotationRunFormatting in review/packages/docx-core/src/primitives/styles.ts:705 directly consumes the canonical extractEffectiveRunFormatting to resolve effective and declared formatting.
.openspec tag ↔ test-assertion drift PASS The PR does not add, move, or change any .openspec tags on any tests, so the precondition is not met.
Library stays general (no downstream-domain leakage) PASS The PR introduces 'extractAnnotationRunFormatting' in 'packages/docx-core/src/primitives/styles.ts:705' and resolves 'docDefaults' in 'packages/docx-core/src/primitives/styles.ts:604', strictly adhering to general OOXML/Word vocabulary with no downstream or agreement-domain concepts.

…e-style probe

- Comment/footnote tagged_text report colour, size and font only where a
  layer above w:docDefaults declares them (extractAnnotationRunFormatting),
  instead of suppressing values equal to the document defaults. The
  equality baseline dropped a direct colour that restated the default over
  a named character style, which docx-markdoc then re-imported as the
  style's colour.
- The table-style probe compares hex colours and font names
  case-insensitively, so a restatement in other casing stays resolved.
@stevenobiajulu

Copy link
Copy Markdown
Member Author

Codex peer review, round 1: CHANGES_REQUIRED, now addressed in 9196b88

Reviewer: Codex (codex-review role). It ran the cited suites, then built and ran probes against the built packages.

# Finding Adjudication Fix
P1 The annotation font baseline (suppress colour/size/font equal to the docDefaults) drops a direct colour that restates the default over a named character style. docx-markdoc's importer keeps the styleId but does not recover direct colour from XML, so the run re-imports as the style's colour. The certificate still says lossy: false. Repro: red docDefaults, Blue style, direct red → imported body: [{"runs":[{"text":"red","style":{"styleId":"Blue"}}]}], and after compile <font color="0000FF">. Accepted. The same class of bug applied to face: a direct default font over an Arial style would have been silently imported as Arial, where before it was a safe rejection. Replaced the equality baseline with extractAnnotationRunFormatting. Comment and footnote runs report colour, size and font only when a layer above w:docDefaults declares them. Toggles, underline and highlight still resolve through docDefaults. This keeps annotation tags as they were before #753 for these three properties, and the ILPA imports stay admitted. formatting_tags.ts is back to its main version. New regression test in docx-markdoc annotation-roundtrip.test.ts, covering comment and footnote: a direct colour/size restating the docDefaults over a named style survives import, edit and compile (asserts <font color="FF0000" size="12">edited</font>). footnotes_structured.test.ts now also asserts that a direct default-font restatement over an Arial style keeps face.
P2 The table-style probe compares declared !== base, so abcdef restating ABCDEF turns a known colour into null. Accepted. Hex colours and font names now compare case-insensitively (sameValue). Tests cover a lowercase restatement and a theme colour that resolves to the same hex.

Other review notes, no change needed: the resolution probes match the intended model (seed-on / style-off → on; table-on → null; table-off → on under style-off-preserves parity). The new exports are reachable from the built package. Codex correctly notes that it did not re-run the Word oracle. The toggle claims rest on the #753 oracle rows and the MS-OI29500 / MS-OE376 citations, as the PR body says.

Gates rerun on 9196b88: build, lint:workspaces, check:cycles, spec-coverage, conformance-citations/doc/explorer, allure-labels/quality/filenames, tool-docs, test:docx-formatting-loss, and the docx-core, docx-mcp, docx-markdoc and docx-compare suites all pass. A confirmation review round follows.

@stevenobiajulu

Copy link
Copy Markdown
Member Author

Codex peer review, confirmation round: APPROVED (9196b88)

Codex re-verified both round-1 findings by running them:

  • P1 fixed. Its round-1 probe now keeps the direct red colour before import, in the imported body (alongside styleId: Blue), and after edit and compile. The new docx-markdoc round-trip test passes for comments and footnotes. All 22 tests in that file pass, ILPA cases included. A separate runtime probe showed that a direct Times New Roman restating the docDefaults over an Arial style gets exactly one face, and inherited values are not tagged.
  • P2 fixed. ABCDEF and abcdef now both resolve to ABCDEF. Case folding was checked against every ST_HighlightColor value in the vendored schema, and no two distinct values collapse.
  • formatting_tags.ts is byte-identical to main, and the CHANGELOG matches the code.

No open findings. Probe files under /tmp were cleaned up, and the repo worktree is clean.

@stevenobiajulu
stevenobiajulu merged commit fb171f0 into main Oct 4, 2026
29 checks passed
@stevenobiajulu
stevenobiajulu deleted the 753-resolve-doc-defaults-formatting-20260729 branch October 4, 2026 01:43
@stevenobiajulu

Copy link
Copy Markdown
Member Author

Post-merge smoke: PASS (fb171f0 vs pre-merge 6e60dcf)

I drove the MCP server (packages/safe-docx/bin/safe-docx.js) over stdio from both builds. Each document got read_file with show_formatting: true and include_footnotes: true, once in JSON and once in TOON. The documents were real forms from ~/.open-agreements/cache and OpenAgreements templates. The smoke compares body_run_formatting per node id across all 15 fields.

Document nodes fontName null (before → after) fontSizePt null null→resolved fields resolved values changed resolved→null size="0" body face= tags footnote face=
nvca-stock-purchase-agreement 368 368 → 0 338 → 2 704 0 0 0 5 → 0 0 → 0
nvca-voting-agreement 164 34 → 0 164 → 0 198 0 0 0 0 → 0 42 → 42
nvca-investors-rights-agreement 391 65 → 54 389 → 0 400 0 0 0 0 → 0 112 → 112
nvca-certificate-of-incorporation 228 227 → 0 11 → 11 227 0 0 0 1 → 1 1 → 1
yc-safe-valuation-cap 79 78 → 0 1 → 1 78 0 0 0 0 → 0 0 → 0
yc-safe-mfn 64 63 → 0 1 → 1 63 0 0 0 0 → 0 0 → 0
bonterms-mutual-nda 49 0 → 0 4 → 4 0 0 0 0 8 → 8 0 → 0
common-paper-cloud-service-agreement 307 3 → 0 5 → 0 8 0 0 0 0 → 0 0 → 0
  • No resolved value changed or became null anywhere, and no size="0" appeared in either build.
  • Body font/size come only from docDefaults in YC SAFE valuation-cap / MFN (Times New Roman), NVCA SPA / COI, and most of the Voting Agreement (sizes). Their previously-null fontName / fontSizePt now resolve.
  • The remaining nulls are explained. IRA has 54: those are table runs where the TableGrid table style declares Times New Roman over Calibri docDefaults, and table styles are still unread (fix(docx-core): expose unresolved run formatting #758 contract). COI has 11 and SPA 2 null sizes, in paragraphs where neither docDefaults nor any consulted layer declares w:sz.
  • SPA body face= tags go from 5 to 0. Those were explicit Times New Roman runs in paragraphs whose modal font used to be unresolved. Times New Roman is now the paragraph norm, so they are no longer deviations.
  • Footnote face= counts are identical to pre-merge (the annotation contract from the review fix).

Harness: /tmp/r759-smoke/smoke.mjs (raw rows: /tmp/r759-smoke/postmerge.json).

This branch was successfully deployed

1 active deployment
Preview — 9196b889 Deployed Oct 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix PR type: bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docx-core: run-formatting resolver ignores w:docDefaults, so most runs resolve fontName to an empty string

1 participant