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

[zmanian]

Summary

• rebase the approved #679 setup wizard model-retention fix onto current staging
• preserve selected_model when re-running setup for the same LLM backend
• clear selected_model only when switching to a different backend
• add focused regression coverage for the shared backend-selection helper

Why this replacement exists

Original PR #679 is approved but dirty after subsequent onboarding changes landed on staging. This replacement preserves the approved behavior change on top of current setup flows and narrows the docs update to what still matches the current onboarding implementation.

Supersedes approved PR #679.

Validation

• cargo test set_llm_backend -- --nocapture
• cargo fmt --check

[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 integrates a crucial fix from a previously approved PR (#679) into the current staging branch, addressing an issue in the LLM setup wizard. The primary goal is to enhance user experience by intelligently managing the selected_model during backend configuration. Specifically, it ensures that a user's model choice is retained if they re-configure the same LLM provider, preventing the need to re-select. The model is now only reset when a user explicitly opts to switch to a different LLM backend, streamlining the setup process and avoiding accidental data loss.

Highlights

• Rebased Fix: Rebased the approved fix from PR fix(setup): preserve model selection on provider re-run #679 onto the current staging branch to ensure compatibility with recent changes.
• Model Preservation: Implemented logic to preserve the selected_model when re-running the setup for the same LLM backend, preventing unnecessary re-selection.
• Conditional Model Clearing: Ensured that selected_model is cleared only when switching to a different LLM backend, aligning with user intent.
• Test Coverage: Added focused regression tests for the new shared backend-selection helper function to validate its behavior.

Changelog

• src/setup/README.md
  • Added a new step to the LLM backend setup instructions, clarifying that selected_model is preserved on same-backend re-runs and cleared only when switching backends.

Activity

• This pull request supersedes and rebases the approved PR fix(setup): preserve model selection on provider re-run #679 onto the current staging branch.
• The author has provided validation steps including running cargo test set_llm_backend -- --nocapture and cargo fmt --check.

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 effectively addresses the issue of preserving the selected model when re-running the setup for the same LLM provider. The introduction of the set_llm_backend_preserving_model helper function is a great refactoring that centralizes the logic and removes duplication across multiple setup functions. The new tests for this helper function are well-written and cover the core logic. I have one suggestion regarding test redundancy to further improve maintainability.

[gemini-code-assist]

These new tests for set_llm_backend_preserving_model are great and cover the new helper function's logic well.

However, they make the existing regression tests for #600 (test_same_provider_preserves_selected_model and test_different_provider_clears_selected_model) redundant. Those tests now simulate logic that has been refactored away, which could be confusing.

To improve clarity and avoid duplication, consider removing the old regression tests for #600. The new tests are more focused and accurately test the current implementation.

[claude]

Code review

Found 2 issues:

1. [CRITICAL:100] Typo in test method name will cause compile failure

In the second added test test_set_llm_backend_clears_model_when_backend_was_unset, the method call has a typo:

wizard.set_llm_backend_preparving_model("openai");

Should be preserving_model (not preparving_model). This test will not compile.

https://github.com/anthropics/ironclaw/blob/cee65036ad7ab68ae04bcee6386c0a0d317afc86/src/setup/wizard.rs#L3881-L3883

2. [HIGH:95] Incomplete README.md documentation per spec requirements

Per CLAUDE.md: "Any code change to src/setup/ must keep this document in sync." The PR updates lines 230-231 of README.md to mention preserving selected_model, but doesn't document the new refactoring pattern set_llm_backend_preserving_model() as the canonical implementation. Maintainers reading the spec should understand that backend changes now go through this centralized method.

https://github.com/anthropics/ironclaw/blob/cee65036ad7ab68ae04bcee6386c0a0d317afc86/src/setup/README.md#L227-L230

Positive findings:

• Effective DRY refactoring eliminating 6 instances of duplicated logic
• Logic correctly handles all backend transitions (None→Some, same, different)
• Tests cover state transitions well (once typo is fixed)
• No security, async, blocking, or allocation issues in the refactored code