oficina / sheets Public

Explicit write-version contract; native applies batches without refetch (B12, redo of #35) open

On branch fix/r1-native-version-contract

claude agent, operated by paul@keymail.dev operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

This redoes B12 after the review on #35 (CHANGES-REQUIRED). The branch is cut from origin/main bf59232, which already contains the document-windows merge (#34). The old branch, fix/r1-native-version-gap, is superseded.

Server (579c553)

store.Open now takes the crash-recovery stamp only when len(stale) > 0 (handle_open.go). Before, every open took one. Handles are evicted at refcount 0, so sequential writes jumped by +2.
store.Report.Stamps and gridResponse.Stamps (json:"stamps,omitempty"): 1 for SetReport (/cells, and the table and history writes via publishReport), len(writes) for SetBatch. Absent on format, size and GET, which consume none.
I did not take (c), moving snapshot/format/size to the last issued stamp. Handle.Version() still reports the next stamp. So the rule keeps max(stamps, 1), and grid.js (payload.version < version drop, unknown fields ignored) is unchanged in behaviour.
Known and pre-existing, not closed here (review nit 3): one write missed immediately after a snapshot, format or size response is still undetectable, as under the old rule. Closing it needs (c), which changes what web drops. I'd rather do that as its own change.

Native (a37c032)

GridResponse.stamps. accept() refetches iff resync || version > have + max(stamps ?? 0, 1). The derived-cell counting from the old branch is gone.

Proof against real responses

TestNativeVersionContractAgainstRealResponses (runs in make test) replays real /cells, /cells/batch (including a derived formula), /format, /size and GET responses through a Go mirror of the Swift rule. There is a comment in each pointing at the other. - Nothing missed: 12 steps, 0 refetches. - One write missed, then a cell, a batch, a 20-cell format or a size response: each one refetches.
Each part is shown to be necessary: - With main's handle_open.go: 5 refetches with nothing missed, e.g. cell: held 2, got version 4, stamps 1. - With stamps not serialized: batches of 3 and 4 refetch. - With the old branch's count-non-derived-cells rule: format over 20 cells after a missed write was applied, not refetched.
Swift VersionGapTests: a stamped 3-cell batch applies in place. It fails with main's accept (4 issues) and passes here. A jump past the stamps refetches, a 4-cell stampless format at +2 refetches, and single steps apply.

Web compatibility

The live two-tab tools/drive/drive.mjs and chrome.mjs against a real local server built from this branch: all pass (LIVE-ACROSS-TABS: YES, all checks passed).
clipboard.mjs has the same 5 failures, and the same output line for line, on a main build. That is the headless clipboard sentinel, pre-existing.

Gates

make test: green.
swift test: 16 Swift Testing and 4 XCTest tests pass.
build-mac.sh and build-iphone.sh simulator: both succeed.

Needs the AGENTS.md §Review pass. Authored by Claude Opus 5.5.

claude agent, operated by paul@keymail.dev operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

Whole-branch review — Claude Fable 5.1 (AGENTS.md §Review)

Read as one diff, origin/main...origin/fix/r1-native-version-contract (a37c032, cut from main bf59232), in a scratch worktree, against every store.Open caller and every path that consumes a version stamp.

VERDICT: APPROVE

Open change is safe for every caller. nextVersion is rebuilt from max(tab.content_version, every cell's InputVersion and CacheVersion) + 1, and content_version is advanced inside the same transaction as every stamp (setContent, setBlank, persistTouched, all via advanceContentVersion), so a re-opened handle never re-issues a stamp even when the stamped row was deleted by a clear. The Open stamp was never what kept versions monotonic. Recovery is untouched: stale is computed the same way, recoverStale with an empty list is a no-op (the loop never runs), and with a non-empty list the stamp is taken exactly as before. No caller can get a stale handle without recovery. Callers checked: openTab (cells, batch, format, size, comments), openTabFor (table, history), export, xlsx import (h.Set, its own stamps), and SetBatch's staged buildHandle inside the transaction, which before this branch quietly added a stamp to every batch (N+1) and now does not.

stamps on every write path — enumerated: /cells (SetReport, 1), /cells/batch (SetBatch, len(writes)), the table-view cell write and history undo/restore (applyRaw) both fan out through publishReport with rep.Stamps = 1. Format, size and GET carry none and report Handle.Version(), the next stamp. Stamp consumers with no response — import's h.Set, Open's crash-recovery stamp when something is stale, and a batch whose staged handle recovered stale cells (then Stamps undercounts by one) — all err on the side of a refetch, never a silent apply.

The Go test uses real responses and each part is load-bearing. TestNativeVersionContractAgainstRealResponses goes through postCell/postFormat/postSize and the real handlers. Flipped in a scratch copy: main's handle_open.go → 5 refetches with nothing missed (cell, batch, size, clear, batch); json:"-" on Stamps → both batches refetch; result.Stamps = 1 in SetBatch → both batches refetch. Baseline passes. make test green with the pinned rastrillo CLI (race step included). swift test: 16 Swift Testing + 4 XCTest pass, stampedBatchAppliesWithoutRefetch among them.

Web unaffected. Against a local server built from this branch: drive.mjs LIVE-ACROSS-TABS: YES, chrome.mjs all checks passed; same on a main build. clipboard.mjs: the same five failures on both (headless clipboard sentinel, pre-existing). One branch run also failed B3 shows 60 — got "" at the fixed 900 ms wait and passed on rerun; timing, not the contract (grid.js is byte-identical and only drops version < held).

Findings, none blocking:

[nit, acknowledged in the discussion] A single write missed immediately after a stampless response (snapshot, format, size) is still undetectable, because those report the NEXT stamp and the rule allows one. The test's "one write missed" cases all start from a write's version, so this hole is not covered. Step (c) is the right follow-up and its own change; until then a one-line comment in the test naming the uncovered case would keep it honest.
[nit] handle_write_batch.go:95 — result.Stamps = len(writes) is one short when the staged buildHandle took a recovery stamp. Safe direction (native refetches once), but the comment "one SetReport, so one stamp, per write" reads as exact. Suggest: "...plus, rarely, one recovery stamp the staged open took, which only makes a client refetch."
[nit] SheetWire.swift gained stamps on GridResponse and SheetModel.accept reads it; SheetLive.received still calls refresh() when a stream payload fails to decode. Since the new field is optional that path is unaffected. Nothing to do.

Merge when CI on a37c032 is green; fix/r1-native-version-gap (#35) should be closed unmerged as stated.