6krrt/plans/pending-ops-fixes.md

Pending ops fixes found live, 2026-09-06/07

Status: in progress -- WAL applied live; the circuit-breaker and stream findings are open

Three findings from directly operating the router today (not from planning), each caught by actually exercising the system rather than reading the code. Recorded so they don't get lost -- none are urgent, but two are real bugs and one already got applied live.

1. DONE (applied live 2026-09-06 22:48 UTC): router.db moved to WAL journal mode

Symptom: python -m eval_proficiency crashed with sqlite3.OperationalError: database is locked inside log_observation's conn.commit() (dispatcher.py:2156), after successfully completing exactly one full model's 57-task set. The default rollback-journal mode (PRAGMA journal_mode was delete) means a writer briefly blocks every reader and vice versa -- and this router now has real concurrent access from multiple processes hitting one file: the live dispatcher (thousands of writes/hour), the poller/seed-energy timers, admin portal reads, the TUI, ad-hoc CLI diagnostics, and now eval scripts.

Applied: sqlite3 router.db "PRAGMA journal_mode=WAL;" -- a one-time, persistent, file-level change (stored in the DB header, no app code or config change needed, every new connection picks it up automatically). Verified: PRAGMA journal_mode now reports wal, and router.db-shm / router.db-wal auxiliary files exist alongside the main file.

Follow-up housekeeping in this same change: .gitignore's *.db pattern doesn't match router.db-shm / router.db-wal (they don't end in .db) -- added explicit patterns for both so the new auxiliary files can never get swept into a git add -A.

Why this was safe to do live: WAL mode is a database-file property, not a live-connection setting -- flipping it doesn't require restarting the dispatcher service, and it was applied at a moment with no other active writer (the eval run had already crashed, so nothing was mid-transaction).

Not yet verified: whether the eval that crashed under delete mode now completes cleanly under WAL against the same concurrent load. First real test of that is the resumed eval run this same investigation kicked off.

2. Circuit breaker is blind to mid-stream upstream breaks

Symptom: gemma-4-31b broke mid-stream twice in ~3 hours (upstream_stream_broken, dispatcher.py:4419, "Response ended prematurely" after 212s and 129s) -- NeuralWatt's own infrastructure apparently has some timeout/limit that gemma-4-31b's unusually slow completions (avg 29.8s vs. 3-7s for every other model on this router; max successful duration 138.6s) occasionally exceeds. ~2 of 22-31 requests (7-9%) in the observed window.

The actual gap, not the symptom: circuit_breaker.record_success(current_model, provider) (dispatcher.py:4228) fires as soon as the upstream responds with a non-error HTTP status -- which happens immediately for a streaming request, before any body has streamed. The upstream_stream_broken handler lives in a separate code block (the SSE generator, dispatcher.py:4405-4420), which only runs after that success was already recorded. So a model that returns 200 and then dies mid-stream is permanently invisible to the one mechanism built to route around unreliable models -- no amount of mid-stream failures for a given model will ever trip its breaker.

Proposed fix: call circuit_breaker.record_failure(current_model, provider, ...) from inside the upstream_stream_broken exception handler too, not only on a bad initial status code. Needs a decision on whether a mid-stream break should count the same as a full request failure for backoff purposes, or a lighter weight -- a stream that produced partial (billable) output before dying is arguably a different severity than one that never started at all.

Not yet decided: whether gemma-4-31b's measured unreliability should also factor into scoring/proficiency somehow, separate from the circuit breaker (which is about availability, not quality). Parked as a separate, smaller question -- the circuit-breaker gap is the concrete bug; scoring for reliability is a judgment call, not a bug fix.

3. dispatcher.py's startup checks run on ANY import, not just when serving

Symptom, discovered while running eval_proficiency.py for an unrelated reason: the eval process allocated 1.7GB of GPU memory and took a real, unexplained delay before its first output -- despite eval_proficiency.py having nothing to do with classification. eval_proficiency.py imports dispatcher.py only for two small helpers (extract_telemetry, log_observation).

Root cause: dispatcher.py:471 (_ensure_tables()) and dispatcher.py:492 (_ensure_classifier_mode_ready()) are both called at bare module level -- unindented, outside any function, outside any if __name__ == "__main__" or FastAPI startup-event guard. They run unconditionally the instant anything imports dispatcher.py, for any reason.

_ensure_tables() is probably benign for another DB-touching script (it's idempotent schema migration), if a little wasteful. _ensure_classifier_mode_ready() is the real risk: when classifier.mode: local_encoder is configured, it calls local_encoder.ensure_available(...), which re-raises on a missing transformers/torch install. That means any unrelated script importing dispatcher.py -- eval_proficiency.py, router_cli.py, seed_energy.py, anything -- can now crash on import alone, in an environment that never needed the classifier, purely because the live service's config happens to have local_encoder mode on. It didn't bite today only because this exact venv happens to have torch installed now.

Proposed fix: move _ensure_classifier_mode_ready() (and probably _ensure_tables(), for the same reason, even though its current risk is lower) into a proper FastAPI @app.on_event("startup") handler -- this project doesn't use that pattern anywhere yet, so it's a new but standard addition -- so they only run when the service actually starts serving requests, never on a bare import by another script.

4. docs/pinch.md keeps losing its protected_max_chars section

Symptom: the protected_max_chars documentation (config example, table row, two prose references) has gone missing from the shared checkout's working tree five separate times on 2026-09-06/07: once blocking a git pull, once mid-/review-work, once found "pre-existing" at the start of a later agent session, and twice more after being restored.

Not a real regression: the content is correct in every commit on origin/main -- verified each time with git show origin/main:docs/pinch.md. This is purely a working-tree artifact.

Leading hypothesis, formed 2026-09-07: the shared checkout is routinely left sitting on a feature branch while agents work in it (observed on fix/admin-profiles-page-polish and feat/cloud-fallback-admin-control after their PRs merged). Branch state moving under an agent mid-session would explain a file reverting to an older revision without any commit doing it. The fix is probably operational -- return the shared checkout to main after each merge -- rather than a code change.

Restore (cannot be done from a worktree-isolated session, see project-direct-implementation-workflow): git checkout -- docs/pinch.md, or extract the blob and command cp it, verifying with git hash-object.

5. A test hardcodes the ambient classifier default instead of setting it

Symptom: tests/test_classifier_modes_dispatch.py::test_default_mode_is_local_llm_and_behaves_as_before asserts classifier.mode == "local_llm" by relying on whatever the ambient config resolves to, rather than constructing a config with that mode explicitly. It fails on any machine whose config.local.yaml overlay picks a non-default mode -- which is exactly this deployment (classifier.mode: local_encoder).

Confirmed as an overlay artifact, 2026-09-07: the same suite run from a worktree with no config.local.yaml passes 1521/1521, including this test and the 8 siblings in test_classifier_backoff.py / test_gaming_mode.py that fail alongside it in the shared checkout. Nothing is actually broken; the tests just read ambient config.

Proposed fix: construct the test's own RouterConfig with classifier.mode="local_llm" set explicitly. Small, standalone, no urgency -- but it currently makes "run the full suite" produce 9 red herrings for every agent working in the shared checkout, which has already cost time twice.

6. The profile update endpoint cannot rename, but the modal offers to

Symptom: admin/frontend/profiles.html's edit modal presents an editable "Profile name" box. Changing it and saving reports success and silently keeps the old name.

Root cause: _ProfileUpdateBody (src/admin.py) has no name field at all, and _profile_block_dict() does body.model_dump(exclude={"name"}), so a submitted name is dropped before it reaches _persist_profile(name, ...) -- which uses the path name. Renaming is delete-plus-create by design (the endpoint's own docstring refers to "rename-by-delete"), but nothing in the UI says so.

Found live 2026-09-07 while adding the inline editor, which initially followed the submitted name after save and landed the detail pane on a profile that did not exist. The inline editor now renders the name readonly with a note pointing at duplicate-then-delete; the modal was deliberately left alone and still shows the misleading editable box.

Proposed fix: make the modal's name field readonly when editing (it must stay editable for create/duplicate, which use a different endpoint), with the same explanation the inline editor carries. Alternatively, support rename server-side -- a bigger change, since it means moving a key in the overlay and deciding what happens if the new name collides with a built-in.

Ordering

None of these block each other. #1 is already done. #2, #3, #5 and #6 are all small, independent, well-understood fixes. #4 is operational rather than a code change, and #5 is worth doing early only because those 9 phantom failures mislead every agent that runs the suite in the shared checkout.