fix(tui): list saved models from all providers in /model picker

[ousamabenyounes]

Summary

• The /model picker only listed the active provider's models plus that provider's saved model, so a custom model saved under a different provider in config (e.g. kimi-k2.6 under moonshot while the active provider is deepseek) was invisible in the picker — even though /model kimi-k2.6 typed directly resolves and the model runs.
• picker_model_rows_for_app now appends cross-provider saved models from provider_models as a clearly labelled tail (<Provider> saved) after the active provider's list. The active provider's models still come first.
• Selecting a cross-provider row switches CodeWhale to that provider on apply, reusing the existing resolved_provider / build_event path (no new apply logic). Config keys that map to no known provider are skipped since they cannot be applied.

Fixes #2596.

Design note

picker_hides_saved_models_from_other_providers previously asserted these models stay hidden. That test is repurposed into picker_lists_saved_models_from_other_providers to reflect the issue's requested behaviour (Option C — keep the active list first, add a labelled cross-provider tail). The apply path already supported a row carrying its own provider, so this only changes what rows are built, not how they apply.

Test verification (RED -> GREEN)

New/updated test: picker_lists_saved_models_from_other_providers.

RED (prod fix reverted, cross-provider loop removed):

test tui::model_picker::tests::picker_lists_saved_models_from_other_providers ... FAILED
panicked at crates/tui/src/tui/model_picker.rs: assert model_ids.contains("deepseek-v4-pro")
test result: FAILED. 28 passed; 1 failed

GREEN (fix applied):

test tui::model_picker::tests::picker_lists_saved_models_from_other_providers ... ok
test tui::model_picker::tests::picker_skips_unknown_provider_saved_models ... ok
test result: ok. 29 passed; 0 failed

Testing

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features
• cargo test --workspace --all-features — 4608 passed, 0 failed (baseline 4607; +1 new test, no regressions)

Checklist

• Updated docs or comments as needed
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes

Greptile Summary

Makes cross-provider saved models visible in the /model picker by appending them as a labelled tail after the active provider's list, and switches the provider on apply via the existing resolved_provider / build_event path.

• picker_model_rows_for_app now iterates provider_models, parses each key with ApiProvider::parse, skips the active provider and unknown keys, sorts the remainder deterministically by config key, and pushes one labelled row per entry.
• picker_hides_saved_models_from_other_providers is repurposed into picker_lists_saved_models_from_other_providers with assertions for ordering, row-level provider attribution, and provider-switch on apply; a new picker_skips_unknown_provider_saved_models test covers the filter-out path.

Confidence Score: 4/5

Safe to merge; the apply path is unchanged and all existing tests pass with one new test converted from failing to green.

The change is contained to row-building logic with no modification to how events are emitted or applied. There is one untested edge case: when the user's active model was set via a direct /model command to a cross-provider ID rather than through the picker, that model now appears both as a labelled tail row and as the current (custom) row — a cosmetic duplication that does not cause incorrect behaviour but could confuse users.

crates/tui/src/tui/model_picker.rs — specifically the ModelPickerView::new selection logic and the interaction between show_custom_model_row and the new cross-provider tail rows.

Important Files Changed

Filename	Overview
crates/tui/src/tui/model_picker.rs	Adds a cross-provider saved-model tail to the picker row list, sorts it deterministically by config key, filters out unknown providers, and updates tests to assert the new behaviour. The core fix is correct; a minor edge case exists where the model can appear twice if set via direct command rather than the picker.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[picker_model_rows_for_app] --> B[Push auto row]
    B --> C[Push active provider known models]
    C --> D{Active provider saved model exists?}
    D -- Yes --> E[Push Active-Provider saved row]
    D -- No --> F
    E --> F[Iterate all provider_models entries]
    F --> G{ApiProvider::parse succeeds?}
    G -- No unknown key --> H[Skip entry]
    G -- Yes --> I{Same as active provider?}
    I -- Yes --> H
    I -- No --> J[Collect cross-provider entry]
    J --> K[Sort by config key string]
    K --> L{model string empty after trim?}
    L -- Yes --> H
    L -- No --> M[Push Provider-saved row with provider set]
    M --> N[Return rows]
    H --> N

Loading

_{Reviews (3): Last reviewed commit: "fix(tui): use sort_by_key to satisfy cli..." | Re-trigger Greptile}

[github-actions]

Thanks @ousamabenyounes for taking the time to contribute.

This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in .github/APPROVED_CONTRIBUTORS will be closed automatically.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant PR access by commenting /lgtm on a pull request.

[gemini-code-assist]

Code Review

This pull request updates the model picker to surface models saved under other providers in the configuration, ensuring they are reachable and clearly labeled. The active provider's models are displayed first, followed by the cross-provider saved models, while unknown provider keys are skipped. Unit tests have been updated and added to verify this behavior. Feedback suggests optimizing the collection of other provider models by using filter_map to parse the ApiProvider once, which simplifies the loop and avoids redundant parsing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

We can optimize this by parsing the ApiProvider once using filter_map instead of parsing it twice (once in filter and once in the loop). This also automatically filters out unknown provider keys and simplifies the loop logic.

    let mut other_provider_models: Vec<(&String, ApiProvider, &String)> = app
        .provider_models
        .iter()
        .filter_map(|(key, model)| {
            let provider = ApiProvider::parse(key)?;
            if provider != app.api_provider {
                Some((key, provider, model))
            } else {
                None
            }
        })
        .collect();
    other_provider_models.sort_by(|(a, _, _), (b, _, _)| a.cmp(b));
    for (_, provider, model) in other_provider_models {
        let model = model.trim();
        if model.is_empty() {
            continue;
        }
        push_model_row(
            &mut rows,
            model.to_string(),
            Some(provider),
            format!("{} saved", provider.display_name()),
        );
    }

[ousamabenyounes]

Done in db0ce5b — collapsed the two ApiProvider::parse calls into a single filter_map that drops unknown keys and the active provider in one pass and carries the parsed provider through to the loop.

One nuance on framing: this is a readability win, not really an optimization. The original code only parsed twice for keys that survive the filter, the map is tiny, and ApiProvider::parse is a cheap string match — so there's no measurable perf delta. The value is fewer moving parts and no duplicated parse logic. Behavior is identical: unknown keys are still skipped, the active provider is still excluded, and ordering stays deterministic via the sort on the provider key.

cargo fmt/clippy/picker tests all green.