[feature] Add CI failure bot for automated responses #524 #562
Conversation
WalkthroughAdds a CI Failure Bot script (.github/scripts/ci_failure_bot.py) and a workflow (.github/workflows/ci-failure-bot.yml). The workflow runs when the "OpenWISP Utils CI Build" workflow_run fails (excluding dependabot), sets up Python, installs dependencies, and invokes the bot with GITHUB_TOKEN, GEMINI_API_KEY, WORKFLOW_RUN_ID, REPOSITORY, and PR_NUMBER. The bot fetches workflow run jobs/logs, collects failed job and step details, retrieves the workflow YAML and optional PR diff, sends a structured context to Gemini for analysis, and posts or updates a PR comment with the AI-derived or fallback response. Sequence Diagram(s)sequenceDiagram
autonumber
participant Actions as GitHub Actions
participant Bot as CI Failure Bot
participant GH as GitHub API
participant Gemini as Gemini API
participant PR as Pull Request
rect rgba(100,149,237,0.08)
Actions->>Bot: workflow_run completed (env: WORKFLOW_RUN_ID, REPOSITORY, PR_NUMBER, GITHUB_TOKEN, GEMINI_API_KEY)
end
Bot->>GH: GET /actions/runs/{id}/jobs
GH-->>Bot: jobs metadata
Bot->>GH: GET workflow run logs archive
GH-->>Bot: logs archive
Bot->>Bot: extract failed jobs & steps (truncate logs)
Bot->>GH: GET workflow YAML
GH-->>Bot: workflow YAML
alt PR_NUMBER provided
Bot->>GH: GET PR diff (title, body, diff)
GH-->>Bot: diff, title, body
end
rect rgba(46,204,113,0.06)
Bot->>Gemini: send structured context (logs, YAML, PR diff, repo info)
Gemini-->>Bot: analysis / remediation message
end
alt Gemini error or failure
Bot->>Bot: generate fallback response
end
alt PR_NUMBER present
Bot->>GH: POST or UPDATE comment on PR
GH-->>PR: comment created/updated
else no PR
Bot-->Actions: log result, exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Around line 109-125: The method generate_commit_message_response is dead code
(never called); either remove it or call it from run() where other validations
happen: locate the run() method and after fetching the commit message (e.g.,
commit_message variable or the payload field you already parse), call
self.generate_commit_message_response(commit_message) and use its return value
to post a comment or append to the bot response flow, or simply delete the
generate_commit_message_response function if you don't want commit message
validation now; ensure you refer to the exact symbol
generate_commit_message_response when removing or invoking it and update any
tests/uses accordingly.
- Around line 134-136: The code currently builds comment_body with an automatic
mention via the variables author and comment_body and always calls
pr.create_issue_comment, which can spam the PR author; update this by removing
the "@{author}" mention or making mentions optional via a config flag (e.g.,
mention_author) and by adding rate-limiting/deduplication before calling
pr.create_issue_comment: record the last_mention timestamp per PR (in-memory
cache or persistent store) and only include the mention or post a comment if the
last mention is older than a configured cooldown, or skip adding the mention
entirely and post a non-mention comment instead; ensure you reference and modify
the creation site that constructs comment_body and the call to
pr.create_issue_comment to enforce the new logic.
In @.github/workflows/ci-failure-bot.yml:
- Around line 28-34: The step setting PR_NUMBER reads
github.event.workflow_run.pull_requests[0].number without checking for an empty
array; update the env for PR_NUMBER in the "Run CI Failure Bot" step to a
conditional expression that yields the PR number when pull_requests exists and
has items, otherwise an empty string (use a GitHub Actions expression checking
github.event.workflow_run.pull_requests and its length before indexing). This
prevents failures for workflow runs that have no associated PR while keeping the
Python script behavior unchanged.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (3)
18-20: Improve error message specificity for missing environment variables.The current error message "Missing env vars" doesn't indicate which variables are missing, making debugging harder.
🔎 Proposed fix for more descriptive error message
if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: missing.append('GITHUB_TOKEN') + if not self.workflow_run_id: missing.append('WORKFLOW_RUN_ID') + if not self.repository_name: missing.append('REPOSITORY') + print(f"Missing required environment variables: {', '.join(missing)}") sys.exit(1)
25-43: Refine exception handling for better error visibility.The broad
Exceptioncatch on line 41 can mask specific API errors. Consider catchingGithubExceptionspecifically or re-raising after logging.Based on static analysis hints, catching blind exceptions makes debugging harder.
🔎 Proposed fix for more specific exception handling
+from github import GithubException + class CIFailureBot: # ... existing code ... def get_workflow_logs(self): try: workflow_run = self.repo.get_workflow_run(int(self.workflow_run_id)) jobs = workflow_run.jobs() failed_jobs = [] for job in jobs: if job.conclusion == 'failure': for step in job.steps: if step.conclusion == 'failure': failed_jobs.append({ 'job_name': job.name, 'step_name': step.name, 'step_number': step.number }) return failed_jobs - except Exception as e: + except GithubException as e: print(f"Error getting logs: {e}") return []
127-139: Validate PR number and improve exception handling.Two concerns:
- Line 128 only checks truthiness of
pr_number; it should validate it's a non-empty string that can be converted to an integer.- Line 138 catches blind
Exception, which can mask specific API errors.🔎 Proposed fix for validation and exception handling
+from github import GithubException + class CIFailureBot: # ... existing code ... def post_comment(self, message): - if not self.pr_number: + if not self.pr_number or self.pr_number.strip() == '': print("No PR number, skipping comment") return try: + pr_num = int(self.pr_number) - pr = self.repo.get_pull(int(self.pr_number)) + pr = self.repo.get_pull(pr_num) author = pr.user.login comment_body = f"@{author} {message}" pr.create_issue_comment(comment_body) - print(f"Posted comment to PR #{self.pr_number}") + print(f"Posted comment to PR #{pr_num}") - except Exception as e: + except (ValueError, GithubException) as e: print(f"Error posting comment: {e}").github/workflows/ci-failure-bot.yml (1)
24-26: Pin dependency versions for reproducibility and stability in CI workflows.Installing packages without version constraints means each workflow run could install different versions, potentially introducing breaking changes. Consider pinning to specific stable versions.
🔎 Proposed fix with current stable versions
- name: Install dependencies run: | - pip install requests PyGithub + pip install requests==2.32.5 PyGithub==2.8.1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
40-40: Consider moving this statement to an else block
(TRY300)
41-41: Do not catch blind exception: Exception
(BLE001)
138-138: Do not catch blind exception: Exception
(BLE001)
⏰ 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.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
🔇 Additional comments (2)
.github/workflows/ci-failure-bot.yml (1)
1-13: LGTM! Workflow trigger and condition are properly configured.The workflow correctly triggers on the completion of the "OpenWISP Utils CI Build" workflow and only runs when the conclusion is 'failure'. The setup is appropriate for this use case.
.github/scripts/ci_failure_bot.py (1)
141-183: LGTM! The orchestration flow is well-structured.The
run()method properly orchestrates the workflow: retrieves logs, analyzes failure types, generates appropriate responses, includes a sensible fallback for unrecognized failures, and posts the final comment. The main execution block follows standard Python conventions.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
4d35151 to
55627fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Line 9: The project imports github.Github in .github/scripts/ci_failure_bot.py
but PyGithub is not declared as a dependency; update the package manifest (e.g.,
setup.py) to add PyGithub (suggestion: 'PyGithub>=2.0.0') to the appropriate
extras_require/dev/ci dependency group so local installs and CI installs include
it, and ensure any CI/dev docs or requirements files reference the same version
constraint.
♻️ Duplicate comments (2)
.github/scripts/ci_failure_bot.py (2)
114-130: Unused method:generate_commit_message_responseis never called.This method is defined but not invoked anywhere in the
run()method. Either integrate commit message validation into the bot's flow or remove this dead code.Would you like me to suggest how to integrate commit message validation into the workflow, or would you prefer to remove this method for now?
139-141: Remove or make the automatic PR author mention optional and rate-limited.GitHub's automation policies explicitly forbid mass-mentioning and repeatedly pinging users. The current implementation automatically mentions the PR author in every CI failure comment (line 140), which could violate GitHub's anti-spam guidelines if failures occur frequently. Either remove the automatic @mention entirely or implement rate-limiting and deduplication to avoid repeated pings on the same PR.
🔎 Remove automatic mention
try: pr = self.repo.get_pull(int(self.pr_number)) - author = pr.user.login - comment_body = f"@{author} {message}" - pr.create_issue_comment(comment_body) + pr.create_issue_comment(message) print(f"Posted comment to PR #{self.pr_number}")
🧹 Nitpick comments (3)
.github/scripts/ci_failure_bot.py (3)
19-21: Improve error message to specify which environment variables are missing.The error message "Missing env vars" doesn't indicate which specific variables are missing, making debugging harder for users.
🔎 More descriptive error message
if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: + missing.append("GITHUB_TOKEN") + if not self.workflow_run_id: + missing.append("WORKFLOW_RUN_ID") + if not self.repository_name: + missing.append("REPOSITORY") + print(f"Missing required environment variables: {', '.join(missing)}") sys.exit(1)
44-46: Make exception handling more specific or enhance error logging.Catching
Exceptionis too broad and may hide important issues like authentication failures, network errors, or API rate limits. At minimum, log the exception type to aid debugging.🔎 Enhanced error logging
except Exception as e: - print(f"Error getting logs: {e}") + print(f"Error getting logs ({type(e).__name__}): {e}") return []Or, better yet, catch specific exceptions:
- except Exception as e: - print(f"Error getting logs: {e}") + except (GithubException, ValueError) as e: + print(f"Error getting workflow logs: {e}") return []Note: You'll need to import
GithubExceptionfromgithubif using the specific approach.
143-144: Enhance exception handling specificity.Similar to the
get_workflow_logsmethod, catching broadExceptioncan hide important API errors. Consider catching specific exceptions or improving error logging.🔎 More specific exception handling
- except Exception as e: - print(f"Error posting comment: {e}") + except (GithubException, ValueError) as e: + print(f"Error posting comment ({type(e).__name__}): {e}")Note: Import
GithubExceptionfromgithubif using this approach.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
43-43: Consider moving this statement to an else block
(TRY300)
44-44: Do not catch blind exception: Exception
(BLE001)
143-143: Do not catch blind exception: Exception
(BLE001)
⏰ 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.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- 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.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
48-63: LGTM! Failure classification logic is sound.The method correctly categorizes failures based on step names with case-insensitive matching and proper deduplication.
65-112: LGTM! Response templates provide clear, actionable guidance.All three response generators (
generate_qa_response,generate_test_response,generate_setup_response) offer specific commands and helpful troubleshooting steps with appropriate documentation links.
146-185: LGTM! Main orchestration logic is well-structured.The
run()method properly coordinates workflow log retrieval, failure analysis, response generation, and comment posting. The fallback to a generic message for unrecognized failures ensures users always receive guidance.
188-190: LGTM! Standard entry point pattern.The script entry point follows Python best practices.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
55627fb to
2baccea
Compare
|
Hi @nemesifier , Got the CI failure bot working! It catches QA failures, test issues, and setup problems, then posts helpful responses with the fix commands and links to contributing docs. Tested it on my own repo first like you suggested - triggers correctly when builds fail and generates the right responses. This covers the first part i mentioned. Let me know how it looks and I can work on the auto-assignment and stalled PR stuff next. |
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Around line 114-124: post_comment currently always creates a new PR comment
which causes duplicates; modify post_comment to fetch existing PR comments via
pr.get_issue_comments(), determine the bot's username via
self.github.get_user().login (or self.github.login), scan comments for ones
authored by the bot and matching the failure signature (e.g., a stable substring
from message like "CI Build Failed" or exact message match), and if found either
update that comment with comment.edit(message) or skip creating a new one; only
call pr.create_issue_comment(message) when no matching bot comment exists and
keep existing try/except behavior for errors.
- Around line 26-46: The code calls int(self.workflow_run_id) in
get_workflow_logs which can raise ValueError if the env var is non-numeric; add
validation in the class __init__ (where self.workflow_run_id is set) to parse
and store it as an integer (or raise/exit with a clear message) and reject
non-integer values, and update get_workflow_logs to use the validated integer
attribute (e.g., self.workflow_run_id_int) instead of converting at call time;
ensure __init__ handles missing/invalid input gracefully so get_workflow_logs no
longer needs to guard against ValueError.
♻️ Duplicate comments (1)
.github/scripts/ci_failure_bot.py (1)
9-9: PyGithub dependency still needs to be added to project manifest.This was flagged in a previous review: the script imports PyGithub but it's not declared in the project's dependency manifest (e.g.,
setup.py). While the workflow installs it directly, it should be tracked in the project's dependencies for consistency and local development support.Based on past review comments.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
100-112: Consider making version numbers configurable.Lines 108-109 hardcode Python and Django version numbers. As new versions are released or old ones deprecated, this will require manual script updates. Consider moving these to environment variables, a config file, or reading from the project's CI matrix configuration.
💡 Example: Use environment variables
def generate_setup_response(self): + python_versions = os.environ.get("SUPPORTED_PYTHON_VERSIONS", "3.10, 3.11, 3.12, 3.13") + django_versions = os.environ.get("SUPPORTED_DJANGO_VERSIONS", "4.2, 5.0, 5.1, 5.2") return f""" ## Setup Failed There was an issue with dependencies or environment setup. Check if you added new dependencies to requirements-test.txt. Verify Python/Django version compatibility: -- Python: 3.10, 3.11, 3.12, 3.13 -- Django: 4.2, 5.0, 5.1, 5.2 +- Python: {python_versions} +- Django: {django_versions} The CI will retry automatically, or push a small change to trigger rebuild. """
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
43-43: Consider moving this statement to an else block
(TRY300)
44-44: Do not catch blind exception: Exception
(BLE001)
123-123: Do not catch blind exception: Exception
(BLE001)
⏰ 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.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | 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.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.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: Python==3.10 | django~=4.2.0
🔇 Additional comments (5)
.github/scripts/ci_failure_bot.py (5)
48-63: LGTM!The failure classification logic is clear and handles the main categories appropriately. Using
set()to deduplicate failure types is a good approach.
65-80: LGTM!The QA response provides clear, actionable guidance with the appropriate command and documentation link.
82-98: LGTM!The test failure response provides helpful debugging steps and covers common issues appropriately.
126-165: LGTM!The orchestration logic is well-structured. The bot correctly handles multiple failure types, provides fallback messaging, and gracefully handles the case when no failure logs are found.
168-170: LGTM!Standard main execution pattern is correctly implemented.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
2baccea to
8d3d6bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci-failure-bot.yml (1)
28-34: The empty pull_requests array issue has been addressed.Line 33 correctly uses the
|| ''fallback to handle cases wherepull_requestsis empty (e.g., manual workflow runs or direct pushes). The Python script'spost_commentmethod gracefully handles an emptyPR_NUMBER..github/scripts/ci_failure_bot.py (1)
1-11: PyGithub dependency handling noted in previous review.The import of PyGithub (line 9) without declaring it in the project's
setup.pyhas already been flagged in a previous review. While the workflow explicitly installs it, adding it to the project's dependency manifest would improve local development support.
🧹 Nitpick comments (2)
.github/workflows/ci-failure-bot.yml (1)
24-26: Consider pinning dependency versions for reproducibility.The workflow installs
requestsandPyGithubwithout version constraints. While this ensures you get the latest versions, it could lead to unexpected breakage if major version changes introduce breaking changes.🔎 Example: Pin dependency versions
- name: Install dependencies run: | - pip install requests PyGithub + pip install requests>=2.31.0 PyGithub>=2.0.0.github/scripts/ci_failure_bot.py (1)
32-53: Consider catching more specific exceptions.Line 50 catches the broad
Exceptiontype. While this makes the bot resilient to API failures, catching more specific exceptions (e.g.,GithubException,RateLimitExceededException) would provide better error diagnosis and allow you to handle different failure modes appropriately.🔎 Example: Catch specific exceptions
from github import GithubException, RateLimitExceededException def get_workflow_logs(self): try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) jobs = workflow_run.jobs() # ... rest of logic except RateLimitExceededException as e: print(f"Rate limit exceeded: {e}") return [] except GithubException as e: print(f"GitHub API error: {e}") return []
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
144-144: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
🔇 Additional comments (11)
.github/workflows/ci-failure-bot.yml (3)
1-9: LGTM: Workflow trigger configuration is correct.The
workflow_runtrigger on the "OpenWISP Utils CI Build" workflow with thecompletedevent type is the appropriate way to respond to CI failures. The additional failure check on line 13 ensures the bot only runs when needed.
10-17: LGTM: Job configuration and checkout step are appropriate.The failure condition and ubuntu-latest runner are correctly configured. Using
actions/checkout@v6is current best practice.
19-22: LGTM: Python setup is appropriate.Python 3.11 is a stable, well-supported version suitable for this automation script.
.github/scripts/ci_failure_bot.py (8)
12-31: LGTM: Input validation has been properly implemented.The
__init__method correctly validates that required environment variables are present (line 19) and thatworkflow_run_idis numeric (lines 23-27) before attempting to use it. This addresses the previous concern about potentialValueErrorexceptions during runtime.
54-69: LGTM: Failure type analysis logic is sound.The method correctly categorizes failures by analyzing step names with case-insensitive matching and returns unique failure types. The keyword-based classification appropriately handles QA checks, tests, and setup failures.
71-118: LGTM: Response generation methods provide helpful guidance.The response templates offer clear, actionable guidance for QA failures, test failures, and setup issues. The commands and links to contributing guidelines will help contributors resolve issues quickly.
Note: The Python and Django versions on lines 114-115 are hardcoded and may need periodic updates as new versions are released.
120-123: LGTM: Early return for missing PR number is appropriate.The check on line 121 correctly handles cases where the bot is triggered by workflow runs without an associated PR (e.g., direct pushes or manual runs).
128-146: Deduplication logic successfully addresses previous concerns.The implementation correctly checks for existing bot comments (lines 128-139) and updates them rather than creating duplicates. This addresses the major issue raised in a previous review.
The search strings on lines 133-136 ("CI Build Failed" and "QA Checks Failed") should continue to work as the fallback message on line 172 and the QA response on line 73 contain these markers.
147-187: LGTM: Main orchestration logic is well-structured.The
runmethod effectively coordinates log retrieval, failure analysis, response generation, and comment posting. The fallback message (lines 169-181) ensures helpful guidance is provided even when specific failure types aren't detected.
189-191: LGTM: Standard script execution pattern.The main execution block follows Python best practices.
124-146: Add validation before converting PR_NUMBER to int.Line 126 converts
self.pr_numbertoint()without validation. IfPR_NUMBERis an empty string (which can happen when the workflow fallback on line 33 of the YAML file triggers), this will raise aValueErrorand crash the bot. The early return on line 121 checks for falsy values, but an empty string passes the truthy check.🔎 Proposed fix to validate PR_NUMBER
def post_comment(self, message): - if not self.pr_number: + if not self.pr_number or self.pr_number == '': print("No PR number, skipping comment") return try: - pr = self.repo.get_pull(int(self.pr_number)) + try: + pr_num = int(self.pr_number) + except ValueError: + print(f"Invalid PR_NUMBER: {self.pr_number}") + return + + pr = self.repo.get_pull(pr_num)Likely an incorrect or invalid review comment.
Implements automated bot responses for common CI failures: - Parses QA check failures (flake8, black, isort) - Detects test failures and setup issues - Provides specific fix suggestions with links - Removes automatic mentions to prevent spam - Handles edge cases for workflow runs without PRs Tested on personal repository before submission. Closes openwisp#524
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
8d3d6bd to
939a127
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/scripts/ci_failure_bot.py (1)
9-9: PyGithub dependency still not declared in project manifest.This was flagged in a previous review and remains unaddressed. The script imports
github.Githubbut PyGithub is not insetup.py. While the workflow installs it explicitly, adding it toextras_requireensures consistency for local development.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (4)
19-21: Consider logging which environment variables are missing.The current message "Missing env vars" doesn't indicate which specific variables are absent, making debugging harder when the workflow fails during initialization.
🔎 Suggested improvement
- if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: + missing.append("GITHUB_TOKEN") + if not self.workflow_run_id: + missing.append("WORKFLOW_RUN_ID") + if not self.repository_name: + missing.append("REPOSITORY") + if missing: + print(f"Missing required env vars: {', '.join(missing)}") sys.exit(1)
50-52: Avoid catching bareException.Catching
Exceptionwill swallow all errors includingKeyboardInterruptandSystemExit. Use more specific exception types from PyGithub (e.g.,GithubException) to handle API errors while allowing unexpected exceptions to propagate.🔎 Suggested fix
+from github import Github, GithubException + # In get_workflow_logs: - except Exception as e: - print(f"Error getting logs: {e}") + except GithubException as e: + print(f"Error getting workflow logs: {e}") return []
134-136: Cache the bot login to avoid repeated API calls.
self.github.get_user().loginmakes a GitHub API call each timepost_commentis invoked. Consider caching this value in__init__to reduce API usage and improve performance.🔎 Suggested approach
# In __init__, after line 30: + self.bot_login = self.github.get_user().login # In post_comment, replace line 135: - bot_login = self.github.get_user().login + bot_login = self.bot_login
150-151: Use specific exception types instead of bareException.Similar to
get_workflow_logs, this catch-all will swallow unexpected errors. Consider catchingGithubExceptionfor API errors andValueErrorfor the PR number conversion (which is already handled separately).🔎 Suggested fix
- except Exception as e: + except GithubException as e: print(f"Error posting comment: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
150-150: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.13 | 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.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
54-69: LGTM!The failure type classification logic is clear and the deduplication using
set()is appropriate.
71-118: LGTM!The response templates are clear, actionable, and provide helpful links to documentation.
153-192: LGTM!The orchestration logic is clean with appropriate fallback handling for unclassified failures.
195-197: LGTM!Standard entry point pattern.
There was a problem hiding this comment.
I am not seeing any response in Srinath0916/ci-bot-test#1. Is it really working?
Would this work as a reusable github actions?
Docs and tests are completely missing but I guess we can spend time on that once we are sure this is going in the right direction.
PS: please let's exclude dependabot PRs.
| WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number || '' }} | ||
| run: python .github/scripts/ci_failure_bot.py |
There was a problem hiding this comment.
would we be able to call the same logic from other repositories in the openwisp org?
There was a problem hiding this comment.
Thanks @nemesifier , for pointing this out, I should have clarified this earlier.
I double-checked the workflow logs: the bot is triggering correctly, detecting the failure type, and attempting to post a comment. In my personal test repository, the comment fails with a 403 Resource not accessible by integration, which is a known limitation when using workflow_run + GITHUB_TOKEN on personal repos. The workflow log reflects this.
In OpenWISP organization repositories, where the token has issue/PR write permissions, this limitation should not apply.
Regarding reuse: Yes, the logic can be made reusable across OpenWISP repositories by parameterizing repository-specific context (repo name, URLs, workflow name). Other repositories would only need to add the workflow file and rely on org-level secrets.
I’ll also exclude Dependabot PRs at both the workflow and script level.
.github/scripts/ci_failure_bot.py
Outdated
| Check the CI logs above for specific error details. Common issues: | ||
| - Import errors from missing dependencies | ||
| - Logic changes that broke existing functionality | ||
| - Missing test dependencies |
There was a problem hiding this comment.
A static message cannot cover the complexity of our incoming PRs. We need a dynamic first line of defense against the current avalanche of contributions.
We are wasting valuable maintainer time explaining basic mistakes that an LLM can identify. Since we have a paid Gemini API token with sufficient capacity, we will use it to analyze build failures and outputs. By feeding Gemini the build logs, the YAML workflow, and the repository context, it will generate tailored feedback that provides immediate, actionable steps for the contributor.
I am opting for a direct and honest communication style. The goal is to provide high-quality technical feedback while maintaining a firm standard; we will not be overly polite to those submitting low-effort or spam contributions. This ensures we save maintainer energy and only engage deeply with PRs that meet our baseline requirements.
Structured Gemini System Prompt
Here's a prompt example:
### ROLE
You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.
### INPUT CONTEXT PROVIDED
1. **Build Output/Logs:** The specific error traces from the failed CI run.
2. **YAML Workflow:** The CI configuration defining how tests are executed.
3. **PR Diff:** The actual code changes submitted.
4. **Project Name:** [Name]
5. **Repository:** [Insert repo URL].
6. **run-qa-checks:** [Insert URL of run-qa-checks script]
7. **runtests:** [Insert URL of runtests script]
### TASK
Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below.
### TONE GUIDELINES
- **Direct & Honest:** Do not use "fluff" or overly polite corporate language.
- **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly.
- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it.
### RESPONSE STRUCTURE
1. **Status Summary:** A one-sentence blunt assessment of the failure.
2. **Technical Diagnosis:** - Identify the specific line/test that failed.
- Explain *why* it failed.
3. **Required Action:** Provide a code block or specific steps the contributor must take.
4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review.
### EXAMPLE RESPONSE STYLE
"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update `tests/logic_test.py` before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing."There was a problem hiding this comment.
You're right @nemesifier , static responses won't scale for the kind of PRs we're getting.
I understand the direction now: use Gemini to analyze real build logs, diffs, and workflow context and generate targeted feedback, rather than relying on predefined messages.
I'll pivot the implementation to this Gemini-based approach for intelligent failure analysis. This will provide the dynamic, context-aware responses that can actually handle the complexity of real PRs.
.github/scripts/ci_failure_bot.py
Outdated
| - Python: 3.10, 3.11, 3.12, 3.13 | ||
| - Django: 4.2, 5.0, 5.1, 5.2 | ||
|
|
||
| The CI will retry automatically, or push a small change to trigger rebuild. |
There was a problem hiding this comment.
I don't understand the point of this. Given my previous comment, do you think this is still needed?
There was a problem hiding this comment.
This static message approach isn't needed given the Gemini integration direction.
The static responses were just to validate the workflow mechanics, but the AI analysis will provide much better, context-specific guidance.
I'll focus on the Gemini approach instead.
|
Hi @nemesifier , I’ve implemented a Gemini-based version locally following the approach and system prompt you outlined (using build logs, PR diff, workflow context, and a direct tone). Before pushing it, I wanted to check if you’re happy with proceeding in this direction and with introducing a GEMINI_API_KEY dependency at the org level. Let me know and I’ll move forward accordingly. |
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
@Srinath0916 before merging it here we need to test this somewhere else.
I do not see any output in Srinath0916/ci-bot-test#1, how are you testing this?
|
Hi @nemesifier , I can show you the working bot demo here: srinath-testing/gemini-ci-bot-demo#1 You can see the bot posted: "🤖 Demo CI Failure Bot (AI-powered)" with structured analysis. The reason you don't see output in Srinath0916/ci-bot-test#1 is that personal repos have token permission limitations with workflow_run triggers. The demo repo I created specifically for this has the right setup. The Gemini integration is ready locally - I can push it once you confirm the demo proves the concept works. Would you like me to proceed with the full AI implementation? |
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
It's encouraging @Srinath0916, please push your progress and then focus on adding automated tests, docs and following up reviews.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Line 71: The requests.get call using variables logs_url and headers has no
timeout and can hang; modify the call to pass a timeout (e.g., timeout=10) so it
becomes requests.get(logs_url, headers=headers, timeout=<seconds>), and add a
surrounding try/except to catch requests.exceptions.Timeout (or
requests.exceptions.RequestException) to log the timeout/error and handle
failure gracefully where response is expected.
- Around line 113-123: requests.get call may hang and the except catches all
exceptions; add a timeout argument to requests.get (e.g., requests.get(diff_url,
headers=headers, timeout=10)) and replace the broad except Exception with except
requests.RequestException as e to only catch network-related errors, then log or
print the error variable as before.
- Around line 16-53: The constructor currently hardcodes self.model =
genai.GenerativeModel("gemini-1.5-flash") which is deprecated; update __init__
to read a GEMINI_MODEL env var (e.g., model_name =
os.environ.get("GEMINI_MODEL", "gemini-2.5-flash")) and pass that into
genai.GenerativeModel so the model can be changed without code edits, and ensure
genai.configure(api_key=...) remains before instantiating self.model; optionally
validate that model_name is non-empty and include its variable name in error
messages.
🧹 Nitpick comments (3)
.github/scripts/ci_failure_bot.py (3)
134-136: Narrow the exception type for better debuggability.Catching bare
Exceptioncan mask unexpected errors. Consider catchingGithubExceptionspecifically.🔎 Proposed fix
- except Exception as e: + except GithubException as e: print(f"Error getting workflow YAML: {e}") return None
154-188: Remove ineffective# noqacomments from within the string literal.The
# noqa: E501comments on lines 154, 157, 169, 173-175, 182, 185 are inside the multi-line string and have no effect on linter behavior. They just clutter the prompt sent to Gemini.🔎 Proposed fix
Remove all
# noqa: E501suffixes from within the context string. If you need to suppress E501 for this file, add a single# noqa: E501comment on line 155 (the opening of the f-string), or configure the linter to ignore this file/section.- # noqa: E501 (ignore line length for multiline string) context = f""" ### ROLE -You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 +You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.Apply similar cleanup to lines 169, 173, 174, 175, 182, and 185.
246-253: Narrow exception handling for actor check.The dependabot check catches bare
Exception. Since this is a non-critical check (bot continues anyway), consider catchingGithubExceptionspecifically or at minimumAttributeErrorfor the case whereactormight be None.🔎 Proposed fix
try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) - actor_login = workflow_run.actor.login.lower() - if "dependabot" in actor_login: + if workflow_run.actor and "dependabot" in workflow_run.actor.login.lower(): print(f"Skipping dependabot PR from {workflow_run.actor.login}") return - except Exception as e: + except (GithubException, AttributeError) as e: print(f"Warning: Could not check actor: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
71-71: Probable use of requests call without timeout
(S113)
93-93: Consider moving this statement to an else block
(TRY300)
113-113: Probable use of requests call without timeout
(S113)
120-120: Do not catch blind exception: Exception
(BLE001)
134-134: Do not catch blind exception: Exception
(BLE001)
154-154: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
252-252: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
1-13: Imports and setup look appropriate.The imports are well-organized and appropriate for the script's functionality. Note that a past review flagged adding
PyGithubandgoogle-generativeaito the project's dependency manifest for local development support—ensure this is tracked.
212-239: Comment deduplication is well-implemented.The logic correctly checks for existing bot comments and updates them rather than creating duplicates. This addresses previous review feedback about preventing comment spam.
197-210: Fallback response is appropriate.The static fallback provides useful guidance when Gemini is unavailable, with actionable commands and a link to documentation.
76-78: Token limits are not a constraint for these truncations.Gemini 1.5 Flash has a 1,048,576-token context window (~4 million characters). With logs truncated to 5000 chars and diff to 8000 chars, plus the prompt template, the total message size (~15-20K characters) is negligible. The truncations are sensible for practical reasons—API response speed, cost optimization, and filtering noise—not because of Gemini's context limitations.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
294b7f6 to
2e7b8f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/workflows/ci-failure-bot.yml:
- Around line 29-32: The comment "Pin dependency versions for reproducibility"
is inaccurate relative to the pip install line under the "Install dependencies"
step; either change the version specifiers in the pip install command to exact
pins using == (e.g., requests==2.31.0 PyGithub==2.0.0
google-generativeai==0.3.0) to match the comment, or update the comment to
something like "Specify minimum dependency versions" to reflect the current >=
constraints—modify the pip install line or the comment accordingly in the
"Install dependencies" step.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (4)
125-136: Narrow exception type for better error handling.Line 134 catches bare
Exception, which can mask unexpected errors. Consider catching specific GitHub API exceptions.🔎 Proposed fix
workflow_file = self.repo.get_contents(workflow_path) return workflow_file.decoded_content.decode("utf-8") - except Exception as e: + except (GithubException, ValueError) as e: print(f"Error getting workflow YAML: {e}") return None
154-154: Remove unusednoqadirective.Line 154 has an unused
noqadirective that can be removed.🔎 Proposed fix
- # noqa: E501 (ignore line length for multiline string) context = f"""
190-195: Narrow exception type in Gemini API call.Line 193 catches bare
Exception. Consider catching specific API exceptions from thegoogle.generativeailibrary for better error diagnostics.🔎 Proposed fix
try: response = self.model.generate_content(context) return response.text - except Exception as e: + except (ValueError, ConnectionError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()
246-253: Narrow exception type in actor check.Line 252 catches bare
Exception. Consider catching specific GitHub API exceptions.🔎 Proposed fix
try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) actor_login = workflow_run.actor.login.lower() if "dependabot" in actor_login: print(f"Skipping dependabot PR from {workflow_run.actor.login}") return - except Exception as e: + except (GithubException, AttributeError) as e: print(f"Warning: Could not check actor: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
71-71: Probable use of requests call without timeout
(S113)
93-93: Consider moving this statement to an else block
(TRY300)
113-113: Probable use of requests call without timeout
(S113)
120-120: Do not catch blind exception: Exception
(BLE001)
134-134: Do not catch blind exception: Exception
(BLE001)
154-154: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
252-252: Do not catch blind exception: Exception
(BLE001)
⏰ 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.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.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~=5.0.0
- GitHub Check: Python==3.13 | 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.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
🔇 Additional comments (3)
.github/scripts/ci_failure_bot.py (1)
212-239: Comment deduplication logic looks good.The implementation correctly checks for existing bot comments and updates them instead of creating duplicates. The exception handling is appropriately narrow.
.github/workflows/ci-failure-bot.yml (2)
1-18: Workflow trigger and permissions configured correctly.The workflow appropriately:
- Triggers only on CI build failures
- Excludes dependabot PRs at the job level
- Grants minimal required permissions (issues/PR write, contents read)
34-41: Verify GEMINI_API_KEY secret is configured.Line 37 references
secrets.GEMINI_API_KEY. Based on the PR comments, this requires organization-level secret configuration. Ensure the secret is added to the OpenWISP organization before merging, or the workflow will fail.Would you like me to generate a verification script to check if the secret is accessible, or would you prefer to confirm this manually with the maintainers?
| - name: Install dependencies | ||
| # Pin dependency versions for reproducibility | ||
| run: | | ||
| pip install requests>=2.31.0 PyGithub>=2.0.0 google-generativeai>=0.3.0 |
There was a problem hiding this comment.
Comment doesn't match implementation.
Line 30 states "Pin dependency versions for reproducibility" but line 32 uses >= (minimum version constraints) rather than == (pinned versions). Either:
- Pin the versions with
==for true reproducibility, or - Update the comment to reflect the relaxed constraint approach.
🔎 Option 1: Actually pin versions
- name: Install dependencies
- # Pin dependency versions for reproducibility
run: |
- pip install requests>=2.31.0 PyGithub>=2.0.0 google-generativeai>=0.3.0
+ pip install requests==2.31.0 PyGithub==2.0.0 google-generativeai==0.3.0🔎 Option 2: Update comment
- name: Install dependencies
- # Pin dependency versions for reproducibility
+ # Install minimum required versions
run: |
pip install requests>=2.31.0 PyGithub>=2.0.0 google-generativeai>=0.3.0📝 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.
| - name: Install dependencies | |
| # Pin dependency versions for reproducibility | |
| run: | | |
| pip install requests>=2.31.0 PyGithub>=2.0.0 google-generativeai>=0.3.0 | |
| - name: Install dependencies | |
| # Install minimum required versions | |
| run: | | |
| pip install requests>=2.31.0 PyGithub>=2.0.0 google-generativeai>=0.3.0 |
🤖 Prompt for AI Agents
In @.github/workflows/ci-failure-bot.yml around lines 29 - 32, The comment "Pin
dependency versions for reproducibility" is inaccurate relative to the pip
install line under the "Install dependencies" step; either change the version
specifiers in the pip install command to exact pins using == (e.g.,
requests==2.31.0 PyGithub==2.0.0 google-generativeai==0.3.0) to match the
comment, or update the comment to something like "Specify minimum dependency
versions" to reflect the current >= constraints—modify the pip install line or
the comment accordingly in the "Install dependencies" step.
2e7b8f1 to
6ab0337
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
188-188: Remove unusednoqadirective.The
# noqa: E501directive is unnecessary as E501 (line length) is not enabled in the linter configuration.🔎 Proposed fix
-""" # noqa: E501 +"""
190-195: Consider narrowing the exception type for Gemini API errors.Catching bare
Exceptioncan mask unexpected errors. Thegoogle.generativeailibrary typically raisesgoogle.api_core.exceptions.GoogleAPIErroror similar for API failures. However, given the fallback behavior is appropriate and the bot should be resilient, this is a minor concern.🔎 Proposed fix to catch more specific exceptions
try: response = self.model.generate_content(context) return response.text - except Exception as e: + except (genai.types.GenerateContentError, ValueError, ConnectionError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()Note: You may need to verify the exact exception types from the
google-generativeailibrary.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
188-188: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.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.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (10)
.github/scripts/ci_failure_bot.py (9)
1-12: LGTM!Imports are well-organized with standard library imports first, followed by third-party packages.
15-54: LGTM!The initialization logic is well-structured with comprehensive environment variable validation, proper integer conversion with error handling, and a configurable Gemini model name via environment variable.
56-97: LGTM!The method correctly fetches failed job logs with a timeout, truncates to avoid token limits, and handles exceptions with specific types.
99-124: LGTM!Previous review feedback was addressed: timeout is set, and exception handling uses specific types.
126-137: LGTM!Simple and focused method with appropriate exception handling.
197-210: LGTM!The fallback response provides useful guidance when AI analysis is unavailable.
212-239: LGTM!The deduplication logic correctly checks for existing bot comments and updates them instead of creating duplicates. This addresses the previous review feedback.
241-271: LGTM!The orchestration flow is clear and includes a defense-in-depth dependabot check. Error handling at each step ensures graceful degradation.
274-276: LGTM!Standard entry point pattern.
.github/workflows/ci-failure-bot.yml (1)
22-27: The action versions are valid. Bothactions/checkout@v6andactions/setup-python@v6are publicly available releases (checkout v6.0.1 and setup-python v6.1.0). The workflow will not fail due to these action versions.Likely an incorrect or invalid review comment.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
6ab0337 to
ab7cd1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Around line 212-239: The comment deduplication in post_comment is brittle
because it looks for specific phrases; update post_comment to use a consistent
hidden HTML marker (e.g., "<!-- ci-failure-bot -->") when creating or editing
comments and check for that marker in existing_comments instead of matching "CI
Build Failed" or "build failed"; ensure the marker is appended to the message
passed to pr.create_issue_comment and comment.edit, and then change the loop
that scans pr.get_issue_comments() to detect comment.body containing the marker
(while preserving the existing bot_login check).
- Around line 99-124: The PR diff extraction in get_pr_diff currently slices
response.text[-8000:], dropping the beginning of the diff where imports and
initial definitions live; change this to capture the start of the diff (e.g.,
use the first 8000 characters of response.text) or, if you want both head and
tail context, concatenate response.text[:4000] + "\n...\n" +
response.text[-4000:] so the function returns the leading context instead of
only the tail.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
139-195: Gemini integration is functional but could be refined.The context construction is comprehensive and the system prompt matches the maintainer's requirements. Two observations:
Optional refactor: Line 193 catches bare
Exception, which static analysis flags. While this provides robustness for external API calls, consider catching specificgenaiexceptions if available for clearer error handling.Context size awareness: The combined context (build logs, PR diff, workflow YAML, prompt template) could grow quite large with multiple failed jobs. Gemini 2.5-flash has a generous context window, so this should work, but monitor for potential token limit issues in practice.
🔎 Optional: Narrow exception handling
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: + except (ValueError, ConnectionError, TimeoutError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()Note: You may want to check the
google.generativeaidocumentation for specific exception types to catch instead of the bareException.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | 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.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
🔇 Additional comments (3)
.github/scripts/ci_failure_bot.py (3)
16-54: Initialization and environment validation is solid.The constructor properly validates all required environment variables with specific error messages, ensures
workflow_run_idis numeric, and makes the Gemini model configurable. The setup logic is clean and maintainable.
56-97: Build log collection is well-implemented.The method correctly fetches logs from failed jobs with appropriate timeout, limits log size to avoid token overflow, and includes both job-level and step-level failure details. Exception handling is appropriate.
241-271: Main execution flow is well-structured.The
runmethod orchestrates the bot's operations logically: validates actor (skips dependabot), gathers context, analyzes with Gemini, and posts results. The early exit when no build logs are found prevents unnecessary API calls. Good defensive programming throughout.
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
2 times, most recently
from
8e5d108 to
5d3181a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Line 189: Remove the unnecessary inline noqa directive from the trailing
docstring terminator (the '""" # noqa: E501' token) so the line ends simply
with '"""'; update the closing triple-quote in the module-level string in
ci_failure_bot.py to remove the "# noqa: E501" comment.
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
56-97: Log truncation approach is reasonable for error context.The method correctly fetches logs from failed jobs and steps. Taking the last 5000 characters of logs (line 78) is a pragmatic choice since error messages typically appear at the end of build logs. However, if early setup or environment context is needed for debugging, consider capturing both the first and last portions (e.g., first 2000 + last 3000 chars).
191-196: Consider narrowing the exception catch for better debugging.While catching
Exceptionensures the bot always falls back gracefully, it can mask unexpected errors (e.g., programming bugs). Consider catching only API-related exceptions likegoogle.generativeai.types.GenerationExceptionor logging unexpected exceptions before falling back.🔎 Optional improvement
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: + except (ValueError, ConnectionError, AttributeError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response() + except Exception as e: + print(f"Unexpected error in Gemini analysis: {e}") + return self.fallback_response()
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
189-189: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
193-193: Consider moving this statement to an else block
(TRY300)
194-194: Do not catch blind exception: Exception
(BLE001)
⏰ 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.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.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.0.0
- GitHub Check: Python==3.12 | 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.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (7)
.github/scripts/ci_failure_bot.py (7)
1-14: LGTM! Clean imports and documentation.The module docstring and imports are well-organized and appropriate for the bot's functionality.
16-54: LGTM! Robust initialization with comprehensive validation.The environment variable validation is thorough with clear error messages,
workflow_run_idis properly validated as numeric, and the Gemini model is configurable. The initialization sequence is correct.
126-137: LGTM! Clean workflow YAML retrieval.The method correctly fetches and decodes the workflow file content with appropriate error handling.
198-211: LGTM! Appropriate fallback message.The static fallback provides useful guidance when AI analysis is unavailable.
242-277: LGTM! Well-structured main execution flow.The run method orchestrates the bot's workflow logically: validates it's not a dependabot PR, gathers context, performs AI analysis, and posts results. The dependabot check prevents unnecessary processing.
119-119: Diff truncation still loses critical early context.This issue was previously flagged: taking the last 8000 characters discards the beginning of the diff where imports, class definitions, and initial functions typically appear. For large diffs, Gemini will miss crucial context.
Likely an incorrect or invalid review comment.
227-230: Comment deduplication remains brittle.This issue was previously flagged: the string matching for "CI Build Failed" or "build failed" is fragile. If Gemini generates a response without these phrases, duplicate comments will be created. A consistent HTML marker (e.g.,
<!-- ci-failure-bot -->) would be more reliable.Likely an incorrect or invalid review comment.
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ # noqa: E501 |
There was a problem hiding this comment.
Remove unused noqa directive.
The # noqa: E501 comment is unnecessary since the E501 (line-too-long) rule is not enabled in your Ruff configuration.
🔎 Proposed fix
Analyze the failure and provide your response:
-""" # noqa: E501
+"""
try:📝 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.
| """ # noqa: E501 | |
| """ |
🧰 Tools
🪛 Ruff (0.14.10)
189-189: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py at line 189, Remove the unnecessary inline
noqa directive from the trailing docstring terminator (the '""" # noqa: E501'
token) so the line ends simply with '"""'; update the closing triple-quote in
the module-level string in ci_failure_bot.py to remove the "# noqa: E501"
comment.
Implements intelligent bot responses using Gemini AI to analyze: - Build logs and error traces from failed CI runs - PR diffs and code changes for context - Workflow YAML configuration - Provides direct, actionable feedback to contributors Replaces static responses with dynamic AI analysis that examines actual failure context to generate specific guidance. Addresses reviewer feedback on dependencies and error handling. Closes openwisp#524
Srinath0916
force-pushed
the
issues/524-ci-failure-bot
branch
from
5d3181a to
94d6de9
Compare
|
@nemesifier Gemini implementation is ready. Let me know if you'd like any adjustments before I proceed working on tests and docs next |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Around line 99-133: In get_pr_diff, the except clause only catches
GithubException and requests.RequestException so a non-numeric self.pr_number
causing int(self.pr_number) to raise ValueError is uncaught; update the
exception handler in get_pr_diff to include ValueError (matching post_comment
behavior) and ensure the error is handled/logged the same way (e.g., include the
exception in the print/processLogger call) so non-numeric PR_NUMBER does not
crash the script.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
200-205: Narrow the exception handling to avoid catching bareException.Catching
Exceptionalong with specific exceptions is redundant (sinceExceptioncatches everything) and overly broad—it will catch system-exiting exceptions and mask unexpected errors. Prefer catching specific exceptions from thegoogle.generativeailibrary.🔎 Proposed fix
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: + except (ValueError, ConnectionError, TimeoutError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()Alternatively, if you need broader coverage for unknown API errors, consider catching
google.api_core.exceptions.GoogleAPIErroror a similar base class from the library rather than bareException.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
198-198: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
⏰ 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.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | 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.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (8)
.github/scripts/ci_failure_bot.py (8)
1-12: LGTM!Imports are appropriate and all are used in the script.
15-54: LGTM!The initialization logic is solid with comprehensive environment variable validation, proper integer conversion for
workflow_run_idwith error handling, and a configurable Gemini model name.
56-97: LGTM!Build log retrieval is well-implemented with timeout on HTTP requests and appropriate exception handling. Truncating to the last 5000 characters is sensible for build logs since errors typically appear at the end.
135-146: LGTM!Clean implementation for fetching the workflow YAML configuration with appropriate exception handling.
207-220: LGTM!The fallback response provides helpful guidance with common fixes and a documentation link when AI analysis is unavailable.
222-250: LGTM!The comment deduplication using a hidden HTML marker is a robust approach. The logic correctly updates existing bot comments rather than creating duplicates.
252-282: LGTM!The execution flow is well-structured with appropriate early exits. The defense-in-depth dependabot check adds resilience even though the workflow also filters.
285-287: LGTM!Standard entry point pattern.
| def get_pr_diff(self): | ||
| """Get the PR diff/changes if PR exists""" | ||
| if not self.pr_number or self.pr_number.strip() == "": | ||
| return None | ||
|
|
||
| try: | ||
| pr_num = int(self.pr_number) | ||
| pr = self.repo.get_pull(pr_num) | ||
|
|
||
| # Get diff content | ||
| diff_url = pr.diff_url | ||
| headers = { | ||
| "Authorization": f"token {self.github_token}", | ||
| "Accept": "application/vnd.github.v3.diff", | ||
| } | ||
| response = requests.get(diff_url, headers=headers, timeout=30) | ||
| if response.status_code == 200: | ||
| diff_text = response.text | ||
| if len(diff_text) > 8000: | ||
| # Take first 4000 and last 4000 chars for context | ||
| diff_text = ( | ||
| diff_text[:4000] | ||
| + "\n\n[...middle truncated...]\n\n" | ||
| + diff_text[-4000:] | ||
| ) | ||
|
|
||
| return { | ||
| "title": pr.title, | ||
| "body": pr.body or "", | ||
| "diff": diff_text, | ||
| } | ||
| except (GithubException, requests.RequestException) as e: | ||
| print(f"Error getting PR diff: {e}") | ||
|
|
||
| return None |
There was a problem hiding this comment.
Add ValueError to exception handler for consistency.
Line 105 can raise ValueError if self.pr_number is non-numeric, but line 130 doesn't catch it. This is inconsistent with post_comment (line 249) which correctly catches ValueError. A non-numeric PR_NUMBER would cause an unhandled exception.
🔎 Proposed fix
- except (GithubException, requests.RequestException) as e:
+ except (GithubException, requests.RequestException, ValueError) as e:
print(f"Error getting PR diff: {e}")🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 99 - 133, In get_pr_diff, the
except clause only catches GithubException and requests.RequestException so a
non-numeric self.pr_number causing int(self.pr_number) to raise ValueError is
uncaught; update the exception handler in get_pr_diff to include ValueError
(matching post_comment behavior) and ensure the error is handled/logged the same
way (e.g., include the exception in the print/processLogger call) so non-numeric
PR_NUMBER does not crash the script.
3 participants
Implements automated bot responses for common CI failures:
Checklist
Reference to Existing Issue
Addresses #524
Description of Changes
Adds GitHub Actions workflow and Python script for automated CI failure responses.
Screenshot
Test repository showing bot functionality: https://github.com/Srinath0916/ci-bot-test