From 66176a173bbcbe858d32a5d3296be4ed4104a45c Mon Sep 17 00:00:00 2001 From: agessaman Date: Thu, 1 Oct 2026 23:00:14 -0700 Subject: [PATCH] refactor(weather): keep wx and gwx's companion-location DEBUG lines The codex review of this port also found that the shared companion lookup dropped the three DEBUG lines wx and gwx logged on dev (no sender key, no row for the key, the position found). get_companion_lat_lon takes a trace flag that wx and gwx pass; rain and aurora, which never logged these, stay quiet. With trace, a bot without a db_manager logs the lookup error at the caller's level again instead of returning silently. --- .../commands/alternatives/wx_international.py | 2 +- modules/commands/wx_command.py | 2 +- modules/location.py | 16 ++++++++-- tests/unit/test_weather_compat_surface.py | 29 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/modules/commands/alternatives/wx_international.py b/modules/commands/alternatives/wx_international.py index 7354cb5..c7c93dc 100644 --- a/modules/commands/alternatives/wx_international.py +++ b/modules/commands/alternatives/wx_international.py @@ -154,7 +154,7 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand): def _get_companion_location(self, message: MeshMessage) -> Optional[tuple[float, float]]: """Get companion/sender location from the contact-tracking database.""" - return get_companion_lat_lon(self.bot, message, self.logger, error_level="warning") + return get_companion_lat_lon(self.bot, message, self.logger, error_level="warning", trace=True) def _get_bot_location(self) -> Optional[tuple[float, float]]: """Get bot location from config ([Bot] bot_latitude, bot_longitude).""" diff --git a/modules/commands/wx_command.py b/modules/commands/wx_command.py index cbaaa22..0b35298 100644 --- a/modules/commands/wx_command.py +++ b/modules/commands/wx_command.py @@ -345,7 +345,7 @@ class WxCommand(WeatherCommandMixin, BaseCommand): def _get_companion_location(self, message: MeshMessage) -> Optional[tuple[float, float]]: """Get companion/sender location from the contact-tracking database.""" - return get_companion_lat_lon(self.bot, message, self.logger, error_level="warning") + return get_companion_lat_lon(self.bot, message, self.logger, error_level="warning", trace=True) def _get_bot_location(self) -> Optional[tuple[float, float]]: """Get bot location from config ([Bot] bot_latitude, bot_longitude).""" diff --git a/modules/location.py b/modules/location.py index be2d157..909f209 100644 --- a/modules/location.py +++ b/modules/location.py @@ -629,15 +629,20 @@ def get_config_default_lat_lon(bot: Any, section: str) -> Optional[tuple[float, def get_companion_lat_lon( - bot: Any, message: Any, logger: Any = None, error_level: str = "debug" + bot: Any, message: Any, logger: Any = None, error_level: str = "debug", trace: bool = False ) -> Optional[tuple[float, float]]: """The sender's most recent advertised position from contact tracking, or None. A lookup error is logged on ``logger`` at ``error_level`` and returns None. + With ``trace`` (wx and gwx), the lookup's outcome is also logged at DEBUG. """ try: sender_pubkey = getattr(message, "sender_pubkey", None) - if not sender_pubkey or not hasattr(bot, "db_manager"): + if not sender_pubkey: + if trace and logger is not None: + logger.debug("No sender_pubkey in message for companion location lookup") + return None + if not hasattr(bot, "db_manager") and not trace: return None query = """ SELECT latitude, longitude @@ -651,7 +656,12 @@ def get_companion_lat_lon( results = bot.db_manager.execute_query(query, (sender_pubkey,)) if results: row = results[0] - return (float(row["latitude"]), float(row["longitude"])) + lat, lon = float(row["latitude"]), float(row["longitude"]) + if trace and logger is not None: + logger.debug(f"Found companion location: {lat}, {lon} for pubkey {sender_pubkey[:16]}...") + return (lat, lon) + if trace and logger is not None: + logger.debug(f"No location found in database for pubkey {sender_pubkey[:16]}...") except Exception as e: if logger is not None: getattr(logger, error_level)(f"Error getting companion location: {e}") diff --git a/tests/unit/test_weather_compat_surface.py b/tests/unit/test_weather_compat_surface.py index 24f37f2..ebafb3c 100644 --- a/tests/unit/test_weather_compat_surface.py +++ b/tests/unit/test_weather_compat_surface.py @@ -4,6 +4,8 @@ import configparser from types import SimpleNamespace from unittest.mock import Mock +import pytest + import modules.commands.alternatives.wx_international as wx_international import modules.commands.wx_command as wx_command from modules.commands.alternatives.wx_international import GlobalWxCommand @@ -51,3 +53,30 @@ def test_solarforecast_honours_its_enabled_setting(): cmd = SolarforecastCommand(_bot({"Solarforecast_Command": {"enabled": "false"}})) message = SimpleNamespace(content="sf Seattle", channel="general", is_dm=True, sender_id="u", sender_pubkey="pk") assert cmd.can_execute(message) is False + + +@pytest.mark.parametrize("cls", [WxCommand, GlobalWxCommand]) +@pytest.mark.parametrize(("pubkey", "rows", "logged"), [ + (None, [], "No sender_pubkey in message for companion location lookup"), + ("ab" * 16, [], "No location found in database for pubkey " + "ab" * 8 + "..."), + ("ab" * 16, [{"latitude": 47.5, "longitude": -122.25}], + "Found companion location: 47.5, -122.25 for pubkey " + "ab" * 8 + "..."), +]) +def test_wx_and_gwx_log_the_companion_lookup_as_on_dev(cls, pubkey, rows, logged): + bot = _bot() + bot.db_manager.execute_query = Mock(return_value=rows) + cmd = cls(bot) + cmd.logger = Mock() + cmd._get_companion_location(SimpleNamespace(sender_pubkey=pubkey)) + cmd.logger.debug.assert_any_call(logged) + + +def test_rain_keeps_its_quiet_companion_lookup(): + from modules.commands.rain_command import RainCommand + + bot = _bot() + bot.db_manager.execute_query = Mock(return_value=[]) + rain = RainCommand(bot) + rain.logger = Mock() + rain._get_companion_location(SimpleNamespace(sender_pubkey="ab" * 16)) + rain.logger.debug.assert_not_called()