fix(super-converter): keep the document when a style record is unreadable (#3861) - #3865
fix(super-converter): keep the document when a style record is unreadable (#3861)#3865caio-pizzol wants to merge 2 commits into
Conversation
|
The Status: PASS This PR is a refactor of the styles.xml reader — it swaps ad-hoc
One non-spec note (not a violation, so it doesn't change the verdict): the tab No non-existent attributes/elements, no missing-required handling, no wrong defaults. |
b743102 to
5ed2957
Compare
|
The tab filter note is already fixed. That review read .filter((tab) => attrValue(tab, "w:val") != null && attrValue(tab, "w:pos") != null)It was a real defect, not just a robustness edge: a Ran the schema lookups the review could not, and both of its claims hold:
|
Code Review by Qodo🔴 Action Required 1. Join preserves deleted formatting
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…able The style catalogue is built outside the importer's per-node recovery boundary, so one unreadable `w:style` aborted createDocumentJson. getSchema then returned null and createDocument fell back to an empty document, discarding the whole body instead of a single node. Guarding the crash was not enough on its own: an outline level with a missing or non-numeric w:val still parsed to NaN, and a tab stop missing w:val or w:pos was still emitted half-formed. Both attributes are required in ECMA-376, so neither record carries anything to apply and both are now dropped. Also skips style records with no w:styleId, since nothing in the document can reference them, and removes the latent-style match loop that collected into a list nothing ever read.
parseInt reads a numeric prefix, so an outline level of "2abc" or "3.9" imported as level 2 or 3 and silently reshaped headings and the TOC. w:val is ST_DecimalNumber, so the whole value must be an integer. An empty w:pos converted to a plausible-looking position 0 rather than an obvious NaN. w:pos is ST_SignedTwipsMeasure, a union of xsd:integer and ST_UniversalMeasure, so the stop is judged by whether it converts to a finite position instead of by matching the raw string, which would reject legal values such as "1.5in".
4272276 to
b7f9d2d
Compare
💡 Codex Review Delete accepted terminal block instead of clearing only its mark
When the accepted whole-block deletion is the last top-level paragraph/list item, ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Out of scope for this PR, but it may be a real finding against a different change.
Neither commit on this branch modifies Those lines came from Not dismissing the substance. If the terminal-block case is genuinely leaving an emptied numbered paragraph behind, that is worth pursuing, just against the change that introduced it rather than here. Happy to open a separate issue referencing this comment if that is useful. |
|
This pull request landed on |
|
🎉 This PR is included in superdoc-cli v0.23.1 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.22.1 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.18.1 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.46.1 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.17.1 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.18.1 |
1 participant
Stops one malformed
w:stylefrom discarding an entire imported document. Same root cause as #3861, much worse outcome, so it is split out for separate review.The style catalogue is built outside the importer per-node recovery boundary. A throw there aborts
createDocumentJson,getSchemareturns null, andcreateDocumentfalls back to an empty document, losing the whole body instead of one node. Theexceptionevent does fire and hosts can subscribe viaonException, so it is not strictly silent, but the default is a blank editable page with no error state.<w:rPr />(and similar) instyles.xmltable styles aborts DOCX import #3861 does not reach this path. It needs an emptyw:outlineLvl, an emptyw:tab, a repeatedw:styleIdwhose second record is empty, or aw:stylewith no attributes. Whether the reporting customer hit this is unknown without their file.w:valstill parsed to NaN, and a tab stop missingw:valorw:poswas still emitted half-formed. Both attributes are required in ECMA-376, so both records are now dropped, and the tests assert parsed values rather than only that import survived.w:styleIdare skipped, since nothing can reference them, and the latent-style loop that collected into an unread list is gone.Not included: the
getSchemafallback of returning null and mounting an empty document. Turning a parse failure into a blank page is arguably wrong, but it changes an initialization contract and deserves its own decision.