fix: unify route dict or-None discipline in /model persist - #85920
Merged
kshitijk4poor merged 1 commit intoAug 14, 2026
Conversation
The route dict in _persist_model_switch_to_session used filtering (omits falsy values) while the top-level keys used (writes explicit None to trigger deletion in _merge_model_config_json). This asymmetry meant stale keys from a previous /model switch survived in the nested gateway_runtime dict even after the fix in NousResearch#85261 that properly deleted them from the top-level keys. Fix: build the route dict with and derive the top-level keys from **route so both shapes always use identical deletion semantics. Also filter None values in session_gateway_runtime's reader since gateway_runtime is replaced as a whole dict (not deep-merged), so None values written by the persist path survive in the nested dict. Found by /simplify-code 3-reviewer review on NousResearch#85261 (all 3 reviewers converged on the route dict asymmetry as the verdict-relevant finding).
kshitijk4poor
enabled auto-merge (rebase)
August 14, 2026 07:48
5 tasks
kshitijk4poor
pushed a commit
that referenced
this pull request
Aug 14, 2026
Salvage of #79604 (webtecnica) + #85721 (pierrenode), combined and rebased onto current main with simplify-code findings folded in. #79604: update_session_model() wrote the model name to sessions.model but never persisted the provider into model_config. On resume, the runtime recombined the persisted model with the config.yaml primary provider (which may not serve that model), producing auth errors. Fix: add optional provider parameter to update_session_model, merged into model_config via the shared _merge_model_config_json helper (not hand-rolled SQL). Wire both gateway /model call sites to pass result.target_provider. #85721: session_gateway_runtime() had no billing_provider fallback. A CLI session that never ran /model has no gateway_runtime or top-level provider in model_config — billing_provider (written on every session's first accounted API call) is the only durable record. Fix: add billing_provider as the last-resort fallback in session_gateway_runtime(), filtering bare billing buckets (auto/custom) that are not routable identities. Simplify-code findings addressed: - Use _merge_model_config_json instead of 40 lines of branched SQL - Share _BARE_BILLING_PROVIDERS from hermes_state.py (was duplicated as a set in tui_gateway/server.py) - Merge None-filtering from #85920 with the billing_provider fallback into one coherent return path Co-authored-by: pierrenode <298902573+pierrenode@users.noreply.github.com>
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
1 participant
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
Follow-up to #85261. The
/simplify-code3-reviewer review on #85261 found that theroutedict in_persist_model_switch_to_sessionusedif vfiltering (omits falsy values) while the top-level keys usedor None(writes explicitNoneto trigger deletion in_merge_model_config_json). This asymmetry meant stale keys from a previous/modelswitch could still survive in the nestedgateway_runtimedict even after #85261's fix that properly deleted them from the top-level keys.The bug: When a user switches from provider A (with
api_mode=anthropic_messages) to provider B (withapi_mode=""), the top-levelapi_modekey was correctly deleted (written asNone). But the nestedgateway_runtime.api_modewas merely omitted from the new dict — and sincegateway_runtimeis replaced as a whole dict by the merge (not deep-merged), the previous switch'sapi_modevalue was overwritten. However,Nonevalues written togateway_runtimesurvived because the reader (session_gateway_runtime) returneddict(runtime)without filteringNone.The fix (2 files + tests):
cli.py: Buildroutewithor Noneand derive top-level from**routeso both shapes always use identical deletion semantics. Eliminates the hand-maintained parallel between the falsy-filteredroutedict and theNone-coalesced top-level dict — the exact divergence that caused the original stale-key bug.hermes_state.py: FilterNonevalues insession_gateway_runtime's reader, sincegateway_runtimeis replaced as a whole dict (not deep-merged), soNonevalues written by the persist path survive in the nested dict.Test plan
tests/cli/test_resume_model_restore.py— 15 passed (updated 2 existing assertions + addedgateway_runtimestale-key assertions to existing test)tests/cli/test_resume_display.py— passedtests/test_hermes_state.py— passed (258 total, 0 failures)Found by /simplify-code review on #85261 — all 3 reviewers (reuse, quality, efficiency) converged on the route dict asymmetry as the verdict-relevant finding.