From f5628acf8088fdf8d2deb78ac648500ec4972803 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 3 Oct 2026 09:59:19 -0700 Subject: [PATCH] refactor(message_handler): one mesh-graph capture guard and one contact-capacity check _mesh_graph_capturing() replaces the three copies of `hasattr(self.bot, "mesh_graph") and self.bot.mesh_graph and self.bot.mesh_graph.capture_enabled`, evaluated in the same order after the same preceding conditions. _manage_contact_capacity(contact_name, after_add) replaces the two copies of the near-limit check in handle_new_contact (device mode, and bot mode after an add attempt). The WARNING and INFO lines keep their exact text, including the " after add" and " after add attempt" suffixes in bot mode; new tests pin all four lines and the cleanup call, and pass against the code before this commit. --- modules/contact_events.py | 50 +++++++++++++--------------------- modules/mesh_graph_recorder.py | 4 +++ modules/message_handler.py | 14 ++-------- tests/test_message_handler.py | 29 ++++++++++++++++++++ 4 files changed, 54 insertions(+), 43 deletions(-) diff --git a/modules/contact_events.py b/modules/contact_events.py index 4ad5646..78a2f04 100644 --- a/modules/contact_events.py +++ b/modules/contact_events.py @@ -16,6 +16,7 @@ class ContactEventsMixin: """Mixed into MessageHandler.""" _advert_rf: Any + _mesh_graph_capturing: Any _new_contact_adds: Any _store_observed_path: Any _update_mesh_graph_from_advert: Any @@ -124,13 +125,7 @@ class ContactEventsMixin: # This can trigger many send_mesh_edge_update() calls in quick succession; # if the web viewer is down, that produces a wave of connection-refused logs. path_byte_length = packet_info.get("path_byte_length") or (len(out_path) // 2 if out_path else 0) - if ( - out_path - and out_path_len > 0 - and hasattr(self.bot, "mesh_graph") - and self.bot.mesh_graph - and self.bot.mesh_graph.capture_enabled - ): + if out_path and out_path_len > 0 and self._mesh_graph_capturing(): self._update_mesh_graph_from_advert(advert_data, out_path, path_byte_length, packet_info) # Store complete path in observed_paths table. Empty-path (direct RF) @@ -348,6 +343,21 @@ class ContactEventsMixin: del self._new_contact_adds[next(iter(self._new_contact_adds))] return True + async def _manage_contact_capacity(self, contact_name: str, after_add: bool = False) -> None: + """Clean up the device contact list when it is near its limit; log either way.""" + status = await self.bot.repeater_manager.get_contact_list_status() + if status and status.get("is_near_limit", False): + self.logger.warning( + "Contact list near limit (%.1f%%) — managing capacity" + (" after add" if after_add else ""), + status["usage_percentage"], + ) + await self.bot.repeater_manager.manage_contact_list(auto_cleanup=True) + else: + self.logger.info( + "Companion %s — contact list has adequate space" + (" after add attempt" if after_add else ""), + contact_name, + ) + async def handle_new_contact(self, event: Any, metadata: dict[str, Any] | None = None) -> None: """Handle NEW_CONTACT events for automatic contact management""" try: @@ -505,18 +515,7 @@ class ContactEventsMixin: "Device mode — companion %s tracked; firmware handles addition; bot may manage capacity", contact_name, ) - status = await self.bot.repeater_manager.get_contact_list_status() - if status and status.get("is_near_limit", False): - self.logger.warning( - "Contact list near limit (%.1f%%) — managing capacity", - status["usage_percentage"], - ) - await self.bot.repeater_manager.manage_contact_list(auto_cleanup=True) - else: - self.logger.info( - "Companion %s — contact list has adequate space", - contact_name, - ) + await self._manage_contact_capacity(contact_name) elif auto_manage_setting == "bot": # One add per advert: NEW_CONTACT can repeat for the same packet. The # tracking result can't tell, since the advert packet itself was @@ -549,18 +548,7 @@ class ContactEventsMixin: self._release_new_contact_add(public_key, packet_hash) self.logger.error("Error adding companion %s to device: %s", contact_name, e) - status = await self.bot.repeater_manager.get_contact_list_status() - if status and status.get("is_near_limit", False): - self.logger.warning( - "Contact list near limit (%.1f%%) — managing capacity after add", - status["usage_percentage"], - ) - await self.bot.repeater_manager.manage_contact_list(auto_cleanup=True) - else: - self.logger.info( - "Companion %s — contact list has adequate space after add attempt", - contact_name, - ) + await self._manage_contact_capacity(contact_name, after_add=True) else: self.logger.warning( "Unknown auto_manage_contacts value %r — treating as manual for %s", diff --git a/modules/mesh_graph_recorder.py b/modules/mesh_graph_recorder.py index c6832c1..de38850 100644 --- a/modules/mesh_graph_recorder.py +++ b/modules/mesh_graph_recorder.py @@ -16,6 +16,10 @@ class MeshGraphRecorderMixin: bot: Any logger: Any + def _mesh_graph_capturing(self) -> Any: + """Truthy when the bot has a mesh graph and it is capturing edges.""" + return hasattr(self.bot, "mesh_graph") and self.bot.mesh_graph and self.bot.mesh_graph.capture_enabled + def _update_mesh_graph(self, path_nodes: list[str], packet_info: dict[str, Any]) -> None: """Update mesh graph with edges from a message path. diff --git a/modules/message_handler.py b/modules/message_handler.py index 714c9cc..40d21a4 100644 --- a/modules/message_handler.py +++ b/modules/message_handler.py @@ -1992,11 +1992,7 @@ class MessageHandler(MeshGraphRecorderMixin, ContactEventsMixin): if path_hashes: path_string = ",".join(path_hashes) self.logger.debug(f"Path from TRACE packet: {path_string} ({len(path_hashes)} hops)") - if ( - hasattr(self.bot, "mesh_graph") - and self.bot.mesh_graph - and self.bot.mesh_graph.capture_enabled - ): + if self._mesh_graph_capturing(): self._update_mesh_graph_from_trace(path_hashes, packet_info) else: path_string = "Direct" if hops == 0 else f"Unknown routing ({hops} hops)" @@ -2006,13 +2002,7 @@ class MessageHandler(MeshGraphRecorderMixin, ContactEventsMixin): path_string, path_nodes, hops = self._get_path_from_rf_data( recent_rf_data, payload_hex=payload_hex, packet_info=packet_info ) - if ( - path_string - and path_nodes - and hasattr(self.bot, "mesh_graph") - and self.bot.mesh_graph - and self.bot.mesh_graph.capture_enabled - ): + if path_string and path_nodes and self._mesh_graph_capturing(): self._update_mesh_graph(path_nodes, packet_info) if path_string and not had_routing_nodes: self.logger.debug(f"Path from fallback decode: {path_string} ({hops} hops)") diff --git a/tests/test_message_handler.py b/tests/test_message_handler.py index e2a022d..0ff3943 100644 --- a/tests/test_message_handler.py +++ b/tests/test_message_handler.py @@ -2944,3 +2944,32 @@ class TestChannelPayloadCorrelation: assert result[RF_MATCH_KEY] == RF_MATCH_FALLBACK assert result["packet_prefix"] == "cc" * 16 + + +@pytest.mark.asyncio +class TestNewContactCapacityLogs: + """The contact-capacity check after a companion advert, in device and bot mode.""" + + @pytest.mark.parametrize( + "mode, near_limit, level, message", + [ + ("device", True, "warning", "Contact list near limit (%.1f%%) — managing capacity"), + ("device", False, "info", "Companion %s — contact list has adequate space"), + ("bot", True, "warning", "Contact list near limit (%.1f%%) — managing capacity after add"), + ("bot", False, "info", "Companion %s — contact list has adequate space after add attempt"), + ], + ) + async def test_capacity_log_and_cleanup(self, new_contact_env, mode, near_limit, level, message): + bot, handler, rm, mesh = new_contact_env + bot.config.set("Bot", "auto_manage_contacts", mode) + rm.get_contact_list_status = AsyncMock( + return_value={"is_near_limit": near_limit, "usage_percentage": 92.5} + ) + await handler.handle_new_contact(_NewContactEvent(_companion_contact_payload()), None) + expected_arg = 92.5 if near_limit else "Alice" + calls = getattr(bot.logger, level).call_args_list + assert any(c.args == (message, expected_arg) for c in calls), calls + if near_limit: + rm.manage_contact_list.assert_awaited_once_with(auto_cleanup=True) + else: + rm.manage_contact_list.assert_not_called()