From c2b2fee45243578d6863b61b7997cc10d2890e12 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 17:59:13 -0400 Subject: [PATCH 1/9] docs(plans): TUI overhaul -- catch the dashboard up to the schema The decision table declares "id, kind, category, tier, ctx, selected, est $, flex" and has never had a timestamp -- observed_at is already carried by decision_row and simply never rendered, so that one is a display fix. Five columns were added to route_decisions across this session's merges and none reached the TUI: profile (PR #25), exploration (proficiency branch), pinch_original_tokens/pinch_final_tokens (PR #18), plus request_id and session_key. The admin portal got a Profile column; the TUI did not, from the same data. Deliberately does NOT add six columns to an eight-column table on a terminal. profile earns a column because it changes which models were considered at all; exploration becomes a flag beside flex, because an epsilon-greedy pick is not a ranking result and must not read as the router's judgement; the rest go to the existing detail popup as per-decision forensics. The quota panel section is explicitly SEQUENCED behind plans/quota-balance-and-burn-rate.md and must not land before it -- a progress bar against a plan figure that is routinely exceeded (146% observed, nothing failed) implies a ceiling that does not exist. If that plan has not landed, skip the panel rather than reimplementing balance/burn in the TUI. The load-bearing success criterion is a test that diffs PRAGMA table_info(route_decisions) against what the TUI model exposes and fails when a column is added without a decision about surfacing it. The rest of this plan is a one-time catch-up; that test is what stops the drift recurring. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- plans/tui-overhaul.md | 119 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 plans/tui-overhaul.md diff --git a/plans/tui-overhaul.md b/plans/tui-overhaul.md new file mode 100644 index 0000000..bd094f9 --- /dev/null +++ b/plans/tui-overhaul.md @@ -0,0 +1,119 @@ +# TUI overhaul: catch the dashboard up to the schema + +**Status: FINAL — decision-complete.** Written 2026-09-04 against `main` at +`a0c7e8e`. + +## Why now + +The TUI (`src/tui.py`, `tui_model.py`, `tui_screens.py`, `tui_sse.py` — ~1,000 +lines total) has drifted behind the data it displays. Five columns were added +to `route_decisions` across this session's merges and none reached the +dashboard, and the decision table has never had a timestamp. + +The admin portal got a Profile column in PR #25. The TUI did not. It is the +same data. + +## 1. The decision table is missing a timestamp + +Declared columns (`src/tui.py:314-316`): + +``` +"id", "kind", "category", "tier", "ctx", "selected", "est $", "flex" +``` + +`observed_at` **is already carried** by `decision_row` in `tui_model.py` — it +is fetched and then never rendered. So this is a display change, not a data +change. + +Add a `time` column. Render it **short** (`HH:MM:SS`), not the full ISO +timestamp: the rows are dense, the date is almost always today, and the full +form would crowd out `selected`, which is the column people actually read. +Put it first — it is the natural scan axis for a live feed. + +## 2. Five columns exist in the schema and are invisible + +Measured by diffing `PRAGMA table_info(route_decisions)` against what +`decision_row` exposes: + +| column | shipped in | why it matters | +|---|---|---| +| `profile` | PR #25 | which named profile served the request | +| `exploration` | proficiency branch | whether epsilon-greedy picked this, not the ranking | +| `pinch_original_tokens` | PR #18 | context pruning input | +| `pinch_final_tokens` | PR #18 | context pruning output | +| `request_id` | proficiency branch | the join key to `/outcome` reports | +| `session_key` | earlier | hashed session fingerprint | + +**Do not add six more columns to the table.** It already has eight and the +terminal is not wide. Instead: + +- Add **`profile`** to the table proper. It changes which models were even + considered, so a decision cannot be read without it — the same argument that + earned it a column in the admin portal. +- Add an **`E` flag** in the existing flags idiom for `exploration`, alongside + how `flex` is already rendered. An exploratory pick is not a ranking result + and must be visually distinguishable, or the operator reads a deliberate + random sample as the router's judgement. +- Put **`pinch_*`, `request_id`, `session_key`** in the **detail popup** + (`tui_screens.py`, opened with Enter or `e`), which already shows the full + decision JSON. They are per-decision forensics, not scan-axis data. + +## 3. The quota panel shows the wrong thing + +**This section depends on `plans/quota-balance-and-burn-rate.md` and must not +land before it.** That plan replaces percentage-of-plan with balance and burn +rate, because the current framing is measurably wrong: the warning says "a +quota is a wall, not a bill — requests fail rather than costing more" while +usage sat at 146% of plan and nothing failed, since the provider bills overage +against a credit balance. + +Once that lands, the TUI panel (`#quota-panel`, `#quota-progress`, +`#quota-legend`) should lead with **balance and projected runway** +("$12.19 left, ~23h at current burn") and demote the percentage bar to +secondary. A progress bar against a plan figure that is routinely exceeded is +actively misleading — it implies a ceiling that does not exist. + +**Sequencing:** if the quota plan has not landed when this one runs, do items +1, 2 and 4 and leave the panel alone. Do NOT reimplement balance/burn +independently in the TUI — `metrics.quota_burn` is the single source and the +TUI reads `/metrics`. + +## 4. Surface the warnings that already exist + +`coverage.warnings` from `/metrics` already carries catalog staleness, quota +burn, scoring coverage gaps and ceiling warnings. `#warnings-panel` exists. +Confirm every warning class actually reaches it — the vision-ceiling incident +on 2026-09-04 showed a whole warning family that was computed and never +displayed, and the fix there was surfacing, not computing. + +This is a verification task as much as a feature: for each warning the +`/metrics` `coverage.warnings` list can emit, assert it renders. + +## Non-goals + +- No new data. Everything here is already in `route_decisions` or `/metrics`. +- Do not widen the decision table beyond one added column plus one flag. +- Do not reimplement any metric in the TUI. `tui_model.py` is the pure data + layer over `/metrics` and `/events/decisions`; keep the computation in + `metrics.py`. +- Do not import `textual` outside the TUI modules. The dispatch path must stay + free of the UI dependency — that separation is deliberate and tested. +- No colour/theme rework. This is about information, not appearance. + +## Success criteria + +- The decision table shows a short `HH:MM:SS` time column, first. +- `profile` is a column; `exploration` renders as a flag beside `flex`. +- `pinch_original_tokens`, `pinch_final_tokens`, `request_id` and + `session_key` appear in the detail popup. +- A test diffs `PRAGMA table_info(route_decisions)` against what the TUI + model exposes and fails if a column is added to the schema without a + decision about surfacing it. **This is the test that stops the drift + recurring** — the rest of this plan is a one-time catch-up, this is the part + that keeps it caught up. +- Every warning class `/metrics` can emit renders in `#warnings-panel`. +- Quota panel leads with balance and runway **if** the quota plan has landed; + otherwise untouched and noted. +- `textual` still imported only by TUI modules (existing test stays green). +- Full suite green with `local_energy.enabled` both true and false. +- The user's `config/config.yaml` values remain uncommitted and verbatim. -- 2.49.1 From 076f5289b015b29bac135ec98b6b259df37ac57d Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:22:10 -0400 Subject: [PATCH 2/9] docs(plans): outline multi-provider support Draft outline for generalizing routing beyond NeuralWatt: the real coupling is narrow (poller.py's fetch_neuralwatt, and three tangled things in dispatcher.py -- SSE-comment usage parsing, the kWh cost model, and single-account refusal handling), while scoring/tiering/routing/proficiency already operate on provider-agnostic DB rows. Recommends Z.ai as the first test provider (same GLM weights already served via NeuralWatt, making it a same-weights cross-provider comparison rather than a disjoint catalog), with OpenRouter, DeepInfra, Together, and Fireworks as further candidates for cheap prepaid-credit testing. Not decision-complete -- several open questions flagged before this graduates to FINAL. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NFq4QnaQx1CPvoEXoNijk2 --- plans/multi-provider-support.md | 120 ++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 plans/multi-provider-support.md diff --git a/plans/multi-provider-support.md b/plans/multi-provider-support.md new file mode 100644 index 0000000..e080950 --- /dev/null +++ b/plans/multi-provider-support.md @@ -0,0 +1,120 @@ +# Generalizing to multiple providers + +**Status: DRAFT — outline only, not decision-complete.** Written 2026-09-04 +against `main` at `340453e`. Needs a pass to firm up before this is handed to +the opencode/Prometheus pipeline — flagged questions below are the gaps. + +## Why this, why now + +Everything downstream of the catalog — `scoring.py`, `tiering.py`, +`routing.py`, `proficiency.py`, `verification.py`, `feedback.py`, the admin +portal, the TUI — already operates on normalized DB rows with no idea which +provider produced them. `dispatch_providers: dict[str, DispatchProvider]` in +config and the `(model_id, provider)` composite key were deliberately kept +when OpenRouter was dropped (`c3484f0`) for exactly this. The coupling that's +actually left is narrow: + +- **`poller.py`** — `fetch_neuralwatt()` is a bespoke function: hardcoded + URL, NeuralWatt's specific catalog JSON shape, `provider="neuralwatt"` + written as a literal. +- **`dispatcher.py`** — three things tangled together that need separating: + 1. Usage/telemetry parsing (NeuralWatt reports energy via SSE **comment** + lines — a protocol quirk, not an OpenAI standard). + 2. Cost semantics (the $8/kWh-billed-capped-at-3x-list model is + NeuralWatt-specific billing behavior). What `routing.estimated_cost` + actually scores on — catalog price × request shape — is already + provider-agnostic; the kWh math is validation for one provider's + billing quirk, not the load-bearing input. + 3. Account-refusal handling (`degrade to local` assumes one cloud account + — see dispatcher.py ~2537). With N providers this mostly *disappears*: + circuit-break the failing provider's rows and let ranking fail over to + the next-best candidate on a different provider. + +**On re-adding OpenRouter specifically:** it was dropped for a real reason, +not a bad one — the project pivoted to scoring on *measured* billing and +carbon, and OpenRouter (an aggregator) doesn't expose per-request +energy/carbon telemetry the way NeuralWatt does. That's not a reason to +avoid it now — it's the first real test case for a `has_energy_telemetry` +capability flag, since eco/energy needs to degrade gracefully per-provider +rather than assuming every row has NeuralWatt's shape. Worth remembering +before re-proposing it as if it were untried. + +## Constraint: cheap, no-commitment testing + +Budget is small and this is for testing the plumbing, not production spend — +prepaid credits in ~$10 increments, no subscriptions. Candidates, **pricing +and minimums need re-verification before committing to one** (this kind of +thing moves): + +| provider | fit | notes | +|---|---|---| +| Z.ai | best first target | Pay-as-you-go API, OpenAI-compatible (`https://api.z.ai/api/paas/v4` or `/api/openai/v1`), no stated minimum top-up. Three free-tier models (GLM-4.7-Flash, GLM-4.5-Flash, GLM-4.6V-Flash) for zero-cost plumbing tests. **Actually the publisher of the GLM family this catalog already serves via NeuralWatt** (`glm-5.2-fast`, `glm-5.2-flex`) — a same-weights cross-provider comparison, which is a stronger generalization test than an unrelated model set, and a possible way to sanity-check NeuralWatt's `static_fallback` carbon figure for GLM against something else. No response-level energy/carbon telemetry — same capability-flag case as OpenRouter, not a differentiator there. Don't confuse this with the separate "GLM Coding Plan" subscription ($18-168/mo) — that's a different product and not what fits the budget constraint here. | +| OpenRouter | good second | prepaid, no minimum, OpenAI-compatible, many free models for zero-cost plumbing tests, broad catalog overlap (kimi/deepseek/qwen/gemma too, not just GLM), and this repo already has git history (`c3484f0` and its parent) to mine for catalog-normalization shape. No per-request energy/carbon. It's an aggregator of aggregators, so if it ever *does* report grid/energy data, treat it as less trustworthy than a direct provider's own figure. | +| DeepInfra | plausible | prepaid, no subscription, OpenAI-compatible, cheap open-weight catalog. No energy telemetry. | +| Together AI | plausible | prepaid credits, OpenAI-compatible, per-token billing (straightforward vs. NeuralWatt's kWh math). | +| Fireworks AI | plausible | same shape as Together. | +| Groq | maybe later | OpenAI-compatible, free tier + pay-as-you-go, but a much smaller catalog — more useful for a latency-tolerance test than a routing-breadth test. | + +Recommendation: **one provider first** — Z.ai, given the GLM overlap makes it +a more informative test than a disjoint catalog — all the way through +poller → dispatch → a handful of real routed requests, before touching a +second. Confirms the abstraction actually generalizes instead of just +looking like it does on paper. + +## Architecture sketch + +A `Provider` protocol/interface — not fleshed out yet, but the shape: + +- `fetch_catalog() -> list[ModelRow]` +- `parse_usage(response) -> UsageInfo` (cost, tokens, energy if present) +- `detect_refusal(error) -> bool` +- capability flags: `has_energy_telemetry`, `has_regional_carbon` + +`fetch_neuralwatt` and the SSE-comment parser move behind it as the first +concrete implementation. `dispatch_providers` entries get a `type:` +discriminator so config knows which implementation to instantiate. + +## What should need ~zero change + +Scoring, tiering, routing, proficiency, verification, feedback, admin, TUI. +If any of these turn out to need provider-aware branching, that's a sign the +interface boundary is in the wrong place — worth treating as a red flag +during implementation, not a shrug. + +## Open questions + +- **Eco/carbon for a provider with no telemetry.** Exclude those rows from + eco ranking entirely, or treat eco as unweighted/missing for them without + disqualifying them? Affects whether a non-NeuralWatt row can ever win on + the eco axis, or only ever competes on cost/quality. +- **Same model, two providers — does proficiency stay per-model or become + per-(model, provider)?** GLM via Z.ai and `glm-5.2-fast` via NeuralWatt are + the same weights but different serving stacks; scoring/tiering may already + key on `(model_id, provider)` everywhere it needs to, but this is the first + case where that distinction actually matters for a real decision rather + than being schema-only. Verify before assuming it's already handled. +- **Circuit breaker: confirmed provider-generic in prose (CLAUDE.md), not + independently re-verified this session.** Check `circuit_breaker.py` + before assuming per-provider breaking just works. +- **Does the account-refusal-degrades-to-local path actually get simpler or + just get an `if` added?** Stated above as an assumption; worth confirming + against the real code before it's a success criterion. +- **Serving-class suffix parsing (`-flex`/`-fast`/`-short`) is NeuralWatt + catalog convention.** Neither Z.ai nor OpenRouter obviously has an + equivalent — does the tier/latency-tolerance filter treat "no serving + class" as a real, not-missing state? + +## Non-goals (this pass) + +- Don't wire up more than one new provider before the first one round-trips + end to end. +- Don't try to make eco/carbon methodology uniform across providers that + don't expose the same telemetry — graceful absence beats a fabricated + number (same principle as the empty `leaderboards.yaml`). +- Don't rebuild the cost model — catalog-price × shape already generalizes; + confirm that rather than redesigning it. + +## Not yet defined + +Success criteria, test plan, and phasing/milestones — fill in once the open +questions above are resolved and this graduates to FINAL. -- 2.49.1 From ad39db29158fba3b2cf2ae420ea08e95d33c1c63 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:25:08 -0400 Subject: [PATCH 3/9] docs(plans): beef up multi-provider draft with measured coupling surface Counts the coupling instead of asserting it: grep -rn neuralwatt src/*.py returns 20 hardcoded refs across 6 files, tabulated as the actual work list. Three findings the draft did not have: - metrics.py:175 computes catalog staleness over provider='neuralwatt' only, so a second provider whose poller stops would keep the warning green while its catalog froze -- the silent-and-open failure mode again, by a new door. - leaderboard.py:125 writes priors to a hardcoded provider, so a curated prior for a shared model attaches to the NeuralWatt row only. Harmless today (leaderboards.yaml ships empty) but it decides the proficiency-sharing question by accident. - dispatcher.py:2879 resolves a pinned model id against NeuralWatt rows only. Since capability flags fail closed, a pin on a second provider would 422 and read as a capability problem rather than provider scoping. Also adds a risk the draft missed: poller.mark_stale runs only inside main(), which returns early on RequestException, so with two providers one fetch failure can skip staleness marking for the other -- or mark rows stale that were never that provider's. Requires per-provider fetch isolation and a per-provider sanity floor. Closes three open questions by inspection: circuit_breaker IS provider-generic (keyed on the (model_id, provider) tuple), 'no serving class' IS a real state (parse_serving_class returns schema defaults), and proficiency's PK already supports per-provider divergence -- though the propagate_to_variants sharing question survives as a genuine design decision. Corrects the 'needs ~zero change' list, which metrics.py and leaderboard.py already contradict. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- plans/multi-provider-support.md | 117 +++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 17 deletions(-) diff --git a/plans/multi-provider-support.md b/plans/multi-provider-support.md index e080950..a8e7473 100644 --- a/plans/multi-provider-support.md +++ b/plans/multi-provider-support.md @@ -1,8 +1,11 @@ # Generalizing to multiple providers -**Status: DRAFT — outline only, not decision-complete.** Written 2026-09-04 -against `main` at `340453e`. Needs a pass to firm up before this is handed to -the opencode/Prometheus pipeline — flagged questions below are the gaps. +**Status: DRAFT — closer, still not decision-complete.** Written 2026-09-04 +against `main` at `340453e`; coupling surface measured and three open +questions closed by code inspection on the same commit. Remaining gaps before +this can go to the opencode/Prometheus pipeline: the proficiency-sharing +decision (a judgement call, not a lookup), the eco-without-telemetry decision, +and the still-empty success criteria / test plan / phasing. ## Why this, why now @@ -48,7 +51,7 @@ thing moves): | provider | fit | notes | |---|---|---| -| Z.ai | best first target | Pay-as-you-go API, OpenAI-compatible (`https://api.z.ai/api/paas/v4` or `/api/openai/v1`), no stated minimum top-up. Three free-tier models (GLM-4.7-Flash, GLM-4.5-Flash, GLM-4.6V-Flash) for zero-cost plumbing tests. **Actually the publisher of the GLM family this catalog already serves via NeuralWatt** (`glm-5.2-fast`, `glm-5.2-flex`) — a same-weights cross-provider comparison, which is a stronger generalization test than an unrelated model set, and a possible way to sanity-check NeuralWatt's `static_fallback` carbon figure for GLM against something else. No response-level energy/carbon telemetry — same capability-flag case as OpenRouter, not a differentiator there. Don't confuse this with the separate "GLM Coding Plan" subscription ($18-168/mo) — that's a different product and not what fits the budget constraint here. | +| Z.ai | best first target | Pay-as-you-go API, OpenAI-compatible (`https://api.z.ai/api/paas/v4` or `/api/openai/v1`), no stated minimum top-up. Three free-tier models (GLM-4.7-Flash, GLM-4.5-Flash, GLM-4.6V-Flash) for zero-cost plumbing tests. **Actually the publisher of the GLM family this catalog already serves via NeuralWatt** (`glm-5.2-fast`, `glm-5.2-flex`) — a same-weights cross-provider comparison, which is a stronger generalization test than an unrelated model set, and a way to sanity-check NeuralWatt's `static_fallback` carbon figure for GLM. That is not hypothetical: `CLAUDE.md` records the GLM rows reporting `grid_id: FI` at 475 gCO2/kWh as `carbon_source: static_fallback` — a substituted constant, not a measurement — and they are currently EXCLUDED from eco scoring for that reason. A second provider serving the same weights is a free experiment on one of this project's standing open questions. No response-level energy/carbon telemetry — same capability-flag case as OpenRouter, not a differentiator there. Don't confuse this with the separate "GLM Coding Plan" subscription ($18-168/mo) — that's a different product and not what fits the budget constraint here. | | OpenRouter | good second | prepaid, no minimum, OpenAI-compatible, many free models for zero-cost plumbing tests, broad catalog overlap (kimi/deepseek/qwen/gemma too, not just GLM), and this repo already has git history (`c3484f0` and its parent) to mine for catalog-normalization shape. No per-request energy/carbon. It's an aggregator of aggregators, so if it ever *does* report grid/energy data, treat it as less trustworthy than a direct provider's own figure. | | DeepInfra | plausible | prepaid, no subscription, OpenAI-compatible, cheap open-weight catalog. No energy telemetry. | | Together AI | plausible | prepaid credits, OpenAI-compatible, per-token billing (straightforward vs. NeuralWatt's kWh math). | @@ -74,6 +77,69 @@ A `Provider` protocol/interface — not fleshed out yet, but the shape: concrete implementation. `dispatch_providers` entries get a `type:` discriminator so config knows which implementation to instantiate. +## Measured coupling surface + +Claims about how narrow the coupling is should be counted, not asserted. +`grep -rn neuralwatt src/*.py` on `main` at `340453e` returns **20 hardcoded +references across 6 files**: + +| file | refs | what they are | +|---|---|---| +| `poller.py` | 8 | the bespoke fetch: URL, `fetch_neuralwatt()`, literal `provider="neuralwatt"`, the sanity-floor count query, log prefixes | +| `eval_proficiency.py` | 5 | `provider: str = "neuralwatt"` defaults, the judge's `dispatch_providers["neuralwatt"]` lookup, and an `identity["provider"] != "neuralwatt"` skip | +| `dispatcher.py` | 4 | passthrough default, and a capability lookup (see below) | +| `metrics.py` | 1 | catalog-staleness query | +| `leaderboard.py` | 1 | `set_leaderboard(..., "neuralwatt", ...)` | +| `seed_energy.py` | 1 | `dispatch_providers["neuralwatt"]` | + +That is the actual work list. Two entries deserve calling out because they +**contradict the "needs ~zero change" section below.** + +### `metrics.py:175` — catalog staleness would ignore a second provider + +```sql +SELECT MAX(last_updated) AS last_updated FROM models WHERE provider='neuralwatt' +``` + +The staleness warning is computed over NeuralWatt rows only. Add a provider +whose poller silently stops and the warning stays green while its catalog +freezes — the exact silent-and-open failure `CLAUDE.md`'s "Run as a service" +section describes, now with a second way in. `metrics.py` is listed under +"needs ~zero change"; it does not. + +### `leaderboard.py:125` — priors are written to a hardcoded provider + +`set_leaderboard(conn, cfg, model_id, "neuralwatt", category, score)` pins the +provider literal, so a curated prior for a shared model (GLM, say) attaches to +the NeuralWatt row and never to the Z.ai one. `leaderboards.yaml` ships empty +today so nothing is broken yet, but this decides the answer to the +proficiency-sharing question above by accident rather than on purpose. + +### `dispatcher.py:2879` — pinned-model capability check is provider-scoped + +```sql +SELECT 1 FROM models WHERE model_id = ? AND provider = 'neuralwatt' +``` + +`_check_pinned_capabilities` resolves a pinned id against NeuralWatt rows only. +A client pinning a Z.ai model id would find no row. Given capability flags +**fail closed** by design (an unconfirmed capability is treated as absent), +that is a 422 on a pin that should have worked — and it will read as a +capability problem, not a provider-scoping one. + +### Risk not in the draft: per-provider fetch isolation + +`poller.mark_stale` runs only inside `poller.main()`, and `main()` returns +early on `RequestException` — **before** `upsert` and **before** `mark_stale`. +With two providers in one run, a failure fetching provider B can skip +`mark_stale` for provider A entirely, or a partial/empty `data` array from B +can mark rows stale that were never B's. `docs/incidents.md` records the +empty-`data` path as the one that can empty the candidate set. + +Requirement: **each provider's fetch, upsert and staleness marking must be +isolated.** One provider's outage must not affect another's rows in either +direction, and the sanity floor (`poller.py:420`) must be per-provider. + ## What should need ~zero change Scoring, tiering, routing, proficiency, verification, feedback, admin, TUI. @@ -81,28 +147,45 @@ If any of these turn out to need provider-aware branching, that's a sign the interface boundary is in the wrong place — worth treating as a red flag during implementation, not a shrug. +**Two of them already fail that test**, per the inventory above: `metrics.py` +hardcodes the provider in its staleness query and `leaderboard.py` hardcodes it +when writing priors. Neither is a deep coupling — both are literals, not +branching — but the list should be read as "should need ~zero change *after* +those two literals are parameterised", not as a claim that they are already +clean. The red flag to watch for during implementation is provider-aware +*logic* appearing in these modules; a hardcoded string is a different and much +cheaper problem. + ## Open questions - **Eco/carbon for a provider with no telemetry.** Exclude those rows from eco ranking entirely, or treat eco as unweighted/missing for them without disqualifying them? Affects whether a non-NeuralWatt row can ever win on the eco axis, or only ever competes on cost/quality. -- **Same model, two providers — does proficiency stay per-model or become - per-(model, provider)?** GLM via Z.ai and `glm-5.2-fast` via NeuralWatt are - the same weights but different serving stacks; scoring/tiering may already - key on `(model_id, provider)` everywhere it needs to, but this is the first - case where that distinction actually matters for a real decision rather - than being schema-only. Verify before assuming it's already handled. -- **Circuit breaker: confirmed provider-generic in prose (CLAUDE.md), not - independently re-verified this session.** Check `circuit_breaker.py` - before assuming per-provider breaking just works. +- **Same model, two providers — per-model or per-(model, provider)?** + PARTLY RESOLVED, and the remaining half is a design decision rather than a + verification. The schema already supports divergence: `proficiency`'s + primary key is `(model_id, provider, category)`. But + `propagate_to_variants` copies scores across variants keyed on + `base_model_id`, on the stated principle that proficiency is "a property of + the weights, not the queue". Same-weights-different-serving-stack is exactly + the case that principle does not decide: quantization and serving + differences could justify separate scores, while the existing logic would + share one. **Decide this explicitly before implementation** — it is the one + question here that code inspection cannot answer. +- **RESOLVED — circuit breaker is genuinely provider-generic.** Verified in + code, not prose: `is_down`, `record_failure` and `record_success` all take + `(model_id, provider)` and `_store` is keyed on that tuple + (`src/circuit_breaker.py:29,37,57`). Per-provider breaking works today with + no change. - **Does the account-refusal-degrades-to-local path actually get simpler or just get an `if` added?** Stated above as an assumption; worth confirming against the real code before it's a success criterion. -- **Serving-class suffix parsing (`-flex`/`-fast`/`-short`) is NeuralWatt - catalog convention.** Neither Z.ai nor OpenRouter obviously has an - equivalent — does the tier/latency-tolerance filter treat "no serving - class" as a real, not-missing state? +- **RESOLVED — "no serving class" is already a real state.** + `poller.parse_serving_class` returns the schema defaults + (`'standard'`, `'default'`, `'full'`) for a base id with no suffix, so a + Z.ai or OpenRouter row lands `standard` and is interactive-eligible rather + than null. No change needed. ## Non-goals (this pass) -- 2.49.1 From 17d7b72c37bd22902f484cf905e108408b078b68 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:29:23 -0400 Subject: [PATCH 4/9] feat(config): gitignored config.local.yaml overlay WIP checkpoint committed by Claude while Atlas was still in its final verification wave. Committed early deliberately: 11 files were sitting uncommitted with two of them untracked, and untracked files in this tree have been destroyed twice today by agent git cleanup -- the user's multi-provider-support plan and a config.local.yaml holding their tariff. Protecting the work cost nothing; losing it would have cost a whole plan. Deployment-specific values previously lived as an UNCOMMITTED modification to the tracked config/config.yaml. That arrangement failed seven times in one session: three agent checkout/stash/restore calls, two `git commit -am` sweeps that each needed a history rewrite, one ordinary branch switch, and one cleanup during this very plan. Once a value is correctly absent from git, every checkout, switch, pull and rebase wipes it -- that is the intended fix behaving as designed, which is what makes the arrangement itself the bug. load_config now deep-merges an optional config/config.local.yaml over the base before validation, so every consumer inherits it (poller, feedback, eval_proficiency, seed_local_dispatch_energy, admin, dispatcher). Mappings deep-merge; lists replace wholesale; the MERGED result is validated once so extra="forbid" still catches an overlay typo. The admin portal now writes to the overlay and never to config/config.yaml. An operator changing a knob in a loopback-only portal is making a local operational decision, not a project decision -- someone changing a project default edits config.yaml and commits it through git. This also removes the shadowing trap by construction rather than guarding against it. config/config.yaml stops being dirty in normal operation, which removes the condition behind every clobbering above. NOTE: final verification wave had not reported when this was committed. The suite was green at 1155 before it started; Atlas may amend or add commits on top. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- .gitignore | 2 + CLAUDE.md | 4 +- README.md | 3 +- admin/frontend/controls.html | 22 ++- config/config.local.yaml.example | 16 +++ docs/config-local-overlay.md | 33 +++++ src/admin.py | 223 ++++++++++++++++++++++++++--- src/config.py | 44 ++++++ tests/test_admin_config.py | 231 ++++++++++++++++++++++++++----- tests/test_admin_frontend.py | 13 ++ tests/test_config.py | 93 ++++++++++++- 11 files changed, 621 insertions(+), 63 deletions(-) create mode 100644 config/config.local.yaml.example create mode 100644 docs/config-local-overlay.md diff --git a/.gitignore b/.gitignore index 4de9287..f8c5b95 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,8 @@ router.log venv/ .omo/ config.yaml.bak.* +config/config.local.yaml +config/config.local.yaml.bak.* node_modules/ package.json package-lock.json diff --git a/CLAUDE.md b/CLAUDE.md index 7ec66ad..bfc9930 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -217,7 +217,7 @@ rather than from months of history. - `tui.py` — Textual dashboard over `/metrics` + `/events/decisions`; live feed, category→model panel, detail popup; data layer split into `tui_model.py`. [architecture](docs/architecture.md). - `router_cli.py` — one-shot `/route` probe (no spend), raw JSON with `--json`. [api](docs/api.md). - `admin.py` / `config/admin_schema.sql` / `admin/frontend/*.html` — loopback `/admin` portal: dashboard, models overrides, decisions log, controls. [admin-portal](docs/admin-portal.md). -- `tests/` — 1020 tests across 41 files, offline, verified on Python 3.10 and 3.14. [README](README.md). +- `tests/` — 1155 tests across 40+ files, offline, verified on Python 3.10 and 3.14. [README](README.md). ## Proficiency: category now changes routing @@ -744,7 +744,7 @@ in the same pass — it was declared, never read, and shadowed the ## Setup -Full install steps (venv, deps, config, first run) in [README ## Installation](README.md#installation). Set `classifier.model`/`base_url` and `objective.plan_kwh_per_period` in `config/config.yaml` (README config table). Model tags (`num_ctx`) + `verification.model` same-tag note in [docs/local-models.md](docs/local-models.md). Requirements are pinned — bump deliberately (README). +Full install steps (venv, deps, config, first run) in [README ## Installation](README.md#installation). Host-local deployment values go in `config/config.local.yaml` (gitignored overlay) — `classifier.model`/`base_url`, `local_energy.*`, host-specific URLs. General defaults in `config/config.yaml` stay shareable. `objective.plan_kwh_per_period` in the README config table. Model tags (`num_ctx`) + `verification.model` same-tag note in [docs/local-models.md](docs/local-models.md). Requirements are pinned — bump deliberately (README). ## Run as a service diff --git a/README.md b/README.md index d7e4b46..6e37083 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,8 @@ Nothing else is assumed about the host — routing itself is SQLite and arithmet python -m venv .venv && source .venv/bin/activate pip install -r requirements.txt sqlite3 router.db < config/schema.sql -cp .env.example .env # fill in NEURALWATT_API_KEY (.env stays at repo root) +cp .env.example .env # fill in NEURALWATT_API_KEY (.env stays at repo root) +cp config/config.local.yaml.example config/config.local.yaml # optional: deployment-specific overlay (gitignored) PYTHONPATH=src python -m poller # populate the catalog PYTHONPATH=src python -m tier # resolve tiers PYTHONPATH=src python -m config # sanity-check config loads diff --git a/admin/frontend/controls.html b/admin/frontend/controls.html index c69a564..1d62059 100644 --- a/admin/frontend/controls.html +++ b/admin/frontend/controls.html @@ -261,7 +261,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important}
-

Allowlisted keys, written to config.yaml.

+

Allowlisted keys, written to config/config.local.yaml (machine-local overlay; config.yaml stays clean).

Loading config…
@@ -516,9 +516,13 @@ function renderConfig(config) { list.innerHTML = '
No config data
'; return; } - const html = Object.entries(config).map(([key, val]) => { + const html = Object.entries(config).map(([key, raw]) => { + const entry = raw || {}; + const val = entry.value; + const source = entry.source; + const isBool = typeof val === 'boolean'; let control; - if (typeof val === 'boolean') { + if (isBool) { // A switch, matching Runtime Knobs — the bare checkbox here was the one // control on the page that didn't look like the others. control = `
@@ -531,9 +535,19 @@ function renderConfig(config) { } else { control = ``; } + + // Provenance badge is legitimate meta: it explains where the effective + // value came from without restating the value the control already shows. + let meta = UNITS[key] ? escapeHtml(UNITS[key]) : ''; + if (source === 'overlay') { + meta += `${meta ? ' ' : ''}overlay`; + } else if (source === 'base') { + meta += `${meta ? ' ' : ''}base`; + } + return settingRow({ key, - meta: UNITS[key] ? escapeHtml(UNITS[key]) : '', + meta, control, dirtyAttrs: ` data-key="${escapeHtml(key)}" data-orig="${escapeHtml(String(val === null ? '' : val))}"`, }); diff --git a/config/config.local.yaml.example b/config/config.local.yaml.example new file mode 100644 index 0000000..d190eca --- /dev/null +++ b/config/config.local.yaml.example @@ -0,0 +1,16 @@ +# config.local.yaml — machine-local overrides +# +# This file is gitignored. It takes precedence over config/config.yaml when +# load_config() merges the overlay, so place deployment-specific or personal +# secrets and knobs here instead of editing the tracked config.yaml: +# - local_energy.enabled + local_energy.tariff_usd_per_kwh +# - host-specific classifier.base_url / verification.base_url +# - any value you change between machines but want to keep in the repo default +# +# Only the keys you want to override need to be present — the rest come from +# config/config.yaml. + +# Example: enable local energy metering on your dev machine. +# local_energy: +# enabled: true +# tariff_usd_per_kwh: 0.159 diff --git a/docs/config-local-overlay.md b/docs/config-local-overlay.md new file mode 100644 index 0000000..26ece06 --- /dev/null +++ b/docs/config-local-overlay.md @@ -0,0 +1,33 @@ +# Local Overlay (`config/config.local.yaml`) + +`config.local.yaml` is a gitignored, deep-merged overlay on top of the +tracked `config/config.yaml`. Anything placed in the overlay takes +precedence when `load_config()` merges the two files — the overlay only +needs the keys you want to change; everything else falls through to the +base. + +## What belongs in the overlay + +Values that are specific to your machine or deployment and would turn +`config/config.yaml` dirty if tracked: + +- `local_energy.enabled` + `local_energy.tariff_usd_per_kwh` — metering + per-machine, tariff often differs between sites. +- `classifier.base_url` / `verification.base_url` — Ollama may live on a + different host or VPN address on each box. +- Any tuning knob you change between machines (`num_ctx` overrides, + per-host profile tweaks). + +## What does NOT belong here + +**Secrets.** Secrets (`NEURALWATT_API_KEY`, etc.) stay in `.env`. The +overlay is YAML loaded as configuration, not as a credential store. + +## A dirty `config/config.yaml` is a smell + +If your working copy of `config/config.yaml` is modified, someone edited +the shared defaults by hand. Move those edits into a new entry in +`config.local.yaml` — the base file should stay clean enough to commit +and share. + +See `config/config.local.yaml.example` for concrete key examples. diff --git a/src/admin.py b/src/admin.py index 56513ca..cc695fc 100644 --- a/src/admin.py +++ b/src/admin.py @@ -302,9 +302,11 @@ _FLEX_VALUES = frozenset(v.value for v in FlexPreference) # Dotted config.yaml paths an operator is allowed to edit. Everything else — # classifier/verification/local_vision URLs and model names, api_key_env, # provider blocks, dispatch_providers, and secrets — is deliberately OFF this -# list. Editing works only on the named scalars. Writes go through -# comment-preserving ruamel.yaml round-trip and are validated via ``RouterConfig`` -# before any byte reaches disk; a backup is made first. +# list. Editing works only on the named scalars. Writes are persisted to +# ``config.local.yaml`` (the machine-local overlay) via a comment-preserving +# ruamel.yaml round-trip; whole-config validation via ``RouterConfig`` happens +# on the merged (base + overlay) result before any byte reaches disk; a backup +# of the overlay file precedes the write. _CONFIG_ALLOWLIST: dict[str, tuple[str, ...]] = { "logging.level": ("logging", "level"), "objective.quality_tolerance": ("objective", "quality_tolerance"), @@ -430,11 +432,169 @@ def _persist_config_value( """Atomically persist *value* at dotted *path* in *config_path*. Thin wrapper around ``_persist_config_block`` preserving the historic - scalar-path signature used by tests. + scalar-path signature used by tests. On the overlay path this writes + to ``config.local.yaml`` (the machine-local gitignored file). """ _persist_config_block(config_path, path, value) +# --- overlay helpers ---------------------------------------------------------- + +_YAML_HEADER = ( + "# Machine-local overlay — gitignored, never committed.\n" + "# Written by the admin portal (POST /admin/api/config/*).\n" +) + + +def _persist_to( + base_path: Path, + overlay_path: Path, + store_path: tuple[str, ...], + value: Any, +) -> None: + """Persist *value* at *store_path* inside *overlay_path*, creating the file on first write. + + The overlay file is created with a header comment if it does not yet exist. + Unlike ``_persist_config_block`` (which preserves the base file's comments), + the overlay starts fresh — no pre-existing comments to keep. + + Whole-config validation via ``RouterConfig`` happens on the **merged** + (base + in-memory overlay with the requested mutation) before any byte + reaches disk; a backup of the overlay file precedes the write; and + ``.tmp`` + ``os.replace`` provides an atomic swap. + """ + with _config_write_lock: + existed = overlay_path.exists() + if existed: + store = load_config_store(overlay_path) # CommentedMap | None + else: + store = CommentedMap() + if store is None: + store = CommentedMap() + + cur = store + for part in store_path[:-1]: + nxt = cur.get(part) + if nxt is None: + nxt = CommentedMap() + cur[part] = nxt + cur = nxt + cur[store_path[-1]] = value + + # Validate the MERGED config (base + intended overlay) before writing. + merged = _load_merged_config_store( + base_path, overlay_path, overlay_override=store + ) + RouterConfig(**merged) + + # Validation passed: create the overlay file now if this is the first + # write, so there is a pre-modification state to back up. + if not existed: + overlay_path.write_text(_YAML_HEADER) + + # Backup the overlay (pre-modification), then atomically swap. + backup = overlay_path.with_name( + f"{overlay_path.name}.bak.{int(time.time())}" + ) + if overlay_path.exists(): + shutil.copyfile(overlay_path, backup) + + # Atomic write. A fresh overlay gets the machine-local header comment; + # on later writes the header is already part of the CommentedMap and + # survives the ruamel round-trip, so we only prepend it explicitly when + # we are creating the file from scratch. + tmp = overlay_path.with_suffix(overlay_path.suffix + ".tmp") + with tmp.open("w") as fh: + if not existed: + fh.write(_YAML_HEADER) + YAML().dump(store, fh) + os.replace(tmp, overlay_path) + + +def _load_merged_config_store( + base_path: Path, + overlay_path: Path, + *, + overlay_override: Optional[dict[str, Any]] = None, +) -> dict[str, Any]: + """Load and merge *base_path* with *overlay_path*, returning a plain dict. + + The overlay values take precedence; any key in the overlay overrides the + corresponding base key. Both files are loaded via ruamel.CommentedMap + so existing comments survive the GET display. + + When *overlay_override* is supplied, it is used instead of loading + *overlay_path*; this lets a write validate against the in-memory mutated + overlay before any byte reaches disk. + + If the overlay does not exist and no override is provided, returns a + plain-copy of the base store that can be iterated by ``_CONFIG_ALLOWLIST`` + paths. + """ + try: + base = load_config_store_safe(base_path) or {} + except HTTPException: + # Corrupt base — surface a clean error. + raise + if overlay_override is not None: + overlay = overlay_override + else: + try: + overlay = load_config_store(overlay_path) or {} + except YAMLError: + raise HTTPException( + status_code=503, + detail=( + "config.local.yaml could not be parsed — " + "fix the overlay file or delete it to recover" + ), + ) + except FileNotFoundError: + # Overlay does not exist yet — treat as empty. + overlay = {} + + # Deep-merge overlay into base (same semantics as config._merge_overlay). + merged: dict[str, Any] = {} + for key, base_val in base.items(): + if ( + key in overlay + and isinstance(base_val, dict) + and isinstance(overlay[key], dict) + ): + # Recursively merge nested dicts. + rec: dict[str, Any] = {} + ov = overlay[key] + for sk, sv in base_val.items(): + if sk in ov and isinstance(sv, dict) and isinstance(ov[sk], dict): + # Flatten: reuse merged parents from outer level. + rec[sk] = _deep_merge_dicts(sv, ov[sk]) + else: + rec[sk] = ov.get(sk, sv) + merged[key] = rec + else: + # Scalars, lists, or overlay-only keys: overlay wins. + merged[key] = overlay.get(key, base_val) + # Insert overlay-only keys not in base. + for key, ov_val in overlay.items(): + if key not in merged: + merged[key] = ov_val + return merged + + +def _deep_merge_dicts(base: dict, overlay: dict) -> dict: + """Recursively merge *overlay* into *base*, overlay winning on conflicts.""" + result = {} + for key, bv in base.items(): + if key in overlay and isinstance(bv, dict) and isinstance(overlay[key], dict): + result[key] = _deep_merge_dicts(bv, overlay[key]) + else: + result[key] = overlay.get(key, bv) + for key, ov in overlay.items(): + if key not in result: + result[key] = ov + return result + + class _AvailabilityBody(BaseModel): availability: str reason: Optional[str] = None @@ -506,8 +666,9 @@ def build_router( """Build the admin APIRouter bound to the caller's config and DB factory. ``base_dir`` is the repo root (the directory holding config.yaml and the - maintenance scripts). The persisted-config endpoints use it to locate - ``config.yaml`` for comment-preserving writes; the maintenance triggers use + maintenance scripts). The persisted-config endpoints (allowlisted scalars) + read from ``config.yaml`` but write to ``config.local.yaml``; profile CRUD + endpoints write to ``config.yaml`` directly. The maintenance triggers use it as their spawn CWD. Defaults to the repo root (parent of ``src/admin.py``), so the router is portable and testable without an explicit base_dir. """ @@ -517,6 +678,7 @@ def build_router( if base_dir is not None else _REPO_ROOT / "config" / "config.yaml" ) + config_local_path = config_path.with_name("config.local.yaml") router = APIRouter() _admin_frontend = _REPO_ROOT / "admin" / "frontend" / "index.html" @@ -800,13 +962,15 @@ def build_router( status_code=403, detail="built-in profiles are read-only", ) - # Read the CURRENT persisted default (the running cfg may be stale - # relative to the file) so the refusal names the setting explicitly. - store = load_config_store_safe(config_path) or {} - if name not in (store.get("profiles") or {}): + # Read the EFFECTIVE default from merged base + overlay so an admin + # cannot delete a profile that is currently the default, regardless of + # whether the default was set in config.yaml or config.local.yaml. + merged = _load_merged_config_store(config_path, config_local_path) + persisted_profiles = merged.get("profiles") or {} + if name not in persisted_profiles: raise HTTPException(status_code=404, detail="profile not found") raw_default = _dict_get_at( - store, _CONFIG_ALLOWLIST["routing.default_profile"] + merged, _CONFIG_ALLOWLIST["routing.default_profile"] ) if raw_default == name: raise HTTPException( @@ -1219,21 +1383,46 @@ def build_router( @router.get("/api/config") def admin_config_get() -> dict: - """The allowlisted config.yaml values, as {dotted_key: value}.""" - return {key: _dict_get_at(load_config_store_safe(config_path), path) - for key, path in _CONFIG_ALLOWLIST.items()} + """The allowlisted config values with provenance. + + Each key maps to ``{"value": …, "source": "base" | "overlay"}`` + where *overlay* means the key exists in ``config.local.yaml`` and + *base* means it comes only from ``config.yaml``. + """ + overlay_exists = config_local_path.exists() + overlay_raw = None + if overlay_exists: + try: + overlay_raw = load_config_store(config_local_path) + except YAMLError: + pass # Corrupt overlay — source all from base. + source_map: dict[str, str] = {} + for key, path in _CONFIG_ALLOWLIST.items(): + try: + _dict_get_at(overlay_raw or {}, path) + source_map[key] = "overlay" + except (KeyError, TypeError): + source_map[key] = "base" + merged = _load_merged_config_store(config_path, config_local_path) + return { + key: {"value": _dict_get_at(merged, path), "source": source_map[key]} + for key, path in _CONFIG_ALLOWLIST.items() + } @router.post("/api/config/{key}") def admin_config_write(key: str, body: _ValueBody) -> dict: - """Persist one allowlisted value to config.yaml (comment-preserving).""" + """Persist one allowlisted value to the local overlay file.""" if key not in _CONFIG_ALLOWLIST: raise HTTPException( status_code=403, detail=f"config key is not editable: {key}", ) try: - _persist_config_value( - config_path, _CONFIG_ALLOWLIST[key], body.value + _persist_to( + config_path, + config_local_path, + _CONFIG_ALLOWLIST[key], + body.value, ) except ValidationError as exc: raise HTTPException( diff --git a/src/config.py b/src/config.py index 1d1771a..5327439 100644 --- a/src/config.py +++ b/src/config.py @@ -997,11 +997,55 @@ class RouterConfig(StrictModel): return self +def _merge_overlay(base: dict, overlay: dict) -> dict: + """Deep-merge *overlay* on top of *base* and return the merged dict. + + Merge semantics: + + * **Mappings** (``dict``): merged recursively, key by key. Keys present + only in *overlay* are inserted; keys only in *base* are preserved. + * **Lists** (``list``): replaced wholesale — the overlay list entirely + overwrites the base list at that key. Lists are never concatenated. + * **Scalars**: overlay value wins. + + An absent overlay file leaves base unchanged, so this helper is + strictly additive. The merged dict is validated once by the caller + so that unknown keys in the overlay surface as Pydantic errors. + + Precedence chain (highest → lowest): environment variables > + overlay > base (the base file). + """ + result = {} + # Start with all base keys + for key, base_val in base.items(): + if ( + key in overlay + and isinstance(base_val, dict) + and isinstance(overlay[key], dict) + ): + # Both sides are dicts → recurse + result[key] = _merge_overlay(base_val, overlay[key]) + else: + # Scalars, lists, or overlay-only keys: overlay wins (or base if absent) + result[key] = overlay.get(key, base_val) + # Insert overlay-only keys that had no counterpart in base + for key, overlay_val in overlay.items(): + if key not in result: + result[key] = overlay_val + return result + + def load_config(path: str | Path = "config/config.yaml") -> RouterConfig: path = Path(path) if not path.exists(): raise FileNotFoundError(f"Config file not found: {path}") raw = yaml.safe_load(path.read_text()) + + local_path = path.with_name("config.local.yaml") + if local_path.exists(): + local_raw = yaml.safe_load(local_path.read_text()) + raw = _merge_overlay(raw, local_raw) + return RouterConfig(**raw) diff --git a/tests/test_admin_config.py b/tests/test_admin_config.py index bc38b02..3f84fee 100644 --- a/tests/test_admin_config.py +++ b/tests/test_admin_config.py @@ -1,13 +1,15 @@ """Tests for the /admin/api persisted-config endpoints. -``admin.py`` exposes ``GET /admin/api/config`` (the allowlisted config.yaml -values) and ``POST /admin/api/config/{key}`` (persist one allowlisted value to -config.yaml with comment-preserving ruamel.yaml round-trip, a backup copy, and -whole-config validation via ``RouterConfig`` BEFORE anything touches disk). +``admin.py`` exposes ``GET /admin/api/config`` (the allowlisted values with +provenance, ``{value, source}``) and ``POST /admin/api/config/{key}`` (persist +one allowlisted value to ``config.local.yaml`` with comment-preserving +ruamel.yaml round-trip, a backup copy, and whole-config validation of the +merged base + overlay via ``RouterConfig`` BEFORE anything touches disk). Each test builds its own isolated router against a temp copy of config.yaml by passing ``base_dir`` (a temp dir) to ``build_router`` — the real repo -``config.yaml`` is never written. It mounts the router on a fresh FastAPI +``config.yaml`` is never written. The overlay endpoints create and mutate +``/config/config.local.yaml``. It mounts the router on a fresh FastAPI TestClient at ``prefix="/admin"``. """ @@ -16,6 +18,7 @@ from __future__ import annotations import re import shutil import sqlite3 +import subprocess import threading from pathlib import Path @@ -91,14 +94,18 @@ def test_config_GET_returns_allowlisted_values(client): "routing.default_flex_preference", ): assert key in body - assert body["logging.level"] == "info" - assert body["objective.quality_tolerance"] == 0.10 - assert body["routing.default_flex_preference"] == "auto" + assert body["logging.level"] == {"value": "info", "source": "base"} + assert body["objective.quality_tolerance"] == {"value": 0.10, "source": "base"} + assert body["routing.default_flex_preference"] == { + "value": "auto", + "source": "base", + } def test_config_POST_preserves_comments_and_changes_value(client): """A valid write keeps the file's comments AND updates the value.""" tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") _insert_sentinel(config_yaml) resp = tc.post("/admin/api/config/logging.level", json={"value": "warning"}) @@ -108,9 +115,14 @@ def test_config_POST_preserves_comments_and_changes_value(client): assert body["value"] == "warning" assert "restart is required" in body["message"] - text = config_yaml.read_text() - assert _SENTINEL in text - assert re.search(r"^\s*level:\s*warning\s*$", text, re.MULTILINE) is not None + # The base file is untouched; the overlay now carries the changed value. + base_text = config_yaml.read_text() + assert _SENTINEL in base_text + assert re.search(r"^\s*level:\s*info\s*$", base_text, re.MULTILINE) is not None + local_text = local_yaml.read_text() + assert re.search( + r"^\s*level:\s*warning\s*$", local_text, re.MULTILINE + ) is not None def test_config_POST_rejects_non_allowlisted_key(client): @@ -123,15 +135,19 @@ def test_config_POST_rejects_non_allowlisted_key(client): def test_config_POST_invalid_value_leaves_file_unchanged(client): """quality_tolerance=1.5 (>1) fails RouterConfig validation -> 422, no write.""" tc, config_yaml = client - before = config_yaml.read_text() + before_base = config_yaml.read_text() + local_yaml = config_yaml.with_name("config.local.yaml") + before_local = local_yaml.read_text() if local_yaml.exists() else "" resp = tc.post( "/admin/api/config/objective.quality_tolerance", json={"value": 1.5} ) assert resp.status_code == 422 - after = config_yaml.read_text() - assert after == before + assert config_yaml.read_text() == before_base + assert ( + local_yaml.read_text() if local_yaml.exists() else "" + ) == before_local def test_config_GET_includes_routing_default_profile(client): @@ -141,12 +157,16 @@ def test_config_GET_includes_routing_default_profile(client): assert resp.status_code == 200 body = resp.json() assert "routing.default_profile" in body - assert body["routing.default_profile"] == "default" + assert body["routing.default_profile"] == { + "value": "default", + "source": "base", + } def test_config_POST_default_profile_persists_valid_builtin(client): """POST routing.default_profile=batch persists and keeps comments.""" tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") _insert_sentinel(config_yaml) resp = tc.post( @@ -158,17 +178,23 @@ def test_config_POST_default_profile_persists_valid_builtin(client): assert body["value"] == "batch" assert "restart is required" in body["message"] - text = config_yaml.read_text() - assert _SENTINEL in text + # Base comments preserved; value lives in overlay. + base_text = config_yaml.read_text() + assert _SENTINEL in base_text + local_text = local_yaml.read_text() assert re.search( - r'^\s*default_profile:\s*["\']?batch["\']?\s*$', text, re.MULTILINE + r'^\s*default_profile:\s*["\']?batch["\']?\s*$', local_text, re.MULTILINE ) is not None -def test_config_POST_default_profile_rejects_unknown_and_leaves_file_unchanged(client): - """Unknown default_profile returns 422 and does not touch config.yaml.""" +def test_config_POST_default_profile_rejects_unknown_and_leaves_overlay_unchanged( + client, +): + """Unknown default_profile returns 422 and does not touch config.local.yaml.""" tc, config_yaml = client - before = config_yaml.read_text() + local_yaml = config_yaml.with_name("config.local.yaml") + before_base = config_yaml.read_text() + before_local = local_yaml.read_text() if local_yaml.exists() else "" resp = tc.post( "/admin/api/config/routing.default_profile", @@ -179,25 +205,29 @@ def test_config_POST_default_profile_rejects_unknown_and_leaves_file_unchanged(c assert "nosuchprofile" in detail assert "default" in detail or "batch" in detail - after = config_yaml.read_text() - assert after == before + assert config_yaml.read_text() == before_base + assert ( + local_yaml.read_text() if local_yaml.exists() else "" + ) == before_local -def test_config_POST_creates_backup_before_write(client, tmp_path): - """A successful write produces a ``config.yaml.bak.`` backup copy.""" +def test_config_POST_creates_overlay_backup_before_write(client, tmp_path): + """A successful write produces a ``config.local.yaml.bak.`` backup copy.""" tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") _insert_sentinel(config_yaml) resp = tc.post("/admin/api/config/circuit_breaker.enabled", json={"value": True}) assert resp.status_code == 200 - backups = sorted(tmp_path.glob("config/config.yaml.bak.*")) + backups = sorted(tmp_path.glob("config/config.local.yaml.bak.*")) assert len(backups) == 1 - # The backup captured the sentinel comment and the PRE-write value. + # The backup captured the pre-write empty overlay (just the header). backup_text = backups[0].read_text() - assert _SENTINEL in backup_text - backup_yaml = yaml.safe_load(backup_text) - assert backup_yaml["circuit_breaker"]["enabled"] is True + assert backup_text.strip() != "" + # The current overlay carries the new value. + local_text = local_yaml.read_text() + assert yaml.safe_load(local_text)["circuit_breaker"]["enabled"] is True def test_config_concurrent_writes_are_atomic_no_zero_byte_backups(tmp_path): @@ -246,8 +276,10 @@ def test_config_concurrent_writes_are_atomic_no_zero_byte_backups(tmp_path): assert b.read_text().strip() != "" -def test_profile_create_preserves_comments_and_user_local_energy(client): - """A profile CRUD write keeps sentinel comments and the user's local_energy block.""" +def test_profile_create_preserves_comments_and_does_not_create_overlay( + client, tmp_path +): + """A profile CRUD write keeps sentinel comments and never touches config.local.yaml.""" tc, config_yaml = client _insert_sentinel(config_yaml) @@ -262,12 +294,8 @@ def test_profile_create_preserves_comments_and_user_local_energy(client): text = config_yaml.read_text() assert _SENTINEL in text - assert re.search( - r"^\s*tariff_usd_per_kwh:\s*0\.159\s*$", text, re.MULTILINE - ) is not None - assert re.search( - r"^\s*enabled:\s*true\s*$", text, re.MULTILINE - ) is not None + assert "local_energy:" in text + assert not (tmp_path / "config" / "config.local.yaml").exists() def test_profile_create_creates_backup_before_write(client, tmp_path): @@ -370,3 +398,130 @@ def test_config_concurrent_block_writes_are_atomic_no_zero_byte_backups(tmp_path assert b.stat().st_size > 0, f"zero-byte backup: {b}" assert b.read_text().strip() != "" + +# --------------------------------------------------------------------------- +# Overlay survivability / provenance edge-case tests +# --------------------------------------------------------------------------- + +_OVERLAY_HEADER_PREFIX = "# Machine-local overlay" + + +def test_overlay_survives_git_restore_of_base(tmp_path): + """A machine-local overlay (config.local.yaml) survives a git checkout of + the base file — it is gitignored and therefore unaffected by ``git + checkout -- config/config.yaml``.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "config").mkdir() + base_yaml = repo / "config" / "config.yaml" + shutil.copyfile(ROOT / "config" / "config.yaml", base_yaml) + + local_yaml = repo / "config" / "config.local.yaml" + local_yaml.write_text( + f"{_OVERLAY_HEADER_PREFIX} — gitignored, never committed.\n" + "logging:\n level: warning\n" + ) + + subprocess.run( + ["git", "init", "-q", str(repo)], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo), "config", "user.email", "test@test.com"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo), "config", "user.name", "test"], + check=True, + capture_output=True, + ) + (repo / ".gitignore").write_text("config.local.yaml\n") + subprocess.run( + ["git", "-C", str(repo), "add", "config/config.yaml", ".gitignore"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo), "commit", "-m", "init", "-q"], + check=True, + capture_output=True, + ) + + text = base_yaml.read_text() + base_yaml.write_text(text.replace('level: "info"', 'level: "debug"')) + + subprocess.run( + ["git", "-C", str(repo), "checkout", "--", "config/config.yaml"], + check=True, + capture_output=True, + ) + + assert local_yaml.exists() + overlay_text = local_yaml.read_text() + assert "level: warning" in overlay_text + + restored = base_yaml.read_text() + assert 'level: "info"' in restored or "level: info" in restored + + +def test_config_GET_shows_overlay_source_when_overlay_exists(client): + """When ``config.local.yaml`` overrides one allowlisted key, GET + /admin/api/config reports ``source: "overlay"`` for that key and + ``source: "base"`` for the rest.""" + tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") + local_yaml.write_text( + f"{_OVERLAY_HEADER_PREFIX} — gitignored, never committed.\n" + "logging:\n level: warning\n" + ) + + resp = tc.get("/admin/api/config") + assert resp.status_code == 200 + body = resp.json() + + assert body["logging.level"]["source"] == "overlay" + assert body["logging.level"]["value"] == "warning" + assert body["objective.quality_tolerance"]["source"] == "base" + + +def test_first_write_creates_overlay_with_header(client): + """POST to an allowlisted key when no overlay exists creates + ``config.local.yaml`` starting with the expected comment header.""" + tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") + assert not local_yaml.exists(), "overlay must not exist before the write" + + resp = tc.post( + "/admin/api/config/circuit_breaker.enabled", json={"value": True} + ) + assert resp.status_code == 200 + + assert local_yaml.exists() + text = local_yaml.read_text() + assert text.startswith("# Machine-local overlay") + assert "# Written by the admin portal" in text + local = yaml.safe_load(text) + assert local["circuit_breaker"]["enabled"] is True + + +def test_second_write_keeps_header(client): + """A second admin write preserves the machine-local header comment.""" + tc, config_yaml = client + local_yaml = config_yaml.with_name("config.local.yaml") + + resp = tc.post("/admin/api/config/logging.level", json={"value": "warning"}) + assert resp.status_code == 200 + + resp = tc.post("/admin/api/config/circuit_breaker.enabled", json={"value": True}) + assert resp.status_code == 200 + + text = local_yaml.read_text() + assert text.startswith("# Machine-local overlay") + # The header must appear exactly once. + assert text.count("# Machine-local overlay") == 1 + local = yaml.safe_load(text) + assert local["logging"]["level"] == "warning" + assert local["circuit_breaker"]["enabled"] is True + diff --git a/tests/test_admin_frontend.py b/tests/test_admin_frontend.py index a089c3f..ad11ebd 100644 --- a/tests/test_admin_frontend.py +++ b/tests/test_admin_frontend.py @@ -152,3 +152,16 @@ def test_quota_modal_has_not_configured_fallback(): index_path = ROOT / "admin" / "frontend" / "index.html" html = index_path.read_text() assert "not configured" in html + + +def test_admin_controls_persisted_config_handles_wrapped_value_shape(admin_client): + """GET /admin/controls contains the updated Persisted Config hint and the + frontend unwrapping logic for the {value, source} response shape.""" + resp = admin_client.get("/admin/controls") + assert resp.status_code == 200 + assert resp.headers["content-type"].startswith("text/html") + text = resp.text + assert "config/config.local.yaml" in text + assert "entry.value" in text + assert "source === 'overlay'" in text + assert "data-orig" in text diff --git a/tests/test_config.py b/tests/test_config.py index f5f920f..8076bb8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -11,8 +11,9 @@ from pathlib import Path import pytest import yaml +from pydantic import ValidationError -from config import RouterConfig +from config import RouterConfig, _merge_overlay, load_config ROOT = Path(__file__).resolve().parent.parent @@ -179,3 +180,93 @@ def test_default_profile_accepts_builtin_name(raw, builtin_name): cfg.setdefault("routing", {})["default_profile"] = builtin_name loaded = RouterConfig(**cfg) assert loaded.routing.default_profile == builtin_name + + + +def _write_configs(tmp_path: Path, base: dict | None, overlay: dict | None): + """Write *base* and *overlay* YAML files into *tmp_path* and return the base path.""" + cfg_dir = tmp_path / "config" + cfg_dir.mkdir(exist_ok=True) + base_path = cfg_dir / "config.yaml" + if base is not None: + base_path.write_text(yaml.safe_dump(base)) + if overlay is not None: + (cfg_dir / "config.local.yaml").write_text(yaml.safe_dump(overlay)) + return base_path + + +def test_load_config_without_overlay_returns_base_only(tmp_path: Path, raw): + """No overlay file ⇒ load_config returns the same as RouterConfig(**base_raw).""" + base_path = _write_configs(tmp_path, raw, overlay=None) + loaded = load_config(str(base_path)) + assert loaded.objective.quality_tolerance == 0.1 + assert loaded.routing.default_profile == "default" + assert loaded.routing.allowed_access_levels == ["public"] + assert loaded.local_energy.enabled is False + + +def test_load_config_overlay_deep_merge_preserves_sibling_keys(tmp_path: Path, raw): + """Overlay sets local_energy.tariff_usd_per_kwh only; base keys remain intact.""" + overlay = {"local_energy": {"tariff_usd_per_kwh": 0.999}} + base_path = _write_configs(tmp_path, copy.deepcopy(raw), overlay) + loaded = load_config(str(base_path)) + assert loaded.local_energy.tariff_usd_per_kwh == 0.999 + assert loaded.local_energy.enabled is False + assert loaded.local_energy.meter == "nvidia_smi" + assert loaded.objective.quality_tolerance == 0.1 + + +def test_load_config_overlay_list_replaces_wholesale(tmp_path: Path, raw): + """An overlay list replaces the base list — it does NOT append.""" + overlay = {"routing": {"allowed_access_levels": ["public", "canary"]}} + base_path = _write_configs(tmp_path, copy.deepcopy(raw), overlay) + loaded = load_config(str(base_path)) + assert loaded.routing.allowed_access_levels == ["public", "canary"] + assert "private" not in loaded.routing.allowed_access_levels + assert loaded.routing.default_profile == "default" + + +def test_load_config_overlay_unknown_key_raises(tmp_path: Path): + """Overlay containing a top-level unknown key triggers Pydantic validation.""" + base = {"objective": {"quality_tolerance": 0.1}} + overlay = {"objective": {"quality_tolerance": 0.1, "total_bullshit_key": 42}} + base_path = _write_configs(tmp_path, base, overlay) + with pytest.raises((ValueError, ValidationError)) as exc_info: + load_config(str(base_path)) + assert "total_bullshit_key" in str(exc_info.value) + + +def test_merge_overlay_helper_list_replacement(): + """_merge_overlay replaces lists rather than appending.""" + base = {"routing": {"allowed_access_levels": ["public"]}} + overlay = {"routing": {"allowed_access_levels": ["private"]}} + merged = _merge_overlay(base, overlay) + assert merged["routing"]["allowed_access_levels"] == ["private"] + assert len(merged["routing"]["allowed_access_levels"]) == 1 + + +def test_overlay_enables_local_energy(tmp_path: Path, raw): + """The overlay can flip local_energy.enabled without touching the tracked base file.""" + overlay = {"local_energy": {"enabled": True, "tariff_usd_per_kwh": 0.12}} + base_path = _write_configs(tmp_path, copy.deepcopy(raw), overlay) + loaded = load_config(str(base_path)) + assert loaded.local_energy.enabled is True + assert loaded.local_energy.tariff_usd_per_kwh == 0.12 + + +def test_merge_overlay_helper_recursive_mapping_merge(): + """_merge_overlay recursively merges nested mappings, preserving sibling keys.""" + base = { + "a": {"x": 1, "y": 2}, + "b": "base", + } + overlay = { + "a": {"y": 99, "z": 3}, + "c": "new", + } + merged = _merge_overlay(base, overlay) + assert merged["a"]["x"] == 1 + assert merged["a"]["y"] == 99 + assert merged["a"]["z"] == 3 + assert merged["b"] == "base" + assert merged["c"] == "new" -- 2.49.1 From d570ebd671bf0017dc12c8502e4800c35c23187a Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:34:12 -0400 Subject: [PATCH 5/9] docs(incidents): #5 -- git clean -fdx destroyed the database, key and venv First incident in this project with real data loss, and the first where recovery depended on luck rather than design. An agent ran `git clean -fdx`. The -x flag removes IGNORED files as well as untracked ones, and everything this deployment needs to run is ignored by design: router.db (truncated to 0 bytes -- 22,776 energy observations, 17,321 route decisions, 148 proficiency rows), .env (the NeuralWatt API key), .venv (the virtualenv systemd's ExecStart runs from), and config/config.local.yaml (the operator's tariff). Router returned 500 with "no such table: energy_observations" while systemd reported active. `git clean -fd` is a reasonable thing for an agent to run. Adding -x turns it from "discard my scratch files" into "delete the deployment", and nothing in the repo warned about that. Recovery was luck: the plan running at the time had made a QA copy in /tmp 25 seconds before the wipe, and that copy happened to include .env. Restore was effectively lossless. A different plan and the entire measurement history would be gone. Records the recovery runbook, and the gap it exposes: router.db has no backup policy despite holding data that either costs money to rebuild (proficiency, via eval runs) or cannot be rebuilt at all (historical observations). Also notes what limited the damage -- committing the in-flight plan's 11 files minutes earlier, and parking the tariff outside the repo -- both of which were reactions to that same file being clobbered seven times the same day. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- docs/incidents.md | 96 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/docs/incidents.md b/docs/incidents.md index f8a82b7..5ffeeb7 100644 --- a/docs/incidents.md +++ b/docs/incidents.md @@ -1,12 +1,16 @@ # Incidents: how this router has broken, and how to tell which one it is -Four times now, a change that looked local to the router has silently degraded +Five times now, a change that looked local to the router has silently degraded the agent depending on it. They share a shape worth naming: **none of them -announce themselves as router problems.** Three of the four presented as an +announce themselves as router problems.** Four of the five presented as an opaque client-side error — a connection refused, an "Unprocessable Content", an "internal server error" — and diagnosing each meant knowing which log or table to look in. +**#5 is the only one so far that destroyed data**, and the only one where +recovery depended on luck rather than design. Read it before running any +cleanup command in this repo. + This page exists so the next one takes minutes rather than hours. Start with the symptom table, then read only the relevant section. @@ -24,6 +28,7 @@ and why the diagnostics below are worth having to hand. | Agent dies mid-task on an opaque 4xx | #3 candidate set empty | `sqlite3 router.db "select task_tier, required_context_tokens, rejected_reason from route_decisions where selected_model is null order by id desc limit 5;"` | | "internal server error", dashboard blank | #4 config/db path | `journalctl --user -u llm-router --since '10 min ago' \| grep -c '" 500'` then `git diff config/config.yaml` | | Prices/windows look wrong, nothing errors | catalog frozen | `sqlite3 router.db "select max(last_updated) from models;"` | +| 500s + `no such table`, venv/.env gone | #5 `git clean -fdx` | `ls -la router.db .env .venv` — a 0-byte db and a missing `.venv` is conclusive | That last row is not an incident yet — it is the silent-staleness failure described in the "Run as a service" section of `CLAUDE.md`. An unpolled catalog @@ -151,14 +156,93 @@ live service reads, restore it in the same step and verify `curl -s localhost:8080/health` before moving on. A QA step that leaves production broken has not passed. +## #5 — `git clean -fdx` destroyed the database, key and venv (2026-09-04) + +**Symptom.** Router returned 500 on every request. Journal showed +`sqlite3.OperationalError: no such table: energy_observations` while systemd +reported the service `active`. + +**Cause.** An agent ran `git clean -fdx` in the repo. The `-x` flag removes +**ignored** files as well as untracked ones, and everything this deployment +needs to run is ignored by design: + +| lost | what it was | +|---|---| +| `router.db` | truncated to 0 bytes — 22,776 energy observations, 17,321 route decisions, 148 proficiency rows | +| `.env` | the NeuralWatt API key | +| `.venv` | the virtualenv the systemd unit's `ExecStart` runs from | +| `config/config.local.yaml` | the operator's electricity tariff | +| `node_modules` | | + +This is worse than it looks from the command. `git clean -fd` is a reasonable +thing for an agent to run to get a clean tree. Adding `-x` turns it from +"discard my scratch files" into "delete the deployment", and nothing in the +repo warns you. + +**Recovery was luck, not design.** There is no backup of `router.db` by policy. +What saved it was that the plan running at the time had made a QA copy at +`/tmp/qa-config-local-overlay-r2/` **25 seconds before the wipe**, and that +copy happened to include `.env`. Integrity check passed and the restore was +effectively lossless. Had that plan been a different one, the entire +measurement history of the project would be gone. + +**Recovery steps, in order:** + +```bash +# 1. find a surviving copy — QA/scratch dirs are the likely place +find /home/alee /tmp -name "router.db" -size +0 +sqlite3 "select count(*) from energy_observations;" +sqlite3 "select integrity_check from pragma_integrity_check limit 1;" + +# 2. restore data, key, overlay +cp -f router.db +cp -f /.env .env && chmod 600 .env +# config/config.local.yaml from your own copy + +# 3. rebuild the venv (pinned, so this is deterministic) +python3 -m venv .venv && .venv/bin/pip install -r requirements.txt + +# 4. restart and verify +systemctl --user restart llm-router.service +curl -s localhost:8080/health +``` + +**What actually limited the damage** was two unrelated decisions made minutes +earlier: the in-flight plan's 11 files had just been committed rather than left +uncommitted, and the operator's tariff had been parked *outside* the repo +instead of restored in place. Both were reactions to the same file being +clobbered repeatedly that day — the seventh time is what prompted moving it out +of reach. + +**Known gap this leaves open.** `router.db` has no backup policy. It holds +every energy/cost observation, every routing decision, and all proficiency +scores — none of which can be rebuilt without re-running evals that cost real +money, and the historical observations cannot be rebuilt at all. A periodic +snapshot is cheap insurance and does not exist. + +**Rules that came out of it:** + +- **Never `git clean -x` in this repo.** Use targeted paths. If you need a + clean tree, `git stash` preserves; `clean` destroys. +- **Never clean untracked files you did not create.** An untracked file in this + tree is as likely to be operator data as build residue. +- Before any destructive git command, ask what is *ignored*, not just what is + untracked. Here that list is the database, the API key and the runtime. + + --- ## The pattern -All four are defensible local decisions — free a port, stop a process, deprecate -an expensive model, point at a test database — that silently removed capability -while the router kept reporting itself healthy. Two of the four were caused by -an agent working on the router, using the router. +All five are defensible local decisions — free a port, stop a process, deprecate +an expensive model, point at a test database, clean the working tree — that +silently removed capability while the router kept reporting itself healthy. +Three of the five were caused by an agent working on the router, using the +router. + +#5 breaks the pattern in one way worth noting: it did not degrade quietly, it +failed loudly and immediately. What it removed was not capability but *state* — +and state, unlike capability, does not come back when you fix the code. The generalisable fix is the same each time: **compute the thing that is actually true, and surface it where the operator is already looking.** That is -- 2.49.1 From 9dfe00acd98b35eb26c1d2516012cdeafb727b25 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:39:22 -0400 Subject: [PATCH 6/9] feat(deploy): hourly database snapshots and a workstation backup tarball Direct response to docs/incidents.md #5, where `git clean -fdx` truncated router.db to 0 bytes and deleted .env, .venv and config/config.local.yaml. Recovery was luck -- a QA copy happened to exist in /tmp from 25 seconds earlier. There was no backup policy at all. llm-router-backup.sh + .service + .timer: hourly, keeps 24. Uses sqlite3 .backup rather than cp, because copying a live database with an open writer can capture a torn page set that passes a size check and fails integrity_check. It verifies the new snapshot with integrity_check BEFORE rotating, so a failing run never leaves fewer copies than it started with. workstation-backup.sh: tarballs what git does not have -- router.db, .env, config/config.local.yaml, .omo/ (plans and evidence ledger), tuned systemd units, opencode plugins, and this project's Claude memory -- plus a RESTORE.md with the clone -> venv -> restore sequence. Excludes .venv and node_modules (rebuildable from pinned requirements) and Ollama models (~30GB, re-pullable, recipes in docs/local-models.md). Backups write OUTSIDE the repository by design. A backup kept inside it, even gitignored, would have been destroyed by the same command that caused the incident. Test-restored before committing: 22,821 observations with integrity=ok, API key present, tariff intact, 7 systemd units, 2 opencode plugins, 11 memory files. That test caught a second loss nobody had noticed -- .omo/plans had also been destroyed by the same clean, taking 35 plan artifacts and 127 evidence files, since .omo/ is gitignored too. Recovered from the same QA copy. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- deploy/llm-router-backup.service | 12 +++++ deploy/llm-router-backup.sh | 55 ++++++++++++++++++++ deploy/llm-router-backup.timer | 14 ++++++ deploy/workstation-backup.sh | 86 ++++++++++++++++++++++++++++++++ 4 files changed, 167 insertions(+) create mode 100644 deploy/llm-router-backup.service create mode 100755 deploy/llm-router-backup.sh create mode 100644 deploy/llm-router-backup.timer create mode 100755 deploy/workstation-backup.sh diff --git a/deploy/llm-router-backup.service b/deploy/llm-router-backup.service new file mode 100644 index 0000000..c43a8aa --- /dev/null +++ b/deploy/llm-router-backup.service @@ -0,0 +1,12 @@ +# Snapshots router.db, .env and config/config.local.yaml to a directory OUTSIDE +# the repository. See docs/incidents.md #5: `git clean -fdx` destroyed all three +# in place, and a backup kept inside the repo would have gone with them. +[Unit] +Description=Snapshot the LLM router database and operator config +Documentation=file:%h/Sources/6krrt/docs/incidents.md + +[Service] +Type=oneshot +WorkingDirectory=%h/Sources/6krrt +Environment=REPO=%h/Sources/6krrt +ExecStart=%h/Sources/6krrt/deploy/llm-router-backup.sh diff --git a/deploy/llm-router-backup.sh b/deploy/llm-router-backup.sh new file mode 100755 index 0000000..794a268 --- /dev/null +++ b/deploy/llm-router-backup.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Snapshot the router's irreplaceable state. +# +# WHY THIS EXISTS: on 2026-09-04 an agent ran `git clean -fdx` in the repo. +# The -x flag removes IGNORED files, and everything this deployment needs is +# ignored by design -- router.db went to 0 bytes, taking 22,776 energy +# observations, 17,321 routing decisions and 148 proficiency scores, plus .env +# and the whole virtualenv. Recovery was luck: a QA copy happened to exist in +# /tmp from 25 seconds earlier. See docs/incidents.md #5. +# +# Backups therefore live OUTSIDE the repository. A backup inside it -- even +# gitignored -- would have been destroyed by the same command. +# +# What is NOT recoverable without this: +# - energy_observations : historical measurements, cannot be regenerated +# - route_decisions : same +# - proficiency : rebuildable only by re-running evals, which costs +# real provider credits +set -euo pipefail + +REPO="${REPO:-$HOME/Sources/6krrt}" +DEST="${LLM_ROUTER_BACKUP_DIR:-$HOME/.local/share/6krrt-backups}" +KEEP="${LLM_ROUTER_BACKUP_KEEP:-24}" + +mkdir -p "$DEST" +stamp=$(date +%Y%m%d-%H%M%S) + +# .backup is the ONLY safe way to copy a live SQLite file. `cp` on a database +# with an open writer can produce a torn copy that passes a size check and +# fails integrity_check. +sqlite3 "$REPO/router.db" ".backup '$DEST/router-$stamp.db'" +gzip -f "$DEST/router-$stamp.db" + +# Operator data that also lives only in ignored files. +[ -f "$REPO/.env" ] && { cp -f "$REPO/.env" "$DEST/env-$stamp.bak"; chmod 600 "$DEST/env-$stamp.bak"; } +[ -f "$REPO/config/config.local.yaml" ] && cp -f "$REPO/config/config.local.yaml" "$DEST/config.local-$stamp.yaml" + +# Verify before rotating: a backup that has never been read is a guess. +tmp=$(mktemp) +zcat "$DEST/router-$stamp.db.gz" > "$tmp" +if [ "$(sqlite3 "$tmp" 'SELECT integrity_check FROM pragma_integrity_check LIMIT 1;')" != "ok" ]; then + rm -f "$tmp" "$DEST/router-$stamp.db.gz" + echo "backup FAILED integrity_check; discarded, older backups retained" >&2 + exit 1 +fi +rm -f "$tmp" + +# Rotate only after a good backup exists, so a failing run never leaves you +# with fewer copies than you started with. +for pat in "router-*.db.gz" "env-*.bak" "config.local-*.yaml"; do + # shellcheck disable=SC2012 + ls -1t "$DEST"/$pat 2>/dev/null | tail -n +$((KEEP + 1)) | xargs -r rm -f +done + +echo "backup ok: $DEST/router-$stamp.db.gz ($(du -h "$DEST/router-$stamp.db.gz" | cut -f1)), keeping $KEEP" diff --git a/deploy/llm-router-backup.timer b/deploy/llm-router-backup.timer new file mode 100644 index 0000000..657c950 --- /dev/null +++ b/deploy/llm-router-backup.timer @@ -0,0 +1,14 @@ +# Hourly, with 24 kept by default -- roughly a day of hourly granularity. +# Deliberately more frequent than the 2h poller: the poller refetches a remote +# catalog that can always be refetched, while energy_observations and +# route_decisions exist nowhere else. +[Unit] +Description=Hourly snapshot of the LLM router database + +[Timer] +OnBootSec=5min +OnUnitActiveSec=1h +Persistent=true + +[Install] +WantedBy=timers.target diff --git a/deploy/workstation-backup.sh b/deploy/workstation-backup.sh new file mode 100755 index 0000000..05018b1 --- /dev/null +++ b/deploy/workstation-backup.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Tarball everything about THIS workstation that git does not have. +# +# The repo itself is pushed to Gitea and needs no backup. What has no other +# copy is the ignored/untracked state around it -- and docs/incidents.md #5 +# records what happens when that is lost: `git clean -fdx` took router.db, +# .env, .venv and config/config.local.yaml in one command, and recovery was +# luck. +# +# INCLUDED (irreplaceable or expensive to recreate): +# router.db measurement history; cannot be regenerated +# .env provider API key +# config/config.local.yaml operator tariff and local overrides +# .omo/ plans, evidence ledger, boulder state +# ~/.config/systemd/user tuned units (the graceful-shutdown fix etc.) +# ~/.config/opencode agent plugins incl. session-registry.js +# ~/.claude/.../memory cross-session memory for this project +# +# EXCLUDED on purpose: +# .venv rebuildable: python -m venv .venv && pip install -r requirements.txt +# node_modules rebuildable +# ollama models ~30GB and re-pullable; Modelfile recipes are in docs/local-models.md +# .git the remote has it +set -euo pipefail + +REPO="${REPO:-$HOME/Sources/6krrt}" +DEST="${LLM_ROUTER_BACKUP_DIR:-$HOME/.local/share/6krrt-backups}" +KEEP="${WORKSTATION_BACKUP_KEEP:-7}" + +mkdir -p "$DEST" +stamp=$(date +%Y%m%d-%H%M%S) +out="$DEST/workstation-$stamp.tar.gz" +staging=$(mktemp -d) +trap 'rm -rf "$staging"' EXIT + +# Snapshot the DB properly rather than tarring a live file: `cp`/`tar` on an +# open SQLite database can capture a torn page set that still looks valid. +sqlite3 "$REPO/router.db" ".backup '$staging/router.db'" + +mkdir -p "$staging/repo/config" +[ -f "$REPO/.env" ] && cp -f "$REPO/.env" "$staging/repo/.env" +[ -f "$REPO/config/config.local.yaml" ] && cp -f "$REPO/config/config.local.yaml" "$staging/repo/config/" +[ -d "$REPO/.omo" ] && cp -a "$REPO/.omo" "$staging/repo/.omo" + +mkdir -p "$staging/home" +[ -d "$HOME/.config/systemd/user" ] && cp -a "$HOME/.config/systemd/user" "$staging/home/systemd-user" +[ -d "$HOME/.config/opencode" ] && cp -a "$HOME/.config/opencode" "$staging/home/opencode" +mem="$HOME/.claude/projects/-home-alee-Sources-6krrt/memory" +[ -d "$mem" ] && cp -a "$mem" "$staging/home/claude-memory" + +cat > "$staging/RESTORE.md" <<'INNER' +# Restoring this workstation + +The repo comes from Gitea; this tarball has only what git does not. + + git clone ssh://git@git.adlee.work:2222/alee/6krrt.git + cd 6krrt + python3 -m venv .venv && .venv/bin/pip install -r requirements.txt + +Then from this archive: + + cp router.db /router.db + cp repo/.env /.env && chmod 600 /.env + cp repo/config/config.local.yaml /config/ + cp -a repo/.omo /.omo + cp -a home/systemd-user/* ~/.config/systemd/user/ && systemctl --user daemon-reload + cp -a home/opencode ~/.config/opencode + +Ollama models are not here. Re-pull and re-tag per docs/local-models.md: + ollama pull qwen2.5-coder:14b && ollama pull qwen3-vl:4b + (then the Modelfile num_ctx tags documented there) + +Verify: + sqlite3 /router.db "select integrity_check from pragma_integrity_check limit 1;" + systemctl --user restart llm-router.service && curl -s localhost:8080/health +INNER + +tar -czf "$out" -C "$staging" . + +# Verify the archive reads back before rotating anything away. +tar -tzf "$out" >/dev/null || { rm -f "$out"; echo "archive FAILED to verify; discarded" >&2; exit 1; } + +# shellcheck disable=SC2012 +ls -1t "$DEST"/workstation-*.tar.gz 2>/dev/null | tail -n +$((KEEP + 1)) | xargs -r rm -f + +echo "workstation backup ok: $out ($(du -h "$out" | cut -f1)), keeping $KEEP" -- 2.49.1 From 4a32b0180f08dca3cfa530281aec42010f3e6c65 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 18:43:23 -0400 Subject: [PATCH 7/9] feat(deploy): 6-hourly off-site backup to a private git repo Closes the last gap from docs/incidents.md #5. The hourly local snapshots survive `git clean -fdx` because they live outside the repo, but they do not survive the disk. This pushes the irreplaceable-and-small state to a private Gitea repo: .omo plans and evidence ledger, tuned systemd units, opencode plugins, this project's Claude memory, config/config.local.yaml, and a DAILY gzipped router.db. The database is committed daily rather than hourly on purpose: it is binary and ~5MB gzipped, so git cannot delta it. Hourly would grow the repo ~120MB/day instead of ~5MB. SYNC_DB=0 turns it off entirely. .env is deliberately NOT synced. There is no usable secret key on this host to encrypt it to (public keys only), so it would sit in git history in plaintext, and history is forever even in a private repo. An API key is replaceable by regenerating it from the provider; 22,821 energy observations are not. It stays in the local backups only, and the user confirmed that trade. Verified by fresh clone: 35 plan artifacts, 127 evidence files, 9 systemd units, 2 opencode plugins, 11 memory files, the tariff, and a 5.1MB db snapshot. Checked the actual 67-character key VALUE appears in zero off-site files -- an earlier check grepped for the variable NAME and for 'sk-', which matches 'task-', and produced 50 false positives. Grep for the secret, not for its label. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- deploy/llm-router-offsite.service | 14 +++++ deploy/llm-router-offsite.timer | 13 +++++ deploy/offsite-sync.sh | 85 +++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 deploy/llm-router-offsite.service create mode 100644 deploy/llm-router-offsite.timer create mode 100755 deploy/offsite-sync.sh diff --git a/deploy/llm-router-offsite.service b/deploy/llm-router-offsite.service new file mode 100644 index 0000000..d61d792 --- /dev/null +++ b/deploy/llm-router-offsite.service @@ -0,0 +1,14 @@ +# Pushes irreplaceable-and-small state to a private off-site git repo. +# Local snapshots survive `git clean -fdx` (docs/incidents.md #5) because they +# live outside the repo; they do NOT survive the disk. This does. +[Unit] +Description=Push LLM router state off-site to a private git repo +Documentation=file:%h/Sources/6krrt/docs/incidents.md +After=network-online.target +Wants=network-online.target + +[Service] +Type=oneshot +WorkingDirectory=%h/Sources/6krrt +Environment=REPO=%h/Sources/6krrt +ExecStart=%h/Sources/6krrt/deploy/offsite-sync.sh diff --git a/deploy/llm-router-offsite.timer b/deploy/llm-router-offsite.timer new file mode 100644 index 0000000..eceaf54 --- /dev/null +++ b/deploy/llm-router-offsite.timer @@ -0,0 +1,13 @@ +# Every 6h. Less frequent than the hourly local snapshot on purpose: this one +# needs the network and pushes a ~5MB binary daily, so the local timer stays +# the fine-grained safety net and this is the off-site floor. +[Unit] +Description=Periodic off-site backup of LLM router state + +[Timer] +OnBootSec=15min +OnUnitActiveSec=6h +Persistent=true + +[Install] +WantedBy=timers.target diff --git a/deploy/offsite-sync.sh b/deploy/offsite-sync.sh new file mode 100755 index 0000000..10e6eac --- /dev/null +++ b/deploy/offsite-sync.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# Push the irreplaceable-and-small state to a private off-site git repo. +# +# Closes the last gap from docs/incidents.md #5: local snapshots survive +# `git clean -fdx` because they live outside the repo, but they do NOT survive +# the disk. This does. +# +# WHAT IS SYNCED +# .omo/ plan artifacts + evidence ledger (13MB, text, versions well) +# systemd units tuned: graceful-shutdown fix, timers +# opencode plugins session-registry.js etc. -- hand-written, not from a package +# claude memory cross-session project memory +# config.local.yaml operator tariff/overrides (personal, NOT a credential) +# router.db gzipped, at most once per day -- see SIZE below +# +# WHAT IS DELIBERATELY NOT SYNCED +# .env — the provider API key. There is no usable secret key on this host to +# encrypt it to (only public keys), so it would land in git history in +# plaintext, and git history is forever even in a private repo. An API +# key is REPLACEABLE: regenerate it from the provider. 22,000 energy +# observations are not. It stays in the local backups only. +# .venv, node_modules — rebuildable from pinned requirements. +# ollama models — ~30GB, re-pullable; recipes in docs/local-models.md. +# +# SIZE: router.db is ~5MB gzipped and binary, so git cannot delta it. It is +# committed at most DAILY (not hourly) to keep growth near 5MB/day rather than +# 120MB/day. Set SYNC_DB=0 to skip it entirely and rely on local snapshots. +set -euo pipefail + +REPO="${REPO:-$HOME/Sources/6krrt}" +REMOTE="${OFFSITE_REMOTE:-ssh://git@git.adlee.work:2222/alee/6kbackups.git}" +WORK="${OFFSITE_WORKDIR:-$HOME/.local/share/6krrt-offsite}" +SYNC_DB="${SYNC_DB:-1}" + +if [ ! -d "$WORK/.git" ]; then + git clone "$REMOTE" "$WORK" 2>/dev/null || { mkdir -p "$WORK"; git -C "$WORK" init -q; git -C "$WORK" remote add origin "$REMOTE"; } +fi +cd "$WORK" +git fetch -q origin 2>/dev/null || true +git checkout -q -B main 2>/dev/null || true +git reset -q --hard origin/main 2>/dev/null || true + +rsync -a --delete "$REPO/.omo/" ./omo/ 2>/dev/null || true +mkdir -p home config +rsync -a --delete "$HOME/.config/systemd/user/" ./home/systemd-user/ 2>/dev/null || true +rsync -a --delete "$HOME/.config/opencode/" ./home/opencode/ --exclude 'cache/' --exclude 'log/' --exclude '*.log' 2>/dev/null || true +rsync -a --delete "$HOME/.claude/projects/-home-alee-Sources-6krrt/memory/" ./home/claude-memory/ 2>/dev/null || true +[ -f "$REPO/config/config.local.yaml" ] && cp -f "$REPO/config/config.local.yaml" ./config/ + +# Daily DB snapshot, same filename so the working tree stays flat. +if [ "$SYNC_DB" = "1" ]; then + today=$(date +%Y-%m-%d) + if [ ! -f .db-stamp ] || [ "$(cat .db-stamp)" != "$today" ]; then + tmp=$(mktemp) + sqlite3 "$REPO/router.db" ".backup '$tmp'" + [ "$(sqlite3 "$tmp" 'SELECT integrity_check FROM pragma_integrity_check LIMIT 1;')" = "ok" ] \ + && { gzip -c "$tmp" > router.db.gz; echo "$today" > .db-stamp; } \ + || echo "db snapshot failed integrity_check; not synced" >&2 + rm -f "$tmp" + fi +fi + +cat > README.md <<'INNER' +# 6krrt workstation backup + +Off-site copy of state the main repo does not carry. See +`docs/incidents.md` #5 in the main repo for why this exists. + +**Not here on purpose:** `.env` (provider API key — regenerate it instead; +there is no usable secret key on the source host to encrypt it to), `.venv`, +`node_modules`, and Ollama models. + +Restore: clone the main repo, rebuild the venv from pinned requirements, then +copy `omo/` back to `.omo/`, `config/config.local.yaml` into place, and +`home/*` to their `~/.config` locations. `router.db.gz` is a daily snapshot. +INNER + +git add -A +if git diff --cached --quiet; then + echo "offsite: no changes" +else + git -c user.name="6krrt-backup" -c user.email="backup@localhost" \ + commit -q -m "backup $(date -Iseconds)" + git push -q -u origin main && echo "offsite: pushed $(git rev-parse --short HEAD)" +fi -- 2.49.1 From 0570123029b2794d7a5dc12ce143b50f6bec9362 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 20:03:36 -0400 Subject: [PATCH 8/9] docs(plans): park multi-provider; correct a misattributed coupling site Provider selection is reopened -- Z.ai was recommended on the strength of the GLM catalog overlap, which was an argument for the *criterion*, not a confirmation that Z.ai's API or terms fit. Downgrade the recommendation to a shortlist and keep the criterion for whatever candidate comes next. Also corrects a real error. The draft attributed dispatcher.py:2879 to _check_pinned_capabilities and called it a spurious-422 risk. That function already takes provider as a parameter and is not a coupling site. 2879 is _model_exists, and it fails open rather than closed: it decides whether a `provider/model` string is an opencode alias to strip or a real id, so a second-provider id resolves to False, gets stripped, and dispatches to whatever the remainder matches -- silently, on a different provider, at a different price. `vendor/model` is the native id format for OpenRouter and most aggregators, so this is the default case, not an edge one. Records that the 20 hardcoded neuralwatt literals are provider-agnostic work that need not wait on the provider decision. Re-verified today: still 20 refs across the same six files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- plans/multi-provider-support.md | 89 ++++++++++++++++++++++++++------- 1 file changed, 72 insertions(+), 17 deletions(-) diff --git a/plans/multi-provider-support.md b/plans/multi-provider-support.md index a8e7473..4b55854 100644 --- a/plans/multi-provider-support.md +++ b/plans/multi-provider-support.md @@ -1,11 +1,29 @@ # Generalizing to multiple providers -**Status: DRAFT — closer, still not decision-complete.** Written 2026-09-04 -against `main` at `340453e`; coupling surface measured and three open -questions closed by code inspection on the same commit. Remaining gaps before -this can go to the opencode/Prometheus pipeline: the proficiency-sharing -decision (a judgement call, not a lookup), the eco-without-telemetry decision, -and the still-empty success criteria / test plan / phasing. +**Status: PARKED 2026-09-04 — DRAFT, not decision-complete, and now blocked on +provider selection rather than on design.** Written 2026-09-04 against `main` +at `340453e`; coupling surface measured and three open questions closed by code +inspection on the same commit. + +Parked by the user: **Z.ai is no longer a settled first target.** The GLM +overlap that made it attractive is a real argument, but fit is unconfirmed and +the candidate search is still open. Nothing below should be read as a +commitment to a specific provider. + +Remaining gaps before this can go to the opencode/Prometheus pipeline: + +1. **Which provider goes first** (was treated as answered; it is not). +2. The proficiency-sharing decision — a judgement call, not a lookup. +3. The eco-without-telemetry decision. +4. Success criteria / test plan / phasing, still empty. + +**What does NOT need to wait.** The measured coupling surface below is +provider-agnostic: the 20 hardcoded `neuralwatt` references are literals that +are wrong regardless of which provider lands second, and three of them are +latent defects today. Re-verified on 2026-09-04 against +`feat/config-local-overlay` — still 20 references, same six-file distribution. +Those can be parameterised independently of this plan and should not be held +hostage to the provider question. ## Why this, why now @@ -58,11 +76,25 @@ thing moves): | Fireworks AI | plausible | same shape as Together. | | Groq | maybe later | OpenAI-compatible, free tier + pay-as-you-go, but a much smaller catalog — more useful for a latency-tolerance test than a routing-breadth test. | -Recommendation: **one provider first** — Z.ai, given the GLM overlap makes it -a more informative test than a disjoint catalog — all the way through -poller → dispatch → a handful of real routed requests, before touching a -second. Confirms the abstraction actually generalizes instead of just -looking like it does on paper. +**Recommendation, downgraded 2026-09-04.** The *shape* still holds: **one +provider first**, all the way through poller → dispatch → a handful of real +routed requests, before touching a second. That is what confirms the +abstraction generalizes instead of just looking like it does on paper. + +Which provider is **reopened**. Z.ai was the pick on the strength of the GLM +overlap — a same-weights cross-provider comparison, and a free experiment on +the `static_fallback` carbon question. That argument is still good and should +be re-used to score whatever candidate comes next; it was never a claim that +Z.ai's API, pricing or terms actually fit. Treat the table above as a +shortlist to re-verify, not a ranking to execute. + +Worth noting for the search: the criterion that made Z.ai attractive — +**catalog overlap with what NeuralWatt already serves** — is separable from +Z.ai itself. OpenRouter carries kimi, deepseek, qwen and gemma alongside GLM, +so it satisfies the same-weights test more broadly, and this repo already has +git history (`c3484f0` and its parent) with a working catalog normalizer to +mine. It was dropped for a reason that no longer disqualifies it, now that +`has_energy_telemetry` is the planned answer to missing energy data. ## Architecture sketch @@ -115,17 +147,40 @@ the NeuralWatt row and never to the Z.ai one. `leaderboards.yaml` ships empty today so nothing is broken yet, but this decides the answer to the proficiency-sharing question above by accident rather than on purpose. -### `dispatcher.py:2879` — pinned-model capability check is provider-scoped +### `dispatcher.py:2879` — `_model_exists` silently strips a real model id + +**Corrected 2026-09-04.** An earlier revision of this plan attributed this line +to `_check_pinned_capabilities`. That was wrong: `_check_pinned_capabilities` +(`dispatcher.py:2887`) already takes `provider` as a parameter and passes it to +its query. It is not a coupling site. Line 2879 belongs to `_model_exists`, +and the defect there is worse. ```sql SELECT 1 FROM models WHERE model_id = ? AND provider = 'neuralwatt' ``` -`_check_pinned_capabilities` resolves a pinned id against NeuralWatt rows only. -A client pinning a Z.ai model id would find no row. Given capability flags -**fail closed** by design (an unconfirmed capability is treated as absent), -that is a 422 on a pin that should have worked — and it will read as a -capability problem, not a provider-scoping one. +`_model_exists` 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 NeuralWatt, a real second-provider id returns +False and gets treated as an alias. + +That matters more than it looks, because **`vendor/model` is the native id +format for the two strongest candidates**: OpenRouter ids are always +`deepseek/deepseek-chat`-shaped, and several other aggregators follow suit. So +the failure is not exotic — it is the default case for a whole class of +provider. + +And it fails **silently in the wrong direction**. A provider-scoped 422 would +at least be loud. This one strips the prefix and proceeds with the remainder, +so a client pinning `deepseek/deepseek-chat` gets whatever `deepseek-chat` +resolves to — a different row, on a different provider, at a different price — +with nothing in the response indicating a substitution occurred. Fail-closed +was the design intent everywhere else in this capability path; here it fails +open. + +Requirement: `_model_exists` must resolve across all configured providers, and +alias-stripping must be decided by something other than "no NeuralWatt row has +this id". ### Risk not in the draft: per-provider fetch isolation -- 2.49.1 From be2c1ee93dc4f2340b919f80929acdb0b8216913 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 4 Sep 2026 20:08:28 -0400 Subject: [PATCH 9/9] docs(plans): profile CRUD to the overlay; provider literal cleanup Two small, contained specs that can run as one wave. admin-profile-writes-to-overlay: profile CRUD still writes config.yaml while allowlisted scalars write the overlay. That split is chronological accident -- profile CRUD shipped in PR #25, the overlay decision came after. Redirect create/update to the overlay, scope delete to overlay-defined profiles, and classify base-defined profiles as read-only alongside built-ins, which sidesteps the fact that a deep merge cannot express "remove" without tombstones. Two defects found while specifying it, both present today. An overlay-defined profile is LIVE in routing but invisible to the portal -- confirmed live: load_config sees it, _persisted_profiles() does not, because the latter reads the base file only. And delete reads the merged store for its default-profile guard but writes the base file, so an overlay-defined profile is found and then 404s on the delete. provider-literal-cleanup: the 20 hardcoded neuralwatt literals are wrong whichever provider lands second, so they need not wait on the parked multi-provider plan. Three are latent defects: _model_exists fails OPEN and silently strips a real vendor/model id (the native format for OpenRouter and most aggregators), catalog staleness is blind to a second provider's frozen catalog, and leaderboard priors are pinned to one provider -- which would decide the still-open proficiency-sharing question by accident, so the spec explicitly refuses to resolve it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U --- plans/admin-profile-writes-to-overlay.md | 192 +++++++++++++++++++++++ plans/provider-literal-cleanup.md | 161 +++++++++++++++++++ 2 files changed, 353 insertions(+) create mode 100644 plans/admin-profile-writes-to-overlay.md create mode 100644 plans/provider-literal-cleanup.md diff --git a/plans/admin-profile-writes-to-overlay.md b/plans/admin-profile-writes-to-overlay.md new file mode 100644 index 0000000..90fbebd --- /dev/null +++ b/plans/admin-profile-writes-to-overlay.md @@ -0,0 +1,192 @@ +# Admin profile CRUD should write to the overlay + +**Status: FINAL — decision-complete.** Written 2026-09-04 against +`feat/config-local-overlay` at `0570123` (PR #26). Depends on that PR landing. + +## Why this exists + +`plans/config-local-overlay.md` states the rule without qualification: + +> **The admin portal writes to `config/config.local.yaml`. It never writes +> `config/config.yaml`.** + +The implementation splits it. Allowlisted scalars go to the overlay; **profile +CRUD writes the base file directly**, documented at `src/admin.py:670`. + +This is not a considered exception. It is chronological accident: profile CRUD +shipped in PR #25 (`340453e`), and the overlay decision was made afterwards +(`e5ee92b`). Plan 8 then implemented the new rule for the scalar path it +touched and left the profile path where it was. + +## The reasoning is the same one the user already endorsed + +Plan 8's justification for the scalar path: + +> An operator changing a knob in a loopback-only admin portal is making a +> **local operational decision**, not a project decision. Someone changing a +> project default edits `config/config.yaml` in the repo and commits it, +> deliberately, through git. + +Creating `onlycheaps` in the portal is that same act. Nothing about the +argument depends on the value being a scalar rather than a nested object — the +only reason the code diverges is the ordering above. + +## What it costs to leave it + +The tariff no longer lives in `config/config.yaml`, so the blast radius is far +smaller than the six clobberings Plan 8 was written for. The residual harm is +real but narrower: + +- Using the profiles UI **re-dirties a tracked file**. `git pull --rebase` + refuses on a dirty tree — observed, and listed in Plan 8's problem statement. +- A `git switch` or `git checkout` silently discards a profile the operator + just created, with nothing indicating why. +- `commit -am` sweeps profiles into unrelated commits. + +The sharpest version: **a partial guarantee is worse than none.** An operator +who has internalized "the portal writes to the overlay, `config.yaml` stays +clean" stops checking. The failure then arrives against an expectation this +project's own fix created. + +## Two defects that exist TODAY, independent of the rule + +Both were found by reading the code on 2026-09-04, and the first was confirmed +live. They are the reason this is a bug fix and not only a consistency tidy. + +### 1. An overlay-defined profile is live in routing and invisible to the portal + +`_persisted_profiles()` (`src/admin.py:~789`) reads **base only**: + +```python +store = load_config_store_safe(config_path) or {} +return store.get("profiles") or {} +``` + +But `load_config` deep-merges the overlay, so a `profiles:` block in +`config/config.local.yaml` **is** live in `cfg.profiles`. Confirmed by writing +a temporary `ghosttest` profile into the overlay: + +| reader | sees | +|---|---| +| `load_config` (what routing uses) | `['ghosttest']` | +| `_persisted_profiles()` (what the portal lists) | `[]` | + +So the portal's profile list can already disagree with what the router will +actually accept as `auto:`. That is the same class of failure as the +vision-ceiling incident — a real state, computed correctly somewhere, never +surfaced. + +Note this is reachable **today** by hand-editing the overlay, before any of +this plan lands. It is not created by the redirect; the redirect is what fixes +it, because the portal will finally be reading the file it writes. + +### 2. Delete reads merged config but writes the base file + +`admin_profile_delete` deliberately reads the merged store so the +`default_profile` guard is correct regardless of which file set it: + +```python +merged = _load_merged_config_store(config_path, config_local_path) +persisted_profiles = merged.get("profiles") or {} +``` + +then deletes from base: + +```python +_persist_config_block(config_path, ("profiles", name), None, delete=True) +``` + +For an overlay-defined profile that is found-then-not-deletable: the existence +check passes on merged, the write raises `KeyError`, and the operator gets a +**404 for a profile the portal just confirmed exists**. The endpoint is already +half-overlay-aware, which is a good sign the split was never intentional. + +## The rule + +| operation | destination | +|---|---| +| create | `config/config.local.yaml` | +| update | `config/config.local.yaml` | +| delete | overlay-defined profiles only | +| base-defined profile | **read-only**, alongside built-ins | + +## The deletion wrinkle, and why it resolves rather than blocks + +Deep merge can add and override. It cannot express *remove*. A profile defined +in base `config/config.yaml` cannot be deleted from the overlay without a +tombstone, and tombstones are exactly the config-framework machinery Plan 8's +non-goals rule out ("resist the config-framework instinct"). + +**Resolve it by classification, not mechanism.** A profile someone committed to +`config/config.yaml` *is* a project artifact — the same category as a built-in. +Render it read-only, refuse edit and delete, and say why in a message that +points at the repo. + +This is cheap because the refusal path already exists at three sites, each +guarding on `name in BUILTIN_PROFILES`: + +- `src/admin.py:902` — create +- `src/admin.py:936` — update +- `src/admin.py:960` — delete + +The change is a widened predicate and a distinct message, not new machinery. +Keep the two refusals **distinguishable**: a built-in cannot be changed at all, +while a base-defined profile can be changed by editing the repo. Collapsing +both into "read-only" would tell the operator less than the code knows. + +It also gives the provenance display Plan 8 already built a real job: base vs +overlay is precisely what explains why a given profile is not editable. + +## Interactions to get right + +- **`routing.default_profile` may name a profile from either file.** The + existing delete guard already reads merged config for this; keep it. Deleting + the current default stays refused (422), and that check must run against the + merged view, not the overlay alone. +- **A name collision between base and overlay** must resolve to the overlay + (standard merge precedence) and be **visible as such** in the listing, not + silently deduplicated. Two definitions for one name is the ambiguity Plan 7 + refused to accept for built-ins; the same reasoning applies here. +- **Built-in name collisions stay rejected at config load**, unchanged. +- **`_profile_probe` / `_probe_candidate_zero_admit` do not change.** Admission + is still computed via `routing.select_candidates`; this plan moves a write + destination and a visibility source, not any routing logic. +- **The zero-admit warning still fires** on save, unchanged. + +## Non-goals + +- No tombstone or delete-marker syntax in the overlay. If a base-defined + profile must go, it goes through git. +- Do not mirror profile writes into both files. Plan 8 settled that: one value, + one home. +- Do not migrate existing base-defined profiles into the overlay automatically. + Same reasoning as Plan 8's Migration section — this plan does not decide + where someone else's committed config lives. +- No changes to `RoutingProfile`'s fields, to built-in definitions, or to how + `auto:` resolves. +- Do not widen `_CONFIG_ALLOWLIST`. + +## Success criteria + +- Creating a profile through the portal writes `config/config.local.yaml` and + leaves `config/config.yaml` **byte-identical** — asserted directly, since the + existing `test_profile_create_preserves_comments_and_does_not_create_overlay` + asserts the opposite and must be inverted rather than deleted. +- Updating an overlay-defined profile writes the overlay only. +- Deleting an overlay-defined profile removes it from the overlay only. +- A base-defined profile returns 403 on update and delete, with a message + distinct from the built-in message and naming `config/config.yaml`. +- **The portal lists overlay-defined profiles.** A test writes a profile into + the overlay and asserts `GET /admin/api/profiles/` returns it — this is the + regression test for defect #1 and must fail on current `main`. +- A base/overlay name collision resolves to the overlay and is labelled with + its source in the listing. +- Deleting the profile named by `routing.default_profile` is still refused when + that setting comes from **either** file. +- The overlay is created on first profile write when absent, with the same + header comment the scalar path writes. +- Backup-before-write and validation-of-merged-config still hold on the profile + path. +- 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, verified after the test run, not before. diff --git a/plans/provider-literal-cleanup.md b/plans/provider-literal-cleanup.md new file mode 100644 index 0000000..87d39a6 --- /dev/null +++ b/plans/provider-literal-cleanup.md @@ -0,0 +1,161 @@ +# Remove the hardcoded `neuralwatt` 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. -- 2.49.1