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
20 changes: 17 additions & 3 deletions internal/opencode/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,27 @@ func JDPhases() []string {
}
}

// RRPhases returns the ordered list of 4R review sub-agent names.
// These are workflow-level agents (not SDD phases) used for multi-dimensional
// PR review: risk assessment, readability, reliability, and resilience.
// They support independent model configuration for diversity of perspective.
func RRPhases() []string {
return []string{
"review-risk",
"review-readability",
"review-reliability",
"review-resilience",
}
}

// ConfigurableAgentPhases returns all agent names that support per-agent
// model configuration. This includes SDD phases + JD agents.
// model configuration. This includes SDD phases + JD agents + 4R reviewers.
// Used by the inject model assignment table builder and the configurable agent set
// in ReadCurrentModelAssignments. The TUI model picker uses SDDPhases() and
// JDPhases() separately for row layout control.
// in ReadCurrentModelAssignments. The TUI model picker uses SDDPhases(), JDPhases(),
// and RRPhases() separately for row layout control.
func ConfigurableAgentPhases() []string {
phases := SDDPhases()
phases = append(phases, JDPhases()...)
phases = append(phases, RRPhases()...)
return phases
}
73 changes: 69 additions & 4 deletions internal/tui/screens/model_picker.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,26 @@ const SDDOrchestratorPhase = "gentle-orchestrator"

// ModelPickerRows returns the row labels for the model picker screen.
// Row 0 is "gentle-orchestrator" (coordinator), row 1 is "Set all phases",
// rows 2-11 are the 10 SDD sub-agent phases, then a separator and JD agents.
// rows 2-11 are the 10 SDD sub-agent phases, then a separator and JD agents,
// then another separator and 4R review agents.
func ModelPickerRows() []string {
rows := make([]string, 0, 2+len(opencode.SDDPhases())+1+len(opencode.JDPhases()))
rows := make([]string, 0, 2+len(opencode.SDDPhases())+1+len(opencode.JDPhases())+1+len(opencode.RRPhases()))
rows = append(rows, SDDOrchestratorPhase)
rows = append(rows, "Set all SDD phases")
rows = append(rows, opencode.SDDPhases()...)
if len(opencode.JDPhases()) > 0 {
rows = append(rows, "--- Judgment Day ---")
rows = append(rows, opencode.JDPhases()...)
}
if len(opencode.RRPhases()) > 0 {
rows = append(rows, "--- 4R Review ---")
rows = append(rows, opencode.RRPhases()...)
}
return rows
}

// ModelPickerRowsForProfile returns model picker rows for profile creation.
// JD agents are excluded because they are global (not profile-scoped).
// JD agents and 4R reviewers are excluded because they are global (not profile-scoped).
func ModelPickerRowsForProfile() []string {
rows := make([]string, 0, 2+len(opencode.SDDPhases()))
rows = append(rows, SDDOrchestratorPhase)
Expand All @@ -158,6 +163,23 @@ func SeparatorRowIdx() int {
return 2 + len(opencode.SDDPhases())
}

// RRSeparatorRowIdx returns the index of the "--- 4R Review ---" separator
// row in ModelPickerRows(). Returns -1 if there are no 4R phases (and thus
// no separator). This is used by the TUI to skip the separator during
// cursor navigation and model selection.
func RRSeparatorRowIdx() int {
rr := opencode.RRPhases()
if len(rr) == 0 {
return -1
}
// After orchestrator + "Set all" + SDD phases + (optional JD separator + JD phases)
base := 2 + len(opencode.SDDPhases())
if jd := opencode.JDPhases(); len(jd) > 0 {
base += 1 + len(jd)
}
return base
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// ProviderEntries returns sorted provider entries with display names and model counts.
func ProviderEntries(state ModelPickerState) []ProviderEntry {
entries := make([]ProviderEntry, 0, len(state.AvailableIDs))
Expand Down Expand Up @@ -424,7 +446,9 @@ func compareVersionKeys(left, right []int) int {
func applyAssignmentPreservingMatchingEffort(state ModelPickerState, assignments map[string]model.ModelAssignment, assignment model.ModelAssignment, preserveEffort bool) map[string]model.ModelAssignment {
phases := opencode.SDDPhases()
jdPhases := opencode.JDPhases()
rrPhases := opencode.RRPhases()
separatorIdx := SeparatorRowIdx()
rrSeparatorIdx := RRSeparatorRowIdx()
switch {
case state.SelectedPhaseIdx == 0:
assignments[SDDOrchestratorPhase] = preserveMatchingEffort(assignments[SDDOrchestratorPhase], assignment, preserveEffort)
Expand All @@ -434,6 +458,14 @@ func applyAssignmentPreservingMatchingEffort(state ModelPickerState, assignments
}
case state.SelectedPhaseIdx == separatorIdx:
// Separator row ("--- Judgment Day ---") — no action, skip.
case state.SelectedPhaseIdx == rrSeparatorIdx:
// 4R separator row ("--- 4R Review ---") — no action, skip.
case state.SelectedPhaseIdx > rrSeparatorIdx:
// 4R agent rows: map to RRPhases() after separator.
rrIdx := state.SelectedPhaseIdx - rrSeparatorIdx - 1
if rrIdx < len(rrPhases) {
assignments[rrPhases[rrIdx]] = preserveMatchingEffort(assignments[rrPhases[rrIdx]], assignment, preserveEffort)
}
case state.SelectedPhaseIdx > separatorIdx:
// JD agent rows: map to JDPhases() after separator.
jdIdx := state.SelectedPhaseIdx - separatorIdx - 1
Expand Down Expand Up @@ -473,7 +505,9 @@ func formatAssignmentLabel(row, provName, modelName, effort string) string {
func applyAssignment(state ModelPickerState, assignments map[string]model.ModelAssignment, assignment model.ModelAssignment) map[string]model.ModelAssignment {
phases := opencode.SDDPhases()
jdPhases := opencode.JDPhases()
rrPhases := opencode.RRPhases()
separatorIdx := SeparatorRowIdx()
rrSeparatorIdx := RRSeparatorRowIdx()
switch {
case state.SelectedPhaseIdx == 0:
assignments[SDDOrchestratorPhase] = assignment
Expand All @@ -483,6 +517,14 @@ func applyAssignment(state ModelPickerState, assignments map[string]model.ModelA
}
case state.SelectedPhaseIdx == separatorIdx:
// Separator row ("--- Judgment Day ---") — no action, skip.
case state.SelectedPhaseIdx == rrSeparatorIdx:
// 4R separator row ("--- 4R Review ---") — no action, skip.
case state.SelectedPhaseIdx > rrSeparatorIdx:
// 4R agent rows: map to RRPhases() after separator.
rrIdx := state.SelectedPhaseIdx - rrSeparatorIdx - 1
if rrIdx < len(rrPhases) {
assignments[rrPhases[rrIdx]] = assignment
}
case state.SelectedPhaseIdx > separatorIdx:
// JD agent rows: map to JDPhases() after separator.
jdIdx := state.SelectedPhaseIdx - separatorIdx - 1
Expand Down Expand Up @@ -633,7 +675,7 @@ func renderPhaseList(
) string {
var b strings.Builder

title := "Assign Models to SDD Phases & JD Agents"
title := "Assign Models to SDD Phases, JD Agents & 4R Reviewers"
if state.ForProfile {
title = "Assign Models to SDD Phases"
}
Expand Down Expand Up @@ -668,7 +710,9 @@ func renderPhaseList(
}
phases := opencode.SDDPhases()
jdPhases := opencode.JDPhases()
rrPhases := opencode.RRPhases()
separatorIdx := SeparatorRowIdx()
rrSeparatorIdx := RRSeparatorRowIdx()

for idx, row := range rows {
focused := idx == cursor
Expand Down Expand Up @@ -702,6 +746,27 @@ func renderPhaseList(
b.WriteString(styles.SubtextStyle.Render(" " + row) + "\n")
}
continue
case idx == rrSeparatorIdx:
// 4R separator row — render as a visual divider with subtle indicator when focused.
if focused {
b.WriteString(styles.SubtextStyle.Render("▸ " + row) + "\n")
} else {
b.WriteString(styles.SubtextStyle.Render(" " + row) + "\n")
}
continue
case idx > rrSeparatorIdx:
// 4R agent rows
rrIdx := idx - rrSeparatorIdx - 1
if rrIdx < len(rrPhases) {
phase := rrPhases[rrIdx]
assignment, ok := assignments[phase]
if ok && assignment.ProviderID != "" {
provName, modelName := resolveNames(assignment, state)
label = fmt.Sprintf("%-20s %s / %s", row, provName, modelName)
} else {
label = fmt.Sprintf("%-20s (default)", row)
}
}
case idx > separatorIdx:
// JD agent rows
jdIdx := idx - separatorIdx - 1
Expand Down
134 changes: 132 additions & 2 deletions internal/tui/screens/model_picker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ func makeTestState(phaseIdx int) *ModelPickerState {

func TestModelPickerRows_Count(t *testing.T) {
rows := ModelPickerRows()
// 1 orchestrator + 1 "Set all" + 10 sub-agents + 1 separator + 3 JD agents = 16
want := 16
// 1 orchestrator + 1 "Set all" + 10 sub-agents + 1 JD separator + 3 JD agents + 1 4R separator + 4 4R agents = 21
want := 21
if len(rows) != want {
t.Fatalf("ModelPickerRows() len = %d, want %d; rows = %v", len(rows), want, rows)
}
Expand Down Expand Up @@ -1243,6 +1243,119 @@ func TestHandleModelNav_JDLastRow(t *testing.T) {
}
}

// ─── handleModelNav: 4R review agent rows ─────────────────────────────────

func TestHandleModelNav_RRAgentRows_AssignCorrectly(t *testing.T) {
rrPhases := opencode.RRPhases()
rrSepIdx := RRSeparatorRowIdx()

for i, expectedPhase := range rrPhases {
t.Run(expectedPhase, func(t *testing.T) {
state := makeTestState(rrSepIdx + 1 + i) // 4R rows start after 4R separator
assignments := make(map[string]model.ModelAssignment)

handled, updated := handleModelNav("enter", state, assignments)

if !handled {
t.Fatal("handleModelNav should return handled=true on enter")
}

// The target 4R phase must be assigned.
a, ok := updated[expectedPhase]
if !ok || a.ProviderID == "" {
t.Errorf("4R phase %q should be assigned; assignments: %v", expectedPhase, updated)
}
if a.ProviderID != "test-provider" {
t.Errorf("4R phase %q ProviderID = %q, want %q", expectedPhase, a.ProviderID, "test-provider")
}
if a.ModelID != "model-alpha" {
t.Errorf("4R phase %q ModelID = %q, want %q", expectedPhase, a.ModelID, "model-alpha")
}

// No other 4R phase must be assigned.
for _, other := range rrPhases {
if other == expectedPhase {
continue
}
if _, exists := updated[other]; exists {
t.Errorf("unrelated 4R phase %q should not be assigned; assignments: %v", other, updated)
}
}

// No SDD phase, JD agent, or orchestrator must be assigned.
for _, sdd := range opencode.SDDPhases() {
if _, exists := updated[sdd]; exists {
t.Errorf("SDD phase %q should not be assigned by 4R row; assignments: %v", sdd, updated)
}
}
for _, jd := range opencode.JDPhases() {
if _, exists := updated[jd]; exists {
t.Errorf("JD agent %q should not be assigned by 4R row; assignments: %v", jd, updated)
}
}
if _, exists := updated[SDDOrchestratorPhase]; exists {
t.Errorf("orchestrator should not be assigned by 4R row; assignments: %v", updated)
}
})
}
}

func TestHandleModelNav_RRFirstRow(t *testing.T) {
// Verify the FIRST 4R row (right after 4R separator) maps to review-risk.
rrPhases := opencode.RRPhases()
if len(rrPhases) == 0 {
t.Skip("no 4R phases defined")
}
rrSepIdx := RRSeparatorRowIdx()
state := makeTestState(rrSepIdx + 1)
assignments := make(map[string]model.ModelAssignment)

_, updated := handleModelNav("enter", state, assignments)

if _, ok := updated[rrPhases[0]]; !ok {
t.Fatalf("first 4R row should assign %q; got: %v", rrPhases[0], updated)
}
}

func TestHandleModelNav_RRLastRow(t *testing.T) {
// Verify the LAST 4R row maps to the last 4R phase.
rrPhases := opencode.RRPhases()
if len(rrPhases) == 0 {
t.Skip("no 4R phases defined")
}
rrSepIdx := RRSeparatorRowIdx()
state := makeTestState(rrSepIdx + len(rrPhases))
assignments := make(map[string]model.ModelAssignment)

_, updated := handleModelNav("enter", state, assignments)

lastPhase := rrPhases[len(rrPhases)-1]
if _, ok := updated[lastPhase]; !ok {
t.Fatalf("last 4R row should assign %q; got: %v", lastPhase, updated)
}
}

func TestHandleModelNav_RRSeparatorRow_NoAssignment(t *testing.T) {
// Verify that selecting the 4R separator row does not assign any phase.
rrSepIdx := RRSeparatorRowIdx()
if rrSepIdx < 0 {
t.Skip("no 4R separator defined")
}
state := makeTestState(rrSepIdx)
assignments := make(map[string]model.ModelAssignment)

handled, updated := handleModelNav("enter", state, assignments)

if !handled {
t.Fatal("handleModelNav should return handled=true on separator enter")
}

// No phase should be assigned when separator is selected.
if len(updated) != 0 {
t.Errorf("separator row should not assign any phase; got: %v", updated)
}
}

// ─── ModelPickerRowsForProfile ──────────────────────────────────────────

func TestModelPickerRowsForProfile_Count(t *testing.T) {
Expand Down Expand Up @@ -1274,3 +1387,20 @@ func TestModelPickerRowsForProfile_NoJDAgents(t *testing.T) {
}
}
}

func TestModelPickerRowsForProfile_NoRRAgents(t *testing.T) {
rows := ModelPickerRowsForProfile()
rrPhases := opencode.RRPhases()
for _, rr := range rrPhases {
for _, row := range rows {
if row == rr {
t.Fatalf("ModelPickerRowsForProfile() should not contain 4R agent %q; got: %v", rr, rows)
}
}
}
for _, row := range rows {
if row == "--- 4R Review ---" {
t.Fatalf("ModelPickerRowsForProfile() should not contain 4R separator; got: %v", rows)
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.