Fix/model switch custom providers - #10737
Conversation
drousselbot
left a comment
There was a problem hiding this comment.
QA review summary
Thanks for the detailed repro and the focused tests. I found a blocking gap before this is ready for QA pass:
- The PR now records
providerandbase_urlinset_runtime_model(...), butagent/auxiliary_client.pyonly consumes_runtime_model_override. Auxiliary routing still reads provider/runtime from config/env (_read_main_provider(),_resolve_custom_runtime(),get_available_vision_backends(),_resolve_auto()), so a runtime provider/base-URL switch is not actually propagated despite the new comments and test names implying it is. - In
cli.py, the new_ensure_runtime_credentials()guard restores onlyself.model. That still allowsself.provider/self.base_url/ related routing state to be overwritten on the next message if the active runtime switch changed provider, which is the same class of regression in a broader form. - The new tests verify the model override and that provider/base URL values are stored, but they do not verify the behavioral claim that auxiliary tasks actually resolve through the switched provider/base URL.
Validation run on this PR head:
python3 -m pytest -q tests/test_model_switch_fixes.py✅ (6 passed)gh pr checks 10737→ no checks reported yet- local code review found the routing gap above
Because of the unresolved runtime-routing mismatch, I’m requesting changes rather than marking this QA-passed.
|
Deterministic workflow-state update could not be persisted after QA review.
QA verdict for this PR is still changes requested / hand back to dev, but the lane state could not be recorded mechanically. Remediation: create the dispatcher workflow labels for this repo, then re-apply the intended QA transition. |
|
Thanks for identifying the custom-provider model-switch failure modes. Current Automated hermes-sweeper review evidence:
No release tag was determined from the inspected history. |
What does this PR do?
The
/modelpicker was broken for users withcustom_providersentries that use amodelsdict - only the singlemodelfield was read, so the picker always showed just one option and switching models was effectively impossible.This PR fixes model detection in the picker and resolves two additional bugs discovered during testing:
modelsdict -list_authenticated_providers()only read themodelfield from eachcustom_providersentry. Themodelsdict (used for multi-model provider configs) was ignored entirely._ensure_runtime_credentials()re-readmodel.defaultfromconfig.yamlon every message and overwrote the session switch._read_main_model()which reads from config, not the active session model. On single-GPU local LLM setups this caused a second model to be requested into VRAM, resulting in OOM errors.Related Issue
N/A
Type of Change
Changes Made
hermes_cli/model_switch.py: include keys frommodelsdict when building the picker's model list forcustom_providersentriescli.py(send_message): saveself.modelbefore_ensure_runtime_credentials()and restore it if the resolver overwrote it with the config defaultcli.py(_apply_model_switch_result): callset_runtime_model()to propagate the switch to auxiliary tasksagent/auxiliary_client.py: addset_runtime_model()and_runtime_model_override;_read_main_model()checks the override before falling back toconfig.yamlHow to Test
custom_providersentry with amodelsdict containing multiple model IDs, run/model- all models should appear in the picker/model, send a message - verify the switched model is used, not the config defaultChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -q- 6 new tests pass; 26 pre-existing failures onmainare unrelated to these changesDocumentation & Housekeeping
docs/, docstrings) - N/Acli-config.yaml.exampleif I added/changed config keys - N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows - N/AScreenshots / Logs
LM Studio server logs before fix - two different models requested per exchange:
After fix - single model throughout:
Co-developed with Claude Code