Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
the
maintainer:e2e:ok
OpenCodeReview — PR #2748
|
WalkthroughBefore this PR, the issue planner workflow could not reliably search for related PRs because the GitHub search query used an unquoted Release NotesBug Fixes
New Features
Tests
Refactor
Changes
Magnitude🎯 1 (S) Related
Pre-merge Checks
Walkthrough generated by LLxprt PR Review. Planner issue: #2256 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
![github-actions[bot]](https://avatars.github.com/in/15368?s=60&v=4){kind=small}
| - name: 'Setup Bun' | ||
| if: steps.issue_gate.outputs.should_review == 'true' | ||
| uses: 'oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76' # ratchet:oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version-file: '.bun-version' |
There was a problem hiding this comment.
[bug/high] This step will likely fail because
oven-sh/setup-bunrequiresbun-version-fileto point to an existing file, but.bun-versiondoes not exist in the repo. Either add a.bun-versionfile or switch to pinningbun-versiondirectly in this step (for example,bun-version: '1.2.x').
| fs.appendFileSync( | ||
| githubEnvPath, | ||
| `OPENAI_API_KEY<<${delimiter}\n${keyValue}\n${delimiter}\n`, | ||
| ); |
There was a problem hiding this comment.
[maintainability/medium] The
exportSelectedKeyToEnvfunction hardcodes the environment variable name asOPENAI_API_KEY. While this matches the current downstream requirement in the planner agent step, it reduces reusability for different providers (e.g., Anthropic-compatible endpoints that might expectANTHROPIC_API_KEY). Consider parameterizing the environment variable name instead of hardcoding it, especially since the PR adds planner-specific provider configuration.
| !Number.isSafeInteger(requests) || | ||
| requests < 0 | ||
| ) { | ||
| const data = (await response.json()) as QuotaResponse; |
There was a problem hiding this comment.
[maintainability/low] The type assertion
(await response.json()) as QuotaResponsebypasses runtime validation of the API response structure. If the API returns an unexpected format (e.g.,subscriptionis a string or array instead of an object), the subsequent destructuring could behave unexpectedly. WhileisValidQuotaCountervalidates the numeric fields, it doesn't validate thatsubscriptionis actually an object-like structure. Consider adding a runtime type guard for the response shape.
| @@ -146,9 +208,10 @@ export async function selectOptimalKey() { | |||
|
|
|||
| const selectedKeyName = selectedKey === key2 ? 'secondary' : 'primary'; | |||
| writeSelectedKeyOutput(selectedKeyName); | |||
| exportSelectedKeyToEnv(selectedKey, selectedKeyName); | |||
There was a problem hiding this comment.
[bug/medium] The
exportSelectedKeyToEnvfunction will throw a TypeError if called with an undefinedkeyValue(e.g., when no API keys are configured). The original code gracefully handled missing keys by writing toGITHUB_OUTPUTwithout crashing. Consider adding a guard for undefined/null keys or ensuring callers validate the key before calling this function, especially in paths like the synthetic test path whereprocess.env[keyVarName]may be undefined.
OpenCodeReview — PR #2748
Findings without a resolvable position
Shadow-mode routing preview (issue #2672)
|
OpenCodeReview — PR #2748
|
84f48e2 to
3250ebd
Compare
OpenCodeReview — PR #2748
OCR stderr excerptOCR preflight excerptOCR preview stderr excerpt
|
…ic model config (Fixes #2747) The Issue Planner workflow failed on 100% of runs at the 'Precompute related PRs/issues candidates' step because gh search prs --state merged is invalid (--state only accepts open|closed). The correct flag is the boolean --merged. The planner LLM agent never reached its run step. This commit also: - Adds PLANNER_PROVIDER/PLANNER_MODEL/PLANNER_BASE_URL/PLANNER_KEY_VAR_NAME repo vars with fallback to the shared CI defaults, so the planner can use a stronger model (e.g. glm-5.2 via z.ai/chutes) without affecting the test matrix. The CLI --key flag is provider-agnostic, supporting both OpenAI-compatible and Anthropic-compatible endpoints. - Fixes a latent key-propagation bug: ci-quota-check.js only wrote the selected key to GITHUB_OUTPUT (step output) but never to GITHUB_ENV, so the agent step's ${OPENAI_API_KEY} would have been empty even after the gh flag fix. The script now exports the selected key to GITHUB_ENV via a random-delimiter heredoc. - Adds regression tests asserting --merged (not --state merged), the planner-specific env vars in the agent step, and GITHUB_ENV key propagation in the quota-check success paths.
OpenCodeReview — PR #2748
OCR stderr excerptOCR preflight excerptOCR preview stderr excerpt
|
OpenCodeReview — PR #2748
|
1 participant
Summary
The Issue Planner workflow (
.github/workflows/issue-planner.yml) failed on 100% of runs since it was introduced in #2720. This PR fixes the root cause and adds planner-specific model configuration so a stronger model can be used for planning.Root cause
The "Precompute related PRs/issues candidates" step used
gh search prs --state merged, which is invalid — the--stateflag only accepts{open|closed}. The correct flag is the boolean--merged. The workflow died at this step on every run, so the planner LLM agent never executed.Changes
Fix 1: Correct the gh search prs flag (Blocker)
--state merged→--mergedin.github/workflows/issue-planner.yml.Fix 2: Planner-specific model configuration
Adds
PLANNER_PROVIDER,PLANNER_MODEL,PLANNER_BASE_URL,PLANNER_KEY_VAR_NAME, andPLANNER_KEY_VAR_NAME_2repo vars, each falling back to the shared CI defaults (LLXPRT_DEFAULT_PROVIDER,LLXPRT_DEFAULT_MODEL,OPENAI_BASE_URL,KEY_VAR_NAME,KEY_VAR_NAME_2). This allows setting a stronger model (e.g. glm-5.2 via z.ai or chutes) for planning without affecting the CI test matrix.The CLI
--keyflag is provider-agnostic, so this supports both:https://llm.chutes.ai/v1)https://api.z.ai/api/anthropic)To enable a stronger planner model, set these repo variables:
PLANNER_MODEL(e.g.glm-5.2)PLANNER_PROVIDER(e.g.anthropicoropenai)PLANNER_BASE_URL(e.g.https://api.z.ai/api/anthropic)PLANNER_KEY_VAR_NAME(e.g.ZAI_KEY— the name of a secret)If unset, the planner falls back to the existing shared defaults.
Fix 3: Key propagation to GITHUB_ENV
Fixed a latent bug in
scripts/ci-quota-check.js: the script wrote the selected key toGITHUB_OUTPUT(step output) but never toGITHUB_ENV. Step-scoped env does not propagate across steps, so the "Run planner agent" step's OPENAI_API_KEY would have been empty even after the gh flag fix. The script now exports the selected key toGITHUB_ENVvia a random-delimiter heredoc (the random suffix prevents delimiter collision with the key value).Fix 4: Regression tests
Added tests in
scripts/tests/issue-planner.test.js:--mergedis used (not the invalid--state merged)PLANNER_PROVIDER/PLANNER_MODEL/PLANNER_BASE_URL(not the shared defaults)Updated tests in
scripts/tests/ci-quota-check.test.jsto verify the selected key propagates toGITHUB_ENVon success paths.Acceptance criteria
gh search prscommand uses--mergedinstead of--state merged--mergedflag is usedTest plan
scripts/tests/issue-planner.test.js— 49 tests pass (4 new)scripts/tests/ci-quota-check.test.js— 25 tests pass (5 updated)Verification
Fixes #2747