fix(strix): preserve NIM fallback evidence - #1254
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Limit 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. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$STRIX_LOG" | | ||
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning)([^[:alpha:]]|$)'; then |
There was a problem hiding this comment.
🔴 Infrastructure-error guard can miss signals in large logs
Under the script's pipefail, grep -Eiq exits on the first match and can kill the upstream grep -Eiv with SIGPIPE, so the pipeline fails despite matching. An early warning in a large log then reads as absent, letting an rc==0 scan that should fail closed pass the gate.
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$STRIX_LOG" | | |
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning)([^[:alpha:]]|$)'; then | |
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$STRIX_LOG" | | |
| grep -Ei '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning)([^[:alpha:]]|$)' >/dev/null; then |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$report_log" | | ||
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)'; then |
There was a problem hiding this comment.
🔴 Report warning-signal check can miss large log artifacts
Under pipefail, grep -Eiq exits on first match and can kill the upstream grep -Eiv with SIGPIPE, so the pipeline fails despite matching. A Fatal/Warn/Timeout signal early in a large report log is then read as absent, and the artifact no longer fails the gate closed.
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$report_log" | | |
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)'; then | |
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$report_log" | | |
| grep -Ei '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)' >/dev/null; then |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$report_log" | | ||
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)'; then |
There was a problem hiding this comment.
🔍 Banner filter only matches the single title line
The exclusion regex matches only a whole line equal to the banner title plus box/padding. If the real NIM MODEL QUALITY WARNING banner spans multiple lines, any body line containing warning/quality still trips the Fatal/Warn detector and fails the scan closed. Safe, but may not fully tolerate clean NIM fallbacks as intended. Worth confirming the actual banner is a single line.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded by #1213 exact head 5edaf36, which preserves the complete NVIDIA fallback while filtering the exact clean model-quality advisory in both console and report evidence, retains all other warning/provider failures as fail-closed, and carries the full shell regression. Closing this duplicate without merge; reopen only if that scope is removed from #1213. |
| if grep -Eiv '^[[:space:]]*[^[:alnum:]]*[[:space:]]*MODEL QUALITY WARNING[[:space:]]*[^[:alnum:]]*[[:space:]]*$' "$report_log" | | ||
| grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)'; then |
There was a problem hiding this comment.
📝 Info: Filter regex correctly narrows to the exact heading
The anchored filter only exempts the bare MODEL QUALITY WARNING heading with box/space decoration. A line such as MODEL QUALITY WARNING: detail keeps alnum after WARNING, so it is not filtered and still fails closed. The exemption stays narrow as intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
MODEL QUALITY WARNINGheading.Validation
bash scripts/ci/test_strix_quick_gate.shbash scripts/ci/strix_required_workflow_smoke.shpython3 -m unittest tests.test_strix_nvidia_nim_not_found_fallbackactionlint .github/workflows/strix.ymlThis is the shared-gate repair for ContextualWisdomLab/contextual-orchestrator#803 and ContextualWisdomLab/contextual-orchestrator#824. Both remain blocked until this PR has passed required checks and independent review, reaches protected
main, and their exact heads receive fresh successful Strix checks.