feat: improve config provider TUI interaction

cmd/opencodereview/config_cmd.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -370,6 +371,32 @@ func parseModelListValue(value string) ([]string, error) {
 	return normalizeModelList(strings.Split(value, ",")), nil
 }
 
+func activeModelForProvider(cfg *Config, providerName string, entry ProviderEntry) string {
+	if entry.Model != "" {
+		return entry.Model
+	}
+	if cfg != nil && cfg.Provider == providerName && cfg.Model != "" {
+		return cfg.Model
+	}
+	return ""
+}
+
+func sortModelsByName(models []string) []string {
+	if len(models) < 2 {
+		return models
+	}
+	sorted := append([]string(nil), models...)
+	sort.SliceStable(sorted, func(i, j int) bool {
+		left := strings.ToLower(sorted[i])
+		right := strings.ToLower(sorted[j])
+		if left == right {
+			return sorted[i] < sorted[j]
+		}
+		return left < right
+	})
+	return sorted
+}
+
 func normalizeModelList(models []string) []string {
 	out := make([]string, 0, len(models))
 	seen := make(map[string]struct{}, len(models))

cmd/opencodereview/provider_cmd.go

@@ -22,7 +22,7 @@ func runConfigProvider() error {
 		return fmt.Errorf("load config: %w", err)
 	}
 
-	m := newProviderTUI(cfg)
+	m := newProviderTUI(cfg, configPath)
 	p := tea.NewProgram(m)
 	finalModel, err := p.Run()
 	if err != nil {
@@ -31,19 +31,11 @@ func runConfigProvider() error {
 
 	final := finalModel.(providerTUIModel)
 
-	if len(final.deletedProviders) > 0 {
-		clearedActive, err := applyProviderDeletions(configPath, cfg, final.deletedProviders)
-		if err != nil {
-			return err
-		}
-		if clearedActive && !final.confirmed {
-			fmt.Fprintf(os.Stderr, "[ocr] WARNING: active provider was deleted; 'provider' and 'model' have been cleared.\n")
-			fmt.Fprintf(os.Stderr, "[ocr] Run 'ocr config provider' to select a new provider.\n")
-		}
-	}
-
 	if !final.confirmed {
-		if len(final.deletedProviders) > 0 {
+		// TUI persists changes (create/edit/model/add/delete) directly to disk
+		// during the session, so the on-disk file is already up to date for any
+		// savedInSession operation. No additional post-TUI apply step is needed.
+		if final.savedInSession {
 			return nil
 		}
 		fmt.Println("Cancelled.")
@@ -82,6 +74,58 @@ func applyProviderDeletions(configPath string, cfg *Config, names []string) (boo
 	return clearedActive, nil
 }
 
+func applyModelDeletions(configPath string, cfg *Config, deletedModels map[string][]string) error {
+	if len(deletedModels) == 0 {
+		return nil
+	}
+	if cfg.CustomProviders != nil {
+		for name, models := range deletedModels {
+			entry, ok := cfg.CustomProviders[name]
+			if !ok {
+				continue
+			}
+			original := entry.Models
+			entry.Models = removeModels(entry.Models, models)
+			modelsChanged := len(entry.Models) != len(original)
+
+			entryChanged := modelsChanged
+			if modelListContains(models, entry.Model) {
+				entry.Model = ""
+				entryChanged = true
+			}
+			if cfg.Provider == name && modelListContains(models, cfg.Model) {
+				cfg.Model = ""
+			}
+			if entryChanged {
+				cfg.CustomProviders[name] = entry
+				for _, m := range original {
+					if !modelListContains(entry.Models, m) {
+						fmt.Printf("Deleted model %q from custom provider %q.\n", m, name)
+					}
+				}
+			}
+		}
+	}
+	// Always persist when deletions were requested; in-session TUI edits may
+	// have already applied changes to cfg, making a diff-based changed flag unreliable.
+	return saveConfig(configPath, cfg)
+}
+
+func removeModels(existing, toRemove []string) []string {
+	removeSet := make(map[string]struct{}, len(toRemove))
+	for _, m := range toRemove {
+		removeSet[m] = struct{}{}
+	}
+	result := make([]string, 0, len(existing))
+	for _, m := range existing {
+		if _, found := removeSet[m]; found {
+			continue
+		}
+		result = append(result, m)
+	}
+	return result
+}
+
 func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) error {
 	if result.url == "" {
 		return fmt.Errorf("URL is required for manual configuration")
@@ -95,13 +139,21 @@ func applyManualConfig(configPath string, cfg *Config, result providerTUIResult)
 	cfg.Llm.URL = result.url
 	cfg.Llm.Model = result.model
 	cfg.Llm.AuthToken = result.apiKey
+	if result.protocol == "anthropic" {
+		useAnthropic := true
+		cfg.Llm.UseAnthropic = &useAnthropic
+	} else {
+		useAnthropic := false
+		cfg.Llm.UseAnthropic = &useAnthropic
+	}
 
 	if err := saveConfig(configPath, cfg); err != nil {
 		return err
 	}
 
 	fmt.Println("\nManual configuration saved.")
 	fmt.Printf("URL: %s\n", result.url)
+	fmt.Printf("Protocol: %s\n", result.protocol)
 	fmt.Printf("Model: %s\n", result.model)
 
 	fmt.Println("\nTesting connection...")
@@ -145,15 +197,28 @@ func applyCustomProviderConfig(configPath string, cfg *Config, result providerTU
 	}
 	cfg.CustomProviders[result.provider] = entry
 
-	if cfg.Provider != result.provider {
-		cfg.Model = ""
+	if !result.isEdit {
+		cfg.Provider = result.provider
+		cfg.Model = result.model
+	} else if cfg.Provider == result.provider {
+		cfg.Model = result.model
 	}
-	cfg.Provider = result.provider
 
 	if err := saveConfig(configPath, cfg); err != nil {
 		return err
 	}
 
+	if result.isEdit {
+		if cfg.Provider == result.provider {
+			fmt.Printf("\nActive provider %q updated.\n", result.provider)
+		} else {
+			fmt.Printf("\nCustom provider %q updated (not currently active).\n", result.provider)
+		}
+		fmt.Printf("Model: %s\n", result.model)
+		fmt.Println("\nTip: run 'ocr config model' to switch model later.")
+		return nil
+	}
+
 	fmt.Printf("\nProvider set to: %s (custom)\n", result.provider)
 	fmt.Printf("Model: %s\n", result.model)
 
@@ -203,6 +268,7 @@ func applyOfficialProviderConfig(configPath string, cfg *Config, result provider
 		cfg.Model = ""
 	}
 	cfg.Provider = result.provider
+	cfg.Model = result.model
 
 	if err := saveConfig(configPath, cfg); err != nil {
 		return err
@@ -243,7 +309,7 @@ func runConfigModel() error {
 	if preset, isPreset := llm.LookupProvider(cfg.Provider); isPreset {
 		provider = preset
 		if entry, ok := cfg.Providers[cfg.Provider]; ok {
-			currentModel = entry.Model
+			currentModel = activeModelForProvider(cfg, cfg.Provider, entry)
 			provider.Models = mergeModelLists(provider.Models, entry.Models)
 		}
 	} else {
@@ -252,15 +318,12 @@ func runConfigModel() error {
 		if !ok {
 			return fmt.Errorf("provider %q is not configured in custom_providers", cfg.Provider)
 		}
-		currentModel = entry.Model
+		currentModel = activeModelForProvider(cfg, cfg.Provider, entry)
 		provider.DisplayName = cfg.Provider + " (custom)"
 		provider.Protocol = entry.Protocol
 		provider.BaseURL = entry.URL
 		provider.Models = mergeModelLists(entry.Models)
 	}
-	if currentModel == "" {
-		currentModel = cfg.Model
-	}
 
 	m := newModelTUI(provider, currentModel)
 	p := tea.NewProgram(m)
@@ -299,6 +362,7 @@ func runConfigModel() error {
 		}
 		cfg.Providers[cfg.Provider] = entry
 	}
+	cfg.Model = selectedModel
 
 	if err := saveConfig(configPath, cfg); err != nil {
 		return err