bug: /api/nodes region filter attributes by observer, not repeater home region (leaks cross-region on relay/propagation anomalies)

We run a regional deployment (multiple IATA-tagged observers a couple hours apart by RF) and hit a variant of #804, but in /api/nodes instead of analytics.

GetNodes in cmd/server/db.go attributes a node to a region if any observer tagged with that IATA has ever heard one of its adverts, full stop, no time bound:

// cmd/server/db.go — GetNodes
subq := fmt.Sprintf(`public_key IN (
    SELECT DISTINCT JSON_EXTRACT(t.decoded_json, '$.pubKey')
    FROM transmissions t
    JOIN observations o ON o.transmission_id = t.id
    JOIN observers obs ON %s
    WHERE t.payload_type = 4
    AND UPPER(TRIM(obs.iata)) IN (%s)
)`, joinCond, strings.Join(placeholders, ","))

A few days ago we got a tropospheric ducting event that let one of our observers briefly hear repeaters from a neighboring region much farther than normal RF range would allow. Because this query has no time window, that one-time fluke permanently qualifies those repeaters for the wrong region filter now — they'll keep showing up under the wrong region's map view forever, since nothing ever ages an observation back out.

Same underlying issue as #804, just showing up in the node list/map instead of analytics.

Proposed fix

#804 already solved this for analytics with computeNodeHomeRegions() in cmd/server/store.go — it derives a node's home region from zero-hop DIRECT adverts only, since the path byte shows those weren't relayed, so the observer that heard it has to actually be in RF range. Seems like the right fix here too: attribute GetNodes/handleNodes the same way, falling back to the old observer-based check when a node has no zero-hop data yet (same fallback #1025 already uses).

Two things worth calling out before anyone starts on this, since /api/nodes is a much hotter path than the analytics tab ever was:

computeNodeHomeRegions() walks every packet currently in memory and unmarshals the JSON on each ADVERT, under s.mu.RLock(). It's fine today because its only caller sits behind GetAnalyticsHashSizes's TTL cache. /api/nodes isn't cached at all and gets polled continuously by the live map, so calling this straight from handleNodes would bring back exactly the kind of request-path cost that perf(nodes): /api/nodes (no limit) takes 32.9s vs /api/nodes?limit=2000 at 4.9s — load-then-paginate bug #1257/perf: P0 hot-path fixes (observers, neighbor-graph, observer-analytics) (#1481) #1483/perf(#1239): /api/analytics/distance — TTL 15s→60s + drop main RLock around compute #1241 all had to go fix. This probably wants its own recomputer instead — same shape as bridge_recomputer.go (background ticker, atomic snapshot, synchronous prewarm on startup) — so handleNodes can read it the same way it already reads the bridge/coverage/redundancy maps.
computeNodeHomeRegions() only sees packets still sitting in the in-memory store, which is bounded by packetStore.retentionHours — 48h in our config. So it only has a home-region answer for nodes that sent a zero-hop advert in roughly the last two days; anything quieter than that falls back to the old (broken) check. That's fine going forward but it won't retroactively fix a node whose one zero-hop advert has already aged out of memory.
Plurality voting on home region means a node that's genuinely zero-hop-audible to observers in two different regions (sitting right on a boundary, legitimately in RF range of both) gets collapsed to a single home region. That trades our false-inclusion bug for a different, narrower false-exclusion case on real boundary nodes — already true of Bug/feature: Use actual repeaters region data and not observer based #804/fix(#804): attribute analytics by repeater home region, not observer #1025 today, not something new this would introduce, but worth having in mind since it's the opposite failure mode from what we're fixing.

Happy to build this and open a PR (red/green per your usual convention) if this shape looks right to you — wanted to run it by you first given how sensitive this endpoint is to extra per-request work.

Ran through this while looking for unclaimed bugs and ended up reading the surrounding code closely. Not picking it up — you offered to build it and you're waiting on a shape review, so this is meant as input on that shape rather than a competing proposal. Four observations, one of which I think changes the recommendation.

The bug is worse than the issue argues

The subquery filters on t.payload_type = 4 and nothing else — no time bound, as you say, but also no hop bound:

SELECT DISTINCT JSON_EXTRACT(t.decoded_json, '$.pubKey')
FROM transmissions t
JOIN observations o ON o.transmission_id = t.id
JOIN observers obs ON ...
WHERE t.payload_type = 4
AND UPPER(TRIM(obs.iata)) IN (...)

Adverts are flooded. So it isn't only a ducting event that qualifies a node for the wrong region — any advert that reaches an observer in region X after being relayed does it, which is ordinary mesh behaviour rather than a rare anomaly. The tropo event is what made you notice, not what caused it.

Your proposed fix handles this correctly, since zero-hop is exactly the right discriminator. computeNodeHomeRegions() already encodes that judgement (store.go:3737-3739, hopCount != 0 → continue) — #804 concluded relayed adverts are worthless for location. You're just underselling the fix: it's not an edge-case cleanup, it's the common case.

The fallback preserves the bug where it's least checkable

computeNodeHomeRegions() reads s.packets, so it only sees the in-memory window (packetStore.retentionHours, 168 default, 48 on yours). You note this. What I'd draw out further: falling back to the observer-based check for nodes without recent zero-hop data means the long tail of quiet nodes keeps the current behaviour — and those are precisely the nodes whose region you have no independent way to verify. Active nodes get fixed, the rest stay wrong, and the result looks like a complete fix from the outside.

Plurality voting is the wrong trade here, not just a narrower one

You flag that a genuinely boundary node collapses to one region. I'd argue it's strictly worse than what it replaces, in this specific surface: an extra node on a map is visible and dismissable, a missing one is indistinguishable from a node that doesn't exist. A filter wants set membership; computeNodeHomeRegions() returns map[string]string because analytics wanted a single label. Reusing it for filtering imports a false-exclusion problem to solve a false-inclusion one.

Whatever mechanism you land on, I'd let a node carry more than one home region. It costs nothing extra and removes the only regression this change would introduce.

The mechanism I'd suggest instead: persist it in the ingestor

There's precedent — UpdateNodeDefaultScope (cmd/ingestor/db.go:1610) maintains a per-node region-ish column at ingest, with a read-before-write short-circuit so the hot path doesn't write on every packet. Different field (transport scope, not observer IATA), same shape.

A node_regions(public_key, iata, count, last_seen) table the ingestor updates when it sees a zero-hop DIRECT advert would:

Remove the memory window. No fallback needed, so no long tail left broken.
Make the filter cheaper, not costlier. The region filter becomes an indexed join instead of the correlated subquery over transmissions × observations that runs today. That dissolves the perf concern that's driving the recomputer, rather than engineering around it — which matters given /api/nodes is uncached and map-polled.
Need no recomputer at all. One fewer piece of derived state in the server.
Give set semantics for free. Boundary nodes get two rows.
Give decay for free. last_seen per (pubkey, iata) row means a node that relocates loses its old attribution. The in-memory version gets decay too, but only inside the retention window.
Keep bug(db): vacuumOnStartup fails with SQLITE_BUSY when ingestor + server share DB (auto_vacuum migration #919 broken in single-container topology) #1283 intact. Writes stay in the ingestor; a recomputer in the server is one more derived-state owner on the read-only side.

Honest cost: schema migration, ingestor changes, and a backfill — more surface and slower to land than your version. The backfill can only see what transmissions still holds, so it has its own coverage limit, though packetDays retention is typically far more generous than retentionHours.

If upstream prefers to keep this small, the recomputer is a defensible first step and I wouldn't argue hard against it. I'd just drop the plurality collapse either way.

🤖 Generated with Claude Code

The shape is right, and yes please build it. Two things to settle first, one of which I would like changed from the #804 design rather than carried over.

Zero-hop attribution: agreed. A node is in a region because an observer in that region heard it directly, not because a relay chain eventually reached one. That is the whole bug, and reusing the rule #804 established keeps one definition of "home region" in the codebase instead of two.

The background recomputer: agreed, and thank you for raising it before writing code. /api/nodes is polled continuously by the live map and is not cached, so a per-request walk over the in-memory store with a JSON unmarshal per advert is exactly the class of regression #1257 and #1483 had to undo. The bridge_recomputer.go shape (ticker, atomic snapshot, synchronous prewarm at startup) is the right precedent, and it lets handleNodes read this the same way it already reads the bridge and coverage maps.

The part I would change: return a set, not a winner. computeNodeHomeRegions() returns map[string]string and picks by plurality, with an alphabetical tie-break on equal counts:

if n > bestCount || (n == bestCount && iata < bestIATA) {

For a node genuinely audible zero-hop from two regions, that decides its region by which IATA code sorts first. You named this as the opposite failure mode and said it is pre-existing rather than new, which is true, but it is the failure mode that matters most on exactly the deployment shape you describe. A node on a boundary is not in one region that we have to guess: it is in both, and the operator on either side has a legitimate claim to see it.

So for this endpoint I would like map[string][]string, with the filter including a node when any of its home regions matches the selection. The query side already takes a multi-value IN (...), so inclusive membership needs no UI change and no new concept: it is the same "show me these regions" the filter already means.

Whether #804 and #1025 should move to the same shape is a separate question and not a condition on this PR. I would rather not have two definitions for long, though, so if the set version works out here, converting the analytics path afterwards is the natural follow-up.

One thing to make visible rather than silent. As you note, this only has an answer for nodes that sent a zero-hop advert inside packetStore.retentionHours, and anything quieter falls back to the old observer-based check. That fallback is the buggy behaviour, so a node can still be attributed wrongly, just less often. If the response can say which of the two answered for a node, an operator chasing a surprising region has something to look at instead of a number that is sometimes right.

Red/green per the usual convention is fine. If the set change turns out to complicate the recomputer more than it is worth, say so and we will talk about it rather than you working around it silently.

One consequence of the set version, worth writing down before it is built rather than fielding it as a bug report later.

If a node can be in two regions, then per-region counts no longer sum to the node total. A boundary node heard zero-hop by an observer in each of two regions is counted once in each filter, so "BRU: 412" and "AMS: 380" against 700 nodes total is arithmetic working correctly, not double counting.

That is fine as behaviour and confusing as a surprise, so:

wherever a count is shown next to a region, it should be safe to read as "nodes visible in this region" rather than "nodes belonging to this region"
the two are the same number today only because the current attribution is exclusive, and it is exclusive by making a wrong choice on exactly the nodes this fixes

It also means the multi-region case is worth a test of its own: a node with zero-hop adverts heard by observers in two IATAs appears in both filters, and disappears from neither when the other is selected. That is the property the set exists for, and it is the one a later refactor is most likely to collapse back to a single winner without noticing.

Nothing else changes about the plan in my previous comment.

fix(nodes): resolve the /api/nodes region filter from the packet store (

@anieto

@anieto, are you still working on this? The shape was agreed on 2026-09-09 and I haven't seen a branch or PR since. It's no problem if it has dropped off your list: just say so and someone else can pick it up, building on the design in this thread. If you are still on it, a draft PR, even unfinished, would let others see that it's taken.

Place nodes in regions by direct hearings, durably

Yep, still on it, sorry for going quiet. Draft PR is up: #2190

Went with the set version like you asked. Nodes heard zero-hop get placed by their home regions (same rule as #804, so the counting is now shared with analytics), everything else falls back to the old check, and node rows get a home_regions field when zero-hop data is what placed them, so you can tell which rule answered. The two-region case has its own test so it can't quietly collapse back to a single winner.

A couple of things I left out on purpose and noted in the PR: the memory window (your node_regions table idea is the real fix there, happy to do it as a follow-up) and a share threshold for one-off long-range hearings. We've been running a version of this on our network and the threshold helped with tropo hearings, but I didn't want to sneak it in without asking.

test(nodes): region fallback places never-zero-hop nodes by fewest ho…

fix(nodes): place never-zero-hop nodes by their fewest hops, capped a…

test(nodes): region filter attributes nodes by zero-hop home regions (K…

fix(nodes): place nodes in regions by zero-hop home regions (Kpa-claw…

test(nodes): region fallback places never-zero-hop nodes by fewest ho…

fix(nodes): place never-zero-hop nodes by their fewest hops, capped a…

perf(nodes): time the whole region recompute, and cut it ~23% (Kpa-cl…

test(nodes): /api/nodes says which rule placed each node (Kpa-clawbot…

fix(nodes): return region_placement on every node (Kpa-clawbot#1879)

test(nodes): regionFallback.maxHops sets the fallback hop cap (Kpa-cl…

fix(nodes): make the region fallback hop cap configurable (Kpa-clawbo…

test(nodes): an empty path_json is an unknown hop count (Kpa-clawbot#…