diff --git a/cmd/launch/integrations_test.go b/cmd/launch/integrations_test.go index 1219f4e5..ef6b996c 100644 --- a/cmd/launch/integrations_test.go +++ b/cmd/launch/integrations_test.go @@ -610,6 +610,36 @@ func TestBuildModelList_CheckedBeforeRecs(t *testing.T) { } } +func TestBuildModelList_CurrentPrefersExactLocalOverCloudPrefix(t *testing.T) { + existing := []modelInfo{ + {Name: "qwen3.5:cloud", Remote: true}, + {Name: "qwen3.5", Remote: false}, + } + + _, orderedChecked, _, _ := buildModelList(existing, []string{"qwen3.5", "qwen3.5:cloud"}, "qwen3.5") + if len(orderedChecked) < 2 { + t.Fatalf("expected orderedChecked to preserve both selections, got %v", orderedChecked) + } + if orderedChecked[0] != "qwen3.5" { + t.Fatalf("expected exact local current to stay first, got %v", orderedChecked) + } +} + +func TestBuildModelList_CurrentPrefersExactCloudOverLocalPrefix(t *testing.T) { + existing := []modelInfo{ + {Name: "qwen3.5", Remote: false}, + {Name: "qwen3.5:cloud", Remote: true}, + } + + _, orderedChecked, _, _ := buildModelList(existing, []string{"qwen3.5:cloud", "qwen3.5"}, "qwen3.5:cloud") + if len(orderedChecked) < 2 { + t.Fatalf("expected orderedChecked to preserve both selections, got %v", orderedChecked) + } + if orderedChecked[0] != "qwen3.5:cloud" { + t.Fatalf("expected exact cloud current to stay first, got %v", orderedChecked) + } +} + func TestEditorIntegration_SavedConfigSkipsSelection(t *testing.T) { tmpDir := t.TempDir() setTestHome(t, tmpDir) diff --git a/cmd/launch/launch.go b/cmd/launch/launch.go index 5704c4aa..2109b05f 100644 --- a/cmd/launch/launch.go +++ b/cmd/launch/launch.go @@ -515,7 +515,7 @@ func (c *launcherClient) selectSingleModelWithSelector(ctx context.Context, titl return "", fmt.Errorf("no selector configured") } - items, _, err := c.loadSelectableModels(ctx, singleModelPrechecked(current), current, "no models available, run 'ollama pull ' first") + items, _, err := c.loadSelectableModels(ctx, nil, current, "no models available, run 'ollama pull ' first") if err != nil { return "", err } @@ -535,10 +535,21 @@ func (c *launcherClient) selectMultiModelsForIntegration(ctx context.Context, ru return nil, fmt.Errorf("no selector configured") } - items, orderedChecked, err := c.loadSelectableModels(ctx, preChecked, firstModel(preChecked), "no models available") + current := firstModel(preChecked) + + items, orderedChecked, err := c.loadSelectableModels(ctx, preChecked, current, "no models available") if err != nil { return nil, err } + if len(preChecked) > 0 { + // Keep list order stable in multi-select even when there are existing checks. + // checked/default state still comes from orderedChecked. + stableItems, _, stableErr := c.loadSelectableModels(ctx, nil, current, "no models available") + if stableErr != nil { + return nil, stableErr + } + items = stableItems + } selected, err := DefaultMultiSelector(fmt.Sprintf("Select models for %s:", runner), items, orderedChecked) if err != nil { diff --git a/cmd/launch/launch_test.go b/cmd/launch/launch_test.go index ca29afcf..3c77bd0f 100644 --- a/cmd/launch/launch_test.go +++ b/cmd/launch/launch_test.go @@ -330,6 +330,60 @@ func TestResolveRunModel_ForcePickerAlwaysUsesSelector(t *testing.T) { } } +func TestResolveRunModel_ForcePicker_DoesNotReorderByLastModel(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + + if err := config.SetLastModel("qwen3.5"); err != nil { + t.Fatalf("failed to save last model: %v", err) + } + + var gotNames []string + DefaultSingleSelector = func(title string, items []ModelItem, current string) (string, error) { + if current != "qwen3.5" { + t.Fatalf("expected current selection to be last model, got %q", current) + } + + gotNames = make([]string, 0, len(items)) + for _, item := range items { + gotNames = append(gotNames, item.Name) + } + return "qwen3.5", nil + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/tags": + fmt.Fprint(w, `{"models":[{"name":"qwen3.5"},{"name":"glm-4.7-flash"}]}`) + case "/api/show": + fmt.Fprint(w, `{"model":"qwen3.5"}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + _, err := ResolveRunModel(context.Background(), RunModelRequest{ForcePicker: true}) + if err != nil { + t.Fatalf("ResolveRunModel returned error: %v", err) + } + + if len(gotNames) == 0 { + t.Fatal("expected selector to receive model items") + } + + glmIdx := slices.Index(gotNames, "glm-4.7-flash") + qwenIdx := slices.Index(gotNames, "qwen3.5") + if glmIdx == -1 || qwenIdx == -1 { + t.Fatalf("expected recommended local models in selector items, got %v", gotNames) + } + if qwenIdx < glmIdx { + t.Fatalf("expected list order to stay stable and not float last model to top, got %v", gotNames) + } +} + func TestResolveRunModel_UsesSignInHookForCloudModel(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) @@ -448,6 +502,74 @@ func TestLaunchIntegration_EditorForceConfigure(t *testing.T) { } } +func TestLaunchIntegration_EditorForceConfigure_DoesNotFloatCheckedModelsInPicker(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + + binDir := t.TempDir() + writeFakeBinary(t, binDir, "droid") + t.Setenv("PATH", binDir) + + editor := &launcherEditorRunner{} + withIntegrationOverride(t, "droid", editor) + + if err := config.SaveIntegration("droid", []string{"qwen3.5:cloud", "qwen3.5"}); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + var gotItems []string + var gotPreChecked []string + DefaultMultiSelector = func(title string, items []ModelItem, preChecked []string) ([]string, error) { + for _, item := range items { + gotItems = append(gotItems, item.Name) + } + gotPreChecked = append([]string(nil), preChecked...) + return []string{"qwen3.5:cloud", "qwen3.5"}, nil + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/tags": + fmt.Fprint(w, `{"models":[{"name":"qwen3.5:cloud","remote_model":"qwen3.5"},{"name":"qwen3.5"}]}`) + case "/api/show": + var req apiShowRequest + _ = json.NewDecoder(r.Body).Decode(&req) + if req.Model == "qwen3.5:cloud" { + fmt.Fprint(w, `{"remote_model":"qwen3.5"}`) + return + } + fmt.Fprintf(w, `{"model":%q}`, req.Model) + case "/api/me": + fmt.Fprint(w, `{"name":"test-user"}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + if err := LaunchIntegration(context.Background(), IntegrationLaunchRequest{ + Name: "droid", + ForceConfigure: true, + }); err != nil { + t.Fatalf("LaunchIntegration returned error: %v", err) + } + + if len(gotItems) == 0 { + t.Fatal("expected multi selector to receive items") + } + if gotItems[0] != "kimi-k2.5:cloud" { + t.Fatalf("expected stable recommendation order with kimi-k2.5:cloud first, got %v", gotItems) + } + if len(gotPreChecked) < 2 { + t.Fatalf("expected prechecked models to be preserved, got %v", gotPreChecked) + } + if gotPreChecked[0] != "qwen3.5:cloud" { + t.Fatalf("expected saved default to remain first in prechecked, got %v", gotPreChecked) + } +} + func TestLaunchIntegration_EditorModelOverridePreservesExtras(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) diff --git a/cmd/launch/models.go b/cmd/launch/models.go index 40cb9e41..3befb5d2 100644 --- a/cmd/launch/models.go +++ b/cmd/launch/models.go @@ -295,12 +295,25 @@ func buildModelList(existing []modelInfo, preChecked []string, current string) ( checked[n] = true } - for _, item := range items { - if item.Name == current || strings.HasPrefix(item.Name, current+":") { - current = item.Name - break + if current != "" { + matchedCurrent := false + for _, item := range items { + if item.Name == current { + current = item.Name + matchedCurrent = true + break + } + } + if !matchedCurrent { + for _, item := range items { + if strings.HasPrefix(item.Name, current+":") { + current = item.Name + break + } + } } } + if checked[current] { preChecked = append([]string{current}, slices.DeleteFunc(preChecked, func(m string) bool { return m == current })...) } diff --git a/cmd/tui/selector.go b/cmd/tui/selector.go index c863f089..28ffda56 100644 --- a/cmd/tui/selector.go +++ b/cmd/tui/selector.go @@ -377,13 +377,24 @@ func (m selectorModel) View() string { // cursorForCurrent returns the item index matching current, or 0 if not found. func cursorForCurrent(items []SelectItem, current string) int { - if current != "" { - for i, item := range items { - if item.Name == current || strings.HasPrefix(item.Name, current+":") || strings.HasPrefix(current, item.Name+":") { - return i - } + if current == "" { + return 0 + } + + // Prefer exact name matches before tag-prefix fallback so "qwen3.5" does not + // incorrectly select "qwen3.5:cloud" (and vice versa) based on list order. + for i, item := range items { + if item.Name == current { + return i } } + + for i, item := range items { + if strings.HasPrefix(item.Name, current+":") || strings.HasPrefix(current, item.Name+":") { + return i + } + } + return 0 } @@ -529,6 +540,7 @@ func (m *multiSelectorModel) toggleItem() { origIdx := m.itemIndex[item.Name] if m.checked[origIdx] { + wasDefault := len(m.checkOrder) > 0 && m.checkOrder[len(m.checkOrder)-1] == origIdx delete(m.checked, origIdx) for i, idx := range m.checkOrder { if idx == origIdx { @@ -536,6 +548,34 @@ func (m *multiSelectorModel) toggleItem() { break } } + if wasDefault { + // When removing the default, pick the nearest checked model above it + // (or below if none above) so default fallback follows list order. + newDefault := -1 + for i := origIdx - 1; i >= 0; i-- { + if m.checked[i] { + newDefault = i + break + } + } + if newDefault == -1 { + for i := origIdx + 1; i < len(m.items); i++ { + if m.checked[i] { + newDefault = i + break + } + } + } + if newDefault != -1 { + for i, idx := range m.checkOrder { + if idx == newDefault { + m.checkOrder = append(m.checkOrder[:i], m.checkOrder[i+1:]...) + break + } + } + m.checkOrder = append(m.checkOrder, newDefault) + } + } } else { m.checked[origIdx] = true m.checkOrder = append(m.checkOrder, origIdx) diff --git a/cmd/tui/selector_test.go b/cmd/tui/selector_test.go index 33e12a02..53700bba 100644 --- a/cmd/tui/selector_test.go +++ b/cmd/tui/selector_test.go @@ -232,6 +232,25 @@ func TestSelectorModelWithCurrent_ScrollsToCurrentInMoreSection(t *testing.T) { } } +func TestSelectorModelWithCurrent_HighlightsExactLocalWhenCloudVariantExists(t *testing.T) { + m := selectorModelWithCurrent("Pick:", []SelectItem{ + {Name: "qwen3.5:cloud", Recommended: true}, + {Name: "qwen3.5", Recommended: true}, + }, "qwen3.5") + + if m.cursor != 1 { + t.Fatalf("cursor = %d, want 1", m.cursor) + } + + content := m.renderContent() + if !strings.Contains(content, "▸ qwen3.5") { + t.Fatalf("expected local qwen3.5 to be highlighted\n%s", content) + } + if strings.Contains(content, "▸ qwen3.5:cloud") { + t.Fatalf("did not expect cloud qwen3.5:cloud to be highlighted\n%s", content) + } +} + func TestRenderContent_SectionHeaders(t *testing.T) { m := selectorModel{ title: "Pick:", @@ -434,6 +453,28 @@ func TestCursorForCurrent(t *testing.T) { } } +func TestCursorForCurrent_PrefersExactLocalOverCloudPrefix(t *testing.T) { + testItems := []SelectItem{ + {Name: "qwen3.5:cloud", Recommended: true}, + {Name: "qwen3.5", Recommended: true}, + } + + if got := cursorForCurrent(testItems, "qwen3.5"); got != 1 { + t.Errorf("cursorForCurrent(%q) = %d, want %d", "qwen3.5", got, 1) + } +} + +func TestCursorForCurrent_PrefersExactCloudOverLocalPrefix(t *testing.T) { + testItems := []SelectItem{ + {Name: "qwen3.5", Recommended: true}, + {Name: "qwen3.5:cloud", Recommended: true}, + } + + if got := cursorForCurrent(testItems, "qwen3.5:cloud"); got != 1 { + t.Errorf("cursorForCurrent(%q) = %d, want %d", "qwen3.5:cloud", got, 1) + } +} + // --- ReorderItems --- func TestReorderItems(t *testing.T) { @@ -799,6 +840,34 @@ func TestMulti_LastCheckedIsDefault(t *testing.T) { } } +func TestMulti_UncheckingDefaultFallsBackToNearestCheckedAbove(t *testing.T) { + // Default is "b", and checked models are "a", "b", "c". + // Unticking default should make "a" (the nearest checked item above) default. + m := newMultiSelectorModel("Pick:", items("a", "b", "c"), []string{"b", "c", "a"}) + m.multi = true + m.cursor = 1 // "b" + m.toggleItem() + + lastIdx := m.checkOrder[len(m.checkOrder)-1] + if m.items[lastIdx].Name != "a" { + t.Fatalf("expected default to fall back to 'a', got %q", m.items[lastIdx].Name) + } +} + +func TestMulti_UncheckingTopDefaultFallsBackToNearestCheckedBelow(t *testing.T) { + // Default is top item "a". With no checked item above, fallback should pick + // the nearest checked item below ("b"). + m := newMultiSelectorModel("Pick:", items("a", "b", "c"), []string{"a", "c", "b"}) + m.multi = true + m.cursor = 0 // "a" + m.toggleItem() + + lastIdx := m.checkOrder[len(m.checkOrder)-1] + if m.items[lastIdx].Name != "b" { + t.Fatalf("expected default to fall back to 'b', got %q", m.items[lastIdx].Name) + } +} + // Key message helpers for testing type keyType = int