Skip to content

Register and place a node whose config carries no position - #24

Merged
Babissimo merged 1 commit into
mainfrom
feat/null-geometry
Sep 9, 2026
Merged

Babissimo merged 1 commit into
mainfrom
feat/null-geometry

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

Library half of the nullable-node-coordinates work. Merges before the
retina-server PR, which pins this branch's tip as its submodule.

Ticket: https://app.clickup.com/t/86cbauhxd

Why

A node's owner cannot always supply the receiver and illuminator coordinates
when setting a board up. The server side of this work (86cbauhxd) makes those
coordinates nullable and carries such a node without placing it: counted,
streaming, archived, and absent from the map, the overlap graph and the solver.

This package needs its own answer to the same question. It is registered
through NodeAnalyticsManager and InterNodeAssociator, both of which read
coordinates straight out of a config dict, and a null reaching the geodesy
raises TypeError: must be real number, not NoneType partway through
registration. The node is then left registered with the manager and absent
from the associator, which is a worse state than either outcome.

What changed

  • has_full_geometry is a new total predicate: never raises, for any input,
    including a non-dict or a coordinate slot holding something other than a
    number. Absent, null and the legacy (0, 0) sentinel all read as unset, on
    either end. Only the exact pair reads as unset, so the equator and the prime
    meridian each survive on their own.
  • _coord is total on the same terms, reusing _is_real_coordinate rather
    than reimplementing it. It also catches the OverflowError from an int too
    large to convert to a float, which the identity check deliberately cannot
    see because it never performs the conversion.
  • A detection area is built only for a positioned node, so the map marker is
    guarded by the data rather than by a filter at the drawing site.
  • Registration no longer rebuilds DetectionAreaState on a byte-identical
    resend, which was resetting n_detections and the delay/Doppler bounds on
    every reconnect. Cache invalidation now follows the same condition.
  • Stale neighbours are dropped and the overlap cache made symmetric, so a node
    that loses its position stops appearing in a neighbour graph built when it
    had one.

Deliberately not relying on the server

The server canonicalises every config at every point one enters shared memory,
so in the current deployment this package is never handed a bad coordinate.
The predicates are total anyway: this is a separate package with its own
consumers, and a predicate that raises on junk input is one careless caller
away from a silent, self-inflicted outage. The same reasoning applies to
retina-geolocator, which still reads coordinates with .get(key, 0) and is
tracked separately as 86cbbenw2.

Verification

438 tests pass. New coverage pins the totality of both predicates directly and
end to end through register_node, plus the three DetectionAreaState
outcomes: a byte-identical resend preserving accumulated state, a genuine
relocation replacing it, and a beam-width-only change still rebuilding.

🤖 Generated with Claude Code

@Babissimo
Babissimo marked this pull request as draft August 28, 2026 23:00
Babissimo added a commit to offworldlabs/retina-server that referenced this pull request Sep 7, 2026
…ff the map

The library's has_full_geometry/detection_area gating and validate_config's
position_status now have something to join: pin retina-analytics to the
commit on offworldlabs/retina-analytics#24 (local-only, pending that PR's
merge) and surface position_status on every node in /api/radar/nodes, which
the dashboard reads. /api/auth/me/nodes carries it too, computed the same
way: the published feed filters out private nodes, so a private positionless
node's owner otherwise had no way to learn it needed a position.
position_status is imported at module level in analytics_refresh, since
node_config imports only math and typing and so cannot cycle with it.

The new test file pins the two behaviours that must hold end to end: a
null-geometry node is counted and visible, but builds no solver pipeline and
joins no overlap zone. The pipeline guard's correctness is currently a
truthiness accident (86cbavanm), and these assertions catch it if the repair
regresses. Only a repair that checks key presence rather than value nullity
would start building pipelines for positionless nodes: an is-not-None repair
would not, since None is not None is False exactly as bool(None) is. The
overlap test registers ten nodes, following test_unpositioned_registration's
shape, so its zero-zones assertion can actually fail rather than hold
vacuously for a single node.

The _clean fixture enumerates its three node IDs explicitly rather than
scanning connected_nodes for a name prefix. That scan is safe in
test_unpositioned_registration, where every test registers through the real
detections route, but test_positionless_node_is_counted_but_not_placed
registers directly against node_analytics/node_associator and never touches
connected_nodes, so the scan would not find it and the node would leak into
the analytics and associator registries. Nothing would break today, because
conftest's autouse reset wipes both before every test, but the fixture should
not depend on that safety net.

_CONFIG is a shared module-level dict and association.py keeps a bare
reference to whatever it is passed rather than copying it, so the four call
sites that hand it to something stateful pass dict(_CONFIG). "Counted" is
asserted as record_detection_frame incrementing total_frames, not merely as
the node having a metrics entry: what the owner needs to see is that a
working receiver's frames are tallied even though it cannot be placed.

Two comments elsewhere described the library as it was before this pin: a
guard function that no longer exists, and a default-to-zero mechanism that
no longer runs. Both now describe has_full_geometry's both-ends rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Babissimo added a commit to offworldlabs/retina-server that referenced this pull request Sep 7, 2026
…ff the map

The library's has_full_geometry/detection_area gating and validate_config's
position_status now have something to join: pin retina-analytics to the
commit on offworldlabs/retina-analytics#24 (local-only, pending that PR's
merge) and surface position_status on every node in /api/radar/nodes, which
the dashboard reads. /api/auth/me/nodes carries it too, computed the same
way: the published feed filters out private nodes, so a private positionless
node's owner otherwise had no way to learn it needed a position.
position_status is imported at module level in analytics_refresh, since
node_config imports only math and typing and so cannot cycle with it.

The new test file pins the two behaviours that must hold end to end: a
null-geometry node is counted and visible, but builds no solver pipeline and
joins no overlap zone. The pipeline guard's correctness is currently a
truthiness accident (86cbavanm), and these assertions catch it if the repair
regresses. Only a repair that checks key presence rather than value nullity
would start building pipelines for positionless nodes: an is-not-None repair
would not, since None is not None is False exactly as bool(None) is. The
overlap test registers ten nodes, following test_unpositioned_registration's
shape, so its zero-zones assertion can actually fail rather than hold
vacuously for a single node.

The _clean fixture enumerates its three node IDs explicitly rather than
scanning connected_nodes for a name prefix. That scan is safe in
test_unpositioned_registration, where every test registers through the real
detections route, but test_positionless_node_is_counted_but_not_placed
registers directly against node_analytics/node_associator and never touches
connected_nodes, so the scan would not find it and the node would leak into
the analytics and associator registries. Nothing would break today, because
conftest's autouse reset wipes both before every test, but the fixture should
not depend on that safety net.

_CONFIG is a shared module-level dict and association.py keeps a bare
reference to whatever it is passed rather than copying it, so the four call
sites that hand it to something stateful pass dict(_CONFIG). "Counted" is
asserted as record_detection_frame incrementing total_frames, not merely as
the node having a metrics entry: what the owner needs to see is that a
working receiver's frames are tallied even though it cannot be placed.

Two comments elsewhere described the library as it was before this pin: a
guard function that no longer exists, and a default-to-zero mechanism that
no longer runs. Both now describe has_full_geometry's both-ends rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Babissimo added a commit to offworldlabs/retina-server that referenced this pull request Sep 7, 2026
…ff the map

The library's has_full_geometry/detection_area gating and validate_config's
position_status now have something to join: pin retina-analytics to the
commit on offworldlabs/retina-analytics#24 (local-only, pending that PR's
merge) and surface position_status on every node in /api/radar/nodes, which
the dashboard reads. /api/auth/me/nodes carries it too, computed the same
way: the published feed filters out private nodes, so a private positionless
node's owner otherwise had no way to learn it needed a position.
position_status is imported at module level in analytics_refresh, since
node_config imports only math and typing and so cannot cycle with it.

The new test file pins the two behaviours that must hold end to end: a
null-geometry node is counted and visible, but builds no solver pipeline and
joins no overlap zone. The pipeline guard's correctness is currently a
truthiness accident (86cbavanm), and these assertions catch it if the repair
regresses. Only a repair that checks key presence rather than value nullity
would start building pipelines for positionless nodes: an is-not-None repair
would not, since None is not None is False exactly as bool(None) is. The
overlap test registers ten nodes, following test_unpositioned_registration's
shape, so its zero-zones assertion can actually fail rather than hold
vacuously for a single node.

The _clean fixture enumerates its three node IDs explicitly rather than
scanning connected_nodes for a name prefix. That scan is safe in
test_unpositioned_registration, where every test registers through the real
detections route, but test_positionless_node_is_counted_but_not_placed
registers directly against node_analytics/node_associator and never touches
connected_nodes, so the scan would not find it and the node would leak into
the analytics and associator registries. Nothing would break today, because
conftest's autouse reset wipes both before every test, but the fixture should
not depend on that safety net.

_CONFIG is a shared module-level dict and association.py keeps a bare
reference to whatever it is passed rather than copying it, so the four call
sites that hand it to something stateful pass dict(_CONFIG). "Counted" is
asserted as record_detection_frame incrementing total_frames, not merely as
the node having a metrics entry: what the owner needs to see is that a
working receiver's frames are tallied even though it cannot be placed.

Two comments elsewhere described the library as it was before this pin: a
guard function that no longer exists, and a default-to-zero mechanism that
no longer runs. Both now describe has_full_geometry's both-ends rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Babissimo
Babissimo marked this pull request as ready for review September 8, 2026 09:50
Registration has to split along the line between what a node is and where
it is: a node that cannot say where it is still has an identity, still
streams frames and still earns trust, so refusing it outright is wrong.

`config.get(key, default)` does not cover an explicit null, so a node
registering with rx_lat/rx_lon (or tx_lat/tx_lon) present but null sent
None straight into the geodesy and raised. Geometry also needs both ends,
not just the receiver, since the beam points broadside to the RX->TX
baseline and every footprint has foci at both: a real receiver paired with
a null illuminator was otherwise handed a (0, 0) transmitter and paired
against a fictitious ~11,000 km baseline, while the dashboard reported no
illuminator position for the same node.

has_full_geometry is the single predicate for that split, and it is total
by construction. This package has consumers beyond the server, so the
predicate must hold for whatever reaches it rather than trust an upstream
validator. It is built on _is_real_coordinate, which is identity-based
rather than coercing: bool is excluded even though it subclasses int, a
string is never parsed (so a numeric string reads as unpositioned,
agreeing with the server), an int is admitted only once converted, since
one too large to represent as a float is not a usable coordinate however
finite it is, and a float is admitted only when finite, which stops NaN
publishing a detection area with NaN foci on an unauthenticated endpoint.
The (0, 0) sentinel is rejected at both ends, not just the receiver.
_coord reuses the same helper and falls back to 0.0 for anything that
fails it. Neither predicate can abort a registration partway through and
leave a node in some of the manager's stores but absent from the
associator, which is the point: a predicate that called an unusable value
real would simply move the raise into its caller.

_register_node_locked writes every identity-shaped store (trust, metrics,
reputation, coverage map) unconditionally, then gates the geometry-shaped
half (detection area, empirical coverage, the FOV prior) behind
has_full_geometry, so a node we cannot place still registers and still
counts frames, just without a footprint. A node that loses geometry on
reconnect has its stale detection area dropped rather than left to serve a
footprint it no longer has. Its empirical coverage is kept, so accumulated
calibration survives a spell without coordinates, but get_node_summary
omits empirical_coverage whenever there is no detection area: with no beam
or range to constrain it, to_polygon() returns a large unconstrained shape
still anchored on the node's former receiver.

DetectionAreaState is rebuilt only when something it is constructed from
has changed, so a byte-identical resend (the server re-registers on every
TCP reconnect) no longer resets n_detections and the delay/Doppler bounds
back to defaults. The comparison covers every field its constructor takes
rather than only those get_node_summary surfaces, fc_hz included:
InterNodeAssociator can omit fc_hz from its own unchanged-geometry check
because it records the fresh config in node_configs regardless of its
early return, and manager.py has no equivalent always-fresh store for
detection_areas.

The 60-second get_all_summaries and get_cross_node_analysis caches are
invalidated on exactly the registrations that change what a summary would
contain. On the positionless path that means tracking whether any of
trust, metrics, reputation or coverage was actually created by this call,
rather than inferring it from one store's membership: record_adsb_correlation
creates a trust_scores entry for a node that never registered, so a node can
be present there and still gain everything else on its first registration.
A byte-identical resend is excluded.

max_range_km is read keyed on None rather than through a .get default,
which an explicit null defeats. The unchanged-geometry comparison subtracts
it, so a node declaring a null registered once and then raised TypeError on
every reconnect.

register_node stores a copy of the config rather than the caller's dict,
which callers reuse across registrations, so a later in-place edit cannot
rewrite the geometry every sharing node reads back.

register_node's pairing loop skips the node's own id. node_configs is
written before the loop runs, so the loop would otherwise read the node's
fresh config against its own stale node_geometries entry, pair a relocated
node against its own former position, compute overlap_zones[(node_id,
node_id)] and leave the node in its own neighbour set, pairing its
tracklets against themselves on every later round. The loop also mirrors
rebuild_zones_for's discard when a recomputed zone comes back empty, so a
node relocated away from a former neighbour no longer holds that
neighbour's slot in the capped neighbour visit rotation.

association.py's _has_receiver_position is replaced by the same
has_full_geometry check: the old name promised a receiver-only check it no
longer performed once callers relied on it to guard the TX side too.

test_learned_fov.py::test_unaimed_with_no_tx_is_omni registered a
receiver-only node to reach the omni-prior fallback, but such a node no
longer registers any geometry to read back. It now calls _resolve_fov_prior
directly, which is what it was actually testing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Babissimo
Babissimo merged commit 13bad15 into main Sep 9, 2026
1 check passed
Babissimo added a commit to offworldlabs/retina-server that referenced this pull request Sep 9, 2026
…ff the map

The library's has_full_geometry/detection_area gating and validate_config's
position_status now have something to join: pin retina-analytics to the
commit offworldlabs/retina-analytics#24 landed on main and surface position_status on every node in /api/radar/nodes, which
the dashboard reads. /api/auth/me/nodes carries it too, computed the same
way: the published feed filters out private nodes, so a private positionless
node's owner otherwise had no way to learn it needed a position.
position_status is imported at module level in analytics_refresh, since
node_config imports only math and typing and so cannot cycle with it.

The new test file pins the two behaviours that must hold end to end: a
null-geometry node is counted and visible, but builds no solver pipeline and
joins no overlap zone. The pipeline guard's correctness is currently a
truthiness accident (86cbavanm), and these assertions catch it if the repair
regresses. Only a repair that checks key presence rather than value nullity
would start building pipelines for positionless nodes: an is-not-None repair
would not, since None is not None is False exactly as bool(None) is. The
overlap test registers ten nodes, following test_unpositioned_registration's
shape, so its zero-zones assertion can actually fail rather than hold
vacuously for a single node.

The _clean fixture enumerates its three node IDs explicitly rather than
scanning connected_nodes for a name prefix. That scan is safe in
test_unpositioned_registration, where every test registers through the real
detections route, but test_positionless_node_is_counted_but_not_placed
registers directly against node_analytics/node_associator and never touches
connected_nodes, so the scan would not find it and the node would leak into
the analytics and associator registries. Nothing would break today, because
conftest's autouse reset wipes both before every test, but the fixture should
not depend on that safety net.

_CONFIG is a shared module-level dict and association.py keeps a bare
reference to whatever it is passed rather than copying it, so the four call
sites that hand it to something stateful pass dict(_CONFIG). "Counted" is
asserted as record_detection_frame incrementing total_frames, not merely as
the node having a metrics entry: what the owner needs to see is that a
working receiver's frames are tallied even though it cannot be placed.

Two comments elsewhere described the library as it was before this pin: a
guard function that no longer exists, and a default-to-zero mechanism that
no longer runs. Both now describe has_full_geometry's both-ends rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Babissimo added a commit to offworldlabs/retina-server that referenced this pull request Sep 9, 2026
* Accept a node that cannot say where it is

rx_lat, rx_lon, rx_alt_ft, tx_lat, tx_lon and tx_alt_ft were required and
non-null, so an owner who cannot yet survey a node's geometry had no way
to register it at all. They join beam_width_deg and beam_azimuth_deg as
required-but-nullable: still keys every payload must send, still bounds-
checked when present, but null is no longer an error, since a substituted
coordinate would be wrong data the server could not later tell apart from
a real survey.

Latitude and longitude move as a pair per side, because a lone coordinate
places nothing and a half-supplied side is a bug upstream rather than a
state worth keeping. Altitude stays independently nullable: it is a small
term the geodesy already defaults to zero. The degenerate-baseline check
is guarded to fire only once both sides are present, so a node still
missing its far end no longer trips a check meant for two real points.

The bounds loop runs before the pair rule, so an out-of-range rx_lat whose
rx_lon is null is reported as out-of-range rather than as an unpaired
coordinate. That ordering is the useful one (the specific fault beats the
structural one) and is pinned by a test, since nothing else fixes it.

position_status folds has-rx/has-tx into the one value a caller needs
("positioned", "missing_rx", "missing_tx" or "missing_both"), rather than
four nullable fields every consumer would otherwise recombine itself. It
is keyed on latitude and longitude together for each side, so a config
carrying a latitude and no longitude reads as missing on that side rather
than as positioned. It also has to hold for a raw dict that never passed
through validate_config: connected_nodes configs are read directly, and a
legacy or bulk-ingested one may carry no geometry at all, or only one
coordinate of a pair.

_is_placed is total: a non-numeric coordinate (a string, a list, a bool)
reads as not placed rather than raising. It runs against unvalidated
connected_nodes configs on every analytics-refresh cycle, where a config
like {"tx_lat": "", "tx_lon": ""}, accepted verbatim by the bulk-detections
route, would otherwise take the whole cycle down: nodes and overlaps
payloads, accuracy stats, missed-detection tracking, per-node and MLAT
verification and stale pipeline eviction all stopping silently, every 30s,
for as long as that config stayed connected. position_status also treats a
coordinate pair at exactly (0, 0) as absent on either side, matching the
legacy broken-config sentinel rule retina-analytics applies, since
rx=(0, 0) with a real tx otherwise passed validation and read "positioned"
on the dashboard while the map, associator and solver all excluded the
node: unplaced and unflagged at once, the exact failure this feature exists
to prevent. Its return type is a Literal, so a fifth state cannot drift in
silently.

tcp_handler._validate_node_config is the second validation door, and it
takes the same view: an explicit rx_lat/rx_lon null is a positionless
registration rather than a "missing lat/lon" NACK of the exact state this
change exists to allow. A genuinely absent key still NACKs, unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Store a position we do not have as null, not as (0, 0)

validate_config now accepts a null rx/tx lat, lon and altitude, but
node_configs still declared all six NOT NULL, so a config that actually
carried one would fail on insert rather than register. This closes that
gap: the six columns become nullable, migration 0005 widens the constraint
without touching a row that already exists, and the wire contract moves to
1.1.3.

The table is append-only, one row per configuration version, because an
archived detection refers to the version it was computed under. That is
why the migration is a widening only: it does not backfill or normalise
anything, and a row that declared (0, 0) stays exactly as declared.
Downgrading past 0005 fails loudly if a positionless row exists rather
than inventing coordinates for it, the same trade-off 0004 made for
beam_width_deg.

0005 is graded rollback_safety = "destructive". The classifier's definition
of safe is that the restored code does not read what a revision added, and
nothing is added here, no column and no table. What changes is the value
space of six columns that every line of pre-1.1.3 code has always read
unconditionally as required floats. Once any node registers without
coordinates, which 1.1.3 exists to allow and which continuous auto-accept
makes routine rather than rare, that row is permanent: the database is then
ahead in a way old code cannot safely serve, and the downgrade path fails
on the same row rather than offering an escape. Both halves of that gap
need a human, which is what "destructive" is for.

1.1.3 is a patch rather than a minor bump for the same reason 1.1.2 was:
NodeConfig's fields are not published, so nothing about this is visible to
a client, and RegisterRequest.config and the PUT /config body both stay
free-form (additionalProperties: true) in the contract. Regenerating it
changes only the version string.

test_a_null_position_round_trips calls refresh() before reading the row
back. The session is built with expire_on_commit=False, so get() or
select() after commit returns the same identity-mapped Python object the
test just constructed, never read from the database at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Carry position_status to the dashboard, and prove a null node stays off the map

The library's has_full_geometry/detection_area gating and validate_config's
position_status now have something to join: pin retina-analytics to the
commit offworldlabs/retina-analytics#24 landed on main and surface position_status on every node in /api/radar/nodes, which
the dashboard reads. /api/auth/me/nodes carries it too, computed the same
way: the published feed filters out private nodes, so a private positionless
node's owner otherwise had no way to learn it needed a position.
position_status is imported at module level in analytics_refresh, since
node_config imports only math and typing and so cannot cycle with it.

The new test file pins the two behaviours that must hold end to end: a
null-geometry node is counted and visible, but builds no solver pipeline and
joins no overlap zone. The pipeline guard's correctness is currently a
truthiness accident (86cbavanm), and these assertions catch it if the repair
regresses. Only a repair that checks key presence rather than value nullity
would start building pipelines for positionless nodes: an is-not-None repair
would not, since None is not None is False exactly as bool(None) is. The
overlap test registers ten nodes, following test_unpositioned_registration's
shape, so its zero-zones assertion can actually fail rather than hold
vacuously for a single node.

The _clean fixture enumerates its three node IDs explicitly rather than
scanning connected_nodes for a name prefix. That scan is safe in
test_unpositioned_registration, where every test registers through the real
detections route, but test_positionless_node_is_counted_but_not_placed
registers directly against node_analytics/node_associator and never touches
connected_nodes, so the scan would not find it and the node would leak into
the analytics and associator registries. Nothing would break today, because
conftest's autouse reset wipes both before every test, but the fixture should
not depend on that safety net.

_CONFIG is a shared module-level dict and association.py keeps a bare
reference to whatever it is passed rather than copying it, so the four call
sites that hand it to something stateful pass dict(_CONFIG). "Counted" is
asserted as record_detection_frame incrementing total_frames, not merely as
the node having a metrics entry: what the owner needs to see is that a
working receiver's frames are tallied even though it cannot be placed.

Two comments elsewhere described the library as it was before this pin: a
guard function that no longer exists, and a default-to-zero mechanism that
no longer runs. Both now describe has_full_geometry's both-ends rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Flag a node we cannot place, without calling it broken

position_status now reaches every dashboard surface that lists or details a
node: a warning-style badge beside the liveness badge on NodeManagementPage
(reusing the existing .badge.warning idiom, not a hand-rolled style), a
banner above NodeDetailPage's RF configuration table, and a new "Needs
Attention" section on OverviewPage that renders only when a node actually
needs it. The badge renders nothing at all for a status outside its label
map, rather than an empty chip.

OverviewPage's list merges the owner's own nodes, from /api/auth/me/nodes,
into the published payload. /api/radar/nodes drops a private node, so
without that merge a private node with no position appears on no surface
its owner reads, which is the one case the flag exists for.

The copy is deliberately reassuring rather than alarming, and states the
position fact (the node's detections are counted and archived) rather than
current activity: /api/radar/nodes includes disconnected nodes, so it cannot
promise that anything is being recorded right now. All three surfaces that
tell an owner their node needs a position carried the same sentence
separately, two of them already drifting apart; the badge exports the string
and the other two import it, so they agree by construction.

ConfigPage is deliberately not one of those surfaces. It renders
/api/admin/config/nodes, which has two response shapes (nodes_config.json
verbatim where that file exists, a payload built from connected_nodes
otherwise), so position_status cannot be added to it server-side
consistently: it would exist in the second shape only. Deriving it
client-side would put a second definition of "positioned" beside
services/node_config.py:position_status, which is exactly what this feature
exists to remove, and would buy little, since that table already shows a
missing coordinate in its own column.

NetworkHealthPage's node-location map filtered nodes on `rx_lat && rx_lon`,
which drops a node genuinely at latitude or longitude 0 the same way it
drops one with no position at all. It now checks for null explicitly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Build a solver pipeline only for a node we can place

get_or_create_node_pipeline fell back to the shared default pipeline for a
node without a usable position (86cbavanm). Such a node's detections were
geolocated against DEFAULT_NODE_CONFIG's fixed receiver and published on the
public map stamped with whichever node last shared that pipeline, which
included legacy nodes carrying no coordinates at all, registered through
POST /api/radar/detections. It now returns None, and process_one_frame skips
geolocation, tracking, association and solver-queueing for the frame while
still counting it through record_detection_frame: an unplaced node stays
visible and stays counted, and contributes nothing positional.

The altitude default applies to an explicit null as well as an absent key,
and without `or`, since 0.0 is sea level and a real altitude. A positioned
node declaring a null altitude previously built no pipeline whatsoever and
produced zero tracks for good, while reading as healthy throughout.

The ADS-B seeding block gates on the node actually being positioned rather
than merely present in the associator's geometry registry, which stores a
NodeGeometry with coordinates coerced to 0.0 even for an unplaced node.
KNOWN_LANE_MODE defaults to binding, so this lane was live: a positionless
node's detections were matched against a fabricated Null Island baseline and
the residual charged to the node's trust bias.

Tests that reached the code under test through the old fall-through now
register their node with a real position, through tests/node_helpers, or with
a pre-seeded pipeline. That includes test_dark_follow's
TestModesInProcessOneFrame, which registered with the associator alone: such a
node is half-registered as far as the frame path is concerned, so it took the
fall-through to reach the lane it names. It now captures the node's own
pipeline, rather than pre-seeding node_pipelines with the shared default,
which would hard-code the arrangement this commit abolishes. Two comments
describing the fallback, in node_stream and in the test mirroring it, are
corrected to match.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Stop a zero coordinate vanishing and a null one raising

Consumers that read a position straight from a config predate both nullable
coordinates and, in several cases, any care about latitude 0. Each is now
keyed on the node being positioned, through position_status, rather than on
a second null-handling scheme of its own.

claim_known_targets gates on the node being positioned rather than on its
presence in the geometry registry, for the same reason the frame processor's
seeding block does: the registry holds coordinates coerced to 0.0 for a node
that has none.

Miss-rate statistics, solver-accuracy verification and the map and RF
environment location displays stop treating a coordinate of exactly 0.0 as
falsy, which silently dropped a fully positioned equatorial or
prime-meridian node from each of them.

_fetch_external_adsb needs no change of its own: region building already
drops an absent or unusable position, in its node loop and again in
regions_for_nodes, so the TypeError that would otherwise take the fleet's
ADS-B ground truth cache offline cannot arise. Adding position_status there
as well would be the second scheme this commit exists to remove.
test_periodic_adsb_bbox pins that outcome end to end instead, because a
positionless fleet is what this feature makes routine, and the failure is
silent and unrecoverable: the caller's except never retries.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Correct a beam-width comment and ONBOARDING's editable installs

The comment on the beam-width default described a path that is never reached
the way it claims: resolve_beam_width_deg already handles the null itself, and
did so before this branch existed.

ONBOARDING.md's backend setup installs all five libs editable, matching the
worktree setup section below it and both justfile and ci.yml. An implementer
working on this branch tripped on the two-lib version.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Give a node's config one canonical shape, established at every entry

"Absent", "null", "exactly (0,0)", "a numeric string", "NaN" and the legacy
flat lat/lon spelling were each conflated differently across about eight
places that had to agree and did not. Patching the leaf sites cannot settle
that, because there is no single point where the shape is decided: every
consumer re-derives it from unvalidated JSON, and a null altitude was read as
900 ft by the pipeline, as 0 by the consensus solver, and crashed the
multinode solver outright.

canonical_config in services/node_config.py is now that point. It folds the
legacy spelling, coerces each coordinate to a float or None, nulls a
half-given pair, collapses the (0,0) sentinel the NOT NULL columns once
forced, and resolves both altitudes to floats. It is applied at all five
writers of state.connected_nodes and again in register_node_blocking, the
single door into both library registries. After it, no consumer can see
anything but a float or None in a coordinate slot, and never None in an
altitude slot, which makes those four disagreements unrepresentable rather
than individually guarded.

The predicates that were guarding for themselves collapse onto it:
position_status becomes four is-None checks, and node_beam_params,
track_gates, analytics_refresh and admin drop their falsy-coordinate
spellings. That last one mattered: a transmitter on the equator or the prime
meridian scored as no transmitter at all, so the node came out
omnidirectional and its miss rate blew up.

node_beam_params no longer coerces a missing coordinate to 0.0, so its
callers must gate on placement first.  All four in the backend already do;
association_bench's copy of the solver's range/bearing rule now does too,
which is what its docstring promises of it.

The config_hash is still computed from the pre-canonical config at both the
v1 and blah2 doors, and the TCP handler compares canonical against canonical,
so no live node sees a spurious config-changed signal on deploy. The durable
node_configs row is untouched: canonicalisation is what happens on the way
into memory, not into storage.

test_storage.py pins the archive against the regression this makes
impossible, which would be silent and permanent: a node sending the legacy
flat spelling, which tcp_handler still accepts, archiving null rx_lat and
rx_lon despite being fully placed, because the snapshot was taken from a
config that had not been folded. Parquet rows cannot be corrected once
published, and a null there cannot afterwards be told apart from a node that
genuinely declared no position.

Four tests had been left vacuous, their node absent from connected_nodes so
the pipeline was None and the path each named was never entered. They now
register a node properly, through a shared helper that does what an entry
point does. TestModesInProcessOneFrame had been repaired by pre-seeding
node_pipelines with the shared default, which hard-codes the exact
arrangement this change abolishes; it now captures the node's own pipeline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Default a null altitude at the geometry boundary, not on the way in

canonical_config was resolving a null altitude to its terrain figure, which
is right for the geodesy and wrong for everyone else: /api/radar/nodes and
the Parquet archive both read a config from connected_nodes, and both were
reporting 900 ft for a node that had declared nothing. Nothing downstream
can tell an invented figure apart from a survey, and archive rows are
permanent and not correctable once published.

Carrying a second dict beside the canonical one, and routing those two
consumers to it, would put the archive and the published location block on
an un-normalised payload: a node using the legacy flat lat/lon spelling,
which tcp_handler still accepts, would archive null geometry despite being
fully positioned, and a coordinate arriving as a numeric string would reach
public_latlon, which returns a non-number untouched and so would publish the
operator's true receiver position unfuzzed. Defaulting at the boundary
removes the need for a second form at all.

canonical_config therefore corrects and never invents: a coordinate becomes
the number it already was or the null it already meant, and an altitude
keeps its declared null. One shape is safe to solve on, to publish and to
archive, which is what the design spec asked for in §3.

node_config.resolve_altitudes is the boundary, applied at the three doors a
null cannot pass. get_or_create_node_pipeline subscripts the altitude and
hands it to passive_radar; get_node_configs, the solver's snapshot, has a
consumer that multiplies it by a metre conversion; register_node_blocking is
the single entry to both library registries, and the associator reads
`(config.get("rx_alt_ft") or 0) * 0.3048`. That third door matters most
quietly: spelled `or 0` rather than as a subscript, it survives a null
instead of raising, so an unsurveyed receiver would sit at sea level in the
registries while the pipeline and the solver placed the same node at 900 ft.
One missing altitude, two different answers, and neither figure is ever
printed. Neither registry publishes an altitude, so resolving at that door
cannot leak a working figure into a payload the way resolving on the way in
did.

resolve_altitudes lives in node_config beside the constant it reads and
opposite canonical_config, its counterpart; node_config is a leaf, so a
caller reaches it without pulling in the frame pipeline. It copies rather
than defaulting in place, or the working figure would be written back into
the dict the archive reads, and it keys on None rather than falsiness,
because a receiver at 0 ft is at sea level and not unsurveyed.

blah2_bridge stops substituting 0.0 for an altitude a node omits. That was
an invention on the way in, and it reached publication and the Parquet
archive as though surveyed; the key is left null now for resolve_altitudes to
fill at a door.

The config-hash test moves to the (0, 0) sentinel to keep its own guard
honest: it asserts the declared and canonical forms differ, and a null
altitude no longer makes them.

test_the_stored_config_is_canonical keeps its folding and coercion
assertions and gives up only the altitude one, which now belongs to the
geometry boundary rather than to canonical_config.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Gate an unplaced node out of the solver's snapshot

get_node_configs returned every connected node, so nothing drawn from it
could assume placement. The FOV block checked for itself; _stamp_contamination
did not, and judged an unplaced node's coerced (0, 0) geometry against the
solve; known_lane's dark-follow claim filter kept a claim on `nid in
node_cfgs` under a comment saying the node was dropped because "the LM needs
its geometry", which stopped being true the moment a coordinate could be null.
That is three consumers of one snapshot, each owing the same check, and a
fourth would have owed it too.

The snapshot is the place that knows, so it decides: get_node_configs now
returns the placed nodes only, membership means the node can be solved with,
and known_lane's filter means what it always claimed. missing_tx counts as
placed, because the range circle and the bearing wedge are both about the
receiver and the bistatic paths test the transmitter separately.

The FOV block keeps its own check. Its configs arrive pickled through the
solver queue, so it cannot see which producer built them, and a boundary that
takes work from a queue is worth guarding whatever today's producers do.

A config is copied per node, for the altitude resolution, so get_node_configs
takes the ids a caller can actually reach: process_one_frame passes the union
of its solver inputs' node ids, which is 2 to 8 against a fleet of about 58,
where before it copied the fleet and configs_for_solver_input discarded most
of them a line later. _solver_input_node_ids is that extraction, shared with
configs_for_solver_input rather than spelled twice.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant