_locate_coordinates, _locate_zipcode and _locate_city each return the error reply (or None), the coordinates and the location prefix; _noaa_forecast_body fetches and formats one forecast type; _with_noaa_alerts builds the default reply and its two-message alert tuple. An unrecognized location_type still geocodes as a city without the city prefix, as before. Old-vs-new over 30,000 randomized cases (fake geocoders, NOAA fetches and alert results, including exceptions and error sentinels) matched on replies, call traces and log calls; four mutations were each caught.
wx passes a named WXSIM source as location_name=, which the fake did not accept, so every named-source wx case recorded the error reply instead of a successful send or multiday split. The regenerated golden passes against both the code before e76383f2 and after it.
geocode_location now dispatches to _geocode_coordinates, _geocode_zipcode and _geocode_city inside its original try, so a failure in any of them still logs and returns four Nones. _format_location_display takes its city from _display_city and its last-resort label from _fallback_location_label. Bodies moved verbatim; an old-vs-new run over 30,000 label cases and 20,000 geocode cases (faked lookups that succeed, miss or raise) found no difference in results or log lines.
The custom MQTT and WXSIM replies, the no-location fallback chain (sender's position, default_city, bot's position) and the reply tail (two-part alerts, multiday split, single message) move into WeatherCommandMixin. Each command keeps its own WXSIM fetch (_fetch_wxsim) and wx keeps naming fallback positions in its log lines (_fallback_place_name). The no-location MQTT reply still never splits a multiday answer. The execute dispatch goldens are unchanged.
Records the ordered calls each request makes, with every provider mocked: custom MQTT and WXSIM sources (default and named), the no-location fallback chain, provider failures, the two-part alert reply, and wx's alert-only geocoding. Pins execute before its shared steps move into WeatherCommandMixin.
TelegramBridgeService returned from __init__ as soon as it found no api_token (or no HTTP library), before setting message_queues, send_times, http_session and _queue_processor_task. start() then returned early as intended, but stop() read _queue_processor_task and raised AttributeError, so every shutdown logged "Failed to stop service 'telegram_bridge'". DiscordBridgeService had the same gap on its no-HTTP-library path. Both now set that state before any early return.
New tests start and stop each self-disabled bridge and check that only the reason it disabled itself is logged as an error; all three fail on dev.
Codex review of 9b86cd63: ChannelBridgeBase ran the DM guard and the channel lookup before parsing "sender: message", but both bridges evaluated `':' in text` first. A non-string text (None, an int) on a DM or unmapped channel used to log "Error handling mesh channel message" with exc_info and now logged a DEBUG skip instead. The split runs first again. The bridge replay now includes None, int, bytes and list texts; it differs from the pre-bridge code on 9b86cd63 and matches it after this commit.
_config_section_values, _restart_only_config_changes, _validate_config_snapshot, _command_config_state and _apply_command_config_state move verbatim (AST-identical) to modules/config_reload.py, mixed into MeshCoreBot. reload_config itself stays in core: the reload-rollback characterization tests patch modules.core.CommandManager, which it calls. _COMMAND_CONFIG_STATE stays on MeshCoreBot (annotated on the mixin for mypy). core.py 1,269 → 1,154 lines.
set_radio_clock, set_device_name and send_startup_advert move verbatim (AST-identical) to modules/device_setup.py, mixed into MeshCoreBot after the existing mixins; callers, the web viewer and tests reach them on the bot as before, and no test patches a name they use. core.py 1,428 → 1,269 lines.
start, stop, on_transport_reconnected, _on_mesh_channel_message (payload copy, DM guard, sender split, profanity filter) and _process_message_queues (retry, backoff, age limit) move to ChannelBridgeBase in modules/service_plugins/channel_bridge_utils.py; the _utils name keeps the service loader from taking the base class for a service. Each bridge keeps what differs through small hooks: its channel lookup (Telegram ignores a leading '#', Discord doesn't), text preparation (Discord formats mentions before the profanity filter, Telegram builds its HTML after it), delivery, rate limit (25 per minute per webhook vs one per second per chat), HTTP session setup and posting, and its own wording for the old-message, retry and processor-error lines through format templates. The requests fallback stays: it still runs when there is no aiohttp session.
Imports the two modules no longer use are gone (copy, contextlib, meshcore's EventType); BaseServicePlugin, censor and contains_profanity stay as re-exports.
Checked by driving both bridges through 48 scripted scenarios (four configs each, async and requests paths, a fake clock, scripted 2xx/429/5xx/timeout/error responses and Discord rate-limit headers, start/reconnect/stop): every post, logger and meshcore call, queue, send time and rate-limit record is identical before and after, apart from bound-method reprs naming ChannelBridgeBase. Eight mutations across the base and both bridges each change the output.
handle_contact_message's copy of the payload/metadata/signal-cache lookup becomes a _message_signal call with its own SNR keys (it also accepts signal_to_noise and signal_noise_ratio), so DMs and channel messages share one implementation. The helper loses a comment that only made sense inline.
A DM replay (1,500 corpus packets as RF rows, 1,500 DMs with varied SNR/RSSI key layouts, metadata, cached signal values keyed by pubkey prefix, with and without radio contacts) gives byte-identical processed messages and logger calls; passing the channel keys instead changes them. The channel replay is unchanged.
handle_channel_message stays on MessageHandler (the plan keeps the handle_* entry points there) and its blocks move to modules/channel_message.py, unchanged apart from returning what they used to assign: _split_channel_sender, _message_signal (SNR/RSSI from payload, metadata or the signal cache; the DM handler can share it), _channel_route_from_rf (signal values, decoded packet, hops and path from the correlated RF row, still refusing a fallback row's route, #80), _decode_scope_packet, _channel_reply_scope, _channel_scope_allowed (the flood_scopes allowlist, returning False where it returned) and _full_sender_pubkey. Two copies of a stale comment are gone.
CHANNEL_SENDER_FALLBACK and _signal_value move with them, since channel_message can't import from message_handler; message_handler imports both back, so existing imports keep working.
Checked with a new replay: the 4,009-packet corpus plus TRACE and truncated packets as RF log rows, interleaved with channel messages (120 decrypted with the public channel key, the rest synthetic, some arriving with an empty RF cache) in four configurations (no allowlist, flood_scopes keys, '*' only, keys plus '*' with old cached messages). Processed messages, flood-scope observations, every logger, mesh graph and web viewer call and the caches are byte-identical, as is the RF log replay. Mutations in six of the helpers each change the output; replacing the raw-hex path guess with None and dropping int() from cached RSSI don't, because the replay's undecodable packets never yield a guessed path and its cached RSSI values are already ints.
handle_rf_log_data goes from 358 to 147 lines. Its blocks become methods, moved without change apart from returning what they used to assign: _rf_packet_hash, _track_own_repeat (matching a heard packet to the bot's own transmissions and recording repeats), _rf_routing_info, _warn_path_inconsistency, _log_rf_routing (the ROUTING INFO / DIRECT MESSAGE line) and _library_scope_fields. The web viewer capture, advert handling and the RF row itself stay inline. One `decoded_packet.get("payload_type", None)` became `.get("payload_type")` (ruff SIM910; the same on a dict).
Replay parity (the 4,009-packet corpus plus TRACE and truncated packets, three setups including a tracker that sometimes finds no repeater prefix and perturbed decodes that fire all three path warnings): caches, logger, tracker, viewer, mesh graph and bot calls are byte-identical. One mutation in each extracted step changes the output.
handle_rf_log_data and _transmission_evidence (only it calls that) move verbatim (AST-identical) to modules/rf_log.py, which MessageHandler mixes in; core's RX_LOG_DATA subscription and the tests call them on the handler as before. message_handler keeps importing calculate_packet_hash as a re-export.
A replay of the 4,009-packet corpus plus TRACE packets from the dev bot and truncated packets, in three setups (no tracker or viewer; a transmission tracker; tracker, web viewer and a capturing mesh graph with perturbed decodes so all three path-consistency warnings fire), gives byte-identical caches, logger, tracker, viewer, mesh graph and bot calls before and after.
The by-name and by-pubkey-prefix loops over meshcore.contacts become two _find_contact calls with the same predicates, in the same order, and the pubkey lookup still runs only when the name lookup found nothing and a prefix is set. An old-vs-new fuzz (20,000 random contact sets, senders and RF prefixes, comparing the result and every logger call) found no differences; two mutations of the predicates each produced over 1,000.
handle_rf_log_data no longer updates the caches itself. _remember_signal replaces its two copies of the LRU-bounded SNR/RSSI cache update, and _insert_rf_row takes the block that files a built row: the authenticated channel cache, the TC_FLOOD debug line, recent_rf_data, the timestamp and prefix indexes, and eviction. Log text and levels are unchanged.
Checked by replaying the 4,009-packet decode corpus through handle_rf_log_data on the old and new code (frozen clock, small cache limits so eviction runs, with and without a transmission tracker, varied SNR/RSSI/metadata/scope fields): the caches, indexes, every logger and tracker call and 400 lookups afterwards are byte-identical, and three mutations of the moved code each change the output.
find_recent_rf_data, _correlate_channel_message_rf_data, the channel payload check (_find_rf_row_matching_chan_payload, _rf_data_matches_chan_payload), the authenticated channel cache (_channel_message_identity, _channel_secrets, _decode_authenticated_channel_identity, _cache_authenticated_channel_rf_data, _find_authenticated_channel_rf_data) and _cleanup_stale_cache_entries move verbatim (AST-identical) to modules/rf_correlation.py. MessageHandler mixes it in, so names, callers (rf_match, scope_gate, multitest) and tests are unchanged, and the cache state stays on the handler, where about 130 test references set or read it, many on object.__new__ instances.
message_handler keeps importing channel_hash_for_key and decrypt_group_text as re-exports.
#342 removed the pending-message correlation list. message_handler keeps this branch's mixin layout without PendingMessageEntry; the default config comment for message_correlation_timeout moves into modules/templates/default_config.ini (core.py keeps reading the template, which matches dev's inline text otherwise); both sides' new tests at the end of test_message_handler.py stay.
The public-API golden drops exactly the five names #342 removed on purpose.
When a channel message had no RF log row yet, the handler stored it in pending_messages, slept 100 ms, then looked up find_recent_rf_data(payload["pubkey_prefix"]) through correlate_message_with_rf_data. Nothing else read the list: try_correlate_pending_messages only set a "processed" flag nobody checked, and cleanup_old_messages pruned it, both on every received packet.
The wait and the second lookup stay, now written out directly. Results only differ where the list misbehaved (found by codex review): two messages entering the wait in the same millisecond with the same packet prefix shared one entry, so both got the second one's row, and now each gets its own; with message_correlation_timeout under 0.1 s the entry could be pruned during the wait, skipping the second lookup; and a non-string pubkey_prefix made the per-packet scan raise, logged as "Error handling RF log data". The list's DEBUG lines are gone with it.
The list, its four methods and PendingMessageEntry were public names, so this goes in as its own change rather than a refactor.
message_correlation_timeout is still read: it bounds how old an RF row can be and still match a channel message by its contents (#255). The example configs described it as a wait time; the comment now says what it does.
New tests cover the wait (a row logged during it is found; no wait without enhanced correlation or for scope lookups). They pass against the previous code and one fails without the wait.
Codex review of f5628acf: returning the `and` chain handed a falsy mesh_graph back to the caller's `if`, which truth-tested it a second time; the inline guards tested it once. The helper now returns a bool built from one test of the graph and one of capture_enabled. New tests count the truth tests and fail on the previous helper.
_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.
handle_new_contact, _process_advertisement_packet, _ensure_contact_meshcore_path_encoding and the NEW_CONTACT helpers from #335 (_remember_advert_rf, _find_advert_rf_data, _claim_new_contact_add, _release_new_contact_add) move verbatim (AST-identical) to modules/contact_events.py. MessageHandler mixes it in, so names, callers (core's NEW_CONTACT subscription, the prefix collision service) and tests are unchanged. message_handler keeps importing upsert_zero_hop_observed_path_via_manager and encode_path_len_byte as re-exports.
The one test that patched modules.message_handler.upsert_zero_hop_observed_path_via_manager now patches it in modules.contact_events, where the moved code looks it up.
_update_mesh_graph, _store_observed_path, _update_mesh_graph_from_advert, _update_mesh_graph_from_trace and the two location lookups they use move verbatim (AST-identical) to modules/mesh_graph_recorder.py. MessageHandler mixes it in, so every method keeps its name and callers and tests are unchanged. message_handler keeps importing update_mesh_graph_from_trace_data as a re-export; unique_recent_repeater_key, which only this branch had imported there, moves with its user.
command_manager: _record_transmission passes extra keyword arguments through, so #336's recipient_key (DMs) and channel_idx (channels) reach record_transmission as on dev.
graph_trace_helper: dev's rewrite from #336, with its two repeater-key lookups (immediate neighbor and _unique_repeater_key) going through contacts_repo.unique_recent_repeater_key, whose queries are identical.
Each attempt waited only for its own tag, so a reply that arrived after its attempt's timeout was dropped once the retry went out. On my test mesh a three-hop trace takes about nine seconds (each repeater waits its random TX delay before forwarding), against a 2.5 s timeout, so every trace failed even though the first attempt's full reply came back.
run_trace now subscribes to TRACE_DATA once, before the first send, and takes a reply to any of its tags until the last attempt's timeout runs out. A reply that lands during the retry delay ends the trace without sending again, and subscribing before the send also closes the window where a fast reply could arrive before wait_for_event was listening.
The timeout defaults go from 1.0 s base and 0.5 s per hop to 2.0 s and 1.5 s, matching the 1.5 s the trace command and its settings schema already used. Configs that set them keep their values.
Codex re-review follow-up. The one-hop shortcut wrote dd->dd both ways when the neighbor shares the bot's prefix; it now skips the update, as the multi-hop path does. A distance error (a stored infinite latitude) no longer drops the trace's edges; the edge is recorded without a distance, as before the rewrite.
Keys and locations follow chain positions instead of prefixes, so a hop whose hash equals the bot's short prefix no longer takes the bot's public key or location, and an edge between two equal prefixes, which the prefix-keyed graph would store as a self-loop, is skipped.
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.
bot_prefix and graph_trace_helper's bot key read meshcore.device, which meshcore_py doesn't have, so both were always None and update_mesh_graph_from_trace_data skipped every trace. Both now come from meshcore.self_info's public_key (device stays a fallback), and bot_prefix is read lazily because the tracker is built before the radio connects. The helper names the bot at the trace's hash width, so the last hop's edge matches the hops' own width, and marks 2-byte edges with prefix_bytes=2.
trace_command read update_graph_two_byte but never used it; 2-byte traces now follow it instead of update_graph_one_byte.
get_noaa_weather's Tomorrow part tested period.get('temperature', '') for truthiness, so a forecast of exactly 0 was treated as missing and the part was left out. It now uses _has_temp, as _noaa_period_str already does for Today and Tonight.
After #334 the two blocks were identical, so get_noaa_weather calls one helper for both. Tomorrow keeps its own thresholds inline. An old-vs-new run over 30,000 generated NOAA forecasts (log calls included) shows no difference; a threshold mutation in the helper shows 1,021.
Codex re-review follow-up. A cancelled add (reconnect cancelling callbacks) kept its claim, so a later NEW_CONTACT for the same advert never added; it now releases and re-raises. A last_advert of 0 is a real advert timestamp, not a missing one. The expiry test now fills the advert cache it is meant to exercise.
Codex review follow-up. A channel message from another node with the same text claimed our record, and names the sender-prefix heuristic cannot split ("Bot [RF]", "Bot: RF") never matched; the match now compares the full decrypted message with "<radio name>: <text>", falling back to the text alone before the name is known. Bot shutdown closes the tracker after disconnecting, which waits for queued repeat-count writes; any later write runs inline.
Codex review follow-up. The RX handler cached an advert's RF entry only after awaiting _process_advertisement_packet, which can yield (geocoding), so a NEW_CONTACT handled meanwhile found nothing or an older advert. Adverts are now recorded in their own short cache (_advert_rf) before that await. The match also requires the advert timestamp to equal NEW_CONTACT's last_advert, so a delayed copy of an older advert no longer stands in for the newer one; without last_advert the most recent advert is used.
A failed or raising add releases its claim, so a later NEW_CONTACT for the same advert tries again.
match_packet_hash gave a pending transmission the hash of the first packet heard within its 30-second window, whatever the packet was. On a busy mesh that was usually someone else's message or advert, so its repeats were counted as ours (the web viewer's repeat counts, DARC MoWaS's ACK check, graph_trace_helper's "our trace" test) and our real repeats were not.
A transmission now records what identifies its packet (the channel index for a channel message, the recipient's key for a DM, the tag for a trace) and match_packet_hash claims it only for a packet carrying the same: a GRP_TXT that decrypts on that channel to the same text, a TXT_MSG addressed to the recipient's hash and, when the radio's key is known, from ours, or a TRACE with the same tag. The RX handler passes that evidence through _transmission_evidence, reusing the authenticated channel decode. Without evidence, match_packet_hash only looks up hashes already matched, so record_repeat and graph_trace_helper no longer claim pending transmissions.
Repeat-count writes go to a single worker thread instead of running sqlite on the event loop, and _update_command_in_database no longer inserts into sys.path on every repeat.
bot_prefix still reads meshcore.device and stays None; fixing it turns on graph_trace_helper's trace updates, which have never run, so it is a separate change.
handle_new_contact walked the last ten RF cache entries and used the first with routing info, whatever its type or sender. That packet's route went into the contact's out_path (and from there to add_contact in bot mode), the mesh graph and observed_paths, and its hash and signal data into tracking.
It now uses _find_advert_rf_data: the cached ADVERT whose payload starts with the contact's public key, newest advert first, and among the copies of that packet the first one heard, within rf_data_timeout. The mesh graph and observed_paths writes are gone: _process_advertisement_packet already records them for the same packet, so a correct match was counted twice.
With the correct hash, tracking reports the packet as a duplicate (the advert packet was tracked first), and bot mode skipped add_companion on that. The add gate now dedupes on the (public key, packet hash) pairs NEW_CONTACT itself added, so repeated events for one advert still add once.
The S1 dead-code slice dropped both because nothing in the tree reads them, but local plugins may (codex review of the dev merge). set_radio_clock records the sync time again after a successful update. Two DEBUG diagnostics S1 removed with dead code stay removed: parse_advert's flags line behind handler.debug and the overwritten contact-path lookup lines.
agessaman