[dborup]

Summary

Port upstream Kpa-clawbot/CoreScope#2102 to this fork: cache /api/nodes?region= node membership by normalized region set, reuse it across count/page requests, refresh additions incrementally after 30 seconds, and rebuild fully every 30 minutes for retention and IATA changes.

Fork-specific changes

• Keep the fork's legacy ADVERT behavior while its asynchronous from_pubkey backfill is incomplete: fall back to decoded_json.pubKey for rows whose from_pubkey is NULL.
• Fix an upstream review finding: replacing an existing cache entry at the 32-entry limit no longer clears the other 31 entries.
• Add focused regression tests for cache reuse, the 32-entry replacement, legacy backfill/delta refresh, and full rebuild after pruning.

Performance and behavior

• The cache is bounded to 32 region sets. A cold region set needs one full observation scan; fresh subsequent pages do not scan membership again. TestNodeRegionCachePortMembershipAndReuse enforces one scan across three equivalent region requests.
• Stale entries are served while one background refresh runs. Region additions may take up to 30 seconds to appear; removals or IATA changes may take up to 30 minutes. This is the upstream cache contract, not immediate-consistency behavior.
• No schema, ingestor, frontend, workflow, configuration or customizer changes.

Verification

• Full cmd/server Go test suite and go vet ./... passed locally.
• Focused region tests passed with -race.
• Full cmd/ingestor Go test suite passed locally. The first sandboxed run could not bind the local MQTT test sockets; rerunning with local test sockets allowed passed.
• Browser validation after push: a local, migrated copy of the E2E fixture was served on loopback. The Nodes page rendered 202 rows; selecting SJC narrowed it to 20 rows, and selecting All restored 202. No browser console errors. The local server was stopped afterward. No staging or production contact.

This is a port for review, not a deployment. No issue-closing keywords or auto-merge.

Related open PR #38 also changes the Nodes region query to use from_pubkey; review its overlap before merging either PR.

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#176 overtagelse — head a8541c0

Review feedback addressed (commit a8541c09, on top of merge commit 54297082)

PR #176 had never been reviewed, so the findings below are the ones from the
takeover brief. Blocking ones first.

1. Merge origin/master, keep #38's query inside the cache [BLOCKING]

Merged origin/master (5cdeffd5) with merge commit 54297082. One conflict,
cmd/server/db.go, in two places: the DB struct fields and the region branch
of GetNodes.

The branch's cache scanned with
COALESCE(t.from_pubkey, JSON_EXTRACT(t.decoded_json,'$.pubKey')). Master's #38
measured that fallback and rejected it: it rescues 0 rows (on staging all 25
ADVERTs with a NULL from_pubkey have no pubKey in decoded_json either), it
re-parses decoded_json for every NULL row, and one corrupt ADVERT fails the
whole query with "malformed JSON". Behind this cache that last point is strictly
worse than before: the failing scan is a refresh, so the entry stays stale for
every later request, not just the one that triggered the error.

scanNodeRegionKeys now selects DISTINCT t.from_pubkey with
t.from_pubkey IS NOT NULL, and #38's comment block — including THE TRAP
paragraph — moved from db.go to the query it documents. db.go keeps a
four-line pointer to it.

[T] Red before the fix, from master's own test, which the merge pulled in:

--- FAIL: TestGetNodesRegionFilter_MalformedAdvertJSONDoesNotFailQuery_PR38
    nodes_region_from_pubkey_test.go:262: GetNodes(region=SJC) with one corrupt
    ADVERT in the region: nodes-region SJC: scan region keys: SQL logic error:
    malformed JSON (1)

[T] The port's TestNodeRegionCachePortLegacyBackfillAndRefresh asserted the
opposite — that a NULL from_pubkey row is rescued by JSON_EXTRACT — so it is
replaced by TestNodeRegionCacheDeltaFollowsFromPubkeyContract_176, which locks
#38's contract on the delta path specifically: a stamped ADVERT is added, a
NULL-from_pubkey ADVERT does not place its node in the region, and a corrupt
decoded_json row in the same region does not fail the refresh.

2. Upper bound and eviction [BLOCKING]

The bound was 32 entries, but it was enforced by replacing the whole map:

if db.nodeRegionCache == nil || (!replacing && len(db.nodeRegionCache) >= nodeRegionMaxEntries) {
    db.nodeRegionCache = make(map[string]*nodeRegionEntry)
}

?region= is a query parameter, so this is reachable from outside: a client
cycling through 33 distinct region sets emptied the cache on every request, and
every following request paid a full observation scan — serialised behind
nodeRegionFullMu, ~1s each on the measured database. That is the exact
collapse the cache exists to prevent, and it was one query parameter away.

Fixes:

• setNodeRegionEntry now evicts the least recently used entry, one at a
  time. getNodeRegionEntry marks a read as a use, so eviction drops the sets
  nobody is paging through rather than the ones in use. The LRU tick is a
  counter under the same mutex, not a clock.
• nodeRegionKey caps a stored key at nodeRegionMaxKeyBytes (256) and stores
  anything longer as "sha256:" + 64 hex — normalizeRegionCodes puts no limit
  on how many codes ?region= may carry. This follows channelListStoreKey's
  rule, including why a digest cannot collide with a short key (normalized codes
  are upper-case).

So: at most 32 region sets, each key at most 71 bytes once digested, each entry
at most one pubkey per node in the region set.

[T] TestNodeRegionCacheEvictsOneLRUEntryAtCapacity_176 fills to the bound,
reads R00 so R01 becomes the victim, inserts a 33rd and asserts that exactly
R01 is gone, the other 31 survive, and the LRU bookkeeping has no leftovers.
TestNodeRegionCacheStaysBoundedUnderChurn_176 inserts 4x the capacity and
checks the bound after every insert.
[T] TestNodeRegionCacheLongRegionSetKeyIsBounded_176: 2000 codes, digest key,
order/duplicates still collapse to one identity, two different long sets still
get different keys, short keys still stored verbatim.

3. Freshness

The documented contract — additions within ~30s, removals and IATA changes
within 30 min — had no test, and one hole: a stale entry was served
unconditionally while a background refresh ran, so if refreshes kept failing
(the corrupt-ADVERT case in finding 1 is one way) the cache served membership of
unbounded age with nothing but a log line to show it.

nodeRegionKeysJSON now has three cases instead of two. Below
nodeRegionFreshTTL: serve. Below nodeRegionMaxStale (= the rebuild interval):
serve and kick one background refresh — the fast path every page uses. At or
above it: wait for the refresh, which at that age is a full rebuild, so what
is served is inside the documented age. A refresh that fails still falls back to
the stale entry; an outage of the observation scan must not turn
/api/nodes?region= into a 500. The cost, stated plainly: the first region
request after 30 minutes with no region traffic pays one full scan, exactly as a
cold start does.

[T] TestNodeRegionCacheAdditionVisibleWithinFreshTTL_176 — a node first heard
after the entry was built is absent inside the TTL (so the cache is real), the
read at the TTL boundary is still served from cache and kicks one refresh, and
the next read has it.
[T] TestNodeRegionCacheRemovalGoneWithinRebuildInterval_176, two subtests —
observations pruned by retention, and the observer's IATA changed. Both assert
the node survives a delta refresh at half the rebuild interval (the delta scan
cannot see a removal; this is what makes 30 min the real bound) and is gone
after the refresh at the rebuild interval.
[T] TestNodeRegionCacheDoesNotServeBeyondMaxStale_176 — past nodeRegionMaxStale,
one call returns the new membership, not two.
[T] TestNodeRegionCacheOverStaleFallsBackWhenRefreshFails_176 — with the scan
failing, the over-stale read attempts exactly one scan before returning and
serves the stale set instead of an error.

nodeRegionQueryHook now returns an error so a test can fail a scan; the
port's one use is updated.

4. Perf proof (rule 0)

cmd/server/nodes_region_cache_perf_176_test.go, env-gated, following
tracked_bytes_perf_202_test.go's convention. It uses only identifiers that
exist on master (OpenDB, GetNodes, Close), so the same file was compiled
against origin/master and against this branch and the two binaries were run
alternately — the before/after difference is the cache, not a rewritten
measurement.

Database generated by the same file: 498 MB, 1 500 002 observations, 749 448
transmissions (1 in 5 an ADVERT), 4 000 nodes, 64 observers over 16 IATA codes,
ANALYZE run so the planner sees sqlite_stat1 the way a live database does.

Unit: one Nodes page load with a region selected = what fetchAllNodes() does,
4 GetNodes pages of 500. GetNodes evaluates the region predicate twice per
call (COUNT(*) and the page), so the uncached shape pays for it 8 times per page
load. 7 interleaved rounds, 1 cold + 3 warm page loads each; medians of the
per-round medians.

	before (master)	after (this branch)
warm page load (4 pages)	3729 ms	31 ms	121x
warm, per GetNodes call	932 ms	7.7 ms
cold page load (4 pages)	4292 ms	705 ms	6.1x
cold, per GetNodes call	1073 ms	176 ms

Master's 932 ms per region call sits inside the 1–1.4 s a region call costs on
prod, so the baseline is the right order of magnitude; the database is 498 MB
against prod's 1.4 GB, so the absolute numbers are conservative, not inflated.

All 7 rounds, page-load ms, sorted:

cold  before [2421, 2651, 2896, 4292, 5543, 8645, 9000]
cold  after  [ 437,  619,  650,  705,  927,  943,  967]
warm  before [3172, 3309, 3701, 3729, 4420, 4987, 5223]
warm  after  [29.4, 29.4, 30.3, 30.8, 46.2, 54.2, 75.2]

Cold is 6.1x rather than 121x because the one full membership scan is still
paid; it replaces the 8 subquery evaluations.

5. Correctness against the uncached query

[T] TestNodeRegionCacheMatchesUncachedForManyRegionSets_176 compares the
cached filter to an oracle built from the uncached subquery, for six region sets
(single, multi, mixed case with padding and duplicates, a region no observer
carries, and a mixed known/unknown set), each queried twice so the second answer
comes from the cache, then again after new ADVERTs arrive and every entry has
been taken past its TTL and refreshed. It fails if fewer than three sets match
any node, so it cannot pass vacuously.
[T] Master's TestGetNodesRegionFilter_FixtureMatchesJSONExtract_PR38 does the
same against the pre-#38 JSON_EXTRACT expression on the committed e2e fixture,
every region alone and combined — it now runs through the cache and passes.
[A] Against a local server on the prepared fixture, /api/nodes?region= matched
a sqlite3 oracle for all 8 sets tried (SJC, sjc, SJC,SJC,
MRY,OAK,SFO,SJC, sfo , SJC, ZZZ, ZZZ,SJC, All), totals and public
keys, on both passes; the server log shows 8 full builds for 8 distinct sets and
none for the repeats.

6. No other changes

Only cmd/server/db.go, cmd/server/nodes_region_cache.go and three test files.
No schema, ingestor, frontend, workflow, config or customizer change.
cmd/server stays read-only: the cache reads, and the only write SQL added is in
a _test.go file, which TestServerSourceHasNoNewWriteSQL does not scan.

Mutants

One per finding, each applied to nodes_region_cache.go, run, reverted.

mutant	result	tests that went red
M1 scan falls back to JSON_EXTRACT (the pre-merge branch query)	KILLED	…MalformedAdvertJSONDoesNotFailQuery_PR38, …DeltaFollowsFromPubkeyContract_176
M2 bound enforced by clearing the map instead of evicting one entry	KILLED	…EvictsOneLRUEntryAtCapacity_176
M3 stored key not capped for a caller-supplied region set	KILLED	…LongRegionSetKeyIsBounded_176
M4 stale entry returned without kicking a refresh	KILLED	…AdditionVisibleWithinFreshTTL_176, …RemovalGoneWithinRebuildInterval_176 (both subtests), …MatchesUncachedForManyRegionSets_176
M5 full rebuild never becomes due, only delta scans run	KILLED	…RemovalGoneWithinRebuildInterval_176 (both subtests), …PortFullRebuildDropsPrunedRegion
M6 over-stale entry served while its refresh runs in the background	KILLED	…DoesNotServeBeyondMaxStale_176

Tests run locally

[T] cmd/server: go build, go vet, go test ./... — ok, 48.7s.
[T] cmd/server with -race (what CI runs): go test -race ./... — ok, 460s.
[T] cmd/ingestor: go vet ./..., go test ./... — ok, 113.8s.
[T] All 14 internal/* modules — ok (4 have no test files).
[T] sh test-all.sh — 228 passed, 0 failed (228 files).
[T] node test-frontend-helpers.js — 709 passed, 0 failed.
[A] E2E against a local Go server (port 13801) on e2e-fixture.db prepared as
in CI (freshen-fixture, Kpa-clawbot#1486 seed, corescope-migrate, Kpa-clawbot#2073/#199/#245 seeds,
instrumented frontend):

• test-e2e-playwright.js — 132/135 passed, 3 skipped
• test-issue-1136-live-region-e2e.js — 5 passed, 0 failed
• test-map-nodes-pagination-e2e.js — 3 passed, 0 failed
• test-area-nodes-map.js — passed
• test-nodes-export-e2e.js — passed
  [A] Browser validation (rule 2), Nodes page at 1440x900: All = 204 rows,
  SJC = 20, SJC+SFO = 44, back to All = 204 — the same counts the API and the
  sqlite oracle give. Requests issued: ?limit=500&offset=0,
  ?limit=500&region=SJC&offset=0, ?limit=500&region=SJC%2CSFO&offset=0. No
  console errors. The local server was stopped afterwards. No staging or
  production contact.

Not changed, on purpose

• normalizeRegionCodes still accepts an unbounded number of codes, so a
  request with thousands of them still expands to thousands of SQL
  placeholders. That is unchanged from master and lives outside this cache; the
  cache's own key is now bounded regardless. Worth its own issue, not filed.
• The cache is still per-process and per-region-set with no cross-set sharing: a
  3-code set does not reuse the 3 single-code entries. Upstream's design; the
  bound makes it safe, and the measured win does not depend on it.

CI per job — run 37616793970, head a8541c09

job	result	time
Go Build & Test	pass	19m46s
Playwright E2E Tests	pass	26m16s
Build & Publish Docker Image	pass	—
Release Artifacts	skipped	—
Publish Badges & Summary	skipped	—
Deploy Staging	skipped	—

No flakes, no reruns. The known-unstable #271 and #301 did not fire.

PR stays a draft. No merge, no ready-for-review, no issue closed.