feat(review): add NVIDIA NIM provider - #672
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughOpenCode, Strix 및 Noema 리뷰 워크플로우에 NVIDIA NIM Nemotron provider와 모델 후보를 추가했다. 저장소 가시성과 API 키에 따라 provider를 선택한다. 키가 없으면 기존 provider로 폴백하거나 NIM 후보를 건너뛴다. ChangesNVIDIA NIM 리뷰 통합
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant RepositoryAPI
participant ReviewGate
participant ModelPoolRunner
participant NVIDIA_NIM_API
Workflow->>RepositoryAPI: 대상 저장소 가시성 조회
RepositoryAPI-->>Workflow: is_private 반환
Workflow->>ReviewGate: 모델과 provider 설정 전달
ReviewGate->>ModelPoolRunner: 후보와 API 키 전달
ModelPoolRunner->>NVIDIA_NIM_API: 유효한 키로 Nemotron 요청
ModelPoolRunner-->>ReviewGate: 성공 또는 기존 provider fallback
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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 @.github/workflows/opencode-review-dispatch.yml:
- Line 3878: Move the nvidia-nim candidate into the is_private == 'false'
conditional branch in OPENCODE_MODEL_CANDIDATES so private PRs cannot route data
to the NVIDIA API Trial endpoint. Preserve the existing public candidate
ordering and fallback candidates, and update
tests/test_opencode_agent_contract.py to assert that the private candidate set
excludes all nvidia-nim models.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 119a53fc-3267-47b7-995b-ce282c7e3ce5
📒 Files selected for processing (5)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_required_workflow_queue_contract.py (1)
379-383: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win문자열 존재만 검증 –
NVIDIA_NIM_API_KEY부재 시 폴백 동작에 대한 회귀 테스트가 없음.이 assert들은 워크플로우 소스에 특정 문자열이 존재하는지만 확인합니다.
run_opencode_review_model_pool.sh의should_skip_model_candidate처럼 실제 bash 로직을 서브프로세스로 실행해,NVIDIA_NIM_API_KEY가 비어있을 때도NOEMA_LLM_API_KEY가 기존OPENAI_API_KEY폴백 값을 유지하는지 검증하는 테스트를 추가하는 것을 권장합니다.🤖 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/test_required_workflow_queue_contract.py` around lines 379 - 383, Extend the workflow contract test around the existing NVIDIA_NIM_API_KEY assertions to execute the relevant run_opencode_review_model_pool.sh Bash logic in a subprocess, using an empty NVIDIA_NIM_API_KEY and a populated OPENAI_API_KEY. Assert that NOEMA_LLM_API_KEY retains the OPENAI_API_KEY fallback value, covering the should_skip_model_candidate-related behavior rather than only checking workflow source strings.
🤖 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 @.github/workflows/noema-review.yml:
- Around line 290-294: Ensure public-repository NVIDIA NIM defaults are used
only when NVIDIA_NIM_API_KEY is configured: in
.github/workflows/noema-review.yml lines 290-294, gate the NOEMA_LLM_API_URL,
NOEMA_LLM_MODEL, and NOEMA_LLM_API_KEY overrides on a non-empty key so the
existing OPENAI_API_KEY fallback remains intact; in .github/workflows/strix.yml
lines 446-520, retain gpt-5.6-luna when the key is absent; and in
tests/test_required_workflow_queue_contract.py lines 379-383, add
execution-based regression coverage proving both workflows preserve their
existing fallbacks without NVIDIA_NIM_API_KEY.
---
Nitpick comments:
In `@tests/test_required_workflow_queue_contract.py`:
- Around line 379-383: Extend the workflow contract test around the existing
NVIDIA_NIM_API_KEY assertions to execute the relevant
run_opencode_review_model_pool.sh Bash logic in a subprocess, using an empty
NVIDIA_NIM_API_KEY and a populated OPENAI_API_KEY. Assert that NOEMA_LLM_API_KEY
retains the OPENAI_API_KEY fallback value, covering the
should_skip_model_candidate-related behavior rather than only checking workflow
source strings.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 64521d2b-e4be-43b5-8d18-06f9695d5f49
📒 Files selected for processing (7)
.github/workflows/noema-review.yml.github/workflows/opencode-review-dispatch.yml.github/workflows/strix.ymlscripts/ci/emit_opencode_failed_check_fallback_findings.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_required_workflow_queue_contract.py
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head evidence but found unresolved reviewer or review-agent threads before approval.
Findings
1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved reviewer thread blocks automated approval
- Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human or review-agent thread evidence on the current pull request.
- Root cause: Reviewer and review-agent feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
- Fix: Address or resolve the listed reviewer thread(s), then re-run OpenCode on the current head.
- Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE, including bot review agents other than OpenCode itself.
Review thread evidence
Latest unresolved reviewer thread evidence
.github/workflows/noema-review.yml line 294
-
Latest reviewer comment: @coderabbitai at 2026-07-30T08:59:21Z
-
Comment URL: #672 (comment)
-
Comment excerpt: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win / 'NVIDIA_NIM_API_KEY' 미설정 시 공개 저장소의 기존 리뷰/스캔 폴백이 깨짐 – 세 파일이 하나의 근본 원인을 공유. / OpenCode 모델 풀('run_opencode_review_model_pool.sh')은 'NVIDIA_NIM_API_KEY'가 없으면 후보를 스킵하고 다음 provider로 넘어가는 우아한 폴백을 구현하지만, Strix와 Noema는 동일한 안전장치 없이 공개 저장소의 기본 provider를 'nvidia_nim'으로 바꿔버려 키가 없으면 전체가 실패합니다. 특히 Noema는 이미 계산해 둔 'OPENAI_API_KEY' 폴백 값을 무조건 덮어써서 파괴하는 확정적 버그이며, 이를 잡아낼 회귀 테스트도 없습니다. / - '.github/workflows/noema-review.yml#L290-L294': 'NVIDIA_NIM_API_KEY'가 비어있지 않을 때만 'N
-
Result: REQUEST_CHANGES
-
Reason: unresolved reviewer or review-agent thread(s) were present before approval.
-
Head SHA:
bea81270085fa06fddfbd9b930c9cda1164773e9 -
Workflow run: 30528448031
-
Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow (3 files)"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (3 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script (3 files)"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (3 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path. Findings1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
gh pr checkout 672 --repo ContextualWisdomLab/.github
git fetch origin main
git merge --no-ff origin/main # or: git rebase origin/main
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:fix/nvidia-nim-provider
# rebase path only: git push --force-with-lease origin HEAD:fix/nvidia-nim-provider
Merge Conflict Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (4 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (4 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
Merge Conflict Guidance
gh pr checkout 672 --repo ContextualWisdomLab/.github
git fetch origin main
git merge --no-ff origin/main # or: git rebase origin/main
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:fix/nvidia-nim-provider
# rebase path only: git push --force-with-lease origin HEAD:fix/nvidia-nim-provider |
Addressed on the current head: Strix and Noema now preserve existing provider fallbacks when NVIDIA_NIM_API_KEY is absent, with execution-based regression coverage.
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 @.github/workflows/strix.yml:
- Around line 456-458: Propagate the normalized fallback selected in the gate
through a gate output instead of retaining it only in the local strix_model
variable. Update the model-file step around the visibility expression to consume
that output and stop recomputing the NIM model, ensuring keyless repositories
use the gpt-5.6-luna fallback consistently with provider_mode=openai_direct.
🪄 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: CHILL
Plan: Pro Plus
Run ID: ff0cd909-6ad9-4ff8-a9ca-aac25cbbfe9c
📒 Files selected for processing (4)
.github/workflows/noema-review.yml.github/workflows/strix.ymlscripts/ci/test_strix_quick_gate.shtests/test_required_workflow_queue_contract.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci/test_strix_quick_gate.sh (1)
318-344: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win시크릿 전달 범위를 job 단위로 검증하세요.
현재 검사는 workflow 전체에서 문자열의 존재만 확인합니다.
coverage-evidence또는 다른 비모델 job에NVIDIA_NIM_API_KEY바인딩을 추가해도 테스트가 통과합니다.PR objective는
NVIDIA_NIM_API_KEY를 privileged central review job에만 전달하는 것입니다. 각 job 블록을 분리해 다음을 확인하세요.
- 키 바인딩은 의도한 review/model job에만 존재합니다.
- bootstrap, coverage, evidence job에는 키 바인딩이 없습니다.
- Strix의 provider-scoped 키 전달도 같은 방식으로 검증합니다.
Also applies to: 613-616
🤖 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 `@scripts/ci/test_strix_quick_gate.sh` around lines 318 - 344, Update the workflow assertions in scripts/ci/test_strix_quick_gate.sh to validate secret bindings within individual job blocks rather than only checking workflow-wide string presence. Use the existing workflow_file checks and job identifiers to confirm NVIDIA_NIM_API_KEY is bound only to the privileged review/model job, absent from bootstrap, coverage, and evidence jobs, and apply the same job-scoped validation to Strix provider-specific key material.
🧹 Nitpick comments (1)
scripts/ci/test_strix_quick_gate.sh (1)
1216-1217: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win공개 OpenCode 후보 목록에서 NIM 우선순위를 고정하세요.
Line 1216은 NIM 후보의 존재만 확인합니다. Line 1217도 후속 후보 목록만 확인합니다. 따라서 NIM이
opencode-free/...뒤로 이동하거나 public 조건 밖으로 이동해도 테스트가 통과합니다.공개 저장소 조건, NIM 후보,
opencode-free/후보의 순서를 하나의 assertion으로 검증하세요.제안된 assertion
+assert_file_contains "$workflow_file" "needs.validate-pr-metadata.outputs.is_private == 'false' && 'nvidia-nim/nvidia/nemotron-3-ultra-550b-a55b opencode-free/" "opencode review starts public repositories with NVIDIA NIM before anonymous hosted candidates"🤖 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 `@scripts/ci/test_strix_quick_gate.sh` around lines 1216 - 1217, Update the assertions for workflow_file so one assertion validates the public repository condition and the candidate ordering together: the NVIDIA NIM candidate must appear before any opencode-free/ candidates. Preserve the existing candidate checks while ensuring the test fails if NIM is moved behind opencode-free or outside the public condition.
🤖 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/ci/test_strix_quick_gate.sh`:
- Around line 17-18: Validate STRIX_TEST_PROCESS_TIMEOUT_SECONDS and
STRIX_TEST_FAKE_SLEEP_SECONDS as positive integers, then enforce fake sleep to
exceed the process timeout so timeout and cleanup scenarios genuinely exercise
expiration. Update the background child setup in the relevant test flow,
including the line-7901 path, to use the validated fake-sleep setting or a
derived value longer than the process timeout instead of a fixed 30-second
delay.
---
Outside diff comments:
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 318-344: Update the workflow assertions in
scripts/ci/test_strix_quick_gate.sh to validate secret bindings within
individual job blocks rather than only checking workflow-wide string presence.
Use the existing workflow_file checks and job identifiers to confirm
NVIDIA_NIM_API_KEY is bound only to the privileged review/model job, absent from
bootstrap, coverage, and evidence jobs, and apply the same job-scoped validation
to Strix provider-specific key material.
---
Nitpick comments:
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 1216-1217: Update the assertions for workflow_file so one
assertion validates the public repository condition and the candidate ordering
together: the NVIDIA NIM candidate must appear before any opencode-free/
candidates. Preserve the existing candidate checks while ensuring the test fails
if NIM is moved behind opencode-free or outside the public condition.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 3428527c-87cc-4863-94dc-a3abe68ddb8e
📒 Files selected for processing (5)
.github/workflows/opencode-review-dispatch.yml.github/workflows/strix.ymlscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_required_workflow_queue_contract.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/test_opencode_agent_contract.py
- tests/test_required_workflow_queue_contract.py
- .github/workflows/opencode-review-dispatch.yml
- .github/workflows/strix.yml
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
c9649649f9d8b3239cc7575bec52f9223202b865. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- scan-pr-queue check run: cancelled (https://github.com/ContextualWisdomLab/.github/actions/runs/30619157025/job/91119303743)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow (3 files)"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (3 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script (3 files)"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (3 files)"]
R3 --> V3["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path.
Findings
1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
- Problem: GitHub reports mergeStateStatus
DIRTYfor this pull request. - Root cause: Branch
fix/nvidia-nim-providercannot be merged cleanly intomain; the changed-file flow below shows which review/runtime path is blocked by the conflict. - Fix: Merge or rebase the latest
mainintofix/nvidia-nim-provider, resolve conflict markers in the PR branch, rerun the focused checks, and push the same branch. - Repair commands:
gh pr checkout 672 --repo ContextualWisdomLab/.github
git fetch origin main
git merge --no-ff origin/main # or: git rebase origin/main
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:fix/nvidia-nim-provider
# rebase path only: git push --force-with-lease origin HEAD:fix/nvidia-nim-provider- Regression test: Keep OpenCode approval gated on mergeability so model-output failures cannot approve a conflicted PR.
Merge Conflict Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (4 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
- Result: REQUEST_CHANGES
- Reason: mergeStateStatus is
DIRTY; mergeable isCONFLICTING. - Head SHA:
5dc981ba9352beaee530b665f7eb5b4cfdf611a7 - Workflow run: 30626187332
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (3 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["CI script (4 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (3 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 10 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 @.github/workflows/opencode-review-dispatch.yml:
- Line 6274: Update both failed-check filtering expressions at the visible jq
select conditions to exclude the scan-pr-queue check for every failure or
cancellation conclusion, not only cancelled results. Keep other check names and
existing evidence filtering unchanged, and add or update tests in
test_opencode_agent_contract.py covering both cancelled and failed conclusions.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 132367e8-c27f-4f5b-b690-66ea370f1edd
📒 Files selected for processing (10)
.github/workflows/noema-review.yml.github/workflows/opencode-review-dispatch.yml.github/workflows/strix.ymlscripts/ci/collect_failed_check_evidence.shscripts/ci/emit_opencode_failed_check_fallback_findings.shscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.pytests/test_required_workflow_queue_contract.py
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
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 @.github/workflows/opencode-review-dispatch.yml:
- Line 4062: Limit the combined runtime of the nvidia-nim/* candidates in the
OPENCODE_MODEL_CANDIDATES configuration so they cannot consume the entire
OPENCODE_TOTAL_RETRY_BUDGET_SECONDS; reduce their per-candidate
OPENCODE_FREE_RUN_TIMEOUT_SECONDS or reserve budget for the existing fallback
candidates. Update tests/test_opencode_model_pool_runner.py to verify that
fallbacks still execute after NIM candidates time out.
In `@tests/test_opencode_model_pool_runner.py`:
- Around line 799-809: Update test_nvidia_nim_candidate_requires_key and its
run_failed_model setup to remove NVIDIA_NIM_API_KEY and NVIDIA_API_KEY from the
inherited environment, then invoke the helper with an extra_env containing only
the explicitly intended values so the candidate is consistently skipped when no
scoped credential is configured.
🪄 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: CHILL
Plan: Pro Plus
Run ID: e2435e51-097b-47f4-acda-152dbb2f151b
📒 Files selected for processing (10)
.github/workflows/noema-review.yml.github/workflows/opencode-review-dispatch.yml.github/workflows/strix.ymlscripts/ci/collect_failed_check_evidence.shscripts/ci/emit_opencode_failed_check_fallback_findings.shscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.pytests/test_required_workflow_queue_contract.py
|
@coderabbitai resume @coderabbitai full review |
|
✅ Action performedReviews resumed. Full review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 18 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 31 seconds. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_opencode_agent_contract.py (1)
1645-1655: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
fixtures블록이 실제 jq 필터를 실행하지 않습니다.
retained계산은 순수 Python 리스트 컴프리헨션으로scan-pr-queue이름만 비교합니다. 이 코드는 워크플로우의 실제select((.name // "") != "scan-pr-queue")jq 표현식을 호출하지 않습니다. 워크플로우의 jq 로직에 회귀가 발생해도 이 assert는 항상 통과합니다. 위에 있는 문자열 존재 검증(Line 1629-1643)과 실질적으로 같은 내용을 재확인할 뿐입니다.이 fixture 검증을 의미 있게 만들려면, 워크플로우에서 추출한 실제 필터 표현식을
jq로 직접 실행해fixtures를 필터링하고 그 결과를 비교하세요.🤖 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/test_opencode_agent_contract.py` around lines 1645 - 1655, Replace the Python list comprehension in the fixture validation with direct execution of the workflow’s extracted jq filter, including select((.name // "") != "scan-pr-queue"), against fixtures. Compare the jq output with the expected real-peer-check entry so the test exercises the actual filtering logic rather than duplicating it in Python.
🤖 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 @.github/workflows/opencode-review-dispatch.yml:
- Around line 4051-4052: Update the model-priority comment near the candidate
list to match the actual ordering: state that NVIDIA NIM is attempted first when
its scoped secret is available, followed by opencode-free models and the
existing fallback candidates. Keep the candidate ordering unchanged.
- Line 4041: Restrict the nvidia-nim provider’s NVIDIA_API_KEY environment
variable to secrets.NVIDIA_NIM_API_KEY only, removing the legacy NVIDIA_API_KEY
fallback in the workflow job. Update tests in test_opencode_agent_contract.py to
verify NIM candidates are excluded when the scoped secret is absent and that the
legacy secret cannot activate NIM.
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 1090-1095: Replace the undefined fail call in the scheduler
self-check filter count validation with the existing record_failure function,
preserving the current failure message and threshold condition. Update the
relevant validation in the test script without changing the surrounding
assertions.
---
Nitpick comments:
In `@tests/test_opencode_agent_contract.py`:
- Around line 1645-1655: Replace the Python list comprehension in the fixture
validation with direct execution of the workflow’s extracted jq filter,
including select((.name // "") != "scan-pr-queue"), against fixtures. Compare
the jq output with the expected real-peer-check entry so the test exercises the
actual filtering logic rather than duplicating it in Python.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 26f0a38c-fd09-48ee-aa2a-76e9f146f153
📒 Files selected for processing (10)
.github/workflows/noema-review.yml.github/workflows/opencode-review-dispatch.yml.github/workflows/strix.ymlscripts/ci/collect_failed_check_evidence.shscripts/ci/emit_opencode_failed_check_fallback_findings.shscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.pytests/test_required_workflow_queue_contract.py
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/test_opencode_agent_contract.py`:
- Around line 201-205: Update the workflow assertions in the relevant contract
test to parse or isolate each job’s step-level env blocks, then verify both
NVIDIA_NIM_API_KEY bindings occur within the privileged central review job.
Reject direct NVIDIA_NIM_API_KEY or fallback NVIDIA_NIM_API_KEY/NVIDIA_API_KEY
references in every other job, while preserving the existing prohibition on the
fallback expression.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4c46dfa6-b676-4d09-aa53-4a19e7128488
📒 Files selected for processing (5)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/test_opencode_model_pool_runner.py
- .github/workflows/opencode-review-dispatch.yml
- scripts/ci/run_opencode_review_model_pool.sh
- scripts/ci/test_strix_quick_gate.sh
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 7 seconds. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Summary
mainreview/scheduler contracts and make timeout regression tests deterministic on slow runnersValidation
python3 -m pytest -q— 744 passed in 125.99s on current head8b58fa0after the NIM fallback-budget fixeszero-findings-timeout-all-models,slow-timeout, andtimeout-cleanupall exited 0bash -n, actionlint expression validation, Ruff, CodeGraph sync/exploration, and diff checks passed; ShellCheck reported only five pre-existing SC2016 literal-matching notes outside the changed linesNVIDIA_NIM_API_KEYvisibility confirmed without reading its valueCloses #671
Unblocks ContextualWisdomLab/saju-caldav#15
Summary by CodeRabbit
새로운 기능
개선 사항
scan-pr-queue검사를 실패 및 승인 대기 대상에서 제외합니다.scan-pr-queuefor every conclusion; focused contract, YAML structure, bash syntax, Ruff, diff, and CodeGraph checks passedrecord_failure, and the regression test executes the extracted jq filter directly; focused tests, filtered Strix, full 744-test pytest, Actionlint, Ruff, and syntax checks passed