Skip to content

fix(spawn): resolve module-named provider preferences to the matching instance, not the last-declared one - #357

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/spawn-provider-module-name-resolution
Sep 4, 2026
Merged

fix(spawn): resolve module-named provider preferences to the matching instance, not the last-declared one#357
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/spawn-provider-module-name-resolution

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

The defect

A provider preference is a (provider, model) pair, but only the provider half ever reached instance resolution.

#355 gave the three helpers in spawn_utils.py one rule for "which instance does the bare module type anthropic mean?" — highest priority wins, ties by declaration order, an explicit id: beats a module-type name. That rule is right and stays. It is also model-blind, and that is the half fixed here.

Measured evidence

Measured 2026-09-02 on a 14-provider host (evidence sessions …c075c72254494a08, …a0a049acf77d43c7). Module provider-anthropic is mounted three times:

instance priority default_model
opus 1 claude-opus-5
sonnet 5 claude-sonnet-5
fable 6 claude-sonnet-4-5

A routing matrix addresses providers by module name and discriminates with the model glob. On this host, every {provider: anthropic, model: …} preference resolved to opus — the highest-priority anthropic mount — whatever model it asked for, and the requested model was then stamped onto opus's config. Reproduced on main @ 52cbf74:

pref(provider='anthropic', model='claude-opus-*')    -> promoted=('opus', 'claude-opus-*')      # correct, by luck
pref(provider='anthropic', model='claude-sonnet-4-5') -> promoted=('opus', 'claude-sonnet-4-5')  # WRONG: fable serves this
pref(provider='anthropic', model='claude-sonnet-5')   -> promoted=('opus', 'claude-sonnet-5')    # WRONG: sonnet serves this

Right model name, wrong instance — and with the instance comes its base_url, context window and cache-retention settings. This is the same substitution class #355 fixed (model resolved off one mount, written onto another), reached by a different route, and it is just as silent: the model name in the child's config still reads correct. Downstream this put a reasoning-role agent on a 65K-context mount and produced 400s.

Before #355 the flat-dict build made this worse still — the module-name key was overwritten by the last declared instance, so anthropic meant fable outright. That half is already fixed on main; this PR is the remaining half. The tests here pin both so neither can regress.

The rule

One function, _resolve_provider_index():

  1. An explicit instance id: is the most specific address there is and wins outright. Unchanged.
  2. Otherwise the name is a module. Among that module's instances, prefer the ones whose locally-declared models satisfy the preference's model hint (case-insensitive glob — the convention resolve_model_pattern() already applies to a live catalog).
  3. Among whatever survives, highest priority wins, ties by declaration order. Never "last declared".

Step 2 only ever narrows an already-correct candidate set. Mount configs usually carry no model metadata at all; when nothing matches, every candidate survives and step 3 decides exactly as it does today — a model hint can never turn a hit into a miss. Single-instance plans are therefore unchanged by construction, whatever the model says (pinned by test).

Mechanism, not policy: the model lookup is synchronous and local, reading only what the mount plan already states (default_model, plus a models list when a plan declares one). It never queries a provider's live catalog — that is resolve_model_pattern()'s async job, and these are sync helpers with sync callers.

Helper agreement

_find_provider_index and _build_provider_lookup are now both thin wrappers over that one function, so their agreement is structural rather than two implementations that happen to coincide.

_build_provider_lookup keeps its exact signature and model-blind semantics — a dict keyed by provider name alone cannot express "which instance for this model" — and its docstring now says so, pointing callers holding a preference at the pair-aware function instead.

_apply_single_override is untouched, so PROTECTED_CONFIG_KEYS handling is unchanged (pinned by test).

Tests

tests/test_spawn_utils.py, +15, built on the exact measured host shape (opus/sonnet/fable + a gemini instance of provider-gemini):

  • {anthropic, claude-opus-*}opus promoted, fable untouched (own priority + own model intact)
  • {anthropic, claude-sonnet-4-5}fable, opus untouched ← the fix proper
  • {anthropic, ""} (no model) → highest-priority instance, opus
  • {anthropic, claude-unknown-9} (model nothing declares) → still applies, falls back to opus — a hint never causes a miss
  • {fable, …} (instance id) → fable, even for a model only a sibling serves
  • single-instance module → unchanged for every model, including unknown and empty
  • a mount declaring a models list is selectable by any entry in it
  • model matching folds case
  • cross-module isolation: gemini untouched
  • both helpers return the same index for every addressable name; _find_provider_index honours the optional hint; unknown names are still a miss
  • async path (apply_provider_preferences_with_resolution) promotes the model-matching instance, and preserves PROTECTED_CONFIG_KEYS

7 fail before this change and pass after. The other 8 characterize behaviour #355 already made correct and pin it against this change.

Suite: 1769 -> 1784 passed, 1 skipped, 0 regressions
Reverting only spawn_utils.py: 7 failed, 8 passed

Relation to #296

Open PR #296 ("deterministic provider glob semantics and canonical async model resolution") also edits spawn_utils.py. The two are orthogonal by axis:

No case-folding work from #296 is duplicated here: this change touches only model matching and deliberately follows the file's pre-existing model-glob convention (fnmatch on lowercased both sides, exactly as resolve_model_pattern() at the available_models filter does), not #296's provider-name fnmatchcase. This PR also leaves the resolve_model_pattern(pref.model, pref.provider, …) call site alone — canonicalising that argument is #296's change to make.

They compose cleanly: #296's glob fallback selects a candidate name, and this PR's _resolve_provider_index is the exact-name resolution both its fast path and its _select_provider_preference helper are built on. A textual conflict is expected in the two apply_provider_preferences* loops (#296 rewrites them wholesale); the merge is mechanical — whichever lands second keeps _resolve_provider_index(providers, pref.provider, pref.model) as the exact-name step and layers the name-glob fallback around it.

Refs: recipes-0ac, model_performance-67u (#355)

… instance, not the last-declared one

A provider preference is a (provider, model) PAIR, but only the `provider`
half ever reached instance resolution. #355 gave the three helpers ONE rule
for "which instance does the bare module type `anthropic` mean?" -- highest
priority wins, ties by declaration order, an explicit `id:` beats a
module-type name. That rule is right and stays. It is also MODEL-BLIND, and
that is the half fixed here.

MEASURED (2026-09-02, 14-provider host; sessions ...c075c72254494a08 and
...a0a049acf77d43c7). Module `provider-anthropic` mounted three times:

    opus    priority 1  default_model claude-opus-5
    sonnet  priority 5  default_model claude-sonnet-5
    fable   priority 6  default_model claude-sonnet-4-5

A routing matrix addresses providers by MODULE name and discriminates with
the model glob. On this host every `{provider: anthropic, model: ...}`
preference resolved to `opus` -- the highest-priority anthropic mount --
whatever model it asked for, and the requested model was then stamped onto
opus's config:

    {anthropic, claude-sonnet-4-5} -> promoted opus, default_model=claude-sonnet-4-5
    {anthropic, claude-sonnet-5}   -> promoted opus, default_model=claude-sonnet-5

Right model name, wrong instance -- and with the instance comes its base_url,
context window and cache-retention settings. This is the same substitution
class #355 fixed (model resolved off one mount, written onto another),
reached by a different route, and it is just as silent: the model name in the
child's config still reads correct. Downstream it put a reasoning-role agent
on a 65K-context mount and produced 400s.

Before #355 the flat-dict build made this WORSE still (the module-name key
was overwritten by the LAST declared instance, so `anthropic` meant `fable`).
That half is already fixed on main; this change is the remaining half, and
the tests here pin both so neither can regress.

THE RULE, now in one function -- `_resolve_provider_index()`:

  1. An explicit instance `id:` is the most specific address there is and
     wins outright. Unchanged.
  2. Otherwise the name is a MODULE. Among that module's instances, prefer
     the ones whose locally-declared models satisfy the preference's model
     hint (case-insensitive glob, the convention resolve_model_pattern()
     already uses on a live catalog).
  3. Among whatever survives, highest priority wins, ties by declaration
     order. Never "last declared".

Step 2 only ever NARROWS an already-correct candidate set. Mount configs
usually carry no model metadata at all; when nothing matches, every candidate
survives and step 3 decides exactly as it does today -- a model hint can
never turn a hit into a miss. Single-instance plans are therefore unchanged
by construction, whatever the model says (pinned by test).

`_find_provider_index` and `_build_provider_lookup` are now both thin
wrappers over that one function, so their agreement is structural rather than
two implementations that happen to coincide. `_build_provider_lookup` keeps
its exact signature and model-blind semantics -- a dict keyed by provider
name alone cannot express "which instance for THIS model", and its docstring
now says so and points callers holding a preference at the pair-aware
function. `_apply_single_override` is untouched, so PROTECTED_CONFIG_KEYS
handling is unchanged (pinned by test).

Model lookup is synchronous and local: it reads only what the mount plan
already states. It never queries a provider's live catalog -- that is
resolve_model_pattern()'s async job, and these are sync helpers with sync
callers.

TESTS: tests/test_spawn_utils.py, +15, built on the measured host shape.
7 fail before this change and pass after; the other 8 characterize behaviour
#355 already made correct and pin it against this change.

Suite: 1769 -> 1784 passed, 1 skipped, 0 regressions.
Reverting only spawn_utils.py: 7 failed, 8 passed.

Refs: recipes-0ac, model_performance-67u (#355)
@bkrabach
Brian Krabach (bkrabach) merged commit 7ca50a8 into main Sep 4, 2026
7 checks passed
@bkrabach
Brian Krabach (bkrabach) deleted the fix/spawn-provider-module-name-resolution branch September 4, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants