oficina / sheets Public

Make the write version contract explicit: stamps, and no idle Open stamp

Native detects missed edits from the version (review of B12, discussion
#35). Two server behaviours made that impossible.

- store.Open took a crash-recovery stamp on every open, even with nothing
  stale. Handles are evicted at refcount zero, so sequential requests each
  re-open the tab and consecutive writes jumped by two. A client cannot
  tell that apart from a write it missed. The stamp is now taken only when
  something is stale.
- A response did not say how many versions its write consumed. Report and
  gridResponse now carry Stamps: 1 for /cells and for the table and history
  writes (publishReport), len(writes) for /cells/batch, and absent for
  format, size and snapshot responses, which consume none.

The client rule is: refetch iff resync || version > held + max(stamps, 1).
The one of slack covers stampless responses, which report the NEXT stamp
(Handle.Version). That semantics is unchanged here, so grid.js, which only
drops older payloads and ignores the new field, behaves as before.

TestNativeVersionContractAgainstRealResponses replays real /cells,
/cells/batch (with a derived formula cell), /format, /size and snapshot
responses through a Go mirror of the native rule:
- Nothing missed: no response refetches. Against main's Open, five steps
  refetch.
- One write missed: the next cell, batch, format or size response
  refetches. Without the stamps field, batches of 3 and 4 refetch with
  nothing missed. Under the earlier branch's count-the-cells rule, a
  20-cell format hides the missed write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 3 weeks ago 579c553d7382f398aa9b0575177a9537e24823a1 parent bf59232

8 files changed, +157 −5

internal/sheets/grid.go +10 −1
internal/sheets/gridstyle.go +1 −0
internal/sheets/handlers_cell_batch.go +1 −1
internal/sheets/handlers_grid.go +1 −0
internal/sheets/store/handle_open.go +11 −2
internal/sheets/store/handle_report.go +7 −1
internal/sheets/store/handle_write_batch.go +1 −0
internal/sheets/version_contract_test.go +125 −0

diff --git a/internal/sheets/grid.go b/internal/sheets/grid.go

index b32d0b1..14f4c18 100644

--- a/internal/sheets/grid.go

+++ b/internal/sheets/grid.go

@@ -63,7 +63,16 @@ type gridResponse struct {

Cols int `json:"cols,omitempty"`

Cells []gridCell `json:"cells"`

Resync bool `json:"resync,omitempty"`

- Client string `json:"client,omitempty"`

+ // Stamps is how many versions the write behind this response

+ // consumed (store.Report.Stamps): 1 for a cell, N for an N-cell

+ // batch, absent for format, size and snapshot responses, which

+ // consume none. A client holding version H missed an edit iff

+ // Version > H + max(Stamps, 1); the one of slack is because those

+ // stampless responses report the NEXT stamp (Handle.Version), not

+ // the last one issued. Native's SheetModel.accept is the reader;

+ // grid.js only drops older payloads and ignores it.

+ Stamps int `json:"stamps,omitempty"`

+ Client string `json:"client,omitempty"`

// Formats is every distinct style the cells above refer to, keyed

// by the id each cell's F names. Absent when nothing in the batch

// is formatted.

diff --git a/internal/sheets/gridstyle.go b/internal/sheets/gridstyle.go

index 893514b..a36eb78 100644

--- a/internal/sheets/gridstyle.go

+++ b/internal/sheets/gridstyle.go

@@ -155,5 +155,6 @@ func (a *App) publishReport(s SheetSummary, h *store.Handle, rep store.Report) {

Cells: cells,

Formats: styles,

Resync: rep.Capped,

+ Stamps: rep.Stamps,

})

}

diff --git a/internal/sheets/handlers_cell_batch.go b/internal/sheets/handlers_cell_batch.go

index 1cb390a..e794bf6 100644

--- a/internal/sheets/handlers_cell_batch.go

+++ b/internal/sheets/handlers_cell_batch.go

@@ -58,7 +58,7 @@ func (a *App) handleCellBatchPost(w http.ResponseWriter, r *http.Request) {

return

}

cells, styles := fromReport(rep, newPosIndex(h), h)

- resp := gridResponse{Version: rep.Version, Cells: cells, Formats: styles, Client: in.Client}

+ resp := gridResponse{Version: rep.Version, Cells: cells, Formats: styles, Stamps: rep.Stamps, Client: in.Client}

a.touchSheet(sheetIDFrom(r))

a.publish(tabIDFrom(r), resp)

writeJSON(w, http.StatusOK, resp)

diff --git a/internal/sheets/handlers_grid.go b/internal/sheets/handlers_grid.go

index 410c930..ad1f098 100644

--- a/internal/sheets/handlers_grid.go

+++ b/internal/sheets/handlers_grid.go

@@ -177,6 +177,7 @@ func (a *App) handleCellPost(w http.ResponseWriter, r *http.Request) {

Cells: cells,

Formats: styles,

Resync: rep.Capped,

+ Stamps: rep.Stamps,

Client: in.Client,

}

a.touchSheet(sheetIDFrom(r))

diff --git a/internal/sheets/store/handle_open.go b/internal/sheets/store/handle_open.go

index 8f3498b..83d3236 100644

--- a/internal/sheets/store/handle_open.go

+++ b/internal/sheets/store/handle_open.go

@@ -154,8 +154,17 @@ func buildHandle(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error

// A single stamp taken from h.nextVersion is, by construction, greater

// than every InputVersion loaded above, so it can never regress

// anything it touches.

- recoveryVersion := h.nextVersion

- h.nextVersion++

+ //

+ // Only when something is stale. Taken unconditionally, every Open

+ // consumed a stamp nothing was written under, and because a handle

+ // is evicted at refcount zero (Release), sequential requests each

+ // re-open it: consecutive writes then jumped by two, and a client

+ // could not tell that from a write it missed (discussion #35).

+ var recoveryVersion uint64

+ if len(stale) > 0 {

+ recoveryVersion = h.nextVersion

+ h.nextVersion++

+ }

// recoverStale (handle_batch.go) is what used to be a plain loop

// recomputing and persisting once per stale key — one transaction (one

// WAL fsync) per key. That is fine at the 5,001-cell scale the

diff --git a/internal/sheets/store/handle_report.go b/internal/sheets/store/handle_report.go

index 835298f..ff7a414 100644

--- a/internal/sheets/store/handle_report.go

+++ b/internal/sheets/store/handle_report.go

@@ -47,6 +47,10 @@ type Report struct {

Version uint64

Cells []ChangedCell

Capped bool

+ // Stamps is how many versions this write consumed: one per typed

+ // cell, ending at Version. A client that last saw version H and

+ // receives this report missed nothing iff Version <= H + Stamps.

+ Stamps int

}

// Version is the version a snapshot taken right now would carry.

@@ -123,7 +127,9 @@ func (h *Handle) SetReport(rowID, colID uint64, raw string, actor uint64) (Repor

if err != nil {

return Report{}, err

}

- return h.report(key, rowID, colID, v, touched, capped), nil

+ rep := h.report(key, rowID, colID, v, touched, capped)

+ rep.Stamps = 1

+ return rep, nil

}

// report turns a recomputed touched set into the cells a client paints.

diff --git a/internal/sheets/store/handle_write_batch.go b/internal/sheets/store/handle_write_batch.go

index f3a2378..74f4340 100644

--- a/internal/sheets/store/handle_write_batch.go

+++ b/internal/sheets/store/handle_write_batch.go

@@ -92,5 +92,6 @@ func (h *Handle) SetBatch(writes []CellWrite, actor uint64) (Report, error) {

h.rowPos, h.colPos = staged.rowPos, staged.colPos

h.rowByPos, h.colByPos = staged.rowByPos, staged.colByPos

h.nextVersion = staged.nextVersion

+ result.Stamps = len(writes) // one SetReport, so one stamp, per write

return result, nil

}

diff --git a/internal/sheets/version_contract_test.go b/internal/sheets/version_contract_test.go

new file mode 100644

index 0000000..f88e83b

--- /dev/null

+++ b/internal/sheets/version_contract_test.go

@@ -0,0 +1,125 @@

+package sheets

+import (

+ "fmt"

+ "net/http"

+ "net/http/httptest"

+ "testing"

+// versionFollower mirrors native SheetModel.accept (native/apple/Sources/

+// SheetsKit/SheetModel.swift), the one client that detects missed edits

+// from the version: drop anything older than what it holds, refetch on

+// resync or when the version moved further than the response accounts

+// for, otherwise apply. Change the two together.

+type versionFollower struct {

+ have uint64

+ refetches int

+func (f *versionFollower) missed(g gridResponse) bool {

+ allowed := uint64(max(g.Stamps, 1))

+ return g.Resync || g.Version > f.have+allowed

+// accept returns whether the response made the follower refetch.

+func (f *versionFollower) accept(t *testing.T, g gridResponse, refetch func() gridResponse) bool {

+ t.Helper()

+ if g.Version < f.have {

+ return false

+ }

+ if f.missed(g) {

+ f.refetches++

+ f.have = refetch().Version

+ return true

+ }

+ f.have = g.Version

+ return false

+// B12 and its review (#35): replay REAL server responses — cell writes,

+// batches (with a derived formula cell), a format, a size, snapshots —

+// through the native rule. With nothing missed, nothing may refetch: an

+// Open used to take a stamp every time, and the handle is evicted between

+// requests, so sequential writes jumped by two and every other one

+// refetched. With one write missed, the next response of every kind must

+// refetch, including a format, whose N cells consume no stamp.

+func TestNativeVersionContractAgainstRealResponses(t *testing.T) {

+ app, cookies := signIn(t)

+ s, cells := newSheet(t, app, cookies, "Budget")

+ snapshot := func() gridResponse {

+ t.Helper()

+ rec := do(t, app, as(httptest.NewRequest(http.MethodGet, cells+"?rows=20&cols=10", nil), cookies))

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET cells = %d: %s", rec.Code, rec.Body.String())

+ }

+ return decodeGrid(t, rec)

+ }

+ ok := func(rec *httptest.ResponseRecorder) gridResponse {

+ t.Helper()

+ if rec.Code != http.StatusOK {

+ t.Fatalf("write = %d: %s", rec.Code, rec.Body.String())

+ }

+ return decodeGrid(t, rec)

+ }

+ cell := func(r, c int, raw string) gridResponse {

+ return ok(postCell(t, app, cookies, cells, fmt.Sprintf(`{"r":%d,"c":%d,"raw":%q}`, r, c, raw)))

+ }

+ batch := func(body string) gridResponse { return ok(postCell(t, app, cookies, cells+"/batch", body)) }

+ format := func(body string) gridResponse { return ok(postFormat(t, app, cookies, s, body)) }

+ size := func(body string) gridResponse { return ok(postSize(t, app, cookies, s, body)) }

+ t.Run("nothing missed, nothing refetched", func(t *testing.T) {

+ f := &versionFollower{have: snapshot().Version}

+ steps := []struct {

+ name string

+ resp func() gridResponse

+ }{

+ {"cell", func() gridResponse { return cell(0, 0, "1") }},

+ {"cell", func() gridResponse { return cell(1, 0, "2") }},

+ {"cell", func() gridResponse { return cell(2, 0, "3") }},

+ {"batch of 3 with a derived formula", func() gridResponse {

+ return batch(`{"cells":[{"r":3,"c":0,"raw":"=SUM(A1:A3)"},{"r":0,"c":1,"raw":"b"},{"r":0,"c":0,"raw":"10"}]}`)

+ }},

+ {"format over 6 cells", func() gridResponse { return format(`{"r1":0,"c1":0,"r2":1,"c2":2,"bold":true}`) }},

+ {"cell after format", func() gridResponse { return cell(1, 1, "x") }},

+ {"size", func() gridResponse { return size(`{"col":2,"size":220}`) }},

+ {"batch of 2", func() gridResponse { return batch(`{"cells":[{"r":5,"c":0,"raw":"5"},{"r":6,"c":0,"raw":"6"}]}`) }},

+ {"clear", func() gridResponse { return cell(5, 0, "") }},

+ {"snapshot", snapshot},

+ {"cell after snapshot", func() gridResponse { return cell(7, 0, "7") }},

+ {"batch of 4", func() gridResponse {

+ return batch(`{"cells":[{"r":8,"c":0,"raw":"8"},{"r":8,"c":1,"raw":"8"},{"r":8,"c":2,"raw":"8"},{"r":8,"c":3,"raw":"8"}]}`)

+ }},

+ }

+ for _, step := range steps {

+ before := f.have

+ g := step.resp()

+ if f.accept(t, g, snapshot) {

+ t.Errorf("%s: refetched with nothing missed (held %d, got version %d, stamps %d)",

+ step.name, before, g.Version, g.Stamps)

+ }

+ })

+ t.Run("one write missed, the next response refetches", func(t *testing.T) {

+ for _, next := range []struct {

+ name string

+ resp func() gridResponse

+ }{

+ {"cell", func() gridResponse { return cell(10, 0, "n") }},

+ {"batch", func() gridResponse { return batch(`{"cells":[{"r":10,"c":1,"raw":"n"},{"r":10,"c":2,"raw":"n"}]}`) }},

+ {"format over 20 cells", func() gridResponse { return format(`{"r1":11,"c1":0,"r2":14,"c2":4,"italic":true}`) }},

+ {"size", func() gridResponse { return size(`{"col":3,"size":140}`) }},

+ } {

+ // Hold the version of a write this follower saw, then miss one.

+ f := &versionFollower{have: cell(9, 0, "seen").Version}

+ cell(9, 1, "missed")

+ g := next.resp()

+ if !f.missed(g) {

+ t.Errorf("%s after a missed write was applied, not refetched (held %d, got version %d, stamps %d)",

+ next.name, f.have, g.Version, g.Stamps)

+ }

+ })