fix(gateway): show channel override model/provider in session info banner - #62886
fix(gateway): show channel override model/provider in session info banner#62886huseyinevecen wants to merge 1 commit into
Conversation
…nner The /new and auto-reset banners resolved model/provider from the global config only, so chats with a channel_overrides entry advertised a model the session would never use. Pass the SessionSource down to _format_session_info and let the override win, matching what the agent actually runs. Also keeps the override provider when runtime credential resolution fails, instead of silently dropping it.
Competing with #62827 for the same fix: both thread |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the reset-banner mismatch; current main does reproduce it because gateway/run.py:12320-12325 formats reset info without a SessionSource, while the runtime resolver applies channel overrides at gateway/run.py:3822-3835.
Problems
- The new override branch still obtains
base_urlandapi_keyfrom the global_resolve_runtime_agent_kwargs()atgateway/run.py:12420-12423. The execution path resolves override-provider runtime settings via_resolve_runtime_agent_kwargs_for_provider()atgateway/run.py:3832-3835. Thus the same info block can show an override model/provider while its context probe or optional endpoint reflects the global provider. - The added tests call
_format_session_info()directly (tests/gateway/test_session_info.py:141) rather than the changed production helper_reset_notice_session_info(source)(gateway/run.py:12320-12323).
Suggested changes
- Reuse or extract the effective runtime resolution needed for display, including provider-specific endpoint/credentials, while preserving the current no-credential fallback.
- Cover the reset helper with an override source, plus a global-versus-override runtime-endpoint case.
Automated hermes-sweeper review.
| @@ -12398,11 +12418,11 @@ def _format_session_info(self) -> str: | |||
| # Resolve runtime credentials for probing | |||
| try: | |||
| runtime = _resolve_runtime_agent_kwargs() | |||
There was a problem hiding this comment.
This still resolves runtime credentials and base_url from the global provider. The execution path uses _resolve_runtime_agent_kwargs_for_provider(ch.provider) for a channel override (gateway/run.py:3832-3835), so context probing and the optional endpoint can remain global even though the displayed model/provider is overridden. Please use the effective override runtime here as well.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the reset-banner mismatch by passing SessionSource into _format_session_info so channel overrides can replace the misleading global model. #62827 additionally attempts /model precedence but does not modify the independent /status path, while #62886 narrowly handles channel model/provider overrides but still mixes override identity with global provider runtime settings.
Related pull requests
- #62827 [closed]
duplicate— (+200/-3) — superseded as the consolidation base: The diff fixes channel-override model selection and adds /model precedence inside _format_session_info, but it does not change the independent /status implementation, does not resolve provider-specific runtime settings, and its /model test does not exercise the real /new path that clears that override. Despite the keep_open review on #62827, keeping it closed is justified because its broader stated scope is not delivered and the still-open #62886 provides the smaller overlapping base. - #62886
related— (+89/-5) — keep open with required changes: The diff directly fixes the reported /new/reset-banner cause by passing SessionSource and selecting the channel override model/provider, but endpoint, credentials, and context probing can still come from the global provider. This matches the keep_open review on #62886; it should reuse the effective provider-specific runtime resolution and test _reset_notice_session_info(source), not only _format_session_info directly.
Duplicates
#62827 and #62886 substantially duplicate the core change of threading SessionSource into _format_session_info and applying channel_overrides; #62827 adds an incomplete /model-resolution extension, while neither diff actually fixes the independent /status path.
Suggested consolidation
Merge #62886 after resolving the contributor review by sharing the effective model/provider runtime resolver, including provider-specific endpoint and credentials, and adding production-path coverage through _reset_notice_session_info(source). Keep #62827 closed as the superseded duplicate; carry over its useful /model-precedence case only if production behavior after /new supports it, and handle /status explicitly rather than claiming this formatter change fixes it.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup62827 ["PRs duplicating each other"]
P62827["PR #62827 (closed)"]
P62886["PR #62886 (open)"]
end
class P62827 closed
class P62886 open
class P62886 target
click P62827 "https://github.com/NousResearch/hermes-agent/pull/62827"
click P62886 "https://github.com/NousResearch/hermes-agent/pull/62886"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 5 kB of issue/PR text, 3 kB of discussion (3 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
What
The
/newreset banner and the auto-reset notice build their session info block from the globalconfig.yamlmodel only. Chats configured with achannel_overridesentry (per-channel model/provider) get a banner advertising a model the session never uses.Why
_format_session_info()had no access to the message source, so it could only resolve the global default. The caller (_reset_notice_session_info) already receives theSessionSource— this change passes it down and applies the same_get_channel_overridelookup the message path uses (chat_id → thread_id → parent_id), so the banner reports what the session actually runs.Also: when runtime credential resolution raises, the override provider was silently dropped — now kept.
How to test
config.yaml:/newin that chat.model.defaultfrom config. After: banner shows the override model/provider.Unit tests:
tests/gateway/test_session_info.py::TestChannelOverrideSessionInfo(override shown, no-source fallback, unmatched chat fallback, runtime-failure keeps override provider). Ran the full gateway session-info/override test files — 104 passed.Platforms tested
Linux (Ubuntu, gateway + Photon/iMessage adapter with a live channel override).