fix(agent): default context lookup for empty model IDs - #65515
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extracting the focused empty-model case from #26873. The premise still holds on current main: agent/model_metadata.py:2166 passes model to _strip_provider_prefix, whose string-membership operation at agent/model_metadata.py:95 raises for None; an empty string can continue to metadata resolution at agent/model_metadata.py:2460.
Problems
- The patch needs a small manual relocation: current main inserted malformed-URL handling at
agent/model_metadata.py:2152-2161, so the submitted hunk does not apply. Place the guard after the custom-provider override and before that URL handling. tests/agent/test_model_metadata.pyshould prove the immediate-return contract. The empty-string assertion alone could pass after the current resolver performs the OpenRouter metadata lookup atagent/model_metadata.py:2461-2472and eventually falls back to 256K.
Suggested changes
- Add whitespace coverage and mock/assert that blank input does not enter a downstream lookup/probe path.
Automated hermes-sweeper review.
|
|
||
| class TestDefaultContextLengths: | ||
| def test_empty_model_uses_fallback_context(self): | ||
| assert get_model_context_length("") == DEFAULT_FALLBACK_CONTEXT |
There was a problem hiding this comment.
This verifies the final value but not the stated early-return guarantee: on current main, "" can reach fetch_model_metadata() and still eventually return the same fallback. Please assert that a downstream resolver/probe is not called (and include whitespace input).
|
Salvaged in #85498 (rebased onto current main with your commit and authorship preserved). Your fix turned out to be load-bearing: the empty-model fuzzy-match footgun you guarded against started failing CI on main for every open PR this week ( |
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.
This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).
Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.
Salvaged from #65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).
Fixes the red slice on #85444, #85452 and every other open PR.
Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.
This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).
Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.
Salvaged from NousResearch#65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).
Fixes the red slice on NousResearch#85444, NousResearch#85452 and every other open PR.
Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.
This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).
Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.
Salvaged from NousResearch#65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).
Fixes the red slice on NousResearch#85444, NousResearch#85452 and every other open PR.
Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
Summary
This is the focused current-main replacement for the unique safety fix in #26873.
get_model_context_length()can be called while a runtime/model selection is still empty. Before this guard, an empty orNonemodel flowed into provider-prefix normalization and then into cache/probe logic. Return the documented 256K fallback immediately instead.The replacement intentionally excludes the unrelated runtime-switch, Feishu, cron, auxiliary-client, metadata, and compressor changes from #26873. Those concerns overlap with later upstream work and should not be resurrected as one conflicted patch.
Verification
scripts/run_tests.sh tests/agent/test_model_metadata.py -q— 122 passedruff check agent/model_metadata.py tests/agent/test_model_metadata.pypython -m py_compile agent/model_metadata.py tests/agent/test_model_metadata.pygit diff --checkCloses the unique empty-model failure mode extracted from #26873; the historical PR remains separate until maintainers decide how to handle its now-overlapping runtime-switch changes.