plans/ held 58 documents and exactly one said whether it was open. The rest
mixed finished work, reviews of shipped work, parked specs and genuinely
pending ones, with nothing distinguishing them, so "how many plans are in
the queue" had no answer short of reading all 58.
Now `grep -H '^Status:' plans/*.md` is the answer:
50 done 3 in progress 2 planned 2 reference 1 parked
Statuses were derived rather than guessed: CLAUDE.md's own built list and
"What's NOT built yet" section, plus checking the subject exists in the
code. A review of work that shipped counts as done -- it records what was
found, it is not a request for anything. `reference` separates the two docs
that are conventions rather than work items (admin-design-standards,
admin-work-framework), which otherwise read as permanently-open plans.
The vocabulary is deliberately five words. A larger one invites "mostly
done" and "blocked-ish", which is how the directory became unreadable.
test_plans_declare_status.py keeps it from rotting: a new plan without a
marker fails, as does an unknown status, one buried below the eighth line,
or an open status with no reason -- "planned" alone is the state that rots,
since nobody can tell later whether it waits on a decision, a dependency,
or just nobody's turn.
Also updates the sweep plan with what landed and what did not, including
that #9 was not a defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U
32 KiB
root-reorg-src-config-plans — plan review
Status: done -- src/ and config/ layout
Target: plans/.omo/plans/root-reorg-src-config-plans.md
Round 1 → v1 (315 lines). Round 2 → v2 (430). Round 3 → v3 (434). Round 4 → v4 (450).
Round 4 — v4 (post dual-review)
Verdict: one line to fix, then go. All three Round-3 findings are applied, and more carefully than asked — V3-1 is guarded in four separate places, and V3-2's "don't over-apply" caveat was carried into an explicit must NOT do. The four dual-review edits are all sound. One of them over-corrected.
V4-1. The concurrency test's ROOT / source path does need to change
Plan line 279:
tests/test_admin_config.py:142-186concurrency test stays unchanged — it passes explicitconfig_yamlpaths directly to_persist_config_value, never usesbase_dir; its ownconfig.yaml.bak.*glob at 182 is correct as-is.
That is right about the destination and the glob, and wrong about one line in between. The block contains:
145: config_yaml = tmp_path / "config.yaml" # destination — STAYS
146: shutil.copyfile(ROOT / "config.yaml", config_yaml) # SOURCE — must move
...
182: backups = list(tmp_path.glob("config.yaml.bak.*")) # glob — STAYS
Line 146's ROOT / "config.yaml" is the repo's real config file, which moves
to config/config.yaml in this very wave. Left alone, the test dies with
FileNotFoundError.
The rule that actually holds, and is worth stating this way in the plan:
Inside 142-186, anything anchored on
tmp_pathstays (it is an explicit path handed straight to_persist_config_value). Anything anchored onROOTmoves like every otherROOT /reference in the suite.
And the safety net is instructed to dismiss it. Wave 2's new sanity grep (plan line 286) reads:
git grep -n 'ROOT / "config.yaml"…' tests/returns only the expected concurrency-test sites attests/test_admin_config.py:142-186(which pass explicit paths) or nothing at all.
Line 146 is precisely the hit that grep would produce, and the acceptance criterion pre-labels it as expected. That turns a catch into a rubber stamp. The criterion should be "returns nothing at all" — full stop.
Cosmetic, same area: plan line 275 has an unbalanced quote —
tmp_path / "config" / "config.yaml — in a line a worker will copy.
Verified by running it, not by reading it
- The new collection gate is not a false red.
pytest --collect-only -q | tail -1emits733 tests collected in 1.43s(with ANSI codes, no trailing blank), sogrep -q "733 tests"→ PASS. Worth having checked;tail -1guards break this way often. - The
"status":"success"gate is achievable and exactly matched.feedback.py --dry-runexitsrc=0against the liverouter.db, and FastAPI renders{"status":"success"}with no space after the colon — so the grep pattern matches literally. Both halves had to be true for that gate to work. deploy/README.mdline references are accurate: 12-14 (the unit table), 34-35 (the sed install loop), 94 (the oneshot-buffering note namingpoller.pyandseed_energy.py).
On Metis's second gap
"pytest silent zero-collection if pythonpath=["src"] is set before src/ exists"
is not a real failure mode — an unimportable test module produces a collection
error, loudly, not a silent zero; and testpaths = ["tests"] still resolves. The
added git mv-before-pyproject ordering and the count assertion cost nothing and
are fine to keep as belt-and-braces. Flagging only so the reasoning isn't reused
somewhere it matters.
Momus's rollback warning was worth taking, and the pre-flight note now states it
correctly: restoring the backup units alone, after modules are already in src/,
crashes — a full revert needs git reset --hard $PREFLIGHT_SHA and the units.
On the documented tradeoff
Keeping the service up through Waves 1-2 is a reasonable owner's call, and it is
now recorded as one rather than left implicit. Two things make the residual risk
smaller than it reads: both timers are stopped at pre-flight, so the likeliest
trigger is gone; and the exposure is a genuine crash-loop only if something
restarts the process in that window. The one path still open is the admin portal's
own /admin/api/restart-service button — worth simply not touching until Wave 3
lands.
Go / no-go
Fix V4-1 (one line in the plan, plus the grep criterion and the stray quote) and execute. Nothing else is outstanding.
Round 3 — v3
Verdict: two real defects and one ordering fix, all small. Fix them before
spending the dual review pass. All ten claimed v2 fixes are present and correct;
I re-verified each. Both new defects are in the same place — admin.py's config
backup write — and neither is a consequence of the v2→v3 reordering; they have
been carried unnoticed since v1, mine included.
Must fix
V3-1. The admin.py:370 backup-path change is wrong and 500s the Controls save
Wave 2 instructs:
src/admin.py:370backup pathf"config.yaml.bak.{ts}"→f"config/config.yaml.bak.{ts}"
The actual code is:
backup = config_path.with_name(
f"config.yaml.bak.{int(time.time())}"
)
Path.with_name() replaces only the filename component and keeps the parent.
Once config_path is <root>/config/config.yaml, it already produces
<root>/config/config.yaml.bak.<ts>. The line needs no change at all.
Applying the instruction gives with_name("config/config.yaml.bak.123"), and
with_name rejects any argument containing a separator. Verified on this repo's
venv (Python 3.14.6):
unchanged -> /repo/config/config.yaml.bak.123
v3 change -> RAISES ValueError Invalid name 'config/config.yaml.bak.123'
That raises out of _persist_config_value, so every persisted config edit from
the Controls page returns 500 — the feature that just shipped in the admin
facelift work.
It does fail loudly: tests/test_admin_config.py:128
(test_config_POST_creates_backup_before_write) and the concurrency test at 142
both exercise the write. So it costs a debug cycle rather than shipping broken.
But it should not be in the plan.
Fix: delete that bullet from Wave 2. Add it to the "must NOT do" list instead,
with the with_name reason, so nobody re-derives it.
(This line was in v1 and v2 too. Round 1 flagged admin.py:370 only for the
gitignore-pattern question and did not check with_name — that was my miss.)
Related, and still correct: .gitignore's config.yaml.bak.* has no slash, so it
matches by basename at any depth. config/config.yaml.bak.* stays ignored with no
.gitignore change — which now matters, since that is genuinely where they land.
V3-2. tests/test_admin_config.py isn't in Wave 2's list and needs real changes
The client fixture (lines 45-58) builds the admin router with
base_dir=str(tmp_path) and stages the config at the top of tmp_path:
config_yaml = tmp_path / "config.yaml"
shutil.copyfile(ROOT / "config.yaml", config_yaml)
...
router = build_router(None, _db_factory, base_dir=str(tmp_path))
Wave 2 changes admin.py:439 to Path(base_dir) / "config" / "config.yaml", so
the fixture must stage into a config/ subdirectory instead:
config_yaml = tmp_path / "config" / "config.yaml"
config_yaml.parent.mkdir(parents=True, exist_ok=True)
shutil.copyfile(ROOT / "config" / "config.yaml", config_yaml)
and the backup assertion at line 135 becomes:
backups = sorted(tmp_path.glob("config/config.yaml.bak.*"))
Note the asymmetry, because it is easy to over-apply: the concurrency test at
142-186 calls _persist_config_value(config_yaml, ...) with an explicit path
and never goes through base_dir. Its tmp_path / "config.yaml" staging and its
glob at line 182 are correct as they stand and must be left alone.
Wave 2's test list names test_proficiency.py, test_config_endpoints.py,
test_poller_parsing.py, test_admin_frontend.py and the generic
ROOT / "config.yaml" sweep. test_admin_config.py matches the generic sweep for
lines 54 and 146, so a mechanical pass would rewrite the source path and leave the
destination and the glob wrong.
V3-3. Wave 1 breaks the admin triggers, Wave 3 restarts production into that state, and the Wave 1 smoke cannot detect it
After Wave 1, _repo_root is corrected but the spawn argv is still
[sys.executable, "poller.py"] with cwd=<repo root> — and poller.py now lives
in src/. Wave 4 is what fixes the argv, three waves later.
Wave 1's acceptance includes:
curl -sf -X POST localhost:8081/admin/api/apply-feedback?dry_run=true
This cannot fail. Verified: a missing script exits rc=2, and _run_steps
turns every failure — spawn error, timeout, nonzero exit — into a _job(...) dict
returned as HTTP 200 with {"status": "failed"}. The endpoint has no raise
path, and curl -sf only inspects the status code.
That is the third false-green of this exact shape across three revisions (v1's
Wave 1 import metrics, v2's Wave 1 python -m pytest, now this). The pattern
worth internalising: a check that passes before the change is applied is not a
check.
Two fixes, take both:
-
Assert the job status, not the HTTP code, at every occurrence of that curl:
curl -sf -X POST 'localhost:8081/admin/api/apply-feedback?dry_run=true' \ | grep -q '"status":"success"' || { echo "TRIGGER BROKEN"; exit 1; } -
Fold Wave 4 into Wave 1. It is four argv lines plus the
test_admin_triggers.pyassertions, it depends on Wave 1 alone, and it removes an intermediate state in which production's Controls buttons are silently dead — a state Wave 3 currently restarts the live service into. This is the same argument v3 already accepted for folding pyproject into Wave 1.
The live service is unaffected during Waves 1-2 (it runs already-loaded code), so the exposure begins precisely at the Wave 3 restart. Folding closes it.
Verified, no action
- Wave 3's reinstall matches the documented install exactly.
REPO=$(pwd)and the sed loop are character-for-character whatdeploy/README.md:31-36does. V2-1 is properly resolved. - All four
-mtargets (poller,tier,seed_energy,feedback) haveif __name__ == "__main__"guards. - No
mypy.ini/ruff.toml/setup.cfg/tox.iniexists, so no lint config references root.pypaths. - No other invoker of root
.pyscripts inopencode.json,package.json, or.claude/. - Root-entry arithmetic checks out:
51 − 29 + 1 − 4 + 1 − 4 + 1 = 17. tests/test_admin_triggers.pymonkeypatchesasyncio.create_subprocess_exec, so Wave 4 spawns nothing real; the cited line numbers (101, 107-108, 140, 149, 161, 168) are accurate.
Round 2 findings — resolution
| # | Round-2 finding | v3 status |
|---|---|---|
| V2-1 | Wave 7 hardcodes a personal path, installs via cp |
Fixed. %h/llm-router kept; sed loop adopted verbatim; "do NOT copy units with cp" is an explicit guard. |
| V2-2 | Subprocess probes lose PYTHONPATH |
Fixed. Both call sites listed, with the env dict and the reason. |
| V2-3 | test_admin_frontend.py .parent assertion |
Fixed. Line 56 → .parent.parent, in Wave 1. |
| V2-4 | Wave 2→7 armed window; wrong dependency | Fixed. Timers stopped at pre-flight; reinstall moved to Wave 3; matrix now carries an explicit "production window armed?" column. |
| V2-5 | Wave 1 committed a broken tree | Fixed. pythonpath folded into the move commit. |
| V2-6 | load_config stays cwd-relative |
Decided, and recorded as a decision in both Scope-OUT and Wave 2. Correct handling — it is now a choice rather than an omission. |
| V2-7 | Smoke port 9000 vs convention | Fixed. 8081, citing CLAUDE.md. |
| V2-8 | Commit count | Fixed. "6 waves, 5 git commits." |
| V2-9 | Root target | Fixed. Exactly 17. |
| V2-10 | Update the stale comment in the unit files | Fixed. Wave 3 updates it for config/ and PYTHONPATH. |
| V2-11 | code_plans/ count |
Fixed by removal. The bad number is gone; the criterion is "all tracked files, no deletions." |
Also correctly carried over: the config/__init__.py guard (confirmed — a
config/ directory does not shadow src/config.py, because PEP 420 records it as
a namespace portion and a regular module found later on sys.path wins; adding
__init__.py would flip that to path order).
Recommended edits before execution
- V3-1 — delete the
admin.py:370bullet; move it to "must NOT do" with thewith_namereason. - V3-2 — add
tests/test_admin_config.pyto Wave 2: fixture stages intotmp_path/config/, line 135 glob gains theconfig/prefix, lines 142-186 left alone. - V3-3 — assert
"status":"success"in every trigger curl, and fold Wave 4 into Wave 1.
With these three applied v3 is ready to execute, and worth the dual high-accuracy review pass.
Round 2 — v2
Verdict: close. Four must-fix items, then it's executable. Every Round-1 blocker is genuinely resolved, the reference list is now verified rather than asserted, and the PYTHONPATH decision is the right call and correctly argued. The remaining problems are all in the two places v1 didn't reach: the systemd install mechanism, and the tests that spawn real subprocesses.
Correction to Round 1
My Round-1 item B3 said "deploy/*.service should be corrected to
%h/Sources/6krrt (or the drift documented)." That was wrong, and v2's Wave 7
implements it. %h/llm-router is not drift — it is a deliberate placeholder,
substituted at install time by the loop already in deploy/README.md:34-35:
for u in deploy/llm-router*.{service,timer}; do
sed "s|%h/llm-router|${REPO}|g" "$u" > ~/.config/systemd/user/"$(basename "$u")"
done
The live units are that loop's output. See V2-1.
Must fix
V2-1. Wave 7 hardcodes a personal path and bypasses the documented install
Wave 7 changes deploy/*.service to WorkingDirectory=%h/Sources/6krrt,
EnvironmentFile=%h/Sources/6krrt/.env, ReadWritePaths=%h/Sources/6krrt,
Documentation=file:%h/Sources/6krrt/CLAUDE.md, and installs with a plain cp.
Two consequences:
- The repo ships one developer's home directory.
%h/llm-routeris the tokendeploy/README.mdgreps for; replacing it means the sed finds nothing and every other clone installs units pointing at~/Sources/6krrt. cpskips the substitution step entirely, so the install procedure indeploy/README.mdand the one in the plan now disagree.
Fix: keep %h/llm-router in deploy/*.service. Add only the layout changes
there — Environment=PYTHONPATH=%h/llm-router/src, ExecStart=... -m poller,
etc. Reinstall with the README's existing loop, not cp:
REPO=%h/Sources/6krrt # or: REPO=$(pwd)
systemctl --user stop llm-router.service llm-router-poller.timer llm-router-seed.timer
for u in deploy/llm-router*.{service,timer}; do
sed "s|%h/llm-router|${REPO}|g" "$u" > ~/.config/systemd/user/"$(basename "$u")"
done
systemctl --user daemon-reload
systemctl --user start llm-router.service llm-router-poller.timer llm-router-seed.timer
Note the sed must also rewrite the new PYTHONPATH line, which it does for free
since that line contains the same token.
V2-2. Three real-subprocess tests break — pytest's pythonpath does not export PYTHONPATH
Verified empirically inside a pytest run: PYTHONPATH env = None. The
pythonpath ini option mutates the pytest process's sys.path; it sets no
environment variable, so child processes inherit nothing.
Two tests spawn a real interpreter:
tests/test_admin_frontend.py:84 subprocess.run([sys.executable, "-c", probe], cwd=str(ROOT))
probe does: import admin
tests/test_admin_health.py:203 subprocess.run([sys.executable, "-c", probe], cwd=str(ROOT))
probe does: import admin; assert 'dispatcher' not in sys.modules
With cwd=ROOT and the modules now in src/, both get
ModuleNotFoundError: No module named 'admin' after Wave 2. Neither file appears
in Wave 2's file list.
Fix: pass the env explicitly at both call sites:
subprocess.run(
[sys.executable, "-c", probe],
check=True,
cwd=str(ROOT),
env={**os.environ, "PYTHONPATH": str(ROOT / "src")},
capture_output=True,
)
This is the price of choosing PYTHONPATH over an editable install — an editable
install would have made these work untouched. The trade is still correct for the
reason v2 gives (a rebuilt .venv without pip install -e . is a silent
outage); it just has this one bill attached, and the plan should pay it
deliberately rather than discover it.
V2-3. tests/test_admin_frontend.py:53-58 hardcodes the old __file__ depth
def test_admin_index_file_exists_at_module_derived_path():
"""The served file lives at Path(__file__).parent/admin/frontend/index.html."""
expected = (
Path(admin.__file__).resolve().parent / "admin" / "frontend" / "index.html"
)
assert expected.is_file(), f"frontend file missing at {expected}"
After Wave 2 this resolves to src/admin/frontend/index.html and fails. Needs
.parent.parent, and the docstring updated to match.
v2's test list names only tests/test_admin_frontend.py:71 — which is the
admin.load_config('config.yaml') string inside the subprocess probe, correctly
caught for Wave 3. The .parent assertion eight lines above it was missed. This
is the same test file that documents the _REPO_ROOT coupling, so it is the one
place the anchor change is asserted.
V2-4. The Wave 2 → Wave 7 window leaves production armed, and the timers will fire in it
Quantified from the live units:
llm-router-poller.timer OnUnitActiveSec=2h
llm-router-seed.timer OnUnitActiveSec=6h
Between Wave 2 (modules move) and Wave 7 (units reinstalled), the installed units
still say ExecStart=… uvicorn dispatcher:app with no PYTHONPATH and
WorkingDirectory=%h/Sources/6krrt. The dispatcher survives only because it is
already loaded. Anything that restarts it — the admin portal's own
/admin/api/restart-service button, an OOM, a reboot, Restart=always after any
crash — brings it back into a crash loop on the port opencode.json uses for the
user's own inference. And a multi-hour reorg will cross at least one 2-hour
poller tick, which fails silently; stale_after_days: 3 then starts the clock on
a catalog that routes nothing.
Two fixes, both cheap:
- Pre-flight:
systemctl --user stop llm-router-poller.timer llm-router-seed.timerbefore Wave 1, restart them in Wave 7. Add to the pre-flight block next toPREFLIGHT_SHA. - Move the unit reinstall to immediately after Wave 3. The dependency matrix
says Wave 7 depends on Wave 4 — it does not. Wave 4 changes
admin.py's internal spawn argv; the unit files never reference it. Wave 7 needs Wave 2 (src/) and Wave 3 (config/paths) and nothing else. Reordering cuts the exposure window from five waves to two, and leaves Waves 4/5/6 as doc/test-only and production-safe.
Should fix
V2-5. Wave 1 commits a tree where the pytest console script is broken
Wave 1 sets pythonpath = ["src"] while the modules are still at the root and
src/ does not exist. Collection then works only under python -m pytest, which
adds the cwd — and the existing comment in pyproject.toml says in as many words
that the setting exists precisely because the bare pytest console script does
not:
Running as
python -m pytesthappens to add the cwd and hides this; thepytestconsole script does not.
So Wave 1's acceptance criterion (python -m pytest --collect-only -q passes) is
a false-green of exactly the kind that made v1's Wave 1 dangerous — it passes for
a reason unrelated to what it claims to check.
Fix: move the pythonpath line into Wave 2's commit, alongside the move it
describes. Keep .gitignore and the pre-flight SHA in Wave 1.
V2-6. config.py's default stays cwd-relative — worth fixing while you're in there
load_config(path="config/config.yaml") still resolves against the current
directory. Every invocation from anywhere but the repo root breaks, which is the
whole reason the live unit carries this comment:
# config.yaml, router.db and router.log are all referenced as relative paths,# so this has to be the repo root.
You are already introducing _REPO_ROOT in admin.py and already editing all 11
call sites. Anchoring the default the same way removes the class:
_REPO_ROOT = Path(__file__).resolve().parent.parent
def load_config(path: str | Path | None = None) -> RouterConfig:
path = Path(path) if path is not None else _REPO_ROOT / "config" / "config.yaml"
Ten of the eleven call sites then collapse to a bare load_config(), and the
9000/8081 smoke stops depending on cwd.
This is a small logic change, not a relocation, so it sits just outside the plan's stated "pure path refactoring" boundary. Flagging it as a decision to make on purpose — taking it or declining it are both fine; arriving at it by accident in Wave 3 is not.
Minor
V2-7. Smoke port 9000 contradicts the project's own convention
CLAUDE.md, in the section written after the pkill outage:
Convention going forward: 8080 is production, always. … A throwaway instance (manual iteration, Playwright smoke tests against the admin frontend, anything that isn't "use the real router") binds 8081 instead.
v2's smoke block uses 9000 throughout. Use 8081, or amend the CLAUDE.md convention in Wave 5 — but don't leave the repo asserting two different answers, given what that convention was written to prevent.
V2-8. Commit count is 6, not 7
Waves 1-5 produce 5 commits; Wave 6 states outright that it generates none (untracked files); Wave 7 produces 1. The TL;DR says 7.
V2-9. Root target is exactly 17
51 − 29 + 1 (src) − 4 + 1 (config) − 4 + 1 (plans) = 17. v2 says "≈ 16". Make
the success-criteria checkbox an exact number so it can actually be checked.
V2-10. Update the stale comment in the unit files
deploy/llm-router.service carries # config.yaml, router.db and router.log are all referenced as relative paths, so this has to be the repo root. It becomes
config/config.yaml. The unit files are already in scope for Wave 7.
V2-11. code_plans/ is ~320 files, not "100 glob results"
The glob was truncated. It doesn't matter any more — v2 wisely dropped the
find plans/ -type f | wc -l = 40 criterion in favour of "all tracked files, no
files deleted", which is both correct and checkable. Noting it only so the number
isn't reused elsewhere. (59 tracked, ~260 ignored inside a nested .omo/ tree.)
Round 1 findings — resolution
| # | Round-1 finding | v2 status |
|---|---|---|
| B1 | src. prefix contradicts flat-module premise |
Fixed. All src. prefixes gone; "must NOT use src. prefixes" is now an explicit guard. |
| B2 | packages=find installs an empty wheel |
Fixed by removal. No packaging at all; PYTHONPATH instead, with the rationale stated. src/__init__.py explicitly forbidden. |
| B3 | Live systemd units not the repo's; service running | Partly. Now a first-class wave with stop/reload/verify and a unit backup — but see V2-1 (install mechanism) and V2-4 (ordering). |
| B4 | admin.py:445-448 frontend paths |
Fixed via _REPO_ROOT. Test assertion still missed — V2-3. |
| B5 | admin.py:662 subprocess cwd |
Fixed via _REPO_ROOT. |
| F1 | Fabricated router_cli.py section |
Fixed. Section deleted; router_cli.py correctly absent from the config call-site list. |
| F2 | Missing leaderboard.py:135 |
Fixed, and flagged as "missed in v1". |
| F3 | Test counts low | Fixed and exceeded. v2 found 7 bare sites in test_proficiency.py (191, 207, 230, 260, 284, 310, 337) — verified accurate, three more than Round 1 listed. |
| F4 | git rm on gitignored backups |
Fixed. Plain rm, no commit, explicitly noted. |
| F5 | Wave 6 file counts wrong | Adequately fixed — bad number survives but the criterion no longer depends on it (V2-11). |
| F6 | Three different root-entry numbers | Fixed to one number; off by one (V2-9). |
| S1 | Unresolved reasoning left in the document | Fixed. Clean throughout; single wave ordering. |
| S2 | 733 tests can't catch B3/B4/B5 | Fixed. Real uvicorn + /admin/ + trigger-POST smoke per wave. |
| S3 | No rollback procedure | Fixed. Per-wave SHA table, pre-flight SHA, and a unit-file backup/restore for the one non-git wave. |
Also correctly carried over: the config/__init__.py guard (confirmed — a
config/ directory does not shadow src/config.py, because PEP 420 records it as
a namespace portion and a regular module found later on sys.path wins; adding
__init__.py would flip that to path order).
Recommended edits before execution
- V2-1 — revert
deploy/*.serviceto the%h/llm-routerplaceholder; install via the README's sed loop, notcp. - V2-2 — add
env={**os.environ, "PYTHONPATH": str(ROOT / "src")}to the twosubprocess.runprobes; add both files to Wave 2's list. - V2-3 —
tests/test_admin_frontend.py:53-58→.parent.parent; add to Wave 2. - V2-4 — stop the two timers at pre-flight; move the unit reinstall to directly after Wave 3 and correct the dependency matrix.
- V2-5 — fold the
pythonpathchange into Wave 2's commit. - V2-6 — decide explicitly on the
config.pyabsolute-default change. - V2-7 – V2-10 — port 8081, commit count 6, root target 17, unit comment.
With 1-5 applied this is ready to execute. It would also survive the dual high-accuracy review now, if you still want that pass — the reference list is sound, which is what was missing before.
Round 1 — v1
Blockers — will break
B1. The src. prefix and the flat-module premise are mutually exclusive
The plan asserts both, in the same wave:
- Wave 2 "must NOT do": "Do NOT change any
.pymodule's internal imports (barefrom config import ...continues to work because editable install putsconfig.pyon sys.path)." - Wave 2 acceptance #4/#5, Wave 3 QA, success criteria:
uvicorn src.dispatcher:app,python -m src.poller,from src.config import load_config.
These cannot both hold. With package-dir = {"": "src"} the modules install as
top-level names — config, dispatcher, metrics. There is no src.
namespace at all. To get src.dispatcher you would need a package.
Plan must pick one world: flat modules with bare imports, or a src/ package
with src. prefixes. The repo uses bare imports everywhere, so the correct path
is the former.
Fix: drop every src. prefix; uvicorn dispatcher:app, python -m poller,
from config import load_config, etc.
B2. Wave 1 installs an empty wheel
The plan says:
pyproject.tomlhas a[project]or[tool.setuptools]section withpackages = findorpackages = ["src"],package-dir = {"": "src"}. ...src/__init__.pyis created
find_packages() will not discover flat .py files directly under src/. It
finds directories with __init__.py. With flat modules and package_dir={"": "src"},
the correct setuptools directive is py-modules = ["admin", "baseline_report", ...]
(or use a package). packages = find produces an empty distribution.
I verified this in a fresh venv: the resulting wheel installed nothing, so
import dispatcher failed. The plan's Wave 1 QA (import src.dispatcher) would
also fail because src.dispatcher does not exist when flat modules are installed
as top-level names.
Fix: either go to a proper package src/llm_router/ (and rewrite every
import), or skip packaging entirely and use PYTHONPATH=src in systemd units +
pytest pythonpath. For a service that isn't distributed as a package,
PYTHONPATH is simpler and avoids the editable-install footgun in CLAUDE.md's
venv warning.
B3. The live systemd units aren't the repo's
~/.config/systemd/user/llm-router.service uses WorkingDirectory=%h/Sources/6krrt
and is already active. deploy/llm-router.service still says %h/llm-router.
The plan updates deploy/*.service but says nothing about installing the updated
units on the live machine. The production service is active now and will restart
on Restart=always after any failure. If the deploy units are not copied to
~/.config/systemd/user/, the next restart will use the old units against the new
src/ layout and crash-loop on port 8080 (which opencode.json uses).
Fix: add a first-class production step: stop timers/service, copy updated
units to ~/.config/systemd/user/, daemon-reload, start, verify /health.
B4. admin.py serves frontend files from the wrong directory after the move
admin.py resolves _module_dir = Path(__file__).resolve().parent and uses it
for the four admin/frontend/*.html paths. After moving admin.py into src/,
this resolves to src/admin/frontend/ instead of admin/frontend/, so all admin
pages 404.
The plan mentions the subprocess cwd but not the frontend paths.
B5. admin.py subprocess cwd is wrong
Line 662 sets _repo_root = str(Path(__file__).resolve().parent) which becomes
src/ after the move. Maintenance scripts are run from cwd=_repo_root, but the
config file and router.db are at the repo root, so spawned scripts will look for
config.yaml in src/.
Fix: same fix as B4 — compute repo root, not file parent.
Fabrication / unverified references
- The plan quotes
router_cli.pyas doingconfig_py = Path(__file__).resolve().parent / "config.yaml". That code does not exist;router_cli.pyhas no__file__path resolution and noload_configcall. This error is the tell that the reference list was not actually checked against the files. - Missing
leaderboard.py:135load_config("config.yaml"). - Claimed "9 bare-path test sites, not 2" is correct; the plan says 2.
- Claimed "23 affected test files" is correct; the plan says "~20".
- The 14
config.yaml.bak.*are gitignored, sogit rmcannot work — the plan saysgit rm. code_plans/holds ~320 files (59 tracked + ~260 ignored nested artifacts), not 38 / 100 / 40 as variously stated.
What it got right
- The real problem is the 29 loose
.pyfiles and 14 backups. - Moving source to
src/(flat modules, bare imports) is the right conceptual answer for this codebase. - The
config/directory does not shadowsrc/config.py(PEP 420 namespace vs regular module resolution), as long asconfig/__init__.pyis never added. - No top-level basename collisions across the four doc dirs.
Recommended revisions before execution
- Resolve B1: strike every
src.prefix; modules stay top-level. - Fix B2:
py-modulesnotpackages; delete thesrc/__init__.pycriterion; make Wave 1's QA assert the install from outside the repo root. Or take thePYTHONPATHroute and drop Wave 1. - Add B3 as a first-class wave step with the stop/copy/daemon-reload/start/verify
sequence, and reconcile
deploy/*.serviceWorkingDirectorywith the live units. - Fold B4 + B5 +
dispatcher.py:2968into one_REPO_ROOTchange inadmin.py. - Delete the fabricated
router_cli.pysection; addleaderboard.py:135; re-grep the test sites for all four path spellings. - Rewrite Wave 5 as
rm, notgit rm; recount Wave 6; pick one root-entry number. - Strip the in-document reasoning and settle on one wave ordering.
- Add the 8081 smoke checks and a per-wave
git reset --hard+ unit-reinstall rollback.
Not ready for the dual high-accuracy review — that would burn a review pass on a document whose reference list is unverified. Fix 1-6 first; 7-8 can land in the same revision.