fix(auxiliary): use a named lane's own default model when a task pins no model - #348
Merged
Merged
Conversation
… no model `auxiliary.<task>.provider: my-local-lane` with no `model:` sent the MAIN chat model's id to that endpoint. `resolve_provider_client` fills a blank model for every concrete provider from `_read_main_model()`, so a task that named one of the user's own `providers:`/`custom_providers:` entries got the main model paired with the entry's base_url — a 404 from a local backend serving a different model set, not an answer. The entry's own `default_model` is what "use that provider" reads as, so it now fills the blank first, in `_resolve_task_provider_model` — the one function every auxiliary entry point funnels through (call_llm, async_call_llm, get_text_auxiliary_client, get_async_text_auxiliary_client, resolve_vision_provider_client), which is also where the raw config string still exists. `auto`, `main` and bare `custom` are excluded: they mean "the lane the main runtime already resolved", so they keep inheriting the main chat model, and `_read_main_model()` stays the last resort for a lane that declares no model of its own. PR #340 added exactly this rule for `auxiliary.route` and deliberately left the per-task path alone to keep its blast radius small. Its inline lookup is now the shared `_named_provider_default_model`, which normalises both config shapes (`providers.<name>.default_model` and `custom_providers[].model`). 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
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.
The line
agent/auxiliary_client.py:4924, insideresolve_provider_client:That is the universal blank-model fallback for every concrete provider, and
it is right for the case it was written for: an
xai-oauthuser withgrok-4.3configured should get
grok-4.3for title generation rather than silentlydropping to some catalog default (NousResearch#31845).
_get_aux_model_for_provideronlyknows built-in providers, so for a name that is one of the user's own
providers:/custom_providers:entries it returns""and_read_main_model()— themodel.modelvalue fromconfig.yaml— wins.So this config:
resolved the endpoint perfectly and then asked it for
deepseek-chat. Theentry's own
default_modelwas never consulted. On a local backend serving adifferent model set that is a 404, or — on a proxy that quietly substitutes —
a silent dud. Measured before/after on the resolver, same config:
Worth being explicit about why this hid so well: provider,
base_urlandapi_keywere all resolved correctly. Any test asserting "did we reach theright endpoint" passed. Only the
modelfield on the request was wrong.Why #340 did not fix it
PR #340 added
auxiliary.routeand hit this exact wall: a route pointed at anamed entry would have shipped the main model to the routed lane, "the one
thing this lane must not do". Its
_auxiliary_route_targettherefore reads theentry's
default_modelitself before handing off, and its comment names theline above as the reason.
That fix was scoped to the route on purpose.
auxiliary.routewas brand-newconfig — nothing on disk anywhere used it, so widening the route's model
resolution could not regress an existing user. The per-task
auxiliary.<task>.providerpin is years-old config that people already run, sochanging what a blank
model:means there is a behavior change that deservedits own change and its own tests rather than riding along inside a feature PR.
This is that follow-up.
What changes for a user
A task that names one of your own provider entries and pins no model now sends
that entry's
default_model. Nothing else moves:provider: my-local-lane(nomodel:)default_modelprovider: my-local-lane,model: tinytinytinyprovider: auto/provider: mainprovider: my-local-lane, entry has nodefault_modelprovider: openrouter(or any built-in)Where the fix lives
In
_resolve_task_provider_model, not at line 4924. Two reasons:call_llm,async_call_llm,get_text_auxiliary_client,get_async_text_auxiliary_client,resolve_vision_provider_client— so thesync path, the async twin and the vision path are all covered by one site.
resolve_vision_provider_clientreachesresolve_provider_clientthrough_get_cached_client, so the vision lane carried the main model too.resolve_provider_clientruns,provider: mainhas already been resolvedinto the main provider's id — on the vision path by
_normalize_vision_provider, insideresolve_provider_clientby_normalize_aux_provider. A fix at line 4924 could not tellprovider: main(which must keep inheriting the main model) fromprovider: custom:my-lane(which must not). Upstream it can, so thesentinels stay honest.
auto,mainand barecustomare excluded as main-lane sentinels — allthree mean "the lane the main runtime already resolved". Bare
custombelongsin that set: it is the anonymous
OPENAI_BASE_URL/model.base_urlendpointthe main lane owns, not a named row. The lookup also runs after the
direct-API alias expansion, so
provider: openai(which rewrites tocustom+api.openai.com) cannot pair a same-named entry's model with thealiased endpoint.
Line 4924 itself is untouched and remains the last resort, so nothing outside
the per-task path —
agent_init's main-agent client, the fallback chains,trajectory_compressor— changes at all._auxiliary_route_target's inline lookup is now the shared_named_provider_default_model, so there is one implementation. It normalisesboth config shapes, because
_get_named_custom_provideralready does:providers.<name>.default_model(dict shape) andcustom_providers[].modelor
default_model(legacy list shape) all arrive asentry["model"].Testing
scripts/run_tests.shover every test file that touches the auxiliary resolver(135 files across
tests/agent,tests/hermes_cli,tests/run_agent,tests/tools,tests/gateway,tests/cron,tests/cli,tests/tui_gateway):3937 tests, 0 failures.
The keep-green set called out for this change —
tests/agent/auxiliary_client/,test_auxiliary_route.py,test_image_routing.py,test_auxiliary_main_first.py— passes unchanged.
New:
tests/agent/test_auxiliary_task_provider_model.py(24 tests). Liketest_auxiliary_route.pyit drives the real resolution path against a tempHERMES_HOMEwith a realconfig.yaml, and asserts on themodelkwarg thatactually reaches
.chat.completions.create()— a stub at the client seam isexactly what let this survive. Headline case: a named lane with no model pinned
sends the entry's model, not the main chat model.
Also covered: a task-level
model:still wins; an explicit call argument stillwins;
auto/main/ barecustomstill inherit the main model even with anentry literally named
customon disk;model: autois still nulled before thelane lookup so the literal string never reaches the wire; an entry with no
default model falls back exactly as before; a first-class provider is untouched;
the
custom:<name>menu-key spelling resolves the same way; both legacycustom_providers[]model spellings work; the vision path and the async clientget the entry's model;
provider: mainon the vision path still inherits themain model; and the shared helper returns
Nonerather than raising when configloading breaks.
Verified the new tests fail against the unfixed resolver: with the new block
disabled, 10 of the 24 fail and the 14 guard tests still pass.