oficina / docs Public

A document no longer stops accepting edits at 4096 updates. Branch worktree-snapshots, 18 commits, gate green at 6162408.

This thread should have existed eighteen commits ago, and that is a process failure worth naming before anything else. The house rules make a branch discussion the review record and the channel between sessions; opening it at the end turns it into a retrospective nobody could have acted on. For the whole of this work — nine tasks, twelve reviews, five defects found — there was nothing on the hub another session could read, question, or object to. A sibling session hitting the same maxRetainedUpdates cliff would have had no way to know it was already being fixed. Paul caught it; I should have. On the next branch the thread opens with the first commit and the decisions land in it as they are made, rather than being reconstructed afterwards.

What follows is the record it should have accumulated.

Why this exists, and it is not latency

internal/core/core.go's maxRetainedUpdates is a cliff rather than a budget. At 4096 retained updates Handle.Apply refuses everything further — ErrUpdateLogFull, surfaced to the browser as a 400 the save queue treats as permanent — and the document stops accepting edits, permanently, with no warning first. At the current 2s debounce that is roughly 2h16m of continuous typing on one document. Someone writing all day reaches it.

Snapshots remove that. They also make a shorter save debounce affordable, which is the thing anyone actually feels, but that is a later change and not this one.

What it does

A snapshot is one ordinary Yjs update encoding a whole document. That single property is what keeps this cheap: Store.replay and the browser's applyServerLog both already know how to apply concatenated updates, so a snapshot needs no new path in either.

• The Rust core gains a state export and a verifier (ABI v3 → v4).
• Store.ApplyUpdate takes a snapshot from the replayed state, before the incoming update is applied, verifies it inside the core, writes it every 256 updates, and deletes the one it supersedes.
• Store.replay and GET /d/{sqid}/updates both read snapshot-then-tail. ?raw=1 returns only log rows, as an escape hatch.
• The browser retries once through ?raw=1 when applying fetched bytes throws.

Measured: replaying 4000 rows takes 3.55ms with a snapshot against 71.10ms without — about 20×, mean of five trials each, warm wasm module on both sides. Verified in a real browser: a document with 300 updates (past the 256 cadence, snapshot at seq 256 confirmed in SQLite) loads, accepts typing, and keeps it across a reload.

The decision the whole thing turns on

The snapshot is taken from the replayed state before Apply, and that placement is the correctness argument rather than a style choice. Taking it after a successful apply — the natural thing to write — is wrong twice:

1. ApplyUpdate's replay runs outside its write transaction. Two concurrent POSTs both replay rows 1..10; one commits seq 11, the other seq 12. The second handle holds 1..10 plus its own update and not row 11. A snapshot from it claims seq 12 while missing row 11's content — and the verification cannot catch that, because it compares the candidate against the same stale handle that produced it.
2. A document already at the cap replays to a full handle, so Apply refuses before any update succeeds. An after-success hook is never reached, and the document stays read-only forever — the exact case this feature exists to fix.

The implementer verified the placement by mutating it to the wrong one and confirming both tests fail 3/3 before reverting.

What review caught, because it is the more useful half

Nine tasks, each with its own review, then a whole-branch review by Fable reading all 18 commits as one diff. Verdict: approve with changes; both changes landed.

Five separate times a test turned out to be incapable of failing against the mistake it guarded. Four of those were in code I wrote in the plan and the task briefs:

• The Docs-Seq regression test asked only with since == MAX, where the derived implementation and the rejected MAX(seq) one agree on every case.
• A client-side test "delivered" its straggler by settling an already-resolved promise — a no-op, so it passed whether the module used max() or plain assignment.
• The concurrency test was vacuous twice over: warming to cadence-1 meant no snapshot was written during the race at all, and giving every racer identical bytes made a stranded row invisible in the render.
• The ordering test had id and seq rising together, so it passed against an implementation that ignored seq entirely.
• And the one the whole-branch review found: mutating either tail query to seq > snapshot.Seq + 1 — dropping the first row after a snapshot — left the entire suite green, because all distinguishable content sat inside the snapshot's range by design and every tail row was idempotent duplicate bytes.

Each is now fixed and, more importantly, each fix was verified by deliberately breaking the code and watching the test fail. That discipline is the only reason the fifth was found.

Rulings taken without asking

Recorded here because they were decisions made on Paul's behalf. The whole-branch review read all eight and judged none wrong.

1. The ABI version moves on both sides of the contract in one commit. Splitting it leaves a state where the host refuses to load the artifact.
2. One task owns internal/core's exports, rather than two later tasks reaching into it.
3. Added a test distinguishing encode_state_as_update_v1 from encode_diff_v1. The latter silently drops the pending queue; the substitution renders identically today and breaks a merge tomorrow, and no render comparison — including the verifier's own — can see it. This is the single most valuable test on the branch.
4. and 5. Promoted two comments that taught wrong rules into fix rounds, on this repo's own standard that a confidently wrong comment is worse than none.
5. Added a whole task for a failure this branch creates. Longer logs are now possible for the first time, so a long document whose snapshot becomes unusable — corrupted, or excluded by the exact-ABI filter after any future bump — would fall back to a full replay, hit the cap mid-tail, and be left both unreadable and unwritable with the self-repair unreachable. An ABI bump would do that to every long document at once. replay now re-seeds a fresh handle from an in-memory snapshot and continues, writing nothing on a read path.
6. Entered a fix round on a finding the reviewer called "not a defect" — the raw=1 spelling was pinned by nothing the gate runs, so a wrong spelling would have silently disabled the escape hatch forever.
7. Overruled an implementer's decision to log nothing on a re-seed. Its reasoning held only for documents still being edited; a finished document never written again re-seeds on every read, forever, at ~20× cost, with latency as the only evidence.

The review's one criticism was of ruling 6's follow-through: it was labelled spec-level in the ledger and then landed only in a task brief and code comments. The spec now carries D8, a §6 failure-table row, a §10 note, and a corrected D4.

Known and deferred

None of these block merge; all are recorded rather than hidden.

• verify_snapshot's delete-set arm is untested — no fixture contains a deletion, so the state-vector arm always discriminates first. The most substantive of these.
• replay still materialises the whole log slice before applying it. Retained-handle memory is now bounded; that slice is not, and this branch is what makes a very long log reachable. Belongs with the compaction-horizon work.
• ?raw=1 still applies maxLogBytes, so a document with an 8MB+ log and a corrupt snapshot is uneditable both ways. Inherent to the hatch's definition.
• A document at exactly 4096 rows still takes one 400 before recovering, while a longer one now succeeds first time. Both correct; the boundary is no longer uniform.
• The live-update raw retry applies onto a possibly partially-poisoned Y.Doc, unlike the initial-load path which uses a fresh one. Requires a snapshot written and corrupted inside a live client's ~256-update lag window.
• Four stale comments across abi.rs, store.go, core.go and tests/snapshot.rs, for a later comment pass.

Not merged

Paul asked for this to stop at the branch and its discussion. Squash-merge and deploy are his call. When it does land, the next piece is the one that changes what a person actually feels: shortening DEBOUNCE_MS, which snapshots are what make affordable.