[dborup]

Summary

Batch both stages of canonical resolved-path lookup for the node Paths endpoint, adapted to this fork's existing immutable observation snapshots and expiring LRU.

• First stage fetches missing longest-observation paths in batches of at most 500 IDs.
• Fallback fetches sibling paths in batches of at most 500 transmission IDs, then chooses only observations captured in the original snapshot, preserving first-in-snapshot tie order.
• Ignore out-of-snapshot historical rows before JSON decoding/retention. They are still read from SQL; this is not a claim that their scan cost disappears.
• Preserve nil/empty/malformed path behavior, the strict LRU expiry boundary, monotonic request clock, exact-pubkey confirmation, collision guards and scalar recovery after failed batches.
• SQL runs outside both the packet-store and LRU locks. The server remains read-only.

Base: 8798f74eeec124f21d0e82a8d64b93b31d7facc0. Three server files only. No schema, config, new persistent cache, dependency or workflow changes. There is overlapping work in #529; synchronize with current master normally before any eventual merge rather than rewriting that branch.

Correctness / independent review

• Parent and independent reviewer inspected immutable trees: APPROVE, no blockers. A review note prompted filtering before decode and an explicit non-empty-result assertion in the allocation regression.
• Actual SQLite query counter: 1,001 transmissions require 3 queries instead of 1,001 on the normal cold path, and 6 instead of 2,002 on fallback. A real HTTP handler test confirms the endpoint uses batches, not just the helper.
• Tests cover legacy NULL/empty/malformed JSON, non-nil empty arrays, snapshot and fallback ties, rows outside the snapshot, warm LRU, exact TTL equality, same-row overwrite after expiry, missing DB, query/scan/row-iteration errors and recovery without poisoning cache state.
• Five behavioral mutants fail their intended assertions: restore the handler's N+1 loop, oversize SQL chunks, reverse tie selection, expire at equality, and decode ignored historical paths. Unmutated scratch tests pass. The first tests-only compile failure for an absent helper is not presented as the behavioral mutation proof.
• Full server suite: pass (104.259 s); full ingestor baseline/candidate check: pass (121.253 s); vet passes.
• Focused race suite, including existing Paths/LRU contracts: pass. Parent independently reran the final new tests under race detection, with non-empty execution (4.586 s).
• Parent actual-browser smoke on the final candidate: 154 pass, 3 fixture-dependent skips, 0 failures. The unchanged frontend/script tree was also fully exercised on the separate background-neighbor port (260 files pass); on this tree the actual Go assignment validator and CI gate checks pass (2,737 entries assigned once; 57 + 33 gate cases).

Measured performance (local synthetic SQLite, cold LRU)

These are repeated before/after runs on the same fixture, not deployment measurements:

Workload	Scalar	Batched
30,000 transmissions × 5 observations, primary present	96–113 ms; 25.9 MB; 750k allocations	48–51 ms; 19.7 MB; 481k allocations
Same workload, sibling fallback	385–414 ms; 79.5 MB; 2.61M allocations	180–211 ms; 74.7 MB; 2.22M allocations
100 transmissions × 200 historical out-of-snapshot rows	77–104 ms; 71.6 MB; 1.565M allocations	13.7–14.7 ms; 25.5 MB; 162k allocations

MB values are cumulative allocated bytes per operation, not retained heap, peak memory or RSS. A separate 800-historical-row regression requires fewer than half the scalar allocations and verifies all expected output paths. Temporary storage still scales with valid snapshot observation fanout within the 500-transmission batch; no production peak-memory claim is made.

Scope / limits

Batch-query failures intentionally fall back to the legacy scalar path, so the small query count is not promised during SQL errors. Other endpoints and deferred no-canonical-path confirmation are not universally batched by this change. Local tests use macOS arm64 / Go 1.26.3 and disposable fixtures; Linux PR CI remains separate.

Draft only, auto-merge off; no ready, merge, deployment, GHCR publishing or live-host access.

Post-push Chromium confirmation on the published commit: Nodes list/detail and real neighbor/Paths API contracts pass on a disposable local fixture. Initial PR CI run: 40067050026; no rerun was requested.

Source credit / discovery

Adapted from the original contribution by efiten. This fork lacked the source's earlier batch helper, so both stages were adapted, not blindly cherry-picked:

Source repository: Kpa-clawbot/CoreScope
Pull request number: 2224
Head commit: 0c627c5563b7bc313acdfcd6774904392bf14a36
Merge commit: abbe19531d538f9afa0b0282dd949941da412113

[adminopenclaw8-sketch]

Review — CS-Macmini PR#542 — head d82ab0a

Dom: APPROVE med nits

Independent, read-only review. git ls-remote showed the same head before and after (d82ab0a0), and master at 4b0e77a8 throughout. Work was done on git archive copies of the head tree and of the merged tree (git merge-tree --write-tree origin/master d82ab0a0 → 77a5fd52, no conflicts), never in another checkout.

Findings

#	Sev	Where	Finding
N1	nit	cmd/server/resolved_index.go (both stages)	LRU eviction sequence is not identical to before. lruPut appends to lruOrder and eviction pops its front, so the insertion sequence is the FIFO eviction order. The scalar loop inserted in handler candidate order; the batched helper inserts in sorted-observation-id order. Same entry set, same values, same hit/miss decisions — but when one request inserts past lruMaxSize (10000) a different subset survives. No test pins it.
N2	nit	resolved_index.go, stage 1 rp, present := stored[id]	The rule that an absent row or a NULL resolved_path must not enter the LRU (scalar returns early before lruPut) is unpinned: dropping the present guard and caching nil unconditionally survives the entire cmd/server suite.
N3	nit	resolved_index.go, sort.Ints(missing) / sort.Ints(txIDs)	Removing both sorts survives every test. The determinism of chunk membership they buy is unverified, so a later refactor can drop them silently.
N4	nit	resolved_index.go, longestTx[longest.ID] = txID	Defensive only: if two snapshots ever shared a longest observation id, longestTx keeps the last writer and the id lands in missing twice, so the chunk loop would resolve it twice for the same tx and the other tx would get neither a result nor a fallback entry. Not reachable through ingest (each StoreObs belongs to one StoreTx), so no behavioural claim — a seen set or map[int][]int would make it structurally impossible.
N5	nit	resolved_index.go, chunk loops	lruMu is acquired and released once per id inside a chunk (up to 500 lock round-trips per batch) rather than once per chunk. Not a regression — the scalar path paid the same per candidate — but it is the remaining obvious win now that the SQL is batched.
I1	info	/api/nodes/{pk}/paths	Response ordering for some nodes is nondeterministic on master too (see "Behavioural equivalence"). Not introduced here.

No blockers. No behaviour change beyond the stated purpose was found.

Answers to the review points

#495 lock rule — no SQL under PacketStore.mu. Holds. handleNodePaths calls the new helper after s.store.mu.RUnlock() (routes.go:2419 unlock, call at :2463) [K]. TestNoSQLUnderStoreLock, TestNoSQLUnderStoreLockDetects and TestStoreSQLDoesNotHoldPacketLock412 are green on the merged tree [T]. The AST guard genuinely covers the new code, not just the old helpers: planting _ = s.loadCanonicalResolvedPathsAt(nil, 0) inside an existing s.mu.RLock() region fails the guard by name, transitively — SQL under store.mu: store.go:1718:6 PacketStore.loadBackgroundChunks calls PacketStore.loadCanonicalResolvedPathsAt [T]. Both lruMu sections are opened only after the store lock is gone, so the documented s.mu → s.lruMu ordering is respected [K].

cmd/server stays read-only. Yes. The server handle is opened mode=ro (db.go:211) [K]; the two new statements are both SELECT and the diff adds no INSERT/UPDATE/DELETE/CREATE/DROP/REPLACE/PRAGMA at all [K]; the read-only guard tests pass [T].

Hard upper bound on batching. Yes. resolvedPathBatchSize = 500, and both stages slice [start : min(start+500, len)], so the IN list carries at most 500 bound variables — under the legacy 999 ceiling and far under the modern 32766 [K]. The bound is enforced by a test, not only by the constant: raising the constant to 1000 is caught [T] (M3 below). strings.Repeat(",?", len(chunk)-1) cannot see an empty chunk, since the loop condition guarantees len(chunk) ≥ 1 [K].

LRU semantics identical. Identical for everything observable per request, with the one exception in N1:

• longest-observation pick keeps the strict > so the first observation wins a tie [K], and the tie rule is pinned (M4 below) [T];
• cache hit, non-nil → returned without re-lruPut, exactly like fetchResolvedPathForObsAt's early return [K];
• cache hit, nil → falls through to the sibling stage, matching bestResolvedPathAt's if rp != nil [K];
• row present (raw.Valid, including "", null and malformed JSON → nil) → lruPut; row absent or NULL → no lruPut. This is precisely the scalar rule [K], though unpinned (N2);
• TTL is still strict (now - storedAt > TTL), equality is still a hit, and the request clock is still read once [K/T];
• []*string{} (stored []) stays distinguishable from nil [T].
  TestCanonicalPaths2224MatchesBestAt compares the batched result against fetchResolvedPathForTxBestAt on the same fixture across 11 shapes (NULL, "", malformed, null, [], nil entries, snapshot ties, fallback ties, nothing stored, malformed path_json, out-of-snapshot sibling), which is the right way to assert equivalence [T].

Errors in one batch do not poison the cache. Confirmed. A failed query, a rows.Err() and a mid-iteration Scan failure all drop that chunk to the per-tx scalar path and write nothing to the LRU from the failed batch [K]; the fault-injection driver exercises all four modes and asserts full scalar recovery [T]. The "failed batch then clean retry" sequence in TestCanonicalPaths2224LRUAndErrors is green [T]. Note the deliberate asymmetry: stage 2 checks rows.Err() where the scalar fetchResolvedPathsForTx does not, and breaks on a Scan error where the scalar skips the row — both make the batched path stricter, and both land in scalar recovery, so neither can lose a path [K].

Perf claim — reproduced. Measured independently on the merged tree, -benchtime 3x -benchmem, macOS arm64 / Go 1.27.0 (the author used 1.26.3), five observations per transmission, cold LRU [T]:

Workload	Scalar	Batched	Ratio
30,000 tx, primary present	94.0 ms, 21.6 MB, 660k allocs	44.6 ms, 14.9 MB, 391k allocs	2.1×
30,000 tx, sibling fallback	312 ms, 61.2 MB, 2.25M allocs	165 ms, 55.5 MB, 1.86M allocs	1.9×
100 tx × 200 out-of-snapshot rows	54.9 ms, 59.0 MB, 925k allocs	12.3 ms, 25.6 MB, 162k allocs	4.5×
1,001 tx, primary present	2.93 ms, 1.46 MB, 21.8k allocs	1.41 ms, 1.23 MB, 12.7k allocs	2.1×

The ratios match the PR body; absolute numbers differ slightly (my scalar baseline is faster), consistent with the different Go version. The query-count claim also reproduces: 1,001 transmissions take 3 queries instead of 1,001, and 6 instead of 2,002 on the fallback arm [T]. As the PR body says, MB here is cumulative allocated bytes per op, not peak or retained heap, and stage 2 still holds the decoded in-snapshot paths for up to 500 transmissions at once — I did not measure peak RSS.

Overlap with the in-flight paths/store work. No conflict. git merge-tree --write-tree origin/master d82ab0a0 produces a tree with no conflict markers and exit 0 [T]. #529/#413 (subpath on-demand) and #540/#541 (background neighbour graph) are already in master at 4b0e77a8, and the merged tree carries both plus this change; it builds, vets clean and every suite below is green on it [T]. The PR body's note about synchronising with master rather than rewriting the branch is the right call and is already satisfied by the current head.

Behavioural equivalence on real data. I built a baseline server from origin/master and one from the merged tree and queried /api/nodes/{pk}/paths for all 204 fixture nodes against the same prepared e2e-fixture.db (ports 13800/13802) [T]. 200 of 204 were byte-identical. The 4 that differed turned out to be pre-existing ordering nondeterminism, not a regression: repeating the same request against the master server alone yields 2–3 distinct byte responses out of 8, and for all four nodes the order-insensitive canonical form is a single value that is identical between master and head. So content is unchanged; only the (already unstable) ordering moves. That is I1, and it is outside this PR's scope.

Tests run

All on the merged tree (origin/master 4b0e77a8 + head d82ab0a0), macOS arm64, Go 1.27.0:

• go vet ./... in cmd/server — clean [T]
• go test ./... in cmd/server — ok 64.2s [T]
• go test ./... in cmd/ingestor — ok 107.2s [T]
• go test -race over the new tests plus the paths/LRU/lock/read-only contracts — 48 tests, ok 6.1s, no races [T]
• sh test-all.sh — 262 passed, 0 failed (262 files) [T]
• node test-frontend-helpers.js — 719 passed, 0 failed [T]
• E2E against a local Go server on port 13800 with e2e-fixture.db prepared as CI does (freshen, the Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486/"Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791 seed SQL, corescope-migrate, then seeds 2073, 199, 245 and 122): test-e2e-playwright.js 154 passed / 3 skipped / 0 failed, test-path-inspector-e2e.js 7/0, test-issue-1146-path-link-contrast-e2e.js 11/0, test-issue-1281-location-row-e2e.js 6/0, test-issue-1206-resize-observer-leak-e2e.js 28/0 [T]. (.git-commit had to be written by hand for the local run; test-issue-1281 needs CHROMIUM_PATH on macOS.) Both servers were stopped by port lookup.
• CI on the head commit: all 15 non-skipped checks green, including the three Go server shards, the three Playwright shards, the Go test-assignment validator and the E2E shard/coverage gate [T]. Neither known flake (flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256 Hash Stats sort, flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267 backfill write-hold) appeared.

Red-before-green: this PR has no issue, so the acceptance criteria are the PR's own. The new tests cannot compile against master (the helper does not exist), which the author correctly declines to present as behavioural proof — so the binding evidence is mutation, below. M5 is literally master's handler restored, and it is red.

My own mutants

Six, run on a private copy of the merged tree; resolved_index.go / routes.go / store.go restored between each.

#	Mutation	Result
M1	Call loadCanonicalResolvedPathsAt inside an existing s.mu.RLock() region	killed — TestNoSQLUnderStoreLock: SQL under store.mu: store.go:1718:6 PacketStore.loadBackgroundChunks calls PacketStore.loadCanonicalResolvedPathsAt
M2	Delete the out-of-snapshot filter (wanted / owner != txID) in stage 2	killed — TestCanonicalPaths2224HistoricalRowsNotDecoded: historical paths still decoded: 37292 allocations, scalar 37698 (want < half). Output stays correct, so only the allocation guard catches it — it earns its place.
M3	resolvedPathBatchSize 500 → 1000	killed — TestCanonicalPaths2224BatchCount: 1001 candidates: 2 SQL queries, want 3 batches (and 4 vs 6 on the fallback arm)
M4	Stage-1 longest pick > → >= (reverse the snapshot tie rule)	killed — .../snapshot_tie_order: batch=first-id-second, scalar=second-id-first
M5	Restore the N+1 loop in handleNodePaths (= master's handler)	killed — TestNodePaths2224UsesBatches: handler still uses per-transmission SQL: SELECT resolved_path FROM observations WHERE id = ?. The handler-level assertion is binding, not just the helper's.
M6	Drop the present guard: lruPut(id, rp) for absent/NULL rows too	SURVIVES the full cmd/server suite (ok 65.1s) → N2
M7	Remove both sort.Ints calls	SURVIVES → N3

For N1 I also ran a throwaway probe comparing lruOrder after the scalar loop against the batched helper, with candidate order [5 1 4 2 3]: scalar leaves [9 1 7 3 5], batched leaves [1 3 5 7 9] — same set, different FIFO eviction sequence [T]. The scratch file was deleted; nothing was left behind.

Repo conventions

• Commit author and committer are both dborup <kontakt@meshview.dk>; one commit, perf(paths): batch canonical resolved-path lookups [T]
• No closing keywords in the diff or the PR body [T]
• Fork guards: 9 in deploy.yml, 1 in release-fast-path.yml, unchanged on head and on the merged tree [T]
• scripts/check-xss-sinks.sh --diff origin/master — no public/**/*.{js,html} changes to scan [T]
• No hardcoded colors: the diff touches no CSS or frontend JS [K]
• No new map[string]interface{} outside tests. The new []interface{} SQL argument slices match the existing chunked-IN idiom in the same file (resolvedPubkeysForEvictionBatch, resolved_index.go:667) [K]
• Three server files, no schema, config, dependency or workflow change [K]

Not verified

• No staging or production access, no server API key, no live host, nothing pushed and nothing changed on the PR. Everything ran against disposable local fixtures.
• Linux. All local runs were macOS arm64 / Go 1.27.0; the Linux evidence is the head commit's own CI.
• N1 at real scale: I did not drive a single /paths request past lruMaxSize (10000) candidates to observe which entries actually survive, so the changed eviction subset is a code-level conclusion, not a measured one.
• Peak memory / RSS. Only -benchmem cumulative allocation, matching the PR body's own framing. Stage 2's temporaries for up to 500 transmissions of in-snapshot fanout were not measured.
• Concurrency at scale: no convoy or load test with many simultaneous /paths requests at 30K transmissions; only -race on the focused suite.
• The 32766 variable limit is never approached, so that path is untested by construction.
• Other endpoints that still resolve canonical paths one at a time are out of scope here and were not re-measured.