feat(odf.js): add a real writeOdp/writeOdpContent presentation writer - #900
Merged
Conversation
Mearman
marked this pull request as ready for review
September 3, 2026 21:44
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. |
…form helpers
typed/odt/write.ts's own numId-run-planning (ListPlanState/listKindOf/canonicalNumId)
and its paragraph/table/cell/image canonical-form helpers were the only writer of
either kind in the package, so both lived as private functions in that one file.
A second writer needs the identical logic verbatim: list runs group and mint
canonical numIds the same way regardless of which container holds the text:list,
and writeOdfTable's own table/cell/image round-trip shape does not change
depending on which writer calls it.
Move the numId planning into typed/shared/list.ts (ListPlanState,
planListMembership, closeListPlan, listKindOf, canonicalNumId, NO_NUM_ID_KEY)
and the canonical-form helpers into a new typed/shared/canonicalise.ts
(canonicalColor, canonicalRun, canonicalParagraph, canonicalTable,
canonicalImage), with typed/odt/write.ts importing both rather than declaring
them locally. Pure extraction: normaliseOdtContent's own behaviour and every
existing odt test are unchanged.
The extracted NO_NUM_ID_KEY sentinel is now a genuine NUL character, matching
its own comment ("NUL -- forbidden in well-formed XML 1.0 content") and
registry.ts's identical FINGERPRINT_SEPARATOR convention -- the pre-existing
local copy was a literal space, which the comment never actually described.
typed/draw/shapes.ts's own readDrawFrame/walkDrawShapes are already shared between readOdp and readOdg -- their write-side mirror belongs in the same place, so a future odg writer reuses shape geometry, insets, and text/table/ image content writing rather than reimplementing it. writeDrawFrame turns one ContentShape into the draw:frame element the reader reads back: svg:x/y/width/height when unrotated, or svg:width/height plus a draw:transform="rotate(...) translate(...)" when rotated -- the exact algebraic inverse of typed/shared/transform.ts's resolveOdfShapeGeometry, derived by solving that module's own center/rotationDeg formulas for the translate() offset a given frame+rotationDeg requires. planShapeContent validates and discriminates a shape's blocks into the one content kind a real draw:frame can carry (table:table XOR draw:text-box XOR draw:image -- never a mix, since ODF has no spelling for one), refusing a heading, a page break, an embedded object, a construct boundary marker, or a mixed table/image by name rather than silently dropping it, and canonicalises any paragraph-level list membership onto the caller's own ListPlanState in the same pass. writeDrawShapes writes a whole page's shapes in document order. Not yet exported from the package barrel or wired into any writer -- the odp writer that actually uses this module lands as its own commit.
The inverse of typed/odp/read.ts, and this package's third content writer (typed/odt/write.ts's own top-of-file note states the shared discipline every writer in this family follows). writeOdpContent takes the flat 'presentation' ContentDocument readOdpContent returns and produces a real .odp Package; writeOdp does the same from the DocumentTree readOdp returns, via flattenTree. A presentation has no office:text body flow at all -- one style:master-page/ style:page-layout pair is minted per slide (a presentation genuinely allows different slides to reference different page geometry, unlike OOXML's single document-level p:sldSz), and each draw:page's own shapes are written by typed/draw/write-shapes.ts's writeDrawShapes. Speaker notes write as presentation:notes, one text:p per line, referencing a page-layout of their own (minted lazily, the first time any slide actually has notes) -- real LibreOffice output always states one, since a notes page is sized for printing independent of its slide's own on-screen size. normaliseOdpContent states the canonical form a written-and-reread document equals, including the one fact ODF forces rather than this writer choosing it: an image's own widthPt/heightPt become its enclosing shape's own frame size, since a draw:image has no size of its own inside a draw:frame. A slide's own residue (transition/animation/sound facts) is dropped, the same deliberate exception writeOdt makes. .odg and the .sxi OpenOffice.org 1.x wrapper are not covered here -- both are separate, tracked follow-up work built on top of what this module and typed/draw/write-shapes.ts establish.
write.test.ts pins the construct-by-construct XML writeOdpContent actually emits (package structure and media type, one style:master-page/page-layout per slide, unrotated vs rotated shape geometry, the graphic-family inset style, text/table/image shape content dispatch, list grouping, and speaker notes) -- the same role typed/odt/write.test.ts plays for writeOdtContent, proving the output is the ODF a real consumer expects rather than merely something this package's own reader happens to agree with. write-round-trip.test.ts states the law itself: normaliseOdpContent(readOdpContent(writeOdpContent(document))) equals normaliseOdpContent(document), covering metadata, multi-slide/multi-page-size documents, formatted runs and whitespace, bullet/ordered lists (including two shapes sharing one raw numId without merging their runs), a table and an image each as a shape's sole content, multiple shapes with insets, multi-line notes, residue dropping, and refusals (a page break, a mixed table, a heading). Rotated-shape geometry is checked separately with an explicit numeric tolerance rather than the blanket equality helper every other case uses, since two independent trig evaluations on either side of a real write- then-read round trip are not guaranteed bit-identical -- typed/draw/ write-shapes.ts's own frameGeometryAttrs is an exact algebraic inverse, not an approximation, but ordinary IEEE-754 rounding still applies to a real round trip. This suite is what caught the one genuine writer bug fixed in the previous commit: an image's own widthPt/heightPt, left to pass through verbatim in the canonical form, must instead become the enclosing shape's own frame size, since ODF's draw:image carries no size of its own at all.
…tion Status and Writing-a-document now cover writeOdp/writeOdpContent the same way they already cover writeOdt/writeOds -- what round-trips, what is refused by name, and where the shared shape writer (typed/draw/write-shapes.ts) sits so a future odg writer knows to reuse it. The Architecture section's own module list is updated to match (typed/shared/list.ts and canonicalise.ts as the write-side helpers odt and odp now share; typed/draw/write-shapes.ts beside shapes.ts). A new LibreOffice verification subsection records the real, independent- implementation check this PR's own bar requires: the exact soffice --headless commands run against a sample .odp covering multiple slides, mixed bold/italic/aligned text, a rotated shape, a nested bullet list, a table with a merged cell, an image, and multi-line speaker notes, plus what was found -- one real gap (a missing style:page-layout-name on presentation:notes) that got fixed, and one found, precisely characterised, and left open (LibreOffice's own undocumented AutoLayout placeholder-binding does not recognise this writer's minimal notes placeholder, though the notes text itself is never lost and the XML is well-formed per the OASIS schema). The .sxi/.sxd notes in the OpenOffice.org 1.x section are corrected: .sxi now has writeOdp to build its own writer against (tracked separately, not done here), and only .sxd still needs a writeOdg underneath it first.
…en part
writeOdp emits presentation:notes and presentation:class, but the shared
prefix list in package-io/scaffold.ts declared no presentation: prefix, so
every .odp carrying speaker notes was not namespace-well-formed XML.
xmllint rejects it outright ("Namespace prefix presentation on notes is not
defined"); LibreOffice instead imported the file with the notes element
unrecognised and re-homed its text onto the slide's own visible shape list,
so notes rendered on the slide and the notes page came back empty.
That symptom was previously recorded here as an AutoLayout placeholder
heuristic in LibreOffice, in writeSlideNotes's own comment and in the
README. It was a missing xmlns declaration; both now state the real cause.
Nothing between a writer and the emitted bytes checks that a qualified
name's prefix is bound, and this package's own reader matches prefixes as
plain strings, so the defect round-trips perfectly and only a real consumer
sees it. namespace-declarations.test.ts closes that structurally, walking
every XML part each writer produces and asserting every prefix used in an
element or attribute name is one the part's own root declares.
…Color objects
The new namespace-declarations.test.ts fixtures wrote background/border
colours as bare hex strings, but ColorSchema is an {r,g,b} object in 0..1 --
the fixtures type-checked as ContentBlock only because tsc's structural
narrowing under `as const satisfies` was never actually run against them
until now. Construct each one through rgbHexToColor instead.
…ent notation Number-to-string switches into exponent notation below 1e-6 and at or above 1e21, so formatOdfLength emitted values like "-7.1e-15pt". The OASIS `length` datatype has no exponent form at all, which made that output invalid ODF this package's own reader then discarded silently: parseOdfTransform drops a translate() whose components fail LENGTH_PATTERN, moving a rotated shape to its own pivot, and parseBox returns undefined for an unrotated frame's svg:x/svg:y, so readDrawFrame drops the whole shape and it vanishes from the slide. That magnitude is ordinary rather than contrived. A rotated frame's translate() components are trig-derived, so a frame at or near the page origin cancels to rounding dust instead of a clean zero at most angles. A 100x100pt frame at (0,0) rotated 270 degrees wrote translate(7.105427357601002e-15pt 100pt), and reading it back placed the shape 100pt from where it was written. Fixed on the write side rather than by widening LENGTH_PATTERN: accepting an exponent on read would leave every other ODF consumer seeing a length outside the datatype. The expansion re-positions the decimal point in the digits the shortest-round-tripping representation already chose, so it is an exact re-spelling rather than a rounding step, and carries no trailing fractional zeros for the same reason.
odf.js parses with processEntities:false, so an attribute value in this package's model is the literal source text: a shape named `Q&A <draft>` is stored as `Q&A <draft>`. readDrawFrame and the custom-shape text reader both projected that straight into ContentShape.name without decoding, so a name carrying any of the five predefined entities reached every consumer still escaped, with no way to know it was. Every other plain-text projection in this reader family already decodes -- svg:title and svg:desc via decodeOdfText, a form control's label via forms.ts, meta.xml's own fields via metadata.ts. An attribute value takes decodeXmlText directly rather than decodeOdfText, since text:s/text:tab/text:line-break are element-level spellings that cannot occur inside an attribute at all. A pre-existing reader defect, exposed for the first time by the odp writer: nothing wrote draw:name before it.
paintOrder is the one ContentShape field the odp/odg reader always populates (paintOrderKey stamps every draw:frame it walks), so the odp writer's canonical form carrying frame/insets/name/rotationDeg/blocks and nothing else made every odp -> odp and odg -> odp conversion silently lose explicit z-ordering. draw:z-index is the one spelling ODF has for a stacking order independent of document position, it is what the reader already resolves, and typed/ods/write.ts's anchored-drawing frames already emit one, so writing it here is existing precedent rather than a new convention. A paintOrder ODF cannot spell -- negative, or fractional, which ContentShapeSchema permits deliberately so a value can later be inserted between two existing ones -- writes no attribute rather than being rounded onto a neighbouring shape's order, and the reader's own document-encounter fallback then says the same thing. canonicalShape reads that decision off odfZIndexOf rather than re-deriving it, so the writer and the canonical form cannot disagree. canonicalShape now also names the fields it drops instead of leaving them silent, matching normaliseOdtContent's own convention. fontScale and lineSpacingReduction are DrawingML a:normAutofit percentages -- the shrink factor PowerPoint computed and stored -- and ODF's own autofit vocabulary is a mode flag carrying no computed factor, so writing one would invent a fact the input never stated while still losing the one it did. sourcePath and source are dropped for the reasons the odt writer already gives for the identical fields. Confirmed against LibreOffice 26.2.5.2: a slide whose shapes were written in an array order deliberately unlike their own paintOrder (3, 1, 0, 2) comes back through --convert-to fodp reordered into paintOrder order, with the attribute dropped and the elements physically moved instead.
…blishes The odp verification section said the conversion carried every piece of content "byte-for-byte", which is not what was checked and could not be: --convert-to fodp re-serialises the whole document through LibreOffice's own writer, renaming styles, reordering attributes, and adding defaults of its own, so its bytes differ from writeOdp's by construction. What the check establishes is that every authored string appears verbatim in that re-serialised output, which the section now says instead. Records what the re-run added: rotated frames at the page origin, shapes whose paint order disagrees with their document order, a draw:name carrying XML special characters, LibreOffice honouring draw:z-index by physically reordering the elements, and the one class of defect a green soffice run cannot catch -- LibreOffice's own length parser accepts values outside the OASIS length datatype, so exponent-notation lengths converted cleanly while being invalid ODF. Also documents paintOrder writing as draw:z-index and fontScale/lineSpacingReduction being dropped rather than approximated.
… surface createDrawShapeWriteState, DrawShapeWriteState, and planShapeContent were exported from the package entry point for a consumer that does not exist: the state constructor takes a StyleRegistry plus the raw XmlElement container automatic styles get appended to, which is internal plumbing odp's own writer and a future odg writer hold between themselves, not a shape any external caller has a use for. ShapeContentPlan goes with them, since only planShapeContent produces one. They stay ordinary exports of their own module, which is all an in-package caller needs. Publishing them would freeze that plumbing into the package's public API ahead of any concrete requirement for it, and turn every later change to it into a breaking one. Nothing outside this package referenced any of the four, and none of them has ever been in a released version -- they were added alongside the odp writer on this same unreleased branch -- so this removes an API surface no consumer can be holding.
odfZIndexOf accepted any Number.isInteger paintOrder, but Number.isInteger(1e21) is true and String(1e21) is "1e+21" -- the exact exponent-notation defect this branch's own formatOdfLength fix already closed for lengths, reintroduced here for draw:z-index. A value beyond 2^53 now writes no attribute at all, matching how a negative or fractional paintOrder already degrades to the reader's own document-encounter order rather than a spec-invalid attribute.
…ation readCustomShapeAsTextShape's own draw:name read was fixed to go through readDrawName rather than a bare attrValue, but nothing pinned it -- the whole suite passed with that call site mutated back to the unescaped form. draw:frame's own sibling call site was already covered.
…them The header claimed "THE THREE FIELDS THIS FUNCTION DROPS" while the bullets underneath named four (fontScale, lineSpacingReduction, sourcePath, source), and canonicalShape drops a fifth -- frames -- that went unmentioned entirely, in a comment whose whole point is that nothing is left silent. normaliseOdtContent already names all three of sourcePath/source/frames together for the identical reason; this states the same precedent rather than two of the three.
Mearman
force-pushed
the
feat/odf-odp-writer
branch
from
September 4, 2026 00:49
fe917c0 to
92e4471
Compare
This was referenced Sep 4, 2026
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.
Scope
Builds
writeOdp/writeOdpContent— the missing write half of odf.js's existingreadOdpContent/readOdp— matching the shape and disciplinewriteOdt/writeOdsalready established for their own formats.Explicitly out of scope, tracked separately:
.odg(drawings) — nowriteOdgyet. This PR factors the shape/page writer (typed/draw/write-shapes.ts) as a shared module specifically so a future.odgwriter can reuse it without reimplementing shape geometry, insets, or text/table/image content writing..sxi(OpenOffice.org 1.x presentations) — the thin wrapper that would inverttransformToOoo1PackageagainstwriteOdp, mirroring howwriteSxw/writeSxcalready wrapwriteOdt/writeOds.writeOdpis the prerequisite this unblocks (issue Add OpenOffice.org 1.x / StarOffice XML (.sxw/.sxc/.sxi/.sxd) read+write support #821); the wrapper itself is separate follow-up work.What's built
writeOdpContent(document)/writeOdp(tree)— the flat and tree-level entry points, mirroringreadOdpContent/readOdp.typed/draw/write-shapes.ts— the shared shape/page writer. OneContentShape→ a realdraw:frame, handling geometry (plainsvg:x/y/width/height, orsvg:width/height+draw:transformwhen rotated — the exact algebraic inverse of the reader'sresolveOdfShapeGeometry), paint order (draw:z-index), text insets (an internedgraphic-family style), and content dispatch (a shape's blocks are exactly one of plain text/list paragraphs, a sole table, or a sole image — refused by name for any other combination, since a realdraw:framecan hold only one ofdraw:text-box/table:table/draw:image, never a mix).writeDrawFrame/writeDrawShapesare the package's public surface here; the write-state constructor andplanShapeContentstay internal to the module, since the only callers are this package's own writers.style:master-page/style:page-layout(a presentation genuinely allows different slides to reference different page geometry, unlike OOXML's single document-levelp:sldSz).presentation:notes, onetext:pper line), referencing a shared notes-pagestyle:page-layoutminted lazily.normaliseOdpContentstates the canonical round-trip form, mirroringnormaliseOdtContent/normaliseOdsContent, and names every field it drops rather than leaving any silent.Refactor alongside it:
typed/odt/write.ts's own list-numId planning and paragraph/table/image canonical-form helpers were the only copies in the package; both are now shared (typed/shared/list.ts, newtyped/shared/canonicalise.ts) sincewriteOdpneeds the identical logic. Pure extraction —writeOdt's own behaviour and test suite are unchanged. Caught and fixed in the same pass: the pre-existingNO_NUM_ID_KEYsentinel was documented as NUL but was actually a literal space; it's now a genuine NUL character.Defects found in review and fixed
typed/shared/units.ts).formatOdfLengthwas a plain template-literal stringification, so any magnitude below 1e-6 came out as e.g.-7.1e-15pt. The OASISlengthdatatype has no exponent form, so that output was invalid ODF, and the reader discarded it silently:parseOdfTransformdrops atranslate()whose components failLENGTH_PATTERN(moving a rotated shape to its own pivot), andparseBoxreturnsundefinedfor an unrotated frame'ssvg:x/svg:y, soreadDrawFramedrops the shape entirely and it vanishes from the slide. This is not an edge case: a rotated frame'stranslate()components are trig-derived, so a frame at the page origin cancels to rounding dust rather than a clean zero at most angles — a 100×100pt frame at (0,0) rotated 270° wrotetranslate(7.105427357601002e-15pt 100pt)and read back 100pt away from where it was written. Fixed on the write side (fixed-point decimal only, an exact re-spelling of the digitsNumber-to-string already chose, not a rounding step) rather than by widening the read-side regex, which would have kept producing lengths outside the datatype for every other consumer.draw:namenot entity-decoded on read (typed/draw/shapes.ts). odf.js parses withprocessEntities:false, so an attribute value is the literal source text; a shape namedQ&A <draft>reached consumers asQ&A <draft>. Both call sites now decode via the samedecodeXmlTextthis reader family already uses forsvg:title/svg:desc, form-control labels, andmeta.xml. Pre-existing reader defect — this PR is just the first writer to emitdraw:nameat all.paintOrdersilently dropped (typed/odp/write.ts). The reader stampspaintOrderon every shape it walks, so the writer's canonical form ignoring it made every odp → odp and odg → odp conversion lose explicit z-ordering with no diagnostic. It now writes asdraw:z-index— the one ODF spelling for stacking order independent of document position, already read back bypaintOrderKey, and already emitted bytyped/ods/write.ts's anchored frames. A value ODF can't spell (negative or fractional, which the schema permits deliberately) writes no attribute rather than being rounded onto a neighbouring shape's order.ContentShapefields, now named rather than silent.fontScale/lineSpacingReductionare DrawingMLa:normAutofitpercentages — the shrink factor PowerPoint computed and stored — and ODF's own autofit vocabulary is a mode flag carrying no computed factor anywhere, socanonicalShapestates the loss instead of inventing a mode from a factor and still losing the factor.sourcePathandsourceare dropped for exactly the reasonsnormaliseOdtContentalready gives for the identical fields (a reader's own diagnostic path; quarantined, opaque residue).Verification
1. Self-consistency (round-trip law).
write.test.tspins the XML shapes construct-by-construct;write-round-trip.test.tsstates and checksnormaliseOdpContent(readOdpContent(writeOdpContent(document))) === normaliseOdpContent(document)across metadata, multi-slide/multi-page-size documents, formatted runs and whitespace, nested bullet/ordered lists (including two shapes sharing one raw numId without merging), a table and an image each as a shape's sole content, insets, multi-line notes, residue dropping, and every refusal. Rotated-shape geometry is checked with an explicit numeric tolerance rather than the blanket equality helper, since two independent trig evaluations on either side of a real round trip aren't guaranteed bit-identical (the inverse itself is exact algebra, not an approximation).Two genuine writer bugs were caught here before they shipped: an image's own
widthPt/heightPtmust become the enclosing shape's frame size on read-back, since ODF'sdraw:imagehas no size of its own inside adraw:frame; and the exponent-notation length above, now pinned by a sweep of 15 rotation angles across 7 frame boxes — several at or centred on the page origin — asserting every shape survives with its geometry intact, plus an unrotated frame whose ownsvg:x/svg:yreach exponent magnitude (which, before the fix, disappeared from the slide entirely).2. Real, independent implementation (LibreOffice 26.2.5.2). A sample
.odpcovering multiple slides (one widescreen, one A4-portrait), a shape with mixed bold/italic/plain runs and centred alignment, rotated shapes both away from and at the page origin, shapes whose paint order disagrees with their document order, adraw:namecarrying an ampersand and angle brackets, a nested bullet list, a shape carrying a table (with a merged cell) as its sole content, a shape carrying an image as its sole content, and multi-line speaker notes:Both exit
0with no error, andxmllint --nooutaccepts every written part. Every authored string appears verbatim in LibreOffice's re-serialised flat XML — all 3draw:pages, thetable:table, bothdraw:transforms, bothtext:lists, the escapeddraw:name, and every string of text. That is a content check, not a byte-identity one, and could not be either:--convert-to fodpre-serialises the whole document through LibreOffice's own writer, which renames styles, reorders attributes and adds defaults of its own. The rendered PDF (3 pages) visually confirms the bold/italic mixed formatting, the centred title, the nested bullets, both rotated shapes sitting where their frames put them, the table with its merged cell, and the A4-portrait slide's own different geometry.draw:z-indexis honoured: slide 1's shapes were written in an array order deliberately unlike their ownpaintOrder(3, 1, 0, 2), and LibreOffice re-emitted them inpaintOrderorder, dropping the attribute and physically reordering the elements instead — the mirror image of what the reader's owndraw:z-indexnote already documents finding in LibreOffice-authored files.One gap found and fixed: an earlier version wrote
presentation:noteswith nostyle:page-layout-nameat all. Real LibreOffice output always states one;writeOdpnow mints a shared notes-page layout lazily.A second gap found and fixed — an undeclared namespace prefix. Speaker notes initially arrived on the slide itself rather than its notes page, which looked like LibreOffice's own AutoLayout placeholder-matching declining to bind a minimal
presentation:notes/draw:frame. It wasn't:presentation:notesand its frame'spresentation:classare the onlypresentation:-prefixed names any writer here emits, andpackage-io/scaffold.ts's shared prefix list never declared that prefix, so the part was not namespace-well-formed XML at all (xmllint --noout content.xml:Namespace prefix presentation on notes is not defined). LibreOffice imported it anyway, treated the unrecognised element as ordinary slide content, and re-homed its text onto the visible shape list. With the prefix declared,--convert-to fodpround-trips the notes insidepresentation:noteswhere they were written, and--convert-to pdfrenders no notes text on the slide page.src/package-io/namespace-declarations.test.tsnow audits every prefix each writer emits against what that part's root declares, so the next one fails a test instead of shipping.Worth knowing about what a green
sofficerun does and doesn't prove: LibreOffice's own length parser accepts values outside the OASISlengthdatatype, so the exponent-notation lengths above converted cleanly and landed in the right place. The verification pass could not have caught that defect — only odf.js's own spec-conforming reader did. It's pinned by unit tests rather than by asofficerun for exactly that reason.3. Full check suite, green from a forced (uncached) run:
pnpm exec turbo run _lint _typecheck _test _test:workers --filter=odf.js --force(1144 tests) and a full-workspacepnpm exec turbo run _lint _typecheck --force(57 tasks).Reviewer checklist
typed/draw/write-shapes.ts'sframeGeometryAttrs(worth checking the derivation againstresolveOdfShapeGeometrydirectly).canonicalShape's stated losses (typed/odp/write.ts) — particularly the claim that ODF has no target forfontScale/lineSpacingReduction, which is the one judgement call rather than a forced fact.draw:z-indexfor a fractional/negativepaintOrder(rather than refusing the write) is the right call.typed/shared/list.ts/typed/shared/canonicalise.tsextraction — confirm it's genuinely behaviour-preserving forwriteOdt(the full pre-existing odt test suite passes unchanged, but worth a second look given it touches shared infrastructure).Independent adversarial review, second round
A final independent pass (against
c8d74f5a, after S1-S4 above already landed) found three real, narrower problems and confirmed everything else holds:odfZIndexOfaccepted anyNumber.isIntegerpaintOrder, butNumber.isInteger(1e21)istrueandString(1e21)is"1e+21"— the identical exponent-notation defect this same branch had just closed for lengths, reintroduced fordraw:z-index. Fixed by tightening the guard toNumber.isSafeInteger, with a regression test.draw:namedecoding fix (S3) had zero test coverage: mutating the odg custom-shape call site (readCustomShapeAsTextShape) back to a bareattrValueleft the entire suite green. Added a test that fails on that exact mutation.canonicalShape's own "nothing dropped silently" comment said "THE THREE FIELDS" while naming four, and never mentioned a fifth (frames) it also drops — corrected to name all five, matchingnormaliseOdtContent's existing precedent for the identical fields.All three verified by reverting each fix individually and confirming the corresponding new test fails, then re-passing with the fix restored. Full check suite green from a forced run afterward (
_lint _typecheck _test _test:workers --filter=odf.js --force, plus a full-workspace_lint _typecheck --force).