From ab527c3d4959f427ac67662314649f5823f6310f Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 3 Oct 2026 00:11:32 -0400 Subject: [PATCH] fix(admin): label the Classifier card and show what is running vs saved The card's inputs had placeholders but no labels, its badge reported the saved mode as if it were live, and a typed 0 was rewritten to the default. - Every input in every mode carries a visible label; fields align to the top. - GET /admin/api/classifier-config now returns running_mode and restart_pending alongside the saved mode. restart_pending compares the whole classifier block, and is null when the saved config no longer validates. - The header badge names the running mode; an amber "restart pending" badge shows until the service restarts, since a save only persists the overlay. - local_decision collect omits blank fields so repo defaults keep floating, and no longer turns a deliberate 0 into the default. - local_encoder threshold placeholder shortened so it is not clipped. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01KkCGRantZsSwmcFpet6FTa --- admin/frontend/controls.html | 98 ++++++++++++++++++--------- docs/admin-portal.md | 12 ++++ src/admin.py | 20 ++++++ tests/test_admin_classifier_config.py | 59 ++++++++++++++++ tests/test_admin_frontend.py | 50 ++++++++++++++ 5 files changed, 207 insertions(+), 32 deletions(-) diff --git a/admin/frontend/controls.html b/admin/frontend/controls.html index a012b56..6e412f4 100644 --- a/admin/frontend/controls.html +++ b/admin/frontend/controls.html @@ -250,6 +250,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important}

Classifier

+ —
@@ -260,7 +261,7 @@ header.navbar{padding-top:2px!important;padding-bottom:2px!important} cascade (stale session → session history → cloud_fallback → the static guess) unmodified.

-
+
+
+
+
@@ -1164,20 +1169,23 @@ function classifierModeFieldsHtml(mode, data) { return `
+
+
+
%
@@ -1188,38 +1196,50 @@ function classifierModeFieldsHtml(mode, data) { const dec = (data && data.decision) || {}; return `
-
+
+
-
+
+
-
- + +
-
- + +
-
- + +
-
- + + @@ -1331,18 +1351,22 @@ async function loadClassifierConfig() { document.getElementById('classifier-mode-fields').innerHTML = classifierModeFieldsHtml(data.mode, data); renderClassifierResolvedPrimary(data); renderClassifierCandidateCategories(data); + /* The header badge states what is RUNNING, not what is saved: a save only + persists, and the router reads this block at startup, so the two differ + until the next restart. The amber badge is that gap made visible. */ + const running = data.running_mode || data.mode; const badge = document.getElementById('classifier-mode-badge'); - badge.textContent = data.mode + (data.mode_source === 'overlay' ? ' (overlay)' : ''); - badge.className = data.mode === 'local_llm' ? 'badge bg-secondary' : 'badge bg-info'; - if (data.mode === 'local_decision' && data.decision) { - const d = data.decision; - if (d.base_url) document.getElementById('classifier-decision-base-url').value = d.base_url; - if (d.model) document.getElementById('classifier-decision-model').value = d.model; - if (d.num_ctx) document.getElementById('classifier-decision-num-ctx').value = d.num_ctx; - if (d.timeout_s) document.getElementById('classifier-decision-timeout-s').value = d.timeout_s; - if (d.confidence_min) document.getElementById('classifier-decision-confidence-min').value = d.confidence_min; - if (d.coverage_min) document.getElementById('classifier-decision-coverage-min').value = d.coverage_min; - document.getElementById('classifier-decision-tier-enabled').checked = !!d.tier_enabled; + badge.textContent = running + (data.mode_source === 'overlay' && !data.restart_pending ? ' (overlay)' : ''); + badge.className = running === 'local_llm' ? 'badge bg-secondary' : 'badge bg-info'; + const pending = document.getElementById('classifier-pending-badge'); + if (data.restart_pending) { + pending.textContent = data.mode !== running + ? `restart pending: ${data.mode}` + : 'restart pending: settings changed'; + pending.title = `running ${running}, saved ${data.mode}; the router reads this block at startup`; + pending.style.display = ''; + } else { + pending.style.display = 'none'; } } @@ -1377,15 +1401,25 @@ function collectClassifierConfigBody() { if (thresholdPct !== '') encoder.confidence_min = parseFloat(thresholdPct) / 100; body.encoder = encoder; } else if (mode === 'local_decision') { - body.decision = { - base_url: document.getElementById('classifier-decision-base-url')?.value || 'http://localhost:11434', - model: document.getElementById('classifier-decision-model')?.value || 'qwen3.5:4b', - num_ctx: parseInt(document.getElementById('classifier-decision-num-ctx')?.value) || 8192, - timeout_s: parseInt(document.getElementById('classifier-decision-timeout-s')?.value) || 10, - confidence_min: parseFloat(document.getElementById('classifier-decision-confidence-min')?.value) || 0.5, - coverage_min: parseFloat(document.getElementById('classifier-decision-coverage-min')?.value) || 0.3, - tier_enabled: document.getElementById('classifier-decision-tier-enabled')?.checked || false, - }; + // A blank field is left OUT so the server default applies and keeps floating + // with the repo. Never `value || default`: that rewrites a deliberate 0 + // (confidence_min 0 means "accept every verdict") into the default. + const decision = {}; + const text = (id) => document.getElementById(id).value.trim(); + const baseUrl = text('classifier-decision-base-url'); + if (baseUrl !== '') decision.base_url = baseUrl; + const model = text('classifier-decision-model'); + if (model !== '') decision.model = model; + const numCtx = text('classifier-decision-num-ctx'); + if (numCtx !== '') decision.num_ctx = parseInt(numCtx, 10); + const timeoutS = text('classifier-decision-timeout-s'); + if (timeoutS !== '') decision.timeout_s = parseInt(timeoutS, 10); + const confidenceMin = text('classifier-decision-confidence-min'); + if (confidenceMin !== '') decision.confidence_min = parseFloat(confidenceMin); + const coverageMin = text('classifier-decision-coverage-min'); + if (coverageMin !== '') decision.coverage_min = parseFloat(coverageMin); + decision.tier_enabled = document.getElementById('classifier-decision-tier-enabled').checked; + body.decision = decision; } return body; } diff --git a/docs/admin-portal.md b/docs/admin-portal.md index 65a522e..0b4ed7f 100644 --- a/docs/admin-portal.md +++ b/docs/admin-portal.md @@ -165,6 +165,12 @@ can't safely represent: plus its companion block are written as one atomic change so an in-between invalid state is never even written transiently. + The header badge names the mode that is **running**, not the one saved. A + save only persists to `config.local.yaml`, and the router reads this block at + startup, so an amber `restart pending` badge sits beside it until the service + restarts. A blank field is left out of the overlay, so the repo default keeps + applying; a typed `0` is saved as `0`. + When `local_decision` is selected the panel reveals a **Local Decision** block with the fields from `LocalDecisionConfig` in `src/config.py`: @@ -481,6 +487,12 @@ intermediate invalid state could land on disk. The GET response includes `routing.cheapest_classifier_candidate` against the current catalog when `cloud_primary_auto` is set, `null` otherwise. +`mode` in that response is the **saved** value. `running_mode` is what the +process is using, and `restart_pending` is `true` when the saved classifier +block differs from the running one in any field, or `null` when the saved +config no longer validates and the comparison cannot be made. The block is read +at import, so only a restart closes the gap. + **Model availability overrides** `POST /admin/api/models/{model_id:path}/{provider}/availability` marks a model diff --git a/src/admin.py b/src/admin.py index cdeacb1..fbd1305 100644 --- a/src/admin.py +++ b/src/admin.py @@ -2684,9 +2684,29 @@ def build_router( candidates = list(prof_categories) excluded = [c for c in prof_categories if c not in candidates] + # What the RUNNING process is using, as opposed to what is saved above. + # cfg binds at import and POST only persists, so the two differ from + # the moment of a save until the next restart -- and reporting the + # saved value as if it were live is the echo-the-config bug this + # docstring opens by warning about. Compare the whole classifier block + # (mode AND its companion blocks): a saved model or confidence_min is + # just as inert until the bounce as a saved mode. + running_mode = cfg.classifier.mode + try: + saved_classifier = RouterConfig(**merged).classifier + restart_pending: Optional[bool] = ( + saved_classifier.model_dump() != cfg.classifier.model_dump() + ) + except ValidationError: + # The saved config does not validate, so "pending" is unknowable. + # Why it is invalid is the write path's job to say, not this field's. + restart_pending = None + return { "mode": classifier.get("mode", "local_llm"), "mode_source": mode_source, + "running_mode": running_mode, + "restart_pending": restart_pending, "candidate_categories": list(candidates), "excluded_categories": excluded, "cloud_primary": classifier.get("cloud_primary"), diff --git a/tests/test_admin_classifier_config.py b/tests/test_admin_classifier_config.py index 739c1e3..7244e1e 100644 --- a/tests/test_admin_classifier_config.py +++ b/tests/test_admin_classifier_config.py @@ -183,6 +183,65 @@ def test_get_classifier_config_includes_decision_block(tmp_path): assert body["decision"]["coverage_min"] == 0.42 +# --- GET: saved vs running --------------------------------------------- +# +# POST only persists and cfg binds at import, so a saved classifier change is +# inert until a restart. GET must say so, or the card reads as live when it is +# not (which cost a restart cycle in practice). The router is built from the +# base config only (tests ignore the overlay), so anything written to the +# overlay is "saved but not running" by construction. + + +def test_get_reports_nothing_pending_when_saved_matches_running(tmp_path): + client, _config_yaml, _local_yaml = _client(tmp_path) + body = client.get("/admin/api/classifier-config").json() + assert body["running_mode"] == "local_llm" + assert body["mode"] == "local_llm" + assert body["restart_pending"] is False + + +def test_get_flags_a_saved_mode_change_as_pending_and_keeps_running_mode(tmp_path): + client, _config_yaml, _local_yaml = _client(tmp_path) + resp = client.post( + "/admin/api/classifier-config", + json={"mode": "local_decision", "decision": {}}, + ) + assert resp.status_code == 200 + + body = client.get("/admin/api/classifier-config").json() + assert body["mode"] == "local_decision" # what is saved + assert body["running_mode"] == "local_llm" # what is actually answering + assert body["restart_pending"] is True + + +def test_get_flags_a_companion_only_change_as_pending(tmp_path): + """Same mode, different block: a saved cloud_fallback is just as inert + until the restart as a saved mode, so comparing modes alone is not enough.""" + client, _config_yaml, _local_yaml = _client( + tmp_path, + overlay_yaml=( + "classifier:\n" + " cloud_fallback:\n" + " base_url: http://127.0.0.1:9/v1\n" + " model: some-model\n" + ), + ) + body = client.get("/admin/api/classifier-config").json() + assert body["mode"] == body["running_mode"] == "local_llm" + assert body["restart_pending"] is True + + +def test_get_reports_pending_as_unknown_when_the_saved_config_is_invalid(tmp_path): + """An overlay that does not validate cannot be compared; null, not a guess.""" + client, _config_yaml, _local_yaml = _client( + tmp_path, + overlay_yaml="classifier:\n mode: local_encoder\n encoder:\n device: tpu\n", + ) + resp = client.get("/admin/api/classifier-config") + assert resp.status_code == 200 + assert resp.json()["restart_pending"] is None + + # --- POST: validated the same way config load is ------------------------ diff --git a/tests/test_admin_frontend.py b/tests/test_admin_frontend.py index 76894f1..bd045a7 100644 --- a/tests/test_admin_frontend.py +++ b/tests/test_admin_frontend.py @@ -743,3 +743,53 @@ def test_controls_html_local_decision_field_rendering(): assert "dec.confidence_min" in fields assert "dec.coverage_min" in fields assert "dec.tier_enabled" in fields + + +def test_classifier_card_inputs_all_carry_a_label(): + """Every input in the Classifier card's mode blocks has a