amadan
Document recalc marker granularity change in batched recovery
recoverStale flushes persistTouched once per batch of origins now, not once per origin, so one AppendRecalc marker's DerivedCount can cover up to openRecoveryBatchSize origins during Open's crash recovery. Checked whether TriggerID becomes misleading as a result: it does not, since persistTouched has always passed triggerID=0 unconditionally on every call site (setContent's single-origin path included) - no marker ever claimed to name the one trigger behind its count, so batching doesn't falsify anything. Comment-only change documenting this at persistTouched and recoverStale; no test or production-logic edits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS
2 files changed,
+22
−0
internal/sheets/store/handle_batch.go+8 −0internal/sheets/store/handle_set.go+14 −0
CI passed — run details
Intent
diff --git a/internal/sheets/store/handle_batch.go b/internal/sheets/store/handle_batch.go| index 29f1b84..2384759 100644 |
| --- a/internal/sheets/store/handle_batch.go |
| +++ b/internal/sheets/store/handle_batch.go |
| @@ -125,6 +125,14 @@ const openRecoveryBatchSize = 500 |
| // (or an ordinary Set that touches it directly) to retry. Batching only |
| // changes when a successfully-recomputed origin's result is committed, |
| // never whether it is recomputed at all. |
| +// |
| +// Each flush's persistTouched call also writes one recalc marker |
| +// (events.go) for the whole batch — up to openRecoveryBatchSize origins |
| +// — rather than one marker per origin the way setContent's single-origin |
| +// recalcAndPersist does. See persistTouched's doc comment for why that |
| +// granularity change is honest rather than misleading: its triggerID is |
| +// unconditionally 0 on every call site, so a batch's marker was never |
| +// claiming to name the one origin that caused it. |
| func (h *Handle) recoverStale(stale []calc.CellKey, v uint64) error { |
| combined := make(map[calc.CellKey]struct{}) |
| for i, origin := range stale { |
diff --git a/internal/sheets/store/handle_set.go b/internal/sheets/store/handle_set.go| index 0b4f8ab..3d62f17 100644 |
| --- a/internal/sheets/store/handle_set.go |
| +++ b/internal/sheets/store/handle_set.go |
| @@ -242,6 +242,20 @@ func (h *Handle) recalcTouched(origin calc.CellKey, includeOrigin bool) (map[cal |
| // back the id of the event row it just created, and correlating one |
| // would cost a second query for bookkeeping only — the marker's real |
| // job, the derived-count, is unaffected. See events.go. |
| +// |
| +// Marker granularity is caller-defined, not fixed by this function: |
| +// setContent/recalcAndPersist call this once per Set, so one marker |
| +// covers one origin's touched set. recoverStale (handle_batch.go) calls |
| +// this once per BATCH of up to openRecoveryBatchSize origins during |
| +// Open's crash recovery, so during batched recovery one marker's |
| +// DerivedCount covers however many cells that whole batch touched, not |
| +// one origin's — marker cardinality no longer matches origin |
| +// cardinality there. This is safe specifically because triggerID is |
| +// already 0 unconditionally, on every call site, not just the batched |
| +// one: it has never claimed to name the one event that caused a given |
| +// marker's cells to be touched, so a marker covering several origins' |
| +// worth of recomputation asserts nothing false about which trigger |
| +// caused it — there was never a single-trigger claim to violate. |
| func (h *Handle) persistTouched(touched map[calc.CellKey]struct{}, v uint64) error { |
| return h.db.Transaction(func(tx *gorm.DB) error { |
| for key := range touched { |