fix(dev-lead): intent-based model dispatch and fix copilot write fallback#204
fix(dev-lead): intent-based model dispatch and fix copilot write fallback#204don-petry wants to merge 19 commits into
Conversation
…back (#195) - Add model_for_intent() to engine.sh: human-pr/fix-bot-comment → haiku, fix-reviews/fix-ci/rebase → sonnet (action), fix-issue/human → sonnet (deep) - Wire model_for_intent into all four dev-lead action scripts via INTENT_TYPE - Fix Bug 2: copilot case in run_writer now calls copilot_chat directly instead of cycling back to the rate-limited Claude engine - Update header comment to document run_triage/run_agentic/run_duck as review-one-pr.sh-only (not used by dev-lead scripts) - Add 9 unit tests for model_for_intent covering all intent branches - Update test_engine_fallback.bats and test_engine_writer.bats to mock copilot_chat (function) rather than a copilot binary stub - Add curl stub + COPILOT_GITHUB_TOKEN to fix_ci/fix_reviews test setup so copilot_chat rate-limit detection works correctly in integration tests Closes #195 (partial — Fix 3/proactive headroom check deferred to follow-up) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds engine-tier mapping and routes writer calls through intent-resolved models. engine.sh gets _setup_engine_vars and model_for_intent, run_writer now accepts a tier_or_model and copilot returns exit 2 for text-only; run_writer_with_fallback iterates engines and treats exit codes 2 and 127 as retryable. Dev-lead scripts set INTENT_TYPE and pass model_for_intent results to writer calls; tests and test setup updated accordingly. ChangesIntent-Based Model Dispatch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
No description provided. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/engine.sh`:
- Around line 453-465: The current model_for_intent function returns a concrete
engine-bound model string which can become invalid if REVIEW_ENGINE is switched
during fallback; instead, make model_for_intent return a symbolic model key
(e.g., TRIAGE, ACTION, DEEP) or otherwise defer resolution, and then resolve
that key to the current ENGINE_* variable only at the point of use (after any
REVIEW_ENGINE change) so fallback always maps to a compatible model; locate
model_for_intent and update callers in the fallback flow to call the resolver
(or re-call model_for_intent mapping step) after REVIEW_ENGINE switches.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48172510-781e-493a-abb7-d22cdeb9c355
📒 Files selected for processing (8)
scripts/dev-lead-fix-ci.shscripts/dev-lead-fix-issue.shscripts/dev-lead-fix-reviews.shscripts/engine.shtests/dev-lead/unit/test_engine_fallback.batstests/dev-lead/unit/test_engine_writer.batstests/dev-lead/unit/test_fix_ci.batstests/dev-lead/unit/test_fix_reviews.bats
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d4143f8f6
ℹ️ 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".
| # GitHub Models REST API — text-only, no Write/Edit tool access. | ||
| # The rate-limit text (HTTP 429) is echoed to stdout by copilot_chat so | ||
| # is_rate_limited() will fire and return exit 2 for engine fallback. | ||
| copilot_chat "$prompt_file" "$ACTION_TIMEOUT_SEC" | tee "$_tmp" || rc=${PIPESTATUS[0]} |
There was a problem hiding this comment.
Keep text-only Copilot from satisfying write jobs
When REVIEW_ENGINE=copilot or fallback reaches Copilot after Claude/Gemini rate-limit, this call can return 0 with only a text response from GitHub Models; the comment above says it has no Write/Edit tool access, and nothing applies the returned patch. Because run_writer_with_fallback treats that 0 as success, write intents such as fix-reviews can post applied and fix-ci can stop with no changes instead of returning the rate-limited/fallback status for a retry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in commit 8d4e2cc. The copilot case in run_writer now returns exit 2 immediately (text-only, no Write/Edit tool access) instead of calling copilot_chat and returning 0. This ensures write intents never post a false status=applied or status=no-changes when copilot is reached — instead the retry cron re-triggers when a write-capable engine is available.
There was a problem hiding this comment.
Pull request overview
This PR reduces Dev-Lead token spend and improves reliability by selecting models based on intent complexity and fixing Copilot’s write/fallback path so rate-limit detection works correctly.
Changes:
- Adds
model_for_intent()toscripts/engine.shand wires intent-based model selection into Dev-Lead scripts. - Fixes Copilot write mode to call
copilot_chatdirectly and ensures rate-limit output is captured foris_rate_limited()→ exit2fallback behavior. - Updates/extends Bats unit tests (including new
model_for_intentcoverage) and adds curl stubs to exercise GitHub Models API 429 handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/engine.sh | Adds intent→model dispatch; fixes Copilot writer path to use copilot_chat and stream output for rate-limit detection. |
| scripts/dev-lead-fix-reviews.sh | Plumbs per-intent model selection into the review-intent dispatcher via build_and_run. |
| scripts/dev-lead-fix-ci.sh | Ensures fix-ci runs with intent-selected model (action tier). |
| scripts/dev-lead-fix-issue.sh | Ensures fix-issue runs with intent-selected model (deep tier). |
| tests/dev-lead/unit/test_engine_writer.bats | Updates tests to assert Copilot uses copilot_chat (no Claude fallback) and adds model_for_intent unit tests. |
| tests/dev-lead/unit/test_engine_fallback.bats | Updates fallback tests to reflect Copilot’s direct copilot_chat path. |
| tests/dev-lead/unit/test_fix_ci.bats | Adds curl stub + Copilot env to exercise GitHub Models API 429 rate-limit detection path in unit tests. |
| tests/dev-lead/unit/test_fix_reviews.bats | Adds curl stub + Copilot env to exercise GitHub Models API 429 rate-limit detection path in unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| local engine_rc=0 | ||
| run_writer_with_fallback "$prompt_file" || engine_rc=$? | ||
| run_writer_with_fallback "$prompt_file" "$(model_for_intent "$INTENT_TYPE")" || engine_rc=$? |
There was a problem hiding this comment.
Fixed in commit 8d4e2cc. model_for_intent now returns a tier key ("triage"/"action"/"deep") instead of a concrete model string. run_writer_with_fallback calls _setup_engine_vars($engine) before each invocation so ENGINE_*_MODEL vars are correct for each provider in the fallback chain. run_writer resolves the tier against those vars, so Gemini and Copilot always receive their own compatible model IDs.
| git checkout -b "$branch" | ||
|
|
||
| if ! run_writer_with_fallback "$prompt_file"; then | ||
| if ! run_writer_with_fallback "$prompt_file" "$(model_for_intent "$INTENT_TYPE")"; then |
There was a problem hiding this comment.
Fixed in commit 8d4e2cc. model_for_intent now returns a tier key ("triage"/"action"/"deep") instead of a concrete model string. run_writer_with_fallback calls _setup_engine_vars($engine) before each invocation so ENGINE_*_MODEL vars are correct for each provider in the fallback chain. run_writer resolves the tier against those vars, so Gemini and Copilot always receive their own compatible model IDs.
|
|
||
| local rc=0 | ||
| run_writer_with_fallback "$prompt_file" || rc=$? | ||
| run_writer_with_fallback "$prompt_file" "$model" || rc=$? | ||
| rm -f "$prompt_file" | ||
| return "$rc" |
There was a problem hiding this comment.
Fixed in commit 8d4e2cc. model_for_intent now returns a tier key ("triage"/"action"/"deep") instead of a concrete model string. run_writer_with_fallback calls _setup_engine_vars($engine) before each invocation so ENGINE_*_MODEL vars are correct for each provider in the fallback chain. run_writer resolves the tier against those vars, so Gemini and Copilot always receive their own compatible model IDs.
|
No description provided. |
There was a problem hiding this comment.
Code Review
This pull request introduces a tiered model selection mechanism via a new model_for_intent function, allowing different tasks (like CI fixes or issue implementation) to use appropriate model strengths. It also refactors the copilot engine handling in run_writer to use copilot_chat directly. However, a significant regression was identified: removing the fallback from Copilot to Claude in run_writer breaks the ability to apply code changes, as the Copilot API is text-only and cannot perform the necessary file operations.
| copilot) | ||
| # Copilot (gh copilot suggest) is text-only — falls back to Claude for write ops | ||
| echo "::warning::Copilot engine is text-only; falling back to Claude for write operations" >&2 | ||
| local saved="$REVIEW_ENGINE" | ||
| REVIEW_ENGINE="claude" timeout "$ACTION_TIMEOUT_SEC" claude --print \ | ||
| --model "$model" \ | ||
| --permission-mode acceptEdits \ | ||
| --allowed-tools "Bash,Read,Write,Edit,Grep,Glob" \ | ||
| < "$prompt_file" | tee "$_tmp" || rc=${PIPESTATUS[0]} | ||
| REVIEW_ENGINE="$saved" | ||
| # GitHub Models REST API — text-only, no Write/Edit tool access. | ||
| # The rate-limit text (HTTP 429) is echoed to stdout by copilot_chat so | ||
| # is_rate_limited() will fire and return exit 2 for engine fallback. | ||
| copilot_chat "$prompt_file" "$ACTION_TIMEOUT_SEC" | tee "$_tmp" || rc=${PIPESTATUS[0]} | ||
| ;; |
There was a problem hiding this comment.
The copilot engine implementation in run_writer is a functional regression. The previous code correctly identified that Copilot (via the GitHub Models API) is text-only and lacks tool-use capabilities (like Write or Edit), so it fell back to Claude to perform the actual file modifications. By removing this fallback and calling copilot_chat directly, the run_writer function will now "succeed" (exit 0) without actually applying any code changes. This will lead to confusing "no changes made by engine" messages in the calling scripts even when the model provides a valid fix in its text response.
There was a problem hiding this comment.
Fixed in commit 8d4e2cc. The copilot case in run_writer now returns exit 2 immediately (with a ::notice:: message) rather than calling copilot_chat and returning 0. Exit 2 exhausts the fallback chain and causes run_writer_with_fallback to emit ::error::All engines rate-limited or unavailable — the calling scripts then post a status=rate-limited marker and the retry cron re-triggers when a write-capable engine (claude, gemini) is available again. No false 'no changes made' or 'applied' markers.
There was a problem hiding this comment.
This is not a good solution. Waiting for Claude to become available is not an acceptable solution. If we can't use Copilot with this architecture then we should disable it until we can design a valid approach.
Should we be using GitHub Agentic Workflows? https://github.github.com/gh-aw/
Address all review comments on PR #204 (CodeRabbit, Copilot, Codex): - Extract engine config into _setup_engine_vars() so run_writer_with_fallback can re-initialize ENGINE_* vars for each fallback engine, ensuring every provider gets a compatible model ID instead of a cross-engine string - model_for_intent() now returns engine-agnostic tier keys ("triage"/"action"/"deep") instead of concrete model strings; run_writer resolves the key via the current ENGINE_*_MODEL vars after _setup_engine_vars has been called for that engine - Copilot case in run_writer returns exit 2 immediately (text-only, no Write/Edit tool access); this exhausts the fallback chain and triggers the retry cron rather than posting a false "applied" marker or silently doing nothing - Updated unit tests: model_for_intent checks tier keys; copilot write test asserts exit 2; fallback "all rate-limited" no longer needs a copilot_chat mock - Added two new engine-agnostic model_for_intent tests (tier key is same for claude and gemini engines; ENGINE_*_MODEL resolves correctly per engine) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Follow-up commit: 8d4e2ccAll review comments (CodeRabbit, Copilot ×3, Codex, Gemini) raised the same two issues. Both are addressed in the second commit. Cross-engine model incompatibility (all reviewers)
Copilot write path (Codex, Gemini)The copilot case in Fix 3 (proactive headroom check)Filed as #206 with a revised scope: the original Anthropic API header probe reflects per-minute token buckets, not the daily cap that caused the incident, and the CI environment uses OAuth not an API key. The issue describes a session-budget approach that does not require additional credentials. |
|
No description provided. |
|
No description provided. |
|
No description provided. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6789814ffc
ℹ️ 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".
| # deep — fix-issue, human: full agentic feature work | ||
| model_for_intent() { | ||
| case "${1:-}" in | ||
| human-pr|fix-bot-comment) echo "triage" ;; |
There was a problem hiding this comment.
Route write intents to an action-capable tier
For INTENT_TYPE=human-pr or fix-bot-comment, this mapping now passes triage into run_writer_with_fallback, which resolves to Haiku/Flash instead of the action model. Those handlers still run prompts that explicitly instruct the agent to use Edit/Write tools and commit fixes for review threads or bot findings, so these write-remediation paths are downgraded from the intended action tier and are more likely to miss or fail requested code changes on every human review or bot-comment event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The triage-tier mapping for human-pr and fix-bot-comment is intentional per issue #195 (see the proposed model_for_intent table in the issue body). The reasoning:
human-pr: triggered on any human PR review. The most common case (the one that caused the 2026-05-16 incident) is a zero-open-threads PR where the agent confirms no action is needed and exits. Haiku handles that cheaply. When threads exist, haiku still has fullWrite/Edittool access — it's a smaller model but capable of straightforward fixes.fix-bot-comment: responds to a single bot comment (SonarQube, CodeRabbit). The scope is narrow and well-defined; haiku is effective for targeted single-file edits.
Both are intentionally the cheapest tier first. If production data shows haiku misses fixes that sonnet would catch, the mapping is a one-line change in model_for_intent. That tuning is better done after re-enabling the workflow than speculatively before. Noted as a follow-up in #206 acceptance criteria.
…n fallback When a fallback engine binary is not present in the CI environment (e.g. gemini not installed), timeout exits with 127 (command not found). Previously this was treated as a non-retriable fatal error, causing the script to exit 127 instead of continuing to the next engine and eventually posting a rate-limited marker for the retry cron. Add exit 127 to the fallback continuation condition alongside exit 2. Two new unit tests cover: (a) 127 causes all-engines-exhausted path (exit 2), and (b) a 127 engine is still reached and skipped before the next candidate. Observed in dispatch CI run 25975341578: claude rate-limited → gemini not found (127) → script exited 127 instead of posting rate-limited marker and exiting 2. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No description provided. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/dev-lead/unit/test_engine_fallback.bats`:
- Around line 131-136: Rename the test title and update its comments to reflect
that the scenario asserts status 2 (all engines exhausted) instead of "next
engine succeeds": change the `@test` header string currently "fallback: engine not
installed (127) → next engine succeeds" to something like "fallback: engine not
installed (127) → all engines exhausted (2)" and adjust the inline comment lines
describing the fallback order so they no longer claim a succeeding next engine;
keep the test body (including the _make_stub "gemini" 2 call) unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51280475-888f-42ff-ad4b-49b6aaa3e04d
📒 Files selected for processing (3)
scripts/engine.shtests/dev-lead/unit/test_engine_fallback.batstests/dev-lead/unit/test_engine_writer.bats
| @test "fallback: engine not installed (127) → next engine succeeds" { | ||
| # claude rate-limited, gemini not installed, but the next engine after gemini succeeds | ||
| # In the actual fallback order: claude → gemini → copilot. Since copilot returns 2 | ||
| # (text-only) in run_writer, we verify with a scenario where gemini is "not found" | ||
| # and a recording stub shows that claude was tried after gemini failed. | ||
| _make_stub "gemini" 2 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Rename this test to match the asserted outcome.
The title/comment says “next engine succeeds,” but the case asserts status 2 (all engines exhausted). Please align the name/comments with the actual expectation to avoid misleading failures.
Proposed rename/comment cleanup
-@test "fallback: engine not installed (127) → next engine succeeds" {
- # claude rate-limited, gemini not installed, but the next engine after gemini succeeds
- # In the actual fallback order: claude → gemini → copilot. Since copilot returns 2
- # (text-only) in run_writer, we verify with a scenario where gemini is "not found"
- # and a recording stub shows that claude was tried after gemini failed.
+@test "fallback: engine not installed (127) → continues and eventually exhausts" {
+ # In fallback order gemini (primary) → claude → copilot, verify 127 is treated
+ # as unavailable/continue and claude is still attempted before exhaustion.📝 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.
| @test "fallback: engine not installed (127) → next engine succeeds" { | |
| # claude rate-limited, gemini not installed, but the next engine after gemini succeeds | |
| # In the actual fallback order: claude → gemini → copilot. Since copilot returns 2 | |
| # (text-only) in run_writer, we verify with a scenario where gemini is "not found" | |
| # and a recording stub shows that claude was tried after gemini failed. | |
| _make_stub "gemini" 2 | |
| `@test` "fallback: engine not installed (127) → continues and eventually exhausts" { | |
| # In fallback order gemini (primary) → claude → copilot, verify 127 is treated | |
| # as unavailable/continue and claude is still attempted before exhaustion. | |
| _make_stub "gemini" 2 |
🤖 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 `@tests/dev-lead/unit/test_engine_fallback.bats` around lines 131 - 136, Rename
the test title and update its comments to reflect that the scenario asserts
status 2 (all engines exhausted) instead of "next engine succeeds": change the
`@test` header string currently "fallback: engine not installed (127) → next
engine succeeds" to something like "fallback: engine not installed (127) → all
engines exhausted (2)" and adjust the inline comment lines describing the
fallback order so they no longer claim a succeeding next engine; keep the test
body (including the _make_stub "gemini" 2 call) unchanged.
|
No description provided. |
|
No description provided. |
|
No description provided. |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
@dev-lead - please fix this PR |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
Address all review comments on PR #204 (CodeRabbit, Copilot, Codex): - Extract engine config into _setup_engine_vars() so run_writer_with_fallback can re-initialize ENGINE_* vars for each fallback engine, ensuring every provider gets a compatible model ID instead of a cross-engine string - model_for_intent() now returns engine-agnostic tier keys ("triage"/"action"/"deep") instead of concrete model strings; run_writer resolves the key via the current ENGINE_*_MODEL vars after _setup_engine_vars has been called for that engine - Copilot case in run_writer returns exit 2 immediately (text-only, no Write/Edit tool access); this exhausts the fallback chain and triggers the retry cron rather than posting a false "applied" marker or silently doing nothing - Updated unit tests: model_for_intent checks tier keys; copilot write test asserts exit 2; fallback "all rate-limited" no longer needs a copilot_chat mock - Added two new engine-agnostic model_for_intent tests (tier key is same for claude and gemini engines; ENGINE_*_MODEL resolves correctly per engine) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Closing — fully superseded by subsequent work on mainBoth core fixes in this PR have since landed in main with better implementations. Closing to keep the backlog clean. Fix 1 — Intent-based model dispatch ✅ already in main
Identical in substance to this PR's acceptance criteria. Fix 2 — Copilot write path ✅ already in main (better implementation)Main went further than this PR proposed. The copilot write case now returns copilot)
rm -f "$_tmp"
echo "::notice::copilot engine is text-only; write operations unavailable — triggering retry" >&2
return 2
;;Additionally, Other divergences that make the branch un-mergeable as-is
The 16+ merge-from-main commits on this branch reflect staleness accumulation, not progress toward resolution. There is nothing left in this PR that would be additive to merge. |
Summary
Addresses issue #195 — the two root causes of the 2026-05-16 token spike that exhausted the daily usage cap.
model_for_intent()toengine.shand wires it into all four dev-lead action scripts.human-prandfix-bot-commentnow useENGINE_TRIAGE_MODEL(haiku-4-5) instead of sonnet unconditionally. The zero-open-threadshuman-prrun that triggered the incident would have been handled by haiku at a fraction of the cost.copilot → warning → Claude again) is replaced with a directcopilot_chatcall. Rate-limit output from the GitHub Models API is echoed to stdout sois_rate_limited()fires correctly and returns exit 2 for engine fallback.engine.shnow clearly notes thatrun_triage/run_agentic/run_duckarereview-one-pr.sh-only functions, not part of the dev-lead write pipeline.model_for_intentcovering all intent branches and the default case.test_engine_writer.batsupdated to mockcopilot_chatas a function (not binary);test_fix_ci.batsandtest_fix_reviews.batssetup updated with acurlstub returning 429 for GitHub Models API and a dummyCOPILOT_GITHUB_TOKEN, so the copilot rate-limit detection path is exercised correctly.Not in scope: Fix 3 (proactive usage headroom check) is deferred — the proposed
ANTHROPIC_API_KEYprobe reflects per-minute token buckets, not the daily subscription cap that caused the incident, and the workflows use OAuth not an API key. Filed as a follow-up.Acceptance criteria covered
human-pr/fix-bot-commentuseENGINE_TRIAGE_MODEL(haiku-4-5)fix-reviews/fix-ci/rebaseuseENGINE_ACTION_MODEL(sonnet-4-6)fix-issue/humanuseENGINE_DEEP_MODEL(sonnet-4-6)copilot_chatdirectly — no claude fallbackmodel_for_intenttests addedTest plan
bats tests/dev-lead/unit/— 112/112 pass locallyhuman-printent logs haiku modelCOPILOT_GITHUB_TOKENsecret)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests