feat: close the remaining D01-D03 acceptance gaps (SVG export, resolved-only publication) - #409

Review

Diff of 5666c64 vs master (3194dec), one commit: engine-side deterministic SVG export plus a resolved-only publication gate. Approving.

What I verified by reading:

• The publication gate sits inside DocumentStore::commit after the replay and lifecycle-transition checks and before any blob is stored or the host CAS runs, so a refusal leaves the document head, the ledger and the model untouched, and it does not consume the operation id; the replay path correctly bypasses the gate because it writes nothing. The e2e test pins all of this, including zero versions after the refusal and a later resolved publish from the same head.
• export-drawing resolves one exact published version through history verification from its own immutable archived source, never opens the live model, and reserves the destination with create_new only after render, so an existing file is never opened or removed; a failed write removes the partial file. The reported source pin comes from the archived snapshot.
• The SVG writer is pure and closed: every textual field passes the escape table, coordinates are validated (non-finite, paper cap) and negative-zero normalized, all numbers render at fixed precision, and the byte cap holds. The checked paper bounds agree with the existing MAX_PAPER_MM used by projection. Arrowheads and the seven-segment digit glyphs are contributor-original strokes, so the no-font no-resource claim holds; the glyph count test (14 strokes for 10.5) checks it.
• The e2e numeric expectations are consistent with the model inputs: a 3000 mm axis at 1:100 gives 30.0000 paper mm, the centred 50 mm section puts the anchored face at paper y -0.25, and the +10 mm offset line lands at 9.7500.
• Never-crash: no new unwrap, expect, panic or unchecked indexing outside tests; refusals are typed CommandError or PlateError throughout.

Optional notes, no change required: export of a version whose revision was compacted out of the document history answers NOT_FOUND, which is honest but worth remembering when document compaction arrives; the gate resolves the projection that derived::encode resolves again immediately after, a harmless duplication; and the byte cap is checked after the string is built, which cannot grow large through the bounded first-slice CLI path.

Why did you chose svg format?

D03 SVG export of one exact published version

Verdict: approved. No changes requested.

Reviewed the PR diff against master at a737573 (7 files, +838): the new engine::drawing::svg exporter, the resolved-only publication gate in document/store.rs, the export-drawing command wiring and help, the shared test module registration, and the two E2E suites.

What I checked:

• Determinism and self-containment. Stroke-only output at fixed four-decimal precision, minus-zero normalization, nonfinite and over-cap coordinates refused, 64 KiB byte cap checked on the final bytes; every textual metadata field is escaped or numeric, and the hostile-substring assertions cover script/image/text/href/url/import. The seven-segment glyph set is contributor-original vector geometry, no fonts or resources.
• Resolved-only publication. The gate runs only for State::Published, before any resource is stored or the head moves, and drafts stay allowed to be explicitly unresolved — the E2E proves the refused publication neither consumes the operation nor moves the head, and a retry from the same head publishes.
• Export correctness. Version-id uniqueness is already enforced at publish, so the single find over history is unambiguous; the projection is recomputed from the immutable archived snapshot, so the live model is never read — pinned by the E2E where a later edit marks the drawing stale while the exported bytes stay identical.
• Destination safety. create_new never opens or removes an existing file; a failed write removes the partial file and returns BackendUnavailable; refusals are asserted to leave the store and the destination untouched.
• Conventions. Headless-first command plus engine API, doc comments on public items, no new dependency, help text updated, tests via the real binary.

Two observations, no change needed: an open failure on the destination (for example permission denied rather than already-exists) maps to INVALID_ARGUMENT with a message covering both cases — a split into BackendUnavailable would be slightly more precise but is defensible as is; and the publish gate resolves the projection once more than derived::encode does internally, which is cheap and keeps the refusal before any resource is staged.

Tests, fmt and clippy were not run in this review; CI covers them.

D03 SVG export of one exact published version (re-review at 5d90f8f)

Verdict: approved. No changes requested.

Re-reviewed the head since the previous approval of 7764135. The only delta is two rule_id: None fields added to the PlateError literals in the export-drawing handler (the not-found refusal and the write-failure path) — required by the struct and consistent with every other PlateError construction in the codebase. Nothing else changed; all findings and observations from the earlier review still apply as stated there, including the remaining body-only notes on the coarse INVALID_ARGUMENT mapping for destination-open failures and the doubled resolve on the publish gate.

Changes requested: CI test jobs are failing for the current head 5d90f8f:

• test (ubuntu-latest, shard 5/8) — FAILURE.
• test (windows-latest, shard 5/8) — FAILURE.
• test (macos-latest, shard 5/8) — FAILURE.

Please investigate and resolve the failing checks, then rerun CI and obtain a green result before requesting review again. This review records the reported CI failures; their root cause has not been diagnosed here.

Review of PR #409 (D01-D03 acceptance gaps: SVG export, resolved-only publication)

Reviewed 9dbf1a6 against base master 112be7b (7 files, +840/−3, one commit).

Verdict: approved. No blocking findings; no inline change requests.

What I checked and found sound:

• The SVG writer is genuinely self-contained and pure: 7-segment contributor-original stroke glyphs for digits (an unknown character advances a blank gap rather than a fabricated glyph), arrowheads as open-V strokes, no font/image/script/URL anywhere, full XML escaping of all five markup-significant characters in every metadata field, fixed four-decimal coordinate formatting, and a hard 64 KiB byte cap checked on the assembled document. The deterministic-bytes, self-containment and escaping tests pin all three properties, including the hostile-token sweep.
• Coordinate admission is complete before rendering: nonfinite refusal, paper-cap refusal, and negative-zero normalization so bytes stay canonical; a collapsed dimension line refuses rather than producing a NaN direction.
• export-drawing reads only immutable history: it finds the exact published version, reopens its own archived source (the live model is never read), resolves from that snapshot, and only then touches the filesystem with create_new semantics — an existing destination is never opened or removed, and a failed write removes only the file this command just created. Every refusal before the open leaves the destination, document head and model head untouched.
• The publication gate is correctly placed inside DocumentStore::commit before any asset is stored or a head moves, applies only to State::Published (drafts may stay unresolved), and re-projects from the draft own verified snapshot. The e2e test proves an unresolved publish keeps the head and zero versions while a later resolved refresh publishes.
• The snapshot-bound determinism e2e (identical bytes across fresh CLI processes and after a live model edit, with reported freshness stale) directly evidences the archived-source claim.
• Help text documents the command and its guarantees; docs match the code.

Minor, non-blocking (no change requested):

• The structural XML check (writer structure plus hostile-token absence) is honest for a fixed-shape writer; if richer SVG ever grows, that is where a real parser would become necessary.
• export looks up the version by scanning full document history; fine at bounded document sizes, a version index would be the follow-up if versions ever grow.