v0.1.0

[leogdion]

No description provided.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d381e5f-d620-430b-889b-e0b397aacdae

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[leogdion]

Code review — PR #43 (v0.1.x → main)

Two-axis review of main...HEAD (17 commits, ~659 files). Axes are intentionally separate: Standards = does the code follow this repo's documented conventions; Spec = does it match the v0.1.0 PLAN / map #12. Generated protobuf, research fixtures/goldens, and agent-skill copies were out of scope except where they touch packaging rules.

Fixed point: main (1937f49) … HEAD: 4b756e0

---

Standards

Verdict: Largely clean against documented standards. No hard breaches found in the hand-written write path.

Hard standards (checked)

• Swift 6.4-only — // swift-tools-version: 6.4; no Package@swift-* (.claude/memory/keynotekit-swift-64-only.md).
• Products / deps — Snappy → IWAFraming → KeynoteKit + KeynoteKitProtobuf; Scripting only in KeynoteKitScripting under #if canImport(ScriptingBridge); no .linkedFramework (Package.swift, PLAN / CI conventions).
• Template resource — .copy("Resources/blank.key"), not .process.
• No Process in Sources/Tests; zip checks use KeyBundle (agent-notes 2026-07-31).
• DSL — TextRunsBuilder / SlideItemsBuilder only accept Text / TextBox / Image; no buildExpression(_: String).
• has* remaps — RecordCloner guards optional refs before remap(&…) (agent-notes 2026-07-31).
• Gates — golden path is ArchiveGraphComparer (structural); ByteIdentityDiagnosticTests is opt-in / non-blocking.
• Snappy — no Apple/Foundation imports; struct Snappy + static let default / private init.
• Lint — LINT_MODE=STRICT in CI; Generated excluded in .swiftlint.yml.

Soft / related (not a hard remap breach)

• KeynoteArchiveSurgeon+DrawableItems.swift: placeholder.super.ownedStorage.identifier with no hasOwnedStorage — read path, local value discarded, but same materialization footgun as the has* rule if ever written back.

Judgement-call smells (watch)

1. Message Chains — placeholder.super.super.super.geometry… / .caption / .title / .parent in KeynoteArchiveSurgeon+DrawableItems.swift and RecordCloner.swift.
2. Duplicated Code — near-identical KeyFixture / KeyFixtureCorpus in IWAFramingTests and KeynoteArchiveNavigationTests (latter notes test-target isolation).
3. Shotgun Surgery — KeynoteArchiveSurgeon+*.swift spread (acceptable vs file_length / one_declaration_per_file; lint wins).

---

Spec

(a) Missing / partial

• 12 — Acceptance — five decks in Keynote 15.3 #24 human DoD still open (not a code gap). PLAN: “Definition of done for v0.1.0: a deck authored purely in Swift opens in Keynote 15.3 with no crash and no repair warning….” Issue 12 — Acceptance — five decks in Keynote 15.3 #24 still unchecked: In/Out/Action + ordering/direction; geometry/formatting/image checks; v0.1.0 tagged. Code side is present: AcceptanceDeckCatalog (9 decks), AcceptanceDecks, AcceptanceDeckTests / GoldenDifferentialTests. research/findings/acceptance_keynote_open.md expanded checklist is blank — tag gate only.
• MotionPath is sharp-polyline only. PLAN Step 5 / decision log: “Full editableBezierPathSource in v0.1.0 (not a simplified Move-only sugar)” / “full MotionPath bezier in v0.1”. Wire format emits editableBezierPathSource, but public/AuthoredMotionPath nodes are sharp only (coincident in/out controls in BuildRecordFactory.pathSource) — curved handles aren’t authorable. Docs on MotionPath claim “full editable bezier path source”; the public surface is polyline nodes.

(b) Scope creep

• Full KeynoteKitScripting body. PLAN products table still says “KeynoteKitScripting … Scaffolded empty in Step 0 … body is Expose a ScriptingBridge escape hatch for direct Keynote control #10, past v0.1.” Diff ships a full ScriptingBridge surface (KeynoteScriptingApplication, docs/slides/text items, transitions, export) via Expose a ScriptingBridge escape hatch for direct Keynote control (#10) #35. Correctly separate product / #if canImport(ScriptingBridge) / not on Deck.write; still beyond the original v0.1 “empty scaffold.” Expose a ScriptingBridge escape hatch for direct Keynote control #10 live verify remains open (acceptance checklist). PLAN table is stale relative to the landed Expose a ScriptingBridge escape hatch for direct Keynote control #10 implementation.
• SlideDrawable + Image + #40 Text runs exceed the original Step 5 “concrete Text only / no item-level protocol,” but match promoted Drawable geometry: width, height, z-order #3/Text formatting on authored Text items #37/Author images as slide drawables #38/Support mixed formatting (runs) within a single text box #40 — intentional, not accidental.

(c) Looks done, wrong / stale vs shipped surface

• Deck.swift still says “Slide content is not modelled yet” while Deck { @SlideBuilder } + SlideContent ship — contradicts Goal sketch / 11 — Authoring API (SlideContent surface) #23.
• TextBox docs: “the only drawable v0.1.0 authors” — false after Image (Author images as slide drawables #38).

Present and matching PLAN (no finding): five library products; KeynoteKit → IWAFraming/Snappy + Protobuf, no ScriptingBridge; bundled blank.key + write(to:basedOn:); #17 semantic round-trip; #18 KeynoteArchiveNavigation non-product + ExtractBuildsParityTests; #20 goldens + UUIDMapVerifier; #22 supply tests; Goal sketch via TextBox/MotionPath (GoalSketchTests); no public Deck(reading…); Linux CI.

---

Summary

Axis	Findings	Worst
Standards	0 hard · 1 soft footgun · 3 judgement smells	Message-chain / optional-field materialization risk in drawable surgery
Spec	2 partial · 1 intentional scope (scripting) · 2 stale docs	#24 human acceptance still the only v0.1.0 tag gate; MotionPath public API is sharp-only vs “full bezier” PLAN wording

Merge posture: code/product shape matches the v0.1.0 PLAN gates that are automatable. Do not treat merge of this PR as the v0.1.0 tag — finish the expanded #24 checklist in research/findings/acceptance_keynote_open.md first. Before tag, also fix the two stale public-API doc comments and clarify whether sharp-only MotionPath satisfies the PLAN’s “full bezier” decision or needs a follow-up ticket.

[leogdion]

Code Review: PR #43 (v0.1.x → main)

This is a two-axis review of PR #43 comparing v0.1.x against main.

---

1. Standards

Documented Repo Standards (Hard Violations)

1. Un-guarded Protobuf Optional Message Access in RecordCloner.swift
   • File: Sources/KeynoteKit/RecordCloner.swift (L153–155)
   • Standard: .claude/agent-notes.md — "NEVER remap a swift-protobuf optional message field without its has* guard — remap(&msg.field) on an absent field MATERIALIZES an empty reference (identifier 0)..."
   • Hunk:

     private static func rewrittenStorage(_ payload: [UInt8], map: [UInt64: UInt64]) throws -> [UInt8] {
       var storage = try TSWP_StorageArchive(serializedBytes: payload, partial: true)
       for index in storage.tableAttachment.entries.indices {
         remap(&storage.tableAttachment.entries[index].object, map: map)
       }
       return try storage.serializedBytes(partial: true)
     }

   • Violation: Accessing storage.tableAttachment.entries without checking if storage.hasTableAttachment materializes an empty optional message field (identifier 0) when tableAttachment is absent, risking silent component load failures or NSSet-nil crashes.

Baseline Code Smells (Judgement Calls)

1. Message Chains — Deep Protobuf Inheritance Traversal (super.super.super)

   • Files:
     • Sources/KeynoteKit/RecordCloner.swift (L136–144) (placeholder.super.super.super.hasCaption, etc.)
     • Sources/KeynoteKit/KeynoteArchiveSurgeon+DrawableItems.swift (L194–200) (placeholder.super.super.super.geometry.position.x, etc.)
   • Smell: Navigating through 3 levels of protobuf message inheritance exposes internal framing.
   • Recommendation: Introduce helper extensions or properties on KN_PlaceholderArchive (e.g. drawableGeometry) to encapsulate the walk.

2. Duplicated Code — Repeated has* & remap Pattern

   • File: Sources/KeynoteKit/RecordCloner.swift (L136–144)
   • Smell: The exact sequence of if hasX { remap(&x, map: map) } repeats 5 consecutive times across deprecatedStorage, ownedStorage, caption, title, and parent.

---

2. Spec

(a) Missing or Partial Requirements

1. magicId Pairing & Lowering is a No-Op
   • Spec Quote: .claude/PLAN.md (L63): "- **magic-id is compile-time only** — Keynote persists no object correspondence; matching is a runtime heuristic. The compiler must emit matched objects as the same type with similar content/geometry."
   • Finding: In Sources/KeynoteKit/Deck+Lowering.swift (L78–85), touchMagicIdentifiers executes _ = text.magicIdentifier and _ = image.magicIdentifier, discarding the identifier without matching or aligning corresponding drawables across adjacent slides.

(b) Scope Creep (Unasked-for Behaviour)

1. Full Implementation of KeynoteKitScripting (Expose a ScriptingBridge escape hatch for direct Keynote control #10)

   • Spec Quote: .claude/PLAN.md (L184): "KeynoteKitScripting | ScriptingBridge escape hatch (#10) | Scaffolded empty in Step 0 (name + product reserved); body is #10, past v0.1"
   • Finding: Commit 068cefe (PR Expose a ScriptingBridge escape hatch for direct Keynote control (#10) #35) fully implemented KeynoteKitScripting with application, document, slide, and text item ScriptingBridge wrappers plus live test suites rather than leaving it scaffolded empty.

2. Drawable Depth & Geometry (Drawable geometry: width, height, z-order #3, Text formatting on authored Text items #37, Author images as slide drawables #38, Support mixed formatting (runs) within a single text box #40)

   • Spec Quote: .claude/PLAN.md (L467–468): "| #3 | Drawable geometry | unblocks size-changing Magic Move" and "| #4 | Shapes and images | authoring doesn't"
   • Finding: Commits 211b956 (PR Drawable depth: geometry, text formatting, and images (#3, #37, #38) #39) and 45b8c4f (PR Mixed formatting runs within a single text box (#40) #41) pulled forward image authoring (Image.swift), text formatting (TextColor.swift), and geometry/z-order into v0.1.0 (promoted into the plan on 2026-07-30).

(c) Requirements Implemented Wrongly

1. Opaque Transition Direction Enum

   • Spec Quote: .claude/PLAN.md (L402–403): "Direction via a typed enum on directional effects only (e.g. .push, .moveIn), chained as .direction(.…)"
   • Finding: Sources/KeynoteKit/TransitionDirection.swift (L38) exposes only public static let moveInNonDefault = TransitionDirection(ordinal: 11) with doc comment "its inspector label was not recorded", offering no semantic direction names (e.g. .leftToRight).

2. Empty Deck().write(to:) Bypass

   • Spec Quote: .claude/PLAN.md (L48): "Definition of done for v0.1.0: a deck authored purely in Swift opens in Keynote 15.3 with no crash and no repair warning"
   • Finding: Sources/KeynoteKit/Deck.swift (L83–86) bypasses slide processing when slides.isEmpty by copying the template file directly to disk, skipping archive surgeon invariant verification.

---

Summary

• Standards: 1 hard violation (missing hasTableAttachment guard in RecordCloner.swift:L153) and 2 judgement smells (super.super.super chains, repeated remap pattern).
• Spec: 5 findings across missing requirements (magicId lowering is a no-op), scope creep (KeynoteKitScripting fully implemented, drawable depth added), and incorrect implementations (unlabelled transition direction ordinal, empty deck write bypass).

[leogdion]

Code review: v0.1.0 (v0.1.x → main)

Overview & scope

This PR lands the entire v0.1.0 release: a Swift package that authors Keynote .key files by byte-surgery on a bundled template, with no Python/keynote-parser/AppleScript at runtime. Of the 659 files / ~326k added lines, only ~20k lines are hand-written — the rest is generated protobuf (Sources/KeynoteKitProtobuf/Generated/), vendored protos, and research/ artifacts. This review covers the hand-written code: Snappy, IWAFraming, KeynoteKitProtobuf (hand-written parts), KeynoteArchiveNavigation, the KeynoteKit surgeon + DSL layers, KeynoteKitScripting, the acceptance-deck targets, tests, and build/CI configuration. Every finding below was verified against the actual code, and severities are calibrated to the project's own documented invariants (.claude/agent-notes.md, .claude/PLAN.md).

Overall assessment: the architecture is strong — clean layer boundaries, research-traceable constants, hard-won crash invariants encoded in code and enforced by a genuinely failable UUIDMapVerifier, and tests that assert archive structure rather than smoke. There is 1 critical bug (multi-image identifier collision), a handful of majors that cluster into three themes — 32-bit / malformed-input trap paths, bundled-template assumptions leaking into the public arbitrary-base API, and DSL result-builder gaps — plus targeted test-coverage and CI-hygiene items.

---

🔴 Critical

C1. Two images on one slide collide on the same TSP.DataInfo identifier

Sources/KeynoteKit/KeynoteArchiveSurgeon+ImageSupply.swift:175

mintImageDrawable allocates data: try nextDataIdentifier(), and nextDataIdentifier() (KeynoteArchiveSurgeon+DataIdentifiers.swift:45-58) computes metadata.datas.map(\.identifier).max() ?? 0 + 1 from TSP.PackageMetadata. But in expandDrawables the minted DataInfo rows accumulate in pending.infos and are only registered into metadata after the per-item loop. Two .image items on the same slide therefore both read the same pre-loop maximum and mint the same data identifier: two metadata.datas rows sharing one id, two Data/ zip members whose filenames embed the same id, and both TSD.ImageArchive.data references resolving to whichever row wins. Cross-slide allocation is safe only by accident (each slide registers before the next runs). ImageAuthoringTests only ever exercises one image per slide, so this is currently invisible to the suite. Fix: register (or at least reserve) each identifier as it is minted; add a two-images-one-slide test.

---

🟠 Major

Surgeon layer — template assumptions leaking into the public arbitrary-base API

KeynoteTemplate.init(contentsOf:) is public, so any Keynote-written (or previously KeynoteKit-written) document is a supported base. Several code paths are only correct for the bundled blank template:

• M1. RecordCloner.remapSlideLists does not remap ownedDrawables (Sources/KeynoteKit/RecordCloner.swift:113-123). The rewrite inventory covers drawablesZOrder, builds, and buildChunks — but the surgeon itself writes slide.ownedDrawables for authored images (ImageSupply.swift:144). Cloning a slide that contains an inserted image leaves the clone's ownedDrawables pointing at the original slide's drawables while drawablesZOrder points at the clones — a cross-slide shared/dangling-ownership violation of exactly the class PLAN treats as crash-inducing. (thumbnails and note-storage attachment tables are also outside the inventory, though those are documented as template-measured scope.)
• M2. expandTextItems' clone path skips objectUuidMapEntries registration (Sources/KeynoteKit/KeynoteArchiveSurgeon+Supply.swift:51-75). cloneBodyPlaceholder appends cloned subtree records to the slide member, but nothing mints uuid-map entries for them — unlike the sibling expandDrawables path (ImageSupply.swift:85-94), which diff-scans and registers every new record. This directly violates the standing invariant (agent-notes 2026-07-31: "Every record added to a slide member needs a fresh-uuid objectUuidMapEntries row") — the documented failure mode is blank component load or NSSet-nil crash. SupplyTests.oversizedTextItemCount passes only because UUIDMapVerifier checks builds, not all minted records.
• M3. Metadata registration helpers silently no-op on a missing component (Sources/KeynoteKit/KeynoteArchiveSurgeon+Metadata.swift:80-90, 104-111, 170-175). registerUUIDEntries(_:componentStem:), registerDataObjectReference, and the per-reference lookup in registerExternalReferences return/continue when no component matches the stem. On a base whose stylesheet member isn't named DocumentStylesheet, minted style edges and thumbnail data references vanish without error — the exact "silent component load failure / crashes layout" modes the findings docs describe. These should throw, as registerUUIDEntries(_:slideIdentifier:) already does.
• M4. Partial bundle mutation when author throws midway (Sources/KeynoteKit/KeynoteArchiveSurgeon+SlideCloning.swift:63-69, KeynoteArchiveSurgeon.swift:72-115). cloneLastSlide inserts the new Index/Slide-<id>.iwa entry with an empty body, filled only by the final serialization loop; Data/ members are upserted per slide. Any throw in between (e.g. targetOutOfRange on a later slide, or the verifier itself) leaves the caller's inout bundle with a zero-byte IWA member and orphan Data/ blobs. A caller that catches and serializes writes a corrupt .key. Document the invalidation, or stage mutations and commit at the end.
• M5. Cached slide record indices assume one slide record per member (Sources/KeynoteKit/KeynoteArchiveSurgeon.swift:85 + +SlideSurgery.swift:159-173). author computes member/record indices once, then insertMintedRecords/cloneSlideNode do mid-member inserts that shift later records. If a base stores two slide records in one Index member (nothing forbids it), the second slide's cached index silently targets the wrong record. Re-locate by identifier per slide, or assert one-slide-per-member at catalog time.

Parsing/decoding — malformed input traps instead of throwing, and 32-bit platforms

The package builds for watchOS (arm64_32, 32-bit Int), and these layers document a throws-not-traps contract:

• M6. TSPArchiveStream traps on a crafted varint length (Sources/KeynoteKitProtobuf/TSPArchiveStream.swift:97). Int(headerLength) uses the trapping initializer on an input-controlled UInt64; a 10-byte varint decoding to 1 << 63 crashes the process instead of throwing TSPArchiveStreamError. Bounds-check in UInt64 space before converting. Related: ProtobufVarint.read (ProtobufVarint.swift:48-56) silently wraps 10-byte overflow (value |= ... << 63 drops bits) instead of returning nil as documented — [0xFF ×9, 0x7F] "succeeds" as UInt64.max, which is precisely the value class that feeds the trap.
• M7. Snappy decoder: attacker-controlled reserveCapacity + three 32-bit trap paths (Sources/Snappy/SnappyDecoder.swift:54; Snappy.swift:56; Varint.swift:67; SnappyDecoder.swift:82). A 6-byte input (FF FF FF FF 0F …) drives an up-front ~4 GiB reserveCapacity before any body validation — a jetsam kill / malloc trap on Apple platforms, contradicting the documented "malformed input never traps" guarantee. On arm64_32: Int(UInt32.max) in maximumBlockSize traps at first touch; Varint's Int(result) traps for lengths > Int32.max; and a width-4 extended literal length shifts into the sign bit, producing a negative length that passes the bounds guard and traps in the slice. Clamp the reserve (per-element expansion is bounded ~22×), compute lengths in UInt32/UInt64, and add a per-element output.count + length <= expectedCount fail-fast.
• M8. IWAFraming zip writer/reader trap edges (Sources/IWAFraming/ZipArchive.swift:61-99; ZipCentralDirectoryRecord.swift:87-89; ZipEndOfCentralDirectory.swift:76). The writer guards each body and the entry count but never the cumulative offsets — multiple sub-4 GiB entries summing past 4 GiB trap in UInt32(...) appends instead of throwing the documented .zip64Unsupported. On the read side, Int(buffer.readUInt32(...)) on untrusted size/offset fields traps on arm64_32 for values ≥ 2³¹. (All majors here concern inputs/scales Keynote itself never produces; nothing threatens re-emission fidelity for genuine Keynote files.)

DSL surface — result-builder and ordering gaps (user-visible at v0.1.0)

• M9. SlideItemsBuilder has no buildOptional/buildEither (Sources/KeynoteKit/SlideItemsBuilder.swift:32-52): Slide { if showCaption { TextBox("Caption") } } does not compile, while SlideBuilder and TextRunsBuilder both support control flow. buildArray is present, so for works but if doesn't — a surprising asymmetry users hit immediately.
• M10. BuildEffectsBuilder has no control-flow support at all (Sources/KeynoteKit/BuildEffectsBuilder.swift:32-44): no buildOptional, buildEither, or buildArray, so build(.in) { if emphasize { Blur() } else { Dissolve() } } fails to compile. Both fixes are the same two-liners TextRunsBuilder already has — worth doing before the API surface is tagged.
• M11. Build delivery order follows z-order, not declaration order (Sources/KeynoteKit/Deck+Lowering.swift:40-42, 88-99). authoredSlide sorts drawables by z-index and then collects builds from that ordering, but PLAN §5 (and this file's own doc comment) fix delivery order as encounter order walking the slide builder. Concretely: TextBox("A").zIndex(10).build(...) declared before TextBox("B").zIndex(0).build(...) delivers B's build first — a purely visual layering property silently reorders the animation timeline. The existing zIndexReordersDrawables test doesn't catch it because its declaration order coincides with z-order. Iterate declaration order and map targetIndex through the ordered list.

Reading/parity infrastructure (internal, but gates depend on it)

• M12. BuildExtractor never scrapes customTwist — documented Python parity is broken for twist effects (Sources/KeynoteArchiveNavigation/BuildExtractor.swift:104-126). Python's _parse_build_options scrapes the whole BuildArchive block including nested animationAttributes; customTwist exists only on KN_AnimationAttributesArchive, which the Swift options bag never reads (customBounce/customTravelDistance also have unscraped shadow copies there). BuildRecord's own docs list customTwist in the bag. The 24-fixture corpus contains zero twist effects, so the parity gate passes vacuously.
• M13. SlideOrderTests never asserts order (Tests/KeynoteArchiveNavigationTests/SlideOrderTests.swift:9-19). The only assertions are set-equality and count — a reversed or arbitrary-order result passes on all 24 fixtures, and no SlideOrderError case is ever exercised. Order is the one property the type exists to provide and is what the writer path consumes. Related: the slide-tree walk is one level deep (SlideOrder.swift:87-109) — grouped/indented slides in a user-supplied base would silently vanish from the result; recurse or throw on unexpected depth.
• M14. Apple-corpus Snappy test asserts only decoded length (Tests/SnappyTests/AppleCorpusTests.swift:19). Copy offsets don't affect output length, so the entire class of offset-decoding bugs is invisible to the module's only cross-implementation evidence. Store a SHA-256 (or plaintext) per fixture alongside uncompressedCount.

---

🟡 Minor (selected, grouped)

Correctness edges

• MotionPath.naturalWidth/Height computed as max coordinate, not extent (max - min) — leftward or offset paths yield 0 or inflated sizes (Sources/KeynoteKit/MotionPath.swift:67-71).
• Action build with motionPath == nil silently emits an empty TSD.PathSourceArchive instead of throwing (Sources/KeynoteKit/BuildRecordFactory.swift:175-177).
• Image documents "JPEG or PNG" but sizes only JPEGs; a PNG without explicit dimensions gets the silent 200×200 pixel-size fallback (Sources/KeynoteKit/Image.swift:77-88, KeynoteArchiveSurgeon+ImageSupply.swift:186-193) — pixelSize is a documented crash/misrender-sensitive field. PNG IHDR sniffing is 8 fixed-offset bytes.
• JPEGSize SOF guard admits length 2–6, reading dimension bytes from the next segment (garbage, in-bounds); require length >= 7. Legal 0xFF fill bytes also make valid JPEGs return nil (Sources/KeynoteKit/JPEGSize.swift:46-55).
• UUIDMapVerifier watermark check only runs when builds exist and only against build ids — an image-only deck regression in bumpLastObjectIdentifier passes the gate (Sources/KeynoteKit/UUIDMapVerifier.swift:69-74); bumpLastObjectIdentifier itself no-ops when the field is absent (+Metadata.swift:190-196).
• ?? 0 fallbacks on identifier-map lookups that must never miss would wire the forbidden id-0 reference instead of throwing (+SlideCloning.swift:56, +Supply.swift:112).
• Paragraph-fork record header omits the stylesheet reference from objectReferences while the payload carries it — an undeclared cross-record edge; the sibling characterStyleRecord declares it (+CharacterStyle.swift:108 vs +CharacterRuns.swift:107).
• Second .action { } call silently replaces the first, unlike accumulating .build calls (TextBox.swift:178-182, Image.swift:141-145); .direction(_:) is chainable on non-directional transitions (even .none), contra PLAN's "typed enum on directional effects only" (SlideTransition.swift:88-92).
• TSPArchiveRecord payloads/messageInfos count invariant unenforced — mismatches serialize mis-parsing streams or silently drop payload bytes via zip (TSPArchiveRecord.swift:55-58, 74; also RecordCloner.swift:55).
• EOCD backward scan accepts a false PK\x05\x06 inside a trailing comment (no comment-length consistency check), and entry paths > 65,535 UTF-8 bytes are silently corrupted via the appendUInt16 mask (ZipEndOfCentralDirectory.swift:49-59, ZipLocalFileHeader.swift:77).
• IWAChunkCodec.maximumUncompressedChunkCount is an unvalidated public var: 0 hangs encode forever; oversized values silently wrap the 24-bit length field (IWAFraming/IWAChunkCodec.swift:51, 85-99).
• Scripting: openDocument's fallback to documents[0] can bind (and later save over) an unrelated open document (KeynoteScriptingApplication.swift:86-97); single-slot AppleEventErrorMonitor.lastError mis-attributes the root cause in multi-event commands (AppleEventErrorMonitor.swift:62-83).

Test-coverage gaps worth closing before tag

• Multi-image slides (would catch C1); a Keynote-opened oversized-text-item deck (M2).
• Trigger lowering has zero automated assertions: .afterPrevious→2 / .withPrevious→3 and chunk.automatic are verified only by the human pass. Similarly untested: autoAdvance, transition/build delay, custom motion-path points, run-level .italic()/.font(), and SlideBuilder's own control-flow paths.
• No known-answer vectors: SHA-1 ("abc" → a9993e36…; the implementation was hand-verified correct against FIPS 180-4, but the only test compares it to itself), CRC-32 ("123456789" → 0xCBF43926), copy1 offsets > 255 in Snappy.
• KeyBundle.upsertEntry (four ordering branches, on the production surgery path) has no direct test; malformed-zip error paths (truncatedArchive variants, zip64Unsupported, multi-disk) largely untested.
• TSPArchiveStream.serialize is never run against real Keynote headers at its own layer (only synthetic round-trips); patch-record resolution (type == 0 / shouldMerge) has zero coverage.
• ArchiveGraphComparer doesn't compare messageInfos.dataReferences — a dropped thumbnail/image data reference would pass the golden differential (Tests/KeynoteKitTests/ArchiveGraphComparer.swift:53-57).

CI / config hygiene

• Release-tag pushes get the reduced matrix: the scope check matches branch refs only, so refs/tags/v0.1.0 sets FULL=false — the tag build that certifies the release skips the platform legs (.github/workflows/KeynoteKit.yml:50-61). Add elif [[ "$REF" == refs/tags/* ]]; then FULL=true.
• Fork PRs will always fail claude-code-review (secrets unavailable on forks); guard with a same-repo if: (claude-code-review.yml:4-5, 38). No pull_request_target/script-injection issues found anywhere.
• Three workflows lack permissions: blocks (KeynoteKit.yml, check-unsafe-flags.yml, swift-source-compat.yml); third-party actions are tag-pinned rather than SHA-pinned (jdx/mise-action, jlumbroso/free-disk-space are the real supply-chain surface).
• .swiftlint.yml lists pattern_matching_keywords in both opt_in_rules (line 60) and disabled_rules (line 147) — disabled wins; delete one deliberately.
• Scripts/lint.sh's ScriptingBridge-import guard doesn't scan AcceptanceDeckCatalog, AcceptanceDecks, or KeynoteArchiveNavigation (lines 83-87); unquoted $PACKAGE_DIR word-splits on paths with spaces.
• cleanup-caches isn't paginated (leftover caches past 30); Deck.write hardcodes SystemRandomNumberGenerator, so a deck failing the human pass can't be regenerated for bisection — thread the existing generator seam through the public API.

Performance (none blocking)

• Surgeon: SlideCatalog rebuilt inside nearly every helper, withPackageMetadata decode/encode round-trips per call (~6× per image slide), and author re-encodes every member whenever any id was minted — quadratic on large bases; a one-time identifier→location index fixes most of it.
• Snappy: fixed 16,384-entry hash table zeroed per encode call regardless of input size (the corpus's smallest block is 18 bytes); [UInt8]-only API forces an Array(payload[start..<end]) copy per 64 KiB chunk in IWAChunkCodec; KeyArchiveIndex.record(withIdentifier:) is a linear scan called repeatedly per slide.

---

✅ Strengths

• The hardest-won invariants are encoded, not just documented: has*-guards throughout RecordCloner (with the materialized-empty-reference rationale inline), UUIDMapVerifier running before every write with error messages explaining why Keynote would crash, and tests proving each invariant can actually fail.
• The golden-differential design is excellent: strip Python's surgery from a real golden, re-author with Swift, demand graph equality with only the two genuinely random fields normalized — deterministic id allocation makes even minted identifiers comparable.
• Research traceability is unusually good: BuildRecordFactory and the transition catalogs cite the exact Python lines, findings docs, and fixture measurements; all 44 scripting transition entries match deckkit.py byte-for-byte including the three deceptive name→archive traps.
• Layer boundaries follow the plan: fine-grained targets, KeynoteKit never links ScriptingBridge (enforced by lint grep + a Linux CI leg), navigation stays package-scoped per the "reading is not v0.1.0 API" directive, Snappy knows nothing about framing.
• Malformed-input handling on 64-bit is genuinely throw-based and fuzz-tested in Snappy/IWAFraming (2,000 random blocks, every truncation prefix, every single-byte corruption); the fixture corpus is pinned with an integrity test that avoids the known Bundle.module vacuous-pass pitfall.
• Live-test gating is exemplary: the only Keynote-launching suite is env-var opt-in, .serialized, with unique temp dirs and an explanation of why the "safe" test can't trigger TCC. CI is green, two-tiered sensibly, and free of script-injection patterns.

Suggested pre-tag punch list

1. Fix C1 (per-slide data-identifier collision) + add a two-images-one-slide test.
2. M9/M10 (result-builder control flow) and M11 (declaration-order builds) — public API surface and documented semantics, cheapest to fix before the tag locks them in.
3. M2/M3 (uuid-map registration + silently no-oping registration helpers) — these violate the repo's own crash invariants on supported inputs.
4. M6/M7/M8 trap paths — or, if deferred, narrow the documented "never traps" claims and file follow-ups.
5. Tag-ref matrix fix in CI so v0.1.0 gets the full build.
6. The one-line known-answer tests (SHA-1, CRC-32) and the trigger-lowering assertion — tiny effort, large regression surface.

Review conducted module-by-module (six parallel deep reads) against .claude/PLAN.md, .claude/agent-notes.md, and the research findings; all findings verified against the code as of this branch's head.