fix(graph_trace_helper): record a trace's links in path order, with known keys only

Codex review of the bot_prefix fix; these helper defects only mattered once traces reached the graph.

- Our own trace also confirms the bot -> first hop link, which was missing. Edges follow the chain bot -> h1 ... hn -> bot with 1-based hop positions in path order, as the message path writer numbers them (they were reversed).
- A public key is stored only when its prefix belongs to one recent repeater. The location lookup still picks a position for the distance, but its pick among colliding prefixes no longer becomes the stored key (also in the one-hop case).
- bot_prefix reads the radio's current key on each access, so reconnecting or rebooting into another radio is picked up.
- extract_repeater_prefixes_from_path no longer drops a last hop equal to our prefix: a companion never forwards, so it is a repeater sharing our short prefix. On dev bot_prefix was always None, so this keeps the attribution dev already had.

modules/graph_trace_helper.py

@@ -97,7 +97,6 @@ def update_mesh_graph_from_trace_data(
     if len(path_hashes) == 0:
         return
 
-    last_node = path_hashes[-1].lower()
 
     if is_immediate_neighbor:
         neighbor_prefix = path_hashes[0].lower()
@@ -133,9 +132,8 @@ def update_mesh_graph_from_trace_data(
             if bot_location:
                 neighbor_result = _get_node_location_from_db(bot, neighbor_prefix, bot_location, recency_days)
                 if neighbor_result:
-                    neighbor_location, selected_neighbor_key = neighbor_result
-                    if not neighbor_key and selected_neighbor_key:
-                        neighbor_key = selected_neighbor_key
+                    # The location match is only for the distance, not the neighbor's identity
+                    neighbor_location, _ = neighbor_result
                     if neighbor_location and bot_location:
                         geographic_distance = calculate_distance(
                             neighbor_location[0], neighbor_location[1],
@@ -165,9 +163,57 @@ def update_mesh_graph_from_trace_data(
         bot.logger.info(f"Mesh graph: Created trusted bidirectional edge with immediate neighbor {neighbor_prefix}")
         return
 
-    # Regular case: trace from elsewhere, bot is destination
-    geographic_distance = None
-    last_node_key = None
+    # Regular case: the bot received the trace at the end of its path. Our own
+    # trace also left from the bot, so it confirms the bot -> first hop link too.
+    chain = [h.lower() for h in path_hashes]
+    if is_our_trace:
+        chain = [bot_prefix] + chain
+    chain = chain + [bot_prefix]
+
+    keys: dict[str, Optional[str]] = {bot_prefix: bot_key}
+    for node in chain:
+        if node not in keys:
+            keys[node] = _unique_repeater_key(bot, node, recency_days)
+
+    # Locations only feed distances; a location match among colliding prefixes is a
+    # guess, so it never becomes the stored public key.
+    locations: dict[int, Optional[tuple[float, float]]] = {}
+    reference = bot_location
+    for i, node in enumerate(chain):
+        if node == bot_prefix:
+            locations[i] = bot_location
+        else:
+            locations[i] = None
+            try:
+                found = _get_node_location_from_db(bot, node, reference, recency_days)
+                if found and found[0]:
+                    locations[i] = found[0]
+            except Exception as e:
+                bot.logger.debug(f"Could not locate trace node {node}: {e}")
+        if locations[i]:
+            reference = locations[i]
+
+    for i in range(len(chain) - 1):
+        from_node, to_node = chain[i], chain[i + 1]
+        geographic_distance = None
+        from_location, to_location = locations[i], locations[i + 1]
+        if from_location and to_location:
+            geographic_distance = calculate_distance(
+                from_location[0], from_location[1], to_location[0], to_location[1],
+            )
+        mesh_graph.add_edge(
+            from_prefix=from_node,
+            to_prefix=to_node,
+            from_public_key=keys.get(from_node),
+            to_public_key=keys.get(to_node),
+            hop_position=i + 1,
+            geographic_distance=geographic_distance,
+            **edge_width,
+        )
+
+
+def _unique_repeater_key(bot: Any, prefix: str, recency_days: int) -> Optional[str]:
+    """The public key of the only recently heard repeater or room server with ``prefix``, else None."""
     try:
         count_query = f"""
             SELECT COUNT(DISTINCT public_key) as count
@@ -176,7 +222,7 @@ def update_mesh_graph_from_trace_data(
             AND role IN ('repeater', 'roomserver')
             AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days')
         """
-        prefix_pattern = f"{last_node}%"
+        prefix_pattern = f"{prefix}%"
         count_results = bot.db_manager.execute_query(count_query, (prefix_pattern,))
         if count_results and count_results[0].get("count", 0) == 1:
             query = f"""
@@ -190,102 +236,7 @@ def update_mesh_graph_from_trace_data(
             """
             results = bot.db_manager.execute_query(query, (prefix_pattern,))
             if results and results[0].get("public_key"):
-                last_node_key = results[0]["public_key"]
+                return results[0]["public_key"]
     except Exception as e:
-        bot.logger.debug(f"Error checking uniqueness for trace last_node {last_node}: {e}")
-
-    try:
-        if bot_location:
-            last_node_result = _get_node_location_from_db(bot, last_node, bot_location, recency_days)
-            if last_node_result:
-                last_node_location, selected_key = last_node_result
-                if not last_node_key and selected_key:
-                    last_node_key = selected_key
-                if last_node_location and bot_location:
-                    geographic_distance = calculate_distance(
-                        last_node_location[0], last_node_location[1],
-                        bot_location[0], bot_location[1],
-                    )
-    except Exception as e:
-        bot.logger.debug(f"Could not calculate distance for trace edge {last_node}->{bot_prefix}: {e}")
-
-    mesh_graph.add_edge(
-        from_prefix=last_node,
-        to_prefix=bot_prefix,
-        from_public_key=last_node_key,
-        to_public_key=bot_key,
-        hop_position=len(path_hashes),
-        geographic_distance=geographic_distance,
-        **edge_width,
-    )
-
-    # Create edges between nodes in the pathHashes (if more than one)
-    previous_location = bot_location
-    for i in range(len(path_hashes) - 1, 0, -1):
-        from_node = path_hashes[i - 1].lower()
-        to_node = path_hashes[i].lower()
-        hop_position = len(path_hashes) - i
-
-        from_node_key = None
-        to_node_key = None
-        for node, key_var in [(from_node, "from_node_key"), (to_node, "to_node_key")]:
-            try:
-                count_query = f"""
-                    SELECT COUNT(DISTINCT public_key) as count
-                    FROM complete_contact_tracking
-                    WHERE public_key LIKE ?
-                    AND role IN ('repeater', 'roomserver')
-                    AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days')
-                """
-                prefix_pattern = f"{node}%"
-                count_results = bot.db_manager.execute_query(count_query, (prefix_pattern,))
-                if count_results and count_results[0].get("count", 0) == 1:
-                    query = f"""
-                        SELECT public_key
-                        FROM complete_contact_tracking
-                        WHERE public_key LIKE ?
-                        AND role IN ('repeater', 'roomserver')
-                        AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days')
-                        ORDER BY is_starred DESC, COALESCE(last_advert_timestamp, last_heard) DESC
-                        LIMIT 1
-                    """
-                    results = bot.db_manager.execute_query(query, (prefix_pattern,))
-                    if results and results[0].get("public_key"):
-                        if key_var == "from_node_key":
-                            from_node_key = results[0]["public_key"]
-                        else:
-                            to_node_key = results[0]["public_key"]
-            except Exception as e:
-                bot.logger.debug(f"Error checking uniqueness for trace node {node}: {e}")
-
-        geographic_distance = None
-        try:
-            if previous_location:
-                from_result = _get_node_location_from_db(bot, from_node, previous_location, recency_days)
-                if from_result:
-                    from_location, selected_from_key = from_result
-                    if not from_node_key and selected_from_key:
-                        from_node_key = selected_from_key
-                    to_result = _get_node_location_from_db(bot, to_node, from_location, recency_days)
-                    if to_result:
-                        to_location, selected_to_key = to_result
-                        if not to_node_key and selected_to_key:
-                            to_node_key = selected_to_key
-                        if from_location and to_location:
-                            geographic_distance = calculate_distance(
-                                from_location[0], from_location[1],
-                                to_location[0], to_location[1],
-                            )
-                            previous_location = from_location
-        except Exception as e:
-            bot.logger.debug(f"Could not calculate distance for trace edge {from_node}->{to_node}: {e}")
-
-        mesh_graph.add_edge(
-            from_prefix=from_node,
-            to_prefix=to_node,
-            from_public_key=from_node_key,
-            to_public_key=to_node_key,
-            hop_position=hop_position,
-            geographic_distance=geographic_distance,
-            **edge_width,
-        )
+        bot.logger.debug(f"Error checking uniqueness for trace node {prefix}: {e}")
+    return None

modules/transmission_tracker.py

@@ -109,21 +109,26 @@ class TransmissionTracker:
 
     @property
     def bot_prefix(self) -> Optional[str]:
-        """Our radio's public key prefix (``prefix_hex_chars`` long), or None before it is known."""
-        if self._bot_prefix is None:
-            self._update_bot_prefix()
-        return self._bot_prefix
+        """Our radio's public key prefix (``prefix_hex_chars`` long), or None before it is known.
+
+        Read from the radio's current key each time, so a reconnect to another radio
+        is picked up; a value assigned to it is kept instead.
+        """
+        if self._bot_prefix is not None:
+            return self._bot_prefix
+        key = own_public_key(self.bot)
+        if key and len(key) >= 2:
+            return key[:self.bot.prefix_hex_chars].lower()
+        return None
 
     @bot_prefix.setter
     def bot_prefix(self, value: Optional[str]) -> None:
         self._bot_prefix = value
 
     def _update_bot_prefix(self):
-        """Update bot prefix from the radio's public key"""
-        key = own_public_key(self.bot)
-        if key and len(key) >= 2:
-            self._bot_prefix = key[:self.bot.prefix_hex_chars].lower()
-            self.logger.debug(f"Bot prefix set to: {self._bot_prefix}")
+        """Log the bot prefix once the radio's key is known (kept for callers; the property reads it live)."""
+        if self.bot_prefix:
+            self.logger.debug(f"Bot prefix set to: {self.bot_prefix}")
 
     def record_transmission(self, content: str, target: str, message_type: str,
                           command_id: Optional[str] = None, *,
@@ -463,10 +468,9 @@ class TransmissionTracker:
             last_node = path_nodes[-1]
             if isinstance(last_node, str) and len(last_node) >= 2:
                 # Take first 2 characters as prefix
-                prefix = last_node[:self.bot.prefix_hex_chars].lower()
-                # Filter out our own prefix
-                if prefix != self.bot_prefix:
-                    return [prefix]
+                # A companion radio never forwards, so the last hop is never us; a
+                # repeater sharing our short prefix still gets the repeat.
+                return [last_node[:self.bot.prefix_hex_chars].lower()]
 
         # Fallback to parsing path string
         elif path:
@@ -481,10 +485,7 @@ class TransmissionTracker:
             if parts:
                 last_part = parts[-1]
                 if len(last_part) >= 2:
-                    prefix = last_part[:self.bot.prefix_hex_chars].lower()
-                    # Filter out our own prefix
-                    if prefix != self.bot_prefix:
-                        return [prefix]
+                    return [last_part[:self.bot.prefix_hex_chars].lower()]
 
         return []  # No valid prefix found
 

tests/test_graph_trace_helper.py

@@ -336,6 +336,35 @@ class TestBotIdentityFromRadio:
         bot = _make_bot(bot_prefix=None)
         bot.meshcore.self_info = {"public_key": "c3d4" + "00" * 30}
         update_mesh_graph_from_trace_data(bot, ["a1b2", "e5f6"], {})
-        last = bot.mesh_graph.add_edge.call_args_list[0].kwargs
+        last = bot.mesh_graph.add_edge.call_args_list[-1].kwargs
         assert (last["from_prefix"], last["to_prefix"]) == ("e5f6", "c3d4")
         assert all(c.kwargs.get("prefix_bytes") == 2 for c in bot.mesh_graph.add_edge.call_args_list)
+
+
+
+class TestOwnTraceChain:
+    def test_our_trace_adds_the_bot_to_first_hop_edge_in_path_order(self):
+        bot = _make_bot(bot_prefix="dd")
+        update_mesh_graph_from_trace_data(bot, ["aa", "bb", "cc"], {}, is_our_trace=True)
+        edges = [(c.kwargs["from_prefix"], c.kwargs["to_prefix"], c.kwargs["hop_position"])
+                 for c in bot.mesh_graph.add_edge.call_args_list]
+        assert edges == [("dd", "aa", 1), ("aa", "bb", 2), ("bb", "cc", 3), ("cc", "dd", 4)]
+
+    def test_someone_elses_trace_starts_at_its_first_hop(self):
+        bot = _make_bot(bot_prefix="dd")
+        update_mesh_graph_from_trace_data(bot, ["aa", "bb"], {}, is_our_trace=False)
+        edges = [(c.kwargs["from_prefix"], c.kwargs["to_prefix"], c.kwargs["hop_position"])
+                 for c in bot.mesh_graph.add_edge.call_args_list]
+        assert edges == [("aa", "bb", 1), ("bb", "dd", 2)]
+
+    def test_a_location_guess_is_not_stored_as_the_public_key(self, monkeypatch):
+        import modules.utils as utils
+
+        bot = _make_bot(bot_prefix="dd")
+        bot.db_manager.execute_query = Mock(return_value=[{"count": 2}])  # two repeaters share "aa"
+        monkeypatch.setattr(utils, "_get_node_location_from_db", lambda *a, **k: ((47.0, -122.0), "aa11" + "0" * 60))
+        update_mesh_graph_from_trace_data(bot, ["aa"], {}, is_our_trace=False)
+        update_mesh_graph_from_trace_data(bot, ["aa"], {}, is_our_trace=True)
+        for c in bot.mesh_graph.add_edge.call_args_list:
+            assert c.kwargs["from_public_key"] != "aa11" + "0" * 60
+            assert c.kwargs["to_public_key"] != "aa11" + "0" * 60

tests/test_transmission_tracker.py

@@ -195,10 +195,11 @@ class TestExtractRepeaterPrefixes:
         result = tracker.extract_repeater_prefixes_from_path(None, path_nodes=["01", "7e", "86"])
         assert result == ["86"]
 
-    def test_filters_own_prefix(self, tracker):
+    def test_a_repeater_sharing_our_prefix_keeps_the_repeat(self, tracker):
+        """A companion never forwards, so a last hop matching our prefix is a repeater."""
         tracker.bot_prefix = "86"
-        result = tracker.extract_repeater_prefixes_from_path("01,7e,86")
-        assert result == []
+        assert tracker.extract_repeater_prefixes_from_path("01,86") == ["86"]
+        assert tracker.extract_repeater_prefixes_from_path(None, ["01", "86"]) == ["86"]
 
     def test_empty_path_returns_empty(self, tracker):
         result = tracker.extract_repeater_prefixes_from_path(None)
@@ -310,6 +311,18 @@ class TestBotPrefixFromSelfInfo:
         assert tracker.bot_prefix == "c3"
 
 
+class TestBotPrefixFollowsReconnect:
+    def test_a_reconnect_to_another_radio_changes_the_prefix(self, mock_logger):
+        bot = Mock()
+        bot.logger = mock_logger
+        bot.prefix_hex_chars = 2
+        bot.meshcore = Mock(self_info={"public_key": "c3" + "00" * 31})
+        tracker = TransmissionTracker(bot)
+        assert tracker.bot_prefix == "c3"
+        bot.meshcore = Mock(self_info={"public_key": "e5" + "00" * 31})
+        assert tracker.bot_prefix == "e5"
+
+
 class TestUpdateBotPrefix:
     """Cover lines 57-67: _update_bot_prefix with str and bytes public_key."""