fix(ci): exclude non-text-input models from review sidecar catalog - #1529
Conversation
contextual_orchestrator_review_launcher.py's model-selection loop only checked output modality (_has_text_output), never input modality, so a model whose input-modality evidence declares e.g. "image" -- NVIDIA NIM's meta/llama-3.2-90b-vision-instruct -- could still be admitted to the plain-text review catalog. The provider then rejects Strix's plain-text security-review request with a non-retryable HTTP 400, which Strix does not discover until it has already burned its ~2-hour job budget failing over through the rest of the pool (see #1415's strix job 99583819068). contextual-orchestrator itself already closed the equivalent gap in its own runtime selection path -- chat_capability.requires_non_text_input, whose docstring cites this exact incident class (#1198) -- but this repo's independently vendored catalog-builder never called it, and the sidecar's emitted agent rows carry no `input:` tag for orchestrator.py's own runtime `_is_general_free_agent` gate to catch either. Now calls requires_non_text_input(model.input_modalities) alongside the existing output-modality check. Verification: PYTHONPATH=. coverage run -m pytest tests -- 2126 passed, 1 skipped; coverage report -- 100%; interrogate -- 100%. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
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 |
|
Generated by Claude Code |
|
Exact-head failure disposition (fresh audit at
No source change, bypass, force push, or synthetic gate was used. This PR remains blocked until the owner harness repair integrates and fresh exact-head required checks plus independent review complete; the failed check is not being treated as passing evidence. |
|
Contextual-Orchestrator를 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. |
…hed job's own ceiling (#1532) Owner-authorized admin bypass past opencode-review: this PR's own diff edits .github/workflows/opencode-review.yml, the required-check pipeline it depends on, and the pre-fix budget (90m poll vs 325m worker job) makes it structurally impossible for this exact fix to pass its own check under the old configuration. Chicken-and-egg bypass explicitly authorized by the repo owner on .github#1500/#1503/#1506/#1527/#1529 today. Full verification (2127 passed, 100% coverage, 100% docstrings) in the PR body.
|
Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오. |
…clude-non-text-models # Conflicts: # CHANGELOG.md
|
Resolved a real merge conflict against current
Head is fresh now, so the required Generated by Claude Code |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review Fresh exact-head review request for |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review exact current head |
|
@opencode-agent review exact head |
|
Fresh current-main revalidation after #1546: protected @opencode-agent fix and review |
|
Current-main convergence is still incomplete: compare against protected |
|
Stack-order correction after reconciling all active launcher owners: use #1476 → #1474 → this #1529. #1476 owns OpenRouter admission/spend evidence; #1474 then owns outage-domain cap/diversity; this PR should be the final child adding only |
|
Current effective diff still includes |
|
Stack normalized onto #1567's current coverage-owner branch. The head is not yet a descendant of that base: the diff still carries the two scheduler-coverage test files. Merge exact #1567 head into this existing branch normally (no force/rebase), then ensure base→head is limited to the review-catalog input-modality fix ( |
|
Parent stack advanced: #1567 is now exact head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
CHANGELOG.md— repository behaviorscripts/ci/contextual_orchestrator_review_launcher.py— review and security gate shell pathtests/test_contextual_orchestrator_review_sidecar_contract.py— regression suitetests/test_pr_review_fix_scheduler.py— regression suitetests/test_pr_review_fix_scheduler_rest_workflow_identity.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["CI script: contextual_orchestrator_review_launcher.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: contextual_orchestrator_review_launcher.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_contextual_orchestrator_review_sidecar_contract.py (3 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_contextual_orchestrator_review_sidecar_contract.py (3 files)"]
R3 --> V3["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
c352014a163d2555ada6150a2452bdfbebb71ce6 - Workflow run: 33969161561
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["required checks"]
Evidence --> S2["CI script: contextual_orchestrator_review_launcher.py"]
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: test_contextual_orchestrator_review_sidecar_contract.py (3 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
…ree cannot materialize (#1973)" (#1985) This reverts commit ad0779b. I wrote that guard and its justification is false. It blocked every OpenCode dispatch on a DIRTY/CONFLICTING head on the grounds that "a conflicting head can only produce a failed dispatch". The run object does conclude failure, but the review is published anyway: the reviewer reads the pull request diff, not a merge tree -- "Coverage is a separate gate", in the review's own words -- and only `coverage-source-tree` needs the merge commit. Measured on .github#1529, the PR that motivated the guard: the last of its 27 dispatches published a 2404-character review at 2026-09-05T19:40:58Z. That review body carries its own run id, 33969161561, which concluded failure at the receipt gate nine seconds later. It is the only OpenCode review that head has (4 reviews total, 1 by opencode-agent[bot]) and it is why the PR now reads as reviewed. The guard would have discarded it. The pattern is not unique to #1529: .github#1555 is CONFLICTING right now and its current head carries a 2019-character CHANGES_REQUESTED review from the same reviewer. I reached "produced nothing" by reading run conclusions, which are roll-ups that cannot name what a run did. The cost the guard claimed to protect is also wrong by two orders of magnitude. Across all 27 dispatches of that head, 109 jobs: 27 were allocated a runner for 0.24 h in total, 82 never were, and the rest of the elapsed time was queue residency. Blocking the whole chain would have saved fourteen minutes of runner time. The repeat itself is a real problem, and it is fixed at its cause rather than here. `active_review_run_refs` matched a run's `name` exactly against the review workflow aliases, but the central review workflows define `run-name:`, so GitHub sends the rendered title in that field and every dispatch run was filtered out before the check could see it -- `already_running` never fired. With that repaired, a conflicting head receives one dispatch and the next is suppressed while it runs, which is the outcome this guard was reaching for without discarding the review. Developer experience: the scheduler no longer prints a skip reason that asserts an outcome contradicted by the runs it cites. User experience: a conflicting pull request receives a review and repair guidance, instead of repair guidance alone. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
seonghobae
added
bug
ChatGPT Codex Connector
2 participants
Root cause
scripts/ci/contextual_orchestrator_review_launcher.py's model-selection loop (inmain()) only checked output modality via_has_text_output, never input modality. A model whose input-modality catalog evidence declares something other thantext(e.g. NVIDIA NIM'smeta/llama-3.2-90b-vision-instruct, which declaresimageinput) can still emit text output and so passed the existing filter, letting it into the plain-text review catalog served ascontextual-orchestrator/orchestrator/free.Strix sends a plain-text security-review prompt with no image content. NVIDIA's NIM endpoint for this specific vision-instruct model rejected that request with a non-retryable
HTTP 400 invalid_request_error. Because the gateway only discovers this at request time (not at catalog-build time), Strix burned essentially its entire ~2-hour job budget failing over through the rest of the candidate pool before failing closed — see.github#1415'sstrixjob99583819068:contextual-orchestratoritself already fixed the equivalent gap in its own runtime selection path:chat_capability.requires_non_text_input(input_modalities), whose docstring explicitly cites this exact incident class (ContextualWisdomLab/.github#1198) and is the shared choke point for bothmodel_discovery._requires_non_text_inputandorchestrator.TaskOrchestrator._agent_requires_non_text_input. But this repo'scontextual_orchestrator_review_launcher.pyindependently reimplements catalog-building rather than reusing that fixed selector, and never called it. Worse, the sidecar's own emitted agent rows (contextual_orchestrator_review_policy.build_zdr_prioritized_catalog) carry only["review", "cost:*", "zdr"/"non-zdr"]tags — noinput:<modality>tag — so evenorchestrator.py's own runtime_is_general_free_agentgate has nothing to read for this independently-built pool. The bug is structurally isolated to this repo's vendored catalog-builder duplicating logic that was already fixed upstream.Fix
Import and call
contextual_orchestrator.chat_capability.requires_non_text_input(model.input_modalities)alongside the existing_has_text_outputoutput-modality check, excluding any model with declared non-text input evidence from the catalog before it's ever built.Verification
PYTHONPATH=. coverage run -m pytest tests: 2126 passed, 1 skipped (unchanged pass count plus updated assertions intest_contextual_orchestrator_review_sidecar_contract.py).coverage report --show-missing: 100%.interrogate: 100%.python3 -m pytest tests/test_contextual_orchestrator_review_sidecar_contract.py -q— 28 passed.Developer experience
Prevents a class of NVIDIA NIM vision-model 400s from silently consuming Strix's/Noema's/OpenCode review's full multi-hour job budget before failing closed; the model is now excluded before the catalog is ever served.
User experience
No user-facing change — CI-only fix.
Refs:
.github#1415(where this recurrence was found),.github#1198(the original incident this class of bug was first fixed for, incontextual-orchestrator's own runtime path).Generated by Claude Code