Five subscribe handlers repeated the same lock-and-flag block, now `_mark_subscribed`; the command, packet and message handlers repeated the same packet_stream query and emit loop, now `_replay_packet_stream` with a type-to-event mapping. Each handler keeps its own debug line and its own 'Error replaying ... history' warning. The command and message replays now also select `type`, which they filter on anyway. Both helpers live in app.py, so `request`, `emit` and `sqlite3` still resolve through `modules.web_viewer.app`.
The three-pass lookup (named Tomorrow, named for tomorrow's weekday, or the periods after today's) moves out of format_tomorrow_forecast, each pass returning early; the write-only current_day_periods counter goes. Old and new agree on 30,000 generated forecasts across every weekday and hour with a frozen clock, including the number of datetime.now() calls and the error logs; a mutation of the two-period cap was caught.
A codex review found that moving dashboard code out of app.py dropped `modules.web_viewer.app.DashboardStatsService`. The public-API golden only recorded names a module defines, so I added a second one, `public_imports.json`, recording the public names each module imports from another project module, generated from dev's tree as the baseline. It found 20 more names earlier refactor commits had stopped importing (the Nominatim and MQTT-weather helpers in wx, gwx, aqi and solarforecast; `PayloadVersion`, the region-warning verdicts and `decode_path_len_byte` in message_handler; two feed helpers and two feed limits in the web viewer). All are imported again as re-exports, so `modules.<module>.<name>` and patches of it resolve as they did on dev.
The tomorrow/multiday branch repeated the request, status check and JSON parse; all three types now share one. The current-conditions block becomes `_open_meteo_current` and the today/tomorrow/precipitation tail `_open_meteo_daily_tail`, both moved verbatim apart from dedenting, and both still raise into get_open_meteo_weather's handler. The unit symbol and `daily` are read before the current conditions instead of in the middle (both are plain lookups). The new Open-Meteo goldens are unchanged.
6267db74 flipped it to `> max_length: break`, which a codex review caught: a NaN max_length then admitted lines instead of none, and a None or string max_length logged the error with '>' instead of '<='. The original comparison is back.
`modules/web_viewer/dashboard.py` holds building the dashboard stats service from config, its database connection, the 7-day multi-byte contact count it is given, and the background refresher: five methods moved verbatim, still on `BotDataViewer`.
`modules/web_viewer/live_stream.py` holds the SocketIO broadcasts to subscribed clients (commands, packets, mesh edges and nodes, messages, log lines), the log tail thread and the database poll: nine methods moved verbatim, still on `BotDataViewer`. `_strip_ansi_codes` and `_ANSI_ESCAPE_RE` moved with them and are re-exported from app.
The two weekday scans in format_multiday_forecast become `_multiday_day_name`, and the three-branch day/night bookkeeping collapses to one condition: a period replaces the stored one unless it is a night period and the stored one is a day period. `is_day` is now `'night' not in name` ("tonight" already contains "night"). Old and new agree on 40,000 generated forecasts, including the error log calls; a mutation of the update rule was caught.
The two copies of the startTime parse become `_parse_noaa_start_time` (its three branches parsed the same way), the 12-hour label is one expression, and each hour's line comes from `_hourly_line`. Exceptions still reach the method's own handler, so error replies are unchanged. Old and new agree on 30,000 generated period lists (offsets, Z times, unparseable and non-string times, missing fields, null precipitation) including the error log calls; two deliberate mutations were caught.
Eight extract_* methods (humidity, precipitation chance and probability, UV index, dew point, visibility, wind gusts, pressure) each looped over their own regex list with an optional range check. Their patterns are now module-level tuples and one helper does the loop; the methods keep their names and signatures. Old and new agree on 152k texts (the NOAA fixtures plus generated keyword/number mixes, including out-of-range values and non-ASCII digits).
The main loop computed the same clamped [Connection]/[Bot] radio_probe_interval_seconds expression inline that the health probe already reads through the helper.
`modules/service_supervisor.py` holds starting services at boot, the backoff-gated restart of unhealthy services and `get_system_health`: six methods moved verbatim, still reachable on `MeshCoreBot`. Stopping services stays in `_stop_inner`, where it is one step of the shutdown order.
`modules/radio_link.py` now holds connecting and reconnecting the transport, the host->radio command serializer and `radio_session`, zombie state and the periodic health probe: 20 methods moved verbatim, so every name is still on `MeshCoreBot`. `_serialize_command_frames` and `_radio_session_held` moved with them and are re-exported from `modules.core`; the serializer's `bot` annotation is now the mixin. `core.py` keeps `import meshcore` because tests patch `modules.core.meshcore.MeshCore`. core.py: 2,164 → 1,599 lines.
Brings in #309, #311, #314, #317, #319 and #320. Conflicts: command_manager's imports (kept both `inspect` and `math`) and wx's alert send, where I kept the S1 removal of the dead `weather_data[3]` statement and took #319's `skip_user_rate_limit=True`. The NOAA wx goldens are regenerated because #320 changes the wind text (`💨N0` → `⬆️N0`); nothing else in them moved.
Review of the get_noaa_weather breakup: computing the abbreviated text before the temperature/forecast check evaluated len(shortForecast) for periods the old code skipped, so a null shortForecast raised inside the catch-all and replaced the whole reply with an error. The abbreviation now runs inside the guard again. A test with null Tomorrow fields passes on the code before the breakup, fails on 5ff0f84 and passes now.
Six copies of 'if value: piece = ...; append if it fits' become one loop over (value, template) in the same order (humidity, dew point, visibility, precipitation, gusts, pressure). Observation values still win and the forecast-text extractors still run only when the observation lacks a value; the precipitation probability still comes from the text alone. Six discarded 'current_weather_length + ...' expressions go. Goldens unchanged; mutating the order, a template, the fallback or the length check fails them.
The Today, Tonight and Tomorrow blocks each rebuilt the same ' | Name: <emoji><forecast> <high/low or temp>' string and the same fit-checked wind suffix. They now call _noaa_period_str (None when the period lacks a temperature or forecast) and _noaa_period_wind with each block's own threshold: max_length - 10 for Today and Tonight, Tomorrow's fixed 115/120. Tomorrow's text shortening is _abbreviate_noaa_forecast, verbatim. Each block's control flow stays in place, including Tonight's details step sitting outside its temperature check. The NOAA goldens are unchanged, and mutating each helper fails them. get_noaa_weather drops from 394 to 281 lines.
abbreviate_wind_direction matched full words (NORTHWEST, NORTH, ...), but NOAA's forecast windDirection is always an abbreviation like WNW, SSE or N. Every value fell through to the fallback, which kept the first two characters behind a generic wind emoji, so replies read '💨WN2' or '💨SS1' and even plain N and NE lost their arrow. The 16 compass abbreviations now return the nearest 8-point arrow plus the abbreviation; full words and other values behave as before. Found while pinning wx's formatting on real NOAA responses for the maintainability refactor.
The seven channel helpers (listing, extra decode channels, number lookup, lowest free index, statistics, and the queued add/remove operations) move verbatim to modules/web_viewer/channels.py, apart from a function-local 'import sqlite3' the module import now covers. BotDataViewer inherits the mixin. app.py drops to about 5,570 lines.
_get_tracking_data, _get_contact_detail and _calculate_distance move to modules/web_viewer/tracking.py (ContactTrackingMixin); the database statistics, table info and description, optimization and cache view move to modules/web_viewer/database_admin.py (DatabaseAdminMixin). All nine bodies are verbatim; BotDataViewer inherits both. tracking.py joins app.py on mypy's suppressed list, since the moved code carries the type errors mypy never checked in app.py. app.py keeps validate_sql_identifier and humanize_span imported so those module attributes still resolve. No test patched anything these methods use. app.py drops to about 5,800 lines, from 9,095 at the start of the refactor.
Review found the same reply-limiter drop in two more places: the earthquake service's link post right after its alert, and the announcements command's 'sent' confirmation right after the announcement. Both now skip the reply limiter. multitest called wait_for_tx itself just before send_response, which waits again; with wait_for_tx claiming the slot that pre-wait made the reply wait a full extra interval, so it goes. A test that only documented RateLimiter's behavior, and guarded nothing, is replaced by an earthquake test that drives _check_earthquakes.
Eight stop() blocks did 'if task: task.cancel(); with suppress(CancelledError): await task'. BaseServicePlugin._cancel_tasks(*tasks) does the same, in order, skipping None, and like awaiting directly it re-raises an error a task had already finished with. Callers that reset their task attributes to None still do; the weather service still does not. DARC MoWaS (cancels without waiting) and packet capture (skips finished tasks) keep their own loops.
The earthquake poll and the weather service's alert, rain-nowcast and lightning loops were the same twelve lines: run the work, sleep the configured interval, stop on cancellation, and on any other error log it and wait 60 s. BaseServicePlugin.run_periodic now holds that loop; the four loop methods keep their names (tests look for them among spawned tasks) and their start logs, and pass the interval as a callable so it is still read every round. The error lines render the same text, now always through %s formatting. The World Cup loop (wakes early on fastcast pushes) and the bridges' queue processors have their own shapes and stay as they are.
The eight methods that derive multi-byte path edges and neighbor-confirmed edges for the mesh graph, and the NeighborEvidenceKeys tuple they return, move verbatim from app.py into modules/web_viewer/mesh_evidence.py; BotDataViewer inherits the mixin and app.py re-exports NeighborEvidenceKeys. The only textual change is an unquoted return annotation. app.py keeps timedelta and NamedTuple imported so those module attributes still resolve. The one test that froze modules.web_viewer.app.datetime to pin _compute_multibyte_evidence_edges' cutoff now freezes it where the method lives. app.py is now about 7,120 lines.
The nine methods behind the multi-byte rollout page (hop-chunk collection and caching, path encoding badges, single-byte relay metrics, legacy degree and the rollout payload) move verbatim from app.py into modules/web_viewer/multibyte_rollout.py; BotDataViewer inherits the mixin, so callers and tests are unchanged. The mixin declares the viewer attributes it reads for mypy. app.py's imports are untouched. app.py drops to about 7,540 lines.
A user who raised bot_tx_rate_limit_seconds to 5 noticed wx's weather-then-alerts reply did not seem to honor it. The gap between those two messages was in fact longer than the limit (every send already waits on the bot TX limiter in _check_rate_limits, and wx sleeps limit + 1 s on top), but looking at it turned up two real bugs.
The second part of wx's and gwx's two-part reply, and every part after the first of 'alert all', went through the reply rate limiter (rate_limit_seconds, 10 s by default) a few seconds after the first part had used it, so it was refused: with default settings the alert message was never sent. These now pass skip_user_rate_limit=True, as the other multi-part senders already did. The repeater command's two-part branch gets the same change for consistency.
BotTxRateLimiter.wait_for_tx only checked the time since the last recorded transmission, and record_tx runs after the radio confirms, so concurrent senders (two replies, a reply and a scheduled message) all passed the wait before any of them recorded and transmitted back to back. wait_for_tx now claims the slot under the lock when it returns. A coverage test that monkeypatched can_tx to drive the old loop is rewritten against the new one.
The twelve feed methods (subscription CRUD, activity, errors, statistics, preview and item formatting) and the two module helpers they use move from app.py into modules/web_viewer/feeds.py; BotDataViewer inherits the mixin, so every method stays on the viewer for routes and tests. Method bodies are unchanged apart from dropping function-local 'import sqlite3' lines the module import now covers, and _validate_feed_interval's annotation (object to Any, for mypy, which now checks this module). app.py keeps every name it imported for them so modules.web_viewer.app.<name> still resolves. The two tests that patched create_safe_requests_session and safe_requests_request to steer _preview_feed_items now patch them where the method lives; the SafeUrlPolicy.validate patches target the shared class and are unchanged. app.py drops to about 8,200 lines.
A second review of the restart fix found more of the same class. The map uploader's stop() removes its file log handler and start() never added it back, so after one restart it logged nothing to file; start() now restores it. DARC MoWaS set _running before binding its port, so a port in use left it dead but healthy, and werkzeug's make_server calls sys.exit when the port is taken, which got past every except Exception and stopped the bot; start() now binds first and turns that exit into an OSError.
Boot-time starts go through _start_service_at_boot, which records a restart failure when start() raises, so that service also waits out the backoff, and no longer reports a disabled service as a configuration problem. The health-loop step is _restart_unhealthy_services. New tests cover both, the bridges' session cleanup and the map uploader restart; all 13 lifecycle tests fail on dev.
decode_path_len_byte, encode_path_len_byte, parse_trace_payload_route_hashes and calculate_packet_hash move verbatim to packet_decode.py, which already handled packet framing and imported decode_path_len_byte from utils; verify_meshcore_advert_ed25519 moves next to the advert parser in meshcore_payload_decode.py. utils re-exports all five as the same objects, and no test patched them. get_packet_hash_placeholder stays in utils with the response template it serves. utils.py drops to 1,630 lines.
Path parsing, hop counting, per-path distances and node location by prefix (ten functions and the hex-token set, about 450 lines) move verbatim from utils.py to mesh_paths.py, and utils re-exports every one of them, so existing imports and module.utils lookups are unchanged. The two functions that need calculate_distance import it from utils at call time, which avoids a circular import and keeps patches of modules.utils.calculate_distance effective. The five tests that patched _get_node_location_from_db to steer calculate_path_distances now patch it where it lives, and a companion test goes through the old modules.utils entry point. utils.py drops to 1,798 lines.
The 17 breaker methods #313 added or rewrote, and the trial context variable, move verbatim from core.py into modules/radio_offline.py; MeshCoreBot inherits them, so every method name and instance attribute stays where callers and tests use it. The only textual change is that the moved annotations are unquoted under from __future__ import annotations. core.py drops to 2,117 lines.
core.py conflicts were S3's simplifications of is_radio_offline, _record_send_failure and _record_send_success colliding with #313's rewrite of the same methods; dev's versions are kept.
Review found that marking a not-running service inactive for good also retired services that only returned early for a transient reason: packet capture and the map uploader give up after waiting 30 s for the radio, and the bridges return when meshcore is momentarily unset, which is exactly when the health loop restarts them. Such a service now records a restart failure, so the next attempt waits out service_restart_backoff_seconds; a transient failure recovers, a missing configuration costs one attempt per backoff instead of one every 5 s. The restart decision is _service_restart_due, which the tests now exercise directly.
Also from the review: packet capture and the map uploader never cleared the should_exit flag stop() sets, so after a restart their background loops exited at once while the service reported healthy; start() now clears it. The Discord and Telegram bridges close the HTTP session they open when they return early because meshcore is not available.
configparser's getfloat accepts 'nan', and NaN fails every comparison, so it passed both the negative-value check and the min() cap and reached send_msg_with_retry as the ACK wait. It now falls back to the default like a negative value. 'inf' was already capped at 60 s; both cases are in the parametrized test, and the nan case fails on dev.
A fifth review found two gaps. A scheduled trial that returned False was re-armed at once, but False also follows exhausted no_event_received retries, so the radio could be tried again and again without another answered probe. And a reply's success settlement ran in a worker thread that a cancelled caller could cancel while it was still queued, stranding the trial in flight.
_end_offline_trial ends a trial that went out without succeeding (False or an exception, from a reply or a scheduled send) under the state lock alone; only paths where nothing reached the send still re-arm. The success settlement is shielded, so it completes even if the caller is cancelled. The cancellation test queues the settlement behind a busy one-worker executor and fails without the shield.
The health loop restarted every enabled service whose is_healthy() was False. A service that returns from start() without running because it has nothing to do (Discord or Telegram with no channels, the map uploader with no key) is never healthy, and its restart raised nothing, so it counted as a success and cleared the backoff: the service was restarted every 5 seconds, logging a warning each time.
Services whose start() returns without running are now recorded as inactive, logged once, and skipped by the health loop. A restart that leaves a service running but unhealthy records a failure, so the next attempt waits out service_restart_backoff_seconds.
Restarts also double-subscribed: services subscribed to meshcore events in start() and never unsubscribed (the comments said meshcore cleans up; it does not). BaseServicePlugin gains _subscribe and _unsubscribe_all; the Discord, Telegram, map uploader, packet capture and prefix-collision services subscribe through it, unsubscribe in stop(), and drop the old instance's subscription before re-subscribing after a transport reconnect. Tests use meshcore's real EventDispatcher and fail on dev.
Only scheduled messages and interval adverts could claim the trial armed by an answered health probe, so a bot with neither stayed offline however healthy the radio became. send_dm and send_channel_message now claim an armed trial when offline and run the send through _send_as_offline_trial, under the trial's identity; its result settles the trial (True clears the outage, False or an exception fails it until the next answered probe), so a dead radio still gets at most one attempt per probe. Settlement runs in a worker thread because it can write bot_metadata. Tests drive the real CommandManager guard, and the alert-thread counting in the recovery tests no longer stops asyncio.to_thread's workers.
geocode_city and geocode_city_sync were two 300-line copies, and geocode_zipcode and geocode_zipcode_sync two more, differing only in await. Each now has one body written as a generator that yields its Nominatim requests; _run_geocode_steps_sync and _run_geocode_steps_async perform them with the matching rate-limited calls and send back the result or throw back the exception, so the body's own error handling (bare excepts included) sees what a direct call would. The async variant keeps its own rate limiter path.
The city copies had drifted on one rule, kept behind async_rules: the async variant always reverse-geocodes an unqualified city's first hit to judge whether it is an obscure place in the default country, with a looser name match, and reuses that lookup for address info; the sync variant only checks when address info is requested. tests/unit/test_geocode_city_variants.py pins both. A side-by-side run of the old and new module over 3,840 city cases and 960 zip cases (scripted results, failures and caches, both variants, with and without address info) matched on return values, Nominatim call sequence and cache calls; flipping any one rule branch produced differences.
Seven places in message_handler and graph_trace_helper ran the same pair of queries, byte for byte: count the repeaters and room servers whose key starts with the prefix and were heard within the recency window, and if exactly one was, fetch its key (starred first, then most recent). contacts_repo.unique_recent_repeater_key now runs it and returns (count, key); each caller keeps its own logging, including the two that report the match count. Same queries, same order, same handling of execute_query's empty result on a database error.
agessaman