fix(store): accurate trackedBytes for paths, caches and resolved relay hops (#113, #164)

[dborup]

Relates to #113, #164

trackedBytes drives packetStore.maxMemoryMB eviction. It undercounted the store (to about 0.5-0.6 of the real heap on a synthetic store), so eviction fired late, and it drifted downward as the store ran. This PR makes it exact for what it charges, charges what it missed, and removes exactly what it charged. Server only; no ingestor, schema or frontend change.

Plan and design

1. Charge record. StoreTx.chargedBytes records what a tx was charged. rechargeTx(tx) returns the delta against a fresh estimate and runs at creation and after every pickBestObservation (Load, loadChunk, LoadChunked, the background pass in main.go, IngestNewFromDB, IngestNewObservations: all six pick sites).
2. Resolved relays (fix(store): memory estimate ignores resolved byPathHop entries and the #162 pathHopResolved side map #164). They arrive after insertion, so they are not in chargedBytes. Their charge is resolvedRelayBytes(len(pathHopResolved[tx])), a function of the side-map record length only:
   • addResolvedPubkeysToPathHopIndex charges the growth (O(1) per appended key);
   • retainResolvedPathHops credits the records it drops (non-live txs, or clear when there is no previous index);
   • eviction derives the same figure back from the record length.
3. Eviction subtracts the charge, in candidate selection and in the removal pass: txChargedBytes(tx) = chargedBytes + resolvedRelayBytes(len(record)). It never re-estimates. The eviction order is unchanged.
4. New estimate terms: the decode cache (4x DecodedJSON, charged up front), the obsKeys dedup key, the observation strings, and a resolved-relay allowance. estimateStoreTxBytesTypical (the cold-load budget) and both docs follow.

The invariant, asserted after Load, after eviction and after every step of a mixed sequence:

trackedBytes == sum over live tx of
    chargedBytes + resolvedRelayBytes(len(pathHopResolved[tx])) + sum of observation charges

with chargedBytes == estimateStoreTxBytes(tx) for every live tx (no stale charge).

StoreTx layout contract: unchanged, 320 bytes

StoreTx has no spare padding (320 = 40 x 8, fields are packed). A per-tx charge field therefore needs bytes from somewhere:

• UniqueObserverCount goes from int to int32 (8 uses, one read, which feeds a JSON number);
• the freed 4 bytes hold chargedBytes uint32 (a tx costs a few KB, and rechargeTx clamps at MaxUint32).

TestStoreTxLayoutFitsRouteMaskInPadding and TestStoreTxLayoutFitsObservedPathHashSizeMaskInPadding pass unchanged.

Differences from upstream

Idea and the decode-cache and dedup-key terms are from Kpa-clawbot/CoreScope PR 2035 (commit e6323ec5), not cherry-picked. What differs, and why:

	Upstream 2035	This PR
Per-tx charge field	accountedBytes int64 (grows StoreTx by 8 bytes)	chargedBytes uint32 in the bytes freed by narrowing UniqueObserverCount; the fork pins 320 bytes
Resolved relay hops	not covered	charged per new key, credited by rebuilds, derived from the side map at eviction (#164)
Rebuild (retainResolvedPathHops)	n/a	credits dropped records; tested, including a record of a tx outside the store
Recharge sites	six pick sites	the same six on this fork, including the loadChunk local batch and the main.go background pass
Eviction candidate selection	uses the charge	same
Constants	calibrated on one production copy	decode cache kept at 4x; resolved allowance (140 B per record, 96 B per key) calibrated on this branch at 100k and 500k tx
Tests	one path-then-evict test	invariant checks over Load, eviction, a 60-step mixed sequence, rebuilds and memory passes

Tests, red before and green after

Fifteen new tests in tracked_bytes_accounting_113_test.go, built on a SQLite store, plus a gated measurement test. Eleven load it through Load(); two go through RunStartupLoad (LoadChunked, and with a hot window the loadChunk background fill), which is how production starts (round 2); two pin the estimate terms. Commit 1 holds the tests and inert scaffolding (the field, and two stubs that return 0), so they fail on assertions: for example after Load: 400 of 400 tx carry a stale charge, trackedBytes = 697012, charges sum to 308000, and trackedBytes is 0.65 of the real heap growth. The fix is commit 2.

Test	What it pins
TestTrackedBytes_LoadChargesPathAndResolvedRelays_113	invariant after Load
TestTrackedBytes_EvictionSubtractsExactlyWhatWasCharged_113	partial and full eviction, exact zero at the end
TestTrackedBytes_EvictionIgnoresAFreshEstimate_113	a stale estimate does not leak into eviction
TestTrackedBytes_RechargeCoversALaterPath_113	delta on a longer path; second recharge is 0
TestTrackedBytes_ResolvedRelaysAreChargedIncrementally_164	per-key delta, repeat observations cost 0, rebuild neutral, rebuild with no previous index credits
TestTrackedBytes_RebuildCreditsRecordsOfTxNotInTheStore_164	the loadChunk window
TestTrackedBytes_MixedSequenceDoesNotDrift_113	ingest, extra observations, resolved relays, eviction; invariant every step; exactly 0 when emptied
TestMemoryEviction_CrossesHighAndReachesLow_113	a pass starts above the high and ends at or below the low watermark
TestEvictionCandidatesMatchWhatIsEvicted_113	candidates equal the evicted set
TestEstimateStoreTxBytesTypical_TracksTheRealCharge_113	cold-load estimate within 10% of the charge of a typical tx
TestTrackedBytesTracksTheHeap_113	20k tx: trackedBytes within 0.85-1.15 of the heap growth (master 0.65)
TestTrackedBytes_LoadChunkedChargesPathAndResolvedRelays_113	invariant after the production startup load (LoadChunked)
TestTrackedBytes_BackgroundFillChargesPathAndResolvedRelays_113	hot window through LoadChunked, the rest through loadChunk; invariant, then eviction of the filled half
TestEstimateStoreObsBytes_CountsEveryTerm_113	each new observation string moves the estimate by its length (twice for observer ID and path: field plus dedup key)
TestResolvedRelayBytes_AllowanceHasARecordAndAKeyPart_164	record and key parts of the allowance, and their physical floors (16 B per key, 32 B per record)

Results on the merged tree (this branch plus master 6f7a5e7c, which includes #182 and #194):

• cmd/server: go test -race -count=1 ./... passed once in full on the merged tree (374 s, round 1); after round 2, go test ./... passes and the targeted accounting, estimate, layout and eviction tests pass with -race -count=3.
• cmd/ingestor: go test ./... passes (unchanged by this PR).
• go vet ./... is clean; gofmt -l is clean for every file this PR adds or changes (eviction_test.go was already unformatted on master and is only touched on three lines).
• sh test-all.sh: 201 passed, 0 failed (201 files).
• No new map[string]interface{}, cmd/server stays read-only, deploy.yml is untouched.

Mutants (each in a copy of the branch, targeted tests)

Mutant	Caught by
No recharge after IngestNewObservations picks a path	TestTrackedBytes_MixedSequenceDoesNotDrift_113
No recharge after Load's pick	7 tests, incl. …LoadChargesPathAndResolvedRelays_113, TestTrackedBytesTracksTheHeap_113
Eviction subtracts a fresh estimate	TestTrackedBytes_EvictionIgnoresAFreshEstimate_113
Resolved keys not charged	7 tests, incl. …ResolvedRelaysAreChargedIncrementally_164
Rebuild with no previous index does not credit	…ResolvedRelaysAreChargedIncrementally_164
Rebuild does not credit records of tx outside the store	…RebuildCreditsRecordsOfTxNotInTheStore_164
Eviction ignores the resolved part	4 tests, incl. …EvictionSubtractsExactlyWhatWasCharged_113
rechargeTx forgets the charge	10 tests
Memory selection (candidates and removal) uses the estimate	TestMemoryEviction_CrossesHighAndReachesLow_113
Candidate selection only uses the estimate	TestMemoryEviction_CrossesHighAndReachesLow_113
Resolved growth charged in full on every call	7 tests, incl. …MixedSequenceDoesNotDrift_113
Decode-cache term removed	TestEstimateStoreTxBytes, TestEstimateStoreTxBytesTypical_…_113
Dedup-key term removed	TestEstimateStoreObsBytes, TestEstimateStoreTxBytesTypical_…_113

Round 1: two mutants survived the first set of tests (the rebuild credit for a tx outside the store, and candidate selection); the two tests above them were added for those.

Round 2 (review by CS-Macmini): four more survived because every invariant test loaded through Load(), or pinned only the heap-ratio tolerance. Each is now red:

Mutant	Caught by
No recharge after the pick in LoadChunked	…LoadChunkedChargesPathAndResolvedRelays_113, …BackgroundFillChargesPathAndResolvedRelays_113
No recharge after the pick in loadChunk's local batch	…BackgroundFillChargesPathAndResolvedRelays_113
The five new observation strings dropped from estimateStoreObsBytes	…EstimateStoreObsBytes_CountsEveryTerm_113
perResolvedRecordBytes = 0	…AllowanceHasARecordAndAKeyPart_164
perResolvedRelayBytes = 0 (extra)	…AllowanceHasARecordAndAKeyPart_164, …ResolvedRelaysAreChargedIncrementally_164

One more mutant, dropping the charge at creation in scanAndMergeChunk, survives and is equivalent: the recharge after the pick charges the whole difference from zero.

Memory: trackedBytes against the real heap

TestTrackedBytesVsHeap_Measure_113 (gated by CORESCOPE_TRACKED_BYTES_MEASURE=100000,500000) loads a synthetic SQLite store through Load() (2.5 observations per tx, 1-5 hop paths, 60 relay pubkeys, 0-4 resolved relays per tx), forces a GC on both sides, and compares trackedBytes with the HeapAlloc growth. "Warm" fills the decode cache the way analytics do. The same harness runs on master.

Store	Resolved relays per tx	master tracked / heap	this PR tracked / heap
100k tx, 250k obs	0 (warm cache)	166 MB / 277 MB = 0.60	280 MB / 276 MB = 1.02
	1	0.55	1.03
	1.9	0.54	1.03
	4	0.51	1.04
500k tx, 1.25M obs	0 (warm cache)	832 MB / 1377 MB = 0.60	1404 MB / 1377 MB = 1.02
	1	0.55	1.01
	1.9	0.53	1.01
	4	832 MB / 1710 MB = 0.49	1653 MB / 1659 MB = 1.00

With the decode cache still cold, this PR is at 1.25 (it charges the cache up front, as upstream does, so eviction starts a little early rather than late). The gap went from 0.49-0.60 to 1.00-1.04. The resolved-relay allowance is the midpoint of two scales: the marginal heap per record plus keys was 110-170 B and 85-105 B per key at 100k and 500k.

Perf

Interleaved master/branch benchmarks, three rounds each. The numbers depend on the machine and its load, so two independent measurements are shown. Neither is a clear win or loss across the board, and no gain is claimed.

Benchmark	Author (3 rounds, -benchtime 5x) master → this PR	Reviewer (Apple M4, 3 rounds, -benchtime 5x) master → this PR
Cold Load, 20k tx	362 → 370 ms (+2%)	366 → 384 ms (+5%)
IngestNewFromDB, 200 tx	49.8 → 28.1 ms	51.2 → 51.3 ms (no difference)
IngestNewObservations, 200 obs	3.8 → 3.4 ms (-11%)	3.4 → 3.8 ms (+11%, ranges do not overlap)
RunEviction, 10k of 20k	276 → 272 ms	280 → 284 ms (+1%)

• The IngestNewFromDB difference in the author's runs does not reproduce on the second machine. That benchmark is dominated by SQLite (about 85% of a CPU profile), so it is not evidence for or against the change.
• The honest cost is one rechargeTx per updated tx: a path re-parse (about 0.5 µs, 14 allocations), O(path), once per pickBestObservation. At 200 observations that is at most about 0.4 ms (the reviewer's +11%); on Load it is the +2-5%.
• Pure Go, per call (author): addResolvedPubkeysToPathHopIndex 176-184 ns before and 178-191 ns after in the steady state (0 allocations), 254-257 vs 255-259 ns when it appends three keys; rechargeTx and estimateStoreTxBytes alone are within noise of each other. The accounting in the resolved-relay path is O(1) per appended key.
• Eviction order is unchanged and eviction does one map lookup per evicted tx.

Operator impact

• Where maxMemoryMB is set, eviction now caps the real store size. A store that fitted before may now evict.
• The cold-load budget, estimateStoreTxBytesTypical(10), goes from 5478 to 8506 bytes, so the same maxMemoryMB admits about 36% fewer packets at startup.
• Unset (the default) is inert.
• docs/go-migration.md still said the Go server ignores the setting; corrected, and docs/deployment.md states what the budget counts.

Interplay with #182 (now merged)

#182 landed on master while this was in progress, so this branch contains a merge of it (clean, no conflicts). It changes how live observations are indexed from resolved_path but still feeds indexResolvedPathHops / addResolvedPubkeysToPathHopIndex, where the charge is taken, and adds no pick site. All of this PR's invariant tests and the full -race suite run on the merged tree.

Known leftovers

• The constants are calibrated on synthetic data. Real decoded_json is larger and the real resolved relays per tx are unknown here (no staging or prod access). Compare packetStore.trackedMB with the Go heap on staging before relying on the exact ratio.
• The resolved-relay allowance folds the per-key byNode, nodeHashes and resolved-pubkey-index entries into one figure; they are not charged separately.
• Existing bug, not fixed here (F5 of the review): when the async hash migration (hash_migrate.go) merges a duplicate transmission it appends the duplicate's observations to the survivor without removing them from the duplicate, which stays in s.packets. Both are evicted later, so those observations are subtracted twice. The total drifts downward, hidden by the clamp at 0. It is the one path where the invariant above does not hold, and it behaves the same on master. Worth its own issue.
• Existing gap, not fixed here (F6): without a persisted resolved_path, relays found by the path_json fallback (resolvePathForObsColdLoad) go into byNode only and are not charged. Small.
• The memory measurement still goes through Load() only; it has not been repeated through LoadChunked or at 500k with the production startup path. The invariant tests do cover that path now.
• Known flake, unrelated: TestIssue1008_HandlerReturns503WhileSubpathIndexLoading failed once in a full -race run on a machine with a load average of about 38 (the subpath index was already built when the request arrived), then passed 20 of 20 in isolation on this branch and on master. A timing race between a background index build and the request.
• The decode cache is charged up front whether or not it is filled; a cold cache is over-charged by about 25% of the store.
• Not measured: Go versions other than the one used here, and GOMEMLIMIT behaviour under the larger estimate.

Verification not done

No deploy, no live-environment measurement, no CI result at the time of writing.

🤖 Generated with Claude Code

[dborup]

Rapport — CS-MacBook PR#198 #164+#113 trackedBytes — head 97e0304

Status: Both issues are implemented and verified locally on the tree merged with master (which now includes #182); the draft PR is open and CI had not finished when this was posted.

Draft PR for #113 and #164: 7 commits plus a merge of master on codex/issue-164-113-tracked-bytes, tests first (commit 1 is red on assertions), then the fix, docs, calibration and benchmarks, then a clean merge of master. Commit identity is dborup <kontakt@meshview.dk>; nothing was amended, rebased or force-pushed. Design, tables and the StoreTx layout argument are in the PR description; this comment is the requirement-by-requirement check.

Evidence tags: [T] test, benchmark, measurement or CI I ran, [A] analysis, [K] known, not run again.

#113 acceptance criteria

Criterion	Status
Record the exact estimate charged per tx	Done. StoreTx.chargedBytes (uint32); rechargeTx returns the delta. [T] …LoadChargesPathAndResolvedRelays_113 asserts chargedBytes == estimateStoreTxBytes(tx) for every live tx.
Apply only deltas when the path or another charged allocation changes	Done at all six pickBestObservation sites (Load, loadChunk local batch, LoadChunked, the main.go background pass, IngestNewFromDB, IngestNewObservations). [T] …RechargeCoversALaterPath_113; second recharge returns 0. [A] audited again after merging #182: it adds no pick site.
Eviction subtracts exactly what was charged	Done in candidate selection and in the removal pass. [T] …EvictionIgnoresAFreshEstimate_113, …EvictionCandidatesMatchWhatIsEvicted_113.
Include path/subpath indexes, decode cache, dedup-key allocations, observation strings	Path indexes were always estimated but charged before the path existed; now charged after. Decode cache (4x JSON, up front), dedup key and observation strings are new terms. [T] TestEstimateStoreTxBytes, TestEstimateStoreObsBytes, mutants for both terms.
Insert → path assignment → recharge → eviction returns to exactly 0	[T] …EvictionSubtractsExactlyWhatWasCharged_113 (partial, then full, exact 0) and the 60-step …MixedSequenceDoesNotDrift_113.
Mixed insert/update/eviction shows no drift; memory eviction crosses high and reaches low	[T] the invariant holds after every mixed step; …CrossesHighAndReachesLow_113 checks a memory pass ends at or below the low watermark without over-evicting.
Cold-load budget and docs use the revised estimate	estimateStoreTxBytesTypical follows (10 observations: 5478 → 8506 B, about 36% fewer packets admitted at the same maxMemoryMB). docs/go-migration.md and docs/deployment.md corrected. [T] …Typical_TracksTheRealCharge_113.
Route-mask, index and cache behaviour intact	[T] the layout tests pass unchanged (unsafe.Sizeof(StoreTx{}) == 320); full cmd/server suite green.

#164 acceptance criteria

Criterion	Status
trackedBytes rises when resolved entries are added and returns after eviction or rebuild, no drift over repeated observations	[T] …ResolvedRelaysAreChargedIncrementally_164: per-key delta, the same relays again charge 0, a rebuild that retains the records is neutral, a rebuild that clears them credits them. …RebuildCreditsRecordsOfTxNotInTheStore_164 covers the loadChunk window.
Tracked-vs-heap measurement at 100k transmissions, before and after, gap shrinks	[T] below.
O(1) per appended key, eviction order unchanged	[T] the pure-Go benchmarks show no measurable change per key (176-184 ns before, 178-191 ns after, 0 allocs steady state). [A] eviction still walks s.packets oldest-first; only the amount per tx differs.

Memory: trackedBytes against the real heap [T]

Synthetic SQLite store loaded through Load(), forced GC on both sides, decode cache filled as analytics do; the same harness on master and on this branch.

Store	Resolved relays per tx	master	this PR
100k tx, 250k obs	0	0.60	1.02
	4	0.51	1.04
500k tx, 1.25M obs	0	0.60	1.02
	4	0.49	1.00

(tracked / heap; intermediate rows are in the PR description.) At 100k with 4 relays per tx that is 166 MB tracked against 328 MB of heap on master and 330 MB against 319 MB on this branch. With the decode cache cold the branch is at 1.25: it charges the cache up front, so eviction starts a little early rather than late.

Perf [T]

Interleaved master/branch, 3 rounds. Cold Load of 20k tx: 362 → 370 ms (+2%). IngestNewObservations: 3.8 → 3.4 ms. RunEviction (10k of 20k): 276 → 272 ms. IngestNewFromDB (200 tx): 49.8 → 28.1 ms, but that benchmark is dominated by SQLite (about 85% of the CPU profile on both sides) and I cannot attribute the difference to this change, so I do not claim it. Pure-Go: addResolvedPubkeysToPathHopIndex and rechargeTx are within noise of their master counterparts.

Tests

• New: 11 tests in tracked_bytes_accounting_113_test.go, a gated measurement test, 8 benchmarks. [T]
• cmd/server go test -race -count=1 ./... on the merged tree: ok (374 s), run once in full as asked. Targeted accounting, estimate, layout and eviction tests also pass with -race -count=3. [T] The first -race run flagged my own eviction test mutating a tx while the background index builds read it; fixed by waiting for the builds in the test loader.
• cmd/ingestor go test ./...: ok. go vet ./... clean. gofmt -l clean for every file this PR touches (eviction_test.go was already unformatted on master). sh test-all.sh: 201 passed, 0 failed (201 files). [T]
• No new map[string]interface{}; cmd/server stays read-only; deploy.yml untouched. [T]

Mutants [T]

13 mutants, each caught; the table with test names is in the PR description. Two survived the first set of tests (the rebuild credit for a record whose tx is outside the store, and candidate selection using the estimate); I added …RebuildCreditsRecordsOfTxNotInTheStore_164 and …EvictionCandidatesMatchWhatIsEvicted_113. One mutant I wrote first did not compile (an unused variable), which is why that case has a rerun.

CI

Pending when this was posted (Go Build & Test had started). I did not poll it, so per-job results are not in this report; they need a follow-up check on the PR.

Differences from upstream 2035

(Upstream = Kpa-clawbot/CoreScope PR 2035, commit e6323ec5; read-only, not cherry-picked.) The per-tx field is a uint32 in bytes freed by narrowing UniqueObserverCount to int32, because this fork pins StoreTx at 320 bytes and upstream's int64 would make it 328. Resolved relay hops (#164) are not in upstream at all: they are charged per new key, credited by rebuilds and derived from the side map at eviction. The resolved allowance is calibrated on this branch at two scales. The decode-cache factor (4x) and the dedup-key term are taken from upstream; the 1.02 ratio above is what vindicates the factor on this fork's synthetic store, not a separate measurement of the cache.

Known leftovers

• Constants come from synthetic data; real decoded_json and real resolved relays per tx are unmeasured here (no staging or prod access). Compare packetStore.trackedMB with the Go heap on staging before trusting the exact ratio. [A]
• The per-key allowance folds in the byNode, nodeHashes and resolved-pubkey-index entries instead of charging them separately. [A]
• fix(store): index live observations from the persisted resolved_path, as Load does (#158) #182 merged while this was in progress; this branch contains a clean merge of it and the invariant tests pass on the merged tree, so there is no collision left to resolve. [T]
• The unfixed items from earlier sessions (for example the packets.js listener race) are out of scope and untouched. [K]

[adminopenclaw8-sketch]

Review — CS-Macmini PR#198 trackedBytes — head 97e0304

Verdict: APPROVE with nits. The accounting is correct on every charge/credit path I could find, and the tracked/heap ratio reproduces (0.61 → 1.03 at 100k). The main nit: the production load path (LoadChunked, loadChunk) is not covered by the invariant tests. Two mutants there survive; a probe test that catches them is below. The PR's perf table is not reproducible for IngestNewObservations.

The review was read-only. It used git archive of head 97e030436f8ba8b7a59b14c2dc48294a4905cbba and of the merge with origin/master 6f7a5e7c (git merge-tree tree 133eefcd, clean). git ls-remote showed the same head before and after. Nothing was pushed, and the PR was not changed.

Evidence tags: [T] test, CI or measurement run here, [A] analysis, [K] taken from the author's report and not rerun.

Findings

#	Prio	Where	Finding
F1	medium	tests, chunked_load.go / store.go loadChunk	All 11 invariant tests build the store with Load(). Production starts through RunStartupLoad → LoadChunked, plus the loadChunk background fill when hotStartupHours > 0. Dropping the recharge after the pick in LoadChunked (M3) or in loadChunk's local batch (M2) survives the whole accounting/eviction/estimate test set [T]. The code is right: my probes LoadChunked → acct113Check and LoadChunked (hotStartupHours: 2) + loadChunk → acct113Check pass on the merged tree and fail under M3, and under M2 or M3 respectively [T]. Adopt something like them. The heap measurements also go through Load() only.
F2	low	PR description, perf table	Not reproduced. IngestNewObservations is 3.4 ms → 3.8 ms (+11 %, ranges do not overlap) here, against the PR's 3.8 → 3.4. IngestNewTx is 51.2 → 51.3 ms (no difference; the PR's 49.8 → 28.1 is, as the author says, not attributable). Load is +5 % (PR: +2 %). The extra cost is one rechargeTx (path re-parse, 14 allocs) per updated tx: O(path) and acceptable, but the table should say so instead of showing a gain.
F3	low	estimateStoreObsBytes, resolved allowance	The new estimate terms are pinned only by the 0.85–1.15 heap-ratio tolerance. Dropping the five new observation strings (M5) or setting perResolvedRecordBytes = 0 (M6) survives [T]. TestEstimateStoreObsBytes uses an observation without ObserverName/IATA/Direction/RawHex/Timestamp; populate them so the test pins the formula.
F4	low	full -race run	My single full go test -race -count=1 ./... on the merged tree failed on one test: TestIssue1008_HandlerReturns503WhileSubpathIndexLoading (got 200, want 503: the subpath index was already built). The machine's load average was about 38 during the run, from other processes. The test then passed 20/20 under -race in isolation on both the merged tree and master, and CI's Go Build & Test on head passed. I classify it as a timing flake (a background index build racing the request), not caused by this PR; the full suite was not rerun (one run, as asked).
F5	info, pre-existing	hash_migrate.go ~114	When the async hash migration merges a duplicate, it appends the duplicate's observations to the survivor without removing them from the duplicate, which stays in s.packets. Both are evicted later, so those observations are subtracted twice: downward drift, hidden by the clamp at 0. The same on master, and outside this PR, but it is the one path where the PR's invariant ("trackedBytes == sum over live tx …") does not hold. Worth a follow-up issue.
F6	info, pre-existing	indexObservationRelayHops path_json fallback	Without persisted resolved_path, relays from resolvePathForObsColdLoad go into byNode only (addToByNode) and are not charged. Small; the PR's "Known leftovers" already says the per-key allowance folds byNode in only for resolved keys.

1. Accounting correctness

• [A] Charge at creation: rechargeTx replaces estimateStoreTxBytes at all 5 creation sites (Load, loadChunk local, scanAndMergeChunk, IngestNewFromDB, test makeTestStore).
• [A] Recharge after every pick: Load 1092, loadChunk 1427, LoadChunked chunked_load.go:489, main.go:339 background pass, IngestNewFromDB 3205, IngestNewObservations 3653. That is all six; grep pickBestObservation( on the merged tree finds no other caller.
• [A] Charges and credits are symmetric:
  • Observations are charged at all 5 append sites and recomputed at eviction.
  • Nothing removes an observation from a live tx; the server has no in-memory prune (geo prune runs in the ingestor).
  • The fields that feed the tx estimate can change outside a pick (hash_migrate.go sets tx.Hash), but that cannot drift, because eviction subtracts chargedBytes, never a fresh estimate.
• [A] Eviction uses txChargedBytes in both candidate selection (evictionCandidateTxIDs, evictStaleInternal) and removal. txChargedBytes is computed before delete(s.pathHopResolved, tx) in the same loop, so the resolved part is still visible.
• [A] Rebuilds:
  • retainResolvedPathHops credits the dropped records (non-live tx) and the clear branch.
  • The loadChunk window is consistent: the resolved charge of a not-yet-merged tx is credited with its record, and its tx charge arrives with localTrackedBytes at merge.
  • Duplicate local txs are merged into s.packets (pre-existing) and evicted with their own charge.
• [A] chargedBytes uint32 is only assigned, never decremented, so it cannot underflow, and rechargeTx clamps at MaxUint32. The delta is computed in int64 (est - int64(chargedBytes)), so negative deltas have the right sign (pinned by …RechargeCoversALaterPath_113: the second recharge returns 0).
• [A] Concurrency: rechargeTx on loadChunk's local batch runs outside s.mu, but it only touches unshared txs. All other calls hold s.mu.

2. Interplay with #182 (9d29daec, on master)

• [A] fix(store): index live observations from the persisted resolved_path, as Load does (#158) #182 adds no pick site (all six are above).
• [A] indexObservationRelayHops (store.go:1993) sends persisted resolved_path through indexResolvedPathHops → addResolvedPubkeysToPathHopIndex, where the growth is charged. That covers Load 1058, scanAndMergeChunk chunked_load.go:659, IngestNewFromDB 3166 and IngestNewObservations 3547. loadChunk calls indexResolvedPathHops directly under the lock (store.go ~1538).
• The only uncharged branch is the path_json fallback into byNode (F6).

3. UniqueObserverCount int32

• [A] There are 6 increment sites and one read: "observer_count": tx.UniqueObserverCount in the grouped-packets map (store.go:2271), which is JSON-encoded as a number.
• No code reads that key back with an .(int) type assertion (grep '\["observer_count"\]': none), and the GroupedPacketResp struct field is not filled from this map.
• It counts distinct observers per tx (tens, maybe hundreds), so int32 truncation is not realistic.
• [T] The layout tests pass, and unsafe.Sizeof(StoreTx{}) stays 320.

4. Tests

• [T] Red before / green after:
  • Commit f5c345d7 (tests plus inert scaffolding): all 9 of its tests fail on assertions, e.g. 400 of 400 tx carry a stale charge, trackedBytes = 697012, charges … sum to 308000, ratio = 0.63.
  • The merged tree passes all 11, including the two added later. Those two reference typicalResolvedRelays, so they cannot compile on the scaffolding; they are covered by the author's mutants.
• [T] go test -race -count=1 ./... on the merged tree, one run: FAIL, on one test only (F4); no DATA RACE reports.
• [T] sh test-all.sh on the merged tree: 201/201.
• [T] go vet is clean. gofmt -l is clean on every touched file except eviction_test.go, which is also unformatted on master.
• [T] CI on head: Go Build & Test passed. Playwright was still running when this was posted.

Own mutants (not in the author's list), each on a copy of the merged tree, run against _113$|_164$|TestEstimate|Evict|Layout|TrackedBytes:

Mutant	Result
M1 no recharge after IngestNewFromDB's pick	red: TestTrackedBytes_MixedSequenceDoesNotDrift_113
M2 no recharge after loadChunk's local pick	survives; red with probe 2 (F1)
M3 no recharge after LoadChunked's pick	survives; red with probes 1 and 2 (F1)
M4 late observation charges the full estimate instead of the delta	red: …MixedSequenceDoesNotDrift_113
M5 the new observation strings dropped from estimateStoreObsBytes	survives (F3)
M6 perResolvedRecordBytes = 0	survives (F3)

The author's list already covers "forget the credit in retainResolvedPathHops" and "recharge only at creation" in their variants, so I did not repeat them.

Probe (scratch only), using the PR's own helpers:

store := NewPacketStore(db, &PacketStoreConfig{RetentionHours: 168, HotStartupHours: 2})
store.LoadChunked(100); store.WaitIndexesReady(30 * time.Second)
store.loadChunk(now.Add(-100*time.Hour), now.Add(-2*time.Hour)) // acct113CreateDB(t, p, 400, 200)
acct113Check(t, store, "after loadChunk")

5. Perf and memory

[T] TestTrackedBytesVsHeap_Measure_113 with CORESCOPE_TRACKED_BYTES_MEASURE=100000, the same harness on master (copied in) and on the merged tree:

100k tx, 250k obs	master tracked/heap	merged tracked/heap
no relays, decode cache cold	166 / 213 MB = 0.78	280 / 224 MB = 1.25
no relays, decode cache warm	166 / 275 MB = 0.61	280 / 271 MB = 1.03
1.0 relays per record	0.56	1.05
1.9 relays per record	0.55	1.05
4.0 relays per record	0.52	1.05

This matches the author's 0.51–0.60 → 1.02–1.04 within 0.01–0.02.

[T] Benchmarks, test binaries built once, 3 interleaved rounds, -benchtime 5x. Apple M4, load average 3.4–4.6. Median [min–max]:

Benchmark	master	merged	change
TrackedBytes_Load_113 (20k tx)	366.3 ms [365.6–369.1]	384.1 ms [371.8–384.7]	+5 %
TrackedBytes_IngestNewTx_113	51.2 ms [49.1–54.9]	51.3 ms [49.8–53.5]	0 %
TrackedBytes_IngestNewObs_113	3.4 ms [3.4–3.4]	3.8 ms [3.5–3.9]	+11 %
TrackedBytes_Evict_113 (10k of 20k)	280.4 ms [279.6–281.4]	283.7 ms [278.2–289.3]	+1 %

[A] Hot paths:

• addResolvedPubkeysToPathHopIndex: one subtraction per call, O(1).
• rechargeTx: one estimate per picked tx, O(path).
• Eviction: one map lookup per tx.
• retainResolvedPathHops: the credit loop is O(records) and runs only at a rebuild.

No O(n²), and the eviction order is unchanged. The IngestNewObs +0.4 ms per 200 observations is the recharge's path re-parse (F2).

6. Operational impact

[A] The high watermark is maxMemoryMB, the low one 85 %. The cold-load budget is maxMemoryMB / estimateStoreTxBytesTypical(10), which is now 8506 B.

Prod (1800 MB, 48 h, about 38K packets, trackedMB about 344 today).

• At master's ratio of about 0.5–0.6, that is roughly 570–690 MB of real store heap.
• With this PR, trackedMB should read about 600–860 MB (1.03 warm to 1.25 cold).
• Far below 1800, so no memory eviction; retention binds first.
• The cold-load budget is 1800 MiB / 8506 B ≈ 222K packets, against about 38K, so it does not bind.

The larger 168 h instance (5000 MB).

• Budget ≈ 616K packets.
• At prod's rate (about 38K per 48 h), 168 h is about 133K packets and about 2.1–3.0 GB tracked: below 5000, but the closest of the two (about 40–60 %).

Caveat: the factor and allowances are synthetic. Real decoded_json is larger, which raises the charge, so compare /api/perf packetStore.trackedMB after deploy.

[A] The docs are correct against the code:

• the cold load stops at the budget (maxPackets);
• eviction runs oldest-first down to 85 %;
• GOMEMLIMIT is derived with 1.5× headroom when neither GOMEMLIMIT nor runtime.maxMemoryMB is set (main.go:139);
• /api/perf exposes packetStore.trackedMB and maxMB.

7. Rules

• [T] Added map[string]interface{}: 0 (the one match is a comment).
• [T] cmd/server stays read-only: all 13 added SQL writes are in test fixtures.
• [T] deploy.yml is unchanged, with 9 fork guards before and after.
• [T] go vet and gofmt as above; test-all.sh 201/201.
• [T] The PR text has no closing keywords. Upstream is credited in code format (Kpa-clawbot/CoreScope PR 2035, e6323ec5), not as a link.

Not verified

• Real data, staging or production: the ratio and constants are synthetic.
• The larger instance's real packet count.
• Measurements through the production LoadChunked path, and at 500k.
• A second full -race run (one run, F4).
• The upstream 2035 diff in detail: I relied on the PR's comparison table for upstream's side [K].
• Playwright CI on head (still running).

[dborup]

Rapport — CS-MacBook PR#198 runde 2 — head 4d0a84f

Status: F1, F2 and F3 are fixed, F4 to F6 are documented in the PR text; all local checks pass on the pushed head and CI was still pending when this was posted.

One new commit on top of 97e03043 (4d0a84fe, tests only; no production code changed). Master had not moved (6f7a5e7c), so no merge was needed. Identity dborup <kontakt@meshview.dk>; nothing amended, rebased or force-pushed. The PR description was rewritten with gh pr edit and still starts with Relates to #113, #164.

Evidence tags: [T] test, mutant or measurement I ran, [A] analysis, [K] taken from the review and not rerun.

Findings

#	Prio	Outcome	Commit	Test / evidence
F1	medium	Fixed (tests)	4d0a84fe	TestTrackedBytes_LoadChunkedChargesPathAndResolvedRelays_113 runs RunStartupLoad (LoadChunked) and checks the invariant; TestTrackedBytes_BackgroundFillChargesPathAndResolvedRelays_113 runs it with hotStartupHours = 2 over a store half of whose rows are 3 days old (so the loadChunk background fill runs), then evicts the filled half. Both green on the code; M2 and M3 are now red. [T]
F2	low	Fixed (PR text)	PR description	The perf section now shows both measurements side by side (author and reviewer), says they depend on the machine and its load, and claims no gain. IngestNewObservations is listed as 3.8 → 3.4 ms on one and 3.4 → 3.8 ms (+11%, non-overlapping ranges) on the other, Load as +2% and +5%, IngestNewTx as 49.8 → 28.1 ms on one and no difference on the other, with the reason (SQLite dominates). The honest cost is one rechargeTx per updated tx.
F3	low	Fixed (tests)	4d0a84fe	TestEstimateStoreObsBytes_CountsEveryTerm_113 populates all seven observation fields with distinct lengths and checks the exact estimate; one more byte of each new string moves it by 1 (observer ID and path by 2: field plus dedup key). TestResolvedRelayBytes_AllowanceHasARecordAndAKeyPart_164 pins the record and key parts and their physical floors (16 B per key, 32 B per record). M5 and M6 are now red. [T]
F4	low	Documented	PR description	Named in "Known leftovers" as a known, unrelated flake: TestIssue1008_HandlerReturns503WhileSubpathIndexLoading. Nothing else done. [K]
F5	info, pre-existing	Not fixed, documented	PR description	"Known leftovers": the async hash migration (hash_migrate.go) appends a merged duplicate's observations to the survivor without removing them from the duplicate, so they are subtracted twice at eviction; the clamp at 0 hides the downward drift; same on master; worth its own issue. [K]
F6	info, pre-existing	Not fixed, documented	PR description	"Known leftovers": relays found by the path_json fallback (resolvePathForObsColdLoad) go into byNode only and are not charged. [K]

Mutants [T]

Each on a copy of the branch, run against the accounting, estimate, layout, eviction and tracked-bytes tests.

Mutant	Before round 2	Now
M3 no recharge after the pick in LoadChunked	survived	red: …LoadChunkedChargesPathAndResolvedRelays_113, …BackgroundFillChargesPathAndResolvedRelays_113
M2 no recharge after the pick in loadChunk's local batch	survived	red: …BackgroundFillChargesPathAndResolvedRelays_113
M5 the five new observation strings dropped	survived	red: …EstimateStoreObsBytes_CountsEveryTerm_113
M6 perResolvedRecordBytes = 0	survived	red: …AllowanceHasARecordAndAKeyPart_164
extra: perResolvedRelayBytes = 0	not run	red: the same test and …ResolvedRelaysAreChargedIncrementally_164
extra: no charge at creation in scanAndMergeChunk	not run	survives, and is equivalent: the recharge after the pick charges the whole difference from zero

The thirteen mutants of round 1 are unchanged and stay red.

Local checks on the pushed head [T]

• New and affected tests (_113, _164, layout, estimate, tracked-bytes, eviction) with -race -count=3: ok, no DATA RACE.
• cd cmd/server && go test ./..., once: ok.
• go vet ./... clean; gofmt -l clean for every file this PR touches (eviction_test.go is unformatted on master as well).
• sh test-all.sh: 201 passed, 0 failed (201 files).

CI

Pending when this was posted (Go Build & Test had started). I did not poll it; per-job results are not in this report.

Not done

Nothing beyond the list: F5 and F6 are not fixed, and the memory measurement still goes through Load() only (the invariant tests now cover the startup path, the 100k and 500k heap ratios do not).