Skip to content

fix(models): honor provider context length overrides (#3717) - #3726

Closed
rodboev wants to merge 2 commits into
nesquena:masterfrom
rodboev:pr/context-length-provider-overrides
Closed

rodboev wants to merge 2 commits into
nesquena:masterfrom
rodboev:pr/context-length-provider-overrides

Conversation

@rodboev

@rodboev rodboev commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Prior context-window fixes made the fallback resolver pass global config and custom_providers, but Bug: Context length indicator ignores providers / custom_providers per-model overrides #3717 reports the remaining configured-provider path.
  • The user-visible bug is the context indicator showing and persisting the wrong window when per-model overrides live under provider-specific config.
  • The fix keeps the existing global default-model guard, but feeds the resolver the effective provider config, base URL, and custom provider metadata for route load, session save, and SSE usage paths.

What Changed

  • api/routes.py: resolve provider-specific/base-url context metadata in _resolve_context_length_for_session_model().
  • api/streaming.py: mirror the same resolution for session-save and live SSE usage fallbacks.
  • tests/test_issue3717_context_length_provider_overrides.py: add regression coverage for providers and named custom_providers per-model overrides.
  • tests/test_issue1896_context_length_fallback_args.py: update source-shape assertions if needed for the shared helper.

Why It Matters

Users who configure custom or provider-specific model windows should see the real context window immediately and persist it correctly. A stale fallback cap causes misleading usage indicators and can trigger compression too early.

Verification

Local tests were skipped for this push so CI can provide the concrete validation result without Windows console-window interference.

Risks / Follow-ups

  • The resolver touches several historical fallback paths. The regression coverage intentionally includes route load, session save, and SSE usage to catch drift.

Model Used

GPT 5.5 via Codex CLI

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes #3717 by introducing a shared _context_length_lookup_inputs_for_model() helper in api/routes.py that resolves the correct context-length metadata (base URL, provider, and context-length overrides) from all three config sources — providers.<name>.models, custom_providers, and model.context_length — and wires it into all three resolution callsites (route load, session save, and SSE usage). Previously, provider-scoped context_length overrides were invisible to the resolver, so the wrong fallback cap was persisted and shown in the usage indicator.

  • api/routes.py: Introduces _ContextLengthLookupInputs, _context_length_lookup_inputs_for_model(), and supporting helpers that replace the inline config-extraction blocks in _resolve_context_length_for_session_model().
  • api/streaming.py: Two context-length resolution sites (session-save and SSE usage fallback) now delegate to the shared route helper; the session-save site pre-assigns _cfg_base_url before calling the helper so the legacy 2-arg TypeError path always has the variable in scope.
  • Tests: New regression suite covers provider-scoped, no-base-url, and named-custom-provider variants; existing test updated to match the renamed _cfg_provider variable.

Confidence Score: 5/5

Safe to merge — the change consolidates duplicated config-extraction logic into a single well-guarded helper, and all three callsites pass the same resolved inputs.

The shared helper is fully defensive: every config access is guarded, the precedence chain (provider → custom_provider → global) is explicit and correct, and effective_base_url is never overwritten once set. In the session-save path _cfg_base_url is pre-assigned before the helper call so the legacy TypeError handler always finds it in scope; in the SSE usage path _cfg_base_url is assigned after the helper but before the inner try/except, eliminating any NameError risk. Test coverage is thorough.

No files require special attention.

Important Files Changed

Filename Overview
api/routes.py Adds shared _context_length_lookup_inputs_for_model() helper plus supporting utilities; replaces the inline extraction block in _resolve_context_length_for_session_model(). Logic is well-guarded and the precedence chain (provider > custom_provider > global) is correct.
api/streaming.py Two context-length fallback sites updated to use the shared helper. _cfg_base_url is now pre-assigned before the helper call in the session-save path, resolving the prior unbound-variable risk in the legacy TypeError handler; the SSE usage path assigns it after the helper but before the inner try/except, so no NameError is possible.
tests/test_issue3717_context_length_provider_overrides.py New regression suite with four functional tests covering named providers with/without base_url, named custom_providers, and the default-model-only guard.
tests/test_issue1896_context_length_fallback_args.py Updated to match renamed _cfg_provider variable and the moved config-extraction logic. Assertions remain meaningful and correctly anchor the source-code contracts.
CHANGELOG.md CHANGELOG entry added under [Unreleased] per AGENTS.md requirements; accurately describes the user-visible fix and the legacy-path hardening.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Context-length resolution needed"] --> B["_context_length_lookup_inputs_for_model()"]
    B --> C{"providers cfg matches provider?"}
    C -- Yes --> D["Read provider.models context_length + base_url"]
    C -- No --> E{"custom_providers matches?"}
    D --> F{"provider_context_length found?"}
    F -- Yes --> G["Use provider_context_length"]
    F -- No --> E
    E -- Yes --> H["Read custom entry context_length + base_url"]
    E -- No --> I{"global model.context_length applies?"}
    H --> J{"custom_context_length found?"}
    J -- Yes --> K["Use custom_context_length"]
    J -- No --> I
    I -- Yes --> L["Use global_context_length"]
    I -- No --> M["config_context_length = None"]
    G & K & L & M --> N["_ContextLengthLookupInputs returned"]
    N --> O["get_model_context_length() called"]
    O -- TypeError --> P["legacy 2-arg fallback"]
    O -- Success --> Q["context_length resolved"]
    P --> Q
Loading

Reviews (2): Last reviewed commit: "fix(#3717): tighten provider override fa..." | Re-trigger Greptile

Comment thread api/routes.py
Comment on lines +2105 to +2110
return _ContextLengthLookupInputs(
config_context_length=provider_context_length or custom_context_length or global_context_length,
custom_providers=custom_providers,
base_url=effective_base_url,
provider=effective_provider,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing CHANGELOG entry for user-visible fix

AGENTS.md requires updating CHANGELOG.md for user-visible behavior changes that are release-note ready. This PR corrects what the PR description calls a "misleading usage indicator" — wrong context-window display when per-provider or custom-provider overrides are configured — which is squarely user-visible. No CHANGELOG entry was added.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Read the full diff plus _resolve_context_length_for_session_model at api/routes.py:2260 (master), the new _context_length_lookup_inputs_for_model helper, both streaming callsites, and cross-checked against the agent resolver at agent/model_metadata.py:1484-1535. Two findings — one that's a real plus, one minor structural issue.

This closes the standard-provider gap that the earlier #3717 analysis flagged as needing a separate fix

The prior writeup on #3717 noted that the custom_providers-with-base_url half was fixable via get_compatible_custom_providers, but the providers.<name>.models.<model>.context_length half (standard provider, no base_url) could not be, because the agent's per-model override lookup is base_url-keyed end to end (agent/model_metadata.py:1524 gates on if custom_providers and base_url and model:).

This PR sidesteps that correctly. provider_context_length is resolved by provider-name match, not URL:

for provider_key, provider_cfg in providers_cfg.items():
    if not _providers_match_for_context(provider_key, effective_provider):
        continue
    ...
    provider_context_length = _models_config_context_length(
        provider_cfg.get("models"), bare_model or model_for_lookup,
    )
    break

and is then fed as config_context_length, which the agent returns first, before any base_url-gated probe:

# agent/model_metadata.py:1517
if config_context_length is not None and isinstance(config_context_length, int) and config_context_length > 0:
    return config_context_length

So a providers.anthropic.models.claude-opus-4-6.context_length: 256000 with no base_url now resolves — exactly the headline repro half that couldn't be fixed at the agent layer. Nice. One gap in coverage: the new tests (test_route_resolver_uses_provider_model_context_length) all give the provider a base_url. Adding one case with providers.anthropic.models.<m>.context_length and no base_url, asserting config_context_length is still forwarded, would lock in precisely the case that was previously unfixable and prevent a future refactor from silently reintroducing the base_url dependency.

Minor: _cfg_base_url can be referenced unbound in the session-save legacy fallback

greptile flagged this and it checks out. At the session-save site (api/streaming.py:6669-6708) the except TypeError handler is at the same level as the helper call:

try:
    from api.routes import _context_length_lookup_inputs_for_model
    _ctx_lookup = _context_length_lookup_inputs_for_model(...)   # 6672
    _cfg_base_url = _ctx_lookup.base_url                          # 6680
    _resolved_cl = get_model_context_length(..., _cfg_base_url, ...)  # 6682
except TypeError:
    _resolved_cl = _legacy_cl(..., _cfg_base_url)                # 6700 — could be unbound

In practice the TypeError is meant to come from get_model_context_length (new kwargs vs. an old agent), by which point _cfg_base_url is bound — so it rarely bites. But if the helper at 6672 ever raises TypeError, the legacy fallback hits NameError instead of degrading gracefully, defeating the older-build safety net. The second callsite at api/streaming.py:6940-6952 already does this right: it nests the try/except TypeError around only the _get_cl(...) call, after _cfg_base_url is assigned. Making the session-save site symmetric (nest the inner try, or use getattr(agent,'base_url','') or resolved_base_url or '' in the fallback) removes the fragility:

_cfg_base_url = _ctx_lookup.base_url
try:
    _resolved_cl = get_model_context_length(..., _cfg_base_url, config_context_length=..., ...)
except TypeError:
    _resolved_cl = _legacy_cl(..., _cfg_base_url)

Not a blocker — the resolution logic itself is correct and well-scoped — just worth tightening before merge, plus a CHANGELOG line since this is user-visible.

@rodboev

rodboev commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 5927949. The session-save fallback now seeds \_cfg_base_url\ before the shared helper runs, so the legacy 2-arg retry cannot hit an unbound local if that helper ever raises \TypeError\. I also added the missing regression case for \providers..models..context_length\ with no configured \�ase_url\, plus the requested CHANGELOG note. Rechecked with \ ests/test_issue3717_context_length_provider_overrides.py tests/test_issue1896_context_length_fallback_args.py tests/test_issue1436_context_indicator_load_path.py -v --timeout=60\ (26 passed).

nesquena-hermes added a commit that referenced this pull request Jun 6, 2026
…des (#3726) (#3752)

@rodboev. providers.<name>.models.<model>.context_length overrides (standard provider,
no base_url) were invisible to the session context resolver → wrong window shown/persisted,
could trip auto-compression at the wrong threshold. New _context_length_lookup_inputs_for_model
helper resolves provider config / base_url / custom_providers across route-load, session-save,
and SSE-usage paths; provider-scoped overrides match by provider name and forward as
config_context_length (returned before any base-url-gated probe).

Maintainer pre-merge items both already satisfied in PR head: no-base_url regression test
(test_route_resolver_uses_provider_model_context_length_without_base_url) present; session-save
_cfg_base_url assigned before the helper call (safe-bound, no NameError on TypeError fallback).
Verified api code byte-identical to PR head; 14 context-length tests pass. + CHANGELOG v0.51.300.

Co-authored-by: nesquena-hermes <[email protected]>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @(author) — this fix shipped in v0.51.300. Your change was cherry-picked onto the release stage during the 2026-06-06 sweep (context-length indicator honors per-model overrides — stage-3726), so this PR is now redundant against master (it shows as conflicting because the fix is already present).

Closing as indirectly merged with full attribution. Appreciate the contribution! 🙏

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

(Correcting attribution above: thanks @rodboev for this — credited in the release CHANGELOG.)

SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…l overrides (nesquena#3726) (nesquena#3752)

@rodboev. providers.<name>.models.<model>.context_length overrides (standard provider,
no base_url) were invisible to the session context resolver → wrong window shown/persisted,
could trip auto-compression at the wrong threshold. New _context_length_lookup_inputs_for_model
helper resolves provider config / base_url / custom_providers across route-load, session-save,
and SSE-usage paths; provider-scoped overrides match by provider name and forward as
config_context_length (returned before any base-url-gated probe).

Maintainer pre-merge items both already satisfied in PR head: no-base_url regression test
(test_route_resolver_uses_provider_model_context_length_without_base_url) present; session-save
_cfg_base_url assigned before the helper call (safe-bound, no NameError on TypeError fallback).
Verified api code byte-identical to PR head; 14 context-length tests pass. + CHANGELOG v0.51.300.

Co-authored-by: nesquena-hermes <[email protected]>
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