Conversation
Correct diagnosis: base_url + model name is genuinely ambiguous when OpenAI-compatible aggregators (new-api/one-api style) serve identically named models under several providers, and provider-scoped matching with legacy fallback is the right shape. Test coverage for case-insensitivity, unknown providers, and backward compatibility is thorough. Findings:
Minor: document the identity precedence ( |
…t shadow Addresses AI review on NousResearch#93024: - get_custom_provider_context_length now scans identified entries first under a provider-scoped lookup; identity-less legacy entries are only a fallback when no identified entry matched. Previously an unnamed entry placed earlier in config order could win the scoped lookup, recreating the cross-provider misresolution this fix exists to prevent. - provider parameter defaults to None (was Optional[str] = ""). - Call sites read agent.provider via getattr for defensive access. - Document provider identity precedence (provider_key > name > slug) in the docstring. Adds test_identityless_entry_does_not_shadow_named_entry for the mixed ordering; renames the legacy fallback test to reflect its new semantics. 15 tests pass.
|
Thanks for the review — all points addressed in eb7dd79 (15 tests pass).
|
eb7dd79 to
863cb1a
Compare
Summary
get_custom_provider_context_length()matchedcustom_providersentries on base_url + model id alone, ignoring which provider the lookup was for. Multiple providers can legitimately share one base_url — an OpenAI-compatible aggregator such as new-api/one-api fronting several upstream accounts is a common setup — and serve identically named models with different context windows. In that configuration every context-length resolution path picked whichever matching entry appeared first in config order.Observed symptom
Two configured providers (
coding-plan-gptandmarket-aigw) both point athttp://127.0.0.1:3000/v1and both definegpt-5.6-terra-2026-07-09:Selecting the model under
market-aigwresolved its per-model override to 500,000 (the other provider's value). The wrong value propagated to every consumer of the resolution chain: status-bar display,/modelswitch confirmation, compression feasibility checks, gateway/info— and in our case surfaced as a hard failure when a large@reference was rejected against the wrong 50% limit.Fix
Add an optional
providerparameter toget_custom_provider_context_length(). When supplied, only entries whoseprovider_key/name/slugequals it (case-insensitive) are eligible; entries without any provider identity remain reachable so hand-written legacycustom_providers:blocks keep working. Callers that pass no provider keep the historical first-match-in-config-order behaviour.All six call sites now thread the active provider through:
agent/model_metadata.py(resolution step 0c)agent/agent_init.py(startup)agent/agent_runtime_helpers.py(mid-session/modelswitch)gateway/run.py×3 (gateway startup, message-path resolution, auxiliary compression)Testing
TestProviderScopedContextLengthregression tests: per-provider resolution of the shared-base_url same-model collision, case-insensitive matching, unknown-provider miss, backward-compatible no-provider ordering, and legacy entries lacking a provider identity.14 passed(9 pre-existing + 5 new).market-aigwand 500,000 undercoding-plan-gpt; no-provider calls unchanged.