From d64812eb5d573f8058002fcad64dc96311acf09e Mon Sep 17 00:00:00 2001 From: Eva H <63033505+hoyyeva@users.noreply.github.com> Date: Wed, 8 Apr 2026 10:39:18 -0700 Subject: [PATCH] cmd: improve multi-select sorting and selection status (#15200) --- cmd/launch/integrations_test.go | 33 +++++++++++++++++++++++++++++++++ cmd/launch/launch.go | 9 --------- cmd/launch/launch_test.go | 6 +++--- cmd/launch/models.go | 11 +++++++++++ cmd/tui/selector.go | 10 +++++++--- 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/cmd/launch/integrations_test.go b/cmd/launch/integrations_test.go index 2460993dc..85527eeb8 100644 --- a/cmd/launch/integrations_test.go +++ b/cmd/launch/integrations_test.go @@ -374,6 +374,39 @@ func TestBuildModelList_PreCheckedFirst(t *testing.T) { } } +func TestBuildModelList_CurrentDefaultFirstAmongCheckedNonRec(t *testing.T) { + existing := []modelInfo{ + {Name: "alpha", Remote: false}, + {Name: "zebra", Remote: false}, + {Name: "middle", Remote: false}, + } + + // "zebra" is the current/default; all three are checked, none are recommended. + // Expected non-rec order: zebra (default), alpha, middle (alphabetical). + items, _, _, _ := buildModelList(existing, []string{"zebra", "alpha", "middle"}, "zebra") + got := names(items) + + // Skip recommended items to find the non-rec portion. + var nonRec []string + for _, item := range items { + if !item.Recommended { + nonRec = append(nonRec, item.Name) + } + } + if len(nonRec) < 3 { + t.Fatalf("expected 3 non-rec items, got %v", nonRec) + } + if nonRec[0] != "zebra" { + t.Errorf("current/default model should be first among checked non-rec, got %v (full: %v)", nonRec, got) + } + if nonRec[1] != "alpha" { + t.Errorf("remaining checked should be alphabetical, expected alpha second, got %v", nonRec) + } + if nonRec[2] != "middle" { + t.Errorf("remaining checked should be alphabetical, expected middle third, got %v", nonRec) + } +} + func TestBuildModelList_ExistingRecommendedMarked(t *testing.T) { existing := []modelInfo{ {Name: "gemma4", Remote: false}, diff --git a/cmd/launch/launch.go b/cmd/launch/launch.go index a44569ad4..cff61e15b 100644 --- a/cmd/launch/launch.go +++ b/cmd/launch/launch.go @@ -540,15 +540,6 @@ func (c *launcherClient) selectMultiModelsForIntegration(ctx context.Context, ru 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 cc072765c..5179504f3 100644 --- a/cmd/launch/launch_test.go +++ b/cmd/launch/launch_test.go @@ -668,7 +668,7 @@ func TestLaunchIntegration_EditorForceConfigure(t *testing.T) { } } -func TestLaunchIntegration_EditorForceConfigure_DoesNotFloatCheckedModelsInPicker(t *testing.T) { +func TestLaunchIntegration_EditorForceConfigure_FloatsCheckedModelsInPicker(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) withLauncherHooks(t) @@ -725,8 +725,8 @@ func TestLaunchIntegration_EditorForceConfigure_DoesNotFloatCheckedModelsInPicke 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 gotItems[0] != "qwen3.5:cloud" { + t.Fatalf("expected checked models floated to top with qwen3.5:cloud first, got %v", gotItems) } if len(gotPreChecked) < 2 { t.Fatalf("expected prechecked models to be preserved, got %v", gotPreChecked) diff --git a/cmd/launch/models.go b/cmd/launch/models.go index ad1c16b9b..fbdccbb0b 100644 --- a/cmd/launch/models.go +++ b/cmd/launch/models.go @@ -379,6 +379,17 @@ func buildModelList(existing []modelInfo, preChecked []string, current string) ( } return recRank[a.Name] - recRank[b.Name] } + // Among checked non-recommended items - put the default first + if ac && !aRec && current != "" { + aCurrent := a.Name == current + bCurrent := b.Name == current + if aCurrent != bCurrent { + if aCurrent { + return -1 + } + return 1 + } + } if aNew != bNew { if aNew { return 1 diff --git a/cmd/tui/selector.go b/cmd/tui/selector.go index 74f905bf3..b93d1847f 100644 --- a/cmd/tui/selector.go +++ b/cmd/tui/selector.go @@ -817,14 +817,18 @@ func (m multiSelectorModel) View() string { s.WriteString("\n") + count := m.selectedCount() if !m.multi { + if count > 0 { + s.WriteString(sectionHeaderStyle.Render(fmt.Sprintf("%d models selected - press tab to edit", count))) + s.WriteString("\n\n") + } s.WriteString(selectorHelpStyle.Render("↑/↓ navigate • enter select • tab add multiple • ← back")) } else { - count := m.selectedCount() if count == 0 { - s.WriteString(selectorDescStyle.Render(" Select at least one model.")) + s.WriteString(sectionHeaderStyle.Render("Select at least one model.")) } else { - s.WriteString(selectorDescStyle.Render(fmt.Sprintf(" %d selected - press enter to continue", count))) + s.WriteString(sectionHeaderStyle.Render(fmt.Sprintf("%d models selected - press enter to continue", count))) } s.WriteString("\n\n") s.WriteString(selectorHelpStyle.Render("↑/↓ navigate • space toggle • tab select single • enter confirm • ← back"))