cache: report a pk row change as one update event #34
No reviewers
Labels
No labels
area:defold
area:docs
area:infra
area:protocol
area:testing
kind:bug
kind:chore
phase:breadth
phase:core
phase:hardening
phase:slice
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
yuris/spacetimedb-defold!34
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/21-update-events"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #21. Body rewritten after two review rounds — the original evidence claims are superseded.
A one-column change on a primary-key table reached the consumer as
delete+insert. The cache state was always right — deletes are applied before inserts so a pk change lands as an upsert — but the event stream reported the row leaving and a different row arriving. In Defold that maps to despawn + respawn: lost tweens, and ago.delete/factory.createpair every tick of movement instead of ago.set_position.apply_database_updatenow decodes each batch up front, pairs deletes with inserts by primary key within oneupdatesentry, suppresses the paired delete event, and emits:The premise is now observed, not inferred
This was the blocker that held the PR: nothing in the repo demonstrated that a real server packs a pk change into a single
updatesentry. If it split them, the feature would have been inert with a green suite.A new
edit_messagereducer, driven over a realv1.json.spacetimedbsocket, one subscription — captured twice, independently, by two different agents on separate servers:tables.length == 1,updates.length == 1, both halves of the same key in that one entry.The edges, probed
deletes=3 inserts=3updatedelete+insert— pairing declines, fails safedelete+insert, no pairingupdate+delete— pre-existing, now filed as #43The pk-value case is the one that looked dangerous going in, since pairing by key cannot work there. It reports
delete+insert, byte-identical tomain, and is now pinned by a test whose two rows are identical apart from the key — so it fails against any implementation that paired by field similarity rather than by key.That last row is also why
cache.luawas not widened to pair across entries: with two distinct queries, entry 1 can be a bare delete of the same key, and any cross-entry join must guess whether it pairs with entry 0's insert. Either guess fabricates or destroys information.Fan-out, measured
Fan-out is per distinct query text, not per subscription — two
SubscribeMultisets sharing a query text produce one entry between them, and the legacySubscribeset counts separately fromSubscribeMultisets. That legacy/multi split is why fixture frame 24 has two entries. Both sets were proven live in the identical-query case, so the single entry is real dedup rather than a dead subscription.Left as-is deliberately:
insertanddeletealready fan out identically (fixture frame 23),counts.inserts/counts.deletescount row operations, and applying the sameupdatetwice is a store no-op.Deliberate boundaries
updbatch — only there is the insert guaranteed applied last, so a synthesizedupdatecan never contradict the storepkkeep today's behaviour exactlyGuarded at three levels
Reverting
cache.luatoorigin/mainturns all three red:dev/test/test_cache.lua—FAIL pk delete+insert pair is one event: expected 1, got 2dev/test/test_fixtures.lua—FAIL a pk change is exactly one event: expected 1, got 2PHASE1 OK light=true, own update still full; pk change is one update event; cached row id=1, and phase 2 confirms the same pk survives the restart. Reverted, it fails end to end through the engine's own socket:SMOKE: FAIL a pk change produced 2 event(s), first of type delete - expected exactly one update→[conformance] FAIL - phase 1 never completed. Phase 1 uses exactly one subscription, so#events == 1there is stable, not incidental.5 passed, 0 failed; stylua clean. Fixture recapture verified stable — 24 frames, identical tag sequence, across a fresh and a dirty database.Known limitation, documented
updateis not emitted for tables keyed by anIdentitycolumn — that pk decodes to a table, so pairing never fires. Filed as #35, a 1.0 blocker, and noted in the README next to theupdaterow.The cache already got the state right — deletes are applied before inserts so a pk change lands as an upsert — but the event stream still described it as a row leaving and a different row arriving. In Defold the natural mapping is event to game object, so a player moving one tile ran go.delete plus factory.create every tick: the sprite flickers and any tween or animation state on it is lost. The alternative was every consumer re-implementing delete/insert diffing by pk, which is exactly the bookkeeping an SDK exists to absorb. A delete and an insert of the same key inside one batch are now one { type = "update", row, old } event. Pairing is by key, never by position: the server sends deletes first today, but the two wire arrays carry no ordering guarantee and depending on one would be a silent trap later. Scope is a single upd, which is also the only scope where the synthesized event cannot contradict the store — within one upd the insert is always applied last, so the paired row is guaranteed present afterwards. Across upds or messages an insert can precede its delete, and an "update" for a row that ended up gone would be worse than the two events it replaced. Tables without a pk keep both events. There is no defensible pairing without a key, and matching on field equality would fold genuine same-shape churn into a bogus update; the official SDKs draw the line in the same place, with on_update living only on TableWithPrimaryKey. counts.inserts and counts.deletes still count row operations rather than synthesized events, so an update adds one to each and the conformance assertions that read those numbers keep their meaning. The store is untouched. The pairing tests were checked falsifiable by reverting cache.lua: the pair, keyed-not-positional and array-order cases all fail without it. The pk-less, unpaired-delete, cross-table and cross-message cases necessarily pass against the old code — they assert behaviour that must not change, and guard against a future over-eager pairing rather than against the old code.Review — automated reviewer, posted for the record
Review: PR #34 —
cache: report a pk row change as one update eventVerified against a clone of
feat/21-update-events(51ebf3c) withorigin/main'scache.luaas the control. Suite:5 passed, 0 failedunderLua 5.1.5. All probe/benchmark numbers below are from runs I did, not from reading.Verdict: approve after should-fixes. I tried hard to break the pairing and could not. Everything I found is in the tests, the docs, or the PR body's claims — with one exception (#11) that is pre-existing but decides whether this feature does anything in production, and therefore has to be decided before the freeze.
What I verified as correct
updbatch (cache.lua:131-139). Per-updrather than per-TableUpdateis the right scope, and for a sharper reason than the PR body gives: the apply loop runs deletes-then-inserts per batch, soupd[2]'s delete can remove a rowupd[1]inserted. Widening the pairing scope to the wholeTableUpdatewould let a synthesizedupdatecontradict the final store. Probe case 8 (delete in batch 1, insert of the same pk in batch 2) →delete+insert, store correct. The conservative scope is load-bearing, not just cautious.nilpk never indexes with a nil key.pair_by_keyguards on the write (cache.lua:102), and every read —paired[row[def.pk]]atcache.lua:148,155,157— is a nil-key read, which is legal in Lua 5.1. Probe case 3 (delete row withid=null) → plaindelete, no error, byte-identical to old. Probe case 4 (insert withid=null) raises the samestoreerror at the same point as old code. No silent pairing of two keyless rows: both are skipped from the map.pk = falsepairs correctly (probe case 5 → oneupdate). This works becausecache.lua:102/109testkey ~= nilrather than truthiness. Worth a one-word comment so a future "simplification" toif key thendoesn't break it.decode_rowstouches no state, and the pk-lessremovefield-wise scan (cache.lua:64-79) compares row contents, which decode order cannot affect; itspairs(row)loop is iteration-order-independent because it only ever breaks on a mismatch. Probe case 10 (pk-less, 2 deletes of duplicate content + 1 insert) gives identical events and identical store on old and new.del(v0),del(v1),ins(v2)) →delete(v0)+update(old=v1),ins=1 del=2. The identity comparison atcache.lua:148does what the PR body says. (But see #3 — it is untested.)counts.inserts/counts.deletesstill count row operations in every probe case.updategenuinely needs nothing inconnection.lua. The only consumers of the return value areconnection.lua:235, 268, 307, 325, which passcountsthrough opaquely; grep finds no branch on event type anywhere instdb/. The README example atREADME.md:179printse.row.senderfor every type and works unchanged forupdate— which is a good argument for having named the new rowrowrather thannew. Endorsing that naming for the freeze: one uniforme.rowacross all three types is worth more than symmetry withold.Findings
1. should-fix —
README.md:285: "The conformance test proves this" now dangles, and is false for what it now followsThe two new blocks were inserted between the
evt.typetable and the paragraph that referred to it. "This" now reads as if conformance proves thee.typetable — and per finding #4, conformance proves nothing aboutupdate. Move the new blocks belowREADME.md:285-286, or re-anchor the sentence ("The conformance test proves the reconnect path against a real server restart…").2. should-fix — block E is a vacuous test;
tbl_deletes_firstis not a different fixturetest_cache.lua:119-125. Lua table constructors with named keys have no order:{ deletes = d, inserts = i }and{ inserts = i, deletes = d }build structurally identical tables, andtbl_deletes_firsteven has the same signature astbl. I confirmed it (same key set, same contents).cache.luareadsupd.deletes/upd.insertsby name — there is no array order to vary. The comment attest_cache.lua:117-118("The server lists deletes first today; pairing must not depend on that") describes a property this test cannot exercise; it is a re-run of block A with different strings.The PR body lists E among "the real guards". It does fail when reverted — but only because it re-asserts A. The real guard for keyed-not-positional is D. Fix: delete block E and its helper, and spend the lines on the untested cases in #3.
3. should-fix — the subtlest line in the diff has no test
cache.lua:146-148(paired[row[def.pk]] == row) is the one non-obvious line, and the PR body asserts its behaviour in prose ("Duplicate deletes of one key are handled by identity comparison, so no event is silently dropped") with nothing exercising it. I verified it works — but a future refactor to the more natural-lookingif not (paired and paired[row[def.pk]]) thensilently drops an event and the suite stays green.Add, in place of block E:
deletethenupdate,deletes == 2(guards the identity comparison)updatetheninsert,inserts == 2null→ still adelete, no error (guards the nil-key path against a futurepaired[key]write)4. should-fix — nothing establishes that a real server packs the delete and the insert into one
updatesentrydev/server/index.jshas exactly two reducers:sendMessage(insert) andclearAll(delete). There is no update reducer anywhere in the repo, and every batch indev/test/fixtures/session.jsonlis deletes-only or inserts-only. The premise this whole PR rests on — inherited from a pre-existing comment, not from an observation — has never been seen on the wire.This matters because
updates[]is a list of QueryUpdates, one per matching subscription query. If the server splits a pk change across two entries, probe case 8 is what consumers get:delete+insert, feature inert, no test failing anywhere. "Conformance not run (needs a live server)" is exactly the wrong caveat for the one behaviour that has no other evidence.Fix: add
editMessage(ctx.db.message.id.update({...})) todev/server/index.js, assert oneupdateevent indev/example/main/smoke.script, recapturesession.jsonl. At an API freeze this is the difference between a feature and a plausible feature.5. should-fix — "the cache's final state is byte-identical" is over-claimed
Probe case 7: batch with
deletes = [row 1],inserts = ["not json"].{2}— delete applied, thendecode_rowraised{1,2}— raised before any delete was appliedconnection.lua:400raises to its caller, so this is reachable from any malformed frame. The new behaviour is better (all-or-nothing per batch), but it is a change, and "byte-identical in every case" is the kind of claim a reviewer checks. Either drop it or claim the improvement deliberately.6. should-fix — event ordering is now frozen contract and is written down nowhere
Consumers iterate
counts.events, so the order is API. It is deterministic — probe case 6 (two pairs, inserts in reverse order of deletes) → twoupdates in insert order, reproducibly — but the rule is only inferable from the code. One README clause:The tradeoff that hides in that sentence, which looks deliberate and correct to me: a paired row's "went away" notification moves later relative to other inserts in the same batch. Position relative to the insert is preserved exactly, and this matches where peer SDKs fire
on_update. Worth stating so nobody re-litigates it post-freeze.7. nit — an unchanged row now reports as
updateProbe case 9:
del [1,"a","v0"]+ins [1,"a","v0"]→ oneupdatewhoseoldcontent-equalsrow.README.md:280says "changed a row in place". Relevant becauseapply_database_updateis also the Subscribe/Unsubscribe-applied path (connection.lua:268), where a delete+insert of one key is query churn rather than a change — so the doc comment atcache.lua:135("the server's own framing for one transaction's changes") is narrower than the code path it describes. One clause in the README, one word in the comment.8. nit — a trailing
insertfor a key that already existsProbe case 1:
del(1) + ins(1) + ins(1)→update(v1),insert(v2). A consumer mappinginsert→ spawn creates a second game object for a key it just updated. Old code was equally wrong (insert+insert) and the server should never emit this, so it is defensive only. Mention or ignore.9. nit — with duplicate deletes,
oldis the last one (cache.lua:103overwrites). Arbitrary but harmless; one word in the comment if you keep the handling.10. Performance — not materially worse; a wash to a small win
Measured,
lua 5.1.5, new vs old:InitialSubscriptionThe up-front decode adds two table allocations per (table ×
upd) unconditionally, plus two more when pairing is attempted. On a largeInitialSubscriptionpair_by_keyearly-returns on#deleted == 0, so the only cost is one N-slot array of references (~8% more allocation churn, no measurable time). On a batch with many pairs it is a net win — halving the event count saves more tables than the pairing allocates. The +7–10% on single-row batches is a microbenchmark where the cache is the only work; against JSON decode and the socket it is noise. Nothing here changes the #30 picture.One lazy improvement if you want the unconditional cost back: a module-level
local EMPTY = {}returned bydecode_rowsfor nil/empty input. Neither array is ever mutated (pair_by_keyand both loops only read), so sharing is safe, and it also kills the throwaway table#(encoded or {})allocates atcache.lua:85. While there, theor nilatcache.lua:139is dead —def.pk and pair_by_key(...)is alreadynilwhendef.pkisnil.11. Pre-existing, but it decides whether #21's value lands: an
Identity-typed pk never pairsprotocol.decode_row(protocol.lua:142-167) never callsprotocol.unwrapon field values, so{"__identity__":"0x…"}decodes to a table and the pk becomes a table reference. Run against both old and newcache.lua, delete+insert of one identity on apk = "owner"table:Two rows in the store for one logical row, and no
update. Aplayertable keyed byIdentityis the canonical SpacetimeDB game pattern, and it is exactly the despawn/respawn footgun the issue opens with. The whole test corpus dodges it:chat_schema.luaisu64/string/string.Not this PR's bug and not this PR's fix — unwrapping row fields changes row field types, i.e. the surface being frozen. But it has to be decided before 1.0, and the honest framing of this PR is that its headline value is conditional on it. File it now; if it will not be fixed for 1.0, say so in the README
updaterow. Same class, worth one line in that issue: au64pk above 2^53 collides in Lua 5.1's doubles, and a pk arriving as"1"in one array and1in the other never pairs.Falsifiability report: honest, with one wrong reason
I reproduced it — each new block extracted and run in isolation (prelude + block) against
origin/main'scache.lua:expected 1, got 2expected 3, got 4expected 1, got 2Exactly the claimed classification, and the quoted failure message is verbatim. Specifically checked:
#counts.events == 1(2 on old). It also assertscounts.events[1].old.text— which on old code would index anilfield on the delete event, so it fails twice over.oldas well asrow(test_cache.lua:76,112). E asserts onlyrow— moot once E goes.deletes=[1,2]withinserts=[3,2!]yields 2 events; D asserts 3.["first"]→["second"], sharing no field, so any equality-based pairing finds no pair and C passes. What C actually trips is positional pairing on pk-less tables (1 event vs the asserted 2). Small, but that paragraph exists to be precise.Does it close #21?
Yes — every acceptance criterion is met:
updatewith correctoldandrowtest_cache.lua:68-82test_cache.lua:89-96deletetest_cache.lua:84-87cache.lua:95-116, tested attest_cache.lua:98-115counts.inserts/counts.deleteskeep counting row operationstest_cache.lua:78-79, 114-115test_cache.lua:136-153updaterow and notes pk-less tables never produce oneREADME.md:276-283Nothing in the issue's acceptance list is unmet. The two gaps are things the issue did not think to ask for: finding #4 (no real-server evidence for the premise) and finding #3 (a claimed behaviour with no test).
Suggested landing order
editMessagereducer + recaptured fixture. I would not freeze this API without it.Verdict: Approve after should-fixes. The pairing logic itself is correct — I could not break it. Six should-fixes, all in tests/docs/claims rather than in cache.lua, plus one pre-existing bug that decides whether this feature actually does anything in production.
Closes issue: Yes. Every acceptance criterion in #21 is met. Two gaps beyond the stated criteria: no real-server evidence that the server packs a pk change into one
updatesentry, and the PR body's duplicate-delete claim has no test.Review actioned — verdict: NEEDS ANOTHER REVIEW
Every should-fix in tests, docs and claims is done. The suite is green, stylua is clean, and the new identity-comparison guard is proven to go red against the plausible refactor.
9358d02 — cache: test the pairing edge cases, document the frozen contract, 3 files, +79 -25.Actioned
{ deletes = d, inserts = i }versus{ inserts = i, deletes = d }, which are the same table — named keys in a Lua constructor carry no order, andcache.luareads them by name. There was no array order to vary; it was block A with different strings. Block D remains the real keyed-not-positional guard.paired[row[pk]] == rowidentity comparison), one delete + two inserts (guards pair consumption, and pins the trailing-insert behaviour as intentional), and a delete with an absent pk. The last one was made deliberately stronger than suggested: with a delete-only batchpairedis nil and Lua short-circuits beforerow[def.pk]is read, so the nil-key path would never be exercised — it now sits alongside a real pair so thepaired[nil]read actually happens.deletes=[valid]+inserts=["not json"]raises and leaves the store untouched. Old code applied the delete first. The corrected PR-body wording is below.updatedocumented; duplicate-deleteoldsemantics recorded; both perf nits taken (local EMPTY = {}, deador nildropped).updaterow: not yet emitted for tables keyed by anIdentitycolumn.Falsifiability, verified by mutation
Replaced the identity comparison with the natural-looking
if not (paired and paired[row[def.pk]]) then:Restored:
5 passed, 0 failed.Corrected claim for finding 5
Why this still needs another pass
Not because of anything in this commit. Finding 4 is unresolved and cannot be resolved on this branch: nothing in the repo demonstrates that a real server packs a pk change into a single
updatesentry.dev/server/index.jshas onlysendMessage(insert) andclearAll(delete) — there is no update reducer anywhere, and every batch insession.jsonlis deletes-only or inserts-only. The premise this PR rests on was inherited from a pre-existing comment, not from an observation.If the server splits a pk change across two
updatesentries, consumers getdelete+insert, this feature is inert, and no test fails. At an API freeze that is the difference between a feature and a plausible feature.dev/server/,dev/test/fixtures/anddev/tools/are owned byfeat/25-codegen-types(PR #36) right now, so this is sequenced: once #36 merges, add aneditMessagereducer, assert exactly oneupdateinsmoke.script, recapturesession.jsonl, and confirm the delete and insert arrive in one entry. Holding this PR until that is proven.9358d0267c783015f5d0Blocker cleared — the premise is ONE ENTRY, observed, not inferred
783015f, rebased ona1e716d.A primary-key change arrives as a delete plus an insert of the same key packed into a single
updatesentry. The feature is real, not plausible.New
edit_messagereducer published to the live 2.8.2 server, driven over a realv1.json.spacetimedbsocket (clear_all→Subscribe→send_message→edit_message), one subscription:tables.length = 1,updates.length = 1, that entry carryingdeletes=1andinserts=1.The two-entry case, and why it confirms rather than contradicts
The recaptured fixture's frame 24 has
updates.length = 2:That is not the delete and insert being split. Each entry independently carries both halves. Two live subscriptions match at that point in the session — the legacy
Subscribefrom step 2, never replaced, plus query set 200 — andupdates[]is one QueryUpdate per matching query. Frame 23, a plain insert at the same point, likewise has two entries each with one insert. So the duplication is per-subscription fan-out, identical to pre-existing insert behaviour, and not introduced here.This settles the correctness question twice:
updatefrom unrelated queries. That is exactly the hazard flagged in review.cache.luawas not widened and was not touched at all.Consumer consequence, now documented
A row matched by two live subscriptions reports two
updateevents — exactly as it already reports twoinserts. Written up in the README next to the events table.Changes
dev/server/index.js—editMessage(id, text)viactx.db.message.id.find+.update, in the existing reducers' style. Themessagedeclaration and every pre-existing reducer/procedure untouched;profile/claim_profileuntouched.dev/server/capture-fixtures.mjs— three steps appended at the end, so frames 1–21 keep their indices, plus alast_inserted_id()helper that reads the pk out of the preceding insert frame rather than predicting the autoinc counter.session.jsonlrecaptured, 21 → 24 frames.chat_schema.luaregenerated (+4 lines, theedit_messageentry).test_fixtures.lua— frame count and tags updated; new replay section asserting a single entry yields exactly oneupdatecarryingold.text="before edit"/row.text="after edit", zero stray delete/insert events,counts.inserts=1/counts.deletes=1still counted, one row upserted in place, and that the full frame yields one update per matching query.Verification
5 passed, 0 failed; stylua cleanPHASE1 OK light=true, own update still full; pk change is one update event; cached row id=1Also corrected the
dev/server/index.jsfrozen-table header per #32's finding: rows are decoded by column order where the wire is positional —TransactionUpdate, on both the insert and delete side — not everywhere.Second-pass review — automated reviewer, posted for the record
Second-pass review: PR #34 —
cache: report a pk row change as one update eventReviewed at
783015f, rebased ona1e716d. Every number below is from a run I did on an isolated server (STDB_PORT=3100,STDB_STATE_DIR=/tmp/spacetimedb-review34b), not from the author's report.Verdict: MERGE. The premise reproduces exactly. Two doc-accuracy should-fixes, neither blocking. Two follow-up issues, neither this PR's.
The premise: independently re-captured, CONFIRMED
Published the branch's
dev/server/index.jsto a live 2.8.2, drove av1.json.spacetimedbsocket myself (clear_all-> legacySubscribe->send_message->edit_message), one subscription:tables.length == 1,updates.length == 1, that one entry carrying both halves of the same key. The feature is real. The blocker is genuinely cleared.The edges the author did not claim to have tested
A throwaway
probemodule withedit_all,rekeyandswapreducers, each scenario on a fresh socket. Wire shapes, then the same shapes fed throughcache.luaon the branch and onorigin/mainas a control.cache.luaorigin/maincontroldeletes=3 inserts=3updatedelete+ 3insertd1i1, keys differdelete(9)+insert(9001)d1i1, keys differdelete(10)+insert(11)update, store 1 rowpair_by_keydeclines and the SDK reportsdelete+insert. It does not fabricate a misleadingupdate, and it is byte-identical toorigin/main. Pairing-by-key fails safe here.The fan-out question — and a correction to the author's stated rule
The author says two subscriptions produce two entries each carrying both halves. That is true of the captured frame but not because of "two subscriptions". Measured:
updatesentriesSubscribeMultisets, identical query textSubscribe+SubscribeMulti, same query textSubscribeMulticarrying two distinct matching queriesThe identical-query dedup is not an artifact of a dead subscription — I proved both sets live: dropping query set 500 deleted nothing (501 still covered the row), then dropping 501 deleted it (
d1i0). Two genuinely live subscriptions, one entry, oneupdateevent.So fan-out is per distinct query text, with the legacy
Subscribeset counted separately fromSubscribeMultisets. Frame 24's two entries come from that legacy/multi pairing, exactly astest_fixtures.lua:340guesses — the frame reading is right, the general rule stated around it is not.Should it be deduplicated before freezing? No — freeze it
send_messageand it has two entries too, soinsertalready fans out identically. Deduplicatingupdatealone would make it the one event type that behaves differently.d1i0). Any cross-entry join must guess whether that delete pairs with entry 0's insert; either guess fabricates or destroys information.cache.luanot being widened is load-bearing, and now there is a captured shape proving it.counts.inserts/counts.deletescount row operations; dedup would desync events from counts.updatetwice is a store no-op (verified: S2b leaves exactly one row), so an idempotent consumer — which a cache consumer must be regardless — is unaffected.The new work
1.
dev/server/index.js— clean.git diff a1e716d HEADis exactly two hunks: the header-comment reword, and a pure insertion ofeditMessageaftersendMessage. Themessageandprofiletable declarations,sendMessage,claimProfile,clearAll,countMessages,echoandalwaysFailsare byte-identical.profile/claim_profileuntouched, as claimed.2.
capture-fixtures.mjs— frame indices did not move. Structural signature (tag, reducer name, request_id, query_id, row lists) of frames 1-21 againsta1e716d's fixture: identical for 20 of 21. The single difference is frame 20's embedded stack-trace line number (spacetimedb_module:8933->:8998), an unavoidable consequence of the module gaining a reducer;test_fixtures.lua:312-316asserts only:find("procedure failed on purpose"), so nothing depends on it.last_inserted_id()under fan-out: sound. Fan-out duplicates the same row across entries, soupdates[0].inserts[0]is always the right row — and this is not theoretical, it is the real capture: frame 23 has two entries, both[5,"editor","before edit"], and the helper produced the correct id 5. Its other assumptions (tables[0]ismessage; the previous frame is a committedTransactionUpdate) fail with a loudTypeErrorrather than silently capturing the wrong shape, which is the stated design goal. Good enough.3.
session.jsonl— recapture is stable. I captured twice on my own server, once against a freshly published db and once against a dirty one, and compared both to the checked-in file. All three: 24 frames, byte-identical tag sequence, identical structural shape for frames 22-24 includingupdates=2on both 23 and 24. Not order-dependent. Only the autoinc ids drift when the db is not freshly published (5 vs 10), and no assertion hardcodes them.Bonus: regenerating
chat_schema.luafrom my published copy of the branch module reproduced the checked-in file byte-for-byte apart from the server URL, so the committed schema really does match the branch module, and#36'sprofile/typespacecontent survived the rebase intact.4. The new replay assertions do depend on the pairing. With
cache.luareverted toorigin/main:Re-running the new block with non-fatal asserts to classify every line: 7 of 17 are hard guards (
#events == 1, event type,old.text,row.text, same-key,#events == #updates, all-updates). The other 10 are fixture-shape assertions and must-not-change invariants (counts.deletes/inserts, one row upserted) — correct to keep, they defend against an over-eager future implementation.5. Conformance — PASS, and the negative control fires. Full run on the branch: PASS both runs, first try, no #39 flake, phase 1 printing
PHASE1 OK light=true, own update still full; pk change is one update event; cached row id=1. Phase 2 confirmed the same pk survived the restart.The guard is real. Same run with
cache.luareverted toorigin/main:Phase 1 uses exactly one subscription, so
#events == 1is a stable assertion, not an accident.Rebase check
Base side (
a1e716d): nothing dropped. Two-hunkindex.jsdiff;chat_schema.lua+4 lines only, with #36'sprofile/typespaceintact and byte-identical to a fresh regen;session.jsonlframes 1-21 structurally intact; README purely additive plus one reworded table cell and the re-anchored conformance sentence.Branch side: could not diff directly. The pre-rebase commits were force-pushed away —
git fetch origin 51ebf3cand9358d02both returncouldn't find remote ref, and neither is reachable from any ref on the server. So I verified the previous round's work by content instead, and all of it is present: block E gone (tbl_deletes_firstabsent repo-wide), the three edge-case tests present, the atomicity test present, the README event-ordering paragraph present,local EMPTY = {}present, the deador nilgone from the pairing line, thekey ~= nilrationale commented, and the #35 caveat present (issue #35 exists and is open). Nothing from round one was lost.Merge-order note:
feat/32-column-order(#32) is not merged and independently rewritescapture-fixtures.mjs's header comment andchat_schema.lua's generated header, and reworkstest_fixtures.lua. #34's third comment says it "corrected theindex.jsfrozen-table header per #32's finding" — that is inindex.js, which #32 does not touch, so no conflict there. But both branches touchcapture-fixtures.mjs(different regions) andtest_fixtures.lua, so whichever lands second wants a look.Should-fix (docs only, not blocking)
1.
README.md:296-297overstates the fan-out trigger. "a row matched by two live subscriptions reports twoupdateevents" is false as written — two live subscriptions with the same query text produce one entry and oneupdate. The "exactly as it already reports twoinserts" symmetry IS correct. Suggested replacement:Same imprecision in
test_fixtures.lua:342's label ("one QueryUpdate per matching subscription") and its comment at 335-341. The assertion itself is fine — it reads a checked-in fixture and cannot fail spuriously — but the label teaches the wrong rule. Note this errs in the safe direction: it warns consumers about more duplication than actually occurs, so no consumer written against it is wrong.2. The PR body is stale. It still says "A, D, E fail when reverted" (block E was deleted in the actioned round) and "Conformance not run (needs a live server)" (conformance is now the headline evidence and passes). The third comment supersedes both, but the body is what a squash-merge carries into history.
Follow-up issues, not this PR
1. A pk change that makes a row leave one of several matching queries drops it from the cache. Wire (two distinct queries, edit makes the row stop matching one): entry 0 = full pair, entry 1 = bare
deleteof the same key. Through the cache:updatethendelete, final store empty — though the row still exists and still matches the other live query.origin/mainproducesdelete, insert, deleteand the same empty store, so this is pre-existing and #34 neither causes nor worsens it; it is the documented overlapping-subscription hazard ("clients would need to track which rows exist in which queries") showing up on the transaction path rather than the unsubscribe path. #34 only changes the reported events.2. Optional one-clause README addition for the pk-VALUE change. The behaviour is correct —
delete+insert, no fabricatedupdate— but it is the one case where pairing-by-key deliberately declines, and a frozen contract may as well say so.MERGE. The premise is observed, not inferred, and I reproduced it. The unit assertions, the fixture replay and the conformance phase-1 assertion all go red when the pairing is removed, so the feature is guarded at three levels including end to end through the engine's own socket. The four unclaimed edges all behave correctly, and the one that looked dangerous going in — a pk-value change, where pairing by key cannot work — fails safe and reports
delete+insert. The fan-out behaviour is acceptable to freeze and should not be deduplicated. Nothing found requires a code change; the two should-fixes are prose.Verdict: MERGE
783015f5d04cdf1e9e6f