fix(store): move content-hash migration writes to the ingestor and merge duplicates in memory (#215)

[dborup]

Relates to #215

Follow-up to #209. The content-hash migration wrote to the database from cmd/server, on its mode=ro handle. This moves the DB half to the ingestor and makes the server half in-memory only, including the merge of colliding transmissions.

What was wrong (from the #209 review)

1. migrateContentHashesAsync ran UPDATE/DELETE on transmissions and observations through the read-only handle. Every statement failed, each failure was logged as collides — merging duplicate, and the same work was retried at every start.
2. In memory, colliding rows stayed as ghosts: both kept their own observations, both took the new hash, byHash pointed at one of them, and QueryPackets returned both.
3. nodeHashes is keyed by hash. Keys made before a migration were never renamed, so a later observation was indexed a second time and eviction left the old key behind. After a merge, evicting one ghost also removed the key the other still needed.
4. The charge of a rehashed row (chargedBytes) was off by the hash length difference.
5. perFallbackRelayBytes was not pinned by any test that runs by default.

Investigation (rule 7): is the migration still needed for new rows?

No. InsertTransmission hashes with ComputeContentHash (cmd/ingestor/db.go), and the two copies of the function are identical. Only rows written before the formula changed (Kpa-clawbot#786, Kpa-clawbot#787) can be stale, so this is a one-time conversion of old data, which is why it belongs in the ingestor and is recorded as done.

Design

Ingestor (cmd/ingestor/hash_migrate.go): async migration content_hash_formula_v1, started from main after the ingest buffer is draining and cancelled on shutdown, like the route-mask backfill. It is deliberately not started from OpenStore: that raced every test that opens a store and seeds rows with made-up hashes (it rehashed and merged them under the test).

• Reads id, raw_hex, hash in id order, in batches; rewrites each batch in one transaction under writerMu.
• No collision: the hash is rewritten (and the copy in ping_triggers).
• Collision: the row with the lowest id survives. Observations move to it; one the survivor already has (same observer and path, which idx_observations_dedup would reject) is dropped, and the survivor's copy wins, as it does when ingest meets a repeat reception. The earlier copy could win only by copying every observation column, the optional ones (resolved_path, raw_hex) included, and the in-memory observation too; the transmission keeps the earliest first_seen instead. The survivor takes the earliest first_seen, the later last_seen, the union of route_mask (a grown mask is logged in route_mask_changes for running servers) and, for each column of an explicit allow-list (route_type, payload_type, payload_version, decoded_json, from_pubkey, channel_hash, scope_name; the ones the table has), the duplicate's value when its own is NULL; a value it has stays, "" in scope_name (transport-scoped, region unknown) included. A nullable column added later is not filled unless it is added to the list, because its NULL may mean "pending". A ping_triggers row follows the survivor, or goes if the survivor has one. The duplicate row is deleted.
• Recorded in _async_migrations, so it does not run again. If the formula changes again, bump the name.
• A cancel (shutdown) is checked at the top of every batch and returned as an error: the run stays unfinished (failed, context canceled) and resumes at the next start. It is logged as cancelled (will resume), not FAILED.

Server (cmd/server/hash_migrate.go): no SQL at all. It rehashes in memory and merges with the same rule (lowest id survives), so a server that has not restarted yet and the DB agree on which transmission is the survivor.

• The duplicate's observations move to the survivor (same dedup rule), and the duplicate leaves s.packets, byTxID, byHash, byPayloadType, byNode, byPathHop, the subpath and resolved-pubkey indexes, the distance index and advertPubkeys. Its charge is credited once; a dropped observation is credited when it is dropped.
• nodeHashes is renamed with the hash, finding the keys of a transmission through the transmission (its decoded pubkeys, its fallback relays, and its resolved relays through the resolved-pubkey index) instead of walking all of nodeHashes per batch; the walk stays only as the fallback when the resolved-pubkey index is off. After a merge the lists that held the duplicate are rebuilt, so the shared hash cannot drop the survivor's key and a key only the duplicate held goes.
• A merge takes the earliest first_seen and fills the scope and route type the survivor lacks (decoded_json and payload_type are not filled in memory: the content hash includes them, so the duplicates agree). The scope fill differs from the ingestor's COALESCE in one rare case, see Known remainders. When the survivor takes an earlier first_seen, s.packets, byPayloadType and the survivor's byNode lists are put back in order (eviction cuts from the head).
• main starts the in-memory pass after StartupLoadDone() (LoadChunked and the background fill), not after the first chunk, so it sees the whole store.
• The charge is refreshed for the new hash length, and the survivor is re-picked (best observation), recharged and re-indexed. For that, the path-change refresh in IngestNewObservations is extracted as reindexTxPath and reused (behaviour unchanged).
• The walk is over a snapshot of s.packets, not offsets into it: a merge removes elements and eviction trims the head. The batch is re-checked under the write lock (evicted or rehashed since is skipped).

Not carried over: the relays of a merged observation are not re-derived for the survivor in memory (that needs resolved_path from the DB). They return at the next load, when the DB holds the merged row. Until then they are attributed to nothing, where before they were attributed to a ghost transmission that counted the same packet twice.

Guards

• TestServerHasNoPacketTableWrites: no UPDATE/DELETE/REPLACE on the packet tables anywhere in cmd/server, and no statement or conn call at all in hash_migrate*.go.
• TestHashMigrate_IssuesNoWriteStatements_215: a probe on a recording driver connection, opened the way the server opens the DB (mode=ro), fails on any prepared statement that is not a read.
• The existing write-SQL ratchet for hash_migrate.go goes from 3 to 0.
• TestPerFallbackRelayBytes_IsPinned_215 pins the constant from both sides in the default run (the = 0 mutant is killed).

Not changed

StoreTx stays 320 bytes (both layout tests pass unchanged), no new map[string]interface{}, cmd/server stays read-only, workflows untouched.

Performance

Interleaved test binaries (same fixtures, median of 5 runs per round, 4 rounds in round 2; the tests are in the PR, env-gated). "Before" is the previous head of this PR, "no rename" the same code with the nodeHashes rename stubbed out, to read off its share:

Server migration	master	previous head	now	now, no rename
2000 rows in 1000 colliding pairs	130–134 ms	2.4–2.8 ms	2.6–3.0 ms	2.1–2.7 ms
5000 rows, no collisions, 3 node keys each	19.1–19.5 ms	5.5–5.7 ms	3.2–3.8 ms	2.0–2.8 ms
50000 rows, batch of 500	241–254 ms	253–272 ms	43–48 ms	22 ms

The rename is now about half of the migration at 50000 rows (21 of 43 ms) and about a third at 5000: it costs what it renames (150000 keys), not what the whole index holds, and it no longer grows with the store. Master's numbers include its DB writes on an in-memory SQLite.

Ingestor migration (file DB)
2000 rows in 1000 pairs	88–90 ms
5000 rows, no collisions	70.6–71.5 ms

Deploy plan

The migration deletes rows (merged duplicates) and a code rollback does not bring them back.

1. Snapshot first. Take a consistent copy of the database before the deploy (ingestor stopped, or an online backup) and keep it until the checks below pass. Optionally run the new ingestor's migration on a copy of that snapshot first, to learn the stale count, the merged count and the duration. Check whether idx_observations_dedup exists on that database: where observations predates it, nothing is dropped as a duplicate and both copies stay.
2. Deploy the image, then wait for the ingestor. Both programs start together under supervisord, so the order cannot be chosen at start. The ingestor is finished when _async_migrations has status = 'done' for content_hash_formula_v1 and the log shows [hash-migrate] rehashed ... and [async-migration] "content_hash_formula_v1" done. A cancelled (will resume) line from a restart in between is expected; the run resumes. Record the ingestor's rehashed X of Y ... merged Z counts and its duration.
3. Then restart only the server program (supervisord), once. This step is required, not just tidy. A server that started before the migration finished merges the rows it loaded in memory, and the ingestor has not yet reached some of them. A reception stored on a row the server already merged away, between the server's pass and the ingestor's batch for that row, is skipped by IngestNewObservations (the row is not in the store) and the poller's cursor moves past it, so it stays missing from memory until a reload. The common case is the "stale old row + current-hash new row" pair: the ingestor finds the new row by hash and stores new receptions on it. A restart after the ingestor is done loads the merged database and recovers them. Smaller reasons, all fixed by the same reload: the survivor's merged columns and observation ids come from the database, the relays of merged observations are re-derived at load, and a scope the server filled from a duplicate where the database kept "" is reconciled (see Known remainders).
   • While the server's first in-memory pass runs on a large store with many collisions, expect /api/packets latency to rise: a batch that merges holds the write lock for time that grows with the store (a reviewer measured ~12 ms at 100k and ~38 ms at 300k packets for a batch of 5000). The pass logs one summary line, [hash-migrate] Rehashed ... merged ...; max write-lock hold X ms over N batches (total Y ms): record it. After the restart the pass finds nothing stale and costs nothing.
4. Verify after.
   • PRAGMA foreign_key_check; returns no rows.
   • SELECT COUNT(*) FROM observations o LEFT JOIN transmissions t ON t.id = o.transmission_id WHERE t.id IS NULL; is 0, and the same for ping_triggers.tx_id.
   • Transmissions before minus after equals the merged count in the [hash-migrate] log line; compare observation counts the same way.
5. Rollback. Code: safe. An older build runs on a migrated database (ComputeContentHash is unchanged, an older server's own pass finds nothing stale and attempts no write, an older ingestor ignores the unknown _async_migrations row). Data: only restoring the snapshot undoes the merges, and loses what was ingested since; the new code then runs the migration again, because the restored database has no record of it.

Known remainders

• Receptions on a merged-away row (review NEW-2). Explained in step 3 of the deploy plan; the restart is the fix. A code fix would be a bounded loser-to-survivor id alias for IngestNewObservations, but it has to live until the ingestor finishes the row, which the read-only server cannot see, so a size cap would trade this gap for another. Not done.
• Scope fill (review NEW-4). The ingestor fills a NULL scope_name and keeps "". In memory NULL and "" are both "" and StoreTx has no spare byte for a flag (it stays 320 bytes), so the server fills a "" survivor from the duplicate. They agree for the usual NULL survivor (a row from before scope_name existed, merged with the same packet heard since). They differ for a "" survivor with a duplicate that matched a region, which needs region keys to have changed between two receptions of one packet; the database keeps "" and memory shows the region until the reload above. Pinned by TestHashMigrate_ScopeFillAgreesWithTheIngestorForNull_215 and TestContentHashMigration_ScopeFillOnlyFillsNull_215.
• Write-lock hold of a merging batch (review NEW-1). Only measured in this PR (the summary line above). finishHashMerge runs slices.DeleteFunc over all of s.packets and over each affected byPayloadType list, and a survivor whose first_seen moves triggers a stable re-sort of s.packets and of those lists. Follow-up issue to be opened if staging shows a problem.
• Unlocked snapshot (review M5). A mutant that clones s.packets without the read lock survives -race: the window is a few instructions long. Not asserted.
• Survivor outside the retention window (review N6). Unchanged, rare.
• Order of fills with three or more copies. The ingestor merges the copies in id order and the server in s.packets (first_seen) order; they could fill a NULL column from different copies when the copies disagree on a non-NULL value. Copies of one packet do not normally disagree. [analysis only]

Tests

New and affected tests with -race -count=3, both full suites once, go vet, gofmt -l on changed files, mutants per acceptance criterion: see the report comment on this PR.

🤖 Generated with Claude Code

[dborup]

Rapport — CS-MacBook PR#222 #215 — head f211164

Status: Draft PR open, one PR (no split needed), all local tests green, 26 of 26 mutants killed, CI pending (one check queued, not polled by me).

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from earlier work or another report, not re-verified here.

Base 0f88865b (master at start). Five commits, no rebase, amend or force-push: tests (c536450c), fix (307ce07b), start from main plus probe and perf checks (5c91a68b), a leak found by the baseline test (74579ca0), perf test parameter (f211164d).

Design and why

• Investigation first [A, T]: InsertTransmission hashes with ComputeContentHash and the ingestor and server copies of that function are identical (diff of the two bodies is empty). New rows already carry the current formula, so the migration is a one-time conversion of rows older than the formula (Incorrect packet hash calculation Kpa-clawbot/CoreScope#786, fix: use payload type bits only in content hash (not full header byte) Kpa-clawbot/CoreScope#787). That is why it is an ingestor migration recorded as done, not something that runs at every start.
• Ingestor (cmd/ingestor/hash_migrate.go): async migration content_hash_formula_v1. Rehashes in id-ordered batches, one transaction per batch under writerMu. On a collision the lowest id survives; observations move to it (one the survivor already has, same observer and path, is dropped, because idx_observations_dedup would reject it); last_seen becomes the later, route_mask the union (a grown mask is logged in route_mask_changes), ping_triggers follows the survivor; the duplicate row is deleted [A].
• Started from main, not OpenStore [T]: my first version scheduled it in OpenStore. The first full ingestor run then failed 5 tests (TestScopeNameMigration, TestIngestorPruneOldPackets, three TestPruneOldPackets*) because the migration rehashed and merged rows those tests seed with made-up hashes. It now starts after the ingest buffer is draining and is cancelled on shutdown, like the route-mask backfill; a test pins the placement.
• Server (cmd/server/hash_migrate.go): no SQL. Rehashes in memory and merges with the same rule (lowest id survives), so a server that has not restarted and the DB agree on the survivor. The duplicate's observations move (same dedup rule); the duplicate leaves s.packets, byTxID, byHash, byPayloadType, byNode, byPathHop, the subpath, resolved-pubkey and distance indexes, advertPubkeys; its charge is credited once. nodeHashes is renamed with the hash and, after a merge, rebuilt from the byNode lists that held the duplicate. The charge is refreshed for the new hash length; the survivor is re-picked, recharged and re-indexed (the path-change block of IngestNewObservations is extracted as reindexTxPath and reused, behaviour unchanged) [A].
• Not carried over [A]: the relays of a merged observation are not re-derived for the survivor in memory (that needs resolved_path from the DB); they return at the next load. Until then they are attributed to nothing, where before they were attributed to a ghost that counted the same packet twice.

Acceptance criteria

Criterion	Test (default run)	Red on master	Mutants (all killed)
No UPDATE/DELETE/INSERT from cmd/server for the migration, enforced by a test	TestHashMigrate_IssuesNoWriteStatements_215 (probe on a recording driver connection, opened with the server's own mode=ro DSN, fails on any prepared statement that is not a read); TestServerHasNoPacketTableWrites (source guard: no write on the packet tables in the server, and no statement or conn call in hash_migrate*.go); TestHashMigrate_NeverWritesOnTheReadOnlyHandle_215 (no collides or write error in the log, DB left untouched); the write-SQL ratchet for hash_migrate.go lowered from 3 to 0	yes, all four [T]	M1: the server issues UPDATE transmissions again, killed by the probe, both guards and TestMigrateContentHashesAsync
Converges once in the ingestor, not repeated at the next start	TestContentHashMigration_ConvergesInTheIngestor_215 (batches of 1, 2, 3, 1000; one row per hash, none stale, lowest id survives, observations, last_seen, route_mask, ping_triggers, route_mask_changes, foreign_key_check), TestContentHashMigration_RunsOnce_215 (recorded as done; a stale row added later is untouched), TestContentHashMigration_StartsAfterBufferReady_215	does not compile on master (no migration) [T]	I1 highest id survives; I2 conflicting observations not deleted; I3 migration not recorded; I4 ping trigger not re-pointed; I5 route_mask not unioned; I6 last_seen not the later; I7 ping hash not refreshed; I8 uncontested rows not rehashed
Exactly one transmission per hash with all observations; QueryPackets cardinality correct	TestHashMigrate_MergesDuplicatesInMemory_215 (batches 1, 2, 100, plus QueryPackets: 9 before, 5 after; survivor is the lowest id; observation counts, TransmissionID, byObsID, unique observers, spTotalPaths) and the three collision tests in hash_migrate_test.go, flipped from the old ghost expectations	yes [T]	M3a duplicate stays in s.packets; M3b observations not moved; M3c identical observation kept twice; M3d highest id survives
After migration and eviction byNode, nodeHashes, trackedBytes return to baseline exactly, without the clamp	TestHashMigrate_EvictionReturnsToBaseline_215: compares a store that migrated and evicted 3 merged groups and 3 single rows against a second store that never held them (byNode, nodeHashes, trackedBytes, byPathHop, spIndex, spTxIndex, byObserver, advertPubkeys, relay records, resolved index, counts), batches 1, 3, 1000; TestHashMigrate_AccountingIsExactAfterTheMerge_215 (acct113Check: every chargedBytes current and trackedBytes equal to the live charges); TestHashMigrate_RekeysNodeHashesSoLateObservationsDoNotDuplicate_215; TestHashMigrationMerge_EvictionCreditsEveryObservationOnce_202 now exact against its ballast	yes [T]	M4a nodeHashes not renamed; M4b duplicate's charge not credited; M4c charge not refreshed for the new hash; M4d nodeHashes not rebuilt; M4e dropped observation not credited; M4f survivor not recharged; M4g advert refcount; M4h resolved index entry; M4i fallback record; M4j path-hop entries left
StoreTx stays 320 bytes, no new map[string]interface{}	TestStoreTxLayoutFitsObservedPathHashSizeMaskInPadding, TestStoreTxLayoutFitsRouteMaskInPadding (unchanged, pass); git diff adds 0 lines containing map[string]interface{} [T]	n/a	M5: an 8-byte field added to StoreTx, killed by both layout tests
perFallbackRelayBytes pinned by a default test	TestPerFallbackRelayBytes_IsPinned_215 (at least the structural floor, at most a resolved relay's allowance)	passes on master, as it pins a constant	M6: perFallbackRelayBytes = 0, killed

The baseline test found a real bug in my first version [T]: with three duplicates across batches, a middle duplicate that had taken over a later one's observations left a resolved relay key in byPathHop after migration and eviction, because I cleared its observation list before the path-hop sweep, which finds those keys through the observed paths. Fixed in 74579ca0; M4j covers it.

Tests run [T]

• New and affected tests, go test -race -count=3, server and ingestor: pass, no race reports.
• cd cmd/server && go test ./...: pass, once, on the final head. cd cmd/ingestor && go test ./...: pass, once, on the final head.
• go vet clean in both; gofmt -l on the changed files prints nothing (the ingestor package has other files listed on master; not touched).
• sh test-all.sh from the worktree root: 214 passed, 0 failed.
• Fork guards: grep -c 'github.repository ==' gives 9 in deploy.yml and 1 in release-fast-path.yml; .github is not in the diff.
• Mutants: 26 run, 26 killed, tree restored byte for byte after each (git status clean).

Benchmark [T]

Interleaved master and PR test binaries, same fixtures, 5 rounds, median of 7 runs per round. The tests are in the PR and env-gated (CORESCOPE_PERF_202, CORESCOPE_PERF_215, CORESCOPE_PERF_215_ROWS); the master binary is the master tree plus the PR's perf file, which uses only functions that exist there.

Measurement	master	PR
Server migration, 2000 rows in 1000 colliding pairs	130.3–133.6 ms	2.68–3.01 ms
Server migration, 5000 rows, no collisions, 3 node keys each	19.1–19.5 ms	5.7–6.4 ms
Same, 50000 rows (3 rounds of 3 runs, batch of 500)	241–254 ms	262–294 ms (+8 to +15 %)
Ingestor migration, 2000 rows in 1000 pairs, file DB	n/a	88–90 ms
Ingestor migration, 5000 rows, no collisions, file DB	n/a	70.6–71.5 ms

• Master's server numbers include its DB writes on an in-memory SQLite, so they are not comparable with a real file; the point is that the server no longer does them.
• The +8 to +15 % at 50000 rows is the nodeHashes rename: one pass over the index per batch that has stale hashes, a lookup per entry. It never runs on a store loaded from a converged DB. That test uses a batch of 500, the server uses 5000, so it does 10 times the passes production would for the same rows. No new work in any hot path: the merge is in the migration only, and reindexTxPath is a straight extraction [A].

CI

Pending: the app's PR monitor shows 1 check queued, 0 passing, 0 failing, PR mergeable, review required [T]. I did not poll it. Nothing in CI has been seen yet.

Remaining

• Relays of a merged observation are not re-attributed in memory until the next load (see design). Not measured against a real store.
• Eviction never decrements spTotalPaths (master behaviour, unrelated). It made the baseline comparison impossible for that field, so the baseline test leaves it out and a separate assertion checks it follows the merge. Not fixed here [A, T].
• handlePostPacket in routes.go also INSERTs into the packet tables on the read-only handle; it is known to the write-SQL ratchet and out of scope for the hash migration, so the source guard only forbids UPDATE/DELETE/REPLACE there [A].
• The server still hashes every loaded transmission once at start to find stale ones (same cost as before, plus an 8-byte-per-transmission snapshot of the slice). Not changed [A].
• Not verified: behaviour on a production-size store, the ingestor migration on a large real table, and the interplay with a live ingestor writing during the migration beyond the unit tests (writerMu serialises it with inserts, and each batch re-checks that its rows still exist) [A]. No staging or production access, as instructed.
• Deploy order is not tested: the intended order is the ingestor first, so the DB converges, then a server restart.

[adminopenclaw8-sketch]

Review — CS-Macmini PR#222 hash-migrate — head f211164

Dom: APPROVE med nits

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from earlier work or another report, not re-verified here.

Base: head f211164d sits directly on origin/master 0f88865b, so the merged tree (git merge-tree --write-tree) is f18fdf97, identical to the head tree [T]. git ls-remote showed the same head and master before and after the review [T].

Summary. The data-safety core holds up. With live inserts running alongside the migration, there were no UNIQUE errors and no observations were lost. A cancelled run resumes to exactly the same DB state as an uninterrupted one. No foreign-key violations or orphans appeared, and the server issues no writes [T]. There are no blocking defects. Before ready, I recommend N1 (cancel/resume test), N2 (the nodeHashes rename is one full pass per batch under the store write lock) and the deploy plan under point 8. Point 8 also shows that the server's in-memory half covers only the first load chunk in production (pre-existing).

Findings

#	Severity	Finding
N1	Medium (test gap, data safety)	No test covers cancel and resume of the ingestor migration. Mutant R2b (a cancelled run returns nil, so it is recorded as done and never resumes) survives every 215/ContentHash test [T]. The behaviour itself is correct: my probe cancels mid-run, and the restarted run converges to exactly the uninterrupted DB state. It kills R2b [T]. This migration deletes rows on the first deploy, and a deploy restart mid-run is realistic, so this path should be pinned.
N2	Medium (perf, rule 0)	rekeyNodeHashes walks the whole nodeHashes index once per batch that has a stale row, under s.mu write lock. The cost is O(total entries) per batch, so a full migration is O(stale batches × store size). One pass took 2.5 ms at 150k entries, 15.6 ms at 1M and 115 ms at 5M [T]. Interleaved at 50k rows, the PR without the rename (rename stubbed out) took 25–70 ms, against 255–299 ms for master and 270–284 ms for the PR (one outlier round under load) [T]. The PR's "+8 to +15 % vs master" is accurate, but the rename is ~80–90 % of the PR's own migration time, which the table doesn't show. In production today the cost is capped by N3 (≤ 2 batches run). Suggest computing all renames first (one staleContentHashes pass over the snapshot under the read lock) and renaming in a single pass, or bounding the pass.
N3	Low, pre-existing	The server's in-memory migration only sees the first load chunk. main.go starts it right after FirstChunkReady, and it snapshots s.packets then. Probe P5 (chunked load, chunk size 3, 22 stale rows): after load plus migration, 19 rows still carry the old hash in memory [T]. Master has the same limit (total := len(store.packets) at start) [A]. Consequence: in production the in-memory merge does almost nothing, so memory only converges at a server restart after the ingestor has finished (see point 8). Until then, DB-backed lookups by the old hash miss, because the DB no longer has it [A]. Could be a follow-up: start the in-memory pass after RunStartupLoad returns.
N4	Low (data loss on merge)	A merge keeps only the survivor's own columns. Probe RP-I3 [T]: the survivor keeps its later first_seen (the duplicate's earlier one is lost). scope_name, channel_hash and from_pubkey set only on the duplicate are lost too. For an observation dropped by the dedup index, the survivor's copy wins even when the duplicate's is earlier or stronger (SNR 5/RSSI −60 dropped, −10/−100 kept). last_seen (MAX) and route_mask (union) are right. Suggest MIN(first_seen) plus COALESCE for the nullable columns, in the ingestor and the server merge alike. The content hash ignores transport codes and route bits, so scope_name can legitimately differ between merged rows.
N5	Low (test gap)	Server dedup key not pinned. Mutant S1 makes the dedup key ignore the path, so distinct observations from the same observer are dropped. It survives every 215/_202/_113/HashMigrat test [T]. The fixtures never have the same observer with different paths across duplicates. One such row would close it.
N6	Low	The survivor can be outside the server's retention window. Probe P6 [T]: the stale old row (lowest id, first_seen −72 h) survives, and the in-window row (current hash) is deleted. With RetentionHours = 48, a new reception (attached to id 1 by the ingestor) is skipped by IngestNewObservations ("transmission not yet in store"). It stays invisible in memory, also after a restart, because id 1 is outside the window. On master it went to the in-window row. This needs identical content re-heard more than one retention period later, so it is rare. Noted, not blocking.
N7	Info	The re-checks are not exercised by any test: R3 (row's hash changed since scan), R4 (row deleted by retention since scan) and S4 (server batch re-check under write lock). All three mutants survive [T]. They are defensive and correct as written [A]. A retention-race test for R4 would be cheap.
N8	Info	On shutdown, a running migration is recorded as failed with context canceled and logged as [async-migration] … FAILED [T]. It resumes correctly at the next start, but the line looks alarming during a deploy restart. Consider logging a cancel separately.
N9	Info, separate issue proposed	POST /api/packets returns 500 on the production handle [T]. Details under point 7.
N10	Info, unrelated	The full server -race run failed on one test-only data race in TestReachRank_RefreshHonoursBackoffAfterQueuedFailure, which the PR does not touch. Details under point 5.

1. Ingestor migration

Data loss on a collision [A, T]:

• The lowest id survives. Observations move with UPDATE OR IGNORE, and the rest are deleted. last_seen = MAX, route_mask = union, and a grown mask writes a route_mask_changes row. ping_triggers follows the survivor and takes its hash. The loser's route_mask_changes rows and the loser row itself are deleted. Mutants R1 (delete the loser's observations before moving them) and R5 (keep the loser's route_mask_changes) are killed by TestContentHashMigration_ConvergesInTheIngestor_215 [T].
• Dropped observations are not identical, only identical on the dedup key (transmission_id, observer_idx, COALESCE(path_json,'')) [A]. SNR, RSSI, timestamp and resolved_path of the dropped copy are lost (RP-I3) [T]. This is the same rule normal ingest applies to a repeat reception (INSERT OR IGNORE), so it is consistent, but the survivor's copy wins rather than the earlier one (N4).
• idx_observations_dedup is only created when observations is created (cmd/ingestor/db.go:447). On a DB whose observations table predates that schema, nothing is dropped: both copies stay in the DB (RP-I4) [T], while the server drops one in memory. That is harmless, but worth a check on the snapshot (point 8).

Other references to transmissions.id / hash [A]:

• Handled: observations.transmission_id (FK, foreign keys on), ping_triggers (tx_id, hash), route_mask_changes.transmission_id. observations.resolved_path is a column and moves with the row. resolved_path_backfill_state holds an observation-id watermark, and observation ids are preserved.
• Server-owned separate file ping_score_history_entries (tx_id, hash): self-heals. planPingScoreHistoryReconcile deletes entries whose tx_id no longer has a trigger and recomputes entries whose hash changed (ping_score_history_index.go:158–168). Until the first cycle, QuickSnapshot refuses to publish a mismatched entry.
• No tx reference: dropped_packets.hash (dropped rows, never joined), neighbor_edges, observer_neighbors, node_changes, client_receptions, inactive_nodes, channel_proposals.
• Old-hash deep links (#/packets/<hash>) break. That is inherent to a rehash.

Transactions and concurrency [A, T]:

• InsertTransmission does its by-hash lookup and insert under writerMu, and each batch runs in one transaction under writerMu, so a new-hash row cannot slip in mid-batch. Probe RP-I1: 3000 rows (1400 stale, 300 colliding pairs), batch 50, and 1500 concurrent InsertTransmission calls. 1000 were for the same packets with the current hash (half of them hitting stale rows, which creates fresh collisions) and 500 for new packets. Result: 0 insert errors, status done, 0 duplicate hashes, foreign_key_check empty, 0 orphans, 0 stale. Observations 6000 → 6900 = 6000 − 600 dedup drops (300 pairs × 2 identical observations) + 1500 inserted, exact [T].
• The batch re-checks that each row still exists and still has the scanned hash (hash_migrate.go:176–186) [A], untested (N7).
• The scan runs outside writerMu, but the ingestor has SetMaxOpenConns(1). During a scan (~26 ms per 2000 rows locally) inserts wait for the connection, and that wait does not show in the writer stats [A, T].

Crash or stop mid-run [T]:

• Each batch is atomic. Probe RP-I2: 4000 rows, cancelled after 40 ms. Status became failed (context canceled) with partial progress, and all invariants held at the cancel point. After reopen and restart: status done, and transmissions, observations, per-hash observation counts, last_seen and route_mask all match an uninterrupted run exactly.
• RunAsyncMigration re-runs pending_async and failed. A resumed run rescans from id 0 and is idempotent. If the process dies before the status write, the row stays pending_async and is re-run too [A].

Run once [A]:

• The record is a done row in _async_migrations, inside the same DB. A DB restored from an older backup has no row, so the migration runs again. That is correct, because its rows are stale again.

Start placement and load [A, T]:

• The migration starts after ingestBuffer.Ready() and is cancelled after Shutting down…, before the MQTT disconnect, as pinned by TestContentHashMigration_StartsAfterBufferReady_215.
• Measured on a file DB with 120,000 transmissions and 600,000 observations (5 per tx), default batch 2000 and yield 20 ms, local SSD [T]:

Scenario	Total	Batches	writerMu hold p50 / p95 / p99	After
10 % stale + 2 % colliding pairs	2.45 s	60	20.9 / 23.7 / 24.4 ms	117,600 tx, 588,000 obs
all stale, 10 % colliding pairs	5.60 s	60	72.7 / 85.4 / 85.6 ms	108,000 tx, 540,000 obs
none stale (scan only)	1.57 s	0	—	unchanged

• Hold per batch is bounded by the batch size, not the DB size. An insert waits at most about one hold plus one scan (≈ 110 ms locally in the worst case). With the 50,000-slot ingest buffer, overflow is not plausible.
• Linear extrapolation to a staging-size DB (~8M observations, so roughly 1.5–2M transmissions at 4–5 per tx): about 30–40 s at 10 % stale, and about 70–95 s with every row stale, plus the scan. It is longer if pages are not cached. The stale fraction of a real DB is unknown to me [K: extrapolation, not measured].

2. Server

No SQL [A, T]:

• hash_migrate.go has no statement. The PR's probe and both guards pass in the full run [T].
• My own P4 on the mode=ro handle: the DB stays at 2 rows with the old hashes, and there is no collides or readonly text in the log [T].

Writes the guards miss [A]:

• TestServerHasNoPacketTableWrites forbids UPDATE/DELETE/REPLACE on the four packet tables everywhere, and INSERT or any .conn write call in hash_migrate*.go.
• TestServerSourceHasNoNewWriteSQL ratchets every write-SQL string literal per file (AST-based).
• Together they catch any literal statement. They miss SQL built by concatenation (e.g. "UPDATE " + tbl) and writes through a helper whose SQL is built dynamically. The INSERTs in handlePostPacket are deliberately allowed (point 7).

In-memory merge, indexes cleared [A, T]:

• Every index that eviction clears is also cleared by the merge: byHash, byTxID, byObsID/byObserver/totalObs (for dropped observations), byNode + nodeHashes (rebuilt for touched lists), byPathHop, byPayloadType, spIndex/spTxIndex (and spTotalPaths, which eviction does not decrement), distHops/distPaths, pathHopResolved, fallbackByNode, advertPubkeys, resolvedPubkeyIndex/Reverse, analytics caches (eviction flag), relay-stats and hash-size caches.
• Not touched, the same as eviction: groupedCacheTxs (TTL-expired), apiResolvedPathLRU (holds dropped observation ids; bounded, harmless), routeMaskParked/routeMaskQueue (may hold the loser id; a missing tx is skipped), the neighbor graph (rebuilt from the DB) and the inspect/clock-skew caches.
• The index list in the report matches PacketStore. I found no missing index. Consistency probe (each nodeHashes key backed by a byNode tx, and the reverse) in P1, P3 and P5: 0 inconsistencies [T].

reindexTxPath [A]:

• Verbatim extraction of the path-change block of IngestNewObservations, with the same order and the same spTotalPaths handling. The caller still sets pathHopMutated. Behaviour-neutral.

Order between server and ingestor [A, T]:

• Both use lowest id, so they agree where both see the rows.
• They diverge in two cases: rows outside the server's snapshot (N3) and survivors outside the server's window (N6).
• Server first, ingestor later (the normal container start): the server merges whatever is in its snapshot, and the ingestor later reaches the same survivor. Rows the server never rehashed keep their old hash in memory. A loser the ingestor deletes stays frozen in memory, and new receptions attach to the survivor's id. Memory converges at the next server restart.
• Ingestor first: the server loads a converged DB and the in-memory pass finds nothing.

3. Accounting — probes P1–P4 repeated on the merged tree [T]

• P1 (two stale duplicates, one identical observation): packets 3 → 2. byTxID[2] is gone, byHash[new] points to tx 1, and the survivor has 3 observations and 3 unique observers. QueryPackets total is 2 (survivor + ballast). acct113Check is exact. On master the result was 2 ghosts sharing one hash [K, fix(store): charge the path_json fallback and pin the hash-migration accounting (#202) #209 review].
• P2 (relay first via fallback, then via a late persisted resolved_path): fallbackByNode = 1 and pathHopResolved = 1 for the same relay. The double allowance is unchanged from fix(store): charge the path_json fallback and pin the hash-migration accounting (#202) #209 and is credited exactly at eviction (trackedBytes equals the ballast, 3172) [T]. Info, as before.
• P3 (migrate, then evict the merged survivor): byNode/nodeHashes drop to the ballast's 5 keys with 0 inconsistencies, and trackedBytes = 3172, exactly the ballast. Clean, where fix(store): charge the path_json fallback and pin the hash-migration accounting (#202) #209 left stale old-hash keys.
• P4 (mode=ro): clean, see point 2.
• spTotalPaths: the merge decrements it correctly (3 → 2 in P1), and eviction still never decrements it (2 → 2 in P3). That is master behaviour, as the author says.

4. Perf

• Interleaved master / PR / PR-without-rename test binaries, same fixtures:

Benchmark	Master	PR	PR, rename stubbed
Rehash 50,000 rows, batch 500, 4 rounds × 3 runs	256 / 274 / 299 / (723 outlier) ms	269 / 270 / 284 / (1102 outlier) ms	25 / 28 / 53 / 70 ms
Merge 2000 rows in 1000 pairs, 3 rounds × 5 runs	146 / 160 / 161 ms	2.4 / 2.8 / 2.9 ms	2.2 / 3.2 / 3.2 ms

Round 3 ran while I was compiling mutants and should be ignored [T].

• That confirms the +8–15 % against master comes from the nodeHashes rename, but see N2: the rename dominates the PR's own cost and scales with store size under the write lock.
• No new work in hot paths [A]: the merge code runs only from the migration. reindexTxPath is a straight extraction, and IngestNewObservations calls it under the same condition as before.
• Ingestor migration time and hold: see point 1.

5. Tests, mutants

Full runs on the merged tree (= head), once each [T]:

• cd cmd/ingestor && go test -race -count=1 ./...: ok (417.7 s), 0 races.
• cd cmd/server && go test -race -count=1 ./...: FAIL, one test, TestReachRank_RefreshHonoursBackoffAfterQueuedFailure ("race detected during execution of test"). Everything else passes.
  • The race: the test writes the package var onDegreeSnapshotLoad (reach_rank_test.go:969) while a singleflight goroutine left over from TestReachRank_ExpiredSnapshotAlwaysRefreshes reads it (reach_rank.go:311).
  • The PR touches neither file. The pair passed 15/15 under -race in isolation on both master and head [T]. It is a timing flake in the test design, unrelated to this PR.
  • Proposed issue: "reach_rank tests: onDegreeSnapshotLoad written while a previous test's singleflight refresh still reads it (-race flake)". Fix direction: have the earlier test wait for its in-flight refresh to finish, or make the hook an atomic/per-server field.
• StoreTx = 320 bytes: unsafe.Sizeof(StoreTx{}) check in route_mask_server_test.go:24 and both layout tests pass [T].
• go vet is clean in both packages, and gofmt -l on the changed files prints nothing [T].

Mutants (11, each restored and checked byte-for-byte with cmp; targeted tests 215|ContentHash for the ingestor, 215|HashMigrat|PacketTable|_202|_113|StoreTxLayout for the server) [T]:

Mutant	Result
R1 delete the loser's observations before moving them (data loss)	killed (Converges, all batch sizes)
R2 cancel during yield returns nil	survived (the cancel usually lands in rewrite, so it is still recorded failed)
R2b any cancel returns nil, so it is recorded done and never resumes	survived all PR tests; killed by my probe RP-I2 (N1)
R3 drop the "row still has the scanned hash" re-check	survived (N7)
R4 a row deleted since the scan becomes an error	survived (N7)
R5 the loser's route_mask_changes rows are not deleted	killed (Converges)
S1 server dedup key ignores the path (drops distinct observations)	survived (N5)
S2 old hash left in byHash	killed (probe test, MergesDuplicates, EvictionReturnsToBaseline)
S3 survivor ObservationCount not incremented	killed (MergesDuplicates)
S4 server batch re-check under write lock removed	survived (N7)
S5 loser not removed from byPayloadType	killed (MergesDuplicates, EvictionReturnsToBaseline)

CI (run 37194718175, head f211164d) [T]:

• Go Build & Test: success.
• Playwright E2E: success.
• Build Docker Image: success (built locally in the runner, not pushed).
• Release Artifacts, Deploy Staging, Badges: skipped.

6. Rules [T]

• cmd/server is read-only for this change: no write SQL in hash_migrate.go, and its ratchet entry is removed (3 → 0). handlePostPacket is pre-existing (point 7).
• No new map[string]interface{}: 0 added lines.
• Fork guards: deploy.yml 9, release-fast-path.yml 1. .github is not in the diff.
• No closing keywords in the PR body or in the five commit messages.
• All five commits have author and committer dborup <kontakt@meshview.dk>.

7. handlePostPacket (author's remainder)

Confirmed [A, T]:

• routes.go:1594, :1610 and :1624 run INSERT INTO transmissions, INSERT OR IGNORE INTO observers and INSERT INTO observations on s.db.conn, the mode=ro handle (db.go:181). The route sits behind requireAPIKey.
• Probe: OpenDB (mode=ro) plus a POST with a valid key returns 500 {"error":"transmission insert: attempt to write a readonly database (8)"}, and the DB is unchanged [T].
• So the endpoint has been dead in production since bug(db): vacuumOnStartup fails with SQLITE_BUSY when ingestor + server share DB (auto_vacuum migration #919 broken in single-container topology) Kpa-clawbot/CoreScope#1283. It also leaks the raw SQLite error text, and it never deduplicates by hash: a known packet would hit the UNIQUE constraint even on a writable handle [A].
• The existing route tests pass only because they use a writable test DB [A].

Yes, it deserves its own issue. Proposed:

Title: POST /api/packets INSERTs on the server's read-only handle and always returns 500

handlePostPacket (cmd/server/routes.go) writes transmissions, observers and observations through s.db.conn, which the server opens with mode=ro (Kpa-clawbot#1283). In production every call fails with attempt to write a readonly database and returns 500 with the raw SQLite error. Route tests pass only because they run against a writable test DB. The handler also inserts without a by-hash lookup, so a repeat packet would violate transmissions.hash UNIQUE even on a writable DB.

Options: (a) hand the packet to the ingestor (queue/command table or MQTT republish) so the write happens where all writes live; (b) remove the endpoint and its OpenAPI entry if nothing uses it. In both cases, drop routes.go from knownServerWriteSQL, extend TestServerHasNoPacketTableWrites to forbid INSERT everywhere, and add a test that POSTs against an OpenDB (mode=ro) store.

Acceptance: no write SQL left in cmd/server outside ping_score_history.go/backup.go; the endpoint either works through the ingestor or is gone; a read-only-handle test pins it.

8. Deploy plan

What "ingestor before server" means in one container [A]:

• supervisord starts both programs together (docker/supervisord-go*.conf), so the order cannot be chosen at start.
• In practice: deploy the image, let the ingestor finish, then restart only the server program. Finished means _async_migrations.status = 'done' for content_hash_formula_v1, plus the log lines [hash-migrate] rehashed … and [async-migration] "content_hash_formula_v1" done.
• The restart is needed, not cosmetic. Because of N3, the server's in-memory pass covers only the first load chunk. Until restarted, the server keeps old hashes for most rows (which the DB no longer has) and keeps deleted losers.

Snapshot: yes. The migration deletes rows and cannot be undone by rolling back code.

• Before deploy, take a consistent copy (ingestor stopped, or an online .backup/VACUUM INTO) and keep it until verification passes.
• Optional but cheap: run the new ingestor's migration against a local copy of that snapshot first, to learn the stale count, the merged count and the duration. Do it on the copy only, never on the server.
• Also check whether idx_observations_dedup exists on that DB (point 1).

Verify after:

• status is done, with no FAILED other than a context canceled from a restart;
• PRAGMA foreign_key_check is empty;
• SELECT COUNT(*) FROM observations o LEFT JOIN transmissions t ON t.id = o.transmission_id WHERE t.id IS NULL = 0, and the same check for ping_triggers.tx_id;
• the transmissions count before minus after equals the merged count in the log line.

Rollback [A]:

• An older build runs on a migrated DB. ComputeContentHash is unchanged from master, so an older ingestor writes the same hashes. An older server's own migration finds no stale rows on a converged DB, so it attempts no write. An older ingestor ignores the unknown _async_migrations row.
• Rolling back code keeps the merges. Undoing the data means restoring the snapshot and losing what was ingested since; the new code would then run the migration again, because the restored DB has no record.
• Stopping mid-run is safe and resumes (RP-I2).

Not verified

• Behaviour and duration on a real production-size or staging DB, and the real stale fraction. The figures under point 1 are local synthetic measurements plus a linear extrapolation. No staging or production access, as instructed.
• The real size of nodeHashes in production, which decides N2's per-batch cost.
• Disk-bound timing (page-cache misses) for the ingestor batches.
• The server reading the DB while the ingestor commits batches (WAL readers), beyond the unit-level probes.
• -race on the master tree on its own. The reach-rank flake was checked only in isolation (15 runs each, master and head).
• Reviewer probes (zz_review_probe_215_test.go in both packages, plus a rekey micro-benchmark and a POST probe) are scratch files, not committed.

[dborup]

Rapport — CS-MacBook PR#222 runde 2 — head 7051289

Status: Draft PR, N1–N5 and N8 fixed, N3 solved (cheap), N7 covered, N6 left as noted; all local tests green, 39 of 39 mutants killed including R2b, R3, R4, S1 and S4; CI for the new head not read by me (not polled).

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from the review or earlier work, not re-verified here.

Review feedback addressed (commit 70512893), on top of the tests-first commit 2dc09064 and a merge of origin/master a0086bdd (6201af62, clean; it brought only the #210 ingestor watchdog files). No rebase, amend or force-push; all commits are by dborup <kontakt@meshview.dk> [T].

1. N1, cancel and resume (medium). TestContentHashMigration_InterruptedRunResumesToTheSameState_215 cancels the run after the fourth row (batch of 1) through a test hook, asserts the status is not done (it is failed, context canceled), that rows 11, 12, 13 and 20 are converted while row 30 is still stale, that foreign_key_check and the orphan count are clean at the cancel point, then restarts and asserts done, no stale row, and a canonical dump of transmissions, observations, ping_triggers and route_mask_changes equal to an uninterrupted run's. Production change: the cancel is checked at the top of every batch and returned as an error (before, it depended on a select between the context and the yield timer). Mutant R2b (a cancelled run returns nil, recorded done): killed by that test [T].
2. N2, rename under the write lock (medium). The rename and the byNode removal of a merged duplicate now find the keys through the transmission: its decoded pubkeys, its fallbackByNode relays, and its resolved relays through the resolved-pubkey index (hash to pubkey through a map built once per batch from the nodeHashes keys, only when a transmission of the batch has resolved relays). A batch costs what it holds, not what the whole index holds. The old whole-index pass stays only as the fallback with the resolved-pubkey index off. TestHashMigrate_RenameDoesNotWalkTheWholeIndex_215 counts the whole-index passes (0 with the index on, more than 0 with it off); TestHashMigrate_EvictionReturnsToBaseline_215 now runs with the index on and off, batches 1, 3 and 1000, against the never-held baseline. Mutants: N2a (rename skips resolved relays) and N2b (always walks the whole index) killed [T]. Benchmark below.
3. N4, data kept on a merge (low).
   • Ingestor: first_seen is MIN, last_seen MAX, route_mask the union, and every nullable column the survivor has no value for (read from pragma_table_info, so scope_name, channel_hash, from_pubkey, route_type, payload_type, payload_version, decoded_json and any later one) is filled with COALESCE; a value the survivor has stays. Server: FirstSeen min, ScopeName and RouteType filled; decoded_json and payload_type are not filled in memory (the content hash includes both, so the duplicates agree, and filling would change the survivor's charge and index membership) [A]. When the survivor takes an earlier first_seen, s.packets, byPayloadType and the survivor's byNode lists are put back in order, because eviction cuts from the head.
   • Dropped observations: the survivor's copy wins, documented in the file comment and the PR text. The earlier copy could win only by copying every observation column, the optional ones (resolved_path, raw_hex) included, over the survivor's row and the same into the in-memory observation, so it is not cheap; the transmission carries the earliest time through first_seen instead.
   • Tests: TestContentHashMigration_MergeKeepsEarliestFirstSeenAndFillsNulls_215 (the RP-I3 shape: a later-first_seen survivor with nothing set against an earlier duplicate with scope, channel and from_pubkey; a second pair where the survivor's own values stay; the colliding observation keeps the survivor's SNR) and TestHashMigrate_MergeKeepsEarliestFirstSeenAndFillsNulls_215 (first_seen, scope, s.packets order with a bystander between the two times, and retention reaching all three). Mutants N4a, N4b (ingestor), N4c, N4d, N4e (server): killed [T]. I did not have the exact RP-I3 probe text, only the finding; the tests follow its description [K].
4. N5, dedup key (low). The server fixture now has the same observer with a different path in every duplicate; TestHashMigrate_MergesDuplicatesInMemory_215 expects all of them (7 per survivor). Mutant S1 (the key ignores the path): killed [T].
5. N8, shutdown log (info). async_migration.go logs a context.Canceled as "<name>" cancelled (will resume); the status is still recorded as unfinished so the next start re-runs it, and a real failure still logs FAILED. TestAsyncMigration_CancelIsLoggedAsResumable_215 pins all three (resumable line, no FAILED for the cancel, FAILED kept for boom, cancel not done). Mutant: killed [T].
6. N3, the in-memory pass saw only the first chunk (low, pre-existing). Cheap, so fixed: main starts migrateContentHashesAsync after <-store.StartupLoadDone() (LoadChunked and the background fill), not after the first chunk. TestMain_StartsHashMigrationAfterStartupLoad_215 pins the placement; mutant N3 (wait removed) killed [T]. The pass then walks a snapshot of the whole store. The deploy plan in the PR no longer depends on the restart for hashes, but still recommends it once, to make memory equal the database (see the PR text).
7. N6, N7. N6: no code change; the survivor outside the retention window needs identical content heard again more than a retention period later, and is rare [K]. N7: added TestContentHashMigration_RowsChangedSinceTheScanAreSkipped_215 (a row deleted and a row whose hash changed between the scan and its batch: the run ends done, the changed row is left for the next run, which converges it) and TestHashMigrate_BatchRechecksUnderTheWriteLock_215 (a stale batch applied after the rows were evicted resurrects nothing). Mutants R3, R4 and S4: killed [T].

PR text: gh pr edit 222 --body-file, Relates to #215 still first; it still has no closing keyword and the PR is still a draft [T]. New sections: the design changes above, the benchmark, and the Deploy plan (snapshot first; wait for _async_migrations.status = 'done' and the log lines; restart only the server program; verification SQL after: foreign_key_check, orphan counts, before/after counts; rollback: code safe, data only by restoring the snapshot).

Tests run [T]

• New and affected tests, go test -race -count=3, server and ingestor: pass, no race reports.
• cd cmd/server && go test ./...: pass, once. cd cmd/ingestor && go test ./...: pass, once. Both on the final head.
• go vet clean in both; gofmt -l on the changed files prints nothing; sh test-all.sh 214 passed, 0 failed.
• Fork guards: grep -c 'github.repository ==' gives 9 in deploy.yml and 1 in release-fast-path.yml; .github is not in the diff.
• StoreTx stays 320 bytes (both layout tests pass); 0 added lines with map[string]interface{}; cmd/server has no write SQL (the ratchet, the probe and the source guard pass).
• Mutants: 39 run (my own set, which includes the review's R2b, R3, R4, S1 and S4 in the forms given above, and one or more per criterion of round 1), 39 killed, tree restored after each. Not re-run: the review's R1, R2, R5, S2, S3 and S5 under their own names; round 1's set covers the same ground (loser's observations, route_mask_changes, old hash in byHash, observation count, byPayloadType) and all of it is killed [T].

Benchmark [T]

Interleaved test binaries (previous head 2dc09064, which has the same migration code as f211164d; the current head; and the current head with the rename stubbed out, to read off its share), same fixtures, median of 5 runs per round, 4 rounds:

Server migration	previous head	now	now, rename stubbed
5000 rows, no collisions, 3 node keys each	5.5–5.7 ms	3.2–3.8 ms	2.0–2.8 ms
50000 rows, batch of 500	253–272 ms	43–48 ms	22 ms
2000 rows in 1000 colliding pairs	2.4–2.8 ms	2.6–3.0 ms	2.1–2.7 ms

• Share of the migration time the rename is now: about half at 50000 rows (21 of 43 ms) and about a third at 5000 (1.0 of 3.2 ms). It is the cost of renaming 150000 keys, and no longer a pass over the whole index per batch, so it does not grow with the store; it was about 90 % of the previous head's time at 50000 rows (253 against 22 ms).
• Master, measured in round 1: 241–254 ms at 50000 rows [K, from the first round's table].
• The merge benchmark moves by noise (the merge now also does the first_seen and column fill, which the fixture, with equal times, does not trigger).
• The fixture has decoded pubkeys but no resolved relays, so it uses the transmission-direct keys only; the one-time map for resolved relays (a pass over the nodeHashes keys per batch, the nodes and not their transmissions) is covered for correctness by the baseline test and was not benchmarked separately [A].

CI

For the new head: the app's PR monitor is showing PR 211 for this session, so I could not read the status of 222 from it, and I did not poll. Nothing in CI has been seen for 70512893. The previous head had a green run [K, Macmini's report].

Remaining

• N6 as noted: the survivor can lie outside the server's retention window (rare, not changed).
• The relays of a merged observation are still not re-attributed in memory until the next load (unchanged).
• N9, the POST /api/packets write on the read-only handle, and N10, the TestReachRank_RefreshHonoursBackoffAfterQueuedFailure race, are separate issues, not touched here.
• Not verified: behaviour and duration on a production-size or staging database, the real stale fraction, and the size of nodeHashes in production (it decides the per-batch cost of the resolved-relay map). No staging or production access, as instructed.

[adminopenclaw8-sketch]

Review — CS-Macmini PR#222 runde 2 — head 7051289

Dom: APPROVE med nits

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from earlier work or another report, not re-verified here.

Scope and tree:

• Reviewed: f211164d..70512893 (2dc09064 tests, 70512893 fix) plus the merge 6201af62, which brings master a0086bdd, i.e. fix(ingestor): quiet watchdog retry noise and make force-reconnect shutdown-safe (#102, #103) #210's ingestor watchdog files.
• The merged tree against current master 376d51c8 is 0fe4cc9f. It differs from the head tree only by master's Active observers vanish as nodes after retention.nodeDays without an advert; observer link dead-ends on 'Node not found' #199 files (node_not_found.go, its test, 3 lines in routes.go) and frontend tests [T].
• git ls-remote showed head 70512893 and master 376d51c8 before and after the review [T].

Summary:

• N1, N2, N3, N4, N5, N7 and N8 are fixed, and the round-1 mutants R2b, R3, R4, S1 and S4 are now killed by the PR's own tests [T].
• Data safety holds under harder probes than round 1: 25 random cancels, 14 of them inside a merge transaction, each left consistent state and resumed to the exact reference DB [T].
• Three new points, none blocking:
  • NEW-1: server write-lock hold per batch is O(store size) when the batch merges. This now applies to the whole store because of the N3 fix.
  • NEW-2: receptions on a merged-away row are lost from memory until a restart, so the restart in the deploy plan is required, not just tidy.
  • NEW-3: three test gaps.

N1–N8 status

#	Round-1 finding	Status
N1	No cancel/resume test	Verified fixed. The cancel is checked at the top of every batch. TestContentHashMigration_InterruptedRunResumesToTheSameState_215 kills R2b in its strongest form: every cancel path (top check, scan, rewrite, yield) returns nil [T]. RP-I2 and 25 random cuts resume to the reference (point 1) [T].
N2	nodeHashes rename = full pass per batch under the write lock	Verified fixed. 0 sweeps with the index on, a sweep only with it off [T]. Lookup is complete in code and under probes (point 2). One test gap: NEW-3 / M2.
N3	In-memory pass saw only the first load chunk	Verified fixed. It starts after StartupLoadDone(), which RunStartupLoad closes after LoadChunked and the synchronous loadBackgroundChunks (chunked_load.go) [A]. P5 is clean [T]. Side effects: NEW-1 and NEW-2.
N4	Merge lost first_seen/scope/channel/from_pubkey	Verified fixed. RP-I3 now gives the survivor the earliest first_seen and fills scope_name, channel_hash and from_pubkey; last_seen MAX and route_mask union are unchanged [T]. Dropped observation: the survivor's copy still wins, now documented. That is acceptable (point 4).
N5	Server dedup key not pinned	Verified fixed. S1 is killed by TestHashMigrate_MergesDuplicatesInMemory_215 [T].
N6	Survivor outside the server's retention window	Unchanged, as noted by the author. Rare. Agreed.
N7	Re-checks untested	Verified fixed. R3 and R4 are killed by TestContentHashMigration_RowsChangedSinceTheScanAreSkipped_215, and S4 by TestHashMigrate_BatchRechecksUnderTheWriteLock_215 [T].
N8	Shutdown logged as FAILED	Verified fixed. All 25 random cuts logged "content_hash_formula_v1" cancelled (will resume) and none logged FAILED. The status stays failed with the error text, so the next start re-runs it. That works through the %w chains (rewrite: merge tx … into …: context canceled) [T]. The mutant is killed by TestAsyncMigration_CancelIsLoggedAsResumable_215 [T].

New findings

#	Severity	Finding
NEW-1	Low–medium (perf, rule 0)	Server write-lock hold per batch grows with store size when the batch merges. finishHashMerge runs slices.DeleteFunc over all of s.packets and over each affected byPayloadType list (plus compactDistIndex). When a survivor's first_seen moves, reorderAfterFirstSeenMoved also does a full SortStableFunc of s.packets and of those lists. Measured hold of one applyContentHashUpdates call (batch 5000, 2 % colliding pairs spread over the store, so every batch merges) [T]: 12 ms at 100k packets and 38 ms at 300k (22 ms at 300k without first_seen moves). The profile splits it roughly half DeleteFunc, half the re-sort. The total is O(n²/5000): 0.25 s at 100k, 2.3 s at 300k. By linear extrapolation that is ~0.1–0.25 s per batch and 15–100 s in all at 1–2M packets, in slices with 100 ms yields [K: extrapolation]. The re-sort alone costs 1.7 ms / 23 ms / 45 ms for 0.1M / 1M / 2M near-sorted packets [T]. The fixture has no distance records, so compactDistIndex is not in these numbers. It is a one-time cost after a deploy, and it was bounded to ~2 batches in round 1 only because of N3. Suggest a follow-up: remove losers and move a survivor by binary search on the first_seen-sorted slices (memmove instead of a scan or sort), or at least log the max hold per batch so staging can measure it.
NEW-2	Low (transient data visibility)	A reception on a merged-away row is lost from memory until the server restarts. Probe P7 [T]: the stale old row (id 1) and the current-hash row (id 2) are merged into 1 by the server at start. Before the ingestor reaches them, a new reception is stored with transmission_id = 2, because the ingestor finds row 2 by hash. IngestNewObservations skips it ("transmission not yet in store"), and the poller cursor still moves past it (websocket.go pollStore takes nextObsID from the DB), so it is never picked up. The window runs from the server's in-memory merge to the ingestor's batch for that row. This "stale old + current new" pair is the common case. The restart in step 3 of the deploy plan recovers it, so the restart is required, and the PR text should say why (point 6). An optional code fix: keep a bounded loser→survivor id alias during the migration window for IngestNewObservations.
NEW-3	Low (test gaps)	Three mutants survive the PR's tests [T]. M2 (key lookup reads only pubKey, not destPubKey/srcPubKey) leaves hanging nodeHashes keys. My completeness probe with a destPubKey/srcPubKey pair kills it (8 inconsistencies), but every PR fixture uses ADVERT pubKey only. M6/M7 (re-sort skips byPayloadType / the survivor's byNode lists): their order is not asserted. M5 (snapshot of s.packets without the read lock) survives -race in both the PR tests and my concurrency probe. That race window is tiny, so it is listed for completeness only.
NEW-4	Info	Scope fill differs slightly between server and DB. The server fills ScopeName when the survivor's is "", which covers both NULL (not transport-scoped) and "" (transport-scoped, unknown region). The ingestor's COALESCE fills only NULL. A survivor with "" can show #x in memory while the DB keeps "", until the next load [A]. Separately, fillableColumns is an opt-out list (every nullable column except six). On today's schema it gives exactly route_type, payload_type, payload_version, decoded_json, from_pubkey, channel_hash, scope_name, all correct to fill [T]. A future nullable column whose NULL means "pending" would be filled silently, though. That only matters if the migration runs again (a restored DB). An explicit allow-list would be safer.

1. N1 cancel/resume

• Cancel at the top of each batch [A]: if err := ctx.Err(); err != nil { return err } before every scan. The scan, the BeginTx and every statement in rewriteContentHashes/mergeTransmissions use ctx, so a cancel inside a batch makes the transaction fail and roll back.
• No half state [T], from TestReviewProbe_RandomCancels (3000 rows, batch 150, cancels at 0–72 ms):
  • 25 cuts: 14 inside mergeTransmissions, 6 in rewrite, 4 at the top or yield, 1 in scan.
  • At every cut: 0 duplicate hashes, foreign_key_check empty, 0 orphan observations.
  • After restart, every run's dump matches the uninterrupted reference exactly: per-row hash, first_seen, last_seen, route_mask and observation count.
• RP-I2 repeated [T]: cancel → failed / context canceled at 3929 tx. The resume reaches done with 3429 tx and 10287 observations, identical to the reference.
• R2b: killed by the PR's own TestContentHashMigration_InterruptedRunResumesToTheSameState_215 [T]. M8 (only the new top-of-batch check removed) survives. It is equivalent: the ctx-aware scan still returns context canceled [T, A].

2. N2, targeted rename

Completeness [A]:

• Every write to byNode/nodeHashes goes through indexByNodeKey, from three callers:
  • indexByNode: decoded pubKey/destPubKey/srcPubKey.
  • indexResolvedPathHops: resolved relays. It always pairs addToByNode with addToResolvedPubkeyIndex(tx.ID, …).
  • addFallbackRelay: records the relay in fallbackByNode whenever it adds an entry.
• nodeKeyFinder.keys covers all three. Resolved pubkeys are mapped back through resolvedPubkeyHash, which lowercases, so mixed-case keys map correctly. My probe uses upper-case resolved relays [T].
• No other source exists: no observer pubkeys, and byPathHop is a separate index that does not feed byNode.
• The resolved index is always on in production (useResolvedPathIndex: true in NewPacketStore; only tests turn it off). CheckResolvedPubkeyIndexSize only warns and never clears it, so the finder cannot meet a silently emptied index [A].

Probes on the merged tree, index on and off [T]:

• P1: the survivor holds 3 observations and takes the loser's earlier first_seen; acct113Check is exact. 0 sweeps with the index on, 4 with it off (batch 1).
• P4: both DB rows keep their old hash, and the log has no collides.
• P3: after eviction, trackedBytes = 3172 = the ballast, with 0 byNode/nodeHashes inconsistencies.
• P2: fallbackByNode = 1 and the resolved reverse index = 1 for the same relay; eviction is exact, as in round 1.
• Completeness probe: 40 groups × 3 duplicates + 30 singles + a TXT_MSG pair with destPubKey/srcPubKey and upper-case resolved relays + fallback relays, at batches 1, 7 and 1000, index on and off.
  • After migration: 72 packets and 0 inconsistencies (every nodeHashes key backed by a byNode tx and the reverse, no packet sharing a hash, byTxID/byHash identity, s.packets sorted).
  • After evicting everything: only the ballast left, trackedBytes exact.
• Fallback pass with the index off: correct in all of the above [T]. TestHashMigrate_EvictionReturnsToBaseline_215 also runs both modes [A].

Interleaved benchmark [T]. Test binaries: master + the PR's perf file (A), f211164d (B), merged head (C), run in order A, B, C, 3 rounds, nothing else running:

Server migration	master (A)	f211164 (B)	head (C)
50,000 rows, batch 500 (median of 3 runs/round)	253 / 253 / 255 ms	259 / 259 / 262 ms	46 / 49 / 50 ms
5,000 rows, batch 500 (median of 5)	20.3 / 20.8 / 21.0 ms	5.8 / 6.2 / 6.9 ms	3.5 / 3.5 / 3.9 ms
2,000 rows in 1000 pairs (median of 5)	145 / 146 / 148 ms	2.5 / 2.6 / 2.6 ms	2.4 / 2.8 / 3.0 ms
50,000 rows with resolved relays (3 of 1000 per row), batch 5000 (reviewer fixture; not in master)	—	76 / 83 / 91 ms	58 / 58 / 60 ms

The resolved-relay fixture covers the one-time-per-batch hash→pubkey map, which the author's fixture does not exercise.

3. N3, whole-store pass

Ordering and locks [A, T]:

• main waits on StartupLoadDone() in the goroutine. The pass takes its snapshot under RLock, picks each batch's stale rows under RLock, and applies it under Lock with re-checks: the byTxID identity and the hash still equal to the scanned one (S4 is killed).
• Live ingest (IngestNewFromDB/IngestNewObservations) and eviction already run then. Each takes s.mu.Lock on its own, so they serialise with the batch; nothing is shared outside the lock.
• A tx evicted between scan and apply is skipped. A new tx ingested in between either does not collide or merges by the lowest id.
• Concurrency probe under -race [T]: 435 stale rows, migration at batch 3 with 1 ms yield, against a goroutine running 400 rounds of DB insert + IngestNewFromDB/IngestNewObservations (half of them same-content collisions) and an eviction loop that evicts stale rows mid-run. Result: no race report, 0 inconsistencies, acct113Check exact.

Write-lock hold per batch: see NEW-1. It is O(store) when a batch merges, and about O(batch) otherwise. The 50k no-collision row in the table works out to about 5 ms per 5000 rows.

P5 repeated [T]: chunked load with chunk size 3, migration started the way main starts it now, then 0 stale rows in memory and 0 inconsistencies (round 1: 19 of 22 stale).

4. N4 MIN/COALESCE

pragma_table_info [T]:

• On a DB created by OpenStore, the fill list is route_type, payload_type, payload_version, decoded_json, from_pubkey, channel_hash, scope_name.
• id (pk), first_seen, last_seen, route_mask, created_at, hash and raw_hex are excluded. All seven are correct to fill only when the survivor's value is NULL. See NEW-4 for the opt-out caveat.
• MIN(first_seen) compares TEXT. That is correct for the RFC3339 forms in use: …Z and ….000Z order correctly, since . < Z [A].

Server re-sort [A, T]:

• Correct: s.packets "sorted oldest-first by FirstSeen" is the documented invariant that LoadChunked restores with sort.SliceStable for GetTimestamps/QueryPackets/eviction. Nothing binary-searches s.packets by id.
• The sort only runs when a survivor actually takes an earlier first_seen. For a "stale old + new" pair the survivor is already earlier, so it is rare.
• Cost when it runs: a full O(n log n) sort per such batch (23 ms at 1M), see NEW-1.
• RP-I3 repeated: see the N4 row [T].

5. Mutants

The review's round-1 mutants against the PR's own tests [T]:

• R2b (all cancel paths return nil): killed by …InterruptedRunResumesToTheSameState_215.
• R3: killed by …RowsChangedSinceTheScanAreSkipped_215.
• R4: killed by the same test.
• S1: killed by …MergesDuplicatesInMemory_215.
• S4: killed by …BatchRechecksUnderTheWriteLock_215.
• N8 (cancel logged as FAILED): killed by TestAsyncMigration_CancelIsLoggedAsResumable_215.

New mutants, focused on N2 completeness and N3 concurrency (each restored and checked with cmp) [T]:

Mutant	Result
M1 key lookup omits fallbackByNode relays	killed (MergesDuplicates, EvictionReturnsToBaseline)
M2 key lookup reads only pubKey	survived the PR tests; killed by my completeness probe (NEW-3)
M3 key lookup takes only the first resolved relay	killed (MergesDuplicates, EvictionReturnsToBaseline)
M4 loser keys read after their records are removed	killed (RenameDoesNotWalkTheWholeIndex, via the fallback sweep)
M5 snapshot taken without the read lock (-race)	survived the PR tests and my concurrency probe (NEW-3)
M6 re-sort skips byPayloadType	survived (NEW-3)
M7 re-sort skips the survivor's byNode lists	survived (NEW-3)
M8 ingestor: top-of-batch cancel check removed	survived, equivalent (point 1)
M9 ingestor: fill overwrites the survivor's values	killed (MergeKeepsEarliestFirstSeenAndFillsNulls)

6. Deploy plan in the PR text

Correct and complete for one container under supervisord, matching my round-1 point 8: snapshot (with the dedup-index check), deploy, wait for done plus the two log lines, restart only the server program, verification SQL, rollback (code safe, data only by restore). A cancelled (will resume) line from a restart in between is correctly called expected.

Is the restart still needed after N3? Yes, it is required. The reason given in step 3 should change:

• The text says deleted rows "are held until a reload". With N3 the server merges those itself.
• The real reason is NEW-2: receptions stored on the current-hash row between the server's merge and the ingestor's batch are skipped, and the cursor moves past them, so only a reload shows them.
• Secondary reasons: NEW-4 (scope fill), relays of merged observations not re-attributed in memory, and DB-only merged columns (decoded_json/payload_type fill).

Two additions:

• Expect API latency spikes while the server's first in-memory pass runs on a large store with many collisions (NEW-1). After the restart the pass finds nothing stale and costs nothing.
• Record the [hash-migrate] counts (rehashed X of Y, merged Z) and the server's Rehashed … merged … line, so they can be compared.

7. Rules [T]

• cmd/server is read-only: the source guard, the AST ratchet and the recording-driver probe pass. The server merge has no SQL.
• StoreTx is 320 bytes: both layout tests pass.
• No new map[string]interface{}: 0 added lines in the PR against master, and 0 in the round-2 commits.
• Fork guards: deploy.yml 9 and release-fast-path.yml 1. .github is not in the diff.
• No closing keywords in the PR body or in the commits f211164d..70512893.
• Commit author: 6201af62, 2dc09064 and 70512893 are author and committer dborup <kontakt@meshview.dk>. The commits brought in by the merge are fix(ingestor): quiet watchdog retry noise and make force-reconnect shutdown-safe (#102, #103) #210's, as on master.
• go vet is clean in both packages, and gofmt -l on the changed files prints nothing.

Tests

Full runs on the merged tree 0fe4cc9f, once each [T]:

• cd cmd/server && go test -race -count=1 ./...: ok (433 s), 0 race reports.
• cd cmd/ingestor && go test -race -count=1 ./...: ok (433 s), 0 race reports.
• The known flakes reach_rank tests: onDegreeSnapshotLoad written while a previous test's singleflight refresh still reads it (-race flake) #224 (reach_rank) and test: TestIssue1008_HandlerReturns503WhileSubpathIndexLoading races the background index build under load (-race flake) #227 (TestIssue1008) did not occur in this run.

CI, run 37199436560, head 70512893 [T]:

• Go Build & Test: success.
• Playwright E2E: still in progress when I posted (started 12:01 UTC). Not seen green.
• Release Artifacts: skipped.

Ingestor perf repeated [T]: 120,000 tx and 600,000 observations, batch 2000, file DB, local SSD:

Scenario	Total	writerMu hold p50 / p95 / p99	Round 1 hold p50 / p99
10 % stale + 2 % pairs	2.53 s	22.4 / 26.1 / 27.0 ms	20.9 / 24.4 ms
all stale + 10 % pairs	6.03 s	79.2 / 90.5 / 91.6 ms	72.7 / 85.6 ms

About +7 % from the column fill. The bound per batch is unchanged.

RP-I1 is unchanged: 1500 concurrent inserts, 0 errors, observations exact (6000 − 600 dedup + 1500). RP-I4 (no dedup index) is unchanged: both copies are kept [T].

Not verified

• A production-size or staging DB: real stale fraction, collision count, durations, page-cache effects. Everything above is synthetic and local.
• NEW-1 on a real store with distance records (compactDistIndex is not in my numbers), and the real HTTP latency impact.
• Playwright for this head (still running at posting).
• The real size of nodeHashes keys in production, which drives the one-time-per-batch resolved map. It is cheap in my fixture (6000 keys).
• Reviewer probes are scratch files, not committed.

Recommendation for the staging round

1. Before, on a local copy of the staging snapshot, never on the server:
   • transmissions and observations counts;
   • whether idx_observations_dedup exists;
   • the stale fraction and the number of colliding groups, by running the new ingestor's migration on the copy. Its log gives rehashed X of Y and merged Z, plus the duration.
2. During the deploy:
   • the ingestor's total duration and writer hold for content_hash_migration (p50/p95/p99 in the writer stats, feat(observability): SQLite write-lock wait/hold instrumentation per component — make starvation detectable Kpa-clawbot/CoreScope#1340);
   • [ingest-buffer] pending/dropped lines; expect 0 drops;
   • the server's Rehashed … merged … line and when it appears relative to StartupLoadDone;
   • API latency (e.g. /api/packets p95) during that pass, for NEW-1;
   • any FAILED line (only cancelled (will resume) is acceptable).
3. After done, and after the server-only restart:
   • the verification SQL from the PR text;
   • transmissions before minus after equals the merged count;
   • the server's trackedBytes/packet count before and after the restart;
   • that the ping-score history settles after one cycle (QuickSnapshot refuses mismatched entries until then).

[dborup]

Rapport — CS-MacBook PR#222 runde 3 — head 741a9d6

Status: Draft PR, NEW-1 (measurement only), NEW-2 (documented, no code), NEW-3 and NEW-4 are done; all local suites green and M2, M6 and M7 are now killed; CI for the new head was still running when I posted.

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from the review or earlier work, not re-verified here.

Review feedback addressed (commit 8b40179d), on top of the tests-first commit 4c07bb55 and a merge of origin/master 7697a826 (741a9d64, clean; it brought #213, whose files this PR does not touch). No rebase, amend or force-push; all commits are by dborup <kontakt@meshview.dk> [T]. Master had moved from 376d51c8 to 7697a826 while I worked, so I merged the newer one and ran every full suite on the merged head.

1. NEW-1 (perf, measurement only). The in-memory pass now ends with [hash-migrate] Rehashed X transmissions ... merged Y duplicates; max write-lock hold A ms over N batches (total B ms). A batch is counted when it had something to apply, and the hold is timed from the moment the write lock is taken to just before it is released. No behaviour change. [T]
   • Tests: TestHashMigrate_LogsMaxWriteLockHold_215 (8 batches at batch size 1, 1 at batch size 100, nothing logged when nothing is stale), TestLockHold_TracksTheMaximumAndTheCount_215.
   • Mutants: H1 (reports the last batch instead of the max) killed by the unit test; H2 (batches not counted) killed by both. H3 (the timer moved outside the lock) survives: the value is wall-clock time and no test can assert it. The optimization itself is left to a follow-up, see the proposal below.
2. NEW-2 (documentation). The deploy plan in the PR text now says the server restart in step 3 is required, and why: a reception stored on a row the in-memory merge already removed is skipped by IngestNewObservations and the poller's cursor moves past it, so it is missing until a reload; the "stale old row + current-hash new row" pair is the common case. It also asks to record the ingestor's and the server's counts and warns about the API latency rise while the first pass runs (with the max write-lock hold line to read it off). A "Known remainders" section was added. No code fix: a loser-to-survivor id alias would have to live until the ingestor has finished the row, which the read-only server cannot see, so a size cap would only trade this gap for another. The same explanation is in the header of cmd/server/hash_migrate.go. [A]
3. NEW-3 (test gaps).
   • M2: TestHashMigrate_RekeysDestAndSrcPubkeys_215 has a TXT_MSG pair and a single whose decoded JSON carries destPubKey/srcPubKey, at batch sizes 1, 2 and 100, with the resolved-pubkey index on and off. It checks nodeHashes, byNode, acct113Check, and a full snapshot equal to a store that never had the rows after eviction. Mutant M2 (lookup reads only pubKey) is now killed (the three index-on subtests). [T]
   • M6/M7: TestHashMigrate_FirstSeenMoveKeepsEveryListOrdered_215 asserts the exact order of s.packets, byPayloadType[ADVERT] and byNode[pubkey] after the survivor takes an earlier first_seen, with a precondition that the load order really was [51, 52, 50], and a generic sortedness check over every list. M6 (re-sort skips byPayloadType), M7 (skips the byNode lists) and M7a (only the keyed byNode sort) are now killed; so is M8 (s.packets re-sort skipped). [T]
   • M5 (snapshot of s.packets without the read lock) is left as a remainder: the window is a few instructions long and it survives -race. It is listed in the PR text.
4. NEW-4.
   • fillableColumns is now an explicit allow-list (fillColumns): route_type, payload_type, payload_version, decoded_json, from_pubkey, channel_hash, scope_name, intersected with the columns the table has. TestFillableColumns_IsAnExplicitAllowList_215 checks today's seven, that an added nullable column is not filled, and that an older schema without scope_name is not an error. It was red before the fix. Mutants: A1 (list lacks scope_name) and A2 (list gains last_seen) killed. [T]
   • Scope fill: justified, not changed. The ingestor fills a NULL scope_name and keeps ""; this is pinned by TestContentHashMigration_ScopeFillOnlyFillsNull_215 (NULL←"#y", NULL←"" stays "", "" keeps its value against "#x", NULL and NULL stay NULL) and the NULL/"" distinction is untouched. In memory ScopeName cannot tell NULL from "" (nullStrVal). I implemented a scopeSet flag to match the ingestor exactly, but StoreTx has no padding left for it (328 bytes, both layout tests failed), so I reverted it. The two agree for the usual NULL survivor and differ only for a "" survivor whose duplicate matched a region, which needs region keys to have changed between two receptions of one packet; the restart above reconciles it. TestHashMigrate_ScopeFillAgreesWithTheIngestorForNull_215 pins both halves of that. [T][A]

Tests (local, head 741a9d6) [T]

• New and affected tests, -race -count=3, server and ingestor, after the merge: ok, 0 race reports.
• cd cmd/server && go test -count=1 ./...: ok (42 s). cd cmd/ingestor && go test -count=1 ./...: ok (106 s).
• go vet ./... clean in both; gofmt -l on the changed files prints nothing; sh test-all.sh: 214 passed, 0 failed.
• Red first: at 4c07bb55 the allow-list test fails and cmd/server does not build (lockHold is undefined). The first version of the scope test and the log test were also red against the code before the fix.
• Mutants, one at a time on a scratch copy of the tree, each confirmed to change the file and to compile: M2, M6, M7, M7a, M8, H1, H2, A1, A2 killed; H3 survives (wall-clock, see item 1).

Rules [A][T]

• StoreTx stays 320 bytes (TestStoreTxLayoutFitsRouteMaskInPadding and the observed-path-hash-size layout test pass); no new map[string]interface{} (0 added lines); cmd/server stays read-only (the source guard, the ratchet and the recording-driver probe pass; the new server code has no SQL).
• Fork guards: deploy.yml 9, release-fast-path.yml 1; .github is not in the diff. No closing keywords in the PR body or the commits; Relates to #215 is still first in the body. Only explicit git add.

CI

Run 37203533545 on head 741a9d64: Go Build & Test was pending when I posted. I have not seen it green and did not poll. [T]

Remainders

• NEW-2 (no alias in code) and the scope-fill difference, both documented above and in the PR text.
• H3 and M5, which no test can assert.
• N6 (survivor outside the retention window) is unchanged.
• Not verified: a production-size or staging database, the real write-lock hold of a merging batch (the new log line is there to measure it), and Playwright for this head.

Proposed follow-up issue for NEW-1 (to be opened only if staging shows a problem)

Title: Hash migration: shorten the in-memory write-lock hold of a merging batch

Text:

The server's in-memory content-hash pass (cmd/server/hash_migrate.go, finishHashMerge and reorderAfterFirstSeenMoved) holds the store's write lock for a time that grows with the number of packets whenever a batch merges duplicates. It runs slices.DeleteFunc over all of s.packets and over each affected byPayloadType list, and a survivor whose first_seen moves triggers a stable re-sort of s.packets and of those lists. A reviewer measured about 12 ms at 100k and about 38 ms at 300k packets for one batch of 5000 with 2 % colliding pairs, roughly half in the deletes and half in the re-sort; the total over the pass grows like n²/batch.

Proposal: remove the losers and move a survivor by binary search on the first_seen-sorted slices (a memmove instead of a scan or a sort), batching the removals of one batch into a single pass over each slice only when many are removed. Keep distIndex compaction in the same pass.

Acceptance: a benchmark with 300k and 1M packets and 2 % colliding pairs showing the maximum single-batch hold, and the [hash-migrate] ... max write-lock hold line from a staging run as the baseline. Do this only if that line shows a hold that hurts API latency; the pass runs once after a deploy and finds nothing on later starts.