fix(setup): preserve model selection on provider re-run

[Protocol-zero-0]

Summary

• preserve selected_model when the setup wizard is re-run with the same LLM backend
• clear the stored model only when the user actually switches to a different backend
• add regression tests and sync the onboarding spec in src/setup/README.md

Problem

Re-running onboarding remembered most prior settings, but the model name was lost and had to be entered again.

Root cause

Provider setup helpers reset selected_model unconditionally, even when the backend did not change.

Fix

Introduce a shared helper that updates the backend while only clearing selected_model on real backend changes.

Tests

• cargo test --lib test_set_llm_backend
• cargo fmt --check
• cargo clippy --all --benches --tests --examples --all-features -- -D warnings

Scope / Non-goals

• does not change model selection UX beyond preserving the existing model on same-backend re-runs
• does not change provider-specific model fetching behavior

Made with Cursor

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the user experience of the setup wizard by intelligently managing the selected LLM model. Previously, re-running the setup process would unconditionally clear the chosen model, forcing users to re-enter it even if they were using the same backend. The changes introduce a new helper function that ensures the model selection is preserved when the backend is not changed, only clearing it when a different backend is explicitly chosen. This streamlines the setup process and reduces redundant user input.

Highlights

• Model Selection Preservation: Implemented a mechanism to preserve the selected_model during the setup wizard's re-run if the LLM backend remains unchanged.
• Conditional Model Clearing: Ensured that the selected_model is only cleared when a user explicitly switches to a different LLM backend, preventing unnecessary re-entry.
• New Helper Function: Introduced a new shared helper function, set_llm_backend_preserving_model, to encapsulate the logic for updating the backend and conditionally clearing the model.
• Regression Tests: Added dedicated regression tests to validate the new model preservation and clearing behavior under different scenarios.
• Documentation Update: Updated the src/setup/README.md documentation to reflect the revised model selection handling during the onboarding process.

Changelog

• src/setup/README.md
  • Updated description for OpenAI-compatible presets to clarify selected_model clearing conditions.
  • Added a point under API-key providers regarding selected_model preservation when the backend is unchanged.
• src/setup/wizard.rs
  • Added a new private method set_llm_backend_preserving_model to handle LLM backend assignment and conditional model clearing.
  • Replaced direct assignments to self.settings.llm_backend and subsequent selected_model clearing with calls to set_llm_backend_preserving_model in setup_no_wizard_provider, setup_nearai, setup_api_key_provider, setup_openai_compatible_preset, and setup_openai_compatible_provider.
  • Added two new unit tests: test_set_llm_backend_preserves_model_when_backend_unchanged and test_set_llm_backend_clears_model_when_backend_changes.

Activity

• Implemented a new helper function to manage LLM backend and model selection.
• Refactored existing setup functions to utilize the new helper.
• Added comprehensive unit tests to cover the new logic.
• Updated relevant documentation to reflect the changes in behavior.
• No external review comments or discussions have occurred yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request fixes a bug where the selected model was unconditionally cleared when re-running the setup wizard, even if the LLM provider remained the same. The fix introduces a new helper function, set_llm_backend_preserving_model, which conditionally clears the selected_model only when the backend provider actually changes. This new function is consistently applied across all provider setup helpers, which not only fixes the bug but also makes the behavior uniform. The pull request includes regression tests covering the new logic and updates the README.md to document the corrected behavior. The changes are correct and improve the user experience of the setup wizard.

[Copilot]

Pull request overview

This PR fixes a setup-wizard regression where re-running onboarding with the same LLM backend unnecessarily cleared the previously selected model, forcing users to re-enter it.

Changes:

• Introduce a shared helper to set llm_backend while only clearing selected_model when the backend actually changes.
• Update provider setup paths to use the new helper consistently.
• Add regression tests for the “preserve vs clear model” behavior and update the onboarding spec.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/setup/wizard.rs	Adds set_llm_backend_preserving_model() and routes provider setup flows through it; adds regression tests.
src/setup/README.md	Updates onboarding documentation to reflect the new “preserve model unless backend changes” behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This section references setup_openrouter, but there doesn’t appear to be any such function in the codebase (provider setup is dispatched via run_provider_setup() and registry SetupHints). Consider updating the spec to describe the actual implementation entrypoint(s) so future changes don’t chase a nonexistent helper.

[Protocol-zero-0]

Addressed the review feedback in commit 810db70.

I updated src/setup/README.md to reference the actual OpenRouter setup path: run_provider_setup() dispatches to setup_api_key_provider(), rather than implying a standalone setup_openrouter helper.

[zmanian]

The helper function is clean and correct. Tests and spec update included.

Blocking: Two call sites missed -- setup_anthropic_oauth and setup_ollama_generic still use the inline pattern instead of the new set_llm_backend_preserving_model() helper. Per CLAUDE.md's "fix the pattern, not just the instance" rule, all call sites should use the shared helper. This is a ~4-line change.

Non-blocking:

• Consider adding a test for the None initial state (first-time setup where llm_backend is None)
• Minor spec phrasing ambiguity: "Clears selected_model only when switching from a different backend" could be clearer

[Protocol-zero-0]

Follow-up pushed in 013c972.

What I updated:

• added a regression test for the initial None -> Some(backend) case to make the helper semantics explicit
• clarified the setup docs wording around same-backend re-runs vs backend switches

Also, I double-checked the two call sites mentioned in review:

• setup_ollama_generic() already uses set_llm_backend_preserving_model() in the current branch
• Anthropic now flows through the shared setup_api_key_provider() path rather than a separate setup_anthropic_oauth() helper

Local validation run:

• cargo test --lib test_set_llm_backend_ -- --nocapture

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test name test_set_llm_backend_preserves_model_when_backend_was_unset doesn’t match the asserted behavior: when llm_backend is unset, set_llm_backend_preserving_model() treats this as a backend change and clears selected_model (the test asserts selected_model == None). Rename the test (or adjust the expectation) so the name reflects what’s actually being verified.

[Copilot]

This OpenRouter onboarding spec is out of sync with the current provider registry: providers.json defines OpenRouter as its own backend id ("openrouter") with its own env var/secret (OPENROUTER_API_KEY / llm_openrouter_api_key), not as llm_backend = "openai_compatible". Please update these bullets (and any related text) to reflect that OpenRouter is stored as openrouter and only shares the openai_compatible_base_url field as a backward-compat fallback in config resolution.

[Protocol-zero-0]

Pushed a follow-up in to address the remaining review drift:\n\n- corrected the OpenRouter setup docs to match the current backend/secret/env names (, , )\n- clarified that is only kept as a compatibility fallback\n- renamed the unset-backend regression test so the name matches the asserted behavior\n\nI also kicked off the focused test target locally again: .

[Protocol-zero-0]

Pushed a follow-up in 34bf3e2 to close the remaining review drift.

Updated:

• aligned the OpenRouter setup docs with the current backend identity and credential names (openrouter, llm_openrouter_api_key, OPENROUTER_API_KEY)
• clarified that openai_compatible_base_url is only retained as a compatibility fallback
• renamed the unset-backend regression test so the name matches the behavior it asserts

This should address the remaining doc/test mismatches called out after the earlier helper rollout.

[Protocol-zero-0]

Focused local validation completed after the latest follow-up:

• cargo test --lib test_set_llm_backend_ -- --nocapture
• Result: 0 failed (the target filter matched the updated helper tests as expected on this branch)

So the doc/test alignment follow-up is now both pushed and locally revalidated.

[Protocol-zero-0]

Thanks again for the review.

I believe the blocking items from the latest review are now addressed on this branch:

• the remaining helper rollout drift was fixed
• the None -> Some(backend) case is now covered explicitly
• the spec/test wording drift was cleaned up
• focused helper tests were re-run locally after the follow-up

If you get a chance, I'd appreciate a re-review.

[zmanian]

All blocking feedback addressed:

1. Missed call sites fixed -- all 5 call sites now use set_llm_backend_preserving_model(), including setup_oauth_provider and the generic provider fallback path.
2. First-time setup test added -- test_set_llm_backend_clears_model_when_backend_was_unset covers the None initial state.
3. Spec phrasing clarified -- README now explicitly documents the preserve/clear behavior.

Clean, focused change. Approved.

Note: This is a fork PR -- CI only ran classify/scope. Verified the logic by reading the diff; the change is small (helper function + consistent call-site updates) and well-tested.

[zmanian]

Replacement PR opened as #987 with the approved model-retention fix rebased onto current staging. It preserves the original behavior change, updates the current shared backend-selection flow, and includes focused regression coverage.

[zmanian]

Superseded by #987, which has merged into staging.