From 5dd1c8878f3864ba32bfbc0e1b74a74900781177 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Mon, 7 Sep 2026 10:41:01 +0100 Subject: [PATCH] Register a node we cannot place, without placing it 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 --- src/retina_analytics/association.py | 67 ++++--- src/retina_analytics/constants.py | 51 +++++ src/retina_analytics/manager.py | 132 ++++++++++--- tests/test_association.py | 156 +++++++++++++++- tests/test_learned_fov.py | 26 ++- tests/test_manager_registration.py | 280 ++++++++++++++++++++++++++++ tests/test_unpositioned_nodes.py | 14 ++ 7 files changed, 661 insertions(+), 65 deletions(-) diff --git a/src/retina_analytics/association.py b/src/retina_analytics/association.py index 6e49f5f..c67d459 100644 --- a/src/retina_analytics/association.py +++ b/src/retina_analytics/association.py @@ -44,7 +44,9 @@ C_KM_US, KM_PER_DEG_LAT, R_EARTH, + _is_real_coordinate, bistatic_max_radius_km, + has_full_geometry, km_per_deg_lon, offset_latlon_m, resolve_beam_azimuth_deg, @@ -1012,28 +1014,21 @@ def _merge_epochs(hist_a: list, node_a_id: str, hist_b: list, node_b_id: str) -> def _coord(config: dict, key: str) -> float: - """A latitude/longitude from a config, absent or explicitly null reading 0.0. - - `config.get(key, 0)` is not enough: a v1 registration may carry the key with - a null value, and None then reaches the geodesy as a float. + """A latitude/longitude from a config, defaulting to 0.0 for anything that + is not a real finite number. + + `config.get(key, 0)` is not enough: a v1 registration may carry the key + with a null value, and None then reaches the geodesy as a float. Total, + like has_full_geometry (reusing its _is_real_coordinate): this builds a + geometry object for a node has_full_geometry has already ruled + unpositioned, so it must not raise on a non-numeric value either. The + conversion below cannot overflow, because _is_real_coordinate admits an + int only once it has converted one itself. """ - return float(config.get(key) or 0.0) - - -def _has_receiver_position(config: dict) -> bool: - """Whether this config says where the receiver actually is. - - Absent coordinates default to (0, 0), a point in the Gulf of Guinea that no - node occupies. Every node registered without a position therefore lands on - one footprint and overlaps every other completely — a pairing that is both - fictitious and, being total, the densest and most expensive grid the pair - can produce. A fleet registered that way makes the neighbour graph - complete, which the multinode solver sees as one enormous candidate. - - Only the exact (0, 0) pair reads as absent. The equator and the prime - meridian are each perfectly good coordinates on their own. - """ - return not (_coord(config, "rx_lat") == 0.0 and _coord(config, "rx_lon") == 0.0) + value = config.get(key) + if not _is_real_coordinate(value): + return 0.0 + return float(value) def _worlds_compatible(world_a, world_b) -> bool: @@ -1418,12 +1413,12 @@ def register_node(self, node_id: str, config: dict): Reconnecting nodes skip the expensive O(n²) overlap recomputation as long as their geometry (RX/TX position) hasn't changed. - A node whose config carries no receiver position is registered but takes - no part in overlap — see _has_receiver_position. A pair whose two nodes - are in known and different worlds gets no zone either — see - node_world_provider and _worlds_compatible. + A node whose config lacks either end of the bistatic pair is + registered but takes no part in overlap: see has_full_geometry. A + pair whose two nodes are in known and different worlds gets no zone + either: see node_world_provider and _worlds_compatible. """ - positioned = _has_receiver_position(config) + positioned = has_full_geometry(config) rx_alt_km = (config.get("rx_alt_ft") or 0) * 0.3048 / 1000.0 tx_alt_km = (config.get("tx_alt_ft") or 0) * 0.3048 / 1000.0 @@ -1452,7 +1447,10 @@ def register_node(self, node_id: str, config: dict): # Recorded before the unchanged-geometry early return: the equality # check below covers geometry only, so a reconnect that changed fc_hz # would otherwise leave the fit using the old carrier. - self.node_configs[node_id] = config + # Copied: the caller keeps its dict and may reuse or mutate it, + # and every read below (and _is_positioned's, rounds later) must + # see the config as it was at registration. + self.node_configs[node_id] = dict(config) existing = self.node_geometries.get(node_id) if existing is not None and ( abs(existing.rx_lat - geo.rx_lat) < 1e-6 @@ -1483,7 +1481,10 @@ def register_node(self, node_id: str, config: dict): # nodes register concurrently from a thread-pool executor). my_world = self._node_world(node_id) for existing_id, existing_geo in list(self.node_geometries.items()): - if not self._is_positioned(existing_id): + # node_configs[node_id] already holds the new config (rewritten + # above), so _is_positioned(node_id) cannot be used to skip this + # node's own stale entry here. + if existing_id == node_id or not self._is_positioned(existing_id): continue pair_key = tuple(sorted([node_id, existing_id])) if not _worlds_compatible(my_world, self._node_world(existing_id)): @@ -1507,6 +1508,12 @@ def register_node(self, node_id: str, config: dict): if zone.delay_pairs: # only real overlaps, not geographic misses self._neighbors.setdefault(node_id, set()).add(existing_id) self._neighbors.setdefault(existing_id, set()).add(node_id) + else: + # A relocated node may no longer overlap a former + # neighbour; leaving the adjacency in place would occupy a + # slot in the capped neighbour rotation forever. + self._neighbors.get(node_id, set()).discard(existing_id) + self._neighbors.get(existing_id, set()).discard(node_id) self.node_geometries[node_id] = geo @@ -1547,8 +1554,8 @@ def _reset_for_tests(self) -> None: setattr(self, name, 0) def _is_positioned(self, node_id: str) -> bool: - """Whether a registered node has a receiver position to pair against.""" - return _has_receiver_position(self.node_configs.get(node_id, {})) + """Whether a registered node has both ends of its geometry to pair against.""" + return has_full_geometry(self.node_configs.get(node_id, {})) def _node_world(self, node_id: str): """This node's world, or None when nothing can say. diff --git a/src/retina_analytics/constants.py b/src/retina_analytics/constants.py index d23ea5b..97cfc3c 100644 --- a/src/retina_analytics/constants.py +++ b/src/retina_analytics/constants.py @@ -189,6 +189,57 @@ def resolve_beam_azimuth_deg(config, rx_lat, rx_lon, tx_lat, tx_lon): return (bearing_deg(rx_lat, rx_lon, tx_lat, tx_lon) + 90.0) % 360.0 +_GEOMETRY_KEYS = ("rx_lat", "rx_lon", "tx_lat", "tx_lon") + + +def _is_real_coordinate(value) -> bool: + """Whether *value* is usable as one coordinate: a real, finite number. + + Identity-based, not coerced: a numeric string is not a coordinate, and + neither is a ``bool`` (a subclass of ``int`` otherwise indistinguishable + from one). An ``int`` must survive conversion to a float, because every + consumer stores one: calling an unrepresentable int real would hand the + caller a coordinate it then raises ``OverflowError`` trying to use, + aborting a registration partway through. A float is admitted only when + ``math.isfinite``, which never raises on an existing float. + """ + if isinstance(value, bool) or not isinstance(value, (int, float)): + return False + if isinstance(value, int): + try: + value = float(value) + except OverflowError: + return False + return math.isfinite(value) + + +def has_full_geometry(config: dict) -> bool: + """Whether this config places both ends of the bistatic pair. + + Both are needed: the beam points broadside to the RX->TX baseline and every + footprint has foci at RX and TX, so one missing side leaves a node + unplaceable rather than approximately placed. + + Total: never raises, for any input, including a non-dict config or a + coordinate slot holding something other than a number. Absent and null + read alike, and so does the legacy (0, 0) sentinel that absent + coordinates used to default to. No node occupies the Gulf of Guinea, and + a fleet defaulted there overlaps completely, which makes the neighbour + graph complete and the solver's candidates pure artefact. Only the exact + (0, 0) pair reads as unset, on either end: the equator and the prime + meridian are each perfectly good coordinates on their own, for either + end. See _is_real_coordinate for what counts as a coordinate at all. + """ + if not isinstance(config, dict): + return False + values = {key: config.get(key) for key in _GEOMETRY_KEYS} + if not all(_is_real_coordinate(v) for v in values.values()): + return False + if values["rx_lat"] == 0.0 and values["rx_lon"] == 0.0: + return False + return not (values["tx_lat"] == 0.0 and values["tx_lon"] == 0.0) + + def resolve_beam_width_deg(config): """Resolve a node's beam width (deg), defaulting to the nominal Yagi spec. diff --git a/src/retina_analytics/manager.py b/src/retina_analytics/manager.py index 2df1f08..07bc554 100644 --- a/src/retina_analytics/manager.py +++ b/src/retina_analytics/manager.py @@ -9,6 +9,7 @@ YAGI_BEAM_WIDTH_DEG, YAGI_MAX_RANGE_KM, bearing_deg, + has_full_geometry, haversine_km, resolve_beam_azimuth_deg, resolve_beam_width_deg, @@ -113,27 +114,17 @@ def register_node(self, node_id: str, config: dict): self._register_node_locked(node_id, config) def _register_node_locked(self, node_id: str, config: dict): + # Whether this call adds anything a summary reads, which is what the + # cache invalidation below turns on. Tracked per store as each is + # written rather than inferred from one of them: a node id can already + # be present in trust_scores without ever having registered, since + # record_adsb_correlation creates entries there too, and its summary + # still gains metrics, reputation and coverage here. + added_to_summary = False + if node_id not in self.trust_scores: self.trust_scores[node_id] = TrustScoreState(node_id=node_id) - - rx_lat = config.get("rx_lat", 0) - rx_lon = config.get("rx_lon", 0) - tx_lat = config.get("tx_lat", 0) - tx_lon = config.get("tx_lon", 0) - # Honour an explicit aim if the config supplies one; else broadside. - beam_az = resolve_beam_azimuth_deg(config, rx_lat, rx_lon, tx_lat, tx_lon) - self.detection_areas[node_id] = DetectionAreaState( - node_id=node_id, - rx_lat=rx_lat, - rx_lon=rx_lon, - tx_lat=tx_lat, - tx_lon=tx_lon, - fc_hz=config.get("fc_hz", config.get("FC", 195e6)), - beam_azimuth_deg=beam_az, - beam_width_deg=resolve_beam_width_deg(config), - max_range_km=config.get("max_range_km", YAGI_MAX_RANGE_KM), - max_bistatic_range_km=config.get("max_bistatic_range_km"), - ) + added_to_summary = True # Preserve accumulated metrics across reconnects. Every other # per-node store here is conditional, but this one was replaced @@ -145,21 +136,97 @@ def _register_node_locked(self, node_id: str, config: dict): node_id=node_id, connected_at=time.time(), ) + added_to_summary = True else: existing_metrics.connected_at = time.time() if node_id not in self.reputations: self.reputations[node_id] = NodeReputation(node_id=node_id) + added_to_summary = True if node_id not in self.coverage_maps: self.coverage_maps[node_id] = HistoricalCoverageMap(node_id=node_id) + added_to_summary = True + + # Identity, metrics and reputation are above and unconditional: a node + # we cannot place is still a node that is working, and its frames are + # counted through `metrics` membership in record_detection_frame. + # + # Geometry is undefined without both coordinate pairs, so everything + # below is skipped. No detection area is what keeps such a node off the + # map, since get_node_summary omits the key and the map only draws a + # marker for a node that has one. + if not has_full_geometry(config): + # Invalidate exactly when this changes what a summary holds: a + # detection area from a previous positioned registration is + # dropped here, or one of trust/metrics/reputation/coverage_map + # was just populated above. + had_area = self.detection_areas.pop(node_id, None) is not None + if added_to_summary or had_area: + self._invalidate_analysis_caches() + return + + rx_lat = config["rx_lat"] + rx_lon = config["rx_lon"] + tx_lat = config["tx_lat"] + tx_lon = config["tx_lon"] + # Honour an explicit aim if the config supplies one; else broadside. + beam_az = resolve_beam_azimuth_deg(config, rx_lat, rx_lon, tx_lat, tx_lon) + beam_width = resolve_beam_width_deg(config) + # Keyed on None, not on absence: a config carrying an explicit null + # reaches here unchanged, and a .get default does not substitute for + # one, so the comparison below would subtract None from None. + max_range_km = config.get("max_range_km") + if max_range_km is None: + max_range_km = YAGI_MAX_RANGE_KM + max_bistatic_range_km = config.get("max_bistatic_range_km") + fc_hz = config.get("fc_hz", config.get("FC", 195e6)) + + # Every field DetectionAreaState is built from, compared against what + # it already holds. Unchanged means a reconnect would rebuild a + # byte-identical object, so the rebuild (and the reset of its + # accumulated n_detections/delay/doppler bounds/furthest_detections + # back to defaults) is skipped, the same way the metrics store above + # is preserved across a reconnect rather than replaced. Mirrors + # InterNodeAssociator.register_node's unchanged-geometry check, but + # that check omits fc_hz deliberately, because it always records the + # fresh config in node_configs regardless of the early return, so a + # changed fc_hz is never lost there even when NodeGeometry keeps its + # old one. Nothing here plays that role for detection_areas, so fc_hz + # has to be part of the comparison instead. + existing_da = self.detection_areas.get(node_id) + detection_area_unchanged = existing_da is not None and ( + abs(existing_da.rx_lat - rx_lat) < 1e-6 + and abs(existing_da.rx_lon - rx_lon) < 1e-6 + and abs(existing_da.tx_lat - tx_lat) < 1e-6 + and abs(existing_da.tx_lon - tx_lon) < 1e-6 + and abs(existing_da.max_range_km - max_range_km) < 1e-4 + and abs(existing_da.beam_azimuth_deg - beam_az) < 1e-4 + and abs(existing_da.beam_width_deg - beam_width) < 1e-4 + and existing_da.max_bistatic_range_km == max_bistatic_range_km + and existing_da.fc_hz == fc_hz + ) + + if not detection_area_unchanged: + self.detection_areas[node_id] = DetectionAreaState( + node_id=node_id, + rx_lat=rx_lat, + rx_lon=rx_lon, + tx_lat=tx_lat, + tx_lon=tx_lon, + fc_hz=fc_hz, + beam_azimuth_deg=beam_az, + beam_width_deg=beam_width, + max_range_km=max_range_km, + max_bistatic_range_km=max_bistatic_range_km, + ) # Recreate empirical coverage when the node is new OR its RX moved — node # IDs are reused across fleet regenerations at different positions, so a # persisted polygon from the old location would otherwise be served for # the new one (stale, beam-mismatched, collapsed). ec = self.empirical_coverages.get(node_id) - cfg_max_range = config.get("max_range_km", YAGI_MAX_RANGE_KM) + cfg_max_range = max_range_km moved = ec is not None and haversine_km(ec.rx_lat, ec.rx_lon, rx_lat, rx_lon) > _RX_RELOCATE_THRESHOLD_KM # A change in the *range rule* invalidates the accumulated polygon just # as surely as the RX physically moving: switching a node from a @@ -173,7 +240,7 @@ def _register_node_locked(self, node_id: str, config: dict): # *keeps* accumulated calibration (see the else branch) because it only # moves the clamp; switching range rules changes the footprint's shape, # which is a different thing. - cfg_bistatic = config.get("max_bistatic_range_km") + cfg_bistatic = max_bistatic_range_km rule_changed = ec is not None and getattr(ec, "max_bistatic_range_km", None) != cfg_bistatic # A polygon accumulated under an older calibration input is discarded on # the same footing. The bistatic key cannot catch this one: switching @@ -222,6 +289,12 @@ def _register_node_locked(self, node_id: str, config: dict): # not the evidence. ec.prior_azimuth_deg, ec.prior_width_deg = prior_az, prior_width + # detection_areas[node_id] was only just rebuilt above when it was not + # already unchanged, and a rebuild is the only way this path alters + # what a summary holds, so the two conditions coincide. + if not detection_area_unchanged: + self._invalidate_analysis_caches() + def coverage_limit_for(self, node_id: str): """A bearing → observed-limit-km callable for one node, or None. @@ -269,6 +342,11 @@ def coverage_digest(self, node_id: str): return None return ec.fov_digest() if self.fov_mode != "off" else ec.constraint_digest() + def _invalidate_analysis_caches(self) -> None: + """Drop the memoised get_all_summaries/get_cross_node_analysis results.""" + self._summaries_cache = None + self._cross_node_cache = None + def retire_node(self, node_id: str) -> dict: """Forget a node entirely — in-memory state and its files on disk. @@ -322,8 +400,7 @@ def retire_node(self, node_id: str) -> dict: logging.warning("could not remove %s during retirement", path, exc_info=True) # Any summary cached before this call still names the node. - self._summaries_cache = None - self._cross_node_cache = None + self._invalidate_analysis_caches() return {"node_id": node_id, "dropped": dropped, "files_removed": files} @@ -434,13 +511,16 @@ def get_node_summary(self, node_id: str) -> dict: if node_id in self.coverage_maps: result["coverage_map"] = self.coverage_maps[node_id].summary() ec = self.empirical_coverages.get(node_id) - if ec is not None: - da = self.detection_areas.get(node_id) + da = self.detection_areas.get(node_id) + # Also gated on da: empirical_coverages outlives a lost geometry (see + # _register_node_locked), so ec alone would publish an unconstrained + # polygon anchored on the node's stale receiver. + if ec is not None and da is not None: fov_mode_active = self.fov_mode != "off" poly_kwargs = {} if fov_mode_active: poly_kwargs["use_learned_wedge"] = True - elif da is not None: + else: poly_kwargs["beam_azimuth_deg"] = da.beam_azimuth_deg poly_kwargs["beam_width_deg"] = da.beam_width_deg poly_kwargs["max_range_km"] = da.max_range_km diff --git a/tests/test_association.py b/tests/test_association.py index 28c06f5..6a01dee 100644 --- a/tests/test_association.py +++ b/tests/test_association.py @@ -9,11 +9,12 @@ InterNodeAssociator, NodeGeometry, _bistatic_delay_at, + _coord, _lla_to_enu, compute_overlap_zone, predict_observation, ) -from retina_analytics.constants import C_KM_S, C_KM_US, R_EARTH +from retina_analytics.constants import C_KM_S, C_KM_US, R_EARTH, has_full_geometry # ── Overlap zone & bistatic delay ──────────────────────────────────────────── @@ -106,6 +107,57 @@ def test_beam_width_in_geometry(self): assoc = self._make_assoc() assert assoc.node_geometries["assoc-A"].beam_width_deg == 41 + def test_reregistering_a_moved_node_does_not_pair_it_with_itself(self): + """node_configs[node_id] is rewritten before the pairing loop runs, so + the loop's _is_positioned(existing_id) check already reads the new + config for this node's own (still-stale) entry in node_geometries and + does not skip it on that basis alone.""" + assoc = self._make_assoc() + moved = { + "rx_lat": 33.939 + 0.09, # ~10 km north: fails the unchanged-geometry shortcut + "rx_lon": -84.651, + "rx_alt_ft": 950, + "tx_lat": 33.756, + "tx_lon": -84.331, + "tx_alt_ft": 1600, + "fc_hz": 195e6, + "beam_width_deg": 41, + "max_range_km": 50, + } + + assoc.register_node("assoc-A", moved) + + assert ("assoc-A", "assoc-A") not in assoc.overlap_zones + assert "assoc-A" not in assoc._neighbors.get("assoc-A", set()) + + def test_reregistering_a_relocated_node_drops_a_stale_neighbour(self): + """A and B start close enough to overlap and pair as neighbours. Once A + re-registers somewhere distant but still positioned, the freshly + computed zone has no overlap and B must no longer be a listed + neighbour of A, nor A of B: otherwise the pairing survives forever + and permanently occupies a slot in the capped neighbour rotation.""" + assoc = self._make_assoc() + assert "assoc-B" in assoc._neighbors.get("assoc-A", set()) + assert "assoc-A" in assoc._neighbors.get("assoc-B", set()) + assert assoc.overlap_zones[("assoc-A", "assoc-B")].delay_pairs + + relocated = { + "rx_lat": 34.05, + "rx_lon": -118.25, # Los Angeles: still positioned, no longer near B + "rx_alt_ft": 950, + "tx_lat": 33.87, + "tx_lon": -117.93, + "tx_alt_ft": 1600, + "fc_hz": 195e6, + "beam_width_deg": 41, + "max_range_km": 50, + } + assoc.register_node("assoc-A", relocated) + + assert not assoc.overlap_zones[("assoc-A", "assoc-B")].delay_pairs + assert "assoc-B" not in assoc._neighbors.get("assoc-A", set()) + assert "assoc-A" not in assoc._neighbors.get("assoc-B", set()) + _SCALING_CFG = { "rx_lat": 33.939, @@ -609,3 +661,105 @@ def max_limit_km(self): assert geo.effective_radius_km == pytest.approx(50.0) # footprint still wider geo.fov.reach = 85.0 assert geo.effective_radius_km == pytest.approx(85.0) + + +# ── has_full_geometry ───────────────────────────────────────────────────────── + + +def test_has_full_geometry_requires_both_sides(): + full = {"rx_lat": 34.85, "rx_lon": -82.39, "tx_lat": 34.90, "tx_lon": -82.45} + assert has_full_geometry(full) is True + assert has_full_geometry({**full, "tx_lat": None}) is False + assert has_full_geometry({**full, "rx_lon": None}) is False + assert has_full_geometry({}) is False + # The legacy sentinel: absent coordinates used to default to (0, 0). + assert has_full_geometry({**full, "rx_lat": 0.0, "rx_lon": 0.0}) is False + # The equator and the prime meridian are each fine on their own, for + # either end. + assert has_full_geometry({**full, "rx_lat": 0.0}) is True + assert has_full_geometry({**full, "tx_lat": 0.0}) is True + # The sentinel applies to the transmitter too: a receiver paired with a + # transmitter at (0, 0) is not a bistatic geometry. + assert has_full_geometry({**full, "tx_lat": 0.0, "tx_lon": 0.0}) is False + + +def test_has_full_geometry_is_total_never_raises(): + """No input, however malformed, may raise: the predicate is reached from + an unvalidated ingest path, where a raised exception kills a node's + registration partway through, leaving it in some of the manager's stores + and absent from the associator.""" + full = {"rx_lat": 34.85, "rx_lon": -82.39, "tx_lat": 34.90, "tx_lon": -82.45} + assert has_full_geometry(None) is False + assert has_full_geometry([]) is False + assert has_full_geometry("not a config") is False + assert has_full_geometry(42) is False + # A numeric string is not coerced: it is simply not a coordinate. + assert has_full_geometry({**full, "rx_lat": "34.85"}) is False + # An integer too large to convert to a float is not a usable coordinate: + # reported unpositioned rather than raising here, and rather than being + # called real and left to raise OverflowError in the caller that stores it. + assert has_full_geometry({**full, "rx_lat": 10**400}) is False + assert has_full_geometry({**full, "rx_lat": 10**400, "rx_lon": 0.0}) is False + + +def test_has_full_geometry_rejects_nan_and_infinity(): + """NaN and infinity are not finite, so neither counts as a real + coordinate: a NaN-foci detection area must never be built.""" + full = {"rx_lat": 34.85, "rx_lon": -82.39, "tx_lat": 34.90, "tx_lon": -82.45} + assert has_full_geometry({**full, "rx_lat": float("nan")}) is False + assert has_full_geometry({**full, "tx_lon": float("nan")}) is False + assert has_full_geometry({**full, "rx_lat": float("inf")}) is False + assert has_full_geometry({**full, "tx_lat": float("-inf")}) is False + + +def test_has_full_geometry_rejects_bool(): + """bool is an int subclass but is not a coordinate.""" + full = {"rx_lat": 34.85, "rx_lon": -82.39, "tx_lat": 34.90, "tx_lon": -82.45} + assert has_full_geometry({**full, "rx_lat": True}) is False + assert has_full_geometry({**full, "rx_lat": False}) is False + + +# ── _coord ───────────────────────────────────────────────────────────────── + + +def test_coord_is_total_never_raises(): + """_coord builds a geometry object for a node has_full_geometry has + already ruled unpositioned, so a junk coordinate must default to 0.0 + rather than raise.""" + assert _coord({"rx_lat": "not a number"}, "rx_lat") == 0.0 + assert _coord({"rx_lat": None}, "rx_lat") == 0.0 + assert _coord({}, "rx_lat") == 0.0 + assert _coord({"rx_lat": float("nan")}, "rx_lat") == 0.0 + assert _coord({"rx_lat": float("inf")}, "rx_lat") == 0.0 + assert _coord({"rx_lat": True}, "rx_lat") == 0.0 + assert _coord({"rx_lat": 0}, "rx_lat") == 0.0 + assert _coord({"rx_lat": 34.85}, "rx_lat") == 34.85 + + +def test_coord_does_not_overflow_on_a_huge_int(): + """_is_real_coordinate rejects a huge int, so _coord is reached with one + only on the unpositioned path, where it must still yield a float rather + than raise OverflowError converting it.""" + assert _coord({"rx_lat": 10**400}, "rx_lat") == 0.0 + + +def test_register_node_does_not_retain_the_callers_config(): + """node_configs must hold a copy, not the caller's dict. + + Callers reuse a config object across registrations (the tests here and in + test_unpositioned_nodes.py register one module-level dict under several + ids), and a later in-place edit would otherwise rewrite the geometry every + sharing node reads back through _is_positioned and the unchanged-geometry + comparison. + """ + assoc = InterNodeAssociator() + shared = {"rx_lat": 34.85, "rx_lon": -82.39, "tx_lat": 34.90, "tx_lon": -82.45} + + assoc.register_node("a", shared) + assoc.register_node("b", shared) + + assert assoc.node_configs["a"] is not shared + assert assoc.node_configs["a"] is not assoc.node_configs["b"] + + shared["rx_lat"] = 0.0 + assert assoc.node_configs["a"]["rx_lat"] == 34.85 diff --git a/tests/test_learned_fov.py b/tests/test_learned_fov.py index 8958ccc..44d501d 100644 --- a/tests/test_learned_fov.py +++ b/tests/test_learned_fov.py @@ -191,14 +191,24 @@ def test_unaimed_with_known_tx_falls_back_to_broadside(self): expected = (bearing_deg(_RX_LAT, _RX_LON, _TX_LAT, _TX_LON) + 90.0) % 360.0 assert ec.prior_azimuth_deg == pytest.approx(expected, abs=1e-6) - def test_unaimed_with_no_tx_is_omni(self): - """No declared aim and no TX to derive broadside from -> None, the - radar3 case: this is where the invented-broadside kill path stops - being invented — every bearing starts inside the prior.""" - m = NodeAnalyticsManager() - m.register_node("N", dict(rx_lat=_RX_LAT, rx_lon=_RX_LON, max_range_km=50)) - ec = m.empirical_coverages["N"] - assert ec.prior_azimuth_deg is None + def test_resolve_fov_prior_returns_omni_without_a_tx(self): + """No declared aim and no TX to derive broadside from gives None, so + no broadside is invented and every bearing starts inside the prior. + + A unit test of the helper, not of registration, and deliberately so: + has_full_geometry now excludes a TX-less config, so this branch is + unreachable through register_node and no node state exists to inspect. + It is kept as a guard on the helper's own contract, for whoever moves + that gate. What a TX-less node actually does on registration (no + detection area, no empirical coverage) is pinned by + test_registration_without_geometry_does_not_raise. + """ + from retina_analytics.manager import _resolve_fov_prior + + cfg = dict(rx_lat=_RX_LAT, rx_lon=_RX_LON, max_range_km=50) + az, width = _resolve_fov_prior(cfg, _RX_LAT, _RX_LON, 0, 0) + assert az is None + assert width is None def test_prior_updates_in_place_on_reconnect_without_losing_calibration(self): cfg = dict(rx_lat=_RX_LAT, rx_lon=_RX_LON, tx_lat=_TX_LAT, tx_lon=_TX_LON, max_range_km=50) diff --git a/tests/test_manager_registration.py b/tests/test_manager_registration.py index 50e68e9..2d1ad52 100644 --- a/tests/test_manager_registration.py +++ b/tests/test_manager_registration.py @@ -2,8 +2,11 @@ import os +import pytest + from retina_analytics.constants import KM_PER_DEG_LAT, bearing_deg from retina_analytics.manager import NodeAnalyticsManager +from retina_analytics.trust import AdsReportEntry _RX_LAT, _RX_LON = 32.90, -97.00 _TX_LAT, _TX_LON = 32.78, -96.80 @@ -213,3 +216,280 @@ def test_schema_survives_a_save_load_round_trip(tmp_path): with open(path, "w") as f: json.dump(d, f) assert EmpiricalCoverageState.load_from_file(path).schema == 1 + + +# ── Registration without geometry ──────────────────────────────────────────── + +_POSITIONED = { + "rx_lat": 34.85, + "rx_lon": -82.39, + "rx_alt_ft": 900.0, + "tx_lat": 34.90, + "tx_lon": -82.45, + "tx_alt_ft": 1200.0, + "fc_hz": 195e6, + "beam_width_deg": None, + "beam_azimuth_deg": None, +} + + +def _cfg(**overrides): + return {**_POSITIONED, **overrides} + + +@pytest.mark.parametrize( + "overrides", + [ + {"rx_lat": None, "rx_lon": None}, + {"tx_lat": None, "tx_lon": None}, + {"rx_lat": None, "rx_lon": None, "tx_lat": None, "tx_lon": None}, + ], + ids=["no-rx", "no-tx", "neither"], +) +def test_registration_without_geometry_does_not_raise(overrides): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(**overrides)) + assert "n1" in m.metrics + assert "n1" in m.trust_scores + assert "n1" in m.reputations + assert "n1" not in m.detection_areas + assert "n1" not in m.empirical_coverages + + +def test_positionless_summary_omits_detection_area(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + summary = m.get_node_summary("n1") + assert "detection_area" not in summary + assert "metrics" in summary + + +def test_null_altitude_still_positions_the_node(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_alt_ft=None, tx_alt_ft=None)) + assert "n1" in m.detection_areas + + +def test_frames_are_counted_for_a_positionless_node(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + assert m.record_detection_frame("n1", {"timestamp": 1.0, "detections": []}) is True + assert m.metrics["n1"].total_frames == 1 + + +def test_losing_geometry_drops_a_stale_detection_area(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + assert "n1" in m.detection_areas + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + assert "n1" not in m.detection_areas + assert "n1" in m.empirical_coverages # retained, not popped: see the guard's comment + + +def test_losing_geometry_stops_publishing_empirical_coverage(): + """The empirical_coverages entry survives the loss of geometry (see + above), but with no detection area there is no beam or range left to + constrain its polygon, so the summary must not publish one.""" + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + assert "n1" in m.empirical_coverages + summary = m.get_node_summary("n1") + assert "empirical_coverage" not in summary + + +def test_reregistration_that_loses_geometry_invalidates_the_summary_cache(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + assert "detection_area" in m.get_all_summaries()["n1"] + + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + + assert "detection_area" not in m.get_all_summaries()["n1"] + + +def test_reregistration_that_gains_geometry_invalidates_the_summary_cache(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + assert "detection_area" not in m.get_all_summaries()["n1"] + + m.register_node("n1", _cfg()) + + assert "detection_area" in m.get_all_summaries()["n1"] + + +# ── Cache invalidation must track actual content changes ───────────────────── +# +# get_all_summaries/get_cross_node_analysis are memoised for _ANALYSIS_CACHE_TTL +# seconds. The server calls register_node on every TCP reconnect regardless of +# whether the config changed, so invalidating unconditionally defeats the cache; +# never invalidating a node's first-ever appearance leaves it missing from +# summaries for up to a minute. + + +def test_byte_identical_resend_does_not_invalidate_the_summary_cache(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + first = m.get_all_summaries() + + m.register_node("n1", _cfg()) # same object contents, e.g. a TCP reconnect + + assert m.get_all_summaries() is first + + +def test_byte_identical_resend_does_not_invalidate_the_cross_node_cache(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + m.register_node("n2", _cfg(rx_lat=_RX_LAT + 0.05, rx_lon=_RX_LON + 0.05)) + first = m.get_cross_node_analysis() + + m.register_node("n1", _cfg()) + + assert m.get_cross_node_analysis() is first + + +def test_first_positionless_registration_invalidates_the_summary_cache(): + """The node just joined trust_scores/metrics/reputations/coverage_maps, so + it belongs in get_all_summaries immediately, not after the TTL expires.""" + m = NodeAnalyticsManager() + m.register_node("n0", _cfg()) + first = m.get_all_summaries() + assert "n1" not in first + + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) # first-ever, no geometry + + summaries = m.get_all_summaries() + assert summaries is not first + assert "n1" in summaries + + +def test_repeat_positionless_resend_does_not_invalidate_the_summary_cache(): + """Unlike the first registration above, a node already known to the + manager that reconnects still positionless changes nothing a summary + holds.""" + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + first = m.get_all_summaries() + + m.register_node("n1", _cfg(rx_lat=None, rx_lon=None)) + + assert m.get_all_summaries() is first + + +def test_relocation_invalidates_the_summary_cache(): + """A genuine geometry change, distinct from gaining or losing geometry + entirely, must still invalidate the cache.""" + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + first = m.get_all_summaries() + + m.register_node("n1", _cfg(rx_lat=_RX_LAT + 0.01)) # ~1.1 km move + + assert m.get_all_summaries() is not first + + +# ── DetectionAreaState is preserved across an unchanged reconnect ──────────── +# +# A rebuild is skipped when nothing DetectionAreaState is built from has +# changed, the same way the metrics store above is preserved rather than +# replaced: a rebuild resets n_detections and the delay/doppler bounds back to +# defaults, which a reconnect that changed nothing has no reason to do. + + +def test_byte_identical_resend_preserves_detection_area_state(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + da = m.detection_areas["n1"] + da.update(delay=12.3, doppler=45.6) + assert da.n_detections == 1 + + m.register_node("n1", _cfg()) # same object contents, e.g. a TCP reconnect + + assert m.detection_areas["n1"] is da # not rebuilt + assert m.detection_areas["n1"].n_detections == 1 # accumulated state kept + + +def test_relocation_replaces_detection_area_state(): + m = NodeAnalyticsManager() + m.register_node("n1", _cfg()) + da = m.detection_areas["n1"] + da.update(delay=12.3, doppler=45.6) + + m.register_node("n1", _cfg(rx_lat=_RX_LAT + 0.01)) # ~1.1 km move + + assert m.detection_areas["n1"] is not da + assert m.detection_areas["n1"].n_detections == 0 + + +def test_beam_width_only_change_still_rebuilds_detection_area_state(): + """Unchanged means unchanged in every field DetectionAreaState is built + from, not just position: a node retuning only its beam width must still + get a rebuild that reflects the new width.""" + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(beam_width_deg=41.0)) + da = m.detection_areas["n1"] + da.update(delay=12.3, doppler=45.6) + + m.register_node("n1", _cfg(beam_width_deg=30.0)) + + assert m.detection_areas["n1"] is not da + assert m.detection_areas["n1"].beam_width_deg == 30.0 + assert m.detection_areas["n1"].n_detections == 0 + + +def test_a_huge_int_coordinate_registers_unpositioned_rather_than_half_registering(): + """An int too large to convert to a float is not a usable coordinate. + + has_full_geometry must say so rather than call it real and leave + _register_node_locked to raise OverflowError building the detection area, + which would abort after the identity stores were written and leave the + node in the manager but absent from the associator. + """ + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(rx_lat=10**400)) + + assert "n1" in m.trust_scores + assert "n1" in m.metrics + assert "n1" not in m.detection_areas + + +def test_a_null_max_range_survives_a_reconnect(): + """max_range_km is read with a .get default, which an explicit null + defeats, and the unchanged-geometry comparison subtracts it: a node that + declares a null must not register once and then fail on every reconnect. + """ + m = NodeAnalyticsManager() + m.register_node("n1", _cfg(max_range_km=None)) + + m.register_node("n1", _cfg(max_range_km=None)) + + assert "n1" in m.detection_areas + + +def test_a_trust_entry_from_an_adsb_report_does_not_suppress_invalidation(): + """record_adsb_correlation creates a trust_scores entry for a node that + never registered, so trust_scores membership is not the same question as + "would a summary already hold this node's metrics and reputation". The + first real registration must still invalidate. + """ + m = NodeAnalyticsManager() + m.record_adsb_correlation( + "ghost", + AdsReportEntry( + timestamp_ms=1000, + predicted_delay=15.0, + predicted_doppler=50.0, + measured_delay=15.2, + measured_doppler=50.5, + adsb_hex="abc123", + adsb_lat=34.0, + adsb_lon=-84.5, + ), + ) + first = m.get_all_summaries() + assert "ghost" in first + + m.register_node("ghost", _cfg(rx_lat=None, rx_lon=None)) + + assert m.get_all_summaries() is not first + assert m.get_all_summaries()["ghost"].get("metrics") is not None diff --git a/tests/test_unpositioned_nodes.py b/tests/test_unpositioned_nodes.py index c4bb9db..b79223f 100644 --- a/tests/test_unpositioned_nodes.py +++ b/tests/test_unpositioned_nodes.py @@ -94,6 +94,20 @@ def test_the_node_is_still_registered(self, assoc): assert "bare" in assoc.node_geometries assert "bare" in assoc.node_configs + def test_a_non_numeric_coordinate_registers_unpositioned_rather_than_raising(self, assoc): + """has_full_geometry and _coord both read a node's config from an + unvalidated ingest path. A junk coordinate must complete registration + rather than abort it partway through, and the node must come out + unpositioned rather than paired against a positioned peer.""" + assoc.register_node("A", POSITIONED_A) + junk = {**POSITIONED_B, "rx_lat": "not a number"} + + assoc.register_node("junk", junk) + + assert "junk" in assoc.node_geometries + assert "junk" in assoc.node_configs + assert ("A", "junk") not in assoc.overlap_zones + def test_supplying_a_position_later_builds_the_zones(self, assoc): """The upgrade path: a node that registers bare and re-registers with real geometry must become a full participant."""