Repository navigation
fix(docx-core): expose unresolved run formatting - #758
Merged
stevenobiajulu merged 3 commits intoOct 4, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
stevenobiajulu
force-pushed
the
752-express-unresolved-run-formatting-20260729
branch
from
October 3, 2026 23:36
8bcf3ff to
2e66725
Compare
stevenobiajulu
marked this pull request as ready for review
October 3, 2026 23:38
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
LLM gate (advisory)All evaluated rules passed - 8 pass, 0 warn, 0 error, 8 skipped, 16 total FindingsNone. All 16 rules (8 evaluated, 8 skipped)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e667255b6
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| function runEmphasis(run, paragraphPPr, paragraphStyleId, styles, theme) { | ||
| const formatting = extractEffectiveRunFormatting({ run, paragraphPPr, paragraphStyleId, styles, theme }); | ||
| const resolved = (value) => value === null ? 'unresolved' : value; |
There was a problem hiding this comment.
When a run explicitly uses a typeface named unresolved, this helper projects that valid fontName to the same value as a run whose font is actually unresolved. Consequently, removing that explicit font declaration can leave the before/after emphasis arrays identical, causing the formatting-loss check to miss a real font change. Preserve null or use a tagged value that cannot collide with arbitrary OOXML font names.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Accepted, fixed in 4ea1db1. Unresolved fields now project as null, which no resolved value ever is, so they cannot collide with a font literally named unresolved. A regression test checks that dropping such a font is still reported by D1.
stevenobiajulu
added a commit
that referenced
this pull request
Oct 3, 2026
- 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).
stevenobiajulu
force-pushed
the
752-express-unresolved-run-formatting-20260729
branch
from
October 3, 2026 23:59
2e66725 to
b40e05d
Compare
Member
Author
Codex peer review, round 1: CHANGES_REQUIREDReviewer: Codex ( Adjudication (all four accepted; fixed in b40e05d)
Main also moved (#1143, #1145) during review, so the branch is rebased again. The only conflict was in CHANGELOG, resolved by keeping both entries. All local gates are green after the rebase: build, lint, cycles, spec-coverage, conformance-citations, allure-labels/quality/filenames, tool-docs, formatting-loss, docx-core, docx-mcp. A confirmation round follows. |
Member
Author
Codex peer review, confirmation round: APPROVEDReviewer: Codex (
Additional checks it ran:
No remaining findings. |
stevenobiajulu
added a commit
that referenced
this pull request
Oct 4, 2026
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 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
- 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).
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)
stevenobiajulu
force-pushed
the
752-express-unresolved-run-formatting-20260729
branch
from
October 4, 2026 00:26
4ea1db1 to
7920735
Compare
stevenobiajulu
deleted the
752-express-unresolved-run-formatting-20260729
branch
October 4, 2026 00:44
Member
Author
Post-merge smoke: PASSThis compares the built merge commit
|
stevenobiajulu
added a commit
that referenced
this pull request
Oct 4, 2026
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
added a commit
that referenced
this pull request
Oct 4, 2026
* fix(docx-core): resolve run formatting from document defaults 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 * fix(docx-core): address #759 review on annotation formatting and table-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.
3 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes "not resolved" representable in docx-core's effective run formatting (#752).
extractEffectiveRunFormattingused to return'',0andfalseboth for properties declared nowhere and for properties declared only in layers it does not read, so a caller could not tell a known default from missing resolver coverage.Rebased onto main (195 commits since the July draft) and reworked. The draft made every undeclared property
null. That would have turned every plain run'sboldintonulland silenced the #687 formatting-convention warning permanently, because that warning must not fire from an unresolved value.Contract (option A from #752: nullable per property)
RunFormattingfields are nullable, andnullmeans only unresolved:rPr,rStylechain, paragraph-markrPr,pStylechain) declares the property, and an unread layer declares a non-default value for it. The unread layers arew:docDefaults/w:rPrDefault/w:rPr, and any table style'srPr/tblStylePr/rPrwhen the run is inside a table; orfontName/fontSizePt, which have no OOXML default, and nothing consulted declares it.A property declared nowhere resolves to its OOXML default:
falsefor toggles andunderline,highlightVal: false(wasnull), andcolorHex: 'auto'(wasnull). A toggle that a direct layer sets is absolute and stays resolved. A toggle reached only through style-level parity over an unread "on" base isnull.StylesModelgains optionaldocDefaultsRPrandtableStyleRPrs. They are probes only and resolve no values. Hand-built{ byId }models still type-check. #759 can replace thedocDefaultsRPrprobe with a real resolution layer under the same field name.Consumers
read_fileoutput): unresolved values form their own modal bucket, so tags for resolved values are unchanged, and unresolved values are never emitted. An unresolved run inside a paragraph whose modal size is resolved no longer gets<font size="0">.truecounts as bold or underline.null. A divergence is reported only on a member that is resolved on both sides and differs, and the message prints an unresolved member asunresolved.'unresolved'.comments.ts,footnotes.ts,document_view.ts,docx-markdoc/inspect.ts: these pass the values through to the tag emitter or to JSON with no coercion, so their code is unchanged.Breaking / client-visible
RunFormatting: all fields are nullable (see CHANGELOG, Unreleased).read_fileJSONbody_run_formatting:fontName/fontSizePtarenullinstead of""/0when unresolved,highlightValisfalseinstead ofnull, andcolorHexis"auto"instead ofnull.Validation
npm run build,lint:workspaces,check:cycles,check:spec-coverage,check:conformance-citations,check:allure-labels,check:allure-quality,check:allure-filenames,check:tool-docstest:docx-formatting-losspassedFixes #752