fix(store): index live observations from the persisted resolved_path, as Load does (#158)

[dborup-agent]

Depends on #162 — stacked on baa68cd

Relates to #158
Relates to #184

Problem

After #162 (no more duplicate byPathHop entries), staging still showed the sum of traffic_share_score over /api/nodes rising with uptime: 14.34 at 2 h 36 m, 14.02 at 5 h 10 m, 20.67 at 18 h 6 m (no node at 1.0, so not the #158 duplicates).

Cause

traffic_share_score counts, per relay, the transmissions indexed under its full pubkey in byPathHop. Those entries come from two different resolvers, depending on how the transmission entered the store:

Path	Input to indexResolvedPathHops
Load, scanAndMergeChunk	observations.resolved_path, written by the ingestor (Kpa-clawbot#1547) with resolvePathWithContext (Kpa-clawbot#1560): an ambiguous prefix stays null unless the neighbor graph gives exactly one adjacent candidate
IngestNewFromDB, IngestNewObservations	the server's resolvePathForObs → prefixMap.resolveWithContext, which always picks a candidate (affinity → geo proximity to the observer → GPS → observation count), and can pick a different one per observer

IngestNewFromDB explicitly did not select resolved_path ("On restart, Load() handles reading persisted resolved_path values", review item #7). That premise held while the server persisted its own resolution. Since Kpa-clawbot#1287/Kpa-clawbot#1547 the ingestor writes it with a different algorithm, so the same transmission counts for more relays when it arrives live than after a restart. The store mixes both halves, so the sum rises until retention (48 h) has replaced the loaded half, and drops back after every restart.

Live evidence (staging, public API, read-only)

WebSocket broadcasts carry the server's per-observation resolution; /api/packets?expand=observations carries the ingestor's persisted one for the same rows. 8 minutes, 2,860 non-advert observations with a path, 21,651 hops:

	Server (live index)	Ingestor (Load index)
Hops resolved	98.5 %	59.9 %
Distinct relay pubkeys per transmission	54.3	31.4
Both resolved, different pubkey	0
(tx, prefix) pairs resolved to >1 pubkey across observers	254 of 3,075 (8 %)	0

A model where the loaded part of the store (first_seen before the restart) contributes the ingestor count and the live part the server count (approximated by distinct hop prefixes) predicts 19.9 at 18 h; /api/nodes showed 20.79. With only ingestor resolution it would be 15.4.

Part of the rise is real traffic: the live windows also have more observers per transmission (more floods), and with the ingestor's resolution alone the per-tx count is 13.4 (loaded windows) vs 17.9 (live windows).

Fix

indexObservationRelayHops (new, store.go) is the one place that feeds the relay-hop indexes from an observation. Load, scanAndMergeChunk, IngestNewFromDB and IngestNewObservations all call it:

• persisted resolved_path → indexResolvedPathHops (byNode, byPathHop, resolved index);
• NULL → path_json re-resolved on unique prefixes, into byNode only (as Load already did, bug(paths-through-node): 1-byte prefix collision resolves arbitrarily — attributes packets to wrong node Kpa-clawbot/CoreScope#1352 / PR fix(nodes): rebuild relay-hop history on startup from path_json Kpa-clawbot/CoreScope#1643).

The two live queries now select o.resolved_path (when the column exists). The ingestor writes it in the same INSERT, so it is there when the server polls. The WebSocket broadcast still carries the server's own resolution; only indexing changes.

Round 2 (review follow-ups, no change to the behaviour above):

• Both live queries take one snapshot of the optional-column flags (DB.ingestCols) for the SELECT list and the Scan destinations. Reading the atomics twice could give Scan the wrong destination count if the schema healer flipped a flag in between, and the swallowed error silently skipped the row.
• resolved_path is decoded before s.mu is taken on the live paths (decodePersistedRelayPath), and decoded straight into []string.
• The two stale "resolved_path is NULL on every live deployment" comments in Load / LoadChunked are corrected.

Behaviour change

Coverage is partial, and this PR does not change that. The ingestor's resolver is conservative, so on staging only about 47 % of the observations that carry a path have a non-null resolved_path (sample of 500 packets: 283 with a path, 132 non-null; inside the non-null rows only 4.3 % of hops are null, so it is mostly all-or-nothing per observation). About 53 % of relay traffic therefore gives live ingest byNode credit only (unique-prefix fallback) and no byPathHop or traffic-share credit, exactly as after a restart today.

What the change does: it makes live and loaded numbers consistent, at the ingestor's conservative level. Live relay attribution (relay counts, traffic share, byNode activity of relays) for hops behind an ambiguous prefix now matches what a restart already showed. Nodes that were credited only through the server's tie-break lose that credit between restarts too. Traffic share will therefore read lower than the old live numbers; that is the consistency being restored, not a regression.

Relates to #184 for the coverage question (why ~53 % of observations stay NULL, whether the resolver or a backfill should close the gap) and for the possible startup race on hasResolvedPath.

The #158 fixture inserted live observations with resolved_path = NULL ("resolved fresh by the server on ingest"). It now writes resolved_path in the same row, as the ingestor does; expectations are unchanged.

Tests

Test	Covers	Killed mutant
TestTrafficShareLiveMatchesLoad_AmbiguousPrefix_158 (live batch, late observation)	live == load per relay and in sum; no tx under more relays than hops. Red on baa68cd: E5 nodes 0.5 live vs 0 after load, full-key sum 2.0 vs 1.0, a 2-hop tx under 3 relays	live ingest ignores resolved_path; live indexes the server's resolution
TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158	a late observation from another observer whose persisted resolved_path names a relay the first one lacked: live byPathHop and share == Load	IngestNewObservations ignores resolved_path (it survived the tests above); IngestNewObservations does not select the column
TestLiveIngestIndexesLikeLoad_NullResolvedPath_158	NULL resolved_path, IngestNewFromDB: byNode only, identical to load	NULL branch also feeds byPathHop
TestLateObservationIndexesLikeLoad_NullResolvedPath_158	the same NULL case for IngestNewObservations	same
TestIngestSchemaFlagFlipBetweenQueryAndScan_158 (14 subcases)	a flag flipped between query and Scan, both directions, both functions, for raw_hex / resolved_path / route_mask / scope_name: no row skipped. Red before the snapshot	Scan re-reads the flags
TestDecodePersistedRelayPath_158	NULL vs all-null vs corrupt vs non-string input; equal to the old extractResolvedPubkeys(unmarshalResolvedPath(s))	all-null persisted path treated as NULL

Moving the decode out of the lock changes no behaviour, so no test fails if it is moved back; the benchmark below is the only guard.

• cd cmd/server && go test ./...: ok. cd cmd/ingestor && go test ./...: ok.
• go test -race -count=10 -run '_158|_115|IngestNew|LoadChunked|ScanAndMerge|1352|1643': ok, no races.

Performance

benchstat, n=10, interleaved runs, same test files on all sides. "base" is baa68cd plus the benchmark file of the first commit.

Benchmark	base (baa68cd)	9e3c519	this PR
IngestNewObservations_158 (20 obs), time	1.87 ms	1.94 ms (~)	1.93 ms (~)
IngestNewFromDB_158 (20 tx × 11 obs), time	4.05 ms	4.58 ms (+13 %)	4.55 ms (+12 %)
IngestNewFromDB_158, allocs/op	29.8 k	33.5 k (+12 %)	32.8 k (+10 %)
time holding s.mu, IngestNewFromDB_158		2.28 ms	1.90 ms (−17 %, p<0.001)
time holding s.mu, IngestNewObservations_158		1.83 ms	1.72 ms (not significant, ±24 %)

These percentages depend on the machine. The table above was measured on a linux/amd64 cloud container. An independent re-review on an Apple M4 (n=8, Mann-Whitney U) found the same direction but smaller sizes: the lock-held time of IngestNewFromDB_158 drops by 6.4 % against 9e3c519 (here −17 %), and its total time is +6.6 % against base (here +12 %). IngestNewObservations_158 also depends on the benchmark's iteration count: it rotates over 511 observers, so with a large auto b.N most iterations only hit the dedupe path (on the M4: +5.7 % with auto benchtime, −0.3 % with -benchtime=400x).

Honest reading: the extra cost is one column, one resolved_path decode and one index step per observation, because the server resolution still runs for the broadcast. Total time per call is unchanged by round 2 (the work moved, it did not go away); what shrank is the part spent under the lock for IngestNewFromDB. Lock time was measured in an instrumented copy (timestamps around the critical section), not in committed code. At staging rates (~4–5 observations/s on the WebSocket) the absolute cost is negligible. No new maps, and no map[string]interface{}.

Not verified

• Staging after deploy (the sum should stay roughly flat over a day and not drop on restart).
• The ingestor's resolved_path = COALESCE(excluded.resolved_path, resolved_path) upsert can fill a row in later; live ingest reads the value present at first poll, Load reads the later one.
• Which resolver is "right" for display is out of scope; the broadcast still shows the server's guess.
• In IngestNewObservations the resolved_path decode runs for every scanned row, outside the lock, including rows that are then skipped under the lock as already ingested (byObsID) or as an observer+path duplicate. Decoding only the surviving rows would need those checks before the decode, i.e. a second pass under the lock, so the small cost (one short JSON decode per row) is accepted.
• Load and LoadChunked still read the optional-column flags twice (query and Scan), and the possible startup race on hasResolvedPath is not addressed here: both belong to ingestor resolves only ~half of relay paths (resolved_path NULL in ~53 % of observations); possible hasResolvedPath startup race #184.
• TestHandleNodePaths_FallbackPreconfirmed_1352 fails with a 503 "index loading" under heavy CPU load, on 9e3c519 as well (its reloadStore does not wait for the index build). Not touched here.

🤖 Generated with Claude Code

https://claude.ai/code/session_017nop1P1pX4SrfRMHAernzQ

---

Generated by Claude Code

[dborup]

Review feedback addressed (commit 504b514; the round consists of f6321b8, 9ac2b9b, 5947bd6, 504b514)

1. Test gap, mutant M2 (f6321b8): TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158 has a late observation from another observer whose persisted resolved_path names a relay the first observation lacked; live byPathHop and traffic share must equal a Load of the same rows. The mutant that makes IngestNewObservations ignore resolved_path is now red (share 0.5 vs 1.0, sum 1.0 vs 2.0, 10 vs 20 transmissions per bucket); it survived the earlier tests.
2. hasResolvedPath() read twice (9ac2b9b): DB.ingestCols takes one snapshot per call of the four optional-column flags (raw_hex, resolved_path, scope_name, route_mask), used for both the SELECT list and the Scan destinations, in IngestNewFromDB and IngestNewObservations. TestIngestSchemaFlagFlipBetweenQueryAndScan_158 flips each flag between query and Scan through a test-only hook, in both directions: 14 subcases, all red before, green after. This does not fix the startup race on hasResolvedPath (Load and LoadChunked still read the flags twice); that stays with ingestor resolves only ~half of relay paths (resolved_path NULL in ~53 % of observations); possible hasResolvedPath startup race #184.
3. Decode under the lock (5947bd6): resolved_path is decoded while the rows are scanned, before s.mu is taken (decodePersistedRelayPath, straight into []string; pinned against the old extractResolvedPubkeys(unmarshalResolvedPath(s))). Benchmarks, n=10, interleaved: total time per call is unchanged (IngestNewFromDB 4.58 → 4.55 ms, p=0.74, the work moved rather than disappeared), allocations −1.8 %, time spent holding s.mu in IngestNewFromDB 2.28 → 1.90 ms (−17 %, p<0.001; measured in an instrumented copy). Against baa68cd the totals stay at +12 % (IngestNewFromDB) and not significant (IngestNewObservations), so the per-call cost review measured is not reduced, only its lock-held part.
4. Stale comments (504b514): the two "resolved_path is NULL on every live deployment (since followup #1283: 13 remaining cachedRW write sites in cmd/server/ — schema migrations, backfills, neighbor persistence Kpa-clawbot/CoreScope#1287)" comments in Load and LoadChunked now match indexObservationRelayHops and resolved_path silently stopped being written since #1289 (~2026-05-21) — all v3.8.x affected Kpa-clawbot/CoreScope#1547.
5. NULL case for IngestNewObservations (f6321b8): TestLateObservationIndexesLikeLoad_NullResolvedPath_158.
6. PR description: "Behaviour change" now states the coverage (about 47 % of observations with a path have a non-null resolved_path, so about 53 % of relay traffic gets byNode credit only), that the change makes live and loaded numbers consistent at the ingestor's conservative level, and "Relates to ingestor resolves only ~half of relay paths (resolved_path NULL in ~53 % of observations); possible hasResolvedPath startup race #184".

Local checks: go test ./... in cmd/server and cmd/ingestor ok; go test -race -count=10 -run '_158|_115|IngestNew|LoadChunked|ScanAndMerge|1352|1643' ok, no races. CI on 504b514 (run 37110718094) is green: Go Build & Test, Playwright E2E and the Docker image build succeeded; release, badges and staging deploy are skipped for a PR. The final report for this round is the next comment.

---

Generated by Claude Code

[dborup]

Rapport — CS-Cloud PR#182 runde 2 — head 504b514

Status: Round 2 is done and CI is green on 504b5147837e24b6b30694075eda8dfd36d798ec. The PR is still a draft and waits for review.

Tags: [T] test or CI result, [A] analysis, [K] taken from the PR description or the earlier comment, not re-run for this report.

Head and commits

• Head before round 2: 9e3c5192. Head after: 504b5147. I checked the remote ref and the PR head; both are 504b5147.
• Round 2 is 4 commits on top of 9e3c5192:
  • f6321b8 test: late observation from another observer whose persisted resolved_path names a relay the first one lacked (mutant M2), plus the NULL case for IngestNewObservations
  • 9ac2b9b fix: one snapshot of the optional-column flags per ingest call, for the SELECT list and the Scan destinations
  • 5947bd6 perf: decode resolved_path before s.mu is taken on the live paths
  • 504b514 docs: the two stale resolved_path comments in Load and LoadChunked
• Identity: the six commits above the stacked base (baa68cdd) all have author and committer name dborup. The PR was opened by the dborup-agent account.
• Diff-stat round 2 (9e3c5192..504b5147): 5 files, +405/−34. store.go 126 lines changed, db.go +20, chunked_load.go 2, schema_snapshot_158_test.go +127 (new), sharegrowth_158_test.go +164. [A]
• Whole PR (GitHub's view against master): 10 commits, 10 files, +1610/−154. Above the stacked base baa68cdd: 6 files, +755/−105. [A]
• The production change is limited to cmd/server (store.go, db.go, chunked_load.go). The ingestor is not touched, and there is no new map[string]interface{}.

Tests

• Targeted run on 504b5147 with the race detector: go test -race -count=1 -run '_158|_115|IngestNew|LoadChunked|ScanAndMerge|1352|1643' in cmd/server: ok (13.5 s). [T] The PR text reports the same selection with -count=10; I re-ran it with -count=1 only.
• go test -count=1 -v -run _158: 17 top-level tests, all PASS, including TestTrafficShareLiveMatchesLoad_AmbiguousPrefix_158, TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158, TestLiveIngestIndexesLikeLoad_NullResolvedPath_158, TestLateObservationIndexesLikeLoad_NullResolvedPath_158, TestIngestSchemaFlagFlipBetweenQueryAndScan_158 and TestDecodePersistedRelayPath_158. [T]
• Full go test ./... for cmd/server and cmd/ingestor: green in CI (steps "Build and test Go server" and "Build and test Go ingestor", see below). [T] I did not run the full suites locally for this report; the earlier comment states they were green locally. [K]

Mutants

I re-ran four of the PR's mutants on a scratch copy of the head (go test -run _158). All are killed: [T]

Mutant	Killed by
IngestNewObservations ignores resolved_path (M2)	TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158 only. This is the test added in round 2; the earlier tests did not catch it.
IngestNewFromDB ignores resolved_path	TestTrafficShareLiveMatchesLoad_AmbiguousPrefix_158 and 5 more (..._158)
NULL resolved_path also feeds byPathHop	TestLiveIngestIndexesLikeLoad_NullResolvedPath_158, TestLateObservationIndexesLikeLoad_NullResolvedPath_158
An all-null persisted path is treated as NULL	TestDecodePersistedRelayPath_158

The schema-flag mutant (Scan re-reads the flags) is described in the PR text with 14 red subcases before the fix. I did not re-run it. [K]

Benchmarks [K]

Not re-run for this report. The PR description gives benchstat, n=10, interleaved runs:

• IngestNewFromDB_158: 4.58 → 4.55 ms per call (not significant), so the total time per call is unchanged by round 2. Time holding s.mu: 2.28 → 1.90 ms (−17 %, p<0.001), measured in an instrumented copy and not in committed code.
• IngestNewObservations_158: 1.94 → 1.93 ms, lock time not significant.
• Against the stacked base: about +12 % for IngestNewFromDB, not significant for IngestNewObservations.
• As the PR says, moving the decode out of the lock changes no behaviour, so no test fails if it is moved back. The benchmark is the only guard.

CI on 504b514 (run 37110718094) [T]

Job	Result
Go Build & Test	success (server, ingestor, channel library, schema packages, anomaly library with -race, CSS lint, JS unit tests, XSS gate and frontend lint all green)
Playwright E2E Tests	success (fail-fast suite and the slide-over flake-gate)
Build & Publish Docker Image	success (local build; the GHCR push steps are skipped for a PR)
Release Artifacts	skipped
Publish Badges & Summary	skipped
Deploy Staging	skipped

Not verified

• [K] Staging after deploy: the sum of traffic_share_score should stay roughly flat over a day and not drop on restart.
• [K] The ingestor's resolved_path = COALESCE(excluded.resolved_path, resolved_path) upsert can fill a row in later; live ingest reads the value present at the first poll, Load reads the later one.
• [K] Coverage is partial by design (about 47 % of observations with a path have a non-null resolved_path); that question and the startup race on hasResolvedPath stay with ingestor resolves only ~half of relay paths (resolved_path NULL in ~53 % of observations); possible hasResolvedPath startup race #184.
• [K] TestHandleNodePaths_FallbackPreconfirmed_1352 can fail with a 503 "index loading" under heavy CPU load, also on 9e3c5192. It is not touched here and it passed in CI on this head.

---

Generated by Claude Code

[adminopenclaw8-sketch]

Review — CS-Macmini re-review PR#182 runde 2 — head 504b514

Dom: APPROVE med nits

Independent re-review of round 2 (9e3c5192..504b5147: f6321b89, 9ac2b9b0, 5947bd65, 504b5147). Read-only: git fetch --no-prune, and every revision was unpacked with git archive into a scratch directory, never into the main checkout. No commits, no pushes, staging and prod untouched.

Tags: [T] test or measurement I ran myself, [A] analysis of code or diff, [K] not checked.

Head

• refs/pull/182/head was 504b5147837e24b6b30694075eda8dfd36d798ec before and after the review (git ls-remote). [T]
• master moved during the review: 751f8fc8 → 1dbe141e (fix(ingestor): assign explicit, collision-resistant MQTT client IDs #141 and fix(store): index a transmission once per relay key in byPathHop (traffic share inflation) #162 merged). baa68cdd is now an ancestor of master, so the stack dependency is satisfied. [T]

Blocking findings

None.

Round 1 findings

Item	Status	Basis
M2 caught (IngestNewObservations ignores resolved_path)	Yes. My M2 mutant is killed by TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158, and only by that test.	[T]
One flag snapshot for query and Scan, on both live paths	Yes. IngestNewFromDB and IngestNewObservations each call s.db.ingestCols() once and use it for the SELECT list and the Scan destinations. The only other flag read is isV3(), which selects one of two query texts and does not change the Scan count.	[A], with mutants M3a/M3b [T]
Decode moved out of s.mu without a behaviour change	Yes. decodePersistedRelayPath returns nil hops in exactly the cases where unmarshalResolvedPath does: an empty string, any json.Unmarshal error (including a type mismatch such as [1,"…"]) and top-level null. null and "" elements are dropped exactly as extractResolvedPubkeys drops them, and present keeps an all-null path separate from a NULL column. The pubkeys slice is not retained (addResolvedPubkeysToPathHopIndex stores only hashes and tx). r.resolvedPath is used only in indexObservationRelayHops. Load and scanAndMergeChunk decode under the lock as before, which is correct because they hold it for the whole load anyway.	[A]; oracle test and mutants M4a–c [T]
Comments correct	Yes. The Load and loadChunk comments now match indexObservationRelayHops and Kpa-clawbot#1547. The ingestCols comment says "main.go's forceTrue", and that call exists (main.go:292).	[A]
NULL test for IngestNewObservations	Yes: TestLateObservationIndexesLikeLoad_NullResolvedPath_158 kills M5.	[T]
PR text mentions the ~53 %	Yes, under "Behaviour change": ~47 % non-null, ~53 % of relay traffic gets byNode credit only.	[A]

Tests I ran

• cd cmd/server && go test -count=1 ./...: ok (34 s). cd cmd/ingestor && go test -count=1 ./...: ok (95 s). [T]
• go test -race -count=10 -timeout 45m -run '_158|_115|IngestNew|LoadChunked|ScanAndMerge|1352|1643' . in cmd/server: ok, 48 matching tests × 10, no DATA RACE (58 s). [T]
• Merge: git merge-tree --write-tree against 751f8fc8 and against the new master 1dbe141e is clean in both cases. The merged tree differs from 1dbe141e by exactly fix(store): index live observations from the persisted resolved_path, as Load does (#158) #182's own delta (6 files, +755/−105), and cmd/server go test ./... on the merged tree is ok. [T]
• deploy.yml: github.repository == 'Kpa-clawbot/CoreScope' occurs 9 times on baa68cdd, 9e3c5192, 504b5147, 751f8fc8 and 1dbe141e, and .github/ is unchanged baa68cdd..504b5147. [T]
• CI on 504b5147 (run 37110718094): Go Build & Test, Playwright E2E and the Docker build pass; release, badges and staging deploy are skipped for a PR. [T] (read from gh pr checks)

Mutants (my own, in the delta)

Each mutant was run as the full cmd/server package in a scratch copy. An unmutated copy with the same layout fails 24 tests that read files outside cmd/server (workflow, compose and server-process tests). Those failures are environmental, so I subtracted them; the list below is the remainder. [T]

#	Mutant	Killed by
M2	IngestNewObservations treats every resolved_path as NULL	TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158
M3a	IngestNewFromDB Scan re-reads all four flags (flag read twice again)	TestIngestSchemaFlagFlipBetweenQueryAndScan_158
M3b	IngestNewObservations Scan re-reads hasResolvedPath()	TestIngestSchemaFlagFlipBetweenQueryAndScan_158
M3c	IngestNewFromDB builds the SELECT from a fresh hasResolvedPath() read, Scan uses the snapshot	survives (see nit 3)
M4a	decode: corrupt JSON treated as NULL (path_json fallback)	TestDecodePersistedRelayPath_158
M4b	decode: ""/null hops kept	TestDecodePersistedRelayPath_158
M4c	decode: all-null path treated as NULL	TestDecodePersistedRelayPath_158
M5	NULL branch of indexObservationRelayHops also feeds byPathHop and the resolved index	TestLiveIngestIndexesLikeLoad_NullResolvedPath_158, TestLateObservationIndexesLikeLoad_NullResolvedPath_158, TestHandleNodePaths_FallbackUniquePrefix_1352, TestNodeActivity_AlternateObservationRelaySurvivesRestart
M6	IngestNewObservations stops selecting o.resolved_path but still scans it	12 tests, including TestIngestNewObservations, TestIngestSchemaFlagFlipBetweenQueryAndScan_158 and TestTrafficShareLiveMatchesLoad_LateObserverNewRelay_158

Benchmarks

Setup: Apple M4. One test binary per revision, all with the same sharegrowth_158_test.go and pathhop_dedupe_158_test.go from 504b5147 (TestDecodePersistedRelayPath_158 removed on the two older revisions because it needs the new symbols; the benchmark bodies are byte-identical). Lock time comes from an instrumented copy that times s.mu.Lock() until the deferred unlock in both live functions, with the same instrumentation on all three revisions. Rounds were interleaved with the order rotated each round. I report medians; p is Mann-Whitney U (exact for n=8, normal approximation for n=30). No benchstat on this machine. [T]

Auto benchtime, n=8:

Benchmark	baa68cd	9e3c519	504b514	504b vs base	504b vs 9e3c
IngestNewFromDB_158, time	1.715 ms	1.832 ms	1.828 ms	+6.6 % (p<0.001)	−0.2 % (p=0.96)
IngestNewFromDB_158, allocs	23.7 k	26.3 k	25.8 k	+8.9 %	−1.8 % (p<0.001)
IngestNewFromDB_158, time holding s.mu	1.056 ms	1.100 ms	1.030 ms	−2.5 % (p<0.001)	−6.4 % (p<0.001)
IngestNewObservations_158, time	164 µs	195 µs	206 µs	+25.8 %	+5.7 % (p<0.001)
IngestNewObservations_158, allocs	3,846	4,641	4,845	+26.0 %	+4.4 %
TrafficShareScoreMap_158 (4 subcases)				within ±2.2 %, p≥0.38	within ±5 %, p≥0.08; allocs identical

IngestNewObservations_158 with -benchtime=400x, n=30 (explained under nit 2):

	baa68cd	9e3c519	504b514	504b vs base	504b vs 9e3c
time	1.088 ms	1.109 ms	1.106 ms	+1.7 % (p<0.001)	−0.3 % (p=0.68)
time holding s.mu	1.032 ms	1.045 ms	1.035 ms	+0.2 % (p=0.16)	−1.0 % (p=0.008)
allocs	31.6 k	31.9 k	31.8 k	+0.6 %	−0.2 %

The PR's claim "lock time −17 %, total about +12 % against baa68cd" holds in direction but not in size on this machine. Round 2 does shrink IngestNewFromDB's lock-held time without changing its total, but by 6.4 %, not 17 %, and the total is +6.6 % against the base, not +12 %. The allocation figure (−1.8 % vs 9e3c5192) matches exactly. Absolute costs stay negligible at staging rates. [T]

Pre-existing flake: TestHandleNodePaths_FallbackPreconfirmed_1352

Confirmed as pre-existing and not caused by #182. [T]

• With -test.cpu 1 it fails deterministically with 503 {"error":"index loading"}: 300/300 on baa68cdd, 9e3c5192 and 504b5147, and 100/100 on master 1dbe141e. With the default GOMAXPROCS: 0/300 on master. All four TestHandleNodePaths_*_1352 tests fail 50/50 under -cpu 1 on baa68cdd and on master.
• Cause [A]: Load() starts the path-hop index build in a goroutine (perf: move post-load index builds to background goroutines (startup Q1) Kpa-clawbot/CoreScope#1008). /api/nodes/{pk}/paths is hard-gated on PathHopIndexReady(), but reloadStore (and the inline store in FallbackUniquePrefix_1352) never calls WaitIndexesReady. fix(store): index live observations from the persisted resolved_path, as Load does (#158) #182 does not touch buildPathHopIndex, index_ready_1008.go, routes.go or that test file.
• Experiment [T]: adding store.WaitIndexesReady(5*time.Second) to reloadStore makes FallbackPreconfirmed_1352 pass 300/300 under -cpu 1. FallbackUniquePrefix_1352 still fails because it builds its own store, so it needs the same wait. I found no existing issue for this, and it is worth a separate one.

Nits (non-blocking)

1. Benchmark numbers in the PR text. Please note that the −17 % and +12 % are machine-dependent. On an M4 they come out at −6.4 % and +6.6 % (table above). [T]
2. Rows that are skipped get decoded anyway in IngestNewObservations. The decode now runs for every scanned row, but under the lock rows are still dropped when the observation is already in byObsID or when observer+path is a duplicate. In production pollStore re-reads all observations since lastObsID right after IngestNewFromDB has ingested the new transmissions' observations, so a large share of these rows are skipped, and for this function the move adds decode work instead of moving it. The cost is small (one short JSON decode per row, outside the lock), so accepting it is fine; the alternative is to keep the raw string and decode after the dedup checks in this one function. Related: BenchmarkIngestNewObservations_158 (from fix(store): index a transmission once per relay key in byPathHop (traffic share inflation) #162) rotates over 511 observers, so with an auto b.N in the thousands most iterations hit only the dedup path. That explains the auto-run +5.7 % against the 400x run's −0.3 %, and why the result depends on the machine's b.N. [A] + [T]
3. Surviving mutant M3c. ingestAfterQueryHook runs after the query, so a regression that builds the SELECT from a fresh flag read while Scan uses the snapshot is not caught. The window is narrow. A hook between ingestCols() and query construction would close it, and it is optional. [T]
4. Trivial: the caller contract of indexResolvedPathHops still says pks "should be the output of extractResolvedPubkeys"; it is now also the output of decodePersistedRelayPath. [A]

Not verified

• [K] Staging behaviour after deploy (the traffic-share sum staying flat across a day and across a restart).
• [K] The upsert window (COALESCE(excluded.resolved_path, resolved_path) filled in after the first poll), the ~47/53 % coverage figure and the hasResolvedPath startup race. I took these from the PR; they belong to ingestor resolves only ~half of relay paths (resolved_path NULL in ~53 % of observations); possible hasResolvedPath startup race #184.
• [K] Playwright and the frontend were not run locally. CI shows them green.

---

Generated by Claude Code

[dborup]

Rapport — CS-cloud PR#182 runde 3 — head 287e853

Status: Round-2 nits handled (1, 3, 4 fixed; 2 left with reasons), branch merged up to master, all local tests and CI green.

Evidence tags: [T] test or CI run, [A] analysis, [K] taken from earlier and not rerun.

Merge with master

• 1459075a: merge of origin/master 8e4b13d0 into the branch. It was a merge commit, with no rebase and no force-push. The merge was clean. [T]
• The merged tree differs from 8e4b13d0 by exactly this PR's own delta: 6 files, +755/−105, the same as the round-2 review measured against the older master. [T]
• go test -count=1 -run '_158|ResolvedPath|TrafficShare' . on the merged tree, before any further change: ok. [T]

Review nits

#	Nit	Result	Commit
1	Benchmark numbers in the PR text	Fixed. The Performance section now says the percentages depend on the machine, quotes the Apple M4 results from the review (lock-held time −6.4 % instead of −17 %, total +6.6 % instead of +12 %), and explains why IngestNewObservations_158 depends on the iteration count (auto benchtime against 400x).	PR text (edited via the API)
2	IngestNewObservations decodes rows that are skipped later	Not changed. The rows are skipped by checks that need s.mu (byObsID, byTxID, the observer+path key in tx.obsKeys, which also changes as earlier rows in the same batch are merged). Decoding only the surviving rows would mean running those checks before the decode. That needs either a second pass under the lock, whose answer can be stale by the time the write lock is taken, or the decode under the lock, which the nit rules out. That is not simple, so the small cost (one short JSON decode per scanned row, outside the lock) is accepted. The PR text says so under "Not verified". No benchmark was run because there is no code change. [A]	—
3	Mutant M3c (SELECT built from a fresh flag read) survives	Fixed. New test-only hook ingestAfterColsHook runs right after the ingestCols() snapshot in both live functions. TestIngestSchemaFlagFlipBetweenQueryAndScan_158 now flips each flag at both points (after_cols, after_query): 28 subtests. M3c (in IngestNewFromDB) and the mirrored M3d (in IngestNewObservations) are each killed by the two …/resolved_path/after_cols/… subtests of their function. [T]	287e8534
4	Caller contract of indexResolvedPathHops	Fixed. pks may be the output of extractResolvedPubkeys or the pubkeys of decodePersistedRelayPath; both drop nils and empties. [A]	287e8534

Commit identity of 1459075a and 287e8534: dborup <kontakt@meshview.dk> (checked with git log -1 --format='%an <%ae>'). [T]

Tests

On head 287e8534, in cmd/server unless noted:

• go test -race -count=3 -run '_158|ResolvedPath|TrafficShare' .: ok (22.4 s). [T]
• go test -count=1 ./...: ok (65.9 s). [T]
• cd cmd/ingestor && go test -count=1 ./...: ok (137.5 s). [T]
• go vet ./...: clean. gofmt -l on the two changed files: clean. [T]
• No flake was seen in these runs, so none is called baseline. The _1352 "index loading" 503 under -cpu 1 is described in the round-2 review and was not rerun here. [K]

CI on 287e8534 (run 37127659476, CI/CD Pipeline, pull_request)

Job	Result
Go Build & Test	success
Playwright E2E Tests	success
Build & Publish Docker Image	success
Release Artifacts	skipped (not run for a PR)
Deploy Staging	skipped (not run for a PR)
Publish Badges & Summary	skipped (not run for a PR)

[T] (read per job from the Actions API)

Not verified

• [K] Staging behaviour after deploy, as in rounds 1 and 2.
• [A] Benchmarks were not rerun this round: there is no production code change apart from the test hook, which is a nil check on each call.

---

Generated by Claude Code