refactor: unify setup wizard provider selection with hermes model - #4200
Merged
Conversation
) setup_model_provider() had 800+ lines of duplicated provider handling that reimplemented the same credential prompting, OAuth flows, and model selection that hermes model already provides via the _model_flow_* functions. Every new provider had to be added in both places, and the two implementations diverged in config persistence (setup.py did raw YAML writes, _set_model_provider, and _update_config_for_provider depending on the provider — main.py used its own load/save cycle). This caused the #4172 bug: _model_flow_custom saved config to disk but the wizard's final save_config(config) overwrote it with stale values. Fix: extract the core of cmd_model() into select_provider_and_model() and have setup_model_provider() call it. After the call, re-sync the wizard's config dict from disk. Deletes ~800 lines of duplicated provider handling from setup.py. Also fixes cmd_model() double-AuthError crash on fresh installs with no API keys configured.
Contributor
|
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 #4180. Eliminates the root cause of the entire class of setup wizard config overwrite bugs by unifying the provider/model selection code path.
setup_model_provider()had 800+ lines of duplicated provider handling that reimplemented the same credential prompting, OAuth flows, and model selection thathermes modelalready provides. Every new provider had to be added in both places, and the two implementations diverged in config persistence patterns (raw YAML writes,_set_model_provider,_update_config_for_providerdepending on which provider — inconsistent and error-prone).Fix: Extract the core of
cmd_model()intoselect_provider_and_model()and havesetup_model_provider()call it. After the call, re-sync the wizard's config dict from disk. Deletes ~800 lines of duplicated provider handling from setup.py.Also fixes
cmd_model()double-AuthError crash on fresh installs with no API keys configured.Changes
hermes_cli/main.py— Extractselect_provider_and_model()fromcmd_model(), fix double-AuthError (+16 lines)hermes_cli/setup.py— Replace 800+ lines of inline provider handling with delegation (-815 lines)tests/hermes_cli/test_setup.py— Rewrite to test delegation + config synctests/hermes_cli/test_setup_model_provider.py— Rewrite to test delegation + config syncNet: -1135 lines
Test plan
hermes setup→ Model & Provider → Custom endpoint → config survives wizard's final savehermes setup→ Model & Provider → OpenRouter → config correcthermes setup→ Model & Provider → Cancel → config unchanged