fix: resolve bare tier names to full model strings before DB storage#2256
fix: resolve bare tier names to full model strings before DB storage#2256marcusquinn merged 1 commit intomainfrom
Conversation
Three bugs caused model_config_error_sonnet_invalid_provider_format: 1. batch.sh cmd_add(): extracted model:sonnet from TODO.md and stored the bare tier name directly in the DB. Added resolve_model() call before INSERT to convert tier names to full strings. 2. ai-actions.sh _exec_escalate_model(): used wrong column name (task_id instead of id) so UPDATE silently matched 0 rows. Also stored bare tier name. Fixed column name, added sql_escape, and resolve_model() before UPDATE. 3. dispatch.sh cmd_dispatch(): wrote dispatch metadata using resolved_model before it was set (line 2874 vs 2906). Moved model resolution before metadata write so logs show the actual model.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses several critical issues related to AI model name handling and database interactions within the supervisor scripts. The primary goal is to ensure that AI model tier names are consistently resolved to their full model strings before being stored in the database or logged, preventing Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThese changes introduce model tier normalization across three supervisor scripts, resolving bare tier names (e.g., "sonnet", "opus") to full model strings using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Feb 25 01:48:05 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses several bugs related to model name resolution and database updates. The changes ensure that bare model tier names are resolved to their full identifiers before being stored, fix a silent database update failure due to an incorrect column name, and improve logging in the dispatch script. My review focuses on consistently applying the repository's guidelines regarding error handling. I've suggested removing instances of 2>/dev/null where appropriate to prevent the suppression of potentially important error messages, which will improve the debuggability of these scripts, while acknowledging specific exceptions for database operations.
| # Resolve bare tier name to full model string before DB update | ||
| local resolved_to_tier="$to_tier" | ||
| if [[ -n "$to_tier" && "$to_tier" != *"/"* ]]; then | ||
| resolved_to_tier=$(resolve_model "$to_tier" "opencode" 2>/dev/null) || resolved_to_tier="$to_tier" |
There was a problem hiding this comment.
The use of 2>/dev/null here suppresses all error output from resolve_model. While the || resolved_to_tier="$to_tier" provides a fallback, it's better to allow errors (like syntax errors in helper scripts or configuration issues) to be visible for debugging. Consider removing 2>/dev/null to improve error visibility.
| resolved_to_tier=$(resolve_model "$to_tier" "opencode" 2>/dev/null) || resolved_to_tier="$to_tier" | |
| resolved_to_tier=$(resolve_model "$to_tier" "opencode") || resolved_to_tier="$to_tier" |
References
- Avoid blanket suppression of errors with '2>/dev/null' to ensure that actual syntax or system errors remain visible for debugging.
| # cause model_config_error when passed to the CLI at dispatch time. | ||
| if [[ -n "$model" && "$model" != *"/"* ]]; then | ||
| local resolved_model | ||
| resolved_model=$(resolve_model "$model" "opencode" 2>/dev/null) || resolved_model="" |
There was a problem hiding this comment.
The use of 2>/dev/null suppresses all error output from resolve_model. While the || resolved_model="" provides a fallback, it's better to allow potential errors (like syntax errors in helper scripts or configuration issues) to be visible for debugging.
| resolved_model=$(resolve_model "$model" "opencode" 2>/dev/null) || resolved_model="" | |
| resolved_model=$(resolve_model "$model" "opencode") || resolved_model="" |
References
- Avoid blanket suppression of errors with '2>/dev/null' to ensure that actual syntax or system errors remain visible for debugging.
| # it discovers the work is incomplete, but starts cheap. | ||
| local resolved_model | ||
| if [[ "$verify_mode" == "true" ]]; then | ||
| resolved_model=$(resolve_model "coding" "$ai_cli" 2>/dev/null) || resolved_model="" |
There was a problem hiding this comment.
The use of 2>/dev/null suppresses all error output from resolve_model. While the || resolved_model="" provides a fallback, it's better to allow potential errors (like syntax errors in helper scripts or configuration issues) to be visible for debugging. This would also make it consistent with the resolve_task_model call in the else block, which does not suppress stderr.
| resolved_model=$(resolve_model "coding" "$ai_cli" 2>/dev/null) || resolved_model="" | |
| resolved_model=$(resolve_model "coding" "$ai_cli") || resolved_model="" |
References
- Avoid blanket suppression of errors with '2>/dev/null' to ensure that actual syntax or system errors remain visible for debugging.
🤖 Augment PR SummarySummary: This PR prevents bare tier names (e.g. Changes:
resolve_model()/resolve_task_model() for tier→model mapping and sql_escape() for SQLite literal safety.
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| # Resolve bare tier names (e.g., "sonnet", "opus") to full model strings | ||
| # (e.g., "anthropic/claude-sonnet-4-6") before storing in DB. Bare tier names | ||
| # cause model_config_error when passed to the CLI at dispatch time. | ||
| if [[ -n "$model" && "$model" != *"/"* ]]; then |
There was a problem hiding this comment.
This condition treats any model value without a / as a tier; since resolve_model always echoes a non-empty default even for unknown inputs, it can silently coerce unexpected values (e.g. a bare model id) to the default model before DB storage.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| # Resolve bare tier name to full model string before DB update | ||
| local resolved_to_tier="$to_tier" | ||
| if [[ -n "$to_tier" && "$to_tier" != *"/"* ]]; then | ||
| resolved_to_tier=$(resolve_model "$to_tier" "opencode" 2>/dev/null) || resolved_to_tier="$to_tier" |
There was a problem hiding this comment.
resolve_model appears to always return success and echo a model string, so the || resolved_to_tier="$to_tier" fallback likely never triggers; any non-/ to_tier (even if it’s not a known tier) will get coerced to the default resolved model.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| # it discovers the work is incomplete, but starts cheap. | ||
| local resolved_model | ||
| if [[ "$verify_mode" == "true" ]]; then | ||
| resolved_model=$(resolve_model "coding" "$ai_cli" 2>/dev/null) || resolved_model="" |
There was a problem hiding this comment.
In verify mode this resolves the coding tier, but resolve_model’s static mapping treats coding as opus; if verify-mode is meant to start cheaper (sonnet), this may not hold when the chain/availability helpers don’t resolve differently.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/supervisor/dispatch.sh:
- Around line 2860-2872: The verify-mode dispatch incorrectly calls
resolve_model with the "coding" tier (via resolve_model "coding"), causing Opus
to be selected; change the resolve_model invocation in the verify-mode branch to
use the intended cheaper tier name (e.g., "sonnet" or the project's verify-tier
identifier) so resolved_model is set to the low-cost model; update the call site
that sets resolved_model (the block referencing verify_mode, resolved_model, and
resolve_model) to pass the correct tier string and keep the existing logging
behavior.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.agents/scripts/supervisor/ai-actions.sh.agents/scripts/supervisor/batch.sh.agents/scripts/supervisor/dispatch.sh
| # Resolve model via frontmatter + fallback chain (t132.5) | ||
| # Moved before metadata write so dispatch log has the resolved model, not bare tier. | ||
| # t1008: For verify-mode dispatches, prefer sonnet tier (cheaper, sufficient for | ||
| # verification checks). The verify worker can escalate to full implementation if | ||
| # it discovers the work is incomplete, but starts cheap. | ||
| local resolved_model | ||
| if [[ "$verify_mode" == "true" ]]; then | ||
| resolved_model=$(resolve_model "coding" "$ai_cli" 2>/dev/null) || resolved_model="" | ||
| log_info "Verify mode: using coding-tier model ($resolved_model) instead of task-specific model" | ||
| else | ||
| resolved_model=$(resolve_task_model "$task_id" "$tmodel" "${trepo:-.}" "$ai_cli") | ||
| fi | ||
|
|
There was a problem hiding this comment.
Verify mode is still resolving to coding (Opus), not the intended cheaper tier.
Line 2867 uses resolve_model "coding", which maps to Opus in this file. That conflicts with the verify-mode cost-saving intent and will over-spend on verification dispatches.
Suggested fix
if [[ "$verify_mode" == "true" ]]; then
- resolved_model=$(resolve_model "coding" "$ai_cli" 2>/dev/null) || resolved_model=""
- log_info "Verify mode: using coding-tier model ($resolved_model) instead of task-specific model"
+ resolved_model=$(resolve_model "sonnet" "$ai_cli" 2>/dev/null) || resolved_model=""
+ log_info "Verify mode: using sonnet-tier model ($resolved_model) instead of task-specific model"
else
resolved_model=$(resolve_task_model "$task_id" "$tmodel" "${trepo:-.}" "$ai_cli")
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/supervisor/dispatch.sh around lines 2860 - 2872, The
verify-mode dispatch incorrectly calls resolve_model with the "coding" tier (via
resolve_model "coding"), causing Opus to be selected; change the resolve_model
invocation in the verify-mode branch to use the intended cheaper tier name
(e.g., "sonnet" or the project's verify-tier identifier) so resolved_model is
set to the low-cost model; update the call site that sets resolved_model (the
block referencing verify_mode, resolved_model, and resolve_model) to pass the
correct tier string and keep the existing logging behavior.
1 participant
Summary
cmd_add()extractedmodel:sonnetfrom TODO.md and stored the bare tier name in the DB. Workers receivedsonnetas the model string, causingmodel_config_error_sonnet_invalid_provider_format. Now resolves viaresolve_model()before INSERT._exec_escalate_model()used wrong column name (task_idinstead ofid) so the UPDATE silently matched 0 rows. Also stored bare tier name. Fixed column, addedsql_escape(), andresolve_model().resolved_modelbefore it was set (fell back to bare$tmodel). Moved model resolution before metadata write.Impact
model_config_error_sonnet_invalid_provider_formatfailures (t003.5, t009.1)_exec_escalate_model()(wrong column name)Summary by CodeRabbit
Release Notes