feat(ingestor): opt-in retention for inactive_nodes, node_changes and observers (#329)

[dborup-agent]

Relates to #329

Supersedes #34: its upstream#1886 port (observerPurgeDays) is carried over here onto current master, with the two neighbour tables that port did not cover (see Observers below). #34 can be closed when this lands.

Scope (operator decision on #329, 2026-10-07): ping_triggers is not pruned. It keeps every row, because the all-time Ping Scores records join it with the history sidecar (#349 / #241). Round 2 removed the pingTriggerDays knob from round 1 with all its code, tests and docs, and added a test that locks ping_triggers as untouched.

Plan

One logical change plus test commits:

1. feat(ingestor): three opt-in retention knobs, the batched prunes, wiring at startup and in the daily observer-retention pass, config.example.json + user-guide docs, and the read-only invariant extended to the three new method names.
2. test(server): Ping Scores entries already marked data_pruned still render (the retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329 acceptance criterion).
3. test(ingestor): an opt-in timing test on a multi-GB synthetic DB.
4. Round 2: test(ingestor) locks ping_triggers out of the retention, and fix(ingestor) drops the pingTriggerDays knob.

Operator note: the target deployment will set all three knobs to 30 days. With observerDays 14, packetDays 30 and metricsDays 30 that is consistent: an observer is purged once its packets and metrics have aged out on the same 30-day clock.

No frontend change, so there are no customizer implications. The knobs are retention policy in config.json, like packetDays.

Change

New knobs in retention. Each is in days; 0 or unset keeps every row, exactly as today:

Knob	Table	Deletes
inactiveNodeDays	inactive_nodes	rows whose last_seen (last advert) is older, except nodes that came back (a nodes row exists) and observers still uploading (#199, same predicate as MoveStaleNodes)
nodeChangeDays	node_changes	rows with detected_at older, oldest first over idx_node_changes_detected_at
observerPurgeDays	observers	rows already soft-deleted (inactive = 1), not seen for N days, and referenced by no observations, observer_metrics, observer_neighbor_metrics or dropped_packets row; their observer_neighbors snapshot goes in the same transaction

• All writes are in cmd/ingestor (table_retention.go). They run at startup (after the metrics and transmission prunes, before the ingest gate opens) and in the existing daily observer-retention pass, right after RemoveStaleObservers.
• Every delete runs in bounded WriterTx batches of 1000 rows, like pruneBatches, so ingest waits for one batch at most. Each table logs a [prune] deleted N … older than D days line; startup logs the enabled windows once.
• cmd/server gains no code. TestServerDBHasNoWriteMethods now also forbids PruneInactiveNodes, PruneNodeChanges and PurgeStaleObservers on the server's *DB.

ping_triggers: kept forever

No knob, no delete. The transmission prune already leaves it alone. A config that still sets pingTriggerDays (from round 1 of this draft) is ignored. The user guide and config.example.json say that ping_triggers is kept and why. TestTableRetentionLeavesPingTriggers sets every other knob and that stale key, runs the transmission prune first, and requires every ping_triggers row to be unchanged, including one older than all windows and one whose transmission was pruned.

inactive_nodes: why returned nodes are kept

MoveStaleNodes does not delete a node's old inactive_nodes row when it comes back. That row still carries confirmed default_scope evidence that UpdateNodeDefaultScope checks across both tables, and it keeps the node out of New Nodes. Deleting it while the node is active would let inference downgrade confirmed scope and list a returning node as new. Once the node goes stale again, MoveStaleNodes replaces the row with a fresh last_seen.

A node purged after inactiveNodeDays that adverts again later counts as new: it gets no resurrected change and shows in New Nodes. That is the intent of retention, and it is documented.

Observers (from #34)

Taken from #34 / upstream#1886 (PurgeStaleObservers, its tests, the docs), not the old branch as a whole. The review on #34 found two observer_id tables the port did not guard. I made these two decisions:

• observer_neighbor_metrics ages out by metricsDays like observer_metrics, so it is a guard: the observer waits until those rows are gone.
• observer_neighbors is a current-only snapshot that only the observer itself replaces. As a guard it would keep a dead observer forever. Left behind, it would keep serving the dead observer's neighbours (GetObserverNeighbors does not join observers). So it is deleted with the observer in the same transaction.

Each batch reads its candidate ids once and deletes exactly those. Rows with a NULL id, which SQLite allows in a TEXT PRIMARY KEY, are skipped.

Tests

Test-first: the new tests were run against a stub with master behaviour (no pruning), and every "deletes" test was red. They are green with the change. Per knob:

• old rows removed, recent rows kept;
• unset knob changes nothing (TestRunTableRetentionUnsetChangesNothing, plus each knob prunes only its own table);
• several batches (retentionBatchRows lowered in the test);
• the guards: returned node, active observer, the four observer reference guards, inactive = 1, NULL id;
• ping_triggers untouched: TestTableRetentionLeavesPingTriggers; pingTriggerDays ignored in TestTableRetentionConfig; neither doc offers it in TestConfigExampleDocumentsTableRetention. All three were red on the round-1 head;
• Ping Scores: TestPingScoresDataPrunedEntryRendersWhileTriggerKept (server);
• config parsing, and config.example.json documents the three knobs at 0.

Mutation testing: round 1 had 24 targeted mutants, all killed. Round 2 adds 8, all killed. The tables are in the report comments.

Perf

TestTableRetentionTiming (opt-in, CORESCOPE_RETENTION_PERF=1) seeds a 5.34 GB synthetic DB:

• 3M transmissions with 12M observations;
• 20k inactive_nodes, 500k node_changes, 50k ping_triggers (no retention deletes them; the test fails if one goes);
• 600 observers, 300 of them soft-deleted.

It runs today's prunes and then the table retention with the three windows at 30 days, in production order. A probe goroutine takes the writer lock every 5 ms, standing in for MQTT ingest. The default [db-slow-writer] threshold of 500 ms applies.

Run 3 is on the round-2 code (three knobs), with the fixture seeded fresh. Runs 1 and 2 (round 1, four knobs) are kept for comparison; their #329 numbers include the removed ping_triggers prune (51 tx, hold max 24 ms in run 2).

Pass	Step	[db-slow-writer] lines run 1 / 2 / 3	Max hold run 1 / 2 / 3	Ingest probe max wait run 1 / 2 / 3	Wall run 3
first run (30 d, 335 days of backlog)	today's prunes	85 / 128 / 16	2219.5 / 14028.8 / 1964.9 ms	2627.9 / 14114.8 / 2052.8 ms	7m52s
first run	#329 table retention	1 / 2 / 2	516.0 / 1156.7 / 1015.5 ms	511.6 / 1607.7 / 1010.4 ms	18.1s
next day (29 d)	today's prunes	0 / 0 / 0	318.3 / 430.6 / 143.8 ms	313.4 / 425.5 / 205.9 ms	5.5s
next day	#329 table retention	0 / 0 / 0	31.5 / 40.4 / 11.6 ms	49.0 / 68.3 / 16.0 ms	26 ms
nothing to delete	today's prunes	0 / 0 / 0	11.6 / 19.8 / 6.3 ms	6.4 / 14.6 / 1.2 ms	11 ms
nothing to delete	#329 table retention	0 / 0 / 0	2.0 / 2.0 / 1.8 ms	0.0 / 0.0 / 0.0 ms	2 ms

Run 3's first-run backlog deletes 18,000 inactive_nodes, 458,918 node_changes and the 225 unreferenced soft-deleted observers, and no ping_triggers. Its two slow lines are both query=COMMIT in prune_node_changes (769 ms and 1016 ms, out of 459 batches; p99 230 ms). Those are commit/checkpoint stalls on a disk that, in the same pass, gave today's prunes 16 slow lines. No pass shows a spike above today's level, and the daily steady state adds no [db-slow-writer] line. Per-component first-run hold max, run 3: prune_inactive_nodes 21.7 ms (19 tx), prune_node_changes 1015.5 ms (459 tx), purge_observers 39.8 ms (1 tx).

Complexity: every batch is bounded (1000 rows deleted). inactive_nodes and node_changes walk their last_seen / detected_at index. The observer guards are four index seeks per candidate (EXPLAIN QUERY PLAN checked).

Verification (round 2, head 7228e083)

• cd cmd/ingestor && go test -timeout 45m ./...: ok. cd cmd/server && go test -race -timeout 45m ./...: ok.
• sh test-all.sh: 228 files passed. node test-frontend-helpers.js: 709 passed.
• E2E against a local Go server on e2e-fixture.db, prepared as in CI (freshen, seed SQL, corescope-migrate, the three seed files): test-e2e-playwright.js 132/135 passed (3 skipped); test-issue-199-inactive-observer-e2e.js 3/3.
• The real ingestor binary, started on a copy of that fixture with all three knobs at 30 and a stale pingTriggerDays: 30, logged the three knobs, deleted the old inactive_nodes and node_changes rows, and left a 200-day-old ping_triggers row byte-identical.
• bash scripts/check-xss-sinks.sh --diff origin/master: clean (no frontend change). No new map[string]interface{} outside tests. Workflows untouched.

🤖 Generated with Claude Code

[dborup]

Scope note for review/round 2: per the operator decision on #329 (2026-10-07), ping_triggers must NOT be pruned (Ping Scores all-time, see #349/#241). The ping_triggers knob and its code/tests must be removed from this PR; the title will be updated accordingly.

[dborup-agent]

Rapport — CS-pve-agent3 PR#350 #329 — head 398af2b

Status: Draft ready for review: all four knobs implemented test-first, 24/24 mutants killed, CI green on every job that runs, and no [db-slow-writer] spike above today's level on a 5.34 GB synthetic DB.

Evidence: [T] = executed (test, command or CI output), [A] = analysis or argument, [K] = read in code or docs.

Requirements

#	Requirement	Evidence	Test(s)	Mutant(s)
1	New retention knobs, unset = identical to today	[T] each "deletes" test red against a no-op stub with master behaviour, green after; unset leaves all four tables untouched	TestRunTableRetentionUnsetChangesNothing, TestRunTableRetentionEachKnob, TestTableRetentionConfig	M7 (days <= 0 guard → < 0), M17 (negatives not clamped), M18 (knob not read), M4 (knob not wired): all killed
2	All writes in cmd/ingestor, at startup and in the daily prune, batched in WriterTx	[K] table_retention.go; startup after the metrics and transmission prunes, daily after RemoveStaleObservers. [T] batch-count assertions per table	Test…Batches (inactive_nodes, node_changes, ping_triggers, observers), TestServerDBHasNoWriteMethods (+4 names)	M6, M10, M16 (loop stops after one batch), M22 (write method on server *DB): killed
3	inactive_nodes: old removed, recent kept	[T]	TestPruneInactiveNodesDeletesOldKeepsRecent, …KeepsReturnedNode, …KeepsActiveObserverNode	M1 (came-back guard), M2 (active-observer guard), M3 (cutoff flipped): killed
4	node_changes: old removed, recent kept	[T]	TestPruneNodeChangesDeletesOldKeepsRecent, …Batches	M5 (cutoff flipped), M6: killed
5	ping_triggers: own age (justified), old removed, recent kept	[A] the server's Ping Scores history drops an entry once its trigger is gone, and data_pruned means "transmission gone", so following the transmission prune would delete every data_pruned entry. [T]	TestPrunePingTriggersDeletesOldKeepsRecent, …KeepsTriggerOfPrunedTransmission, TestPruneOldPacketsLeavesPingTriggers, …Batches, …NonPositiveTxID	M8 (follow the transmission prune), M9 (cutoff flipped), M10, M11 (ping_triggers added to the transmission prune batches), M24 (walk starts at 0): killed
6	Soft-deleted observers, based on #34 (upstream#1886)	[K] #34's code and tests carried over onto current master (not the old branch); #34's review gap resolved: observer_neighbor_metrics guards, observer_neighbors deleted with the observer. [T]	ported #34 tests + TestPurgeStaleObserversDeletesNeighborSnapshot, …KeepsObserverWithNeighborMetrics, …Batches, …SkipsNullID	M12 (keep neighbours), M13 (no neighbour-metrics guard), M14 (no observations guard), M15 (no inactive = 1), M16, M23 (no NULL-id guard): killed
7	Ping Scores entries already marked data_pruned still render	[T] server test: an entry marked data_pruned with its transmission deleted still renders in the all-time records; its counterpart shows the entry leaves the history store once its trigger is deleted	TestPingScoresDataPrunedEntryRendersWhileTriggerKept, …DroppedWithItsTrigger (server); TestPrunePingTriggersKeepsTriggerOfPrunedTransmission (ingestor)	M21 (snapshot skips data_pruned), M20 (reconcile keeps orphans), M8: killed
8	Perf: prune duration and write-lock hold on a multi-GB DB, no [db-slow-writer] spike above today's level	[T] TestTableRetentionTiming, 5.34 GB, windows 30 d, two runs. Slow lines, today's prunes vs #329: first run 85 vs 1 (run 1), 128 vs 2 (run 2); next day 0 vs 0; nothing to delete 0 vs 0. #329 steady-state hold max 31.5 / 40.4 ms. Full table in the PR description	TestTableRetentionTiming (opt-in)	n/a (measurement)
9	Knobs documented in config.example.json	[T] the example sets all four to 0 and documents each; also in docs/user-guide/configuration.md, with the operator's 30-day setting noted	TestConfigExampleDocumentsTableRetention	M19 (knob missing from the example): killed

Always

Rule	Evidence
Red before / green after, ≥1 mutant per point	[T] stub run red; 24 targeted mutants, all killed (table above)
cmd/server stays read-only	[K] no server code changed; [T] TestServerDBHasNoWriteMethods extended (M22 killed)
No new map[string]interface{} outside tests	[T] git diff origin/master adds none; the one map[string]any is in table_retention_test.go
No hardcoded colours; scripts/check-xss-sinks.sh --diff origin/master clean	[T] "no public/**/*.{js,html} changes to scan", exit 0
Fork guards unchanged (9 / 1)	[T] grep -c gives 9 in deploy.yml and 1 in release-fast-path.yml; workflows untouched

Local runs

Suite	Result
cd cmd/ingestor && go test ./...	[T] ok (1013 s)
cd cmd/server && go test -race ./...	[T] ok (1013 s)
sh test-all.sh	[T] 227 passed, 0 failed
node test-frontend-helpers.js	[T] 709 passed, 0 failed
E2E on a local Go server, e2e-fixture.db prepared as in CI (freshen, seed SQL, migrate, three seed files)	[T] test-e2e-playwright.js 132/135 (3 skipped); test-issue-199-inactive-observer-e2e.js 3/3; /api/ping-scores, /api/analytics/node-changes, /api/observers 200. Server stopped via pid

CI (run 37608978680, pull_request, head 398af2b)

Job	Result
Go Build & Test	[T] pass (23m18s)
Playwright E2E Tests	[T] pass (25m22s)
Build & Publish Docker Image	[T] pass (57s)
Release Artifacts	skipped (tag-only)
Deploy Staging	skipped (push/fork-guarded)
Publish Badges & Summary	skipped (push-only)

The known flaky test #271 did not appear; no re-run was needed.

Remaining

• [A] On first enable, the backlog delete can emit one or two [db-slow-writer] lines at COMMIT: 1 line (516 ms) in run 1 and 2 lines (615 ms and 1157 ms) in run 2. Today's prunes emit 85–128 in the same pass, so this is below today's level, but an operator may see them once. The daily steady state emitted none in either run.
• [T] The two perf runs differ by disk noise: today's first-run prune_metrics hold was 2.1 s in run 1 and 14.0 s in run 2. The run-2 binary predates the last commit's start-key change (math.MinInt64 instead of 0), which does not affect timing [A].
• [T] The local full Go suites ran before that last start-key change; CI ran both full suites on the final head.
• [K] Like the other retention values, the four knobs are read at ingestor startup, not hot-reloaded.
• [A] With pingTriggerDays set, the all-time Ping Scores records are bounded to that window. This is the cost of removing sender names, and it is documented.
• No staging or production validation was done.
• port(upstream#1886): opt-in observerPurgeDays hard-delete for long-inactive observers #34 is left open; the PR description says this PR supersedes it.

[dborup-agent]

Rapport — CS-pve-agent3 PR#350 runde-2 — head 7228e08

Review feedback addressed (commit 7228e083; the test commit is 5fdce2af)

Evidence: [T] = executed (test, command or CI output), [A] = analysis or argument, [K] = read in code or docs.

1. Blocking. ping_triggers must not be pruned (operator scope decision on retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329): remove the knob with its code, tests and docs. Fixed in 7228e083.
   • [K] PingTriggerDays is removed from RetentionConfig and TableRetention, along with Enabled(), the startup log line, PrunePingTriggers and its row in runTableRetention. A config that still sets pingTriggerDays is ignored.
   • [A] deleteInKeyRanges existed only to share the key-range walk with PrunePingTriggers, so it is removed too. PruneOrphanRouteMaskChanges is back to master's code byte for byte (git checkout origin/master -- cmd/ingestor/maintenance.go), which keeps this PR out of code it no longer needs to touch.
   • [K] TestServerDBHasNoWriteMethods no longer names PrunePingTriggers. The perf test no longer prunes ping_triggers, still seeds 50k of them, and now fails if retention deletes one.
   • [K] config.example.json and docs/user-guide/configuration.md drop the knob. Both now say that ping_triggers has no window and is kept forever, and why.
   • Removed tests ([A] they tested the removed knob): TestPrunePingTriggersDeletesOldKeepsRecent, …KeepsTriggerOfPrunedTransmission, …Batches, …NonPositiveTxID, the ping_triggers subtest of TestRunTableRetentionEachKnob, and the server test TestPingScoresDataPrunedEntryDroppedWithItsTrigger. That server test only pinned what the knob would have cost, and it covered existing server reconcile behaviour that this PR does not change. TestPruneOldPacketsLeavesPingTriggers went as well, because the new lock test covers the transmission prune.
   • Kept ([A] still the retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329 acceptance criterion): TestPingScoresDataPrunedEntryRendersWhileTriggerKept, inlined now that it has no sibling.
2. Blocking. Add a test that locks ping_triggers as untouched when the other knobs are set. Added in 5fdce2af.
   • [T] TestTableRetentionLeavesPingTriggers builds its config from JSON with packetDays, inactiveNodeDays, nodeChangeDays and observerPurgeDays at 30, plus a stale pingTriggerDays: 30. It seeds three triggers: one 90 days old, one 1 day old, and one whose transmission the packet prune removes. It runs PruneOldPackets and then runTableRetention. It requires every ping_triggers row to be byte-identical (tx_id|hash|channel_hash|sender|first_seen). It also requires the other three tables to be pruned, so the test cannot pass vacuously.
   • [T] TestTableRetentionConfig/pingTriggerDays_ignored and the extended TestConfigExampleDocumentsTableRetention (neither doc mentions pingTriggerDays).
   • [T] Red first: run on 259684f3 (the round-1 head merged with master), all three failed. The stale knob deleted the 90-day trigger and the pruned transmission's trigger; TableRetention() returned PingTriggerDays:30 with Enabled()=true; both docs listed the knob. All three are green on 7228e083.
3. PR title and description. [T] The title is now "feat(ingestor): opt-in retention for inactive_nodes, node_changes and observers (retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329)". The description is rewritten for three knobs: a scope note, a "ping_triggers: kept forever" section, round-2 tests, perf run 3 and round-2 verification. It uses "Relates to retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329" and no closing keywords.
4. inactive_nodes, node_changes and observers (port(upstream#1886): opt-in observerPurgeDays hard-delete for long-inactive observers #34 basis) unchanged; rerun the full cmd/ingestor suite. [K] No change to those prunes, guards or wiring. [T] The full suite is green (below).

No finding was disputed.

Mutants (round 2, one or more per finding, all killed)

Run on a copy of the tree, one at a time, with the named test.

#	Finding	Mutant	Killed by
R1	1, 2	PruneNodeChanges also deletes ping_triggers older than its cutoff	TestTableRetentionLeavesPingTriggers [T]
R2	1, 2	the transmission prune batches also delete the batch's ping_triggers ("follow the transmission prune")	TestTableRetentionLeavesPingTriggers [T] (the pruned transmission's trigger disappeared)
R3	2	the observer purge rewrites ping_triggers (sender set to NULL) instead of deleting	TestTableRetentionLeavesPingTriggers [T]
R4	2	runTableRetention does nothing (vacuity check)	TestTableRetentionLeavesPingTriggers [T]
R5	1	config.example.json offers pingTriggerDays again	TestConfigExampleDocumentsTableRetention [T]
R6	1	the user guide documents retention.pingTriggerDays again	TestConfigExampleDocumentsTableRetention [T]
R7	1	the restored PruneOrphanRouteMaskChanges loses its orphan condition	TestPruneOrphanRouteMaskChanges [T]
R8	1	the Ping Scores snapshot skips data_pruned entries	TestPingScoresDataPrunedEntryRendersWhileTriggerKept [T]

[T] Re-adding the knob itself (config field, TableRetention, prune and wiring) is the round-1 head, where the red run above failed all three new tests.

Local runs (head 7228e083)

Suite	Result
cd cmd/ingestor && go test -count=1 -timeout 45m ./...	[T] ok (1360 s)
cd cmd/server && go test -race -count=1 -timeout 45m ./...	[T] ok (1741 s)
sh test-all.sh	[T] 228 passed, 0 failed
node test-frontend-helpers.js	[T] 709 passed, 0 failed
E2E on a local Go server, e2e-fixture.db prepared as in CI (freshen, seed SQL, corescope-migrate, three seed files)	[T] test-e2e-playwright.js 132/135 (3 skipped); test-issue-199-inactive-observer-e2e.js 3/3; /api/ping-scores, /api/analytics/node-changes, /api/observers returned 200. Server stopped via pid
Real ingestor binary on a copy of that fixture: all three knobs at 30 and a stale pingTriggerDays: 30, with a local stub MQTT broker so startup passes the connect step	[T] logged table retention enabled (0 = off): inactiveNodeDays=30 nodeChangeDays=30 observerPurgeDays=30; deleted the 200-day-old inactive_nodes and node_changes rows (kept the #199 seeded node, last seen 10 days ago); ping_triggers dump md5 identical before and after, including a 200-day-old row
TestTableRetentionTiming (opt-in, 5.34 GB), run 3	[T] PASS, 0 ping_triggers deleted. #329 first run: 2 [db-slow-writer] lines (both prune_node_changes COMMIT, 769 / 1016 ms) against 16 for today's prunes in the same pass; next day 0, nothing to delete 0. Full table in the PR description
bash scripts/check-xss-sinks.sh --diff origin/master	[T] no frontend changes to scan
No new map[string]interface{} outside tests; workflows untouched	[T] git diff origin/master...HEAD adds none; .github/ diff is empty

[T] The first attempt at the two full Go suites hit Go's default 10-minute -timeout while both ran in parallel with the mutant runs, with no failing test before the timeout. They were rerun with -timeout 45m (CI uses 20m) and passed.

Branch

[T] origin/master 5cdeffd5 was merged in with a merge commit (259684f3, no conflicts) before the round-2 commits. Master has since moved to e8e99c4a (frontend-only changes), and git merge-tree shows that it still merges cleanly, so no second merge was made. No rebase, amend or force-push; the push was a fast-forward 398af2b6..7228e083.

CI (run 37622679705, pull_request, head 7228e08, attempt 1)

Job	Result
Go Build & Test	[T] pass (20m3s)
Playwright E2E Tests	[T] pass (23m13s)
Build & Publish Docker Image	[T] pass (48s)
Release Artifacts	skipped (tag-only)
Deploy Staging	skipped (push/fork-guarded)
Publish Badges & Summary	skipped (push-only)

The known flaky tests #271 and #301 did not fail; no job was re-run.

Remaining

• [K] The round-1 feature commit 7ebf4e7e still names ping_triggers in its subject. History was not rewritten (no amend or force-push); the round-2 commits record the removal.
• [A] On first enable, the backlog delete can still emit one or two [db-slow-writer] COMMIT lines (run 3: two in prune_node_changes). Today's prunes emitted 16 in the same pass. The daily steady state emitted none in all three runs.
• [K] Like the other retention values, the three knobs are read at ingestor startup, not hot-reloaded.
• No staging or production validation was done. port(upstream#1886): opt-in observerPurgeDays hard-delete for long-inactive observers #34 is left open; the description says this PR supersedes it. The PR stays a draft.

[dborup]

Review — CS-Minimax PR#350 — head 7228e08

Dom: APPROVE med nits

Independent, read-only round-2 review. Evidence: [T] = I executed it, [A] = analysis, [K] = read in code/docs. Everything below was run on the merged tree (git merge-tree --write-tree origin/master 7228e083 → tree 2dd68837, merged clean against current master e8e99c4a) in a throwaway checkout, never in a working copy. git ls-remote showed 7228e083 before and after the review.

Findings

#	Sev	Where	Finding
1	nit	commit 7ebf4e7e subject	The round-1 feature commit still reads "…inactive_nodes, node_changes, ping_triggers and observers". Self-reported, and history was deliberately not rewritten; PR title and description are clean. Only visible if this lands as a merge commit rather than a squash. [K]
2	nit	cmd/server/ping_score_history_retention_329_test.go	The Ping Scores acceptance criterion is a guard, not a red-before test: I copied the file onto master's tree and it passes unchanged. That is the right shape for a "no regression" criterion, but it is the one acceptance item with no red-before state. [T]
3	nit	docs/user-guide/configuration.md	The observerPurgeDays table row says "once no packet, metric or dropped-packet row references them" and does not name observer_neighbor_metrics, which is a fourth guard. config.example.json does name it. [K]
4	nit	#329 issue body	The issue still lists a pingTriggerDays knob and asks ping_triggers to "follow the transmission prune". The operator comment of 2026-10-07 overrides it, so the issue text is now out of sync with the agreed scope. I did not change the issue. [K]
5	nit	docs	Enabling inactiveNodeDays without observerPurgeDays eventually drops the inactive_nodes row of an observer that stopped uploading more than N days ago, so the observer→node cross-link falls back to "no node record at all" instead of the inactive-node page. That is retention working as asked, but the user guide only documents the New Nodes consequence. [A]
6	info	PR state	Still a Draft. [K]

No blocking finding. No behaviour change outside the three opt-in knobs: with every knob unset the only added work is three calls that return immediately, which I also confirmed empirically on a real fixture DB (point 2 below). [T][A]

The review points

1. ping_triggers must no longer be touched — no knob, no code, and a test that pins it. Confirmed.

• [K] pingTriggerDays / PingTriggerDays appear nowhere in the tree except three test assertions that require them to be ignored and undocumented. cmd/ingestor has no retention code for the table; the only remaining ping_triggers writes are the pre-existing insert path and the hash-migrate dedupe, both untouched.
• [K] maintenance.go is not in the diff at all, so PruneOrphanRouteMaskChanges is byte-identical to master — the round-1 deleteInKeyRanges detour is gone.
• [T] Red before: on the round-1 head merged with master (259684f3) with only table_retention_test.go swapped in, all three round-2 tests fail: TestTableRetentionConfig/pingTriggerDays_ignored (TableRetention() returned PingTriggerDays:30, Enabled()=true), TestConfigExampleDocumentsTableRetention (both docs listed the knob) and TestTableRetentionLeavesPingTriggers (rows changed). All three are green on 7228e083.
• [T] TestTableRetentionLeavesPingTriggers cannot pass vacuously: it also requires the other three tables to be pruned, and my vacuity mutant (M6) fails it on exactly those three assertions.

2. Per knob: old rows removed, recent kept, unset changes nothing, batched inside WriterTx. Confirmed for all three.

• [K] deleteInBatches loops WriterTx(tag, …) with a LIMIT ? of retentionBatchRows (1000) until a batch deletes fewer than the limit; PurgeStaleObservers does the same by hand so that its candidate SELECT and the two DELETEs share one transaction. WriterTx holds the process-global writer mutex across BEGIN..COMMIT, so the observer guards are evaluated and acted on atomically with respect to every other ingestor writer — no TOCTOU window. [A]
• [K] inactive_nodes walks idx_inactive_nodes_last_seen and skips both a node that came back (a nodes row exists) and the node of an observer seen since the cutoff — the latter predicate is staleNodesWhere verbatim, including the lower() on both sides and the id IS NOT NULL that keeps a single NULL from poisoning NOT IN. node_changes walks idx_node_changes_detected_at with ORDER BY detected_at; both indexes exist in internal/dbschema.
• [K] The observer purge guards on observations.observer_idx (rowid), observer_metrics, observer_neighbor_metrics and dropped_packets, and deletes observer_neighbors with the row. I enumerated the schema: those four plus observer_neighbors are the only tables keyed on an observer (client_observers is pubkey-keyed mobile-client data, unrelated), so the guard set is complete. [A] Rowid reuse after a hard delete is safe precisely because the observations guard means nothing points at the freed rowid.
• [T] Red before (per criterion), via my vacuity mutant M6 (all three prunes return 0, nil = master behaviour): TestPruneInactiveNodesDeletesOldKeepsRecent, TestPruneInactiveNodesBatches, TestPruneNodeChangesDeletesOldKeepsRecent, TestPruneNodeChangesBatches, TestPurgeStaleObserversDeletesUnreferencedRow, …DeletesNeighborSnapshot, …Batches, …SkipsNullID and TestTableRetentionLeavesPingTriggers all fail. The "unset changes nothing" criterion is red under M4, the batching criterion under M3, and the config.example.json criterion is red when master's config.example.json and user guide are put back (retention.inactiveNodeDays = <nil>, …). [T]
• [T] On a real DB, not just the unit store: I ran runTableRetention against a copy of the CI e2e-fixture.db (migrated as in CI, then seeded with a 200-day and a 1-day ping_triggers row, a 200-day and a 2-day node_changes row, a 200-day and a 9-day inactive_nodes row, and one soft-deleted 200-day observer with no observations and an observer_neighbors snapshot). Pass 1 with every knob unset changed nothing, ping_triggers md5 unchanged. Pass 2 with all three at 30: the old inactive_nodes and node_changes rows went, the recent ones stayed, the Active observers vanish as nodes after retention.nodeDays without an advert; observer link dead-ends on 'Node not found' #199 seeded node stayed, the observer was hard-deleted with its neighbour snapshot (31 → 30 observers), ping_triggers md5 byte-identical. Pass 3 with inactiveNodeDays: 5: the 9-day row went but the Active observers vanish as nodes after retention.nodeDays without an advert; observer link dead-ends on 'Node not found' #199 node was still kept by the still-uploading-observer guard, ping_triggers md5 unchanged.

3. Perf on a large DB. Reproduced independently and the reported shape holds; my numbers are better than the author's, on a faster disk.

[T] CORESCOPE_RETENTION_PERF=1 go test -run TestTableRetentionTiming on the merged tree. Seeded fixture 5.34 GB (same as reported), seed 4m42s, test 527.8s, PASS, 0 ping_triggers deleted in every pass.

Pass	Step	[db-slow-writer]	writer-lock hold max	ingest-probe max wait	wall
first run (30 d, 335 d backlog)	today's prunes	0	prune_packets 207.7 ms (8401 tx), prune_metrics 443.8 ms	437.6 ms	3m56.9s
first run	#329 table retention	0	prune_inactive_nodes 10.0 ms (19 tx), prune_node_changes 21.0 ms (459 tx, p99 18.7), purge_observers 20.5 ms (1 tx)	30.0 ms	4.3s
next day (29 d)	today's prunes	0	prune_packets 56.8 ms, prune_metrics 43.8 ms	73.9 ms	3.4s
next day	#329 table retention	0	2.5 / 4.6 / 0.4 ms	3.9 ms	10 ms
nothing to delete	today's prunes	0	0.4 ms	0.0 ms	0s
nothing to delete	#329 table retention	0	—	0.0 ms	2 ms

Deleted on the first run: inactive_nodes 18,000, node_changes 458,912, observers 225, ping_triggers 0 — matching the reported run 3 within the seed's time drift. The worst write-lock hold the new code took on a 5.34 GB DB was 21 ms, and ingest never waited more than 30 ms — an order of magnitude below what today's prunes cost in the same pass. My run produced no [db-slow-writer] line anywhere, so I could not reproduce the two prune_node_changes COMMIT lines reported for run 3; [A] those are commit/checkpoint stalls of the disk, not of the batch size, which is consistent with today's prunes having emitted 16 of them in the same reported pass and none in mine. Every batch is bounded at 1000 deleted rows and walks an index, so the complexity argument holds. [A]

4. Writes, docs, title/description. Confirmed.

• [T] git diff origin/master <merged tree> --name-only: the only non-test files are cmd/ingestor/config.go, cmd/ingestor/main.go, cmd/ingestor/table_retention.go, config.example.json and docs/user-guide/configuration.md. cmd/server's two changed files are both _test.go, so the server stays read-only; TestServerDBHasNoWriteMethods now also forbids PruneInactiveNodes, PruneNodeChanges and PurgeStaleObservers.
• [K] The three knobs are documented in config.example.json (all at 0, plus a _comment_tableRetention that explains each one, the ordering advice and why ping_triggers is kept) and in the user guide.
• [T] Title: "feat(ingestor): opt-in retention for inactive_nodes, node_changes and observers (retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329)" — no ping_triggers. Description uses "Relates to retention: opt-in pruning for inactive_nodes, node_changes, ping_triggers and soft-deleted observers #329"; no closing keyword in the title, the body or any of the six commit messages.

Always-checks

Check	Result
Every acceptance criterion has a test, red before / green after	[T] yes for the per-knob deletes, the unset-knob, the batching and the config.example.json criterion (red under my M6/M4/M3 mutants and with master's docs restored); the Ping Scores criterion is a guard that is green on master — finding 2
No behaviour change beyond the purpose	[T][A] unset knobs touch nothing (verified on the real fixture); maintenance.go is out of the diff entirely
cmd/server read-only	[T] only _test.go files changed there; invariant test extended
No new map[string]interface{} outside tests	[T] the diff adds none anywhere
No hardcoded colors	[T] no added line matches a hex/rgb/hsl color; no frontend or CSS file in the diff
scripts/check-xss-sinks.sh --diff origin/master	[T] "no public/**/*.{js,html} changes to scan" — [A] consistent with the diff touching no public/ file
Fork-guards 9 and 1	[T] deploy.yml 9, release-fast-path.yml 1; .github/ diff is empty
No closing keywords	[T] none in title, body or commits
Commit author	[T] all six commits author and committer dborup <kontakt@meshview.dk>

Tests I ran (merged tree, against master e8e99c4a)

Suite	Result
cd cmd/ingestor && go test -count=1 -timeout 45m ./...	[T] ok, 108.8s
cd cmd/server && go test -race -count=1 -timeout 45m ./...	[T] ok, 529.1s
go vet ./... in both modules	[T] clean
sh test-all.sh	[T] 230 passed, 0 failed (230 files)
node test-frontend-helpers.js	[T] 709 passed, 0 failed
test-e2e-playwright.js against a local Go server on the CI-prepared e2e-fixture.db (freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073, 199 and 245)	[T] 132/135 passed, 3 skipped
test-issue-199-inactive-observer-e2e.js	[T] 3/3
test-issue-2073-recent-adverts-e2e.js	[T] 10/10
test-issue-1639-observers-sort-e2e.js, test-observer-iata-1188-e2e.js	[T] pass
/api/ping-scores, /api/analytics/node-changes, /api/observers	[T] 200
TestTableRetentionTiming (opt-in, 5.34 GB)	[T] PASS, table above
Real-fixture retention run (three passes, ping_triggers md5 pinned)	[T] PASS

The server was started on a free high port and stopped by looking the listener up by port, never with $!. [T] One environment note: with a server built outside a git checkout, /api/health reports commit: unknown, so public/perf.js renders no Version card and test-e2e-playwright.js fails fast on "Version info lives on Perf dashboard, not in navbar". [T] It fails identically on master's tree under the same conditions, and CI avoids it because the server runs inside the checkout, where resolveCommit() falls back to git rev-parse. I reproduced CI by writing a .git-commit file; after that the suite ran to completion.

CI

[T] Run 37622679705, pull_request, head 7228e083, attempt 1, no re-runs. Per job: Go Build & Test pass (20m3s), Playwright E2E pass (23m13s), Build & Publish Docker Image pass (48s); Release Artifacts, Deploy Staging and Publish Badges skipped (tag-/push-/fork-gated). Neither known flake (#256 Hash Stats sort, #267 backfill write-hold) appeared.

My mutants (7, all killed)

Applied one at a time to a copy of the tree, each reverted from a pristine snapshot before the next.

#	Mutant	Killed by
M1	pruneInactiveNodesBatch loses the NOT EXISTS (… FROM nodes …) returned-node guard	TestPruneInactiveNodesKeepsReturnedNode [T]
M2	the observer purge loses the observer_neighbor_metrics guard	TestPurgeStaleObserversKeepsObserverWithNeighborMetrics [T]
M3	deleteInBatches returns after the first batch	TestPruneInactiveNodesBatches, TestPruneNodeChangesBatches (deleted=2 left=4, 1 tx instead of 3) [T]
M4	PruneNodeChanges guards on days < 0, so an unset knob prunes with cutoff = now	TestRunTableRetentionUnsetChangesNothing, TestRunTableRetentionEachKnob [T]
M5	PruneNodeChanges also deletes ping_triggers older than its cutoff	TestTableRetentionLeavesPingTriggers [T]
M6	vacuity / master behaviour: all three prunes return 0, nil	9 tests, incl. every "deletes" test and TestTableRetentionLeavesPingTriggers on its non-vacuity assertions [T]
M7	the observer purge leaves observer_neighbors behind	TestPurgeStaleObserversDeletesNeighborSnapshot [T]

Not verified

• No staging or production validation, and no server API key used — out of scope by instruction.
• The real ingestor binary end-to-end (it needs an MQTT source to pass the connect gate). I covered the same ground with the real-fixture run above, which exercises the production schema and the real WriterTx path but not main()'s startup ordering; that ordering I only read. [K]
• CORESCOPE_RETENTION_PERF was run once, not repeated, and on a different disk than the reported runs, so the absolute millisecond figures are not comparable to run 3's — only the relative claim is.
• scripts/check-xss-sinks.sh was run in a non-git export, so it reported "nothing to scan" rather than diffing; the conclusion rests on the file list of the diff. [A]
• The Go suites were run without the ingestor -race flag (matching CI, which only races cmd/server).
• The other CI jobs' content (channel lib, decrypt CLI, Docker image, css-vars lint) — taken from the green CI run, not re-run locally.
• Whether #34 is actually closed when this lands; I only read that the description says it supersedes it.