feat(ppt-codec): write real MS-PPT presentations - #877
Merged
Conversation
Mearman
marked this pull request as ready for review
September 3, 2026 12:20
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. |
record/write.ts now owns the atom/container builders and little-endian byte primitives that src/test-support/records.ts previously duplicated for fixtures alone. Every test file and test-support/presentation.ts import writeAtom/writeContainer (aliased to the atom/container names they already used) directly from record/write.ts instead, so a test fixture and a genuinely written record are built by identical code with nothing left to drift between two copies.
Adds a genuine [MS-PPT] write path alongside the existing reader: one slide per input slide, with plain text-box shapes carrying paragraph text, character formatting (bold/italic/underline, font family, size, literal RGB colour), and paragraph alignment/list level, wrapped in a real [MS-CFB] compound file via archive-codec's writeCompoundFile. Every record byte layout mirrors the equivalent read function's own field order exactly, so round-tripping written bytes back through the existing, independently-built reader (writePptContent -> readPptContent, writePpt -> readPpt) is the primary correctness check. Content this writer cannot express throws PptUnsupportedContentError: a non-presentation document, or slides that do not share one size, since DocumentAtom states exactly one slide size for the whole presentation. A block kind the OfficeArt shape tree has no room for (image, table, embeddedObject, a construct marker) is excluded from the written text body rather than erroring, the same documented-gap convention the reader already applies to its own unsupported constructs. Scope deliberately narrower than the reader: no images, tables, OLE embeddings, masters, layouts, scheme colours, speaker notes, or document metadata. Text always goes directly on a shape's own OfficeArtClientTextbox rather than through the OutlineTextRefAtom placeholder indirection, since every shape this writer produces is a plain text box with no placeholder role.
Adds a "Writing a document" usage section alongside the existing "Reading a document" one, and "What it writes"/"What it does not write yet" tables mirroring the read-side pair exactly, so a caller can tell from the README alone whether a construct it cares about round-trips. Updates the Architecture module table with every new write-side module, the Conventions/Specification sections to note the writer follows the same throw-on-malformed discipline (via PptUnsupportedContentError on the write side), and the workspace root README's package table and conversion-engine note, which previously described this package as read-only.
Mearman
force-pushed
the
worktree-agent-a27bf3e0aab3fe8cc
branch
from
September 3, 2026 12:21
8143635 to
350186c
Compare
This was referenced Sep 3, 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.
Summary
Adds a genuine [MS-PPT] write path to
ppt-codec, alongside the existing reader. This is not full read/write parity with #817's stated scope, and does not close it — see below for exactly what's covered.FontCollectionContainer), size in points, and literal RGB colour.CurrentUserAtom->UserEditAtom->PersistDirectoryAtom) and a genuine [MS-CFB] compound file viaarchive-codec'swriteCompoundFile.PptUnsupportedContentError(a non-presentation document, or slides that don't share one size, sinceDocumentAtomstates exactly one slide size for the whole presentation). A block kind the shape tree has no room for (image/table/embeddedObject/construct marker) is silently excluded from the written text body, mirroring how the reader already documents its own unsupported constructs rather than erroring on them.Every record's byte layout is the direct inverse of the equivalent read function's own field order (not a fresh guess at the spec) —
record/write.tsand each*-write.tsmodule cites the same [MS-PPT]/[MS-ODRAW] sections the corresponding reader module already does.Verification
The primary verification is a direct round trip through this package's own, independently-built reader: write real records, then read them back via
readPptContent/readPpt, and assert the recovered content equals what was written (write.test.ts,content-write.test.ts,text/style-write.test.ts— 38 new tests). This proves the written bytes are genuinely conformant [MS-PPT], not merely internally self-consistent, since the reader was built and tested against the specification alone before any writer existed. The Cloudflare Workers suite (test/workers/ppt-codec.test.ts) now also exercises a write-then-read round trip inside a realworkerdisolate, proving the write path (andarchive-codec's CFB writer beneath it) is genuinely Worker-isomorphic too.As a side effect,
src/test-support/records.ts's byte builders (used by every existing read-path fixture) are promoted intorecord/write.tsas real production code, so fixtures and genuinely written files are now built by identical code rather than two copies of the same logic.What's out of scope (tracked, not silently dropped)
Documented exhaustively in the package README's new "What it does not write yet" section: images/tables/OLE embeddings, grouped/rotated shapes, per-shape text insets and autofit, masters/layouts/scheme colours, speaker notes and document metadata, hyperlinks/bullets/spacing/margins/list numbering identity,
strike(no [MS-PPT]TextCFExceptionbit exists for it at all), and animations/transitions/comments.Test plan
pnpm exec turbo run _lint _typecheck _test _test:workers --filter=ppt-codec— greenpnpm exec turbo run _lint _typecheck(full workspace sanity pass) — green