Skip to content

feat(tui): add 4R review agents to model picker#936

Open
decode2 wants to merge 2 commits into
Gentleman-Programming:mainfrom
decode2:feat/4r-model-picker-tui
Open

feat(tui): add 4R review agents to model picker#936
decode2 wants to merge 2 commits into
Gentleman-Programming:mainfrom
decode2:feat/4r-model-picker-tui

Conversation

@decode2

@decode2 decode2 commented Jun 19, 2026

Copy link
Copy Markdown

Summary

Adds 4R review agents (risk, readability, reliability, resilience) to the TUI model picker, allowing users to configure independent models for each reviewer.

Changes

Core Implementation

  • internal/opencode/models.go: Added RRPhases() function returning the 4 review agent names, updated ConfigurableAgentPhases() to include them
  • internal/tui/screens/model_picker.go:
    • Added RRSeparatorRowIdx() for 4R separator navigation
    • Updated ModelPickerRows() to include 4R section after JD agents
    • Updated �pplyAssignment() and �pplyAssignmentPreservingMatchingEffort() to handle 4R rows
    • Updated
      enderPhaseList() to render 4R separator and agent rows
    • Updated title to mention 4R reviewers
    • Updated ModelPickerRowsForProfile() comment to clarify 4R exclusion

Tests

  • internal/tui/screens/model_picker_test.go:
    • Updated TestModelPickerRows_Count (16 → 21 rows)
    • Added 5 new tests:
      • TestHandleModelNav_RRAgentRows_AssignCorrectly (4 subtests)
      • TestHandleModelNav_RRFirstRow
      • TestHandleModelNav_RRLastRow
      • TestHandleModelNav_RRSeparatorRow_NoAssignment
      • TestModelPickerRowsForProfile_NoRRAgents

Row Layout

\
Row 0: gentle-orchestrator (coordinator)
Row 1: Set all SDD phases
Rows 2-11: SDD phases (10)
Row 12: --- Judgment Day ---
Rows 13-15: JD agents (3)
Row 16: --- 4R Review ---
Rows 17-20: 4R agents (4)
\\

Notes

  • The 4R agents already exist in the overlay JSON for all providers (Claude, Cursor, Kimi, Kiro, OpenCode)
  • Only OpenCode has a TUI model picker, so this change is scoped to OpenCode
  • 4R agents are workflow-level (like JD) and excluded from profile-scoped pickers
  • Follows the exact JD pattern for consistency

Closes #928

Summary by CodeRabbit

  • New Features
    • Added a dedicated 4R Reviewers section (Risk, Readability, Reliability, Resilience) to the model picker, including a separator divider.
    • Updated the TUI phase labeling and row rendering to display 4R reviewer assignment options alongside existing agent selections.
  • Bug Fixes
    • Correctly maps selected 4R reviewer rows to their corresponding 4R phases, and ignores the 4R separator row without creating assignments.
  • Tests
    • Expanded model picker and navigation tests to cover 4R row counts, profile exclusions, and correct assignment behavior.

- Add RRPhases() function with 4 review agents (risk, readability, reliability, resilience)
- Add RRSeparatorRowIdx() for 4R separator row navigation
- Update ModelPickerRows() to include 4R section after JD agents
- Update applyAssignment() and applyAssignmentPreservingMatchingEffort() to handle 4R rows
- Update renderPhaseList() to render 4R separator and agent rows
- Update title to mention 4R reviewers
- Update ModelPickerRowsForProfile() comment to clarify 4R exclusion
- Update ConfigurableAgentPhases() to include 4R agents
- Update TestModelPickerRows_Count (16 → 21 rows)
- Add 5 new tests for 4R navigation and assignment
- Update ConfigurableAgentPhases() comment to mention 4R reviewers

Closes Gentleman-Programming#928
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aa160fa9-d5dd-45da-a586-bf3873acedf8

📥 Commits

Reviewing files that changed from the base of the PR and between c05891e and 2aaf102.

📒 Files selected for processing (2)
  • internal/tui/screens/model_picker.go
  • internal/tui/screens/model_picker_test.go

📝 Walkthrough

Walkthrough

RRPhases() is added to internal/opencode/models.go and appended to ConfigurableAgentPhases(), making 4R agents configurable. The TUI model picker gains a conditional "--- 4R Review ---" separator row and four 4R agent rows, a new RRSeparatorRowIdx() helper for indexing, extended assignment mapping in both apply functions to handle RR rows as no-ops or map them to corresponding phases, updated rendering to display the RR section with provider/model labels, and tests validating the new row count, per-row assignments, separator no-op behavior, and profile exclusion.

Changes

4R Review Agent Model Picker Support

Layer / File(s) Summary
RRPhases helper and ConfigurableAgentPhases update
internal/opencode/models.go
RRPhases() returns the four 4R agent names (review-risk, review-readability, review-reliability, review-resilience); ConfigurableAgentPhases() appends them alongside existing SDD and JD phases.
ModelPickerRows expansion and RRSeparatorRowIdx
internal/tui/screens/model_picker.go
ModelPickerRows conditionally appends a "--- 4R Review ---" separator and RR agent rows with updated capacity calculation. RRSeparatorRowIdx() computes the separator offset after orchestrator, SDD, JD section, and returns -1 when no RR phases exist.
Assignment mapping for 4R rows
internal/tui/screens/model_picker.go
applyAssignmentPreservingMatchingEffort and applyAssignment compute RR locals; extend switch branches to treat the RR separator row as a no-op and map RR agent rows to the corresponding RRPhases() entry for assignment.
renderPhaseList 4R rendering
internal/tui/screens/model_picker.go
Screen title updated to "JD Agents & 4R Reviewers"; rrSeparatorIdx local added; row switch branches render the RR separator divider and RR agent rows with provider/model assignment labels from the assignments map.
Model picker tests for 4R rows
internal/tui/screens/model_picker_test.go
Expected row count raised from 16 to 21; new handleModelNav test suite validates per-row RR phase assignment correctness, first/last row mappings, and separator no-op; TestModelPickerRowsForProfile_NoRRAgents confirms profile rows exclude all 4R agents and the separator.

Sequence Diagram

sequenceDiagram
  participant User
  participant ModelPicker as Model Picker UI
  participant RowMapper as Row Index Mapper
  participant AssignmentEngine as Assignment Engine
  participant Renderer as Renderer
  User->>ModelPicker: Select 4R agent row
  ModelPicker->>RowMapper: SelectedPhaseIdx (RR range)
  RowMapper->>RowMapper: Compute RRSeparatorIdx
  RowMapper->>RowMapper: Map row to RRPhases() entry
  RowMapper->>AssignmentEngine: Selected RR phase + provider/model
  AssignmentEngine->>AssignmentEngine: Apply assignment (no-op if separator)
  AssignmentEngine->>Renderer: Update assignments map
  Renderer->>Renderer: Compute rrSeparatorIdx
  Renderer->>Renderer: Render 4R separator divider
  Renderer->>Renderer: Render RR agent row with label
  Renderer->>User: Display updated picker with assignment
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding 4R review agents to the model picker UI component.
Linked Issues check ✅ Passed The PR fully implements all coding requirements from issue #928: adds RRPhases() function, updates ConfigurableAgentPhases(), implements UI navigation with RRSeparatorRowIdx(), handles 4R assignments in both applyAssignment functions, and includes comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly related to the scope of issue #928. No unrelated modifications to Judgment Day behavior, profile-scoping semantics outside 4R agents, or other features are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@internal/tui/screens/model_picker_test.go`:
- Around line 1391-1401: The test TestModelPickerRowsForProfile_NoRRAgents
currently only verifies that individual RR agents from opencode.RRPhases() are
excluded from the rows returned by ModelPickerRowsForProfile(), but it does not
verify that the "--- 4R Review ---" separator row itself is excluded. Add an
additional assertion after the nested loop to explicitly check that the RR
separator string is not present in the returned rows, ensuring a UI regression
where only the separator appears would be caught by the test.

In `@internal/tui/screens/model_picker.go`:
- Around line 166-177: The RRSeparatorRowIdx function unconditionally adds 1 for
the JD separator in its row index calculation, but the ModelPickerRows function
only conditionally adds the JD separator when len(opencode.JDPhases()) is
greater than 0. To maintain consistency and prevent potential off-by-one errors,
modify the RRSeparatorRowIdx function to add the same conditional check: only
include the +1 for the JD separator in the return statement when
len(opencode.JDPhases()) > 0, matching the logic used in ModelPickerRows.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 62767fdc-bd7a-4ba2-a7de-12ea3b9b0f31

📥 Commits

Reviewing files that changed from the base of the PR and between 0a9ffdd and c05891e.

📒 Files selected for processing (3)
  • internal/opencode/models.go
  • internal/tui/screens/model_picker.go
  • internal/tui/screens/model_picker_test.go

Comment thread internal/tui/screens/model_picker_test.go
Comment thread internal/tui/screens/model_picker.go
- Add defensive check in RRSeparatorRowIdx() to match ModelPickerRows() logic
- Add test assertion to verify 4R separator is excluded from profile rows

Addresses CodeRabbit review comments on PR Gentleman-Programming#936
@decode2

decode2 commented Jun 20, 2026

Copy link
Copy Markdown
Author

Applied CodeRabbit review feedback:

  1. Added defensive check in \RRSeparatorRowIdx()\ to handle empty JDPhases() consistently with ModelPickerRows()
  2. Added test assertion to verify 4R separator is excluded from profile rows

All tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(tui): allow configuring 4R review agent models in OpenCode SDD picker

1 participant