Uh oh!
There was an error while loading. Please reload this page.
fix(strix): correct STRIX_FALLBACK_MODELS provider prefix (hyphen -> underscore) - #1285
fix(strix): correct STRIX_FALLBACK_MODELS provider prefix (hyphen -> underscore)#1285seonghobae wants to merge 2 commits into
Conversation
…underscore) The 2026-08-22 fix that replaced the retired GitHub Models universal fallback with a direct-OpenAI fallback used the hyphenated openai-direct/gpt-5.6-luna alias in STRIX_FALLBACK_MODELS. But child_model_for_api_base() in scripts/ci/strix_quick_gate.sh only recognizes the underscore openai_direct/gpt-5.6-luna form when translating a fallback candidate to the real litellm provider name (openai/gpt-5.6-luna). normalize_model() in strix_model_utils.sh does no hyphen/underscore normalization, so the hyphenated alias reaches litellm completely unmodified on every fallback attempt and fails immediately with "litellm.BadRequestError: LLM Provider NOT provided", independent of quota/capacity state. Root-caused via static reading of child_model_for_api_base() and normalize_model() (confirmed: an unrecognized case falls through to the catch-all pass-through, no translation happens anywhere else in that path). Corrected all four STRIX_FALLBACK_MODELS branches (github_models, openai_direct, openrouter, nvidia_nim) to match the underscore form already used correctly by the primary-model translation case statement and every other openai_direct/* reference in this workflow. Updated the three tests pinning the old (buggy) literal string and added regression guards rejecting the hyphenated form outright. Evidence trail: #624 (comment 2026-08-22/23 "Secondary, distinct bug"), LineageWeave #355/#258, .github#1246. Does not touch or resolve the separate NVIDIA NIM/OpenRouter/OpenAI billing/quota exhaustion also documented in #624 -- that remains a capacity problem requiring an org-billing action. This fix only ensures the openai_direct fallback slot is reachable at all once capacity exists, and unblocks the small subset of scans whose primary model fails but do NOT depend on quota (e.g. transient/schema-shape provider failures already fixed in #1246). Validation: - python3 -m unittest tests.test_strix_nvidia_nim_not_found_fallback -v: 15/15 passed - scripts/ci/strix_required_workflow_smoke.sh: passed - bash -n on all touched shell scripts: clean - scripts/ci/test_strix_quick_gate.sh (full suite, ~12k lines): no FAIL: lines in the portion completed locally under heavy machine contention before the harness's own timeout; CI's dedicated runner will give the authoritative full-suite signal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01491gBgBW98rZ7dm2fiTwF3
Warning Review limit reachedNext included review available in 31 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: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
seonghobae
commented
Aug 24, 2026
Confirmed the `strix` check's own failure here is expected, structural friction — not a problem with this fix: The "Self-test Strix required workflow contract" step runs `TRUSTED_STRIX_REQUIRED_SMOKE` (`strix_required_workflow_smoke.sh`) from a frozen trusted copy (`trusted-strix-source`, pinned to protected main) against this PR's proposed `strix.yml`. That's a deliberate security boundary for `pull_request_target`: the executable CI logic can't be sourced from an untrusted PR branch. Since this PR updates `strix.yml` (hyphen→underscore) and `strix_required_workflow_smoke.sh`'s matching assertion together, the self-test necessarily compares my fixed workflow against main's still-unfixed trusted smoke test and fails on the very string this PR corrects: `FAIL: Strix tries another NVIDIA hosted model before falling back to direct OpenAI (missing 'nvidia_nim/nvidia/llama-3.3-nemotron-super-49b-v1.5 openai-direct/gpt-5.6-luna')` This is unavoidable for any PR that must change both the workflow and its matching trusted-smoke-test assertion in the same change — the two can't be in sync until merge makes them the new trusted baseline together. Same class of self-referential friction already noted on `.github#1246`'s own gate. Nothing further to fix on this PR's side; flagging so review isn't blocked on a misread of this specific check. |
seonghobae
commented
Aug 24, 2026
Transferred valid finding from duplicate PR #1284 for canonical follow-up: when the primary provider is NVIDIA NIM or OpenRouter and the fallback selects |
seonghobae
commented
Aug 24, 2026
Closing as superseded by the canonical Strix owner PR #1263. Exact head |
seonghobae
commented
Aug 24, 2026
Post-race revalidation of final head |
| if [[ "$candidate" == openai_direct/* ]] && [[ "$PRIMARY_MODEL" != openai_direct/* ]] && | ||
| { [ -z "$STRIX_OPENAI_API_KEY" ] || [ -z "$STRIX_OPENAI_API_BASE_FILE" ]; }; then | ||
| echo "Skipping direct-OpenAI fallback '$candidate' because its dedicated key and endpoint are unavailable; primary provider credentials will not be reused." >&2 | ||
| continue | ||
| fi |
There was a problem hiding this comment.
🟡 Wrong reason logged when the only fallback is skipped for missing OpenAI credentials
When the sole fallback is a cross-provider openai_direct/* model and the dedicated OpenAI key or endpoint is missing, the new skip continues without marking any fallback as tried. Execution then reaches the no-fallback-tried branch, which counts the skipped candidate as configured and logs "All configured fallback models are the same as the primary model" — untrue, since the candidate was distinct and only skipped for missing credentials. The scan still fails closed; only the operator-facing reason is wrong.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo "ERROR: GitHub Models Strix scans require LLM_API_BASE_FILE to select the GitHub Models inference endpoint." >&2 | ||
| return 2 | ||
| fi | ||
| if [[ "$model" == openai_direct/* ]] && [ "$PRIMARY_MODEL" != "$model" ] && [ -n "$LLM_API_BASE_FILE" ]; then | ||
| echo "ERROR: Cross-provider direct-OpenAI Strix fallbacks require STRIX_OPENAI_API_BASE_FILE." >&2 | ||
| return 2 | ||
| fi |
There was a problem hiding this comment.
📝 Info: Cross-provider direct-OpenAI isolation is consistent
Key selection (strix_quick_gate.sh), endpoint selection (:2415-2420), and the loop skip (:4229-4233) agree: a cross-provider openai_direct/* fallback uses the dedicated OpenAI key and https://api.openai.com/v1, never the primary provider's credential/endpoint, and is skipped when that pair is absent. The error branch at :2434-2437 is unreachable (the loop skip pre-empts the combination) but harmless.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| 2026-08-24 KST provider-string wiring bug: the `STRIX_FALLBACK_MODELS` | ||
| fallback wired on 2026-08-22 used the hyphenated `openai-direct/gpt-5.6-luna` | ||
| alias, but `child_model_for_api_base()` in `scripts/ci/strix_quick_gate.sh` | ||
| only recognizes the underscore `openai_direct/gpt-5.6-luna` form when | ||
| translating a fallback candidate to the real litellm provider name | ||
| (`openai/gpt-5.6-luna`). The hyphenated alias therefore reached litellm | ||
| unmodified on every fallback attempt and failed immediately with | ||
| `litellm.BadRequestError: LLM Provider NOT provided`, independent of any | ||
| quota/capacity state -- confirmed org-wide across ContextualWisdomLab/.github | ||
| issue #624's evidence trail (`.github#1246`, LineageWeave #355/#258). Root- | ||
| caused via static reading of `child_model_for_api_base()` and | ||
| `normalize_model()` in `scripts/ci/strix_model_utils.sh` (no hyphen/underscore | ||
| normalization happens anywhere in that path, so an unrecognized case falls | ||
| through to the catch-all `printf '%s\n' "$model"`, passing the string through | ||
| unchanged). Fix: corrected all four `STRIX_FALLBACK_MODELS` branches | ||
| (`github_models`, `openai_direct`, `openrouter`, `nvidia_nim`) to the | ||
| underscore form already used correctly by the primary-model translation case | ||
| statement (`.github/workflows/strix.yml`'s "Prepare Strix model input file" | ||
| step) and by every other `openai_direct/*` reference in this workflow. | ||
| Updated the three tests pinning the old (buggy) literal string | ||
| (`scripts/ci/test_strix_quick_gate.sh`, `strix_required_workflow_smoke.sh`, | ||
| `tests/test_strix_nvidia_nim_not_found_fallback.py`) and added a regression | ||
| guard rejecting the hyphenated form outright. Does not touch, and does not | ||
| resolve, the separate NVIDIA NIM/OpenRouter/OpenAI billing/quota exhaustion | ||
| documented in `.github#624` -- that remains a capacity problem requiring an | ||
| org-billing action, not a wiring bug; this fix only ensures the | ||
| `openai_direct` fallback slot is reachable at all once capacity exists. | ||
| 2026-08-24 KST cross-provider credential and endpoint isolation: the corrected | ||
| `openai_direct/*` fallback still needed its own credential boundary when the | ||
| primary provider was NVIDIA NIM, OpenRouter, or GitHub Models. Without that | ||
| boundary, `run_strix_once()` would pass the primary `LLM_API_KEY` and | ||
| `LLM_API_BASE_FILE` into a direct-OpenAI attempt; this could turn a fallback | ||
| into a request authenticated against the wrong provider and would make a | ||
| successful security scan impossible to interpret. The workflow now materializes | ||
| `STRIX_OPENAI_API_KEY_FILE` and `STRIX_OPENAI_API_BASE_FILE` only for the | ||
| cross-provider branches, using the scoped `STRIX_OPENAI_API_KEY`/ | ||
| `OPENAI_API_KEY` secret and `https://api.openai.com/v1`. The gate selects these | ||
| inputs only for `openai_direct/*` candidates and skips that candidate when the | ||
| pair is unavailable, never reusing a primary-provider secret. Added a fake | ||
| Strix regression covering NVIDIA NIM → NVIDIA NIM → direct OpenAI routing and | ||
| asserting both credential and endpoint separation, plus static workflow and | ||
| trusted-input smoke assertions. This is a provider-boundary remediation; it | ||
| does not relax provider-signal fail-closed behavior or the required security | ||
| review gate. |
There was a problem hiding this comment.
🔍 Audit-file contract test not updated in this PR
Per AGENTS.md, tests/test_pr_governance_audit_contract.py pins exact strings and structure of PR_GOVERNANCE_AUDIT.md. This PR appends two dated entries but does not touch that test. Confirm the contract test still passes against the appended content.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
STRIX_FALLBACK_MODELS(wired 2026-08-22 to replace the retired GitHub Models universal fallback) used the hyphenatedopenai-direct/gpt-5.6-lunaalias.child_model_for_api_base()inscripts/ci/strix_quick_gate.shonly recognizes the underscoreopenai_direct/gpt-5.6-lunaform when translating a fallback candidate to the real litellm provider name (openai/gpt-5.6-luna) --normalize_model()does no hyphen/underscore normalization anywhere else in that path, so an unrecognized case falls through to the catch-all pass-through unchanged. The hyphenated alias therefore reached litellm on every fallback attempt and failed immediately withlitellm.BadRequestError: LLM Provider NOT provided, independent of quota/capacity state -- flagged but not fixed in.github#624's "Secondary, distinct bug" comment (2026-08-22/23).Root-caused via static reading of
child_model_for_api_base()andnormalize_model()(scripts/ci/strix_model_utils.sh) -- no guessing at the correct prefix; the underscore form is already the established, tested-correct convention used by the primary-model translation case statement in this same workflow and by every otheropenai_direct/*reference in it.Change
STRIX_FALLBACK_MODELSbranches (github_models,openai_direct,openrouter,nvidia_nim) to the underscore form.scripts/ci/test_strix_quick_gate.sh,scripts/ci/strix_required_workflow_smoke.sh,tests/test_strix_nvidia_nim_not_found_fallback.py.assert_file_not_contains ... "openai-direct/gpt-5.6-luna") in each so this exact bug class can't silently return.PR_GOVERNANCE_AUDIT.mddocumenting the fix (append-only, does not rewrite the 2026-08-22 historical entry).What this does NOT fix
The separate NVIDIA NIM/OpenRouter/OpenAI billing/quota exhaustion documented in
.github#624is a capacity problem requiring an org-billing action (top up OpenRouter credits or restore OpenAI quota) -- outside what a PR can resolve. This fix only ensures theopenai_directfallback slot is actually reachable once capacity exists, and should unblock the subset of scans whose primary model fails for non-quota reasons (e.g. the transient/schema-shape provider failures already fixed in.github#1246).Validation
python3 -m unittest tests.test_strix_nvidia_nim_not_found_fallback -v: 15/15 passedscripts/ci/strix_required_workflow_smoke.sh: passedbash -non all touched shell scripts: cleanscripts/ci/test_strix_quick_gate.sh(full ~12k-line suite): noFAIL:lines in the portion completed locally before hitting this environment's own resource contention (30+ concurrent unrelated processes on the same machine make the full run take 10+ minutes here); this PR's own CI run on a dedicated GitHub Actions runner will give the authoritative full-suite signal.Evidence trail:
ContextualWisdomLab/.github#624,LineageWeave#355,LineageWeave#258,.github#1246.🤖 Generated with Claude Code
https://claude.ai/code/session_01491gBgBW98rZ7dm2fiTwF3