Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 33 additions & 12 deletions internal/tui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,29 @@ func sanitizeKnownModelEffort(assignment model.ModelAssignment, sddModels map[st
return assignment
}

func (m Model) hasProfileAssignmentContext() bool {
return m.ProfileEditMode ||
m.ProfileDraft.Name != "" ||
m.ProfileDraft.OrchestratorModel.ProviderID != "" ||
m.ProfileDraft.OrchestratorModel.ModelID != "" ||
len(m.ProfileDraft.PhaseAssignments) > 0
}

func (m Model) withBaseOpenCodeModelAssignments() Model {
m.Selection.ModelAssignments = nil
settingsPath := opencode.DefaultSettingsPath()
if current, err := readCurrentAssignmentsFn(settingsPath); err == nil && len(current) > 0 {
m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
}
return m
}
Comment on lines +169 to +176

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | πŸ”΅ Trivial | ⚑ Quick win

Silent failure on opencode.json read error.

When readCurrentAssignmentsFn returns an error, the assignments are silently dropped with no indication to the user. Elsewhere in the same subsystem, NewModelPickerState surfaces a ConfigWarning for the equivalent read/parse failure of opencode.json. Since this helper feeds directly into a user-initiated action ("Configure OpenCode models"), a silent empty picker on read failure could look identical to a config with no assignments, hiding the real cause.

♻️ Suggested fix
 func (m Model) withBaseOpenCodeModelAssignments() Model {
 	m.Selection.ModelAssignments = nil
 	settingsPath := opencode.DefaultSettingsPath()
-	if current, err := readCurrentAssignmentsFn(settingsPath); err == nil && len(current) > 0 {
-		m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
+	current, err := readCurrentAssignmentsFn(settingsPath)
+	if err != nil {
+		m.ModelPicker.ConfigWarning = fmt.Sprintf("Could not load current OpenCode model assignments: %v", err)
+	} else if len(current) > 0 {
+		m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
 	}
 	return m
 }
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (m Model) withBaseOpenCodeModelAssignments() Model {
m.Selection.ModelAssignments = nil
settingsPath := opencode.DefaultSettingsPath()
if current, err := readCurrentAssignmentsFn(settingsPath); err == nil && len(current) > 0 {
m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
}
return m
}
func (m Model) withBaseOpenCodeModelAssignments() Model {
m.Selection.ModelAssignments = nil
settingsPath := opencode.DefaultSettingsPath()
current, err := readCurrentAssignmentsFn(settingsPath)
if err != nil {
m.ModelPicker.ConfigWarning = fmt.Sprintf("Could not load current OpenCode model assignments: %v", err)
} else if len(current) > 0 {
m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
}
return m
}
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/model.go` around lines 169 - 176, Update
withBaseOpenCodeModelAssignments to handle errors from readCurrentAssignmentsFn
by surfacing the same ConfigWarning behavior used by NewModelPickerState for
opencode.json read/parse failures, while preserving the existing assignment
sanitization for successful non-empty reads.


func (m Model) withoutProfileAssignmentContext() Model {
m.ProfileEditMode = false
m.ProfileDraft = model.Profile{}
return m
}

// codexPhaseModelsFromCustomAssignments converts the TUI's CustomAssignments map
// (phase β†’ CodexCustomAssignment) to the state-layer map (phase β†’ model id string)
// used by Selection.CodexPhaseModelAssignments and state.InstallState.
Expand Down Expand Up @@ -1864,18 +1887,16 @@ func (m Model) confirmSelection() (tea.Model, tea.Cmd) {
} else {
m.ModelPicker = screens.ModelPickerState{}
}
// Pre-populate with existing assignments from opencode.json.
// Only when there are no in-session assignments yet β€” the nil guard
// ensures we don't overwrite changes the user already made this session.
if m.Selection.ModelAssignments == nil {
settingsPath := opencode.DefaultSettingsPath()
if current, err := readCurrentAssignmentsFn(settingsPath); err == nil && len(current) > 0 {
// Sanitize loaded assignments: clear any stale effort values for
// models that no longer report variants (e.g. provider refreshed
// their catalog since the user last synced). Without this, a stale
// effort would be preserved in the picker and re-injected on the
// next sync even if the model no longer supports that effort level.
m.Selection.ModelAssignments = sanitizeKnownModelEfforts(current, m.ModelPicker.SDDModels)
// Pre-populate with base OpenCode assignments from opencode.json when
// entering the standalone flow fresh, or after profile editing. Profile
// editing also uses Selection.ModelAssignments, so preserving that map here
// can leak a custom profile into the base/default OpenCode config screen.
// Non-profile in-session model config edits are still preserved.
profileAssignmentContext := m.hasProfileAssignmentContext()
if m.Selection.ModelAssignments == nil || profileAssignmentContext {
m = m.withBaseOpenCodeModelAssignments()
if profileAssignmentContext {
m = m.withoutProfileAssignmentContext()
}
}
m.setScreen(ScreenModelPicker)
Expand Down
49 changes: 49 additions & 0 deletions internal/tui/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,55 @@ func TestModelConfig_OpenCodePickerNavigation(t *testing.T) {
}
}

func TestModelConfigOpenCodeReloadsBaseAssignmentsAfterProfileEdit(t *testing.T) {
originalReadCurrentAssignmentsFn := readCurrentAssignmentsFn
t.Cleanup(func() { readCurrentAssignmentsFn = originalReadCurrentAssignmentsFn })

readCurrentAssignmentsFn = func(settingsPath string) (map[string]model.ModelAssignment, error) {
return map[string]model.ModelAssignment{
screens.SDDOrchestratorPhase: {ProviderID: "anthropic", ModelID: "claude-sonnet-4"},
"sdd-apply": {ProviderID: "openai", ModelID: "gpt-5"},
}, nil
}

m := NewModel(system.DetectionResult{}, "dev")
m.Screen = ScreenModelConfig
m.Cursor = 1
m.ProfileEditMode = true
m.ProfileDraft = model.Profile{
Name: "work",
OrchestratorModel: model.ModelAssignment{ProviderID: "profile-provider", ModelID: "profile-orchestrator"},
PhaseAssignments: map[string]model.ModelAssignment{
"sdd-apply": {ProviderID: "profile-provider", ModelID: "profile-apply"},
},
}
m.Selection.ModelAssignments = map[string]model.ModelAssignment{
screens.SDDOrchestratorPhase: {ProviderID: "profile-provider", ModelID: "profile-orchestrator"},
"sdd-apply": {ProviderID: "profile-provider", ModelID: "profile-apply"},
}

updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter})
state := updated.(Model)

if state.ProfileEditMode {
t.Fatal("profile edit mode was not cleared")
}
if !reflect.DeepEqual(state.ProfileDraft, model.Profile{}) {
t.Fatalf("profile draft was not cleared: %+v", state.ProfileDraft)
}
if got := state.Selection.ModelAssignments[screens.SDDOrchestratorPhase].ProviderID; got != "anthropic" {
t.Fatalf("orchestrator provider = %q, want base OpenCode assignment", got)
}
if got := state.Selection.ModelAssignments["sdd-apply"].ModelID; got != "gpt-5" {
t.Fatalf("sdd-apply model = %q, want base OpenCode assignment", got)
}
for phase, assignment := range state.Selection.ModelAssignments {
if strings.HasPrefix(assignment.ProviderID, "profile-") || strings.HasPrefix(assignment.ModelID, "profile-") {
t.Fatalf("phase %q kept profile assignment: %+v", phase, assignment)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// TestModelConfig_BackNavigation verifies that selecting cursor 4 (Back) from
// ScreenModelConfig returns to ScreenWelcome.
// Index 3 is now "Configure Codex models"; Back moved to index 4.
Expand Down