cmd: TUI model ordering (#14814)

This commit is contained in:
Parth Sareen
2026-03-13 10:19:22 -07:00
committed by GitHub
parent f676231de9
commit 76925f1284
6 changed files with 296 additions and 11 deletions

View File

@@ -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)

View File

@@ -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 <model>' first")
items, _, err := c.loadSelectableModels(ctx, nil, current, "no models available, run 'ollama pull <model>' 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 {

View File

@@ -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)

View File

@@ -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 })...)
}

View File

@@ -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)

View File

@@ -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