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
164 lines
7.7 KiB
Markdown
164 lines
7.7 KiB
Markdown
# Remove the hardcoded `neuralwatt` literals
|
|
|
|
Status: done -- provider column, no literals
|
|
|
|
**Status: FINAL — decision-complete.** Written 2026-09-04 against
|
|
`feat/config-local-overlay` at `0570123`.
|
|
|
|
## Scope, stated first because it is easy to over-read
|
|
|
|
This plan does **not** add a provider, and does **not** build the `Provider`
|
|
protocol sketched in `plans/multi-provider-support.md`. That plan is PARKED
|
|
pending provider selection.
|
|
|
|
This one removes 20 hardcoded string literals that are wrong regardless of
|
|
which provider ever lands second — and fixes the three that are latent defects
|
|
today. It is separable from the parked plan by construction: nothing here needs
|
|
to know what the second provider is.
|
|
|
|
`(model_id, provider)` is already the composite key throughout the schema, and
|
|
`dispatch_providers: dict[str, DispatchProvider]` (`src/config.py:779`) is
|
|
already a mapping. The literals are the gap between that design and the code.
|
|
|
|
## The inventory
|
|
|
|
`grep -rn neuralwatt src/*.py` — **20 references across 6 files**, re-verified
|
|
2026-09-04, unchanged from the count taken at `340453e`.
|
|
|
|
| file | refs | character |
|
|
|---|---|---|
|
|
| `poller.py` | 8 | the bespoke fetch — URL, function name, literal `provider=`, sanity-floor query, log prefixes |
|
|
| `eval_proficiency.py` | 5 | defaults and a `dispatch_providers[...]` lookup |
|
|
| `dispatcher.py` | 4 | one real defect, one passthrough default, two conditionals |
|
|
| `metrics.py` | 1 | staleness query — **defect** |
|
|
| `leaderboard.py` | 1 | prior write — **defect** |
|
|
| `seed_energy.py` | 1 | `dispatch_providers[...]` lookup |
|
|
|
|
## The three that are defects now
|
|
|
|
### 1. `dispatcher.py:2879` — `_model_exists` fails OPEN
|
|
|
|
```python
|
|
def _model_exists(model_id: str) -> bool:
|
|
row = conn.execute(
|
|
"SELECT 1 FROM models WHERE model_id = ? AND provider = 'neuralwatt'",
|
|
(model_id,),
|
|
).fetchone()
|
|
```
|
|
|
|
This decides whether a `provider/model` string a client sent is an
|
|
opencode-style alias to **strip** (`llm-router/...`) or a real id that merely
|
|
contains a slash. Scoped to one provider, a real second-provider id returns
|
|
False and is treated as an alias.
|
|
|
|
`vendor/model` is the **native id format** for OpenRouter and most aggregators
|
|
(`deepseek/deepseek-chat`), so this is the default case for a whole class of
|
|
provider, not an edge one.
|
|
|
|
And the failure direction is wrong. A provider-scoped 422 would be loud. This
|
|
strips the prefix and proceeds with the remainder, so the request dispatches to
|
|
whatever the remainder resolves to — a different row, potentially a different
|
|
provider, at a different price — with nothing in the response marking a
|
|
substitution. Every other capability check in this path **fails closed** by
|
|
design; this one fails open.
|
|
|
|
**Note the near miss.** `_check_pinned_capabilities` (`dispatcher.py:2887`) sits
|
|
immediately below it, already takes `provider` as a parameter, and is correct.
|
|
An earlier revision of the multi-provider draft blamed 2879 on that function;
|
|
it does not. Fix the right one.
|
|
|
|
### 2. `metrics.py:175` — catalog staleness ignores any other provider
|
|
|
|
```sql
|
|
SELECT MAX(last_updated) AS last_updated FROM models WHERE provider='neuralwatt'
|
|
```
|
|
|
|
The staleness warning is computed over one provider's rows. A second provider
|
|
whose poll silently stops leaves the warning green while its catalog freezes.
|
|
That is precisely the **silent-and-open** failure `CLAUDE.md`'s "Run as a
|
|
service" section describes, gaining a second entrance.
|
|
|
|
### 3. `leaderboard.py:125` — priors are pinned to one provider
|
|
|
|
```python
|
|
set_leaderboard(conn, cfg, model_id, "neuralwatt", category, score)
|
|
```
|
|
|
|
A curated prior for a shared model attaches to the NeuralWatt row and never to
|
|
any other. `leaderboards.yaml` ships empty, so nothing is broken yet — but this
|
|
silently *decides* the still-open proficiency-sharing question from
|
|
`multi-provider-support.md` by accident rather than on purpose.
|
|
|
|
Because that question is genuinely undecided, **do not resolve it here.**
|
|
Parameterise the call so the provider is passed in rather than assumed, and
|
|
leave the sharing policy to the parked plan. Passing the literal from one call
|
|
site is a fix; inventing a fan-out rule is a decision this plan has no mandate
|
|
to make.
|
|
|
|
## The rest
|
|
|
|
Mechanical, and worth doing in the same pass because they are what make the
|
|
three above verifiable rather than isolated patches.
|
|
|
|
- **`poller.py` (8).** Keep `fetch_neuralwatt` as the single concrete fetcher —
|
|
this plan does not introduce the protocol — but move the URL, the provider
|
|
string, the sanity-floor query and the log prefix so they derive from one
|
|
named provider value rather than being spelled out five times. The sanity
|
|
floor (`poller.py:420`) must count rows for **the provider being polled**.
|
|
- **`eval_proficiency.py` (5)** and **`seed_energy.py` (1).** Turn the
|
|
`provider: str = "neuralwatt"` defaults and `dispatch_providers["neuralwatt"]`
|
|
lookups into an explicit provider argument threaded from the caller. The
|
|
`identity["provider"] != "neuralwatt"` skip at `eval_proficiency.py:724`
|
|
becomes a comparison against that argument.
|
|
- **`dispatcher.py:3258-3259`** — the passthrough default and its conditional.
|
|
Same treatment.
|
|
|
|
## Risk to respect
|
|
|
|
`poller.mark_stale` runs only inside `poller.main()`, and `main()` returns early
|
|
on a `RequestException` — **before** `upsert` and **before** `mark_stale`.
|
|
That is existing single-provider behaviour and this plan does not change it.
|
|
|
|
But do not parameterise the fetch in a way that makes a future second provider
|
|
share one `main()` failure path: `docs/incidents.md` records the empty-`data`
|
|
array as the one route that can empty the candidate set, and a shared path
|
|
would let one provider's outage mark another's rows stale. **Isolating per
|
|
provider is the parked plan's job**; this plan's obligation is not to build a
|
|
structure that makes isolation harder later. Where a choice arises, prefer the
|
|
shape that keeps one provider's fetch, upsert, sanity floor and staleness
|
|
marking together.
|
|
|
|
## Non-goals
|
|
|
|
- Do not add a second provider, or provider config beyond what
|
|
`dispatch_providers` already holds.
|
|
- Do not build the `Provider` protocol, capability flags
|
|
(`has_energy_telemetry`), or a `type:` discriminator. Parked plan.
|
|
- Do not decide the proficiency-sharing question (see defect 3).
|
|
- Do not change the cost model, eco scoring, or any ranking behaviour. This
|
|
plan must be observably behaviour-neutral on a single-provider deployment.
|
|
- Do not rename the `neuralwatt` provider value itself. Existing rows,
|
|
`proficiency` keys and `energy_observations` reference it; a rename is a
|
|
migration and is not in scope.
|
|
|
|
## Success criteria
|
|
|
|
- `grep -rn neuralwatt src/*.py` returns only the places where the value is
|
|
*configured or named* — not places where behaviour is conditioned on it.
|
|
State the expected remaining count in the PR so it can be re-checked.
|
|
- `_model_exists` resolves across all configured providers, and
|
|
alias-stripping is decided by something other than "no NeuralWatt row has
|
|
this id". A test pins that a `vendor/model` id belonging to a non-NeuralWatt
|
|
row is **not** stripped.
|
|
- Catalog staleness is computed per provider; a test with two providers' rows
|
|
shows a stale second provider raising a warning.
|
|
- `set_leaderboard` receives its provider from the caller; no literal.
|
|
- The poller's sanity floor counts rows for the provider being polled, proven
|
|
by a test with rows from more than one provider present.
|
|
- **Behaviour-neutral on the live single-provider deployment**: routing
|
|
decisions, staleness warnings and poller output are unchanged. Pin this with
|
|
a before/after comparison on a real catalog, not by inspection.
|
|
- Full suite green with `local_energy.enabled` both true and false.
|
|
- The user's `config/config.local.yaml` values are untouched — tariff `0.159`
|
|
and `enabled: true` verbatim.
|