Remove hardcoded codex role models; add role_model config - #161
Conversation
Reviewer's GuideThe PR removes model assumptions from bundled roles and Codex rendering, introduces an optional per-agent Sequence diagram for role model resolution and Codex renderingsequenceDiagram
participant Config as dotagents.yaml
participant Renderer as renderAgentRole
participant Role as agentRole
participant Codex as renderCodexAgentRole
participant Output as Codex role TOML
Config->>Renderer: load agentConfig.RoleModel
Renderer->>Role: inspect Model
alt role.Model is empty and RoleModel is configured
Renderer->>Role: apply agent.RoleModel
end
Renderer->>Codex: renderCodexAgentRole(role)
Codex->>Role: read Codex.Model
alt Codex.Model is empty
Codex->>Role: read Model
end
Codex-->>Output: emit resolved model or omit model
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="cmd/dotagents/agents.go" line_range="410" />
<code_context>
model := strings.TrimSpace(role.Codex.Model)
if model == "" {
- model = codexModelFor(role.Model)
+ model = strings.TrimSpace(role.Model)
}
effort := strings.TrimSpace(role.Codex.ModelReasoningEffort)
</code_context>
<issue_to_address>
**issue (broader_impact):** Legacy roles that still declare canonical models such as `opus`, `sonnet`, or `haiku` are emitted to Codex with those names unchanged. Codex previously translated these aliases through `codexModelFor`, so those roles now fail when Codex rejects the unsupported model identifier.
**Triggers:** When an existing custom role or an imported role retains a non-empty canonical model value.
**Suggested fix:** Preserve a compatibility mapping for legacy canonical aliases, or migrate/validate existing role models before rendering them for Codex.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: cmd/dotagents/agents.go:410
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4743dd9660
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb78b13f5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # README.md # agents/researcher.md
Sourcery withdrew this approval because the latest commits introduced blocking findings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d736ad4d18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary by Sourcery
Remove hardcoded model assignments from built-in roles and support configurable, harness-aware model selection through
role_model.New Features:
role_modelconfiguration for pinning a default model across rendered roles without explicit model settings.Bug Fixes:
Enhancements:
Documentation:
role_modelconfiguration.Tests:
Chores: