oficina / sheets Public
Atomic cell batch implementation contract
Status: revision 3; Claude Fable 5.1 (claude-fable-5-1) approved revision 2 design at 1aca44c with these wording amendments, and requested code changes for committed browser queue proof and obsolete comments. Focused code re-review remains pending. Implements the overnight suite readiness request, under the existing Sheets design and next-three batch-route item.
- POST /api/sheets/{sheetID}/tabs/{tabID}/cells/batch accepts {cells: [{r,c,raw,fromR?,fromC?}], client?}. Require an editor using openTab, preserving sheet/tab pairing and 404 isolation.
- Limit to 500 distinct destinations and 1 MiB body. Validate every coordinate, copy origin, raw length, translated raw length, and formula before any committed writes. Duplicate destinations, empty batches, and trailing JSON are invalid. Reuse existing fixed response strings; add no UI copy.
- Hold the live handle mutex while a transaction-scoped independent handle is built from the DB. It shares no mutable graph or maps with the live handle. Set its next version at least to the live handle version. Ensure rows/columns and perform existing SetReport operations through that transaction handle. Existing nested transactions become savepoints; only the outer transaction commits. Reject capped recalculation and over-limit resulting cell count atomically.
- Preserve existing per-cell input history and recalculation semantics. Merge reports by coordinate, keeping final values and marking directly edited cells as direct. Return only after outer commit; swap graph/maps/version into the live handle after commit. On failure discard the transaction handle; no partial history, axes, data, or live publication.
- One response and SSE publication contain all changed cells and the final version. Keep the current 500-cell paste cap. The existing cell queue submits one batch instead of one request per cell. Larger clear selections retain serial batches of 500; a failed batch must stop the queue. Existing post currently absorbs failures; make it return explicit success for queue control while preserving existing callers.
- No format, row/column semantics, tab lifecycle, formula grammar, or permission changes. Clipboard provenance is a separate next slice; this changes only transmission, no behaviour change to clipboard.js; its batching comments are updated.
- Cost: a second tab graph is built per batch. Measure a 500-cell batch and document this bounded first implementation. Avoid adding a graph clone API or redesigning calculation before evidence demands it.
- Tests: dependent formulas and clears, overlapping formulas, encryption/history, invalid late formula, injected persistence failure after earlier writes, axis rollback, shared live handle unchanged on failure, viewer/stranger/cross-sheet isolation, request bounds, one publication, client queue one-request and stop-on-failure behavior in tools/drive/batch-checks.mjs, called by clipboard.mjs and canary-batch.mjs. These use real editor clipboard actions, intercept a forced first-chunk failure, and await the queue completion barrier rather than assuming a delay means success. Browser checks are a deployed-canary gate, not part of make test. Run make test and exact-head CI. Build with make release, deploy isolated canary, run browser checks, obtain Fable branch review, then coordinate squash merge and production deployment with root.
Revision 2 review answers
- The independent handle is built with
buildHandle(tx, ...); itsdbis exactlytx. Recovery, count, axis allocation, input/event writes, and nested transactions all use that handle. No Open/registry path runs inside the transaction. TestCellBatchRecoversStaleCacheInsideTransaction deliberately seeds a stale cell after opening the live handle, then requires recovery and batch completion within three seconds. Building from the outer DB deadlocks that test. - Measured with CELLBATCHBENCH_N and the committed TestCellBatchLargeTabTiming fixture: 500 changed cells on a 50,000-cell tab took 660ms; on a 200,000-cell tab took 2.326s on this development machine. These are transaction duration including rebuild, and therefore instance-wide writer blocking durations. The fixture has one feeder and many dependent formulas; paste changes leaf cells. Production measurements remain required. This initial bounded implementation accepts this cost to preserve atomic rollback without inventing graph cloning. The trigger for moving the rebuild out of the writer transaction is measured unacceptable production/pilot latency; it must preserve recovery and handle consistency. Each SetReport uses nested savepoints, which GORM retains until outer commit (roughly 1,000 per 500-cell batch).
- The first non-error operation after outer Transaction returns is swapping graph and mutable maps into the live handle under its mutex. Report assembly occurs inside the transaction before commit; response formatting happens after the live state is updated. There is no fallible operation between commit and the swap. Failure to encode or deliver the HTTP response does not invalidate the now-current live handle; reconnect/snapshot recovers delivery.
- A merged report may contain at most calc.MaxDirtyPerPass (5,000) distinct cells. Exceeding either an individual recalc cap or the merged report cap rejects and rolls back the whole batch. This deliberately differs from the single-cell path: the batch success contract guarantees complete final values without depending on the pre-existing resync behavior. No new user copy is introduced; the existing write failure string is reused. Supporting larger dependency fanouts is a later slice with dependable resync and explicit recovery semantics. TestCellBatchRejectsMergedReportPastCalculationCap proves the aggregate limit is applied even when each individual edit is within its cap.
- Both the implementation and failure-injection tests exist. The live handle is preserved on rollback and updated immediately on success. Local make test and remote CI were green at initial implementation 50914fb; revision 2 requires fresh checks and review.
Revision 3 records the aggregate request bound explicitly: the 1 MiB body limit may reject a set of individually valid 8 KiB cells; callers must stay within both limits. Batch copy origins must include both coordinates, stricter than the legacy single-cell route. For pilot measurement, a p95 batch latency above 1 second at 50,000 cells or 3 seconds at 200,000 cells triggers the next transaction-duration design review; these are investigation thresholds, not a readiness claim.