ADR-018: Model routing, per-node model selection, and the unified catalog - #305
Merged
Conversation
ModelRef(provider, model_id) replaces "task" as the addressing scheme for which provider+model a chat call actually hits. graphlink_model_catalog.py gains ModelRef/ResolvedModel, the resolution chain (resolve_model_ref), three auto policies over the catalog (choose_auto_model_ref: cheapest- capable/fastest/best-quality, always capability-filtered), and unified_catalog() - a pure aggregation of Ollama/llama.cpp scan results plus each API provider's cached catalog into one list, annotated with per-mtok cost via a caller-supplied price_lookup (token_counter.py's pricing table, extracted into price_per_mtok so both the cost estimator and the catalog price off the same numbers). api_provider.chat()/chat_stream() accept an optional model_ref kwarg that takes precedence over the existing task-keyed dict lookup; every call site that doesn't pass one is unaffected. A resolved ref naming Ollama or Llama.cpp is always constructible regardless of the session's configured mode (neither needs credentials) - the realistic mixed local+cloud comparison this ADR exists for. A ref naming a cloud provider other than the session's currently configured one raises an actionable error rather than silently falling back or reaching for credentials the request snapshot was never given; genuine simultaneous multi-cloud-credential routing is out of scope for this stage. Test plan: - backend/tests/test_model_routing.py (17 tests): the three auto policies including capability-filtering (never routes a vision requirement to a text-only model) and the ready/available filter; the full resolution chain's precedence order and its "explicit pins are never capability- filtered" posture; unified_catalog's aggregation and price annotation; api_provider.chat()/chat_stream() actually dispatching on a supplied model_ref (bypassing an intentionally-unconfigured task table), routing to Ollama while the session is in API mode, and the cross-cloud-mismatch actionable error. - tests/test_node_state_migration.py: added the one new non-SceneNode "provider" access shape (ModelDescriptor, not SceneNode) the new test file's own iteration introduced. - backend/tests/test_providers.py + test_api_provider_reasoning.py + test_backend_composer.py + test_agents.py: 337 passed, confirming the dispatch rewrite is behavior-preserving for every existing call site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ChatState gains override_provider/override_model_id - an explicit model pin, the input-routing opposite of the existing provider/model provenance fields (which record what generated a completed reply, not what should generate the next one). SceneDocument.set_model_override/ clear_model_override write it; resolve_model_for_node reads it back for a node and, separately, for its branch root - mirroring _resolve_branch_ system_prompt's exact root-walk shape (get_branch_root, then read one field off the root) rather than inventing a new inheritance mechanism. AgentDispatcher._resolve_model_ref_for_dispatch computes node-override -> branch-override (auto/workspace-default stay out of scope for this stage - see the method's own docstring) and threads the result through the same omit-when-None kwarg chain persona overrides already use: _dispatch -> _call_chat_agent(_stream) -> ChatAgent.get_response -> ChatWorker.run -> api_provider.chat/chat_stream(model_ref=...). No pin anywhere means the kwarg is never even passed - every existing call site is unaffected. Test plan: - backend/tests/test_agents.py (7 new): resolve_model_for_node's precedence order (node pin beats branch-root pin) and root-walk correctness at every depth; sendMessage/regenerateResponse actually carrying the resolved ModelRef through the real dispatch pipeline into _call_chat_agent_stream; the negative case (no pin anywhere never adds a model_ref kwarg at all, not even as None). - backend/domain/branches.py sanity-checked directly (set/clear/root-walk precedence) before writing the dispatch-level tests. - Full suite from repo root: 2180 passed, 16 skipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…for the model-override pin SceneNodeRow gains overrideProvider/overrideModelId (additive, "" default, codegen regenerated), populated in scene_payload() from ChatState's own fields. session_save.py/session_load.py round-trip them the same way provider/model already do. Two new WS intents (setModelOverride/ clearModelOverride, backend/api/intents_model_routing.py) wrap SceneDocument.set_model_override/clear_model_override in record_command - classified "A" (undoable) in the ADR-010 close-out table, same posture as setGroupColor. Test plan: - backend/tests/test_session_save.py + test_session_load.py: the pin round-trips through build_chat_data/restore_chat_payload; absent in a save (every pre-18.3 row) restores to "" / no pin, never a crash. - backend/tests/test_canvas.py: the two intents actually mutate the node and publish through the real WS dispatch path. - tests/test_node_state_migration.py: golden scene_payload() key-set snapshot updated (additive keys only). - tests/test_undo_classification_gate.py + tests/undo_classification.py: both new intents classified and the locked registered-intent population count updated 140 -> 142. - contracts/codegen.py --check: clean, two-field diff only. - Full suite from repo root: 2185 passed, 16 skipped. Frontend (badge + picker UI) is the remaining half of this stage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A distinct badge (📌, tinted not muted) renders whenever a node carries a model pin - deliberately separate from the existing provenance badge, since they answer different questions (what generated the last reply vs. what pins the next one) and can legitimately co-occur. The node menu gains "Pin to Current Model" (pins to whatever the Composer's live route currently resolves to - reading composerStore.getComposer() at click time, never subscribed, since only the click matters) and, when a pin exists, "Clear Model Pin (<model>)". A full "browse the installed catalog" picker needs a live catalog fetch this stage doesn't wire to the frontend - "pin to current" is the honestly-scoped, fully-functional slice; see ADR-018's own status note. Wiring: sceneStore gains setModelOverride/clearModelOverride (plain fireIntent wrappers, queueable like setGroupColor - both operations are idempotent). SceneCanvas's stable per-node dispatcher gains the two callbacks; getComposerRoute threads through as an optional getter (App.tsx's real render supplies it, every other <SceneCanvas> render - every existing test - keeps its harmless no-op default). Test plan: - ChatNodeView.test.tsx: badge absent/present/tooltip, co-rendering with the provenance badge, both menu items' click behavior including the current-route-unresolved no-op case. - SceneCanvas.test.tsx: dispatcher wiring reads getComposerRoute at click time and calls the right store method; the no-op case when the route has nothing resolved yet. - sceneStore.test.ts: both new methods send the right intent/args. - Fixed 5 pre-existing test fixture builders (sceneStore.test.ts, SceneCanvas.test.tsx, ChatNodeView.test.tsx, renderCountGate.test.tsx, SceneCanvas.pinSearchJump.test.tsx, SceneCanvas.virtualization.test.tsx) that needed the two new SceneNodeRow/ChatNodeData fields to keep satisfying the (now stricter) generated wire schema/types. - Full `npm run check` (schema + typecheck + lint + test + build + bundle-size): clean. 1582 tests passed across 60 files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…gs UI Adds SettingsManager.get/set_auto_model_policy (persisted, closed-vocabulary cheapest-capable/fastest/best-quality) and wires it into the auto rung of the resolution chain: api_provider.chat()/chat_stream() now attempt an auto-fallback via unified_catalog + the persisted policy at the exact point they would otherwise raise "no model configured" - filtered to only providers this session can actually dispatch to right now (both local providers always; a cloud provider only when it is the session's live credentialed one), so the auto rung can never hand back a ref _provider_for_model_ref would then reject. settings_manager threads down the same additive, omit-when-None kwarg chain already built for model_ref (agents.py -> graphlink_chat_agent.py -> api_provider.py). Adds the setAutoModelPolicy WS intent and a General-page dropdown in the Settings dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wraps api_provider.chat()/chat_stream() in a fallback-chain outer layer: when a task in FALLBACK_ENABLED_TASKS (task_title, task_web_validate - "naming/triage" per the ADR's own framing) hits a retryable/unavailable failure after ADR-006's own same-provider transport retry is exhausted, one more attempt fires against a different provider chosen by the same auto-policy catalog stage 18.4 built, excluding the provider that just failed. Correctness-sensitive tasks (task_chat, task_chart, ...) are untouched - no fallback fires for them, matching "off by default for correctness-sensitive tasks" from the ADR's decision #4. The substitution is never silent: on_fallback threads down the same additive kwarg chain as model_ref/settings_manager, and backend/agents.py's dispatch surfaces it as a warning notification naming both the provider that failed and the one substituted in. chat_stream's own fallback is additionally guarded so it can only fire before any real text has reached the caller - mirroring the existing "nothing forwarded yet" invariant the transport-retry layer already established, so a partially-delivered reply is never silently replayed against a different model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cursion leak Two bugs surfaced by an adversarial review of the full 5-stage branch: - unified_catalog() stored llama.cpp scanned models as their full scanned path, but _provider_for_model_ref's llama.cpp branch only ever accepts a model_id matching the BASENAME of a configured path (the same convention describe_active_model already uses - llama.cpp has no "load any installed model by id" catalog the way Ollama does). Every auto-pick or fallback landing on a llama.cpp candidate was therefore unconditionally rejected. Fixed by reducing to Path(...).name at catalog-build time. - The "no model configured" auto-pick branches inside chat()/chat_stream() (now _chat_dispatch/_chat_stream_dispatch after 18.5's rename) recurse into the module-level chat()/chat_stream() names to dispatch the auto-picked ref - but that name now resolves to 18.5's own fallback wrapper, which had already popped settings_manager into a local before ever calling into the dispatch body, and the recursive call never re-included it. A failure on an auto-picked model (exactly the "nothing was configured" population FALLBACK_ENABLED_TASKS targets) could never trigger a further fallback attempt as a result. Fixed by re-including settings_manager in both recursive calls, in both functions. Adds regression tests for both (pure-function catalog shape, live dispatch through the real basename-matching path, and a live end-to-end trace through the previously-broken recursion). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements ADR-018 end to end, all 5 stages on one branch:
ModelRef(provider, model_id) replaces task-keyed dispatch as the addressing unit; a unified catalog aggregates all 5 providers' models with capabilities, cost, and latency metadata.task_title,task_web_validate) retries once against a different provider, with the substitution surfaced as a visible notification. Correctness-sensitive tasks (chat, chart, ...) never auto-swap.An adversarial review pass after stage 18.5 surfaced two real bugs, both fixed with regression tests in the final commit:
unified_catalog()stored llama.cpp scanned models by their full filesystem path, but dispatch only ever accepts the basename — any auto/fallback pick landing on a llama.cpp candidate was unconditionally rejected.chat()/chat_stream()droppedsettings_manager, silently disabling the fallback rung specifically for the population of requests that reach it via auto-pick.Test plan
python -m pytest -q(full suite) — 2199 passed, 16 skippednpm run check(schema + typecheck + lint + test + build + bundle-size) — 1584 tests passed, clean build