fix(placeholders): unify the keyword formatter's hop count

format_keyword_response_with_placeholders kept the third copy of the hop-count
logic, and it was the weakest: it consulted message.hops and then went straight
to parsing the path display string, never looking at routing_info. So a keyword
reply and a command reply could describe the same packet differently.

Three cases disagreed, all now resolved:

    case                     keyword   command
    routing path_length          ?  ->  2
    routing path_nodes           ?  ->  3
    routing beats path text      1  ->  2

The last is the one that was actually wrong rather than merely unhelpful: with
both present it took the display string over the packet's own path_length.
routing_info is the decoded packet, so it wins, which is what BaseCommand
already did.

All three formatters now call utils.message_hop_count. Behaviour is unchanged
wherever routing_info is absent, so a message carrying only a path string still
parses the same way and "?" still means the count cannot be determined at all.
This commit is contained in:
agessaman
2026-08-25 10:12:26 -07:00
parent 7debb92e9e
commit 85fca4fa4c
3 changed files with 63 additions and 18 deletions
+7
View File
@@ -8,6 +8,13 @@ semantic versioning.
### Fixed
- `{hops}` and `{hops_label}` in a `[Keywords]` response now report the same hop count
a command response would for the same packet. The keyword formatter carried its own
implementation that consulted only `message.hops` and the path display string, so it
answered `?` whenever the count was known only from `routing_info`, and preferred a
stale path string over the packet's own `path_length` when both were present. All
three formatters now share `utils.message_hop_count`.
- `pathbytes_min` no longer treats a direct message as a multi-byte path, so
`{path_distance|pathbytes_min:2|prefix_if_nonempty: | Path Dist: }` stops printing
`| Path Dist: N/A` on a hopless packet. `bytes_per_hop` describes how a path is
+8 -18
View File
@@ -2455,26 +2455,16 @@ def format_keyword_response_with_placeholders(
replacements['timestamp'] = time_str
replacements['packet_hash'] = get_packet_hash_placeholder(message)
# Total hops: use message.hops when set, else parse from path string (e.g. "01,5f (2 hops)")
hops_val = getattr(message, 'hops', None)
if hops_val is not None and isinstance(hops_val, int):
replacements['hops'] = str(hops_val)
else:
path_str = message.path or ""
hop_match = re.search(r'\((\d+)\s*hops?', path_str, re.IGNORECASE)
if hop_match:
replacements['hops'] = hop_match.group(1)
elif re.search(r'\bdirect\b|\b0\s*hops?\b', path_str, re.IGNORECASE):
replacements['hops'] = "0"
else:
replacements['hops'] = "?"
# Pluralized label: "1 hop", "2 hops", or "?" when unknown
h = replacements['hops']
if h == "?":
# Shared with BaseCommand.get_hops_display_values and the hops_min filter, so
# a keyword response and a command response report the same hop count for the
# same packet. "?" only when the count cannot be determined at all.
hops_val = message_hop_count(message)
if hops_val is None:
replacements['hops'] = "?"
replacements['hops_label'] = "?"
else:
n = int(h)
replacements['hops_label'] = "1 hop" if n == 1 else f"{n} hops"
replacements['hops'] = str(hops_val)
replacements['hops_label'] = "1 hop" if hops_val == 1 else f"{hops_val} hops"
else:
# No message - use defaults for message-based placeholders
replacements['sender'] = "Unknown"
+48
View File
@@ -712,6 +712,54 @@ class TestFormatKeywordResponseWithPlaceholders:
result = format_keyword_response_with_placeholders("{hops_label}", msg, bot)
assert result == "3 hops"
def test_hops_prefers_routing_info_over_the_path_string(self):
"""The keyword formatter used to parse only the path string, so it reported a
different hop count than the command formatter for the same packet."""
bot = self._bot()
msg = self._msg(hops=None, path="01 (1 hop)", routing_info={"path_length": 2})
with patch("modules.utils.calculate_path_distances", return_value=("", "")):
result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot)
assert result == "2|2 hops"
def test_hops_falls_back_to_counting_routing_path_nodes(self):
bot = self._bot()
msg = self._msg(hops=None, routing_info={"path_nodes": ["01", "02", "03"]})
with patch("modules.utils.calculate_path_distances", return_value=("", "")):
result = format_keyword_response_with_placeholders("{hops}", msg, bot)
assert result == "3"
def test_hops_still_parses_the_path_string_without_routing_info(self):
bot = self._bot()
msg = self._msg(hops=None, path="01,5f (2 hops)")
with patch("modules.utils.calculate_path_distances", return_value=("", "")):
result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot)
assert result == "2|2 hops"
def test_hops_is_unknown_only_when_nothing_can_be_determined(self):
bot = self._bot()
msg = self._msg(hops=None, path=None, routing_info=None)
with patch("modules.utils.calculate_path_distances", return_value=("", "")):
result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot)
assert result == "?|?"
def test_hops_matches_the_command_formatter_for_the_same_message(self):
"""One implementation, so a keyword reply and a command reply cannot disagree."""
from modules.commands.base_command import BaseCommand
bot = self._bot()
for kwargs in (
{"hops": 3},
{"hops": None, "routing_info": {"path_length": 2}},
{"hops": None, "path": "Direct"},
{"hops": None, "path": "01,5f (2 hops)"},
{"hops": None},
):
msg = self._msg(**kwargs)
with patch("modules.utils.calculate_path_distances", return_value=("", "")):
keyword_hops = format_keyword_response_with_placeholders("{hops}", msg, bot)
command_hops, _ = BaseCommand.get_hops_display_values(Mock(), msg)
assert keyword_hops == command_hops, kwargs
def test_connection_info_contains_snr_rssi(self):
bot = self._bot()
msg = self._msg(snr=12, rssi=-75)