Repository navigation
Decide the runtime response to routing-suppressed OpenAI-compat models (matrix now exists) #1830
Description
Overview
Split out of #1829, which built the measurement this decision needs. #1829 shipped the four-arm routing-reliability matrix and documented the finding; it deliberately did not add a runtime mitigation.
What the matrix showed
packages/agent/tests/live_openai_compat_routing.rs, run against a local Ollama:
model baseline stage1_only routed_full routed_names
gemma4:e4b fires fires fires fires
mistral:7b fires fires SUPPRESSED SUPPRESSED
- The locked native model is unaffected on every arm. The shipped configuration does not have this problem.
- On
mistral:7b, the block's presence is the variable, not its content.routed_namesstrips the candidate to a bare skill name and it still suppresses. ADR-064 rule 4's mechanism (procedural content suppresses calling), measured on the locked model, does not explain this.
Why this is not yet actionable — the evidence is too thin to choose a mitigation
This issue cannot be closed responsibly until the matrix is wider. One served model exhibits the failure and none is yet confirmed clean, which means neither available generalization is supported:
- "All served models are at risk" — not supported; only one has been measured failing.
- "This is specific to
mistral:7b" — not supported either; there is no clean served comparison.
Any mitigation design implicitly picks one of those. A mitigation keyed on native-vs-served assumes the first; one keyed on a per-model verdict assumes the second. Building either now means encoding an assumption the data does not carry.
An earlier revision of this issue cited ornith:9b as a clean served comparison. That has been removed: ADR-056 deleted the Qwen family — including the Qwen3.5-based ornith-1-9b-q4km — from the NodeSpace catalog entirely, so it is not a model this project runs and must not be evidence for its design. It appeared in the matrix only because it happened to be pulled on the tester's Ollama. The matrix test now excludes removed-catalog models by name so this cannot recur.
Prerequisite
- Widen the matrix to more served models NodeSpace would plausibly run, so there is at least one clean and one failing served data point. Until that exists, the fork below cannot be decided on evidence.
The fork, once evidence exists
- A. Do nothing; document only. Current state after OpenAI-compat path has no routing-reliability testing; Stage-2 candidate injection can silently suppress tool-calling on served models #1829. The limitation is recorded in ADR-034 (user-facing), ADR-038 (routing risk), and ADR-056 (matrix). A user on an affected model gets prose where they expected an action, with no in-product signal.
- B. Runtime auto-degrade. When a routed turn yields zero tool calls, retry once without the candidate block and warn. Self-correcting and needs no per-model config, but costs an extra generation on every failure and risks masking ordinary "no tool was appropriate" turns as suppression.
- C. One-time probe per model. At discovery, run a synthetic routed turn against each configured model, cache a
routing_okverdict, skip injection for models that fail. No per-turn cost and a clean signal, but adds startup latency, a cache to invalidate when a model is updated in place, and a false-negative risk from a single probe.
B and C are not exclusive — a probe could seed the verdict and runtime degradation could correct it.
Acceptance Criteria
- The matrix covers enough served models to support a generalization either way
- Decide between A, B, and C (or a hybrid), recorded as an ADR update rather than only in code
- If B or C: the mechanism keys on whatever the widened matrix actually supports — not on an assumed native-vs-served split
- If B or C: a suppressed turn produces a signal the user or a log reader can actually see, not a silent degradation
- The routing matrix test is extended to cover whatever mechanism lands, so the mitigation itself has a regression guard
- If A: close with the rationale recorded, so this is a decision rather than an omission
Out of scope
- Re-litigating the native model lock (ADR-056) — the locked model is measured unaffected
- Removing any model from the OpenAI-compatible path — ADR-034 keeps it open by design
- Guaranteeing routing on all possible served models — not achievable; the goal is graceful handling
Activity
OpenAI-compat path has no routing-reliability testing; Stage-2 candid…
malibio commentedon Jul 31, 2026
Frontend baseline: 171 test files passed, 4112 tests passed (52.13s).
malibio commentedon Jul 31, 2026
Widened the matrix from 1 served model to 3 (local Ollama), reproduced twice with identical results:
```
model baseline stage1_only routed_full routed_names
llama3.1:8b fires fires fires fires
mistral:7b fires fires SUPPRESSED SUPPRESSED
gemma4:e4b fires fires fires fires
```
(gemma4:e4b here is the OpenAI-compat/Ollama path, not the native locked model — included as a served comparison point; it also routes clean.)
This settles the prerequisite: there is now one clean served model beyond the locked native model (`llama3.1:8b`) and one failing served model (`mistral:7b`), reproduced across two runs. Neither generalization the issue named holds as stated — but the evidence now clearly favors "narrow, per-model property" over "all served models are at risk": 2 of 3 measured served models are clean, and the one failure is content-independent (block presence alone suppresses, not its content), which contradicts ADR-064 rule 4's predicted mechanism on this model specifically.
`ornith:9b` excluded per the matrix's own exclusion list (removed catalog).
Proceeding to the fork decision on this evidence.
malibio commentedon Jul 31, 2026
Fork decision: C (one-time probe per model), keyed on per-model verdict
Why not A (document only): the widened matrix now has a clean signal to act on — 2/3 served models route safely, one doesn't, and it's reproducible. Leaving users on the failing model with prose-only guidance when a cheap, targeted fix exists doesn't match the evidence anymore.
Why not B alone (runtime auto-degrade / retry-on-zero-tool-calls): its cost is paid on every ambiguous turn, on every model, forever — including the 2/3 of served models that are already clean. It also can't tell "suppressed" apart from "no tool was appropriate here," which is a real outcome for
search_nodes-shaped requests. Given the evidence shows a stable, deterministic per-model property (identical results across 2 full-matrix runs), paying a per-turn cost to rediscover something static every time is the wrong shape.Why C: the failure is static and cheap to characterize once.
OpenAiCompatConfig(daemon.toml,[[openai_compat.configs]]) is already the natural home for a persistedrouting_okverdict — no new storage subsystem needed.load_model_and_collect_eventsinlocal_agent_service.rs(daemon) already round-trips config load → engine construction → a"ready"event on every OpenAI-compat model load; that's the natural hook to run one synthetic routed turn, cache the verdict on the config, and skip Stage-2 injection for models that fail it. Cost is paid once per model load, not per turn.Signal requirement: the
"ready"event gets arouting_okfield; a failing probe logs a warning and the event carries a visible note (not just skips injection silently) so a log reader — and eventually the Settings UI — can see it. Existingtracing::info!at the injection point (stage2_candidates_injected) already distinguishes "routed but nothing matched" from "never routed"; this adds "routing disabled for this model" as a third, visible state rather than folding it into the second.Keyed on: the widened matrix's actual finding — a per-model property, not native-vs-served (the native-locked model and 2/3 measured served models are clean; only
mistral:7bfails). No mechanism assumes OpenAI-compat-as-a-whole is risky.Hybrid note: not adding B's runtime retry now. The probe is deterministic per the reproduced matrix; if a false negative from a single probe run turns out to be a real problem in practice, B is the natural follow-up (seed from C, correct at runtime) rather than something to build speculatively now.
Proceeding to implement C, extend the routing matrix test to cover it, and update ADR-034/038/056 to record the decision — unless redirected.
malibio commentedon Jul 31, 2026
Progress note + one open item for live re-verification.
Implemented Option C.
routing_probe::probe_routing_ok(new module,packages/agent/src/local_agent/routing_probe.rs) runs one synthetic routed turn (Stage 1 + therouted_names-equivalent Stage 2 block — verified byte-identical to the matrix's ownrouted_namesrendering via a new unit test,the_probes_block_matches_the_routed_names_arm) against a served model at load time. The verdict is cached onOpenAiCompatConfig.routing_okindaemon.toml(survives a Settings-GUI resave unless base_url/model actually changed) and gatessession.routing_disabledin the agent loop, which skips only the prompt-block injection — tool-surface scoping (ADR-038's trust boundary) stays in effect regardless, since that's not the mechanism the matrix implicated. A disabled turn logs a visibletracing::warn!plus astage2_routing_disabledfield on the existing structured turn log, so it's a distinct, visible third state rather than folding into "routing produced nothing."Unit coverage:
routing_probemodule tests, two newagent_looptests (routing_enabled_injects_the_candidate_block,routing_disabled_skips_injection_but_keeps_tool_scoping— the second confirms tool scoping survives even with injection off), and six newsettings_servicetests covering persistence/carry-forward/invalidation of the cached verdict. All green.Live re-verification is currently blocked. I added
tests/live_routing_probe.rs(--ignored) to confirm the production probe function agrees with the matrix against a real server. First run: it agreed onmistral:7b(suppressed) but disagreed onllama3.1:8b(probe said suppressed; matrix said clean). Investigating with rawcurl, I found the actual cause: after ~30 minutes of the matrix running repeatedly back-to-back today, the local Ollama server degraded —llama3.1:8b(and latermistral:7b) started returning a zeroed/garbageHTTP 200response ("created":-62135596800,"model":"",...) for every request, including a bare "say hello" with no tools and no candidate block at all. This is a server/runtime fault, not a probe bug or a real suppression finding — confirmed becausegemma4:e4bkept working correctly on the same degraded server for a while, then it too stopped responding. I did not restart the Ollama process myself since it's outside this task's normal edit/test loop and could affect things you're using it for outside this session.This doesn't block landing the PR —
live_routing_probe.rsis--ignoredlike the matrix test itself, doesn't run intest:all, and I've documented the failure-mode-that-looks-like-a-bug directly in the test file so it isn't mistaken for a regression on a future run. But I'd like to actually confirm the probe against a healthy server before calling this fully verified. Could you restart the local Ollama server (or let me know once it's healthy) so I can re-runcargo test -p nodespace-agent --test live_routing_probe -- --ignored --nocapture? Not blocking — I'll continue with quality checks and the PR in the meantime and can re-run this afterward.malibio commentedon Jul 31, 2026
Agreed — C is the right fork, and the reasoning against A and B is solid. Verified the two load-bearing claims before endorsing:
llama3.1:8b(clean) +mistral:7b(suppressed), reproduced twice,ornith:9bcorrectly auto-excluded by the matrix's own exclusion list from OpenAI-compat path has no routing-reliability testing; Stage-2 candidate injection can silently suppress tool-calling on served models #1829. That's the actual prerequisite satisfied, not inferred.OpenAiCompatConfig's shape supports this cleanly.id/name/base_url/api_key/model— arouting_okfield is a small, natural addition, not a stretch fit.One gap to close before implementing: cache invalidation on config edit, not just config creation.
OpenAiCompatConfigis user-editable settings (daemon.toml), not immutable — if a user changes themodelstring on an existing config entry (points the same endpoint at a different model) without deleting and recreating the config, a stalerouting_ok: truefrom the old model would silently survive and skip Stage-2 injection for a new model that might actually suppress. The probe needs to re-run wheneverbase_urlormodelchanges on a config, not just on first creation — tie invalidation to whatever change-detectionload_model_and_collect_eventsalready has for other config-drift cases, if any exists, rather than inventing a new one.Everything else — the "ready" event carrying a visible
routing_okstate as a third case alongside "routed but nothing matched" / "never routed", the matrix-test extension, and holding B in reserve rather than building it speculatively — agreed as scoped.Proceed, with the invalidation-on-edit case added to the acceptance criteria before this closes.
malibio commentedon Jul 31, 2026
Good catch — and it turned out to go one level deeper than the disk-cache invalidation I'd already handled (that part —
set_open_ai_compat_configsdroppingrouting_okwhenbase_url/modelchanges,record_routing_probe_verdictno-oping if the target no longer matches — was already in place and covered by tests).The gap: for a single-model server config with no
/modelsdiscovery,model_idresolves to the bareopenai-compat:<uuid>regardless of whatconfig.modelis set to. So editingmodelin Settings on an existing config doesn't changemodel_id, which meansreplace_engine_if_changed's "already loaded" check (keyed onmodel_id) sees no change — my original code used thatswappedflag to decide whether to even consult the cache, so it would've silently kept serving the previous model's verdict after an in-place edit, without ever touching disk to notice the config changed.Fixed by keying the in-memory routing-probe cache on the actual
(base_url, model)pair that was measured, independent ofswapped/model_id. Added a regression test (editing_model_on_an_undiscovered_config_forces_a_reprobe) that reproduces the exact shape — samemodel_idacross two loads,modelfield edited in between — and confirmed it fails against the oldswapped-only logic (reverted the fix locally, watched it fail with the stale key, put the fix back) before trusting it as a real regression guard.Continuing to test:all / quality:fix / PR.