starred/ollama
1
0
Fork 0
mirror of https://github.com/ollama/ollama.git synced 2026-04-17 22:54:05 +02:00
Code Issues Packages Projects Releases Wiki Activity

cmd: TUI UX improvements (#14198)

Browse Source
This commit is contained in:
Parth Sareen
2026-02-11 10:18:41 -08:00
committed by GitHub
parent 2dbb000908
commit f08427c138
13 changed files with 2103 additions and 1931 deletions
Download Patch File Download Diff File Expand all files Collapse all files

444
cmd/config/integrations_test.go
View File

@@ -94,9 +94,7 @@ func TestLaunchCmd(t *testing.T) {
mockCheck := func(cmd *cobra.Command, args []string) error {
return nil
}
// Mock TUI function (not called in these tests)
mockTUI := func(cmd *cobra.Command) {}
cmd := LaunchCmd(mockCheck, mockTUI)
t.Run("command structure", func(t *testing.T) {
@@ -396,7 +394,7 @@ func names(items []ModelItem) []string {
func TestBuildModelList_NoExistingModels(t *testing.T) {
items, _, _, _ := buildModelList(nil, nil, "")
want := []string{"glm-4.7-flash", "qwen3:8b", "glm-4.7:cloud", "kimi-k2.5:cloud"}
want := []string{"kimi-k2.5:cloud", "glm-4.7:cloud", "glm-4.7-flash", "qwen3:8b"}
if diff := cmp.Diff(want, names(items)); diff != "" {
t.Errorf("with no existing models, items should be recommended in order (-want +got):\n%s", diff)
}
@@ -417,9 +415,10 @@ func TestBuildModelList_OnlyLocalModels_CloudRecsAtBottom(t *testing.T) {
items, _, _, _ := buildModelList(existing, nil, "")
got := names(items)
want := []string{"llama3.2", "qwen2.5", "glm-4.7-flash", "glm-4.7:cloud", "kimi-k2.5:cloud", "qwen3:8b"}
// Recommended pinned at top (local recs first, then cloud recs when only-local), then installed non-recs
want := []string{"glm-4.7-flash", "qwen3:8b", "kimi-k2.5:cloud", "glm-4.7:cloud", "llama3.2", "qwen2.5"}
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("cloud recs should be at bottom (-want +got):\n%s", diff)
t.Errorf("recs pinned at top, local recs before cloud recs (-want +got):\n%s", diff)
}
}
@@ -432,9 +431,10 @@ func TestBuildModelList_BothCloudAndLocal_RegularSort(t *testing.T) {
items, _, _, _ := buildModelList(existing, nil, "")
got := names(items)
want := []string{"glm-4.7:cloud", "llama3.2", "glm-4.7-flash", "kimi-k2.5:cloud", "qwen3:8b"}
// All recs pinned at top (cloud before local in mixed case), then non-recs
want := []string{"kimi-k2.5:cloud", "glm-4.7:cloud", "glm-4.7-flash", "qwen3:8b", "llama3.2"}
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mixed models should be alphabetical (-want +got):\n%s", diff)
t.Errorf("recs pinned at top, cloud recs first in mixed case (-want +got):\n%s", diff)
}
}
@@ -485,9 +485,10 @@ func TestBuildModelList_ExistingCloudModelsNotPushedToBottom(t *testing.T) {
// glm-4.7-flash and glm-4.7:cloud are installed so they sort normally;
// kimi-k2.5:cloud and qwen3:8b are not installed so they go to the bottom
want := []string{"glm-4.7-flash", "glm-4.7:cloud", "kimi-k2.5:cloud", "qwen3:8b"}
// All recs: cloud first in mixed case, then local, in rec order within each
want := []string{"kimi-k2.5:cloud", "glm-4.7:cloud", "glm-4.7-flash", "qwen3:8b"}
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("existing cloud models should sort normally (-want +got):\n%s", diff)
t.Errorf("all recs, cloud first in mixed case (-want +got):\n%s", diff)
}
}
@@ -502,9 +503,10 @@ func TestBuildModelList_HasRecommendedCloudModel_OnlyNonInstalledAtBottom(t *tes
// kimi-k2.5:cloud is installed so it sorts normally;
// the rest of the recommendations are not installed so they go to the bottom
want := []string{"kimi-k2.5:cloud", "llama3.2", "glm-4.7-flash", "glm-4.7:cloud", "qwen3:8b"}
// All recs pinned at top (cloud first in mixed case), then non-recs
want := []string{"kimi-k2.5:cloud", "glm-4.7:cloud", "glm-4.7-flash", "qwen3:8b", "llama3.2"}
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("only non-installed models should be at bottom (-want +got):\n%s", diff)
t.Errorf("recs pinned at top, cloud first in mixed case (-want +got):\n%s", diff)
}
for _, item := range items {
@@ -578,6 +580,101 @@ func TestBuildModelList_ReturnsExistingAndCloudMaps(t *testing.T) {
}
}
func TestBuildModelList_RecommendedFieldSet(t *testing.T) {
existing := []modelInfo{
{Name: "glm-4.7-flash", Remote: false},
{Name: "llama3.2:latest", Remote: false},
}
items, _, _, _ := buildModelList(existing, nil, "")
for _, item := range items {
switch item.Name {
case "glm-4.7-flash", "qwen3:8b", "glm-4.7:cloud", "kimi-k2.5:cloud":
if !item.Recommended {
t.Errorf("%q should have Recommended=true", item.Name)
}
case "llama3.2":
if item.Recommended {
t.Errorf("%q should have Recommended=false", item.Name)
}
}
}
}
func TestBuildModelList_MixedCase_CloudRecsFirst(t *testing.T) {
existing := []modelInfo{
{Name: "llama3.2:latest", Remote: false},
{Name: "glm-4.7:cloud", Remote: true},
}
items, _, _, _ := buildModelList(existing, nil, "")
got := names(items)
// Cloud recs should sort before local recs in mixed case
cloudIdx := slices.Index(got, "glm-4.7:cloud")
localIdx := slices.Index(got, "glm-4.7-flash")
if cloudIdx > localIdx {
t.Errorf("cloud recs should be before local recs in mixed case, got %v", got)
}
}
func TestBuildModelList_OnlyLocal_LocalRecsFirst(t *testing.T) {
existing := []modelInfo{
{Name: "llama3.2:latest", Remote: false},
}
items, _, _, _ := buildModelList(existing, nil, "")
got := names(items)
// Local recs should sort before cloud recs in only-local case
localIdx := slices.Index(got, "glm-4.7-flash")
cloudIdx := slices.Index(got, "glm-4.7:cloud")
if localIdx > cloudIdx {
t.Errorf("local recs should be before cloud recs in only-local case, got %v", got)
}
}
func TestBuildModelList_RecsAboveNonRecs(t *testing.T) {
existing := []modelInfo{
{Name: "llama3.2:latest", Remote: false},
{Name: "custom-model", Remote: false},
}
items, _, _, _ := buildModelList(existing, nil, "")
got := names(items)
// All recommended models should appear before non-recommended installed models
lastRecIdx := -1
firstNonRecIdx := len(got)
for i, name := range got {
isRec := name == "glm-4.7-flash" || name == "qwen3:8b" || name == "glm-4.7:cloud" || name == "kimi-k2.5:cloud"
if isRec && i > lastRecIdx {
lastRecIdx = i
}
if !isRec && i < firstNonRecIdx {
firstNonRecIdx = i
}
}
if lastRecIdx > firstNonRecIdx {
t.Errorf("all recs should be above non-recs, got %v", got)
}
}
func TestBuildModelList_CheckedBeforeRecs(t *testing.T) {
existing := []modelInfo{
{Name: "llama3.2:latest", Remote: false},
{Name: "glm-4.7:cloud", Remote: true},
}
items, _, _, _ := buildModelList(existing, []string{"llama3.2"}, "")
got := names(items)
if got[0] != "llama3.2" {
t.Errorf("checked model should be first even before recs, got %v", got)
}
}
func TestEditorIntegration_SavedConfigSkipsSelection(t *testing.T) {
tmpDir := t.TempDir()
setTestHome(t, tmpDir)
@@ -630,7 +727,7 @@ func TestShowOrPull_ModelExists(t *testing.T) {
u, _ := url.Parse(srv.URL)
client := api.NewClient(u, srv.Client())
err := showOrPull(context.Background(), client, "test-model")
err := ShowOrPull(context.Background(), client, "test-model")
if err != nil {
t.Errorf("showOrPull should return nil when model exists, got: %v", err)
}
@@ -647,7 +744,7 @@ func TestShowOrPull_ModelNotFound_NoTerminal(t *testing.T) {
client := api.NewClient(u, srv.Client())
// confirmPrompt will fail in test (no terminal), so showOrPull should return an error
err := showOrPull(context.Background(), client, "missing-model")
err := ShowOrPull(context.Background(), client, "missing-model")
if err == nil {
t.Error("showOrPull should return error when model not found and no terminal available")
}
@@ -672,12 +769,104 @@ func TestShowOrPull_ShowCalledWithCorrectModel(t *testing.T) {
u, _ := url.Parse(srv.URL)
client := api.NewClient(u, srv.Client())
_ = showOrPull(context.Background(), client, "qwen3:8b")
_ = ShowOrPull(context.Background(), client, "qwen3:8b")
if receivedModel != "qwen3:8b" {
t.Errorf("expected Show to be called with %q, got %q", "qwen3:8b", receivedModel)
}
}
func TestShowOrPull_ModelNotFound_ConfirmYes_Pulls(t *testing.T) {
// Set up hook so confirmPrompt doesn't need a terminal
oldHook := DefaultConfirmPrompt
DefaultConfirmPrompt = func(prompt string) (bool, error) {
if !strings.Contains(prompt, "missing-model") {
t.Errorf("expected prompt to contain model name, got %q", prompt)
}
return true, nil
}
defer func() { DefaultConfirmPrompt = oldHook }()
var pullCalled bool
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/show":
w.WriteHeader(http.StatusNotFound)
fmt.Fprintf(w, `{"error":"model not found"}`)
case "/api/pull":
pullCalled = true
w.WriteHeader(http.StatusOK)
fmt.Fprintf(w, `{"status":"success"}`)
default:
w.WriteHeader(http.StatusNotFound)
}
}))
defer srv.Close()
u, _ := url.Parse(srv.URL)
client := api.NewClient(u, srv.Client())
err := ShowOrPull(context.Background(), client, "missing-model")
if err != nil {
t.Errorf("ShowOrPull should succeed after pull, got: %v", err)
}
if !pullCalled {
t.Error("expected pull to be called when user confirms download")
}
}
func TestShowOrPull_ModelNotFound_ConfirmNo_Cancelled(t *testing.T) {
oldHook := DefaultConfirmPrompt
DefaultConfirmPrompt = func(prompt string) (bool, error) {
return false, ErrCancelled
}
defer func() { DefaultConfirmPrompt = oldHook }()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/show":
w.WriteHeader(http.StatusNotFound)
fmt.Fprintf(w, `{"error":"model not found"}`)
case "/api/pull":
t.Error("pull should not be called when user declines")
default:
w.WriteHeader(http.StatusNotFound)
}
}))
defer srv.Close()
u, _ := url.Parse(srv.URL)
client := api.NewClient(u, srv.Client())
err := ShowOrPull(context.Background(), client, "missing-model")
if err == nil {
t.Error("ShowOrPull should return error when user declines")
}
}
func TestConfirmPrompt_DelegatesToHook(t *testing.T) {
oldHook := DefaultConfirmPrompt
var hookCalled bool
DefaultConfirmPrompt = func(prompt string) (bool, error) {
hookCalled = true
if prompt != "test prompt?" {
t.Errorf("expected prompt %q, got %q", "test prompt?", prompt)
}
return true, nil
}
defer func() { DefaultConfirmPrompt = oldHook }()
ok, err := confirmPrompt("test prompt?")
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if !ok {
t.Error("expected true from hook")
}
if !hookCalled {
t.Error("expected DefaultConfirmPrompt hook to be called")
}
}
func TestEnsureAuth_NoCloudModels(t *testing.T) {
// ensureAuth should be a no-op when no cloud models are selected
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -748,3 +937,230 @@ func TestEnsureAuth_SkipsWhenNoCloudSelected(t *testing.T) {
t.Error("whoami should not be called when no cloud models are selected")
}
}
func TestHyperlink(t *testing.T) {
tests := []struct {
name string
url string
text string
wantURL string
wantText string
}{
{
name: "basic link",
url: "https://example.com",
text: "click here",
wantURL: "https://example.com",
wantText: "click here",
},
{
name: "url with path",
url: "https://example.com/docs/install",
text: "install docs",
wantURL: "https://example.com/docs/install",
wantText: "install docs",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := hyperlink(tt.url, tt.text)
// Should contain OSC 8 escape sequences
if !strings.Contains(got, "\033]8;;") {
t.Error("should contain OSC 8 open sequence")
}
if !strings.Contains(got, tt.wantURL) {
t.Errorf("should contain URL %q", tt.wantURL)
}
if !strings.Contains(got, tt.wantText) {
t.Errorf("should contain text %q", tt.wantText)
}
// Should have closing OSC 8 sequence
wantSuffix := "\033]8;;\033\\"
if !strings.HasSuffix(got, wantSuffix) {
t.Error("should end with OSC 8 close sequence")
}
})
}
}
func TestIntegrationInstallHint(t *testing.T) {
tests := []struct {
name string
input string
wantEmpty bool
wantURL string
}{
{
name: "claude has hint",
input: "claude",
wantURL: "https://code.claude.com/docs/en/quickstart",
},
{
name: "codex has hint",
input: "codex",
wantURL: "https://developers.openai.com/codex/cli/",
},
{
name: "openclaw has hint",
input: "openclaw",
wantURL: "https://docs.openclaw.ai",
},
{
name: "unknown has no hint",
input: "unknown",
wantEmpty: true,
},
{
name: "empty name has no hint",
input: "",
wantEmpty: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IntegrationInstallHint(tt.input)
if tt.wantEmpty {
if got != "" {
t.Errorf("expected empty hint, got %q", got)
}
return
}
if !strings.Contains(got, "Install from") {
t.Errorf("hint should start with 'Install from', got %q", got)
}
if !strings.Contains(got, tt.wantURL) {
t.Errorf("hint should contain URL %q, got %q", tt.wantURL, got)
}
// Should be a clickable hyperlink
if !strings.Contains(got, "\033]8;;") {
t.Error("hint URL should be wrapped in OSC 8 hyperlink")
}
})
}
}
func TestListIntegrationInfos(t *testing.T) {
infos := ListIntegrationInfos()
t.Run("excludes aliases", func(t *testing.T) {
for _, info := range infos {
if integrationAliases[info.Name] {
t.Errorf("alias %q should not appear in ListIntegrationInfos", info.Name)
}
}
})
t.Run("sorted by name", func(t *testing.T) {
for i := 1; i < len(infos); i++ {
if infos[i-1].Name >= infos[i].Name {
t.Errorf("not sorted: %q >= %q", infos[i-1].Name, infos[i].Name)
}
}
})
t.Run("all fields populated", func(t *testing.T) {
for _, info := range infos {
if info.Name == "" {
t.Error("Name should not be empty")
}
if info.DisplayName == "" {
t.Errorf("DisplayName for %q should not be empty", info.Name)
}
}
})
t.Run("includes known integrations", func(t *testing.T) {
known := map[string]bool{"claude": false, "codex": false, "opencode": false}
for _, info := range infos {
if _, ok := known[info.Name]; ok {
known[info.Name] = true
}
}
for name, found := range known {
if !found {
t.Errorf("expected %q in ListIntegrationInfos", name)
}
}
})
}
func TestBuildModelList_Descriptions(t *testing.T) {
t.Run("installed recommended has base description", func(t *testing.T) {
existing := []modelInfo{
{Name: "qwen3:8b", Remote: false},
}
items, _, _, _ := buildModelList(existing, nil, "")
for _, item := range items {
if item.Name == "qwen3:8b" {
if strings.HasSuffix(item.Description, "install?") {
t.Errorf("installed model should not have 'install?' suffix, got %q", item.Description)
}
if item.Description == "" {
t.Error("installed recommended model should have a description")
}
return
}
}
t.Error("qwen3:8b not found in items")
})
t.Run("not-installed local rec has VRAM in description", func(t *testing.T) {
items, _, _, _ := buildModelList(nil, nil, "")
for _, item := range items {
if item.Name == "qwen3:8b" {
if !strings.Contains(item.Description, "~11GB") {
t.Errorf("not-installed qwen3:8b should show VRAM hint, got %q", item.Description)
}
return
}
}
t.Error("qwen3:8b not found in items")
})
t.Run("installed local rec omits VRAM", func(t *testing.T) {
existing := []modelInfo{
{Name: "qwen3:8b", Remote: false},
}
items, _, _, _ := buildModelList(existing, nil, "")
for _, item := range items {
if item.Name == "qwen3:8b" {
if strings.Contains(item.Description, "~11GB") {
t.Errorf("installed qwen3:8b should not show VRAM hint, got %q", item.Description)
}
return
}
}
t.Error("qwen3:8b not found in items")
})
}
func TestLaunchIntegration_UnknownIntegration(t *testing.T) {
err := LaunchIntegration("nonexistent-integration")
if err == nil {
t.Fatal("expected error for unknown integration")
}
if !strings.Contains(err.Error(), "unknown integration") {
t.Errorf("error should mention 'unknown integration', got: %v", err)
}
}
func TestLaunchIntegration_NotConfigured(t *testing.T) {
tmpDir := t.TempDir()
setTestHome(t, tmpDir)
// Claude is a known integration but not configured in temp dir
err := LaunchIntegration("claude")
if err == nil {
t.Fatal("expected error when integration is not configured")
}
if !strings.Contains(err.Error(), "not configured") {
t.Errorf("error should mention 'not configured', got: %v", err)
}
}