[ci] Added LLM based transient error re-run mechanism#663
Conversation
Added LLM based transient error re-run mechanism and kept it optional with current implementation.
📝 WalkthroughWalkthroughAdds an LLM-driven CI retry classifier to the CI failure analyzer. Two functions were introduced: Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant AF as analyze_failure.py
participant Gemini as Gemini API
participant FS as File System
GH->>AF: Invoke analyze_failure.py with logs & env
AF->>AF: Read CI_RETRY_MODE and GEMINI_MODEL
alt CI_RETRY_MODE == "llm" or "both"
AF->>Gemini: _should_retry_ci(error_log, model, tag_id)
Gemini-->>AF: Short reply (first line)
AF->>AF: _parse_retry_decision(first_line)
end
AF->>AF: Compute transient_only detection
AF->>AF: Combine decisions per retry_mode -> should_retry
alt should_retry == true
AF->>FS: Write "transient_failure" marker
FS-->>AF: Marker created
else
AF->>AF: Do not write marker
end
AF->>Gemini: Generate full analysis report (GEMINI_MODEL)
Gemini-->>AF: Full analysis
AF-->>GH: Print analysis and retry outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
coderabbitai
Bot
added
enhancement
github_actions
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The PR introduces a well-designed LLM-based retry classification system with three configurable modes:
Security considerations are properly handled with unique XML-like tags for prompt injection protection. Error handling is comprehensive with graceful fallbacks. Test coverage is thorough with new tests for the retry logic and updates to existing tests. Incremental Updates:
Files Reviewed (4 files)
Reviewed by kimi-k2.5 · 157,970 tokens |
Test Failures in CIHello @stktyagi,
To fix this, review the logic within the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
.github/actions/bot-ci-failure/test_analyze_failure.py (1)
777-803:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest will fail: mode mismatch between assertion and mock setup.
This test expects the marker file to be created when
transient_only=True, but:
- No
CI_RETRY_MODEis set, so it defaults to"llm"- In
"llm"mode,should_retrydepends only on_should_retry_ci()(lines 404-405 in main)mock_retry.return_value = False, soshould_retry = False- The marker file won't be created
This contradicts the test at line 848-864 (
test_retry_mode_llm_no_retry_when_llm_says_no) which correctly assertsmock_file.assert_not_called()for the same scenario.🐛 Proposed fix: set CI_RETRY_MODE to preserve old behavior
`@patch.dict`( os.environ, - {"GEMINI_API_KEY": "fake_key", "PR_AUTHOR": "test", "COMMIT_SHA": "abc"}, + {"GEMINI_API_KEY": "fake_key", "PR_AUTHOR": "test", "COMMIT_SHA": "abc", "CI_RETRY_MODE": "transient_only"}, ) def test_transient_failure_creates_marker_file( self, mock_retry, mock_repo, mock_logs, mock_genai, mock_print, mock_file ): mock_logs.return_value = ("marionette error", False, True) - mock_retry.return_value = False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-ci-failure/test_analyze_failure.py around lines 777 - 803, Test fails because CI_RETRY_MODE defaults to "llm" so main() uses _should_retry_ci() (mocked False) and no marker file is written; fix by ensuring the test forces the retry mode to transient-only: update the `@patch.dict` for environment in test_transient_failure_creates_marker_file to include "CI_RETRY_MODE": "transient-only" (or alternatively make mock_retry return True) so main() computes should_retry as True and the "transient_failure" marker is created when calling main() (references: test_transient_failure_creates_marker_file, main, _should_retry_ci, CI_RETRY_MODE).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 303-311: The _parse_retry_decision function can IndexError when
text is whitespace-only because text.strip().splitlines() can be empty; fix by
first computing stripped = text.strip() and returning None if not stripped (or
checking splitlines length) before accessing [0], then proceed to derive first =
stripped.splitlines()[0].strip().upper() and the existing startswith checks to
return True/False/None.
---
Outside diff comments:
In @.github/actions/bot-ci-failure/test_analyze_failure.py:
- Around line 777-803: Test fails because CI_RETRY_MODE defaults to "llm" so
main() uses _should_retry_ci() (mocked False) and no marker file is written; fix
by ensuring the test forces the retry mode to transient-only: update the
`@patch.dict` for environment in test_transient_failure_creates_marker_file to
include "CI_RETRY_MODE": "transient-only" (or alternatively make mock_retry
return True) so main() computes should_retry as True and the "transient_failure"
marker is created when calling main() (references:
test_transient_failure_creates_marker_file, main, _should_retry_ci,
CI_RETRY_MODE).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: aba13696-af1e-43c7-a352-da4b6da86ec1
📒 Files selected for processing (2)
.github/actions/bot-ci-failure/analyze_failure.py.github/actions/bot-ci-failure/test_analyze_failure.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Kilo Code Review
🔇 Additional comments (6)
.github/actions/bot-ci-failure/analyze_failure.py (2)
314-363: LGTM!The retry classifier implementation is well-structured:
- Security tagging with random
tag_idto prevent prompt injection- Appropriate temperature (0.2) and token limit for deterministic yes/no responses
- Safe fallback to no-retry on errors or invalid output
400-415: LGTM!The retry mode logic is clean and covers the three documented modes appropriately:
llm: uses only LLM decisionboth:transient_only OR llmfor maximum retry coverage- fallback: uses existing
transient_onlylogicModel selection moved earlier for reuse across both retry classification and analysis is a good refactor.
.github/actions/bot-ci-failure/test_analyze_failure.py (4)
14-14: LGTM!Import added for the new
_parse_retry_decisionfunction being tested.
535-549: LGTM!Test coverage for
_parse_retry_decisionhandles the core cases. Consider adding a test for whitespace-only input (e.g.," ") once the IndexError bug is fixed.
576-604: LGTM!The
mock_retrypatches correctly isolate tests from the new_should_retry_cifunction. Settingmock_retry.return_value = Falseis appropriate for tests focused on other aspects ofmain().
805-897: LGTM!The new LLM retry mode tests correctly validate:
test_llm_retry_creates_marker_file: LLM says YES → marker createdtest_retry_mode_llm_no_retry_when_llm_says_no: LLM says NO → no marker (even withtransient_only=True)test_retry_mode_llm_uses_llm_decision: verifies LLM decision drives marker creationThese tests properly cover the new retry classification behavior.
Added CI_RETRY_MODE parameter to prevent test failure
Changed ref from master to testing branch
Added notices in ci to check what mode is working
Multiple Test Failures in
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/actions/bot-ci-failure/analyze_failure.py (1)
303-311:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard whitespace-only classifier output before indexing the first line.
" \n"passes theif not textcheck, then Line 306 and Line 358 both dosplitlines()[0]on an empty list. In the defaultllmpath that turns an invalid classifier response into an unhandledIndexErrorinstead of a safeFalse.Suggested fix
def _parse_retry_decision(text): if not text: return None - first = text.strip().splitlines()[0].strip().upper() + stripped = text.strip() + if not stripped: + return None + first = stripped.splitlines()[0].strip().upper() if first.startswith("YES"): return True if first.startswith("NO"): return False return None @@ - preview = raw_text.strip().splitlines()[0] if raw_text else "<empty>" + stripped = raw_text.strip() if raw_text else "" + preview = stripped.splitlines()[0] if stripped else "<empty>"Also applies to: 356-358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-ci-failure/analyze_failure.py around lines 303 - 311, The _parse_retry_decision function fails on whitespace-only text because it checks only `if not text` then immediately does `splitlines()[0]`; update it to guard for whitespace-only input by trimming and verifying there is at least one non-empty line before indexing (e.g., compute stripped = text.strip(); if not stripped: return None; then take first = stripped.splitlines()[0].upper() and proceed). Apply the same defensive check to the other identical classifier-handling block referenced around the second occurrence so both paths return None for blank/whitespace-only classifier output instead of raising IndexError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 406-417: The code reads CI_RETRY_MODE into retry_mode and branches
based on "llm"/"both"/other, but the workflow wiring doesn't export
CI_RETRY_MODE so the default "llm" is always used; update the reusable workflow
environment block to export CI_RETRY_MODE from a repository variable or input
(same place GEMINI_MODEL is exported) so the branch in retry_mode sees the
intended value; ensure the env key name matches exactly ("CI_RETRY_MODE") and
preserve the existing default behavior in analyze_failure.py by keeping
os.environ.get("CI_RETRY_MODE", "llm").
In @.github/workflows/bot-ci-failure.yml:
- Line 75: The reusable workflow reference is pinned to a feature branch in the
`uses:
openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@refactor/transient-rerun-mechanism`
line; update that reference to a stable ref (e.g., `@master` or a specific
commit SHA) so the CI bot always uses a post-merge, stable version instead of a
transient feature branch.
---
Duplicate comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 303-311: The _parse_retry_decision function fails on
whitespace-only text because it checks only `if not text` then immediately does
`splitlines()[0]`; update it to guard for whitespace-only input by trimming and
verifying there is at least one non-empty line before indexing (e.g., compute
stripped = text.strip(); if not stripped: return None; then take first =
stripped.splitlines()[0].upper() and proceed). Apply the same defensive check to
the other identical classifier-handling block referenced around the second
occurrence so both paths return None for blank/whitespace-only classifier output
instead of raising IndexError.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d2e0508d-8a06-4fa2-95db-341df4d2deee
📒 Files selected for processing (3)
.github/actions/bot-ci-failure/analyze_failure.py.github/actions/bot-ci-failure/test_analyze_failure.py.github/workflows/bot-ci-failure.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T16:40:12.455Z
Learnt from: stktyagi
Repo: openwisp/openwisp-utils PR: 631
File: .github/workflows/bot-ci-failure.yml:71-74
Timestamp: 2026-03-24T16:40:12.455Z
Learning: When reviewing .github/workflows/bot-ci-failure.yml, treat the reusable-bot-ci-failure.yml auto-retry step as authenticated via a custom GitHub App token passed through GH_TOKEN (generated from APP_ID and PRIVATE_KEY secrets and used by the gh CLI). Because this uses the App token (not the workflow’s GITHUB_TOKEN), workflow permissions settings (the `permissions` block, which only restricts GITHUB_TOKEN) do not apply to the `gh api -X POST` call. Therefore, do not flag the `gh` POST as a permissions problem if `actions: read` is set in the reusable workflow.
Applied to files:
.github/workflows/bot-ci-failure.yml
| actions: write | ||
| contents: read | ||
| uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@master | ||
| uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@refactor/transient-rerun-mechanism |
There was a problem hiding this comment.
Don't leave the reusable workflow pinned to a feature branch.
After this merges, master will still call reusable-bot-ci-failure.yml from refactor/transient-rerun-mechanism. If that branch is deleted or drifts, the CI failure bot on master starts running stale or broken logic. Point this back to the post-merge stable ref (or a commit SHA if you need a temporary pin during validation).
Suggested change
- uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@refactor/transient-rerun-mechanism
+ uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@master📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@refactor/transient-rerun-mechanism | |
| uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@master |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bot-ci-failure.yml at line 75, The reusable workflow
reference is pinned to a feature branch in the `uses:
openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@refactor/transient-rerun-mechanism`
line; update that reference to a stable ref (e.g., `@master` or a specific
commit SHA) so the CI bot always uses a post-merge, stable version instead of a
transient feature branch.
CI_RETRY_MODE was configurable in code but not reachable from the workflow wiring.
Test Failures in
|
Loosened print statement assertions to accomodate notices logging.
Changed ref to current branch for testing
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (/3). |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-ci-failure/test_analyze_failure.py:
- Around line 810-907: Add unit tests covering CI_RETRY_MODE="both": create two
tests similar to existing LLM tests that call main()—one where get_error_logs
returns a transient-only condition (transient_only=True) and assert that
open("transient_failure","w") was called and _should_retry_ci was never called
(use mock_retry.assert_not_called()), and another where transient_only=False and
mock_retry returns True and assert the marker file was created; reference the
functions/variables main(), _should_retry_ci, get_error_logs, CI_RETRY_MODE, and
ensure you patch analyze_failure._should_retry_ci and builtins.open as in the
other tests.
In @.github/workflows/reusable-bot-ci-failure.yml:
- Line 55: Change the hardcoded reusable workflow ref back to a stable branch
before merging: replace the string "ref: refactor/transient-rerun-mechanism"
with "ref: master" (or your repository's long-term default branch) in the
"Checkout Reusable Workflow" step so callers continue to resolve the ref after
this feature branch is deleted or diverges.
- Line 130: The workflow currently sets CI_RETRY_MODE to an empty string when
vars.CI_RETRY_MODE is unset, causing the Python logic that reads retry_mode (via
os.environ.get("CI_RETRY_MODE", "llm")) to treat it as "" and enable
transient-only mode; fix by providing a fallback in the workflow like using the
|| operator when setting CI_RETRY_MODE (so CI_RETRY_MODE: ${{ vars.CI_RETRY_MODE
|| 'llm' }}) or alternatively update the Python side to compute retry_mode as
(os.environ.get("CI_RETRY_MODE") or "llm").strip().lower() so an empty env var
won't override the intended default.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6c2db5d3-6c5f-42ed-83a0-b4c99bd34b82
📒 Files selected for processing (2)
.github/actions/bot-ci-failure/test_analyze_failure.py.github/workflows/reusable-bot-ci-failure.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Kilo Code Review
🔇 Additional comments (1)
.github/actions/bot-ci-failure/test_analyze_failure.py (1)
535-548: LGTM —TestParseRetryDecisioncovers the core cases.The four assertions cover the two valid tokens, case/whitespace normalisation, and an unexpected output. The
if not text: return Nonebranch (empty string /Noneinput) is not exercised, but those paths are trivial and the omission carries negligible risk.
| with: | ||
| repository: openwisp/openwisp-utils | ||
| ref: master | ||
| ref: refactor/transient-rerun-mechanism |
There was a problem hiding this comment.
Revert ref to master before merging — feature branch will break callers post-merge.
ref: refactor/transient-rerun-mechanism is the development branch used for testing this change. Once this PR is merged and the branch is deleted or diverges, every repository that invokes this reusable workflow will fail at the "Checkout Reusable Workflow" step because the ref no longer resolves.
🛠 Proposed fix
- ref: refactor/transient-rerun-mechanism
+ ref: master🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/reusable-bot-ci-failure.yml at line 55, Change the
hardcoded reusable workflow ref back to a stable branch before merging: replace
the string "ref: refactor/transient-rerun-mechanism" with "ref: master" (or your
repository's long-term default branch) in the "Checkout Reusable Workflow" step
so callers continue to resolve the ref after this feature branch is deleted or
diverges.
Forced default CI retry marker to llm
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
Looks good!
/3 issue
I have noticed again this:
It should be (1/3). Can you please fix this as well here?
_parse_retry_decision crashes on whitespace-only output
analyze_failure.py:303-311. The guard if not text: return None catches
empty strings but not whitespace, because "\n" and " " are truthy.
The next line then does text.strip().splitlines()[0], which raises
IndexError: list index out of range on any whitespace-only input.
>>> _parse_retry_decision("\n")
IndexError
>>> _parse_retry_decision(" ")
IndexError
>>> _parse_retry_decision(" \n ")
IndexError
The call site in _should_retry_ci (line 357) is outside the
try/except that wraps the API call, and main() does not wrap
_should_retry_ci either, so the exception propagates and crashes the
whole step. Gemini occasionally returns just a newline or empty token
under load, especially with max_output_tokens=5. With the default mode
now llm, this takes down the bot for every failure on that run.
Fix: change the early return to if not text or not text.strip() (or
guard the indexing with a length check), and add a test case for
whitespace-only input.
llm mode silently disables retry when the classifier API fails
analyze_failure.py:350-355 returns False on any classifier exception,
and analyze_failure.py:410-411 ignores transient_only entirely in
llm mode. Default mode is now llm (commit 83b1c05).
Concrete impact: when Gemini is rate-limited, returns 5xx, or replies
with an unparseable token, an obvious transient failure that the
deterministic heuristic already matches (selenium WebDriverException,
network unreachable, dependency install OSError, etc.) will not be
retried. This is a regression vs. the pre-PR behaviour, where the
heuristic was independent of Gemini availability. both mode does not
have this problem because the or short-circuits on transient_only.
Fix: in llm mode, fall back to transient_only when
_should_retry_ci returns False due to a classifier error. The
heuristic is free, and it is exactly the case where you want a
safety net.
CI_RETRY_MODE is undocumented
reusable-bot-ci-failure.yml:130 exposes the new variable but
docs/developer/reusable-github-utils.rst still describes only the
deterministic heuristic ("When the bot detects that all failures are
transient..."). Operators of other OpenWISP repos will not know:
- the default is now
llm, so enabling the bot on a new repo doubles
Gemini calls per failed run with no opt-out documented; - the recognised values are
llm,both, and any other value (the
elsebranch at line 416, including unset, empty, or typo) selects
heuristic-only; - in
bothmode the classifier call is skipped when the heuristic
already matched.
The CI Failure Bot section needs the variable, its values, and the
default spelled out.
| self.assertTrue(_parse_retry_decision(" yes \n")) | ||
|
|
||
| def test_unexpected_output(self): | ||
| self.assertIsNone(_parse_retry_decision("MAYBE")) |
There was a problem hiding this comment.
so what happens if the response is not YES or NO?
There was a problem hiding this comment.
in LLM mode that now falls back to the heuristic transient_only retry decision.
To prevent unnecessary re-run, hardened the prompt
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/actions/bot-ci-failure/analyze_failure.py (1)
303-306:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard both first-line extractions against whitespace-only model output.
A classifier response like
" \n"still crashes here:text.strip().splitlines()[0]in_parse_retry_decisionandraw_text.strip().splitlines()[0]in the preview path both hitIndexError. That bypasses the intended “invalid output => no retry” behavior and can abort the action on a benign malformed response.🐛 Proposed fix
def _parse_retry_decision(text): if not text: return None - first = text.strip().splitlines()[0].strip().upper() + stripped = text.strip() + if not stripped: + return None + first = stripped.splitlines()[0].strip().upper() if first.startswith("YES"): return True if first.startswith("NO"): return False return None @@ - raw_text = response.text if response else "" + raw_text = response.text if response else "" decision = _parse_retry_decision(raw_text) - preview = raw_text.strip().splitlines()[0] if raw_text else "<empty>" + stripped_response = raw_text.strip() if raw_text else "" + preview = stripped_response.splitlines()[0] if stripped_response else "<empty>"Also applies to: 356-358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-ci-failure/analyze_failure.py around lines 303 - 306, The _parse_retry_decision function (and the preview-path code that uses raw_text.strip().splitlines()[0]) can raise IndexError for whitespace-only model outputs; update both to guard before indexing by computing stripped = text.strip() (or raw_stripped = raw_text.strip()), return None if stripped is empty, then extract the first line via stripped.splitlines()[0]. This ensures whitespace-only responses are treated as invalid (no retry) without crashing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 323-330: The decision rule currently biases ambiguous cases toward
retry by instructing "If unsure, answer YES."; update the rule text in the
Decision rules block so the fallback is "If unsure, answer NO." and clarify that
YES is only for concrete transient indicators (network/DNS/service outages,
dependency download errors, browser crashes, flaky UI/selenium/webdriver issues)
to avoid unnecessary auto-reruns. Ensure the exact phrasing replacing "If
unsure, answer YES." is "If unsure, answer NO." and add a brief note that YES
should be reserved for logs with concrete transient signals.
---
Duplicate comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 303-306: The _parse_retry_decision function (and the preview-path
code that uses raw_text.strip().splitlines()[0]) can raise IndexError for
whitespace-only model outputs; update both to guard before indexing by computing
stripped = text.strip() (or raw_stripped = raw_text.strip()), return None if
stripped is empty, then extract the first line via stripped.splitlines()[0].
This ensures whitespace-only responses are treated as invalid (no retry) without
crashing.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: a71dfe88-b655-41fc-a94e-b0b0f98d7ba7
📒 Files selected for processing (1)
.github/actions/bot-ci-failure/analyze_failure.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Kilo Code Review
🔇 Additional comments (1)
.github/actions/bot-ci-failure/analyze_failure.py (1)
406-425: Mode selection and marker write path look good.Computing
should_retryonce and only writingtransient_failurebehind that gate keeps the workflow contract explicit and avoids mutating the marker on non-retry outcomes.
Added tests to increase coverage, added documentation and comments.
nemesifier
changed the title
2 participants
Added LLM based transient error re-run mechanism and kept it optional with current implementation.
Checklist
Description of Changes
Kept the LLM retry logic as the default and kept current implementation optional along with it. The LLM mode uses a strict yes/no prompt and only touches the retry marker, while the original analysis flow stays unchanged.