Large File Refactor Plan

Status: In progress · Started: 2026-08-06

docs/development/AGENTS.md sets the size guidance for this repository:

Aim to keep Python modules under ~400 lines and HTML templates under ~300 lines. When adding more than one new class or multiple functions to a module already above 350 lines, create or use a sibling module/package instead of expanding the existing file.

When this plan was written (2026-08-06) the tree violated that guidance in 131 Python modules, 75 templates and 16 JavaScript files. That number will not go to zero in one pass, and trying would produce an unreviewable diff across the whole codebase. This document is the running plan: it records the inventory, the extraction strategy per file, and which phases have landed.

Do not read those figures as current. Counting every tracked *.py over 400 lines outside __pycache__/node_modules/venv/migrations:

The Python figure went up while eight files were being split, because the tree is also growing — the phases below retired roughly 20,000 lines of oversized module, and new work added more oversized modules than that removed. The template count has not moved at all: Phase 5 has not started. Treat the per-phase tables as the source of truth for what has actually landed, and re-run the count rather than trusting any number written here.

Ground rules for every extraction

These are what make a split reviewable and safe. Follow them on every file.

1. Move code verbatim. A refactor commit changes where code lives, not what it does. Behavioural changes go in a separate commit, so a reviewer can read the split diff as pure motion.
2. Keep the old import path working. The original module becomes a shim that re-exports the public names from the new package. Nothing outside the refactored area should need to change in the same commit — from app_core.radio.demodulation import FMDemodulator must keep working.
3. One seam per module. Split along a real boundary (data vs. logic, DSP kernels vs. protocol decode, route handlers vs. helpers), never at an arbitrary line number just to get under the cap.
4. Preserve __all__ and the module docstring on the shim so help() and star-imports behave as before.
5. Run the tests that cover the file before and after, and name them in the commit message. If a file has no coverage, that is worth knowing before it is moved.
6. Audit every __file__-relative path before moving a module. This is the one way "identical code" can still change behaviour: a module in a new package sits one directory deeper, so os.path.dirname(os.path.dirname(__file__)) now points somewhere else. AST-equality checking cannot catch it — the code really is identical, it just means something different. Grep for __file__ in the file you are about to split, and assert the resolved values against the pre-split module afterwards. This bit Phase 2b: the brand logo path and the tile disk-cache path both silently resolved one level short of the repository root, and neither failure raises — _load_logo() swallows the error and renders a card with no logo. Both are now pinned by tests.
7. Follow the existing naming convention. When a module is superseded, the old one is renamed with an _old suffix — never _new for the replacement. In practice most splits here need no rename because the original path stays as the shim.

The repository already has a worked example of this pattern: webapp/routes_audio_archive.py (928 lines) became the webapp/audio_archive/ package — fsutil.py, config.py, metadata.py, routes.py — in 2.133.1. New splits should look like that one.

Phase 1 — Data/code separation ✅

The cheapest wins: modules that are mostly a static data table with a thin layer of lookup logic wrapped around it. Nothing executable moves, so the risk is close to zero and the line-count win is large.

Landed in 2.134.0. The table is FCC/Census reference data, not code — keeping it inline meant every reader of the lookup helpers scrolled past 3,200 lines of county names, and every diff touching the module rendered them. The data file keeps the identical FIPS|ST|Name pipe format, so it stays greppable and diffable, and it is resolved relative to __file__ the same way the module already resolves the NWS partial-county .dbf from assets/. (The project is deployed as a git checkout, not an installed wheel, so no package_data entry is needed.)

Remaining Phase 1 candidates

None found above 800 lines. app_utils/event_codes.py and app_utils/zone_catalog.py carry data tables but are already under the cap.

Phase 2 — Signal-processing and rendering packages

Large single-purpose modules with clean internal seams. These are pure library code with no Flask involvement, which makes them the safest of the big splits.

The last two files are not the same kind of problem as the first three. 2a–2c were each several independent top-level definitions sharing one file, so splitting them was pure motion verifiable by ast.dump() comparison. These two are each one god-class: GPSManager was 2741 of gps_manager.py's 2893 lines, and _SoapySDRReceiver is 1822 of drivers.py's 2187. Module-level splitting cannot shrink a single class, so a claim in the 2c pull request that Phase 2 was complete was wrong.

The way through is two different techniques, in order:

1. Move the stateless parts as motion (2d) — verify with ast.dump().
2. Extract collaborators for the stateful parts (2e) — ast.dump() is useless here since the code is deliberately restructured, so verify with a characterization harness built before the refactor: snapshot all mutated state across a realistic input stream, confirm the baseline is discriminating, then diff after.

gps_manager.py has had both applied (2893 → 2313). drivers.py has had neither.

2a. app_core/radio/demodulation.py → app_core/radio/demod/ ✅

The single largest module in the tree, and six unrelated concerns in one file: Numba JIT kernels, generic DSP helpers, configuration dataclasses, the RBDS (Radio Broadcast Data System) decoder, the FM demodulator and the AM demodulator. The RBDS decoder alone is ~2,300 lines and has its own test file (tests/test_rbds_demodulation.py), yet could not be imported without pulling in the whole FM chain.

The dependency graph is strictly one-directional, with no cycles:

kernels ─┬─> dsp ─┬──────────────┐
         │        │              ├─> fm ──┐
         └────────┴─> rbds_worker┘        ├─> factory
rbds_constants ─> rbds_decoder ─┘         │
types ─────────────────────────> am ──────┘

app_core/radio/demodulation.py remains as a re-export shim so every existing from app_core.radio.demodulation import … keeps resolving — including the private _NUMBA_AVAILABLE that webapp/routes_monitoring.py reads.

Verification. The split was checked to be pure motion by comparing ast.dump() of every top-level definition before and after: 23 definitions, 23 matches, zero AST differences. tests/test_rbds_demodulation.py, tests/test_fm_stereo_decoder.py, tests/test_early_decimation.py and tests/test_eas_resampler.py pass unchanged (103 tests).

Still over the cap — follow-up needed. rbds_worker.py (2293) and rbds_decoder.py (1238) are each a single class, so module-level splitting cannot shrink them further. RBDSWorker has 27 methods covering pilot estimation, interference notching, timing recovery, Costas carrier recovery and group decoding — those are five collaborators wearing one class. Bringing them under the cap means extracting mixins or helper objects, which is a behaviour-adjacent change and belongs in its own commit with its own review. Tracked as Phase 2a-ii. fm.py (795) needs the same treatment, at lower priority.

2b. app_utils/image_export.py → app_utils/image_export/ ✅

The alert share-image renderer, whose seams were already marked by section comments in the file.

Dependency graph, generated from the actual imports and verified acyclic:

logo, layout, palette, fonts, text, icons   leaves
theme       -> palette
drawing     -> fonts, palette
weather_fx  -> drawing, layout, theme
tiles       -> layout
map_style     -> fonts
storm_overlay -> fonts, layout, tiles
maps        -> fonts, layout, map_data, map_style, palette,
               storm_overlay, theme, tiles
panels_text -> drawing, fonts, nws_text, palette, text
panels      -> drawing, fonts, icons, nws_text, palette, panels_text
render      -> drawing, fonts, layout, logo, maps, palette, panels,
               panels_text, text, theme, weather_fx

Here the package __init__.py is the compatibility shim — it re-exports all 125 names the single-file module exposed (including logger), so from app_utils.image_export import … is unchanged for app_core/notifications/alert_image.py and webapp/admin/api/.

Verification. 68 top-level definitions, 68 ast.dump() matches, zero differences. The slicing script also asserted that every non-blank line of the original landed in exactly one module — nothing silently dropped. tests/test_image_export_themes.py passes (123 tests, up from 120).

Follow-up (2.152.0). maps.py was split again when the map gained its basemap treatment: the storm overlay and the PostGIS lookups moved to siblings, bringing it from 904 back to 473 lines.

Follow-up (2.151.0). panels.py was still over the ~400-line guidance, so the three prose drawers moved into a new panels_text.py alongside the new nws_text.py parser they depend on. panels re-exports them, so no caller changed.

One real bug was introduced and caught. _LOGO_PATH and _TILE_DISK_CACHE_DIR_DEFAULT are built by walking up two directories from __file__. That was the repository root when the renderer was a single app_utils/image_export.py; inside the package every module is one level deeper, so both resolved one short — the share card rendered with no brand logo, and OSM tiles cached into app_utils/data/tile-cache (colliding with the directory Phase 1 had just created for the FIPS table). Neither failure raises, and the 120 existing tests all still passed. It surfaced only because a stray app_utils/data/tile-cache/ appeared in git status. Both constants now derive from a named _REPO_ROOT with a comment explaining the depth, and three tests pin the resolved paths — verified to fail without the fix.

The test needed two changes, and they are worth understanding before the next split. Both come from the same fact: a package has more than one namespace where the module had one.

1. The test deliberately loads the renderer by file path rather than importing it, to avoid pulling in the whole app_utils package. Loading a package that way needs submodule_search_locations on the spec, otherwise its from .theme import … internal imports resolve back through app_utils and undo the isolation.
2. Nine monkeypatch.setattr(image_export, …) calls had to move to the module that calls the patched name (tiles for _http, maps for _fetch_tile and _fetch_county_outlines, render for _render_map). Rebinding a name on the re-exporting package does not change what maps._render_map sees in its own globals. This was verified to be load-bearing rather than assumed: pointing the patches back at the package makes 5 tests fail.

Pre-existing issues found, deliberately left alone (fixing them is a behaviour change and does not belong in a pure-motion commit): maps.py has an unused shadow local (F841, present in the monolith), and test_render_map_draws_counties_and_scale_bar asserts only the output image's size and mode — despite its name and docstring it never checks that county outlines or the scale bar were drawn, so it passes whether or not its stubs take effect.

Import style

Both packages use relative intra-package imports (from .theme import …), matching webapp/audio_archive/, app_core/flask/, app_core/config/ and app_core/database/. Beyond consistency this is what lets a test load the package standalone, as tests/test_image_export_themes.py does. demod/ was converted from absolute to relative in the same release for this reason.

2c. app_utils/gpio.py → app_utils/gpio/ ✅

Four distinct subsystems shared one file: the GPIO backend abstraction (lgpio/sysfs/null), the GPIOController + behaviour manager, the NeoPixel controller, and the tower-light controller.

pin_types, backends, tower_light   leaves
neopixel        -> pin_types
controller      -> backends, pin_types
behavior        -> controller, pin_types
config_loaders  -> neopixel, pin_types, tower_light

Each optional-dependency probe moved to sit with its only consumer: get_gpio_settings / _GPIO_SETTINGS_AVAILABLE with the database loaders, PixelStrip / NeopixelColor / _NEOPIXEL_LIB_AVAILABLE with the NeoPixel controller, Device / OutputDevice / MockFactory with the backends. The package __init__.py re-exports all 72 names the single-file module exposed.

Verification. 28 top-level definitions, 28 ast.dump() matches, zero differences, plus the every-line-placed assertion. All six production importers were imported to confirm they still resolve.

Two lessons this phase added.

1. Do not name a package module types.py. It shadows the stdlib types for any process whose working directory is the package directory, and the stdlib import chain (dataclasses → re → enum → types) then fails with a confusing partially-initialised-module error. Renamed to pin_types.py. app_core/radio/demod/types.py has the same footgun — it is harmless in normal operation, so it was left alone rather than churning merged code, but it is a cleanup candidate.

2. Find monkeypatch targets with an AST scan, not a grep. String matching found 24 of the 31 patch sites here; it missed multi-line monkeypatch.setattr(\n gpio,\n "name",\n …) calls and a bare real_sleep = gpio.time.sleep attribute read. Walking the test files' ASTs for setattr calls whose first argument resolves to the package catches every form. The reusable check:

   # For each tests/*.py: find setattr calls targeting the package.
   for n in ast.walk(ast.parse(path.read_text())):
       if (isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
               and n.func.attr == 'setattr' and n.args):
           ...  # report the target expression and attribute name
   

2d. app_core/gps/gps_manager.py — stateless timing math extracted ⚠️ partial

GPSManager is 2741 lines in one class, 54 methods. Profiling it by self usage splits the class cleanly in two:

Six of the eight stateless methods (359 lines) moved out verbatim. Two stayed, deliberately:

• _sat_key (2 lines) — used only by _record_sat_seen / _record_sat_used, which remain on the manager. Moving a two-line helper would add a cross-module hop for no gain.
• _scan_capture (25 lines) — UBX frame scanning. Its natural home is the existing app_core/gps/ubx.py, not a timing-statistics module; folding it in here would have mixed two unrelated concerns in one commit.

The six that moved:

These were @staticmethod in all but name, so they were pure functions trapped inside a class. Extracting them makes them directly importable and testable — tests/test_gps_stability_metrics.py was already testing them through GPSManager._compute_allan_deviation(...), reaching past the class to get at a pure function.

Verification. All 6 moved functions are ast.dump()-identical to their originals once the @staticmethod decorator and docstring indentation are normalised (a module-level function indents its docstring 4, not 8); every non-blank removed line was asserted present in the new modules; and both implementations were run side by side over 5 interval datasets (including empty, single-sample and constant edge cases) with zero output differences. 135 GPS tests pass.

What is left, and why it is not motion. The other 2235 lines are stateful: _handle_sentence alone is 246 lines with 50 self references, accumulating fix state, satellite tracking and publishing as it parses. The plan's original "split manager / NMEA parsing / survey" wording assumed these were separable files; they are not. Doing it properly means giving the NMEA parser explicit state to return rather than mutating self, which changes behaviour-bearing code on the GPS/timing path and cannot be verified by AST comparison. That deserves its own design pass and reviewed commit — the same conclusion already recorded for controller.py and behavior.py in 2c.

A lesson this phase added — do not let head truncate a reference search. Grepping for the six method names before extracting appeared to show no test references, so the first attempt moved them without retargeting any tests. The search had been piped through head -20, and 20 unrelated _safe_read_text matches in app_utils/system.py consumed the entire output budget before a single real hit was printed. 17 tests then failed. When a search is being used to prove a negative, either drop the limit or filter the known-irrelevant matches out first — a truncated search cannot establish that something is absent.

2e. GPSManager._handle_sentence → app_core/gps/nmea.py ✅

The first collaborator extraction in this effort, as opposed to pure motion. _handle_sentence was 246 lines with 50 self references: four sentence branches (GGA/RMC/GSV/GSA) that interleaved NMEA field-mapping with manager state — satellite history, the holdover anchor, the system-clock sync policy and a Redis publish.

The seam is what the sentence says vs. what the manager does about it. app_core/gps/nmea.py (326 lines) owns the former and returns the latter:

_handle_sentence is now 30 lines: take the lock, call apply_sentence, apply the effects, publish. The clock-sync policy moved to its own _queue_time_sync. _FIX_QUALITY and _safe_int moved to nmea.py too — the NMEA path was their only consumer — and are re-exported from gps_manager so existing imports still resolve. gps_manager.py: 2893 → 2313 lines.

Deferring the effects is safe because none of _record_sat_seen, _record_sat_used or _mark_3d_fix reads self._fix; that was checked by AST before the seam was drawn, not assumed. Had any of them read the fix dict, applying effects after the parse instead of mid-parse could have changed behaviour.

Verification — this is where a characterization harness earns its keep. ast.dump() comparison is useless here: the code is deliberately restructured. So the safety net was built first, against the pre-refactor code:

1. A 28-sentence multi-GNSS stream — full cycles, multi-constellation GSV groups, several GSAs per cycle, no-fix → 2D → 3D transitions, empty GLGSA, blank/malformed fields, out-of-order GSV group numbers.
2. Snapshot all mutated state after every sentence: the fix dict, the GSV buckets, the GSA accumulator and cycle flag, the pending time sync, the per-PRN satellite history and the 3D-fix anchor.
3. Confirm the baseline is discriminating — 28 frames, 28 distinct states. A harness that records constant state proves nothing.
4. Refactor, re-run, diff with wall-clock timestamps scrubbed: 0 differing frames of 28.

tests/test_gps_nmea_sentences.py (19 tests) makes the harness permanent and adds direct coverage the old shape could not have: each rule is now assertable without constructing a GPSManager. Two of them were mutation-checked rather than trusted — reverting the GSV per-talker bucketing and the GSA union each failed exactly one test, and only that test.

Two traps this phase hit.

1. Re-typing a constant instead of moving it. _FIX_QUALITY and _safe_int were re-written from memory into the new module. Both were wrong: the fix-quality labels came out "invalid"/"gps"/"dgps" instead of "no_fix"/"gps_fix"/"dgps_fix", and the re-typed _safe_int dropped the int(float(s)) conversion so "12.0" would raise instead of yielding 12. Neither raises at import; both would have silently changed the dashboard. Move shared helpers by reference — grep for the real definition and cut it — and never re-type one from memory.
2. A truncated grep cannot prove a negative. See 2d: | head -20 hid every real hit behind unrelated matches, and the conclusion drawn from it ("no test references these") was wrong.

What is left in gps_manager.py

2313 lines, still over the guidance. The remaining bulk is the gpsd client (_gpsd_reader_loop, _gpsd_connect, _handle_gpsd_tpv, _handle_gpsd_sky, the watchdog and daemon restart), the serial reader loop, the PPS/kernel-PPS handling and get_status. Each is a plausible next collaborator and each wants the same treatment: characterize first, then extract. app_core/radio/drivers.py (one 1822-line _SoapySDRReceiver) is untouched and needs the same approach.

Phase 3 — Web layer

Flask modules where route handlers and their helpers are interleaved. The webapp/audio_archive/ package is the template to copy: helpers in topic modules, routes.py holding only handlers.

3a. webapp/admin/audio_ingest.py → webapp/admin/audio_ingest/ ✅

The largest Flask module in the tree, and the first web-layer split. Helpers and handlers were interleaved down its whole length — the stream-URL probe helpers sit at line 1575, between two route handlers.

blueprint, sanitize, probe        leaves
controller   -> (nothing in-package)
streaming    -> controller
serialization-> controller, sanitize, streaming
radio_sources-> controller, streaming
routes_*     -> blueprint + the helpers each one needs

The entry point is unchanged: register_audio_ingest_routes(app, logger) stays the package's only __all__ entry, so webapp/admin/__init__.py did not move.

Verification. 69 of the 73 top-level definitions are ast.dump()-identical before and after, every non-blank line of the original lands in exactly one module, and the full suite is green (1,997 passed). The four deliberate differences are each pinned by a test in tests/test_audio_ingest_package.py.

Three things this phase adds to the checklist, all of them consequences of one file becoming many namespaces.

1. A Blueprint's import_name changes when it moves. Blueprint('audio_ingest', __name__) in a blueprint.py resolves to webapp.admin.audio_ingest.blueprint, one level deeper than before — the same class of silent drift as the __file__ bug in 2b, since Flask derives the blueprint's root path (and any template/static folder it later gains) from it. __package__ is the pre-split value, so that is what the new module passes. Pinned by a test.
2. Never import a mutable module global across modules. remove_radio_managed_audio_source reads _audio_controller directly. A generated from .controller import _audio_controller binds None at import time and never sees the singleton _get_audio_controller later installs, so the local-controller fallback would have silently stopped removing sources — no exception, since if controller and … simply skips. It goes through a new _peek_audio_controller() accessor instead, mirroring the _get_auto_streaming_service() that the sibling global already had. A test AST-scans the package for by-value imports of any of the six mutable globals.
3. A logger global that a registration hook rebinds has to be fanned out. register_audio_ingest_routes did global logger; logger = logger_instance. With one module that was the whole story; with fifteen, rebinding the package's logger leaves every line that actually logs on its own. The hook now walks a _LOGGING_MODULES tuple, and a test asserts that tuple covers every module in the package that defines a logger.

The monkeypatch retarget was load-bearing, and loudly so. Four fixtures across three test files reset _audio_controller, _auto_streaming_service, _initialization_started, _streaming_lock_file, _audio_initialization_lock_file, _start_audio_sources_background, _reload_auto_streaming_from_env, _read_audio_metrics_from_redis and _restore_audio_source_from_db_config on the module. Deliberately not re-exporting the mutable globals from the package __init__ is what made this safe: monkeypatch.setattr raises AttributeError on a missing attribute, so all 9 patch sites failed immediately instead of turning into silent no-ops. Had the shim re-exported them for completeness, the resets would have kept "passing" while resetting nothing.

3a-ii. api_get_audio_sources → listing.py + source_payload.py ✅

The one module 3a left over the cap. routes_sources.py was 749 lines because api_get_audio_sources alone was 327: a single handler that decoded the audio service's Redis snapshot in four possible shapes, queried three tables, reconciled the database against the local controller, and built two different JSON bodies depending on which won.

Module-level splitting cannot shrink one function, so this is a collaborator extraction — the 2e technique, not the 2a–2d one — and it was verified the same way.

The write endpoints moved to routes_sources_write.py in the same pass, split along the permission boundary: all three carry @require_permission('receivers.configure'), and neither read endpoint does.

Verification — the harness came first, as in 2e.

1. tests/test_audio_source_listing.py (12 tests) was written and run green against the pre-refactor handler, covering all three live-state paths, the envelope counters, the dead-service escalation rule, malformed Redis payloads and a failing streaming service.
2. Confirmed discriminating before trusting it: five deliberate mutations — disabling the dead-service escalation, reversing Redis/database metric precedence, inverting db_only_count, letting Redis override the local streaming service, and dropping the reconstructed Icecast URL — each failed exactly the test that covers it, and only that test.
3. A separate dump script rendered the full response body for 8 scenarios (8 distinct bodies — a harness recording constant state proves nothing) against a git worktree at the pre-extraction commit and against the working tree: 0 differing bytes, with the script printing which module it patched so a silent fallback to the old shape could not masquerade as a match.

A trap worth recording: a mutation that does not apply looks exactly like a mutation that survived. The first run of the five reported four caught and one survived. The surviving one had been written with the wrong indentation in its search string, so str.replace matched nothing and the "mutant" was the original code. Assert that the mutation target was found before drawing any conclusion from the result.

A second one, from tidying up afterwards: ruff check --select F401 --fix over the package removed 124 genuinely unused imports — and would have removed the shim's re-exports too, silently breaking every external importer, except that ruff exempts __init__.py from F401 by default. That exemption is the only thing that made the command safe. Verify the shim still resolves after any automated import cleanup rather than relying on it.

3b. webapp/routes_public.py → webapp/public/ ✅

The second web-layer split, and structurally unlike 3a. audio_ingest.py had 73 top-level definitions sharing a file; routes_public.py had one. Its entire body was a single 2,779-line register(app, logger) with all 21 route handlers nested inside it, so no top-level definition could be moved at all.

That turned out to make the split easier, not harder. Every handler closed over exactly two names — app (for the @app.route decorator) and route_logger — which was checked by walking the AST for Name nodes resolving to register's scope rather than assumed:

So each surface keeps its own register(app, route_logger) and the handler bodies move verbatim — still nested inside a register, just a much smaller one. No reindentation, no rebinding, no signature changes.

_load_logs_data is a helper, not a route, and it is the only piece the three /logs handlers share. Wrapping it in build_logs_loader(route_logger) rather than re-signaturing it to take route_logger as a parameter keeps its 1,057-line body byte-identical — only the enclosing scope changed. logs.py receives it as a parameter named _load_logs_data, so the three call sites resolve it unchanged.

webapp/routes_public.py remains as a 31-line shim re-exporting register, so the route-module registry in webapp/__init__.py is untouched.

Verification. Three independent checks, each confirmed discriminating before its result was trusted:

1. AST equality — 21 of 21 handlers ast.dump()-identical before and after, plus the every-non-blank-line-placed-exactly-once assertion (2,753 lines, 0 unplaced).
2. URL map — every rule, endpoint and method set, sorted and diffed against a git worktree at the pre-split commit: 549 rules, 0 differences. Mutation-checked by deleting one register() call, which the diff caught.
3. Response bodies — all 28 public surfaces fetched through the test client on both sides and hashed with volatile content scrubbed: 28/28 identical, 28 distinct digests.

A trap this phase hit — an "identical length, different hash" diff is a scrubbing bug, not a regression. Nine pages first compared as differing with byte-for-byte identical lengths, which is the signature of an unscrubbed fixed-width random value rather than a content change. It was the per-session CSRF token, which base.html emits in three shapes; the harness only knew the <input value=…> one and missed <meta name="csrf-token"> and window.CSRF_TOKEN. Diff the raw bodies before concluding anything from a digest mismatch — the length equality was the tell.

Still over the cap — follow-up needed. logs_data.py (1116), stats.py (693) and alerts.py (648) are each dominated by one enormous function: _load_logs_data is 1,057 lines, stats is 645, alerts is 385. Module-level splitting cannot shrink a single function, so these need collaborator extraction with a characterization harness built first — the 2e / 3a-ii technique, not this one. Tracked as Phase 3b-ii.

3b-ii. _load_logs_data → webapp/public/logs_sources/ ✅

The first of the three modules 3b left over the cap. _load_logs_data was 1,057 lines in one function: a seventeen-way if/elif chain on log_type, where each branch queried its own table (or the systemd journal, or the compliance ledger, or an FCC report builder) and shaped rows into the generic log dict the template renders.

The seam was already drawn by the branches, so the interesting design question was not where to cut but what contract the pieces share. Every loader now takes one LogQuery and returns one LogPage:

webapp/public/logs_data.py: 1116 → 80 lines, now only a dispatch, with MIN_LOGS_PER_CATEGORY re-exported so the old import path still resolves.

aggregate.py is deliberately not a loop over the focused loaders, however much it looks like one. Three things differ, and each would be a visible regression if "de-duplicated":

1. The row shape. Merged rows carry a category label and a trimmed details payload; the focused views carry the full payload and an alert_identifier.
2. Fault tolerance. Each merged category is wrapped so one broken table cannot take the whole page down; in a focused view a failure should surface.
3. The cap. Each category is limited separately before the merge, so a chatty table cannot crowd the others out.

The one thing that was de-duplicated is get_sort_key, which the all and services branches each carried a private copy of — verified ast.dump() -identical before collapsing them into common.timestamp_sort_key.

Verification — the harness came first, as in 2e and 3a-ii. The function had no test coverage at all, which ground rule 5 says to find out before moving code rather than after.

1. tests/test_public_logs_data.py (79 tests) was written and run green against the pre-refactor function, asserting the full returned triple — display name, every key of every row, and the report metadata — for every log_type, plus the level-derivation rules, the fallback strings, and the limit arithmetic.
2. Confirmed discriminating before being trusted: 15 deliberate mutations, 15 caught, 0 survived, 0 misapplied, each failing only the tests that cover it.
3. A dump script rendered the loader's full output for 115 scenarios (23 log types × 5 parameter combinations) against a git worktree at the pre-refactor commit and against the working tree: 0 differing scenarios, 26 distinct outputs. The script prints which module it patched, so a silent fallback could not masquerade as a match.
4. The mutation run was repeated against the refactored structure — 16 mutations in their new homes, 16 caught — because passing on one shape says nothing about the other.

Three traps this phase hit.

1. cd persisted between the two dump runs, so the "after" run executed in the pre-refactor worktree and reported a perfect match against itself. The patched: line is what caught it: it named webapp.public.logs_data, a module that no longer carries the systemd collaborators after the split. This is precisely the failure the 3a-ii note predicted — print what the harness bound to, and check it.
2. A source restore does not invalidate the bytecode cache. The mutation that swapped two branches of the audio_health level rule reordered text of identical length, and shutil.move restored the backup's mtime — so CPython saw a matching (mtime, size) and kept running the mutated .pyc after the file had been restored. The symptom was one test failing in every subsequent mutation, which reads exactly like a flaky test. Purge __pycache__ on restore. (Note the family resemblance to the 3b lesson: equal length is a tell, not a coincidence.)
3. Seeding fires audit listeners. Inserting EASMessage / ManualEASActivation rows writes eas.broadcast / eas.manual_activation audit entries stamped with the real wall clock, which then showed up as four "differing" audit scenarios. They were genuinely different between runs and had nothing to do with the refactor — the fix is to seed the audit trail deterministically after everything else, not to scrub the diff.

All nine modules are under the 400-line guidance. aggregate.py came out at 418 on the first pass and was split again along the collector boundary rather than being recorded as a follow-up.

3b-ii (cont). stats() → webapp/public/stats_sections/ ✅

The second of the three. Where _load_logs_data was a dispatch — exactly one branch runs per request — stats() is an accumulation: seventeen try/except blocks in a row, each running a few queries, writing into a shared stats_data dict, and declaring its own fallback so one failing query cannot lose the whole dashboard. The repeated try/rollback/log/fallback shape is the thing worth extracting, and it becomes the StatsSection contract:

webapp/public/stats.py: 693 → 50 lines. Every module is under the guidance.

The pipeline order is declared explicitly in __init__.py, not implied by module grouping. It is load-bearing — EAS_FORWARDING, MESSAGE_TYPES and COVERAGE_OVERLAP each divide by total_alerts — and keeping the original sequence also keeps the error-log order unchanged on a broken database.

Verification. Harness first, as always: 32 characterization tests written and green against the pre-refactor handler, reaching the payload by replacing render_template with a capture and calling the view directly (the app's global login redirect otherwise gets in the way). Mutation-checked at 17/17 before the split and 17/17 after. The rendered payload was then compared key-by-key against a worktree at the pre-refactor commit across an empty and a populated database: 70 keys, 0 differences, with the dump printing the line count of register so the two runs could be proven to be different code.

Two findings worth recording.

1. 29 of the 31 trailing setdefault calls were dead. Each section already set its keys on both its success and its failure path, so the defaults could never fire. Only avg_durations and lifecycle_timeline had no producer at all. This was established with a static check, not by eye, after a mutation deleting one of the redundant defaults survived — the right outcome for a semantically null change, and the signal that sent me looking. The section contract now guarantees the property structurally, so only the two real defaults remain.
2. Retargeting mutations is part of the job. Seven of the seventeen stopped applying after the split, because the code they matched had legitimately been reworded — - 5 became - MAX_YEAR_LOOKBACK, the success test became _is_success. "Not applied" proves nothing, so each was rewritten against its new home before the run counted.

3b-ii (cont). alerts() → webapp/public/alerts_page/ ✅

The last of the three, and a third distinct shape. _load_logs_data was a dispatch (one branch runs per request); stats() was an accumulation (all sections run, contributing to one dict); alerts() is a pipeline — each stage consumes what the last produced, so the modules are stages and the seams are sequential.

parse_filters        request args → AlertFilters (clamped, allow-listed)
load_filter_options  dropdown values + headline counts
build_alert_query    AlertFilters → a filtered, sorted query
paginate_alerts      one page of rows, with a fallback
build_audio_map      generated EAS audio for those rows
load_manual_messages recent operator-originated activations
backfill_ipaws_audio lazy extraction for pre-extractor alerts

webapp/public/alerts.py: 648 → 112 lines. Phase 3b-ii is complete — every module in webapp/public/ is now within the guidance.

One duplication left in place, deliberately. The page and the PDF export have separate query builders, and the export's is a strict subset: it has no date-range, VTEC or superseded handling, so a PDF exported from a filtered page can contain rows the page was hiding. Unifying them would have been the obvious tidy-up and would have silently changed what operators get in a compliance export. It is documented at the top of pdf_export.py and pinned by test_pdf_export_ignores_filters_the_page_supports — recorded as current behaviour, not endorsed.

Verification. 73 characterization tests written and green against the pre-refactor handlers, reaching the boundary by replacing render_template and generate_pdf_document with captures. Mutation-checked 26/26 before and 26/26 after — with all 26 needing retargeting in between, since almost every mutated line had moved or been reworded. Template kwargs and PDF arguments were then compared across 34 query strings on both routes against a worktree at the pre-refactor commit: 68 scenarios, 37 distinct outputs, 0 differences.

A dead variable removed. The PDF export captured per_page and never used it — a standing ruff F841. ruff check --select F,E9 is now clean across the whole of webapp/public/.

The operational lesson from this phase is about the harness, not the code. The mutation runner rewrites files in place and restores them afterwards. It was started in the background while new modules were still being written into the same directory, and it duly picked the half-written files up as mutation targets — backing up, mutating and "restoring" them, which left one committed file altered and one new file silently carrying a mutant's arithmetic. Nothing was lost (git had the committed file; the new one was caught by reading the diff), but the rule is simple: never run a file-rewriting harness in the background against a tree you are still editing. Run it in the foreground, or against a worktree copy.

3c. webapp/routes_settings_radio.py → webapp/radio_settings/ ✅

The largest remaining Flask module, and a hybrid of the two shapes seen so far: eight module-level helpers (like 3a) plus a 2,114-line register() with 26 handlers nested inside it (like 3b). The 3b closure analysis is what made it tractable — walking the AST for Name nodes resolving to register's scope showed every handler captures only app and route_logger:

So the bodies move verbatim into topic modules that each keep their own small register(app, route_logger) — no reindentation anywhere.

webapp/routes_settings_radio.py remains as a 52-line shim. All 17 modules are within the guidance.

The one deliberate deviation from verbatim motion is deps.py. The radio tests inject fakes for get_redis_client, get_radio_manager, _log_radio_event, RADIO_CAPTURE_DIR and the other capture constants — names used by up to 19 definitions each, which after the split are spread across ten modules. A from .deps import get_redis_client in each would snapshot the real object at import time, so a stub set in one place would be silently ignored everywhere else. Route modules therefore go through the module (deps.get_redis_client()), giving exactly one patch point, and a module added later honours the stub automatically. 51 references were routed this way; every other line is untouched.

The shim deliberately does not re-export those names, so a monkeypatch.setattr aimed at the old location raises AttributeError instead of quietly patching something nothing reads. That is what made the retarget safe: all 11 patch sites failed immediately and pointed at the real seam.

Verification.

1. AST equality — 34 of 34 moved definitions ast.dump()-identical, plus the every-non-blank-line-placed-exactly-once assertion (2,394 lines placed; the 54 unplaced are the licence header, the import block, the register signature and __all__).
2. URL map — every rule, endpoint and method set diffed against a worktree at the pre-split commit: 549 rules, 0 differences. Mutation-checked by dropping one module from the registration tuple, which the diff caught (3 missing rules).
3. Logger identity — _module_logger was logging.getLogger(__name__), which would have silently become webapp.radio_settings.deps. It is pinned to the pre-split "webapp.routes_settings_radio", and register() derives the same getChild("routes_settings_radio") once and passes it down, so every log line keeps its original name.

Three traps this phase hit, all in the tooling rather than the code.

1. partition(")\n") found the licence header, not the import block. The copyright line ends (KR8MER), so a naive split to separate imports from body landed on line 3 and the "body" rewrite corrupted every module's imports. Split on a sentinel you actually control.
2. A regex cannot rewrite a name safely; it needs scope. Three functions import get_redis_client from app_core.redis_client locally, shadowing the module-level import from app_core.extensions — a different object. A regex rewrote both the import aliases (from x import deps.get_redis_client, a syntax error ruff caught) and those shadowed call sites, which would have been a silent behaviour change. The rewrite became AST-driven, skipping any function that rebinds the name.
3. ast.walk() does not respect scope boundaries. The first scope-aware version collected local bindings with ast.walk(fn), which descends into nested functions — so register() inherited every handler's local import and appeared to shadow names it never touches. Since shadowing is inherited downward, that silently skipped the rewrite for every handler. The symptom was a plausible-looking count (46 rewrites instead of 51). Cut nested FunctionDef/Lambda/ClassDef off explicitly when computing a scope.

3d. webapp/admin/api.py → webapp/admin/api/ ✅

The easiest of the four web-layer splits so far, and worth recording why: 3b and 3c were each one enormous register() with the handlers nested inside it, so the unit of motion was a closure. This file was 21 ordinary top-level definitions decorated with @api_bp.route, which is the 2a–2c shape. Every definition moved verbatim.

blueprint, county, hostinfo, motion       leaves
display_data           -> motion
routes_geometry        -> blueprint, county
routes_alert_detail    -> blueprint, county, display_data
routes_alert_export    -> blueprint, display_data
routes_alerts_list     -> blueprint, county
routes_boundaries      -> blueprint
routes_system          -> blueprint, hostinfo
routes_system_history  -> blueprint
routes_smart           -> blueprint

All 14 modules are within the guidance. register_api_routes(app, logger) is unchanged and is the package's only __all__ entry.

Verification. 21 of 21 top-level definitions are ast.dump()-identical, every non-blank line of the original lands in exactly one module, and the URL map is unchanged at 549 rules, 0 differences against a worktree at the pre-split commit. The 11 original lines that appear nowhere in the package are the re-rendered import statements, the module docstring and the Blueprint(...) line.

Derive import blocks; do not write them. Hand-writing them produced 127 ruff errors on the first attempt — a mix of F401 (imports the module does not need) and F821 (names it does need and did not get). The generator now narrows each of the original's import statements to the names a module actually uses, which keeps the original grouping and gets the answer right.

Compute free variables with symtable, not by counting ast.Name nodes. The first derived version still emitted two errors, because _extract_alert_display_data has a local variable named desc and name counting read that as a use of from sqlalchemy import desc (F401 + F811). This is the third appearance of the same bug — Phase 4a hit it with a parameter named text, and 3c with ast.walk() ignoring scope boundaries. symtable answers the question directly: at module scope keep names that are referenced and never bound, and inside a function keep the symbols is_global() reports.

Three dead imports fell out — flask.current_app, app_utils.vtec.extract_vtec_identity and optimized_parsing.json_dumps were imported by the single-file module and used by none of it. Deriving the import blocks removes this class of cruft for free.

A blueprint's root_path changes even when import_name does not. The checklist already says to pass the package name rather than the module's __name__ (Phase 3a), and blueprint.py does — __package__ is exactly webapp.admin.api. But import_name now names a package rather than a module, so Flask derives root_path as webapp/admin/api where it used to be webapp/admin. It is inert here (no template_folder, no static_folder, no open_resource), and a test pins that so adding one later is deliberate. Any split that converts a module into a package of the same name inherits this.

Tests that read the source as text break on the move, and the failure mode is asymmetric. tests/test_api_field_fixes.py and tests/test_detect_county_wide_false_positive.py both open() the API source and assert against the text. Pointed at a deleted file they fail loudly, which is fine — but pointed at a shim that no longer holds the code they would pass vacuously, which is worse than deleting them. Both now scan the package directory. Add this to the pre-split checklist alongside the spec_from_file_location item: grep the tests for the module's path, not just its import name.

A new test that passes alone can still fail in the suite — pytest imports every test module during collection. The new putnam_ohio fixture did import app_core.location and then reached through app_core.location. That works in isolation but raises AttributeError: module 'app_core' has no attribute 'location' under a full run: the submodule attribute is only set on the parent package by the import that first executes the submodule, and by collection time something else has already put it in sys.modules. Use importlib.import_module('app_core.location'), which returns sys.modules[name] and never touches the parent. tests/conftest.py documents the identical trap for app_core.auth. Run a new test file inside the whole suite, not just on its own — the failure only appears there, and the first full run of this phase reported 11 errors that a targeted run could not reproduce.

A test that mirrors the logic cannot catch a change to the original. test_detect_county_wide_false_positive.py reimplemented both heuristics locally and then grepped the source to confirm the original still matched — which is what the two brittle source checks were doing there. _detect_county_wide only reads alert.area_desc and alert.raw_json, so tests/test_api_package.py now exercises the real function against a stub alert and a monkeypatched get_location_settings. Both heuristics are mutation-checked (2 failures each). Writing those cases also surfaced that state_code holds the two-letter postal code (OH), not the spelled-out state — the _multi_county_list guard counts ", <state_code>" occurrences, so a fixture using "ohio" reinstates the false positive it was written to prevent.

3e. webapp/admin/certbot.py → webapp/admin/certbot/ ✅

The same shape as 3d — 22 ordinary top-level definitions — so the motion was easy. What made this one interesting is that it carried both hazards the checklist warns about, and it had no test coverage at all to catch either.

log, blueprint                 leaves
paths                  -> log
failures               -> paths
nginx                  -> log
staging                -> log, paths
install                -> log, nginx, paths
routes_pages           -> blueprint, log
routes_settings        -> blueprint, log
routes_status          -> blueprint, log, paths
routes_obtain          -> blueprint, log, paths
routes_obtain_execute  -> blueprint, failures, install, log, nginx, paths, staging
routes_renew           -> blueprint, failures, log, paths, staging
routes_actions         -> blueprint, install, log, paths

The __file__ hazard fired, exactly as the checklist predicted. CERTBOT_BASE_DIR = Path(__file__).parent.parent.parent / 'certbot_data' — three hops from webapp/admin/certbot.py to the repository root, but only two thirds of the way from webapp/admin/certbot/paths.py. Unfixed it resolves to <repo>/webapp/certbot_data, and nothing raises: certbot builds a fresh empty tree there and every existing certificate appears to have vanished. This is the same failure shape as Phase 2b's brand logo, and the same reason AST-equality cannot see it — the code really is identical, it just means something else. Fixed to four hops, pinned by a test asserting the resolved value, and a second test rejects any new Path(__file__).parent.parent.parent added at this depth.

A module-level logger is a __name__ hazard too, not just Blueprint and getLogger in the checklist sense. One logging.getLogger(__name__) served all 92 call sites, so per-module loggers would have renamed every record from webapp.admin.certbot to webapp.admin.certbot.<submodule> — invisible in tests, but it breaks a journald grep or a log filter keyed on the old name. There is one log.py holding getLogger(__package__) and every module imports logger from it.

Whether a by-value logger import is safe depends on the registration hook. 3a needed a fan-out list because register_audio_ingest_routes rebinds the module's logger; here register_certbot_routes only writes one line through the logger it is handed and never rebinds, so from .log import logger is safe. Check which kind you have before copying either pattern — the fan-out is unnecessary machinery when nothing rebinds, and a by-value import is silently wrong when something does.

routes_obtain_execute.py was 449 lines — over, at the time. obtain_certificate_execute is a single 387-line try block. Module-level splitting cannot shrink one function; that needs collaborators extracted from the body, which is behavioural and needs the behaviour pinned first. Doing that on a module with zero existing coverage is its own piece of work. Tracked as Phase 3e-ii, landed 2026-09-18 — see below.

Zero coverage is worth stating before the move, not after. Ground rule 5 says to run the tests that cover the file and name them in the commit message; here there were none, on the code that drives certificate issuance. The split added tests/test_certbot_package.py — 11 tests, all three structural guards mutation-checked.

Verification. 22 of 23 definitions ast.dump()-identical; the one difference is register_certbot_routes, retyped into the package __init__, whose docstring lost the trailing whitespace on one blank line. URL map unchanged at 549 rules, 0 differences, 14 certbot endpoints intact, and all four CERTBOT_* paths resolve to their pre-split values.

3e-ii. obtain_certificate_execute → obtain_validation.py + obtain_methods.py ✅

The last known-exception module in the whole 400-line audit. Unlike the module-level splits above, obtain_certificate_execute was one 387-line try block — a dispatch, in the 3b-ii sense: pre-flight validation and prerequisite checks shared by every request, then exactly one of three near-identical certbot invocations (standalone, nginx, webroot) runs per request, each shelling out to certbot and (for two of the three) nginx.

routes_obtain_execute.py: 454 → 99 lines — parse the request, validate, clear stale locks, check certbot is installed, handle a staging→production cert switch, then dispatch through a {'standalone': ..., 'nginx': ..., 'webroot': ...} lookup table. No known exceptions remain in the certbot package's size guard.

Zero coverage, same as 3e itself. tests/test_certbot_obtain_execute.py (28 tests) is the module's first-ever test, written and run green against the pre-refactor handler before anything moved. The permission decorator is bypassed via __wrapped__ (Phase 3h's technique) rather than standing up an authenticated Flask test client, since this file is about the handler's own control flow, not the auth layer.

The retargeting trap, avoided by not needing it. subprocess.run and time.sleep are the true I/O boundary and are patched globally (patch("subprocess.run", ...)), the same reasoning as psutil in 4a-ii: both are shared singleton modules, so a global patch reaches every module that calls them regardless of which one ends up owning the call after a split. That sidesteps the retarget entirely for those two — but the higher-level collaborators (_check_nginx_status, _install_certificate_internal, _explain_certbot_failure, _ensure_webroot_directory) are ordinary functions imported by value, so those patches still had to move from routes_obtain_execute to obtain_methods once the method bodies did.

A mutation sweep (20 mutations) caught 18 on the first pass; the other two were the same shape of isolation gap 4a-ii hit, not missing coverage.

1. The standalone port-in-use test asserted "already in use" in body["error"] against a mocked raw error message that already contained "already in use" verbatim — so the assertion passed whether or not the augmentation branch that adds "Another process may be using it" actually ran. Fixed by asserting on text only the augmented wrapper adds.
2. The webroot method's own "Permission denied" augmentation branch (as opposed to its sibling "No such file or directory" branch, which was tested) had no test at all. Added test_webroot_method_augments_permission_error.

No __file__ hazard — neither new module resolves a path relative to its own depth; both new modules sit at the same directory depth as their siblings, so paths.py's existing CERTBOT_* constants are unaffected.

Verification. All 28 new tests and the existing 10-test test_certbot_package.py suite pass unchanged (the latter's size-guard test was updated to drop routes_obtain_execute.py from known_exceptions, per its own comment: "if the exception ever gets fixed, this test should stop excusing it"). grep -n "__file__" across both new modules is empty.

3f. webapp/admin/maintenance.py → webapp/admin/maintenance/ ✅

31 top-level definitions, none over 250 lines, no module-level calls — the 3d/3e shape, and every one of the 15 modules landed under the guidance. 31/31 ast.dump()-identical, URL map unchanged at 549 rules / 0 differences.

The __file__ hazard fired again — second phase running. repo_root resolves tools/create_backup.py and tools/inplace_upgrade.py, is passed as their cwd, and locates the .env the environment editor writes. One hop short and all three point into webapp/. Two consecutive files have now carried this; assume the next one does too and grep for it first.

__all__ is not the public surface — the tree is. get_operation_status is a route handler, absent from this module's ten-name __all__, and imported by app_core/websocket_push.py inside a function whose caller swallows failures. Listing exports by hand is what let AudioSourceConfigDB break; the test AST-walks the repository for from webapp.admin.maintenance import … and asserts each name resolves.

Two adjacent blueprints, two opposite prefix conventions. maintenance_bp is registered with no url_prefix and writes /admin into every route decorator. certbot_bp, in the sibling package, is registered with url_prefix='/admin' and its decorators must not repeat it. Both are correct; either one "corrected" to match the other silently doubles or drops the prefix. The package tests now pin the resulting rules on both sides.

Check a retyped function against the original, not against memory. register_maintenance_routes is the one definition that does not move as a slice — it goes into the package __init__. Writing it from memory produced app.register_blueprint(maintenance_bp, url_prefix='/admin'), which is the certbot convention and would have moved all 17 routes to /admin/admin/…. The AST check would have caught it, but only after the fact; reading the six original lines first is cheaper.

3h. webapp/routes/alert_verification.py → package ✅

The second-hardest shape in Phase 3, and the one that best shows why the closure has to be measured. 687 of 1,668 lines were one register(app, logger) holding eight helpers and seven handlers.

Two kinds of motion, chosen by symtable rather than by eye. Four nested helpers capture nothing from register, so they were dedented to module scope — semantically identical, and ast.dump is unchanged because it ignores col_offset. The other four capture route_logger, repo_root or app, so each topic module keeps its own register(app, logger) rebuilding exactly those locals. Emitting both locals unconditionally left an unused repo_root in three modules (F841); emit only what that module's handlers actually capture.

The silent-no-op hazard, demonstrated rather than asserted. The checklist has said since 3a not to re-export mutable globals a test patches. This split is where that was actually measured: with _progress_dir and friends re-exported and the async tests patching the package, all six tests still pass — writing to the real temp directory instead of tmp_path. The patch rebinds a copy; OperationResultStore keeps reading the original. Nothing fails, and the test quietly stops testing isolation. Left off the package, the same call raises AttributeError naming the module. If you are ever unsure whether a re-export matters, re-export it and run the tests: passing is the bad outcome.

Group by the call graph, not by topic name. _process_temp_audio_file → _detect_comprehensive_eas_segments → _build_composite_audio_segment is one chain crossing three layers. Putting the two ends in one composite_audio module — which their names invite — sandwiched eas_detection into an import cycle. Python reports that as a partially-initialised module at startup, which names the symptom, not the mistake. The generator now walks the emitted imports and fails with the cycle spelled out; the package test does the same so a later edit cannot reintroduce one.

Inspect handlers through __wrapped__. app.view_functions[name] is the outermost decorator (require_auth), whose only free variable is the function it wraps — the first closure assertions passed nothing but {'f'}. Unwrap before reading co_freevars.

Do not assert co_freevars == () on a module-level function. It is always empty; the test is tautological. The invariant that can actually break is textual — a future edit inside a dedented helper reaching for route_logger compiles fine and raises NameError only on the branch that runs it. Assert on the source instead.

No __file__ hazard here: repo_root comes from app.root_path, so it does not move with the module's depth. Worth checking for explicitly rather than assuming, given 3e and 3f both had one.

Verification. 27 of 28 definitions — top-level and nested — ast.dump()-identical; the one difference is register, deliberately restructured to fan out. URL map unchanged at 549 rules, 0 differences, all 7 endpoints intact.

3g. app.py (1869) — assessed, not started ⚠️

Do not plan this one as another package split. It is a different problem from every file in Phase 3 so far, and the difference is not visible from the line count.

app.py does not contain an application factory. It builds the app at import time — app = Flask(__name__) at line 324 — and then configures it with 103 module-level statements (52 Assign, 24 bare calls, 20 If, 6 Try, 1 AnnAssign) interleaved with the decorated handlers. create_app(), at line 1839, is fifteen lines that update app.config, optionally call initialize_database(), and hand back the module-level singleton. Its shape:

• 103 module-level configuration statements, against only 28 definitions
• 4 error handlers, 2 context processors, 2 template filters
• before_request (200 lines), after_request (72), _record_traffic (120)
• initialize_database (192), _load_db_settings_into_config (71)
• 4 Click CLI commands
• an if __name__ == '__main__' block

The behaviour here is the statement order, and ast.dump() cannot verify order. Every check this plan leans on — definitions compare equal, every line lands once, the URL map is unchanged — would pass on a split that reorders configuration and silently changes what the app does. Secret-key resolution, SETUP_MODE, the database URL, session-cookie flags and the background-service guards all read and write app.config in sequence, and several are conditional on values set a few lines earlier.

What it actually needs, roughly in order:

1. Turn the module-level singleton into a real factory first, as its own commit with no file movement. Until construction is a function, "where a statement lives" and "when it runs" are the same thing, and nothing can move safely.
2. Characterize the resulting config: build the app under several environment combinations (setup mode on/off, sqlite vs. postgres, SKIP_BACKGROUND_SERVICES, secret key present/absent/placeholder) and snapshot the full app.config plus the registered extension list. That snapshot is the baseline a reorder is checked against — the URL map alone is not enough here.
3. Only then extract the handler groups, which are ordinary motion.

Step 1 is the whole risk. Budget it as its own phase, and do not start it in the same session as a routine split.

Phase 4 — Long-running services

Highest risk: these are the alert path. The plan's original assumption was that each file is dominated by one very large class, needing mixins or extracted collaborators rather than free functions, with behaviour pinned by tests before anything moves. That assumption held for system.py (not a god-class, landed as 4a) and did not hold for eas.py (also not a god-class by the time it was split — see 4b). Check which shape a file actually is, the way the Phase 3 table note already says to, before assuming a characterization harness is the only way in. poller/cap_poller.py is the one file here confirmed to still be a genuine god-class.

4a. app_utils/system.py → app_utils/system/ ✅

The one Phase 4 file that is not a god-class, and the plan's own note — "mostly independent helpers — easier than it looks" — held up. 47 top-level definitions shared the file, the internal call graph is strictly acyclic, and grep -n "__file__" came back empty, so this is the 2a–2c technique with no new hazards.

common, dependencies, services, badges, network, device_tree,
disks, smart_fields, subsystems                              leaves
block_devices -> common
osinfo        -> common
temperature   -> common
rtc           -> common
hardware      -> common, block_devices, device_tree
smart         -> common, disks, smart_fields
snapshot      -> badges, dependencies, hardware, network, rtc,
                 services, smart, subsystems, temperature, osinfo

Verification. 48 top-level definitions, 48 ast.dump() matches, zero differences, plus the every-non-blank-line-placed-exactly-once assertion. ruff check --select F,E9 is clean on the package apart from one pre-existing F841. All four production consumers were imported to confirm the shim resolves (app_utils/__init__.py, app_core/system_health.py, webapp/admin/api/, scripts/diagnose_smart.sh). 43 system-health tests pass.

The mutable-global rule paid for itself again. DEVICE_TREE_CANDIDATES is a list that two tests replace wholesale. It is deliberately absent from the package __init__, so both patches raised AttributeError instead of silently patching a name nothing reads — the same loud-failure property that made the 3a retarget safe. They now patch app_utils.system.device_tree. The CPU test's Path and psutil patches moved to app_utils.system.hardware for the same reason.

One generator bug worth recording: a parameter name is not a use of an import. The script that derived each module's import block from the names its definitions reference treated the parameter of _escape_shields_io_text(text: str) as a use of from sqlalchemy import text, and gave badges.py (and device_tree.py, via a local) an import they do not need. ruff --select F caught both as F811/F401. Only snapshot.py actually issues SQL. Lint the generated package before trusting an inferred import block — name-level inference cannot see scope.

Still over the cap at the time — follow-up needed. snapshot.py (478) and smart.py (429) were each one function: build_system_health_snapshot was 406 lines and _collect_smart_health is 396. Module-level splitting cannot shrink them, so they need collaborator extraction with a characterization harness built first — the 2e / 3a-ii technique. Tracked as Phase 4a-ii. snapshot.py landed 2026-09-18 (below); smart.py is still open.

4a-ii. build_system_health_snapshot → app_utils/system/{cpu,memory,disk_usage,processes,loadavg,db_health,status}.py ✅

The easier of the two Phase 4a-ii files, as predicted: the function's try block was already a sequence of independent figures assembled into one dict — CPU, memory, disk, network, process table, load average and a database probe — followed by a status computation reading several of them. No single piece touched another's state, so the seam was cutting each block out to its own _collect_* function and reassembling the calls in snapshot.py.

snapshot.py: 480 → 158 lines, now pure orchestration. Every module in app_utils/system/ is within the 400-line guidance for the first time since this plan started.

Verified by characterization, not ast.dump() — dedenting an inline block into a function is restructuring, not motion, in the same sense as 2e. tests/test_system_health_snapshot_package.py (18 tests) was written and run green against the pre-refactor function first, mocking only the true I/O boundary (psutil, socket, os, the database session, the logger) plus the twelve already-extracted sibling collectors this phase does not touch. Re-run green after the split with no changes to the assertions.

The harness caught a real assertion bug in itself before the refactor even started. running_processes reads proc.info["status"] before the per-process try/except (NoSuchProcess, AccessDenied) — so a process that disappears mid-scan still counts toward "running." A first-draft test expected it excluded; running the harness against the untouched original function failed immediately and pointed at the wrong assumption, not a bug.

A mutation sweep (14 mutations, one per collector) caught 12 on the first pass and found two of the test's own isolation gaps. Both are the same shape as the audit trail lesson in 3b-ii: a test that only checks a correct outcome, not the specific mechanism that produced it, cannot catch a mutation to a different mechanism that happens to produce the same outcome.

1. The original critical-status test set CPU to 95% and forced a database failure in the same call, then asserted status == "critical" and any("CPU usage" in reason for reason in status_reasons). Both survive even with the CPU-critical threshold mutated to 190%, because the DB failure alone forces critical, and the warning-branch reason text also contains the substring "CPU usage". Split into test_status_critical_cpu_alone / test_status_critical_memory_alone (DB healthy, asserting status_reasons == ["CPU usage is 95.0%"] exactly) and test_status_critical_db_alone_overrides_healthy_cpu.
2. The disk permission-error test put the denied partition at a fake mount and the surviving one at / — so if except PermissionError is mutated to except KeyError, the exception escapes to the outer handler, which falls back to querying / itself, and the assertion (mountpoint == "/") passes anyway. Moved the surviving partition to /data and made the fake disk_usage raise AssertionError on any query for /, so the fallback path is unreachable in a correct implementation and loudly wrong in a broken one.

No __file__ hazard, checked rather than assumed — grep -n "__file__" across every touched file came back empty, and none of the collectors resolve a path relative to their own module depth.

4a-ii (cont). _collect_smart_health → app_utils/system/{smart_command,smart_query,smart_status,smart_attributes}.py ✅

The harder of the two Phase 4a-ii files, also as predicted. Unlike snapshot.py's independent figures, _collect_smart_health is one 425-line pipeline per device — locate smartctl once, then for each device: build a query command, run it, validate/parse its JSON output, infer a health status when smartctl's own verdict is absent, and populate ~35 result fields from the report. Each stage consumes what the last produced, so — like 3b-ii's alerts() split — the modules are pipeline stages, not independent collectors.

smart.py: 425 → 191 lines — _device_result_skeleton (the per-device default dict) and _collect_one_device (the per-device pipeline, calling the four new modules in sequence) plus the top-level _collect_smart_health loop. Every module in app_utils/system/ is now within the 400-line guidance — Phase 4a-ii is complete.

Existing coverage was narrower than it looked. tests/test_smart_health.py (8 tests) covers the exit-code health-inference fallback in real depth, but nothing else: smartctl discovery, every subprocess failure mode, output validation, or field-extraction wiring. tests/test_smart_health_package.py (24 tests) was written and run green against the pre-refactor function first to cover the rest — deliberately not re-testing what smart_fields.py's own extractors already have dedicated tests for elsewhere, only that _collect_smart_health wires their results into the right keys.

A mutation sweep (18 mutations) caught 16 on the first pass; the other two were structural, not assertion gaps, and both point at the same underlying fact: _detect_device_type() (disks.py) currently only ever returns "nvme" or "auto".

1. The bit0 ("Invalid command line arguments") exit-code branch had no test at all — bit1 and bit2 were covered, bit0 was an oversight. Added test_nonzero_exit_with_empty_output_invalid_command_line.
2. command.extend(["-n", "standby"]) only fires when device_type_flag in ("ata", "sat") — a code path _detect_device_type cannot currently produce, so no route through _collect_smart_health() can reach it end to end. Since _build_smartctl_command() is now its own importable unit (it wasn't, before this split), it gets a direct unit test instead: test_standby_flag_added_for_ata_and_sat_device_types / _omitted_for_other_device_types. First attempt asserted "-n" not in command, which is ambiguous — sudo -n (don't prompt) is also "-n" — and passed for the wrong reason; fixed to check for the literal "standby" argument instead.

The retargeting trap fired exactly as predicted, immediately. Both tests/test_smart_health.py and the new package test patched "app_utils.system.smart.subprocess.run" and "app_utils.system.smart.os.path.exists" — those attributes moved to smart_query.py and smart_command.py respectively, so every one of those 23 patch sites would have become a silent no-op (real subprocess.run / os.path.exists executing against the test host) had they not been retargeted before the split landed. Caught before merging, not after, by retargeting on the same pass as the extraction rather than treating it as a follow-up.

No __file__ hazard — grep -n "__file__" across all four new modules came back empty, matching snapshot.py.

Phase 4a-ii is complete. Every module in app_utils/system/ is now within the 400-line guidance.

4b. app_utils/eas.py → app_utils/eas/ ✅

The single largest file in the tree at the time of the split: 4,246 lines, grown from the 3,848 the plan was originally scoped against. The plan's own table called this "config loading · SAME header build+describe · TTS normalisation · audio generation · EASBroadcaster" and grouped it with the god-class files. Profiling it first showed that description was stale. 48 top-level definitions shared the file — mostly independent functions, the 2a/2b pure-motion shape — plus two large classes (EASAudioGenerator, 791 lines/5 methods; EASBroadcaster, 445 lines/5 methods) that are god-classes in their own right but are a small fraction of the file. This is the same lesson 4a already taught, generalized: profile a Phase 4 file before assuming it needs a characterization harness. system.py wasn't a god-class either; cap_poller.py, still unstarted, is confirmed to be one.

app_utils/eas.py is gone; app_utils/eas/__init__.py is the compatibility shim, re-exporting every one of the 37 distinct names anything in the tree (production or tests) ever imported from app_utils.eas — enumerated by an AST scan across app_core/, app_utils/, webapp/, scripts/, services/, tools/, poller/ and tests/ first, rather than trusted from memory or from the pre-existing __all__ (which, as every prior phase has found, is not the same thing as the real public surface).

Verification. 48/48 top-level functions/classes and 32/32 module-level constants ast.dump()-identical before and after. Every non-blank original line placed in exactly one new module, checked programmatically (the only "missing" lines were the __all__ block, deliberately relocated to __init__.py). Full test suite green: 3,362 passed, 282 skipped, 62 xfailed, 9 xpassed, 0 failures. Every production consumer (app_core.audio.*, app_core.eas_processing, app_core.gpio_input_listener, app_core.websocket_push, eas_monitoring_service, poller.cap_poller, scripts.manual_eas_event, scripts.resend_eas_broadcast, scripts.run_eas_broadcaster, services.gpio.alert_indicators, tools.generate_sample_audio, webapp.admin, webapp.eas, webapp.routes.broadcast_control, webapp.routes_monitoring, webapp.routes_rwt_schedule) imported directly to confirm the shim resolves. grep -n "__file__" across all 14 new modules is empty.

A confirmed internal-cross-call hazard — found by scoping, not by accident. EASBroadcaster.handle_alert() calls build_same_header() and clear_broadcast_active() as same-module bare names today, resolved through the file's own global namespace. tests/test_gpio_centralized_keying.py monkeypatches both at the app_utils.eas module level and then calls broadcaster.handle_alert() directly, expecting the patch to intercept that internal call — this is exactly the shape the checklist's "map the closure before splitting a module that is one big function" item warns about, except here it is two module-level names a class method calls bare, not a closure. An AST scan across all 36 test files that reference app_utils.eas in any form (import ... as, from ... import, or bare import app_utils.eas — not just the subset a naive grep for the literal substring "app_utils.eas." would catch) found this as the only two sites where the patched name is also called from inside a class that is itself moving. Retargeted to eas.broadcaster.build_same_header / eas.broadcaster.clear_broadcast_active, and verified load-bearing by temporarily reverting the retarget: the test fails with KeyError: 'present', because the recording wrapper that patch installs never gets called — the real, unpatched function runs instead, silently.

subprocess and time are re-exported from the shim as modules, not just the functions that use them. tests/test_gpio_dump_broadcast.py patches eas_module.subprocess.Popen and tests/test_airchain_fringe_cases.py patches 'app_utils.eas.time.sleep' via unittest.mock.patch's string-path resolution — both require app_utils.eas.subprocess / app_utils.eas.time to exist as attributes, which a package __init__.py doesn't get for free the way a single-file module's own import subprocess did. Both are shared singleton stdlib modules (the same reasoning as psutil in 4a-ii and subprocess in 3e-ii): patching an attribute on the module object affects every importer of it, so import subprocess / import time in the shim is enough — no further retargeting needed for these two, unlike the two same-module bare-name calls above.

Two rounds of missing-import bugs, both caught by actually importing the result rather than trusting a script. The generator script derived each module's free-variable set with a hand-rolled AST scope walker (symtable's own get_identifiers() turned out to conflate "referenced in this scope" with "bound in this scope" for module-level names, so it silently swallowed genuine free references — P_DIGIT_LABELS in same_header_constants.py came back with zero free names when it plainly needed one). The walker also didn't attribute type-annotation expressions on bare module-level AnnAssign constants (PRIMARY_ORIGINATORS: Tuple[str, ...] = (...)) to the same free-variable pass as everything else, so several modules' derived typing imports were incomplete (missing Dict, Optional, Sequence, or Tuple depending on the module). Neither was caught by py_compile — a missing name inside a function body only raises at call time, and a missing name in a bare module-level annotation only raises at import time, which py_compile doesn't execute. The fix in both cases was the same: actually import app_utils.eas and iterate on the NameErrors it raised, rather than trusting the derivation. This is the fourth appearance of the symtable/free-variable lesson in this plan (4a, 3c, 3d, now 4b) — it remains the single most repeated mistake in this effort.

chime.py split out from tone_generation.py after the fact, once line counts included the license header and import block. The generator script's own line-count estimates were computed on the bare function bodies before headers/imports were added, which put tone_generation.py at an estimated 395 lines — under the cap on paper, 446 once assembled. Rather than accept the overage, _generate_chime (231 lines, self-contained except for the DTMF frequency table it already owned) moved to its own module, bringing both tone_generation.py (209) and the new chime.py (263) comfortably under. Lesson for the next phase: budget for header + import overhead (~35-40 lines per module) when estimating module sizes up front, not just the bare body content.

generator.py (848) and broadcaster.py (489) are known exceptions. EASAudioGenerator and EASBroadcaster are each one class; module-level splitting cannot shrink a single class, the same conclusion 2a reached for RBDSWorker/RBDSDecoder and 3e reached for obtain_certificate_execute before its own 3e-ii follow-up. Bringing them under the cap needs collaborators extracted from the class bodies — a behavioural change needing a characterization harness built first, the 2e technique. Not started; tracked as a follow-up. config.py (408) is the same shape at a smaller scale: one 369-line function (load_eas_config) dominates a module whose only other content is a 5-line helper and a 3-line constant list.

4c. poller/cap_poller.py — stateless methods extracted, god-class remains ⚠️ partial

CAPPoller is confirmed to be a genuine god-class, exactly as the plan originally assumed for Phase 4 (unlike system.py and eas.py, which turned out not to be): 4,933 lines total, CAPPoller itself ~3,978 of them, 59 methods. Profiling by self usage — the same technique 2d used on GPSManager — splits it cleanly:

The nine that moved, to poller/cap_alert_parsing.py (187 lines): _select_cap_info, _extract_cap_event_codes, _extract_cap_parameters, _summarise_geometry, _apply_cancellation_status, _validate_ugc_code, _normalize_same_code (already @staticmethod), _coords_equal, _safe_json_copy. cap_poller.py: 4933 → 4800 lines — still far over the guidance, because module-level splitting only ever had 143 of the 4,933 lines to work with here. The other 3,711 lines are the real Phase 4c work, not started.

Verified the same way 2d verified GPSManager's stateless methods: all 9 are ast.dump()-identical to their originals once self is stripped from the signature and docstring indentation is normalized (one dedent level, 8→4 spaces) — the exact same two normalizations 2d needed, for the exact same reason (a class method's docstring and a module function's docstring are conventionally indented one level apart, and that's a real textual difference ast.dump() correctly reports, not a false mismatch).

19 internal call sites rewritten from self._method(...) to _method(...) across cap_poller.py — found by grep -c "self\.$method(" per method rather than assumed, matching the "a truncated search cannot prove a negative" lesson from 2d/2e.

One test file called a moved method through a live instance and had to be retargeted. tests/test_ipaws_event_code_extraction.py built a CAPPoller via object.__new__(CAPPoller) specifically to reach poller._extract_cap_event_codes(...) — the only one of the 36 usages of these 9 names anywhere in the tree (production or tests, AST-scanned) that called through an instance rather than importing load_eas_config-style or not touching them at all. Retargeted to import _extract_cap_event_codes directly from poller.cap_alert_parsing and call it bare, dropping the now- pointless _make_test_poller() instance entirely for that test class.

The ET/CAPAlert type hints needed real imports, not the from __future__ import annotations shortcut. Three of the nine methods type-hint parameters as ET.Element or CAPAlert without using either as a runtime value (pure duck-typing via .findall()/getattr() in the bodies). The first draft relied on from __future__ import annotations to defer hint evaluation and skip importing either — py_compile and even a real import poller.cap_alert_parsing both stayed silent about this, since neither actually evaluates annotation expressions. ruff check (not available in this sandbox by default — installed into a scratch venv to get a real lint pass rather than trusting py_compile alone) caught both as F821 immediately. Fixed by importing ET the same way cap_poller.py itself resolves it (get_element_tree_module() from app_utils.optimized_parsing) and CAPAlert directly from app_core.models — both harmless from poller.cap_alert_parsing, which already sits downstream of both in the dependency graph.

A CodeQL false-positive class worth naming for the next phase. The PR was flagged with 3 "new" alerts: a polynomial-regex pattern in tts_normalize.py, a log-injection pattern in audio_conversion.py (both landed in 4b, not 4c), and a stack-trace-exposure pattern in webapp/admin/pending_alerts.py — a file this session never touched at all. All three were confirmed pre-existing: the first two are ast.dump()-identical to code already on main before the split, and the third has a byte-identical diff (none) against main. GitHub's PR-scoped CodeQL analysis identifies an alert partly by file path, so moving a file makes its pre-existing findings reappear as "new," and a data-flow source passing through a moved file (here, load_eas_config) can do the same to an untouched file downstream of it. main already carries ~100 open alerts of these same rule categories elsewhere in the tree (including in this plan's own already-merged certbot/obtain_methods.py split), and main has no branch protection requiring CodeQL to pass — confirmed via gh api repos/.../branches/main/protection (404) and gh pr view --json mergeable,mergeStateStatus (MERGEABLE/UNSTABLE, not blocked) rather than assumed. Fixing a security-flagged regex or log call is a behaviour change and does not belong in a pure-motion commit; documented here rather than silently fixed or silently ignored, matching the plan's standing rule for pre-existing issues found mid-phase.

What was left after 4c, before 4c-ii. The remaining 50 stateful methods, 3,711 lines, were the actual Phase 4c work — dominated by poll_and_process (464 lines, 153 self references), __init__ (285 lines), _insert_new_alert (225), fetch_cap_alerts (208), process_intersections (170), and 45 more. These need the 2e technique: a characterization harness built before any restructuring, the same way _handle_sentence was pinned before its nmea.py extraction. Highest-risk work remaining in this entire plan, now that system.py and eas.py are done and turned out not to need it.

4c-ii. poller/cap_poller.py — CAP-geometry collaborator, 2e technique ✅

The first slice of the 50 stateful methods above to actually land, using the 2e technique this section called for: characterization tests written and run green against the pre-extraction bound methods first, extraction second, same tests retargeted at the extracted free functions third.

Profiling by self-attribute usage (not just reference count) found 12 methods touching only self.logger — set once in __init__, never reassigned — or each other, never self.db_session or the poller's zone/SAME-code configuration: _parse_ipaws_xml_feed, _convert_cap_alert, _extract_cap_resources, _extract_area_details, _parse_cap_polygon, _parse_cap_circle, _approximate_circle_polygon, _message_type_priority, _alert_sort_key, _should_replace_alert, parse_cap_alert, _count_vertices — a CAP-XML-to-GeoJSON-feature parsing concern, cleanly separable from the DB/config-coupled methods (fetch_cap_alerts, get_alert_relevance_details, _has_geometry_changed, ...) that stay behind. Moved to poller/cap_geometry.py along with the module-level _serialize_alert_for_sig helper (_convert_cap_alert's only caller) and the MESSAGE_TYPE_PRIORITIES class attribute (actually a shared constant, not instance state).

self.logger became an explicit logger parameter, not a new logging.getLogger(__name__). Phase 3e's lesson applies here unchanged: a fresh per-module logger would silently rename every record these 12 functions emit from poller.cap_poller to poller.cap_geometry, breaking any journald filter or log search keyed on the old name. cap_poller.py's 4 remaining call sites (_parse_feed_payload, fetch_cap_alerts, _set_alert_geometry, poll_and_process) now pass self.logger explicitly to the module functions instead of calling self.method(...).

Not pure motion, so no bare ast.dump() equality this time — every signature changed (dropped self, added logger), so verification had to be behavioral: 59 characterization tests (tests/test_cap_geometry.py) written against the pre-extraction bound methods, confirmed 2 of them load-bearing via targeted mutation spot-checks (a _should_replace_alert CANCEL-priority flip and a _parse_cap_polygon minimum-vertex-count change, both caught immediately), then the same 59 assertions retargeted at the extracted free functions and re-run green.

Three existing test files needed retargeting off the removed bound methods — test_ipaws_event_code_extraction.py, test_cap_poller_batching.py, test_cap_poller_per_item_isolation.py. The last one needed a real fix, not just an import change: test_parse_ipaws_xml_feed_one_malformed_alert_does_not_drop_the_others patched fake_self._convert_cap_alert (an instance attribute) expecting to intercept _parse_ipaws_xml_feed's internal call — but that call is now a same-module bare name inside cap_geometry.py, so an instance-attribute patch has nothing left to intercept. Retargeted to monkeypatch.setattr(cap_geometry, '_convert_cap_alert', ...), the same same-module-bare-call hazard this plan has hit repeatedly (eas.py's build_same_header, eas_storage.py's collect_compliance_log_entries).

cap_poller.py: 4933 → 4800 (4c) → 4244 lines (4c-ii). CAPPoller itself is ~3,183 lines across the remaining 38 methods — still the actual Phase 4c work, not started, and still the highest-risk item left in this plan (poll_and_process alone is 464 lines / 153 self references).

4d. app_core/eas_storage.py — pure motion, package of 14 modules ✅

Profiled first rather than assumed, per the "check shape before planning" note this plan added after 4c: 2,825 lines, 57 top-level functions, zero classes — the same pure-motion shape as system.py (4a) and eas.py (4b), not a god-class. Grouped into 14 modules by topic — audio-decode logging, disk file caching/purging, schema migrations, one-time backfills, delivery records/trends, the FCC compliance log (parsing + collection + CSV/PDF export), weekly/monthly summary reports (common window helpers + the received/initiated builders + the summary builders + CSV/PDF export), and precedence — plus a __init__.py shim re-exporting all 40 public names (and format_local_datetime/utc_now, two pass-through names some callers import from this module rather than app_utils.time directly).

All 69 top-level definitions verified ast.dump()-identical with zero normalization needed — a first for this plan. Every prior split needed at least the self-stripping/docstring-dedent pair (2d, 4a-ii, 4c) because code moved out of a class or a closure; nothing here did, so the AST comparison caught real textual differences with no false positives to filter out.

Built the call graph before laying out modules, not after. A free- variable scan across all 57 functions found collect_compliance_dashboard_data calls collect_compliance_log_entries as a same-module bare name — the exact eas.py build_same_header/clear_broadcast_active hazard shape. Since tests/test_public_logs_data.py monkeypatches eas_storage.collect_compliance_log_entries expecting to intercept calls made through collect_compliance_dashboard_data, the two had to land in the same module (compliance_log.py) rather than being split further by sub-topic — confirmed by grepping the whole tree for the call graph first, not discovered by a test failure after the fact.

The free-variable scanner needed to cover constants, not just functions. The first pass only walked FunctionDef/AsyncFunctionDef bodies for import dependencies, so _CAP_SOURCE_ORIGINATORS — a module-level dict literal referencing ALERT_SOURCE_NOAA and friends — silently lost its import and failed at collection time (NameError, not a lint-catchable issue, since nothing evaluates a dict literal's values at parse time). Re-ran the scan over Assign/AnnAssign nodes too and caught two more constants missing an import the same way (Tuple in compliance_export.py, Dict in reports_common.py) before they became runtime failures.

Comments attached to a constant, not a function, are invisible to ast. The mechanical per-node extraction (slicing exact source text by node.lineno/end_lineno) silently dropped four standalone comment blocks that preceded a constant rather than sitting inside a docstring — including a six-line FCC Part-11 citation above _CAP_SOURCE_ORIGINATORS and a four-line rationale above REPORT_MAX_ROWS. Caught by the plan's own "every non-blank line lands in exactly one module" check (a line-set diff against the original file), not by any AST-based verification, since a Comment node doesn't exist in Python's AST at all.

One confirmed-dead import dropped: ORIGINATOR_DESCRIPTIONS from app_utils.eas, imported in the original file but referenced nowhere in it. The free-variable scan simply never listed it as a dependency of anything; confirmed dead (not a false negative) by grepping the whole original file for the name before dropping it.

compliance_log.py lands at 405 lines — negligible, deliberate overage to keep the internal-call pair above together. Full suite green: 3367 passed, identical to the pre-split count.

Phase 5 — Frontend

The pattern for templates is the one used on the Audio Archives page in 2.133.1: inline <script> moves to static/js/pages/<page>.js, repeated markup moves to templates/components/, and the page keeps only its structure.

Progress log

Next up

Phase 3b-ii is complete. All three webapp/public/ modules that Phase 3b left over the cap — logs_data.py, stats.py and alerts.py — have landed, and every module in the package is now within the guidance.

Phase 3 is complete except for app.py. api.py landed as 3d, certbot.py as 3e, maintenance.py as 3f and alert_verification.py as 3h. Every web-layer file the plan listed is now a package of modules within the guidance, bar two known exceptions (routes_obtain_execute.py at 449, tracked as 3e-ii) and app.py itself.

app.py (1869) is assessed in 3g above and deliberately deferred. It is not a package split at all: it needs the module-level singleton turned into a real factory first, as its own commit, because until then statement order is the behaviour and none of this plan's verification catches a reorder. Budget it as its own phase.

Check which shape a file is before planning it. 3d/3e/3f were ordinary top-level definitions and went quickly; 3b, 3c and 3h were single enormous register() functions and needed the closure mapped first.

Phase 3e-ii is complete, landed 2026-09-18: routes_obtain_execute.py (454 → 99 + obtain_validation.py + obtain_methods.py). No known exceptions remain in webapp/admin/certbot/.

Phase 4a-ii is complete, both landed 2026-09-18: snapshot.py (480 → 158

• 7 new modules) and smart.py (425 → 191 + 4 new modules). Every module in app_utils/system/ is now within the 400-line guidance.

Phase 3 and Phase 4a-ii are now both fully complete (bar app.py, 3g, deliberately deferred). app_utils/eas.py landed as 4b, 2026-09-18.

poller/cap_poller.py (4933, was 3996 when the plan was written) is confirmed a genuine god-class, unlike system.py (4a) and eas.py (4b). Its 9 stateless methods (143 lines) landed as 4c, 2026-09-18 — the same 2d technique used on GPSManager. A further 12 methods (the CAP-geometry collaborator: only touched self.logger and each other, never self.db_session or the poller's zone/SAME-code config) landed as 4c-ii, 2026-09-18, using the 2e technique in earnest for the first time — characterization tests against the pre-extraction bound methods first, extraction second. 38 methods (3,183 lines) remain, still not started, and this is still the highest-risk item left in the whole plan: poll_and_process (464 lines, 153 self references) alone is bigger than most files this plan has split in their entirety. Needs the same 2e technique 4c-ii proved out, just at a much larger scale and against methods that are genuinely DB/config-coupled rather than "only needs a logger." Budget it as its own dedicated multi-session effort, not a routine split. app_core/eas_storage.py (2825, 57 top-level functions) was profiled and confirmed pure-motion shape — like system.py and eas.py, not a god-class — and split into a 14-module package, 2026-09-18. main()-shaped files remain unstarted: sdr_hardware_service.py (2924 lines) is dominated by one 891-line process_commands function, and eas_monitoring_service.py (2546 lines) by one 1079-line main() — both need the 2e characterization-harness technique cap_poller's remaining 50 methods need, not the pure-motion technique that worked for eas_storage.py. Check shape before planning either one; do not assume either is safe to split the same way.

Pre-split checklist

Run all of this before touching a file. Each item exists because skipping it cost a debugging cycle in an earlier phase.

• grep -n "__file__". Every __file__-relative path shifts meaning when a module moves one directory deeper. AST-equality cannot catch it. Assert the resolved values against the pre-split module afterwards. (Phase 2b: silently dropped the brand logo from every share image.)
• AST-scan the tests for monkeypatch.setattr targets, not grep — grep misses multi-line calls and bare attribute reads. Retarget each to the module that calls the name. (Phase 2c: grep found 24 of 31 sites.)
• Check for tests loading the module by file path (spec_from_file_location). A package needs submodule_search_locations on the spec. (Phase 2b.)
• Lay the modules out by the call graph, not by topic name. A chain that crosses three layers (_process_temp_audio_file → _detect_comprehensive_eas_segments → _build_composite_audio_segment) looks like one topic and is three. Putting its two ends in one module sandwiches the middle into an import cycle. Walk the emitted imports and fail on a cycle by name — Python's own error names the symptom, not the layering. (Phase 3h.)
• Unwrap __wrapped__ before inspecting a handler's closure. app.view_functions[name] is the outermost decorator; its only free variable is the function it wraps. (Phase 3h.)
• Never assert co_freevars == () on a module-level function — it is always empty, so the test cannot fail. Assert on the source that the function does not reference names its old enclosing scope provided. (Phase 3h.)
• Read the original of any definition you retype rather than move. Exactly one function usually does not travel as a source slice — the register_* that goes into the package __init__. Writing it from memory gave maintenance the certbot blueprint convention (url_prefix='/admin' on a blueprint whose decorators already carry /admin), which would have moved all 17 routes to /admin/admin/…. (Phase 3f.)
• Check the blueprint's prefix convention against its own decorators. Sibling packages here use opposite ones: certbot_bp takes url_prefix='/admin' and its routes omit it; maintenance_bp takes no prefix and its routes include it. Neither is wrong; making one match the other silently doubles or drops the prefix. (Phase 3f.)
• Grep the tests for the module's path, not just its import name — open('webapp/admin/api.py'), py_compile.compile(...), Path(...). Tests that assert against source text break on the move, and the failure mode is asymmetric: pointed at a deleted file they fail loudly, but pointed at a shim that no longer holds the code they pass vacuously. Retarget them at the package directory. (Phase 3d: five such tests.)
• Run any new test file inside the full suite, not just on its own. pytest imports every test module during collection, so a module you never look at can change what yours sees. Reaching a submodule as parent.child after import parent.child is the usual casualty — use importlib.import_module('parent.child'). (Phase 3d: 11 errors that no targeted run reproduced.)
• Prefer replacing a source-text assertion with a call to the real function. A test that mirrors the logic locally and then greps the original to confirm they still match cannot catch a change to the original — that is what the grep was standing in for. If the function only needs a stub object and one monkeypatched lookup, test it directly and mutation-check it. (Phase 3d.)
• Do not name a module types.py — it shadows the stdlib. (Phase 2c.)
• Assert every non-blank line of the original lands in exactly one module, so nothing is silently dropped by an off-by-one slice.
• Compare ast.dump() of every top-level definition old vs new.
• Verify any test retarget is load-bearing by confirming the tests fail without it. A retarget that changes nothing means the test was passing vacuously and you have learned something either way.
• Import every production consumer to confirm the shim resolves.
• Check what __name__ is passed to. A Blueprint(name, __name__) or logging.getLogger(__name__) means something different one directory deeper. Pass __package__ where the pre-split value is what matters. (Phase 3a.) A module-level logger counts: one getLogger(__name__) serving the whole file becomes one logger per module, renaming every record from webapp.admin.certbot to webapp.admin.certbot.<submodule>. No test sees it; a journald grep or log filter keyed on the old name does. Put the logger in its own module and import it. (Phase 3e.)
• Decide by-value vs. fan-out for the logger by asking whether the registration hook rebinds it. If register_* reassigns the module's logger, a by-value from .log import logger is silently wrong and you need 3a's fan-out list. If it only writes through the logger it is handed — as register_certbot_routes does — the fan-out is unnecessary machinery and a by-value import is correct. (Phase 3a vs. 3e.)
• Never let a module import a mutable global from a sibling. from .x import _some_global snapshots the value at import time. Add an accessor function instead, and AST-scan the package to prove none crept in. (Phase 3a: would have silently disabled a cleanup path.)
• Do not re-export mutable globals from the shim. Tests reset them with monkeypatch.setattr, which raises on a missing attribute but silently no-ops on a re-exported one. The loud failure is what tells you where the patch has to point. (Phase 3a.) Phase 3h measured the cost: with _progress_dir and friends re-exported and the patch aimed at the package, all six async tests still passed — writing to the real temp directory instead of tmp_path. If you are unsure whether a re-export matters, add it and run the tests: passing is the bad outcome.
• Fan out any global a registration hook rebinds — logger is the usual one. One module meant one global; a package means one per module. (Phase 3a.)
• Assert a mutation actually applied before concluding it survived. A search string with the wrong indentation replaces nothing, and the "surviving mutant" is the untouched original. (Phase 3a-ii.)
• Re-check the shim after any automated import cleanup. ruff --fix would strip a re-export shim bare; it only spares __init__.py because F401 exempts it by default. A shim that is not an __init__.py — like webapp/routes_public.py — has no such exemption and survives only because its __all__ marks the re-export as used. Verify, don't assume. (Phase 3a-ii, 3b.)
• Map the closure before splitting a module that is one big function. Nested handlers can be moved verbatim into per-topic register() functions only if you know exactly which enclosing names each one captures. Walk the AST for Name nodes resolving to the outer scope rather than eyeballing it. (Phase 3b: all 21 handlers captured just app and route_logger, which is what made the split pure motion.)
• Derive per-module import blocks from the original's own import statements; do not hand-write them. Narrowing each original statement to the names a module uses keeps the grouping and gets the answer right. Hand-writing them cost 127 ruff errors in one go. (Phase 3d.)
• Compute the free-variable set with symtable, not ast.Name counting — name counting cannot see scope. A parameter called text reads as a use of from sqlalchemy import text, and so does a local variable called desc. symtable answers directly: at module scope keep names referenced but never bound; inside a function keep the symbols is_global() reports. This bug has now appeared three times (4a, 3c, 3d) in three different disguises — it is the single most repeated mistake in this plan.
• Lint the generated package anyway. ruff check --select F,E9 is the gate, whatever produced the imports: F401 catches an import nothing needs, F821 an import a module needed and did not get. (Phase 4a.)
• Rewrite names with the AST, never a regex — a regex has no scope. It will rewrite the alias inside from x import name (a syntax error, if you are lucky enough for the linter to catch it) and, worse, call sites inside functions that shadow the name with their own local import. Phase 3c had three functions importing get_redis_client from a different module locally; rewriting those would have been a silent behaviour change, not a move.
• ast.walk() does not stop at scope boundaries. Computing "names bound in this function" with ast.walk descends into nested functions, so an outer function inherits every inner one's local imports. In Phase 3c that made register() appear to shadow names it never touches, and because shadowing inherits downward it silently skipped the rewrite for every nested handler — leaving a plausible-looking count rather than an error. Cut nested FunctionDef/Lambda/ClassDef off explicitly.
• Split generated files on a sentinel you control, not on punctuation. A partition(")\n") intended to find the end of an import block matched the (KR8MER) in the copyright header instead, and the "body" rewrite then corrupted every module's imports. (Phase 3c.)
• Never run a file-rewriting harness in the background against a tree you are still editing. A mutation runner backs up, rewrites and restores files in place; started in the background while new modules were being written into the same package, it picked the half-written files up as targets, leaving one committed file altered and one new file carrying a mutant's arithmetic. Run it in the foreground, or point it at a worktree copy. (Phase 3b-ii.)
• Retarget every mutation after the split, and count "not applied" as a failure. A refactor moves and rewords the lines the mutations matched — after the alerts() split, all 26 stopped applying. A mutation that matched nothing proves nothing, so the run is only meaningful once every one has been pointed at its new home. (Phase 3b-ii, and 4 of 17 in the stats() split.)
• Purge __pycache__ when a harness restores a mutated source. A mutation that swaps two same-length branches leaves the byte count unchanged, and a restore that preserves the backup's mtime lets CPython reuse the mutant's .pyc. The tree then keeps running code you believe you reverted, and the symptom looks exactly like one flaky test. (Phase 3b-ii.)
• Make the harness print what it bound to, and read it. A comparison script that silently falls back to the pre-split shape reports a perfect match against itself. Naming the patched module is what catches a cd that leaked between runs, an import that resolved to the wrong tree, or a patch target that no longer exists. (Phase 3a-ii, 3b-ii.)
• Seed fixtures deterministically, including what the ORM writes for you. Insert-triggered listeners (audit events, onupdate stamps) carry real wall-clock values, and they will show up as diffs that have nothing to do with the refactor. Re-seed those tables last rather than scrubbing the comparison until it goes quiet. (Phase 3b-ii.)
• Treat an equal-length, unequal-hash diff as a scrubbing bug first. It is the signature of an unscrubbed fixed-width random value, not a content change. Diff the raw bodies before concluding a regression. (Phase 3b: the per-session CSRF token, emitted in three shapes by base.html, of which the harness knew only one.)