diff --git a/admin/frontend/controls.html b/admin/frontend/controls.html index 3025b23..c69a564 100644 --- a/admin/frontend/controls.html +++ b/admin/frontend/controls.html @@ -188,6 +188,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important} @@ -298,6 +299,8 @@ function icon(name, size) { zap: ``, settings: ``, database: ``, + chevronDown:``, + alert: ``, default: ``, }; return svgs[name] || svgs.default; @@ -505,6 +508,8 @@ async function toggleKnob(knob, value) { PERSISTED CONFIG EDITOR ═══════════════════════════════════════ */ +let _profilesCache = null; + function renderConfig(config) { const list = document.getElementById('config-list'); if (!config || !Object.keys(config).length) { @@ -519,6 +524,8 @@ function renderConfig(config) { control = `
`; + } else if (key === 'routing.default_profile') { + control = profileSelect(val); } else if (ENUM_VALUES[key]) { control = enumSelect(key, val, { dataAttr: 'data-config-input' }); } else { @@ -532,9 +539,46 @@ function renderConfig(config) { }); }).join(''); list.innerHTML = html; + updateDefaultProfileHint(); markDirty(); } +function profileSelect(currentValue) { + // Always build a fresh option list so a failed /admin/api/profiles fetch or a + // hand-edited default_profile value never leaves the control blank. If the + // persisted value is not in the live list, render it as the lone option so + // the operator still sees (and can save back) the current value. + const profiles = _profilesCache || []; + const options = profiles.slice(); + const optionNames = new Set(profiles.map(p => p.name)); + if (currentValue !== null && currentValue !== undefined && currentValue !== '' && !optionNames.has(currentValue)) { + options.push({ name: currentValue, zero_admit: false, source: 'orphan' }); + } + const opts = options.map(p => + `` + ).join(''); + return ``; +} + +function updateDefaultProfileHint() { + const row = document.querySelector('#config-list .setting-row[data-key="routing.default_profile"]'); + if (!row) return; + const input = row.querySelector('[data-profile-select]'); + if (!input) return; + const profiles = _profilesCache || []; + const selected = profiles.find(p => p.name === input.value); + const meta = row.querySelector('.setting-meta'); + const existing = meta.querySelector('.default-profile-hint'); + if (existing) existing.remove(); + if (selected && selected.zero_admit) { + const hint = document.createElement('span'); + hint.className = 'badge bg-warning default-profile-hint ms-2'; + hint.title = 'This profile currently admits no models'; + hint.innerHTML = `${icon('alert', 12)} admits 0 models`; + meta.appendChild(hint); + } +} + /* The inputs hold the values, so the only thing worth flagging is an edit that hasn't been written yet: an amber rail on the row, a live count on Save. */ function rowValue(row) { @@ -604,6 +648,10 @@ async function saveAllConfig() { async function loadControls() { const runtime = await apiFetch(`${API}api/runtime`); if (runtime) renderRuntime(runtime); + if (!_profilesCache) { + const profiles = await apiFetch(`${API}api/profiles`); + if (profiles) _profilesCache = profiles; + } const config = await apiFetch(`${API}api/config`); if (config) renderConfig(config); } @@ -625,7 +673,12 @@ function init() { // Delegated so it survives every re-render of the list. const configList = document.getElementById('config-list'); configList.addEventListener('input', markDirty); - configList.addEventListener('change', markDirty); + configList.addEventListener('change', (e) => { + markDirty(); + if (e.target && e.target.dataset && e.target.dataset.profileSelect != null) { + updateDefaultProfileHint(); + } + }); loadControls(); connectSSE(); } diff --git a/admin/frontend/decisions.html b/admin/frontend/decisions.html index 016b5aa..e0f51c0 100644 --- a/admin/frontend/decisions.html +++ b/admin/frontend/decisions.html @@ -171,6 +171,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important} diff --git a/admin/frontend/index.html b/admin/frontend/index.html index ea85895..a51ef4b 100644 --- a/admin/frontend/index.html +++ b/admin/frontend/index.html @@ -277,6 +277,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important} diff --git a/admin/frontend/models.html b/admin/frontend/models.html index 4b0e606..9df72ec 100644 --- a/admin/frontend/models.html +++ b/admin/frontend/models.html @@ -156,6 +156,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important} diff --git a/admin/frontend/profiles.html b/admin/frontend/profiles.html new file mode 100644 index 0000000..5ddc646 --- /dev/null +++ b/admin/frontend/profiles.html @@ -0,0 +1,696 @@ + + + + + +Profiles · LLM Router Admin + + + + + + + + + + + + + + + + +
+
+ +
+
+
+
+
+
+
Loading…
+
+
+
+
+
+
+ +
+
+ + + + + + + + + + + + + diff --git a/config/config.yaml b/config/config.yaml index 9323185..df5545d 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -291,6 +291,12 @@ routing: # tolerate flex latency globally. default_flex_preference: auto + # Bare `auto` resolves to this profile. This is the profile name the router + # uses when the client does not specify one explicitly. It must name a + # built-in profile or a config-defined profile; the value is validated at + # load and will be editable from the admin Controls page in a later wave. + default_profile: "default" + # Minimum tool_use_agentic proficiency required of a model when the REQUEST # carries tool definitions. A filter, not a weight, because it is a # capability requirement rather than a preference. diff --git a/plans/admin-profile-management.md b/plans/admin-profile-management.md new file mode 100644 index 0000000..58bdf5a --- /dev/null +++ b/plans/admin-profile-management.md @@ -0,0 +1,169 @@ +# Admin portal: profile visibility and selection + +**Status: FINAL — decision-complete. §C was answered by the user on +2026-09-04: A, B and C are all in scope.** Written 2026-09-04 against `main` at `d08aed3`. + +## The principle this serves + +Stated by the user: the admin portal does not need 100% coverage of every +configurable lever, but **the main features should each have basic +functionality and configurability there**. Named routing profiles shipped in +PR #24 with no portal surface at all — they exist only as code defaults and an +`auto:` string a client has to know to send. That fails the bar. + +## What exists today + +- `BUILTIN_PROFILES` in `src/dispatcher.py:110-116` — five profiles, defined as + predicates over catalog columns: + + | profile | definition | + |---|---| + | `default` | `RoutingProfile()` | + | `batch` | `latency_tolerance="batch"` | + | `locality` | `provider="ollama-local"` | + | `bigboybritches` | `min_tier=3` | + | `onlycheaps` | `max_cost_per_1m_completion=0.5` | + +- `config.RoutingProfile` (`src/config.py:197`) — fields `provider`, + `min_tier`, `max_tier`, `latency_tolerance`, + `max_cost_per_1m_completion`, `allowed_model_ids`. All optional. +- A `profiles:` config section is supported but **no block exists in + `config/config.yaml`**, so today every profile is a code default. +- **No `routing.default_profile` knob.** Bare `auto` resolves to the built-in + `default` and there is no way to change that without editing code. +- `admin.py:_CONFIG_ALLOWLIST` — dotted config paths an operator may edit. + Writes go through comment-preserving `ruamel.yaml` round-trip, are validated + through `RouterConfig` before any byte reaches disk, and back up first. + `routing.default_flex_preference` is already on it and is the **exact + precedent** for what Part B needs. +- Admin pages: `index.html`, `models.html`, `decisions.html`, `controls.html`. + `decisions.html` already has a Profile column and filter (PR #24). + +## Part A — Show what profiles are and what they resolve to + +Read-only, and the highest value per unit of work. + +A profile's *definition* is not the useful fact; what it **currently admits** +is. Those differ in ways that are invisible today. Measured on the live +catalog on 2026-09-04: + +- `bigboybritches` (`min_tier=3`) matches **7** models — but only **4** are + reachable interactively, because three are `-flex` rows and the default + `latency_tolerance=interactive` filters them at the hard-filter stage. +- `locality` matches **1** model, and only for `file_summarization` / + `diff_checking`, because per-model `eligible_categories` ANDs on top. +- `onlycheaps` matches **4**, and the cheapest is the LOCAL model at + $0.229/1M completion — a measured electricity figure sitting in the same + column as cloud list prices. + +None of that is derivable from reading `min_tier=3`. + +**Add a Profiles panel** (new `profiles.html`, or a section on `models.html` — +implementer's choice, state which and why) showing per profile: its name, +whether it is built-in or config-defined, its definition rendered as fields, +the **count and list of models it currently admits**, and the count reachable +under `interactive` specifically. + +Compute admitted sets by calling `routing.select_candidates` with the +profile's `restrict_to`, exactly as live routing does — do NOT reimplement the +predicate. `metrics.context_ceilings` is the precedent: it delegates to +`select_candidates` so its numbers cannot drift from routing's. + +**A profile that currently admits ZERO models must be visibly flagged.** That +is the shape of the 422 that cost ~19 hours in incident #3, and a profile can +reach it silently through admin deprecations — exactly what happened to the +vision-capable set on 2026-09-04. + +## Part B — Let the operator choose the default profile + +Add `routing.default_profile` (string, defaults to `"default"`), and put it on +`_CONFIG_ALLOWLIST` so it is editable from `controls.html` alongside +`routing.default_flex_preference`. + +Effect: bare `auto` — which is what `opencode.json` and every existing client +sends — resolves to the operator's chosen profile instead of the hard-coded +`default`. That is what makes profiles *usable* without touching client +config, and it is the smallest change that turns them from a curiosity into an +operational lever. + +Constraints: + +- Validate against the known profile set at config load. An unknown name must + fail loudly at load, consistent with how `auto:nonsense` returns 422 rather + than silently degrading. +- `auto:` on a request still wins over the default. The default only + fills in for a bare `auto`. +- The control must be a **select populated from the live profile list**, not a + free-text field. A typo here silently redirects all default traffic. +- Warn in the UI if the chosen default currently admits zero models. + +## Part C — SCOPE DECISION: create and edit custom profiles? + +**DECIDED 2026-09-04 by the user: IN SCOPE.** Ship A, B and C together. + +The cost note below still stands and should shape sequencing — do A and B +first so the read-only view and the default selector exist before CRUD is +layered on. C is most of the work; treat it as its own wave. + +The honest cost difference: `_CONFIG_ALLOWLIST` edits **named scalars**. A +profile is a structured object with six optional fields, one of them a set of +model ids. Persisting one means new machinery — a nested-write path through +the ruamel round-trip, a create/delete flow, and a UI for six fields rather +than a text input. That is most of the work in this plan. + +**Sequencing, not deferral:** build A and B before C. A and B make profiles +visible, understandable and selectable — the "basic functionality and +configurability" bar — at a fraction of the cost, and they give C something to +build on: the read-only panel from A is where a created profile is verified, +and B's `default_profile` is what C's delete-guard has to protect. Landing C +first would mean writing CRUD against a surface nobody can see. + +### C requirements, now that it is in scope + +- **Reuse the validated-write path**: comment-preserving `ruamel.yaml` + round-trip, `RouterConfig` validation before any byte reaches disk, backup + first. Do not add a second way to write `config.yaml`. +- **Built-ins are not editable or deletable.** Render them read-only. A + config-defined profile whose name collides with a built-in must be **rejected + at config load** with a clear message, not silently override or be silently + ignored. Ambiguity about which definition is live would be worse than either. +- **`allowed_model_ids` needs a multi-select populated from the live catalog**, + not free text. A typo'd model id yields a profile that silently admits fewer + models than intended — the failure this plan exists to make visible. +- **Deleting the profile named by `routing.default_profile` must be refused**, + with a message saying which setting depends on it. Same for a rename. The + alternative is a config that fails to load on next start, which is the + refuse-at-load behaviour of Part B turned into an outage. +- **Creating a profile that admits zero models is allowed but must warn** on + save, consistent with Part A's flag. It may be deliberate (a profile for a + model not yet in the catalog); it must not be silent. +- Deleting or editing a profile takes effect on reload like any other persisted + config edit. Say so in the UI rather than implying it is instant. + +## Non-goals + +- No ranking changes. Profiles filter; they do not reorder. (Settled in + `plans/named-routing-profiles.md` §3: filter-only.) +- No new profile *fields* on `RoutingProfile`. +- Do not surface profiles as runtime-only toggles that reset on restart. A + default profile is a deployment preference and belongs in persisted config. +- Do not let the portal auto-fix a zero-model profile by relaxing it. Warn; + the operator decides. +- Do not touch `auto:batch` resolution. + +## Success criteria + +- The portal lists every profile — built-in and config-defined — with its + definition and the models it **currently admits**, computed via + `routing.select_candidates`, not a reimplementation. +- A profile admitting zero models is visibly flagged. +- `routing.default_profile` is editable from `controls.html` as a select + populated from the live profile list, persists through the validated + round-trip write, and takes effect for bare `auto` after reload. +- An invalid `default_profile` fails at config load with a clear message. +- A test proves `auto:` still overrides the configured default. +- Full suite green with `local_energy.enabled` both true and false. +- The user's `config/config.yaml` tariff lines remain uncommitted and verbatim. + **Note for the implementer: this plan writes to `config/config.yaml` + programmatically. The write path must preserve unrelated lines — the tariff + included — and that is worth an explicit test, not an assumption.** diff --git a/plans/classifier-fallback-cascade.md b/plans/classifier-fallback-cascade.md new file mode 100644 index 0000000..237b7e4 --- /dev/null +++ b/plans/classifier-fallback-cascade.md @@ -0,0 +1,131 @@ +# Classifier failure should degrade gradually, not to a fixed guess + +**Status: FINAL — decision-complete.** Written 2026-09-04. + +## The gap + +`classify()` (`src/dispatcher.py:~555-590`) has exactly one endpoint +(`_classifier_client()`, `cfg.classifier.base_url`) and one failure behaviour: +return `cfg.classifier.fallback_category` / `fallback_tier` — currently +`general_chat` / tier 2 — flagged `source="fallback"`. + +That is a fixed guess, and `general_chat` is the worst possible one: +`proficiency_score` is the ONLY category-dependent term in ranking, so a +request mislabelled `general_chat` loses the single signal that makes routing +category-aware. Every request in the outage window routes as if it were small +talk. + +**The session cache cannot help, because of where it sits.** +`session_cache.get()` is called at `dispatcher.py:3077`, *before* classifying. +`classify()` resolves its own failure internally and never sees the session +key. So a session with 200 prior turns, every one classified +`coding_refactor`, still degrades to `general_chat` the moment the local model +is unavailable. + +## Why this matters now, and why it hasn't yet + +`source='fallback'` appears **zero times in 16,744 recorded decisions** +(`cached` 11,089, `classifier` 5,631, `override` 24). The path is real but +cold — which is precisely why it should be hardened rather than trusted: +untested code that only runs during an outage is the worst kind. + +And it is about to get warmer. The operator stops Ollama to play games on the +same GPU. That is a deliberate, recurring, whole-session outage of the local +classifier — exactly the condition this path exists for. + +## The fix: a cascade, cheapest first + +Replace the single static fallback with an ordered cascade. Each step is tried +only if the previous failed, and each records a **distinct** +`classification_source` so the fallback mix is visible in `route_decisions` +rather than collapsed into one bucket. + +1. **Local classifier** — unchanged. `source='classifier'`. +2. **Stale session reuse** — `source='session_stale'`. On classifier failure, + reuse this session's most recent classification **ignoring the staleness + bound**. A stale but real classification of the same conversation beats a + fixed guess, and this is free: no network, no tokens, works when everything + external is down. +3. **Durable session history** — `source='session_history'`. If the in-memory + cache has nothing (it is module-level and resets on restart, so a router + restart during an Ollama outage empties it), read the most recent + `task_category` / `task_tier` for this `session_key` from + `route_decisions`. One indexed query on a table that is already written on + every decision. +4. **Cloud classifier** — `source='classifier_cloud'`. Optional, off unless + configured. See below. +5. **Static fallback** — unchanged, `source='fallback'`. Last resort only. + +Steps 2 and 3 are the valuable ones: free, offline, and they cover the actual +scenario (a running session whose local model went away mid-conversation). +Step 4 covers a *new* session started during an outage, which steps 2-3 cannot. + +## The cloud classifier step + +`cfg.classifier` already accepts any OpenAI-compatible endpoint, so this is a +**second** endpoint plus failover, not new transport. + +It is worth having, and it is cheap. Measured previously and recorded in +`CLAUDE.md`: `deepseek-v4-flash` classifying the same prompts returned +**1.02s mean, 5/5 categories correct, 0 hard failures**, at **$0.093 per 1,000 +calls** — 0.19% of the plan allowance. Against a local `qwen3.5` that averaged +11.58s with one hard failure. + +Requirements: + +- New optional block (e.g. `classifier.cloud_fallback`) with its own + `base_url`, `model`, `api_key_env`, `timeout_seconds`. **Default disabled** — + it spends the user's credits, so it must be opt-in. +- **Do not reuse the primary `classifier` block's fields.** The whole point is + a different endpoint. +- `max_retries=0`, same as the primary. The primary's timeout already becomes a + 3x wall-clock bound with SDK retries on; do not repeat that mistake. +- Its timeout must be **short**. This runs only after the local attempt already + failed, so the request has already spent that budget. A slow cloud fallback + turns one bad request into two. +- **Never fall back to cloud when the account is out of credits.** Plan 5 added + `_account_level_refusal`; a cloud classifier call during a credit outage + wastes latency to reach a foregone 4xx. Skip step 4 if a recent + account-level refusal is known. + +## Observability + +The distinct `classification_source` values are the point. Today every +degradation is one bucket, so an operator cannot tell "Ollama is down but +sessions are being reused correctly" from "everything failed, we are guessing". + +- `metrics.scoring_coverage` warnings: warn when the non-`classifier`, + non-`cached` share of recent decisions exceeds a threshold. That is the + signal that the classifier is down, and nothing currently reports it. +- Surface the source mix on the admin dashboard. `decisions.html` already + displays `source`; the values just need to be recognised and styled. + +## Non-goals + +- Do not change `fallback_category` / `fallback_tier` semantics or defaults. + They remain the last resort. +- Do not make the cloud classifier the primary. Local-first is the project's + premise, and the local model currently wins on latency (1.07s vs 1.02s is + parity, and local costs no credits). +- Do not classify with a *dispatch* model as a side effect. Step 4 is an + explicit configured endpoint, not an opportunistic reuse of a routed call. +- Do not extend the session-cache staleness bound for the normal path. Step 2 + ignores staleness ONLY on the failure path; normal operation keeps its + existing freshness guarantee. + +## Success criteria + +- A test with the classifier raising and a populated session cache yields the + session's prior category with `source='session_stale'`, not `general_chat`. +- A test with the classifier raising and an EMPTY in-memory cache but existing + `route_decisions` rows for that `session_key` yields + `source='session_history'`. +- A test with the classifier raising, no session data, and cloud fallback + disabled yields `source='fallback'` — the current behaviour, preserved. +- A test with cloud fallback configured and the local classifier raising yields + `source='classifier_cloud'`. +- Cloud fallback is skipped when a recent account-level refusal is known. +- A warning fires when the degraded share of recent decisions crosses the + threshold. +- Full suite green with `local_energy.enabled` both true and false. +- The user's `config/config.yaml` tariff lines remain uncommitted and verbatim. diff --git a/plans/quota-balance-and-burn-rate.md b/plans/quota-balance-and-burn-rate.md new file mode 100644 index 0000000..8b36a8f --- /dev/null +++ b/plans/quota-balance-and-burn-rate.md @@ -0,0 +1,140 @@ +# Quota: track balance and burn rate, not percentage of plan + +**Status: FINAL — decision-complete.** Written 2026-09-04 from live data. + +## The modelling error + +`metrics.quota_burn` reports usage as a fraction of `plan_kwh_per_period`, and +`coverage.warnings` renders it as: + +> metered usage is 146% of the 6.25 kWh plan allowance. **A quota is a wall, +> not a bill — requests fail rather than costing more.** + +That claim is false for this account. Usage sailed past 100% and nothing +failed, because the provider bills overage against a credit balance. The +warning is simultaneously alarming and unactionable: it says a wall was hit +when no wall exists, and it offers no number an operator can act on. + +Meanwhile the authoritative signal — **the provider reports +`allowance_remaining_usd` on every single response** — is captured +(`config/schema.sql:156`, read at `src/dispatcher.py:1416`), persisted to +`energy_observations`, and then used by exactly one thing: `seed_energy.py`, +for sweep accounting. Nothing watches it. + +## What the recorded data actually shows + +Queried 2026-09-04 from `energy_observations`: + +| date | calls | balance range (USD) | +|---|---|---| +| 2026-09-04 | 1,853 | 19.70 → **12.19** | +| 2026-09-03 | 174 | 19.95 → 19.70 | +| 2026-09-02 | 3,573 | 20.01 → **0.0071** | +| 2026-09-01 | 3,116 | 14.34 → 4.34 | + +The 2026-09-02 provider outage is in the table. The balance walked down through +`$0.4991 → $0.4967 → $0.4924` at 05:54:26–05:54:32 and bottomed at **$0.0071**. +**The router observed the balance approaching zero, request by request, and +said nothing.** Hours of warning were available and discarded. + +Current state: **$12.19 remaining**, burning **$7.51 over 1,853 calls in +14.3 hours** ≈ **$0.52/hour** ≈ **~23 hours of runway**. + +That is the sentence the dashboard should be showing. "146% of plan" is not. + +## Three defects, in order of value + +### 1. The wrong signal + +Warn on **balance and projected time-to-zero**, sourced from +`allowance_remaining_usd`, not on percentage of a kWh plan. + +- Headline: current balance, recent burn rate, projected hours remaining. +- Warn when projected runway drops below a configurable threshold (hours, not + percent). Default it to something an operator can act within — a few hours. +- Keep the kWh plan figure as **secondary context**. It is still the right unit + for the subscription; it is the wrong thing to alarm on. + +**The implementation trap, and it is the whole difficulty:** a top-up makes +the balance JUMP UP. On 2026-09-02 it went `0.0071 → 20.0071`. A naive +`MAX - MIN` over a window reports a burn of ~$20 when the real burn was ~$20 +*down* plus a $20 credit. Compute burn from **consecutive decreasing deltas +only**, or segment the series at every increase and use the most recent +segment. A test must cover a window containing a top-up; getting this wrong +produces a confidently wrong runway estimate, which is worse than none. + +Handle `allowance_remaining_usd IS NULL` (older rows, and any response that +omits it) by excluding those rows, not by treating them as zero. + +### 2. The wrong window + +`quota_burn` sums `energy_kwh` over a **30-day rolling window** while the +subscription resets monthly on `objective.billing_reset_day` (currently 6). +Those are different periods, so the reported fraction does not correspond to +the billing period it appears to describe. It also returns +`reset_date = today − 30 days`, which is the rolling-window *start* named as +if it were a billing reset — the same confusion `billing_reset_day` was added +to fix in the admin modal. + +Compute the kWh figure over **the current billing period** (since the most +recent `billing_reset_day`), and rename the rolling-window field so it cannot +be mistaken for a reset date. Keep the rolling figure if it is useful, but +label it honestly. + +### 3. The wrong words + +`"A quota is a wall, not a bill — requests fail rather than costing more"` +appears in the warning text and the same framing is in `config/config.yaml` +(`max_energy_per_request`, `plan_kwh_per_period` comments) and in `CLAUDE.md`. +It is demonstrably false for this account and it changes what an operator +does: a wall means "stop", overage means "you are being billed, decide if that +is fine." + +Correct all three places. State plainly that overage is billed against a +credit balance, and that `plan_kwh_per_period` **gates nothing** — verified: +it appears only in `metrics.py` reporting and the admin allowlist. + +## Surfacing + +- Admin dashboard quota chip: show **balance and runway** as the headline, + percentage-of-plan demoted to detail. +- `/metrics` `quota` block gains the balance/burn fields alongside the existing + ones. Do not remove existing keys — the TUI and admin snapshot read them. + +## Non-goals + +- **Do not gate or refuse requests on quota.** `plan_kwh_per_period` gates + nothing today and this plan does not change that. Refusing traffic because a + local estimate says the balance is low would turn a billing question into an + outage — and the estimate can be wrong (see the top-up trap). +- Do not add a second source of truth. `allowance_remaining_usd` is the + provider's own number; do not reconstruct a balance from summed `cost_usd`. +- Do not touch `max_energy_per_request` behaviour. +- Do not change how `energy_observations` rows are written. + +## Success criteria + +- `/metrics` `quota` reports current balance, burn rate over a stated window, + and projected hours to zero, all derived from `allowance_remaining_usd`. +- A test with a top-up inside the window (balance increasing) produces a + correct burn rate, not one inflated by the credit. +- A test with all-NULL `allowance_remaining_usd` degrades gracefully rather + than reporting a zero balance. +- The kWh figure is computed over the current billing period, and no field + named `reset_date` returns a rolling-window start. +- The "wall, not a bill" framing is corrected in `metrics.py`, + `config/config.yaml` and `CLAUDE.md`, and states that `plan_kwh_per_period` + gates nothing. +- The admin quota chip leads with balance and runway. +- Existing `/metrics` `quota` keys still present. +- Full suite green with `local_energy.enabled` both true and false. +- The user's `config/config.yaml` tariff lines remain uncommitted and verbatim. + +## The pattern worth naming + +This is the third plan in a row whose finding is **"the signal was already +recorded and nothing surfaced it"** — after the capability-gated ceiling +collapse and the unread `route_decisions` rejections. The router is good at +capturing evidence and poor at putting it where an operator looks. Worth +considering a general review of what is persisted versus what is displayed, +rather than a fourth one-off detector. diff --git a/plans/t10-controls-default-profile-1400.png b/plans/t10-controls-default-profile-1400.png new file mode 100644 index 0000000..2e71742 Binary files /dev/null and b/plans/t10-controls-default-profile-1400.png differ diff --git a/plans/t10-controls-default-profile-800.png b/plans/t10-controls-default-profile-800.png new file mode 100644 index 0000000..39133c8 Binary files /dev/null and b/plans/t10-controls-default-profile-800.png differ diff --git a/plans/t11-profiles-crud-1400.png b/plans/t11-profiles-crud-1400.png new file mode 100644 index 0000000..60f664c Binary files /dev/null and b/plans/t11-profiles-crud-1400.png differ diff --git a/plans/t11-profiles-crud-800.png b/plans/t11-profiles-crud-800.png new file mode 100644 index 0000000..e782c43 Binary files /dev/null and b/plans/t11-profiles-crud-800.png differ diff --git a/plans/t9-profiles-1400.png b/plans/t9-profiles-1400.png new file mode 100644 index 0000000..75e3818 Binary files /dev/null and b/plans/t9-profiles-1400.png differ diff --git a/plans/t9-profiles-800.png b/plans/t9-profiles-800.png new file mode 100644 index 0000000..53555a1 Binary files /dev/null and b/plans/t9-profiles-800.png differ diff --git a/src/admin.py b/src/admin.py index ea029d2..56513ca 100644 --- a/src/admin.py +++ b/src/admin.py @@ -29,16 +29,25 @@ import uuid from collections.abc import Callable from datetime import datetime, timezone from pathlib import Path -from typing import Any, List, Optional +from typing import Any, List, Literal, Optional from fastapi import APIRouter, BackgroundTasks, HTTPException from fastapi.responses import FileResponse from openai import OpenAI, OpenAIError from pydantic import BaseModel, ValidationError from ruamel.yaml import YAML +from ruamel.yaml.comments import CommentedMap +from ruamel.yaml.error import YAMLError import metrics -from config import FlexPreference, RouterConfig, load_config +import routing +from config import ( + BUILTIN_PROFILES, + FlexPreference, + RouterConfig, + RoutingProfile, + load_config, +) _logger = logging.getLogger(__name__) @@ -307,6 +316,7 @@ _CONFIG_ALLOWLIST: dict[str, tuple[str, ...]] = { "pinch.enabled": ("pinch", "enabled"), "pinch.relevance.enabled": ("pinch", "relevance", "enabled"), "routing.default_flex_preference": ("routing", "default_flex_preference"), + "routing.default_profile": ("routing", "default_profile"), } # Order preserves config.yaml layout for the GET response. @@ -321,6 +331,7 @@ _CONFIG_GET_ORDER: list[str] = [ "pinch.enabled", "pinch.relevance.enabled", "routing.default_flex_preference", + "routing.default_profile", ] @@ -337,14 +348,6 @@ def _dict_get_at(store: Any, path: tuple[str, ...]) -> Any: return cur -def _dict_set_at(store: Any, path: tuple[str, ...], value: Any) -> None: - """Set *value* at dotted *path* in a plain-nested mapping.""" - cur = store - for part in path[:-1]: - cur = cur[part] - cur[path[-1]] = value - - def load_config_store(config_path: Path) -> Any: """Load *config_path* as a ruamel CommentedMap so comments survive a dump.""" yaml = YAML() @@ -352,22 +355,64 @@ def load_config_store(config_path: Path) -> Any: return yaml.load(config_path.read_text()) -def _persist_config_value( - config_path: Path, path: tuple[str, ...], value: Any -) -> None: - """Atomically persist *value* at dotted *path* in *config_path*. +def load_config_store_safe(config_path: Path) -> Any: + """``load_config_store`` that surfaces YAML corruption as a clean 503. - The load/validate/backup/write sequence runs under the module-level - ``_config_write_lock`` so concurrent writes cannot interleave. The write is - a ruamel.yaml round-trip (comments survive), the WHOLE config is re- - validated via ``RouterConfig`` before any byte touches disk, and the on- - disk replacement uses a ``.tmp`` file + ``os.replace`` so a reader never - observes a truncated config.yaml. Raises ``ValidationError`` (leaving no - backup on disk) if the candidate config is invalid. + config.yaml is hand-editable; a corrupted file (duplicate keys, bad + syntax) must not crash the admin endpoints with a raw 500. The detail + carries the ruamel error (which names the offending line and column) + and points the operator at the config.yaml.bak.* backups. + """ + try: + return load_config_store(config_path) + except YAMLError as exc: + raise HTTPException( + status_code=503, + detail=( + f"config.yaml could not be parsed: {exc} " + f"— fix the file or restore a config.yaml.bak.* backup" + ), + ) from exc + + +def _persist_config_block( + config_path: Path, + block_path: tuple[str, ...], + value: Any, + *, + delete: bool = False, +) -> None: + """Atomically persist *value* at dotted *block_path* in *config_path*. + + Unlike the scalar ``_persist_config_value`` helper, this creates + intermediate mappings when absent and supports deleting a leaf block while + pruning an empty parent. The same discipline applies: whole-config + validation via ``RouterConfig`` BEFORE any byte touches disk, a backup copy + after validation, and ``.tmp`` + ``os.replace`` atomic swap. """ with _config_write_lock: - store = load_config_store(config_path) - _dict_set_at(store, path, value) + store = load_config_store_safe(config_path) + cur = store + for part in block_path[:-1]: + nxt = cur.get(part) + if nxt is None: + nxt = CommentedMap() + cur[part] = nxt + cur = nxt + leaf = block_path[-1] + if delete: + if leaf not in cur: + raise KeyError(leaf) + del cur[leaf] + if not cur: + parent_path = block_path[:-1] + if parent_path: + parent = store + for part in parent_path[:-1]: + parent = parent[part] + del parent[parent_path[-1]] + else: + cur[leaf] = value RouterConfig(**store) backup = config_path.with_name( f"config.yaml.bak.{int(time.time())}" @@ -379,6 +424,16 @@ def _persist_config_value( os.replace(tmp, config_path) +def _persist_config_value( + config_path: Path, path: tuple[str, ...], value: Any +) -> None: + """Atomically persist *value* at dotted *path* in *config_path*. + + Thin wrapper around ``_persist_config_block`` preserving the historic + scalar-path signature used by tests. + """ + _persist_config_block(config_path, path, value) + class _AvailabilityBody(BaseModel): availability: str @@ -406,6 +461,25 @@ def _set_at(cfg: Any, path: tuple[str, ...], value: Any) -> None: setattr(cur, path[-1], value) +class _ProfileCreateBody(BaseModel): + name: str + provider: Optional[str] = None + min_tier: Optional[int] = None + max_tier: Optional[int] = None + latency_tolerance: Optional[Literal["interactive", "batch"]] = None + max_cost_per_1m_completion: Optional[float] = None + allowed_model_ids: Optional[list[str]] = None + + +class _ProfileUpdateBody(BaseModel): + provider: Optional[str] = None + min_tier: Optional[int] = None + max_tier: Optional[int] = None + latency_tolerance: Optional[Literal["interactive", "batch"]] = None + max_cost_per_1m_completion: Optional[float] = None + allowed_model_ids: Optional[list[str]] = None + + def _runtime_state(cfg: Any) -> dict: """Read every toggle knob's current in-memory value off ``cfg``.""" return { @@ -449,6 +523,7 @@ def build_router( _admin_controls = _REPO_ROOT / "admin" / "frontend" / "controls.html" _admin_models = _REPO_ROOT / "admin" / "frontend" / "models.html" _admin_decisions = _REPO_ROOT / "admin" / "frontend" / "decisions.html" + _admin_profiles = _REPO_ROOT / "admin" / "frontend" / "profiles.html" _admin_logo = _REPO_ROOT / "admin" / "frontend" / "6krrt-logo.webp" # No-cache: these pages get hand-edited and reloaded constantly during @@ -477,10 +552,287 @@ def build_router( def admin_decisions_page() -> FileResponse: return FileResponse(_admin_decisions, media_type="text/html", headers=_NO_CACHE_HEADERS) + @router.get("/profiles") + def admin_profiles_page() -> FileResponse: + return FileResponse(_admin_profiles, media_type="text/html", headers=_NO_CACHE_HEADERS) + @router.get("/api/health") def admin_health() -> dict: return {"status": "ok"} + @router.get("/api/profiles") + def admin_profiles() -> list: + """All routing profiles with live admission counts. + + Counts are produced by the same routing predicates that live requests + use (``routing.restrict_to_from_profile`` + ``routing.select_candidates``) + so the admin UI can never drift from real dispatch eligibility. The + canonical probe uses ``required_context_tokens=0`` and ``required_tier=1`` + so the count reflects profile filtering only; a secondary interactive + probe rounds flex rows out of the picture. + + Config profiles are enumerated from the persisted config.yaml store + (not the in-memory ``cfg``) so the list reflects CRUD writes made + through this endpoint without a service restart. + """ + conn = _db_callable() + try: + rows = [dict(r) for r in conn.execute("SELECT * FROM models")] + exclude_models = metrics._admin_deprecated_models(conn) + + result: list[dict] = [] + for name, profile in BUILTIN_PROFILES.items(): + candidates, interactive = _profile_probe( + profile, rows, exclude_models + ) + result.append( + _profile_record(name, "builtin", profile, candidates, interactive) + ) + for name, entry in _persisted_profiles().items(): + try: + profile = RoutingProfile(**entry) + except ValidationError as exc: + raise HTTPException( + status_code=503, + detail=( + f"profiles[{name!r}] in config.yaml is invalid: " + f"{exc.errors()[0]['msg']}" + ), + ) from exc + except TypeError as exc: + # A hand-edited entry that is not a mapping at all. + raise HTTPException( + status_code=503, + detail=( + f"profiles[{name!r}] in config.yaml is invalid: {exc}" + ), + ) from exc + candidates, interactive = _profile_probe( + profile, rows, exclude_models + ) + result.append( + _profile_record(name, "config", profile, candidates, interactive) + ) + return result + finally: + conn.close() + + def _profile_definition(profile) -> dict: + """Serialize a profile as a plain dict, sorting allowed_model_ids for JSON stability.""" + definition = profile.model_dump() + if definition.get("allowed_model_ids") is not None: + definition["allowed_model_ids"] = sorted(definition["allowed_model_ids"]) + return definition + + def _persisted_profiles() -> dict: + """Config-defined profiles from the persisted store (raw, unparsed).""" + store = load_config_store_safe(config_path) or {} + return store.get("profiles") or {} + + def _profile_probe(profile, rows, exclude_models): + """The canonical admission probe shared by GET and the CRUD endpoints. + + Returns ``(candidates, interactive)``: the candidate list under the + profile's own (or default) latency tolerance, and under interactive + tolerance, so a flex-only profile is visible as zero-admit. + """ + restrict_to = routing.restrict_to_from_profile(profile, rows) + latency_tolerance = ( + profile.latency_tolerance or cfg.routing.default_latency_tolerance + ) + candidates = routing.select_candidates( + rows, + required_context_tokens=0, + required_tier=1, + latency_tolerance=latency_tolerance, + allowed_access_levels=cfg.routing.allowed_access_levels, + exclude_stale=cfg.freshness.exclude_stale, + exclude_deprecated=cfg.freshness.exclude_deprecated, + exclude_models=exclude_models, + min_tool_proficiency=None, + require_vision=False, + require_json_mode=False, + task_category=None, + restrict_to=restrict_to, + ) + interactive = routing.select_candidates( + rows, + required_context_tokens=0, + required_tier=1, + latency_tolerance=routing.INTERACTIVE, + allowed_access_levels=cfg.routing.allowed_access_levels, + exclude_stale=cfg.freshness.exclude_stale, + exclude_deprecated=cfg.freshness.exclude_deprecated, + exclude_models=exclude_models, + min_tool_proficiency=None, + require_vision=False, + require_json_mode=False, + task_category=None, + restrict_to=restrict_to, + ) + return candidates, interactive + + def _profile_record( + name: str, source: str, profile, candidates, interactive + ) -> dict: + return { + "name": name, + "source": source, + "definition": _profile_definition(profile), + "admitted_count": len(candidates), + "interactive_count": len(interactive), + "zero_admit": len(candidates) == 0 or len(interactive) == 0, + "admitted_models": [r["model_id"] for r in candidates], + } + + def _profile_block_dict(body) -> dict: + """A profile as a ruamel-dumpable dict: sorted list (or None) ids.""" + value = body.model_dump(exclude={"name"}) + if value.get("allowed_model_ids") is not None: + value["allowed_model_ids"] = sorted(value["allowed_model_ids"]) + return value + + def _probe_candidate_zero_admit(candidate) -> bool: + conn = _db_callable() + try: + rows = [dict(r) for r in conn.execute("SELECT * FROM models")] + exclude_models = metrics._admin_deprecated_models(conn) + candidates, interactive = _profile_probe( + candidate, rows, exclude_models + ) + return len(candidates) == 0 or len(interactive) == 0 + finally: + conn.close() + + def _persist_profile(name: str, profile_dict: dict) -> None: + try: + _persist_config_block(config_path, ("profiles", name), profile_dict) + except ValidationError as exc: + raise HTTPException( + status_code=422, + detail=exc.errors()[0]["msg"], + ) from exc + except KeyError as exc: + raise HTTPException(status_code=404, detail="profile not found") from exc + except FileNotFoundError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + + def _profile_save_response(name, candidate, zero_admit) -> dict: + response = { + "profile": { + "name": name, + "source": "config", + "definition": _profile_definition(candidate), + }, + "zero_admit": zero_admit, + "message": "A restart is required for this change to take effect", + } + if zero_admit: + response["warning"] = ( + "Profile admits zero models under the current catalog" + ) + return response + + @router.post("/api/profiles/") + def admin_profile_create(body: _ProfileCreateBody) -> dict: + """Create a new configured routing profile in config.yaml.""" + name = body.name + if name in BUILTIN_PROFILES: + raise HTTPException( + status_code=403, + detail="built-in profiles are read-only", + ) + if name in _persisted_profiles(): + # The plan does not fix a status for create-over-existing; 409 with + # a pointer at the update endpoint is the operator-friendly choice. + raise HTTPException( + status_code=409, + detail=f"profile {name!r} already exists; use the update " + f"endpoint POST /admin/api/profiles/{name}", + ) + + profile_dict = _profile_block_dict(body) + try: + candidate = RoutingProfile(**profile_dict) + except ValidationError as exc: + raise HTTPException( + status_code=422, + detail=exc.errors()[0]["msg"], + ) from exc + + zero_admit = _probe_candidate_zero_admit(candidate) + _persist_profile(name, profile_dict) + return _profile_save_response(name, candidate, zero_admit) + + @router.post("/api/profiles/{name}") + def admin_profile_update(name: str, body: _ProfileUpdateBody) -> dict: + """Replace an existing configured routing profile in-place. + + The profile named by ``routing.default_profile`` MAY be edited; only + delete (and rename-by-delete) is refused. + """ + if name in BUILTIN_PROFILES: + raise HTTPException( + status_code=403, + detail="built-in profiles are read-only", + ) + if name not in _persisted_profiles(): + raise HTTPException(status_code=404, detail="profile not found") + + profile_dict = _profile_block_dict(body) + try: + candidate = RoutingProfile(**profile_dict) + except ValidationError as exc: + raise HTTPException( + status_code=422, + detail=exc.errors()[0]["msg"], + ) from exc + + zero_admit = _probe_candidate_zero_admit(candidate) + _persist_profile(name, profile_dict) + return _profile_save_response(name, candidate, zero_admit) + + @router.delete("/api/profiles/{name}") + def admin_profile_delete(name: str) -> dict: + """Delete a configured routing profile from config.yaml.""" + if name in BUILTIN_PROFILES: + raise HTTPException( + 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 {}): + raise HTTPException(status_code=404, detail="profile not found") + raw_default = _dict_get_at( + store, _CONFIG_ALLOWLIST["routing.default_profile"] + ) + if raw_default == name: + raise HTTPException( + status_code=422, + detail=( + f"Cannot delete profile {name!r}: it is the current " + f"routing.default_profile. Point that setting at another " + f"profile first." + ), + ) + try: + _persist_config_block( + config_path, ("profiles", name), None, delete=True + ) + except ValidationError as exc: + raise HTTPException( + status_code=422, + detail=exc.errors()[0]["msg"], + ) from exc + except KeyError as exc: + raise HTTPException(status_code=404, detail="profile not found") from exc + return { + "deleted": name, + "message": "A restart is required for this change to take effect", + } + @router.get("/api/models") def admin_models() -> list: """All models with per-category proficiency, for the admin model table.""" @@ -868,7 +1220,7 @@ 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(config_path), path) + return {key: _dict_get_at(load_config_store_safe(config_path), path) for key, path in _CONFIG_ALLOWLIST.items()} @router.post("/api/config/{key}") diff --git a/src/config.py b/src/config.py index 8b25f77..1d1771a 100644 --- a/src/config.py +++ b/src/config.py @@ -246,12 +246,29 @@ class RoutingProfile(StrictModel): return v +# Built-in routing profiles. Profiles are selectable as ``auto:``; they +# restrict the candidate set and may override the effective latency_tolerance. +# Defined in config.py so RouterConfig validators and modules that may not +# import dispatcher (e.g., admin.py, metrics.py) can enumerate them directly. +BUILTIN_PROFILES: dict[str, RoutingProfile] = { + "default": RoutingProfile(), + "batch": RoutingProfile(latency_tolerance="batch"), + "locality": RoutingProfile(provider="ollama-local"), + "bigboybritches": RoutingProfile(min_tier=3), + "onlycheaps": RoutingProfile(max_cost_per_1m_completion=0.5), +} + + class RoutingConfig(StrictModel): allowed_access_levels: list[str] default_latency_tolerance: str # Operator's default stance on flex serving-class rows for requests # that do not state one explicitly. default_flex_preference: FlexPreference = FlexPreference.auto + # Bare ``auto`` resolves to this profile. It names a built-in profile by + # default; operators may override it via config (or the admin Controls page + # in a later wave) to change the implicit behavior of unqualified ``auto``. + default_profile: str = "default" # Applied only when the REQUEST carries tool definitions. None disables it. min_tool_proficiency: Optional[float] = 0.5 tool_use_category: str = "tool_use_agentic" @@ -287,6 +304,13 @@ class RoutingConfig(StrictModel): ) return v + @field_validator("default_profile") + @classmethod + def default_profile_non_empty(cls, v: str) -> str: + if not v or not v.strip(): + raise ValueError("routing.default_profile must be a non-empty string") + return v + class VerificationConfig(StrictModel): local_llm_enabled: bool = True @@ -795,6 +819,40 @@ class RouterConfig(StrictModel): ) return self + @model_validator(mode="after") + def profile_names_do_not_shadow_builtins(self) -> "RouterConfig": + """Configured profile names must not collide with built-in profiles. + + Built-ins are the shared enumeration that admin.py, RouterConfig, and + dispatcher.py all consult; allowing a config profile to shadow one would + make the merge semantics order-dependent and surprise callers. + """ + reserved = set(BUILTIN_PROFILES) + for name in self.profiles: + if name in reserved: + raise ValueError( + f"profiles[{name!r}] collides with a built-in profile. " + f"Reserved names: {sorted(reserved)}" + ) + return self + + @model_validator(mode="after") + def default_profile_names_a_known_profile(self) -> "RouterConfig": + """routing.default_profile must resolve to an existing profile. + + Bare ``auto`` and the implicit default profile both resolve through + this name, so a typo or deletion here would silently change routing. + Valid names are the built-in profiles plus any configured ones; the + non-empty check is handled by the field-level validator above. + """ + valid = sorted(set(BUILTIN_PROFILES) | set(self.profiles)) + if self.routing.default_profile not in valid: + raise ValueError( + f"routing.default_profile {self.routing.default_profile!r} " + f"is not a known profile. Valid: {valid}" + ) + return self + @model_validator(mode="after") def local_dispatch_categories_are_real_categories( self, diff --git a/src/dispatcher.py b/src/dispatcher.py index 1fd704c..63d8be7 100644 --- a/src/dispatcher.py +++ b/src/dispatcher.py @@ -46,7 +46,6 @@ import time from datetime import datetime, timezone from pathlib import Path from statistics import median -from collections.abc import Sequence from typing import Any, Literal, Optional import requests @@ -64,7 +63,7 @@ import local_energy import logs import session_cache from capabilities import detect_capabilities, iter_image_url_values -from config import FlexPreference, RouterConfig, RoutingProfile, load_config +from config import BUILTIN_PROFILES, FlexPreference, RouterConfig, RoutingProfile, load_config from context_prune import ( _text_only, estimate_tokens, @@ -89,6 +88,7 @@ from routing import ( capability_gate_reason, rank_candidates, rejection_reason, + restrict_to_from_profile, select_candidates, ) from verification import ( @@ -104,16 +104,8 @@ from verification import ( # model id and dispatched as asked. ROUTER_MODEL = "auto" -# Built-in routing profiles. Profiles are selectable as `auto:`; they -# restrict the candidate set and may override the effective latency_tolerance. -# Unknown profiles raise 422, so unknown built-ins do not silently fall back. -BUILTIN_PROFILES: dict[str, RoutingProfile] = { - "default": RoutingProfile(), - "batch": RoutingProfile(latency_tolerance="batch"), - "locality": RoutingProfile(provider="ollama-local"), - "bigboybritches": RoutingProfile(min_tier=3), - "onlycheaps": RoutingProfile(max_cost_per_1m_completion=0.5), -} +# Built-in routing profiles are now defined in config.py so config validators +# and other modules can enumerate them without importing dispatcher. # Observations from the fixed reference workload in seed_energy.py. Only # these steer routing; organic traffic is logged for accounting but varies @@ -309,8 +301,14 @@ def _all_profile_names() -> list[str]: def _resolve_profile(requested: str) -> tuple[str, RoutingProfile]: + """Resolve an ``auto`` or ``auto:`` request to a concrete profile. + + A bare ``auto`` fills from ``cfg.routing.default_profile`` so operators can + change the implicit default without rewriting every client request. + ``auto:`` still overrides, exactly as before. + """ if requested == ROUTER_MODEL: - name = "default" + name = cfg.routing.default_profile else: name = requested[len(ROUTER_MODEL) + 1 :] # after "auto:" return _resolve_profile_name(name) @@ -338,6 +336,9 @@ def _resolve_profile_name(name: str) -> tuple[str, RoutingProfile]: return name, (configured if configured is not None else builtin) +_restrict_to_from_profile = restrict_to_from_profile + + def _effective_latency(profile: RoutingProfile, req: Optional[TaskRequest]) -> str: return ( profile.latency_tolerance @@ -346,33 +347,6 @@ def _effective_latency(profile: RoutingProfile, req: Optional[TaskRequest]) -> s ) -def _restrict_to_from_profile(profile: RoutingProfile, rows: Sequence[dict]) -> set[str] | None: - allowed: set[str] = {r["model_id"] for r in rows} - if profile.provider is not None: - allowed &= {r["model_id"] for r in rows if r.get("provider") == profile.provider} - if profile.min_tier is not None or profile.max_tier is not None: - def tier_ok(r: dict) -> bool: - tier = r.get("tier") - if tier is None: - return False - if profile.min_tier is not None and tier < profile.min_tier: - return False - if profile.max_tier is not None and tier > profile.max_tier: - return False - return True - allowed &= {r["model_id"] for r in rows if tier_ok(r)} - if profile.max_cost_per_1m_completion is not None: - allowed &= { - r["model_id"] - for r in rows - if r.get("cost_per_1m_completion") is not None - and r["cost_per_1m_completion"] <= profile.max_cost_per_1m_completion - } - if profile.allowed_model_ids is not None: - allowed &= profile.allowed_model_ids - return allowed if allowed != {r["model_id"] for r in rows} else None - - def _db() -> sqlite3.Connection: conn = sqlite3.connect(cfg.database.path) conn.row_factory = sqlite3.Row @@ -930,7 +904,9 @@ def route( profile_obj: Optional[RoutingProfile] = None, ) -> RouteResponse: if profile_obj is None: - profile_obj = BUILTIN_PROFILES["default"] + profile_obj = _resolve_profile_name( + cfg.routing.default_profile if profile == "default" else profile + )[1] latency_tolerance = _effective_latency(profile_obj, req) if req.task_category and req.task_tier and req.required_context_tokens is not None: @@ -978,7 +954,7 @@ def route( ), task_category=classification.task_category, ) - restrict_to = _restrict_to_from_profile(profile_obj, rows) + restrict_to = restrict_to_from_profile(profile_obj, rows) eligible = select_candidates(rows, restrict_to=restrict_to, **filters) if logs.enabled_for_debug(): # "No model satisfies the hard filters" is otherwise a dead end with no diff --git a/src/routing.py b/src/routing.py index d1cdd85..f241a36 100644 --- a/src/routing.py +++ b/src/routing.py @@ -315,6 +315,49 @@ def apply_flex_preference( return swapped, True, flex_forced, cost +def restrict_to_from_profile( + profile, rows: Sequence[dict] +) -> set[str] | None: + """Return the model ids a profile admits, or None if it admits everything. + + ``profile`` is duck-typed to keep routing.py free of a config import. The + caller (dispatcher.py) passes a ``RoutingProfile`` instance; only these + attributes are read: ``provider``, ``min_tier``, ``max_tier``, + ``max_cost_per_1m_completion``, ``allowed_model_ids``. + """ + allowed: set[str] = {r["model_id"] for r in rows} + if getattr(profile, "provider", None) is not None: + allowed &= { + r["model_id"] for r in rows if r.get("provider") == profile.provider + } + if getattr(profile, "min_tier", None) is not None or getattr( + profile, "max_tier", None + ) is not None: + + def tier_ok(r: dict) -> bool: + tier = r.get("tier") + if tier is None: + return False + min_tier = getattr(profile, "min_tier", None) + max_tier = getattr(profile, "max_tier", None) + return not ( + (min_tier is not None and tier < min_tier) + or (max_tier is not None and tier > max_tier) + ) + + allowed &= {r["model_id"] for r in rows if tier_ok(r)} + if getattr(profile, "max_cost_per_1m_completion", None) is not None: + allowed &= { + r["model_id"] + for r in rows + if r.get("cost_per_1m_completion") is not None + and r["cost_per_1m_completion"] <= profile.max_cost_per_1m_completion + } + if getattr(profile, "allowed_model_ids", None) is not None: + allowed &= profile.allowed_model_ids + return allowed if allowed != {r["model_id"] for r in rows} else None + + def select_candidates( rows: Sequence[dict], *, diff --git a/tests/test_admin_config.py b/tests/test_admin_config.py index 57c66c0..bc38b02 100644 --- a/tests/test_admin_config.py +++ b/tests/test_admin_config.py @@ -24,7 +24,8 @@ import yaml from fastapi import FastAPI from starlette.testclient import TestClient -from admin import _CONFIG_ALLOWLIST, _persist_config_value, build_router +from admin import _CONFIG_ALLOWLIST, _persist_config_block, _persist_config_value, build_router +from config import load_config ROOT = Path(__file__).resolve().parent.parent @@ -55,10 +56,17 @@ def client(tmp_path, monkeypatch): config_yaml = tmp_path / "config" / "config.yaml" shutil.copyfile(ROOT / "config" / "config.yaml", config_yaml) + schema_sql = (ROOT / "config" / "schema.sql").read_text() + conn = _make_db(tmp_path) + conn.executescript(schema_sql) + conn.close() + def _db_factory() -> sqlite3.Connection: return _make_db(tmp_path) - router = build_router(None, _db_factory, base_dir=str(tmp_path)) + cfg = load_config(str(config_yaml)) + + router = build_router(cfg, _db_factory, base_dir=str(tmp_path)) app = FastAPI() app.include_router(router, prefix="/admin") return TestClient(app), config_yaml @@ -126,6 +134,55 @@ def test_config_POST_invalid_value_leaves_file_unchanged(client): assert after == before +def test_config_GET_includes_routing_default_profile(client): + """GET /admin/api/config exposes routing.default_profile with the repo default.""" + tc, _ = client + resp = tc.get("/admin/api/config") + assert resp.status_code == 200 + body = resp.json() + assert "routing.default_profile" in body + assert body["routing.default_profile"] == "default" + + +def test_config_POST_default_profile_persists_valid_builtin(client): + """POST routing.default_profile=batch persists and keeps comments.""" + tc, config_yaml = client + _insert_sentinel(config_yaml) + + resp = tc.post( + "/admin/api/config/routing.default_profile", json={"value": "batch"} + ) + assert resp.status_code == 200 + body = resp.json() + assert body["key"] == "routing.default_profile" + assert body["value"] == "batch" + assert "restart is required" in body["message"] + + text = config_yaml.read_text() + assert _SENTINEL in text + assert re.search( + r'^\s*default_profile:\s*["\']?batch["\']?\s*$', 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.""" + tc, config_yaml = client + before = config_yaml.read_text() + + resp = tc.post( + "/admin/api/config/routing.default_profile", + json={"value": "nosuchprofile"}, + ) + assert resp.status_code == 422 + detail = resp.json()["detail"] + assert "nosuchprofile" in detail + assert "default" in detail or "batch" in detail + + after = config_yaml.read_text() + assert after == before + + def test_config_POST_creates_backup_before_write(client, tmp_path): """A successful write produces a ``config.yaml.bak.`` backup copy.""" tc, config_yaml = client @@ -188,3 +245,128 @@ def test_config_concurrent_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() != "" + +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.""" + tc, config_yaml = client + _insert_sentinel(config_yaml) + + resp = tc.post( + "/admin/api/profiles/", + json={"name": "minit", "max_tier": 2}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["profile"]["name"] == "minit" + assert body["profile"]["definition"]["max_tier"] == 2 + + 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 + + +def test_profile_create_creates_backup_before_write(client, tmp_path): + """A successful profile write produces a config.yaml.bak backup with pre-write contents.""" + tc, config_yaml = client + _insert_sentinel(config_yaml) + + resp = tc.post( + "/admin/api/profiles/", + json={"name": "minit", "max_tier": 2}, + ) + assert resp.status_code == 200 + + backups = sorted(tmp_path.glob("config/config.yaml.bak.*")) + assert len(backups) == 1 + backup_text = backups[0].read_text() + assert _SENTINEL in backup_text + assert "profiles:" not in backup_text + + +def test_profile_create_blocked_nested_path_returns_403(client): + """The scalar config endpoint refuses the new nested profiles namespace.""" + tc, _ = client + resp = tc.post("/admin/api/config/profiles.foo", json={"value": {}}) + assert resp.status_code == 403 + + +def test_profile_create_builtin_name_returns_403(client): + """Creating a profile named after a builtin is refused with 403.""" + tc, _ = client + resp = tc.post("/admin/api/profiles/", json={"name": "batch", "max_tier": 2}) + assert resp.status_code == 403 + assert "built-in" in resp.json()["detail"].lower() + + +def test_config_GET_corrupt_yaml_returns_clean_503(tmp_path): + """A duplicate-key config.yaml refuses with 503, not a raw 500.""" + (tmp_path / "config").mkdir(parents=True, exist_ok=True) + config_yaml = tmp_path / "config" / "config.yaml" + config_yaml.write_text( + (ROOT / "config" / "config.yaml").read_text() + + "profiles:\n a:\n min_tier: 1\nprofiles:\n b:\n min_tier: 2\n" + ) + + cfg = load_config(ROOT / "config" / "config.yaml") + + def _db_factory() -> sqlite3.Connection: + return _make_db(tmp_path) + + router = build_router(cfg, _db_factory, base_dir=str(tmp_path)) + app = FastAPI() + app.include_router(router, prefix="/admin") + tc = TestClient(app, raise_server_exceptions=False) + + resp = tc.get("/admin/api/config") + assert resp.status_code == 503 + assert "config.yaml" in resp.json()["detail"] + + +def test_config_concurrent_block_writes_are_atomic_no_zero_byte_backups(tmp_path): + """Concurrent _persist_config_block writes never truncate config or leave 0-byte backups.""" + config_yaml = tmp_path / "config.yaml" + shutil.copyfile(ROOT / "config" / "config.yaml", config_yaml) + + stop_reader = threading.Event() + + def reader() -> None: + while not stop_reader.is_set(): + try: + text = config_yaml.read_text() + except FileNotFoundError: + continue + assert text.strip() != "", "config.yaml observed empty" + assert "logging:" in text + + reader_thread = threading.Thread(target=reader) + reader_thread.start() + + def writer(name: str, max_tier: int) -> None: + _persist_config_block(config_yaml, ("profiles", name), {"max_tier": max_tier}) + + threads = [ + threading.Thread(target=writer, args=("alpha", 1)), + threading.Thread(target=writer, args=("beta", 2)), + ] + for t in threads: + t.start() + for t in threads: + t.join() + stop_reader.set() + reader_thread.join() + + final = yaml.safe_load(config_yaml.read_text()) + assert final["profiles"]["alpha"]["max_tier"] == 1 + assert final["profiles"]["beta"]["max_tier"] == 2 + + backups = list(tmp_path.glob("config.yaml.bak.*")) + assert backups, "expected at least one backup" + for b in backups: + assert b.stat().st_size > 0, f"zero-byte backup: {b}" + assert b.read_text().strip() != "" + diff --git a/tests/test_admin_frontend.py b/tests/test_admin_frontend.py index ef4f0ff..a089c3f 100644 --- a/tests/test_admin_frontend.py +++ b/tests/test_admin_frontend.py @@ -121,6 +121,24 @@ def test_admin_models_returns_html_with_availability_marker(admin_client): assert "Model Availability" in resp.text +def test_admin_profiles_returns_html_with_profiles_marker(admin_client): + """GET /admin/profiles returns 200, text/html, and contains the Chart.js + tag and the Profiles page marker.""" + resp = admin_client.get("/admin/profiles") + assert resp.status_code == 200 + assert resp.headers["content-type"].startswith("text/html") + assert "chart.js" in resp.text + assert "Profiles" in resp.text + + +def test_admin_pages_include_profiles_nav_link(admin_client): + """Every served admin page body contains the Profiles nav link markup.""" + for path in ["/admin/", "/admin/models", "/admin/profiles", "/admin/decisions", "/admin/controls"]: + resp = admin_client.get(path) + assert resp.status_code == 200 + assert ">Profiles<" in resp.text, f"Profiles nav link missing on {path}" + + def test_quota_modal_templates_next_reset_date(): """The quota modal template references next_reset_date, not reset_date.""" index_path = ROOT / "admin" / "frontend" / "index.html" diff --git a/tests/test_admin_profiles.py b/tests/test_admin_profiles.py new file mode 100644 index 0000000..aa40343 --- /dev/null +++ b/tests/test_admin_profiles.py @@ -0,0 +1,699 @@ +"""Tests for GET /admin/api/profiles. + +Each test builds an isolated admin router over a temp DB and a temp copy of +config.yaml so the real repo config and DB are never touched. The endpoint +must compute admission counts by delegating to ``routing.select_candidates`` +and ``routing.restrict_to_from_profile`` — the tests include a source-level +guard asserting those function names appear in src/admin.py. +""" + +from __future__ import annotations + +import re + +import sqlite3 +from pathlib import Path + +import pytest +from fastapi import FastAPI +from starlette.testclient import TestClient + +from admin import build_router +from config import BUILTIN_PROFILES, load_config + +ROOT = Path(__file__).resolve().parent.parent +SCHEMA_SQL = (ROOT / "config" / "schema.sql").read_text() +ADMIN_TABLE_SQL = """ +CREATE TABLE IF NOT EXISTS admin_model_overrides ( + model_id TEXT NOT NULL, + provider TEXT NOT NULL, + availability TEXT NOT NULL, + reason TEXT, + updated_at TEXT NOT NULL, + PRIMARY KEY (model_id, provider) +); +CREATE INDEX IF NOT EXISTS idx_admin_model_overrides_availability + ON admin_model_overrides (availability); +""" + + +def _make_db(tmp_path: Path) -> sqlite3.Connection: + conn = sqlite3.connect(str(tmp_path / "profiles.db")) + conn.row_factory = sqlite3.Row + return conn + + +def _seed_models(conn: sqlite3.Connection, rows: list[dict]) -> None: + cols = [ + "model_id", "provider", "base_model_id", "display_name", + "cost_per_1m_prompt", "cost_per_1m_completion", "context_window", + "effective_context_window", "max_output_tokens", "tier", + "supports_tools", "supports_json_mode", "supports_vision", + "supports_reasoning", "reasoning_default_enabled", "latency_class", + "reasoning_mode", "context_variant", "access_level", "deprecated", + "availability", "last_updated", + ] + placeholders = ",".join(["?"] * len(cols)) + for r in rows: + values = [r.get(c) for c in cols] + conn.execute( + f"INSERT INTO models ({','.join(cols)}) VALUES ({placeholders})", + values, + ) + conn.commit() + + +def _profile_client(tmp_path, profiles_yaml: str | None = None): + """Build a TestClient for /admin with a temp DB and config.yaml copy.""" + (tmp_path / "config").mkdir(parents=True, exist_ok=True) + config_yaml = tmp_path / "config" / "config.yaml" + + base_text = (ROOT / "config" / "config.yaml").read_text() + if profiles_yaml is not None: + config_yaml.write_text(base_text + profiles_yaml) + else: + config_yaml.write_text(base_text) + + cfg = load_config(str(config_yaml)) + + def _db_factory() -> sqlite3.Connection: + return _make_db(tmp_path) + + router = build_router(cfg, _db_factory, base_dir=str(tmp_path)) + app = FastAPI() + app.include_router(router, prefix="/admin") + return TestClient(app), cfg, config_yaml + + +def _model_row( + model_id: str, + *, + provider: str = "neuralwatt", + tier: int, + cost_completion: float = 1.0, + availability: str = "active", + deprecated: int = 0, + latency_class: str = "standard", +) -> dict: + return { + "model_id": model_id, + "provider": provider, + "base_model_id": model_id, + "display_name": model_id, + "cost_per_1m_prompt": cost_completion * 0.5, + "cost_per_1m_completion": cost_completion, + "context_window": 131072, + "effective_context_window": 65536, + "max_output_tokens": 8192, + "tier": tier, + "supports_tools": 1, + "supports_json_mode": 1, + "supports_vision": 1, + "supports_reasoning": 1, + "reasoning_default_enabled": 1, + "latency_class": latency_class, + "reasoning_mode": "default", + "context_variant": "full", + "access_level": "public", + "deprecated": deprecated, + "availability": availability, + "last_updated": "2026-08-22T00:00:00+00:00", + } + + +@pytest.fixture +def client_no_models(tmp_path): + """Fresh DB, no rows; tests zero_admit and 200 behaviour.""" + client, _cfg, _ = _profile_client(tmp_path) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + conn.close() + yield client, _cfg + + +@pytest.fixture +def client_with_models(tmp_path): + """DB seeded with tiered/cost-differentiated rows.""" + client, _cfg, _ = _profile_client(tmp_path) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + # 1 cheap tier-1 model within onlycheaps cost cap (0.5) + # 2 tier-2 models above cost cap + # 2 tier-3 frontier models + # 1 flex tier-2 model (admitted under batch, excluded under interactive) + rows = [ + _model_row("t1-cheap", tier=1, cost_completion=0.4), + _model_row("t2-mid-a", tier=2, cost_completion=1.5), + _model_row("t2-mid-b", tier=2, cost_completion=1.6), + _model_row("t3-front-a", tier=3, cost_completion=2.0), + _model_row("t3-front-b", tier=3, cost_completion=2.5), + _model_row("t2-flex", tier=2, cost_completion=1.5, latency_class="flex"), + ] + _seed_models(conn, rows) + conn.close() + yield client, _cfg + + +@pytest.fixture +def client_with_local_only(tmp_path): + """DB seeded with only an ollama-local model for the locality profile.""" + client, _cfg, _ = _profile_client(tmp_path) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + rows = [ + _model_row( + "local-model", + provider="ollama-local", + tier=1, + cost_completion=0.0, + ), + ] + _seed_models(conn, rows) + conn.close() + yield client, _cfg + + +# --------------------------------------------------------------------------- +# Source-level guard (grep check) +# --------------------------------------------------------------------------- + + +def test_implementation_uses_routing_helpers(): + """src/admin.py must call select_candidates and restrict_to_from_profile. + + This guards against a future refactor that re-implements the profile + predicate logic inside the endpoint. + """ + text = (ROOT / "src" / "admin.py").read_text() + assert "select_candidates" in text + assert "restrict_to_from_profile" in text + + +# --------------------------------------------------------------------------- +# Enumeration and shape +# --------------------------------------------------------------------------- + + +def _by_name(body: list[dict]) -> dict[str, dict]: + return {r["name"]: r for r in body} + + +def test_profiles_endpoint_lists_builtins_and_config_profiles(tmp_path): + """Endpoint returns all 5 builtins plus a config-defined profile.""" + profiles_block = """ +profiles: + myconfig: + min_tier: 1 + max_tier: 2 +""" + client, cfg, _ = _profile_client(tmp_path, profiles_block) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + conn.close() + + resp = client.get("/admin/api/profiles") + assert resp.status_code == 200 + body = resp.json() + names = [r["name"] for r in body] + + assert names == ["default", "batch", "locality", "bigboybritches", "onlycheaps", "myconfig"] + by_name = _by_name(body) + for builtin_name in BUILTIN_PROFILES: + assert by_name[builtin_name]["source"] == "builtin" + assert by_name["myconfig"]["source"] == "config" + assert by_name["myconfig"]["definition"]["min_tier"] == 1 + assert by_name["myconfig"]["definition"]["max_tier"] == 2 + + +def test_profiles_definition_serialises_allowed_model_ids_set(tmp_path): + """allowed_model_ids is exposed as a sorted stable list, not a set.""" + profiles_block = """ +profiles: + allowlist: + allowed_model_ids: [z-model, a-model, m-model] +""" + client, cfg, _ = _profile_client(tmp_path, profiles_block) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + conn.close() + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["allowlist"]["definition"]["allowed_model_ids"] == ["a-model", "m-model", "z-model"] + + +# --------------------------------------------------------------------------- +# Admission counts +# --------------------------------------------------------------------------- + + +def test_profiles_exact_counts_by_profile(client_with_models): + """Counts match the seeded set and the canonical probe definition.""" + client, cfg = client_with_models + resp = client.get("/admin/api/profiles") + assert resp.status_code == 200 + by_name = _by_name(resp.json()) + + # default: allowed public/active; interactive drops flex -> 5 rows + assert by_name["default"]["admitted_count"] == 5 + assert by_name["default"]["admitted_models"] == [ + "t1-cheap", "t2-mid-a", "t2-mid-b", "t3-front-a", "t3-front-b" + ] + # batch: same as default + flex -> 6 + assert by_name["batch"]["admitted_count"] == 6 + # bigboybritches: tier >= 3 -> 2 + assert by_name["bigboybritches"]["admitted_count"] == 2 + assert by_name["bigboybritches"]["admitted_models"] == ["t3-front-a", "t3-front-b"] + # onlycheaps: completion <= 0.5 -> 1 (the tier-1 cheap model) + assert by_name["onlycheaps"]["admitted_count"] == 1 + assert by_name["onlycheaps"]["admitted_models"] == ["t1-cheap"] + + +def test_locality_profile_with_local_model(client_with_local_only): + """locality filters to ollama-local provider rows.""" + client, cfg = client_with_local_only + resp = client.get("/admin/api/profiles") + assert resp.status_code == 200 + by_name = _by_name(resp.json()) + # Know edge documented in plan: canonical probe task_category=None rejects + # rows with non-NULL eligible_categories. The seed here has no + # eligible_categories, so locality should succeed. + assert by_name["locality"]["admitted_count"] == 1 + assert by_name["locality"]["admitted_models"] == ["local-model"] + + +# --------------------------------------------------------------------------- +# Zero admission +# --------------------------------------------------------------------------- + + +def test_zero_admit_for_narrow_config_profile(tmp_path): + """A config profile with an allowlist of non-existent models has zero admit.""" + profiles_block = """ +profiles: + empty: + allowed_model_ids: [does-not-exist] +""" + client, cfg, _ = _profile_client(tmp_path, profiles_block) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + _seed_models(conn, [_model_row("real", tier=1, cost_completion=0.1)]) + conn.close() + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["empty"]["admitted_count"] == 0 + assert by_name["empty"]["interactive_count"] == 0 + assert by_name["empty"]["zero_admit"] is True + + +def test_admin_override_removes_only_admitted_model(tmp_path): + """Deprecating via admin_model_overrides drops a profile's count to zero.""" + profiles_block = """ +profiles: + single: + allowed_model_ids: [only-me] +""" + client, cfg, _ = _profile_client(tmp_path, profiles_block) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + _seed_models(conn, [_model_row("only-me", tier=1, cost_completion=0.1)]) + conn.close() + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["single"]["admitted_count"] == 1 + + conn = _make_db(tmp_path) + conn.execute( + "INSERT INTO admin_model_overrides (model_id, provider, availability, updated_at) " + "VALUES (?, ?, ?, ?)", + ("only-me", "neuralwatt", "deprecated", "2026-08-22T00:00:00+00:00"), + ) + conn.commit() + conn.close() + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["single"]["admitted_count"] == 0 + assert by_name["single"]["zero_admit"] is True + + +def test_empty_models_table_returns_zero_admit_for_every_profile(client_no_models): + """With no models every builtin profile returns 0 and 200, never 500.""" + client, cfg = client_no_models + resp = client.get("/admin/api/profiles") + assert resp.status_code == 200 + body = resp.json() + assert len(body) == len(BUILTIN_PROFILES) + for r in body: + assert r["admitted_count"] == 0 + assert r["interactive_count"] == 0 + assert r["zero_admit"] is True + assert r["admitted_models"] == [] + + +# --------------------------------------------------------------------------- +# Interactive vs admitted +# --------------------------------------------------------------------------- + + +def test_batch_profile_admits_more_than_interactive(client_with_models): + """A profile with latency_tolerance=batch returns admitted >= interactive+1.""" + client, cfg = client_with_models + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["batch"]["admitted_count"] == 6 + assert by_name["batch"]["interactive_count"] == 5 + + +# --------------------------------------------------------------------------- +# Shape contract +# --------------------------------------------------------------------------- + + +def test_profiles_response_fields(client_with_models): + """Every record contains the expected top-level keys.""" + client, cfg = client_with_models + resp = client.get("/admin/api/profiles") + body = resp.json() + for r in body: + assert set(r.keys()) == { + "name", + "source", + "definition", + "admitted_count", + "interactive_count", + "zero_admit", + "admitted_models", + } + assert r["source"] in {"builtin", "config"} + assert isinstance(r["admitted_count"], int) + assert isinstance(r["interactive_count"], int) + assert isinstance(r["zero_admit"], bool) + assert isinstance(r["admitted_models"], list) + + +# --------------------------------------------------------------------------- +# CRUD endpoints +# --------------------------------------------------------------------------- + + +def _profile_client_with_models(tmp_path, profiles_yaml=None): + client, cfg, config_yaml = _profile_client(tmp_path, profiles_yaml) + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + rows = [ + _model_row("t1-cheap", tier=1, cost_completion=0.4), + _model_row("t2-mid-a", tier=2, cost_completion=1.5), + _model_row("t2-mid-b", tier=2, cost_completion=1.6), + _model_row("t3-front-a", tier=3, cost_completion=2.0), + _model_row("t3-front-b", tier=3, cost_completion=2.5), + _model_row("t2-flex", tier=2, cost_completion=1.5, latency_class="flex"), + ] + _seed_models(conn, rows) + conn.close() + return client, cfg, config_yaml + + +def test_profile_crud_round_trip(tmp_path): + """Create, update, delete a config profile and see it reflected in GET.""" + client, cfg, config_yaml = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "minit", "max_tier": 2}) + assert resp.status_code == 200 + body = resp.json() + assert body["profile"]["name"] == "minit" + assert body["profile"]["definition"]["max_tier"] == 2 + assert "zero_admit" in body + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["minit"]["source"] == "config" + assert by_name["minit"]["definition"]["max_tier"] == 2 + + resp = client.post("/admin/api/profiles/minit", json={"max_tier": 3}) + assert resp.status_code == 200 + assert resp.json()["profile"]["definition"]["max_tier"] == 3 + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["minit"]["definition"]["max_tier"] == 3 + + resp = client.delete("/admin/api/profiles/minit") + assert resp.status_code == 200 + + resp = client.get("/admin/api/profiles") + assert "minit" not in _by_name(resp.json()) + + assert load_config(str(config_yaml)) + text = config_yaml.read_text() + assert "profiles:" not in text + + +def test_builtins_are_read_only(tmp_path): + """All CRUD mutating actions on built-in profiles return 403.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "batch", "max_tier": 2}) + assert resp.status_code == 403 + + resp = client.post("/admin/api/profiles/batch", json={"max_tier": 1}) + assert resp.status_code == 403 + + resp = client.delete("/admin/api/profiles/batch") + assert resp.status_code == 403 + + +def test_delete_guard_current_default(tmp_path): + """Deleting the currently configured default profile returns 422 naming the setting.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "minit", "max_tier": 2}) + assert resp.status_code == 200 + + resp = client.post( + "/admin/api/config/routing.default_profile", json={"value": "minit"} + ) + assert resp.status_code == 200 + + resp = client.delete("/admin/api/profiles/minit") + assert resp.status_code == 422 + assert "routing.default_profile" in resp.json()["detail"] + + +def test_rename_via_create_delete_refused_by_default(tmp_path): + """A rename is create new + delete old; delete is blocked when old is default.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "oldname", "max_tier": 2}) + assert resp.status_code == 200 + client.post( + "/admin/api/config/routing.default_profile", json={"value": "oldname"} + ).raise_for_status() + + resp = client.post("/admin/api/profiles/", json={"name": "newname", "max_tier": 2}) + assert resp.status_code == 200 + + resp = client.delete("/admin/api/profiles/oldname") + assert resp.status_code == 422 + assert "routing.default_profile" in resp.json()["detail"] + + +def test_zero_admission_save_warning(tmp_path): + """Saving a profile that admits zero models returns zero_admit true with warning.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + + resp = client.post( + "/admin/api/profiles/", + json={"name": "ghost", "allowed_model_ids": ["does-not-exist"]}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["zero_admit"] is True + assert "warning" in body + + resp = client.post("/admin/api/profiles/", json={"name": "normal", "max_tier": 3}) + assert resp.status_code == 200 + body = resp.json() + assert body["zero_admit"] is False + assert "warning" not in body + + +def test_allowed_model_ids_null_round_trip(tmp_path): + """allowed_model_ids: null persists as null, not a list.""" + client, cfg, config_yaml = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "nolist", "max_tier": 2}) + assert resp.status_code == 200 + + text = config_yaml.read_text() + assert re.search( + r"^\s+allowed_model_ids:\s*$", text, re.MULTILINE + ) is not None + assert re.search( + r"^\s+allowed_model_ids:\s*\[\]\s*$", text, re.MULTILINE + ) is None + + resp = client.get("/admin/api/profiles") + by_name = _by_name(resp.json()) + assert by_name["nolist"]["definition"]["allowed_model_ids"] is None + + +def test_profile_create_existing_returns_409(tmp_path): + """Creating a profile whose name already exists returns 409.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + + resp = client.post("/admin/api/profiles/", json={"name": "minit", "max_tier": 2}) + assert resp.status_code == 200 + + resp = client.post("/admin/api/profiles/", json={"name": "minit", "max_tier": 2}) + assert resp.status_code == 409 + assert "update" in resp.json()["detail"].lower() + + +def test_profile_update_unknown_returns_404(tmp_path): + """Updating a non-existent config profile returns 404.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + resp = client.post("/admin/api/profiles/nosuch", json={"max_tier": 2}) + assert resp.status_code == 404 + + +def test_profile_delete_unknown_returns_404(tmp_path): + """Deleting a non-existent config profile returns 404.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + resp = client.delete("/admin/api/profiles/nosuch") + assert resp.status_code == 404 + + +def test_profile_create_bad_tier_returns_422(tmp_path): + """A profile with an out-of-range tier is rejected at the boundary.""" + client, cfg, _ = _profile_client_with_models(tmp_path) + resp = client.post( + "/admin/api/profiles/", json={"name": "bad", "min_tier": 9} + ) + assert resp.status_code == 422 + + +# --------------------------------------------------------------------------- +# Config-file corruption: clean 503s, no writes, no crashes +# --------------------------------------------------------------------------- + +# Two top-level `profiles:` maps: ruamel raises DuplicateKeyError on load. +_DUPLICATE_PROFILES_BLOCK = """ +profiles: + aaa: + min_tier: 1 +profiles: + bbb: + min_tier: 2 +""" + + +def _corrupt_config_client(tmp_path, corrupt_block: str): + """A client over a hand-corrupted temp config.yaml. + + The in-memory cfg is loaded from the CLEAN repo config, mirroring a + running service whose startup cfg is fine but whose file has since been + hand-broken. ``raise_server_exceptions=False`` pins an unhandled endpoint + exception as a raw 500 response instead of raising in the test. + """ + (tmp_path / "config").mkdir(parents=True, exist_ok=True) + config_yaml = tmp_path / "config" / "config.yaml" + config_yaml.write_text( + (ROOT / "config" / "config.yaml").read_text() + corrupt_block + ) + + cfg = load_config(ROOT / "config" / "config.yaml") + + def _db_factory() -> sqlite3.Connection: + return _make_db(tmp_path) + + router = build_router(cfg, _db_factory, base_dir=str(tmp_path)) + app = FastAPI() + app.include_router(router, prefix="/admin") + client = TestClient(app, raise_server_exceptions=False) + + conn = _make_db(tmp_path) + conn.executescript(SCHEMA_SQL) + conn.executescript(ADMIN_TABLE_SQL) + conn.close() + return client, cfg, config_yaml + + +def test_profiles_GET_duplicate_profiles_keys_returns_503(tmp_path): + """A duplicate-key config.yaml refuses with a clean 503, not a crash.""" + client, cfg, _ = _corrupt_config_client( + tmp_path, _DUPLICATE_PROFILES_BLOCK + ) + + resp = client.get("/admin/api/profiles") + assert resp.status_code == 503 + assert "config.yaml" in resp.json()["detail"] + + +def test_profiles_GET_malformed_entry_names_the_profile(tmp_path): + """A hand-edited invalid profile entry refuses with 503 naming it.""" + bad_block = """ +profiles: + badprofile: + min_tier: 9 +""" + client, cfg, _ = _corrupt_config_client(tmp_path, bad_block) + + resp = client.get("/admin/api/profiles") + assert resp.status_code == 503 + assert "badprofile" in resp.json()["detail"] + + +def test_profiles_GET_non_mapping_entry_names_the_profile(tmp_path): + """A profile entry that is not a mapping at all refuses with 503 naming it.""" + bad_block = """ +profiles: + scalar: 5 +""" + client, cfg, _ = _corrupt_config_client(tmp_path, bad_block) + + resp = client.get("/admin/api/profiles") + assert resp.status_code == 503 + assert "scalar" in resp.json()["detail"] + + +def test_profile_create_against_corrupt_config_refuses_and_writes_nothing(tmp_path): + """A create on a corrupt config.yaml is a clean 503 and touches no bytes.""" + client, cfg, config_yaml = _corrupt_config_client( + tmp_path, _DUPLICATE_PROFILES_BLOCK + ) + before = config_yaml.read_bytes() + + resp = client.post( + "/admin/api/profiles/", json={"name": "minit", "max_tier": 2} + ) + assert resp.status_code == 503 + assert "config.yaml" in resp.json()["detail"] + + assert config_yaml.read_bytes() == before + assert not list(tmp_path.glob("config/config.yaml.bak.*")) + + +def test_profile_delete_against_corrupt_config_refuses_and_writes_nothing(tmp_path): + """A delete on a corrupt config.yaml is a clean 503 and touches no bytes.""" + client, cfg, config_yaml = _corrupt_config_client( + tmp_path, _DUPLICATE_PROFILES_BLOCK + ) + before = config_yaml.read_bytes() + + resp = client.delete("/admin/api/profiles/whatever") + assert resp.status_code == 503 + assert "config.yaml" in resp.json()["detail"] + + assert config_yaml.read_bytes() == before + assert not list(tmp_path.glob("config/config.yaml.bak.*")) diff --git a/tests/test_chat_completions.py b/tests/test_chat_completions.py index 34b455f..1263681 100644 --- a/tests/test_chat_completions.py +++ b/tests/test_chat_completions.py @@ -23,6 +23,7 @@ from starlette.testclient import TestClient import dispatcher import session_cache +from config import RoutingProfile from dispatcher import Classification, app ROOT = Path(__file__).resolve().parent.parent @@ -738,6 +739,62 @@ def test_unknown_profile_returns_422_naming_valid_profiles(router): assert not calls +def test_auto_routes_through_configured_default_profile(router, monkeypatch): + """Bare ``auto`` uses routing.default_profile, not the hard-coded default. + + CHEAP wins under both interactive and batch tolerance (every fixture row + is latency_class='standard'), so the model pick alone cannot prove the + batch-local profile ran; the persisted decision row carries the actual + latency_tolerance='batch' and profile='batch-local' under a bare auto. + """ + client, calls, db_path = router + monkeypatch.setattr(dispatcher.cfg.routing, "default_profile", "batch-local") + monkeypatch.setitem( + dispatcher.cfg.profiles, + "batch-local", + RoutingProfile(latency_tolerance="batch"), + ) + + resp = client.post( + "/v1/chat/completions", + json={"model": "auto", "messages": _messages()}, + ) + assert resp.status_code == 200 + assert resp.headers["X-Router-Model"] == CHEAP + assert calls[0]["body"]["model"] == CHEAP + + conn = sqlite3.connect(db_path) + conn.row_factory = sqlite3.Row + row = conn.execute( + "SELECT profile, latency_tolerance FROM route_decisions " + "ORDER BY id DESC LIMIT 1" + ).fetchone() + conn.close() + assert row is not None + assert row["latency_tolerance"] == "batch" + assert row["profile"] == "batch-local" + + +def test_auto_named_profile_overrides_configured_default(router, monkeypatch): + """`auto:` still overrides routing.default_profile.""" + client, calls, _ = router + + monkeypatch.setattr(dispatcher.cfg.routing, "default_profile", "batch-local") + monkeypatch.setitem( + dispatcher.cfg.profiles, + "batch-local", + RoutingProfile(latency_tolerance="batch"), + ) + + resp = client.post( + "/v1/chat/completions", + json={"model": "auto:batch", "messages": _messages()}, + ) + assert resp.status_code == 200 + assert resp.headers["X-Router-Model"] == CHEAP + assert calls[0]["body"]["model"] == CHEAP + + def test_v1_models_lists_all_profiles(router): """Every valid profile appears as auto: in the models list.""" client, _, _ = router @@ -909,7 +966,7 @@ def test_local_fallback_refuses_a_degenerate_image_url_part(router, monkeypatch) def test_local_fallback_wont_masquerade_an_empty_answer_as_200(router, monkeypatch): """A 200 with empty content is a failed answer, and must fall through to 422 - rather than return an empty 200 — a hidden failure must not look like a win.""" + rather than return an empty 200 - a hidden failure must not look like a win.""" client, calls, db_path = router _drop_cheap_by_tier(dispatcher, db_path) monkeypatch.setattr(dispatcher.cfg.local_vision, "enabled", True) @@ -1012,7 +1069,7 @@ def test_a_pinned_non_json_model_with_json_response_format_422s(router): def _session_messages(text="write me a function", system="You are a coding agent"): """Messages with a stable opening (system) message, so the session - fingerprint is constant across turns — the condition a cache hit needs.""" + fingerprint is constant across turns - the condition a cache hit needs.""" return [ {"role": "system", "content": system}, {"role": "user", "content": text}, diff --git a/tests/test_config.py b/tests/test_config.py index 3eb01d8..f5f920f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -36,15 +36,15 @@ def test_profiles_empty_dict_defaults(raw): def test_a_valid_locality_profile_loads(raw): cfg = copy.deepcopy(raw) - cfg["profiles"] = {"locality": {"provider": "ollama-local"}} + cfg["profiles"] = {"my_locality": {"provider": "ollama-local"}} loaded = RouterConfig(**cfg) - assert loaded.profiles["locality"].provider == "ollama-local" + assert loaded.profiles["my_locality"].provider == "ollama-local" def test_a_profile_with_all_fields_loads(raw): cfg = copy.deepcopy(raw) cfg["profiles"] = { - "onlycheaps": { + "my_onlycheaps": { "min_tier": 1, "max_tier": 2, "latency_tolerance": "batch", @@ -53,7 +53,7 @@ def test_a_profile_with_all_fields_loads(raw): } } loaded = RouterConfig(**cfg) - profile = loaded.profiles["onlycheaps"] + profile = loaded.profiles["my_onlycheaps"] assert profile.min_tier == 1 assert profile.max_tier == 2 assert profile.latency_tolerance == "batch" @@ -112,3 +112,70 @@ def test_profile_empty_allowed_model_ids_is_rejected(raw): cfg["profiles"] = {"locality": {"allowed_model_ids": []}} with pytest.raises(ValueError, match="allowed_model_ids"): RouterConfig(**cfg) + + +# --- default_profile + builtin-profile collisions -------------------------------- + + +def test_default_profile_defaults_to_default(raw): + """``routing.default_profile`` defaults to the literal string "default".""" + cfg = copy.deepcopy(raw) + loaded = RouterConfig(**cfg) + assert loaded.routing.default_profile == "default" + + +@pytest.mark.parametrize("value", ["", " "]) +def test_default_profile_rejects_empty_string(raw, value): + """A blank default_profile is a config error: bare `auto` needs somewhere to resolve.""" + cfg = copy.deepcopy(raw) + cfg.setdefault("routing", {})["default_profile"] = value + with pytest.raises(ValueError, match="default_profile"): + RouterConfig(**cfg) + + +@pytest.mark.parametrize( + "builtin_name", + ["default", "batch", "locality", "bigboybritches", "onlycheaps"], +) +def test_config_profile_name_collision_with_builtin_is_rejected(raw, builtin_name): + """A configured profile may not shadow any built-in profile name.""" + cfg = copy.deepcopy(raw) + cfg["profiles"] = {builtin_name: {"latency_tolerance": "batch"}} + with pytest.raises(ValueError, match=builtin_name) as exc: + RouterConfig(**cfg) + detail = str(exc.value) + # The error must also list the reserved names so the operator does not have to guess. + assert "default" in detail + assert "batch" in detail + + +def test_default_profile_rejects_unknown_name(raw): + """routing.default_profile must name a builtin or configured profile.""" + cfg = copy.deepcopy(raw) + cfg.setdefault("routing", {})["default_profile"] = "nosuchprofile" + with pytest.raises(ValueError, match="nosuchprofile") as exc: + RouterConfig(**cfg) + detail = str(exc.value) + assert "default" in detail + assert "batch" in detail + + +def test_default_profile_accepts_configured_profile(raw): + """A configured profile name is a valid default_profile.""" + cfg = copy.deepcopy(raw) + cfg["profiles"] = {"somesuch": {"min_tier": 1}} + cfg.setdefault("routing", {})["default_profile"] = "somesuch" + loaded = RouterConfig(**cfg) + assert loaded.routing.default_profile == "somesuch" + + +@pytest.mark.parametrize( + "builtin_name", + ["default", "batch", "locality", "bigboybritches", "onlycheaps"], +) +def test_default_profile_accepts_builtin_name(raw, builtin_name): + """Any built-in profile name is a valid default_profile.""" + cfg = copy.deepcopy(raw) + cfg.setdefault("routing", {})["default_profile"] = builtin_name + loaded = RouterConfig(**cfg) + assert loaded.routing.default_profile == builtin_name diff --git a/tests/test_routing_profiles_integration.py b/tests/test_routing_profiles_integration.py index fa66c83..cddcea0 100644 --- a/tests/test_routing_profiles_integration.py +++ b/tests/test_routing_profiles_integration.py @@ -364,17 +364,17 @@ def test_chat_auto_batch_selects_same_model_as_auto_under_batch_default( profile_router, monkeypatch ): """``POST /v1/chat/completions`` with model=auto:batch returns the same - selected model as model=auto when the default profile defaults to batch. + selected model as model=auto when the configured default profile is batch. - The default profile is overlaid with latency_tolerance=batch via - cfg.profiles (the configured-override merge path), so ``auto`` and - ``auto:batch`` must both pick the flex twin. If ``auto:batch`` were still - handled by an old leftover ternary ignoring the profile layer, the two - requests would disagree here. + Since Wave A config profiles may not shadow built-in names, the test sets + ``routing.default_profile`` to a new config profile named ``batch-local`` + whose only override is ``latency_tolerance=batch``. Both ``auto`` and + ``auto:batch`` must then pick the flex twin through the profile layer. """ client, _, _ = profile_router + monkeypatch.setattr(dispatcher.cfg.routing, "default_profile", "batch-local") monkeypatch.setitem( - dispatcher.cfg.profiles, "default", RoutingProfile(latency_tolerance="batch") + dispatcher.cfg.profiles, "batch-local", RoutingProfile(latency_tolerance="batch") ) auto = client.post( @@ -754,3 +754,19 @@ def test_route_endpoint_batch_profile_round_trip(profile_router): assert row["profile"] == "batch" assert row["latency_tolerance"] == "batch" assert row["selected_model"] == M1_FLEX + + +def test_bare_auto_resolves_to_configured_default_profile(profile_router, monkeypatch): + """A bare ``auto`` model fills from ``routing.default_profile``, not a hard-coded "default". + + Monkeypatching ``cfg.routing.default_profile = "batch"`` causes a route + whose profile kwarg is the default to behave exactly like ``auto:batch``. + """ + _, _, _ = profile_router + monkeypatch.setattr(dispatcher.cfg.routing, "default_profile", "batch") + + decision = route(_route_req()) + assert decision.latency_tolerance == "batch" + assert decision.selected is not None + assert decision.selected.model_id == M1_FLEX + assert decision.selected.latency_class == "flex"