3523dcf93e
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
184 lines
10 KiB
Markdown
184 lines
10 KiB
Markdown
# 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.
|