fix(model-metadata): guard against models.dev underreports at step 5 - #37083
Closed
ignaciolagosruiz wants to merge 1 commit into
Closed
fix(model-metadata): guard against models.dev underreports at step 5#37083ignaciolagosruiz wants to merge 1 commit into
ignaciolagosruiz wants to merge 1 commit into
Conversation
The resolution order in get_model_context_length() trusts the models.dev lookup at step 5 over the curated DEFAULT_CONTEXT_LENGTHS table at step 8. When models.dev reports a stale/incorrectly-low value, the correct curated default is never reached. Concrete case: models.dev reports `minimax-m3-free` on the opencode provider as 200K context, but `DEFAULT_CONTEXT_LENGTHS['minimax-m3']` is 1M. Without this guard, the agent's effective context window is 5x too small and Hermes auto-compresses at 72% of 200K (~144K) when the model actually accepts ~720K of input. Complements upstream's PR NousResearch#36726 which drops stale <=204,800 cache entries for the M3 family at step 1 (catches the symptom — stale cache from pre-catalog builds). This patch catches the root cause — any future fresh-lookup underreport, not just M3. Mirrors the existing Kimi guard in the OpenRouter path (step 6 below): same pattern, generalised to any curated-vs-live drift. Adds the `_curated_context_length` helper that mirrors the longest-key-first substring match used by step 8 so the guard can compare apples to apples. Tests: - 3 helper tests (M3 family -> 1M, M2.5 -> 204,800, unknown -> None) - 3 step-5 guard tests (underreport rejected, larger value accepted, equal value accepted) - 1 end-to-end live-resolution test for the original bug Did NOT add a generic step-1 cache invalidation: cached values are persistent user data, and a `curated > cached` heuristic cannot reliably distinguish a known underreport from a legitimate provider-specific cap (Codex gpt-5.5 is 272K vs curated 1.05M; Nous qwen3.6-plus is cached at 1M vs curated 1,048,576). The fresh-lookup guard at step 5 covers the in-the-wild underreport case without false-positive risk.
Contributor
|
Thanks for the detailed reproduction and for avoiding unsafe generic cache invalidation. Automated hermes-sweeper review found that current
Closing as implemented on main. |
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.
Summary
The
get_model_context_length()resolution order trusts the models.dev lookup at step 5 over the curatedDEFAULT_CONTEXT_LENGTHStable at step 8. When models.dev reports a stale/incorrectly-low value, the correct curated default is never reached.Concrete case: models.dev reports
minimax-m3-freeon the opencode provider as 200K context, butDEFAULT_CONTEXT_LENGTHS['minimax-m3']is 1M. Without this guard, the agent's effective context window is 5× too small and Hermes auto-compresses at 72% of 200K (≈144K) when the model actually accepts ≈720K of input.How it complements existing work
Together the two fixes make
minimax-m3-freeresolve to 1M regardless of whether the bug surfaces as a stale cache entry or a fresh models.dev probe.Implementation
Adds the
_curated_context_length(model)helper that mirrors the longest-key-first substring match used by step 8 (so the guard compares apples to apples), and a step-5 guard that logs and falls through to the curated value when the live lookup is a known underreport. Mirrors the existing Kimi guard in the OpenRouter path (step 6 below) — same pattern, generalised to any curated-vs-live drift.Why no step-1 cache invalidation
I prototyped and reverted a generic
curated > cachedcache invalidation at step 1. It false-positived on:gpt-5.5: cached 272K (correct Codex-OAuth cap) vs curated 1.05M (direct-API value)qwen3.6-plus: cached 1M vs curated 1,048,576 (rounding noise, not a real underreport)Cached values are persistent user data; auto-invalidating them on a heuristic is too risky. Users with a stale cached underreport can delete the entry from
~/.hermes/context_length_cache.yamldirectly. The fresh-lookup guard at step 5 covers the in-the-wild case without that risk.Tests
7 new tests in
TestCuratedDefaultGuard:minimax-m3*→ 1M,minimax-m2.5→ 204,800, unknown → None)All 147 tests pass in
test_model_metadata.pyandtest_minimax_provider.py. No regressions in the broader 7-file sweep (204 tests).Reproduction
Filed from the Hermes TUI gateway in production use. Confirmed working against
https://opencode.ai/zen/v1/chat/completions(HTTP 200, normal token usage).