# Four small fixes, scoped down from the refactoring survey Status: planned -- decision-complete; execute in order, each item lands on its own Superseded scope. This file used to hold a four-item refactoring survey of `src/`. That survey was reviewed against the code (`review-code-analysis-refactoring-opportunities.md`) and two of its four recommendations rested on things that are not in the codebase -- a `pinch()` function that does not exist, and six `config.py` classes that do not exist. The survey is kept in git history; do not execute it. What replaces it is the work that survived the review, plus three defects found while doing it. Nothing here is a refactor for its own sake: items 1-3 are live defects with a reproduction, and item 4 is the one structural change the review endorsed. **Intended executor: opencode (Atlas), directly.** These are small and independent; no Prometheus plan artifact is needed beyond this file. Land each item as its own commit so any one can be reverted alone. Every figure below was measured on the live database and the live service at `1e2e806` on 2026-09-12. Re-check before trusting any of them. --- ## Item 1 -- the TUI renders the literal string "None" in three cells **Priority: high** (visible defect, one-line fix, already has a precedent in the same function) `src/tui.py:763-775` builds each decision row by wrapping every value in `str()`. Three of those columns are nullable, so a NULL renders as the literal `None`: | cell | source column | NULL rows today | |---|---|---| | category | `task_category` | 4 | | tier | `task_tier` | 4 | | ctx | `required_context_tokens` | 4 | The rows are `kind='passthrough'`, which is why this shows up now: passthrough decisions do not carry a classification. The fix is already written two lines below, for the column someone hit first: ```python # `or ""` so an absent profile is blank, never the literal "None". str(r.get("profile") or ""), ``` Apply the same treatment to `category`, `tier` and `required_context_tokens`. Do **not** touch `selected`: `tui_model.py:218` already maps it to `"none"` via `or "none"`, deliberately, and that is a different string from the Python repr. Recorded previously in `.omo/notepads/tui-overhaul/issues.md` -- close that note out as part of this item. **Verify**: add a row to the existing TUI table test asserting that a decision with NULL `task_category` / `task_tier` / `required_context_tokens` renders empty cells and that the string `"None"` appears nowhere in the rendered row. --- ## Item 2 -- harden the demand-ceiling comparison, and correct CLAUDE.md **Priority: high** (the documented safety claim is now false) `CLAUDE.md` says of the demand-ceiling `TypeError`: > Latent only: zero such rows exist today, checked on the live DB. That is no longer true. There are **4** rows with NULL `required_context_tokens` as of 2026-09-12. **It has not fired, and the plan must not claim it has.** `metrics.py:1433-1441` filters `kind = 'chat' AND task_tier = ?`, and all four NULL rows are `kind='passthrough'` with a NULL `task_tier`, so none reach `_percentile`. `/metrics` returns 200. Two filters are the only thing standing between the current data and the crash. Two changes: 1. `src/metrics.py` -- make `_percentile` (line 46) and the three `required_context_tokens` collection sites NULL-safe. Filter NULLs out in SQL (`AND required_context_tokens IS NOT NULL`) rather than in Python: the `MAX(...)` sites at lines 1374, 1382, 1406, 1644, 1652, 1670 already ignore NULLs the way SQL aggregates do, so filtering in the one row-returning query at line 1436 makes every site consistent. A NULL is "this request did not state a context requirement", which is not a zero and must not be averaged in as one. 2. `CLAUDE.md` -- replace the "zero such rows exist today" sentence with what is actually true: NULL rows exist, they are `kind='passthrough'`, and the `kind`/`task_tier` filters are what keep them out of the comparison. **Verify**: a test that inserts a `kind='chat'` decision with NULL `required_context_tokens` at a tier that has a ceiling, then calls the warning function and asserts it returns without raising. That test must fail before the fix. --- ## Item 3 -- cap the config backup retention **Priority: medium** (unbounded growth in the directory holding the irreplaceable file) `config/` holds **45** `config.local.yaml.bak.` files. Two write paths create them and neither prunes: - `src/admin.py:467-470` -- `config.yaml.bak.` (0 on disk today) - `src/admin.py:587-591` -- `config.local.yaml.bak.` (45 on disk today) `config.local.yaml` is gitignored and holds the live classifier settings, so this is the one directory where backups genuinely matter -- which is the argument for making them legible, not for keeping every one forever. Add a retention cap after each successful write: keep the newest N (N = 10, as a named module constant, not a literal), delete older ones, matching only `.bak.` so nothing else in `config/` can be caught. Deleting a backup must never fail the write -- wrap the prune so an error is logged and swallowed; the write has already succeeded by that point and reporting it as failed would be worse than the clutter. **Also worth fixing while in there**: the filename is `int(time.time())`, so two writes in the same second silently overwrite each other's backup. The existing files show consecutive-second writes (`...877`, `...878`), so this is reachable. Either widen to a monotonic counter or accept it and say so in a comment -- decide and record which. **Verify**: a test that performs 15 overlay writes and asserts exactly 10 backups remain, that they are the 10 newest, and that no non-matching file in the directory was touched. --- ## Item 4 -- move the loader out of config.py (OPTIONAL, do last) **Priority: low, and more expensive than the review first estimated.** The review proposed lifting `_merge_overlay`, `load_config` and `summary_lines` (`src/config.py:1489-1601`, 112 lines) into `src/config_loader.py`, leaving `config.py` as schema only. The reasoning stands: the schema is declarative and rarely read top to bottom, while the loader has real control flow (overlay merging, `ROUTER_IGNORE_LOCAL_CONFIG`, backup rotation) and is read when something has gone wrong. **The cost was under-stated and this is the correction.** `load_config` / `summary_lines` are imported by **49 files** (9 in `src/`, 39 in `tests/`, plus `baseline_report.py`). The obvious mitigation -- re-exporting from `config.py` -- does not work: `config_loader` must import `RouterConfig` from `config`, so a re-export in the other direction is a module-level import cycle. So this item is genuinely "move 112 lines and update 49 import statements". The edit is mechanical and the test suite covers it completely, but it is churn bought for readability alone. **Do this only if items 1-3 land clean and there is appetite.** If it is skipped, say so and close the item rather than leaving it open -- an indefinitely-deferred refactor in a plans file is noise. --- ## Explicitly out of scope Carried over from the review, so nobody re-derives them: - **Splitting `config.py` by config section** (the survey's nine-file tree). Of 72 validators, eleven hang off `RouterConfig` and cross-check sections against each other; those 380 lines stay in one file under any split. The proposal moved the easy 1108 lines and left the hard third untouched. - **`Source` as a plain Enum.** The strings appear at 14 comparison sites in `src/` and 32 in `tests/`, against 3 annotation sites. A plain `Enum` makes all of them silently `False`. If ever done, it must be `class Source(str, Enum)`. - **Extracting `_summarize_prose()` from the pinch stage.** There is no prose summarization; the stage trims tool results only, on purpose. `prune_context` at 216 lines (`context_prune.py:240-455`) is a real extract candidate, but it is the money path and has a documented `protected_max_chars` gotcha in `docs/pinch.md`. Not now. --- ## Definition of done - Items 1-3 each landed as their own commit, each with the test described. - `CLAUDE.md`'s demand-ceiling paragraph reflects the live database. - `.omo/notepads/tui-overhaul/issues.md` note closed. - Item 4 either done or explicitly closed as skipped. - Full suite green (1918 at the time of writing) on Python 3.10 and 3.14. - No change to `config/config.local.yaml` -- `md5sum` it before and after.