Repository navigation
[feature] Added changelog entry bot using Gemini AI - #584
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/developer/changelog-bot.rst`:
- Around line 126-129: The docs claim an automatic trigger on PR approval but
the reusable workflow lacks a pull_request_review event; either remove that docs
section or add support by updating the reusable workflow to handle the
pull_request_review event (add a top-level pull_request_review: entry) and route
it to the same job that handles manual triggers (e.g., the generate_changelog or
changelog-suggest job), guarding execution with a condition like if:
github.event.review.state == 'approved' (or equivalent) so only approvals invoke
the workflow.
- Around line 77-92: The docs list a non-existent trigger "pull_request_review";
update docs/developer/changelog-bot.rst to match the actual workflow by removing
the pull_request_review block and clarifying the bot triggers only on
issue_comment (created) with the trigger-phrase input (e.g., "@openwisp-bot
changelog"); ensure the example shows only the issue_comment trigger and
mentions the use of the trigger-phrase and GEMINI_API_KEY secret so it reflects
changelog-bot.yml's current behavior.
🧹 Nitpick comments (5)
.github/actions/changelog-generator/test_generate_changelog.py (2)
82-109: Consider adding a test for diff fetch failure.The
get_pr_difffunction returns an empty string on HTTP/URL errors (graceful degradation). Adding a test case that verifies this behavior would improve coverage.💡 Optional test for error handling
`@patch`("generate_changelog.urlopen") def test_returns_empty_on_error(self, mock_urlopen): from urllib.error import HTTPError mock_urlopen.side_effect = HTTPError(None, 404, "Not Found", {}, None) result = get_pr_diff("org/repo", 123, "token") self.assertEqual(result, "")
361-372: Test scenario is slightly unrealistic but still validates error handling.The test mocks
urlopento return a response with status 403, but in realityurlopenraisesHTTPErrorfor non-2xx status codes rather than returning a response object. The implementation'sresponse.status != 201check (inpost_github_comment) would only trigger for unusual cases like status 200 or 204.However, this test still validates that non-201 responses are rejected, so the overall behavior is correct.
💡 More realistic error test
`@patch`("generate_changelog.urlopen") def test_raises_on_http_error(self, mock_urlopen): from urllib.error import HTTPError mock_urlopen.side_effect = HTTPError( "https://api.github.com/...", 403, "Forbidden", {}, None ) with self.assertRaises(HTTPError): post_github_comment("org/repo", 123, "Test comment", "token").github/actions/changelog-generator/generate_changelog.py (1)
140-141: Consider logging the exception for debugging purposes.The broad
except Exceptionwithcontinueis reasonable here since linked issues are supplementary data, but logging the error would help diagnose issues in production.💡 Optional: Log the exception
except Exception: + print(f"Warning: Could not fetch issue #{issue_num}", file=sys.stderr) continue.github/actions/changelog-generator/action.yml (1)
31-33: Consider pinning thegoogle-genaidependency version.The
pip install google-genaicommand installs the latest version, which could lead to unexpected behavior if the SDK introduces breaking changes.💡 Pin dependency version
- name: Install dependencies shell: bash - run: pip install google-genai + run: pip install google-genai==1.0.0Replace
1.0.0with the tested version..github/workflows/reusable-changelog-bot.yml (1)
24-27: The fallback in the condition is redundant.The
|| '@openwisp-bot changelog'fallback at line 27 is unnecessary sincetrigger-phrasealready has a default value defined at line 15. The condition can be simplified.💡 Simplify condition
if: | github.event_name == 'issue_comment' && github.event.issue.pull_request && - contains(github.event.comment.body, inputs.trigger-phrase || '@openwisp-bot changelog') + contains(github.event.comment.body, inputs.trigger-phrase)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/actions/changelog-generator/action.yml.github/actions/changelog-generator/generate_changelog.py.github/actions/changelog-generator/test_generate_changelog.py.github/workflows/changelog-bot.yml.github/workflows/reusable-changelog-bot.ymldocs/developer/changelog-bot.rstruntests.py
🧰 Additional context used
🧬 Code graph analysis (2)
.github/actions/changelog-generator/test_generate_changelog.py (1)
.github/actions/changelog-generator/generate_changelog.py (9)
build_prompt(170-242)call_gemini(145-167)get_env_or_exit(37-43)get_linked_issues(114-142)get_pr_commits(102-111)get_pr_details(66-78)get_pr_diff(81-99)post_github_comment(245-260)main(263-283)
runtests.py (1)
.github/actions/changelog-generator/generate_changelog.py (1)
main(263-283)
🪛 Ruff (0.14.14)
.github/actions/changelog-generator/generate_changelog.py
[error] 54-54: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 56-56: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 89-89: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 91-91: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 140-141: try-except-continue detected, consider logging the exception
(S112)
[warning] 140-140: Do not catch blind exception: Exception
(BLE001)
[warning] 161-161: Abstract raise to an inner function
(TRY301)
[warning] 161-161: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 163-163: Consider moving this statement to an else block
(TRY300)
[warning] 165-165: Do not catch blind exception: Exception
(BLE001)
[error] 255-255: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 256-256: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 283-283: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (3)
- 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
🔇 Additional comments (24)
.github/actions/changelog-generator/test_generate_changelog.py (7)
1-22: LGTM! Well-structured test suite.The test file is well-organized with clear test classes for each function, proper use of mocking, and good coverage of edge cases including empty inputs, missing fields, and error conditions.
24-42: LGTM!Complete coverage for environment variable validation including the edge case of empty strings.
45-79: LGTM!Good coverage of PR details extraction with proper handling of missing fields.
112-138: LGTM!Good coverage of commit extraction including SHA truncation and the 10-commit limit.
141-180: LGTM!Good coverage of issue linking patterns and limits. The empty body test correctly expects an empty list without needing API mocking.
183-279: LGTM!Comprehensive test coverage for the Gemini API integration including success paths, configuration validation, and error handling.
282-342: LGTM!Good coverage of prompt construction including all components and truncation behavior.
.github/actions/changelog-generator/generate_changelog.py (9)
37-43: LGTM!Clean implementation for required environment variable validation.
46-63: LGTM!The static analysis warning (S310) about URL scheme audit is a false positive here since the URL is always constructed with the hardcoded
https://api.github.comprefix.
66-78: LGTM!Good use of defensive
.get()calls with defaults for safe field extraction.
81-99: LGTM!Good graceful degradation on error, and sensible truncation to prevent overwhelming the LLM with large diffs.
102-111: LGTM!Clean extraction of commit information with sensible limits.
145-167: LGTM!Appropriate error handling for external API calls. The static analysis warnings (TRY301, TRY003, TRY300, BLE001) are stylistic preferences; the current implementation is clear and functional.
170-242: LGTM!Well-structured prompt with comprehensive instructions for RST changelog generation and sensible truncation limits.
245-260: LGTM!Proper comment posting implementation. Note that
urlopentypically raisesHTTPErrorfor 4xx/5xx status codes before reaching the status check, but the check provides additional defense for unexpected non-201 success codes.
263-287: LGTM!Clean orchestration of the changelog generation flow with proper error propagation.
.github/actions/changelog-generator/action.yml (1)
1-49: LGTM overall!Well-structured composite action with clear input definitions and proper environment variable mapping.
runtests.py (1)
20-27: LGTM!Clean extension of pytest test discovery to include the new changelog generator tests.
.github/workflows/changelog-bot.yml (1)
1-15: LGTM!Clean entry-point workflow that delegates to the reusable workflow with appropriate inputs and secrets.
docs/developer/changelog-bot.rst (1)
1-195: LGTM overall!Comprehensive documentation covering setup, configuration, usage, troubleshooting, and security considerations. The example RST format and workflow configuration are helpful.
.github/workflows/reusable-changelog-bot.yml (4)
41-56: LGTM!Good permission validation ensuring only users with write access can trigger changelog generation.
58-65: LGTM!Efficient use of sparse checkout to fetch only the required action directory.
67-77: LGTM!Good UX providing immediate feedback via the rocket reaction.
79-87: LGTM!Proper invocation of the changelog generator action with all required inputs.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
ebf86f4 to
10b3e9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/actions/changelog-generator/generate_changelog.py:
- Around line 145-167: The long system instruction string in call_gemini exceeds
line length (E501); split or wrap the string passed to
types.GenerateContentConfig.system_instruction (inside the call_gemini function)
into multiple shorter string literals (e.g., use implicit concatenation across
multiple quoted parts or join a list) so the combined instruction remains the
same but no single line exceeds the limit; ensure you only change the string
formatting and keep the same content, model, and other config fields.
- Around line 114-142: The get_linked_issues function can abort the process
because github_api_request raises SystemExit on HTTP/URL errors; update
get_linked_issues to guard calls to github_api_request by catching SystemExit in
addition to Exception (or replace the call with a non-exiting wrapper), so a
failing fetch for any issue_num does not terminate the workflow; modify the
try/except around github_api_request("/repos/{repo}/issues/{issue_num}", token)
in get_linked_issues to handle SystemExit (and continue) and ensure issue_data
failures simply skip that issue.
In @.github/actions/changelog-generator/test_generate_changelog.py:
- Around line 4-21: The test module triggers flake8 E402 because sys.path.insert
runs before importing generate_changelog; fix by suppressing the linter for that
legitimate import order: add "# noqa: E402" to the "from generate_changelog
import ( build_prompt, call_gemini, get_env_or_exit, get_linked_issues,
get_pr_commits, get_pr_details, get_pr_diff, post_github_comment, )" import line
so the linter ignores the post-statement import while keeping sys.path.insert
and the referenced symbols unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/actions/changelog-generator/action.yml.github/actions/changelog-generator/generate_changelog.py.github/actions/changelog-generator/test_generate_changelog.py.github/workflows/changelog-bot.yml.github/workflows/reusable-changelog-bot.ymldocs/developer/changelog-bot.rstruntests.py
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/reusable-changelog-bot.yml
- docs/developer/changelog-bot.rst
- .github/workflows/changelog-bot.yml
- runtests.py
- .github/actions/changelog-generator/action.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/changelog-generator/test_generate_changelog.py (1)
.github/actions/changelog-generator/generate_changelog.py (9)
build_prompt(170-242)call_gemini(145-167)get_env_or_exit(37-43)get_linked_issues(114-142)get_pr_commits(102-111)get_pr_details(66-78)get_pr_diff(81-99)post_github_comment(245-260)main(263-283)
🪛 GitHub Actions: OpenWISP Utils CI Build
.github/actions/changelog-generator/test_generate_changelog.py
[error] 12-12: Flake8: E402 module level import not at top of file.
.github/actions/changelog-generator/generate_changelog.py
[error] 154-154: E501 line too long (174 > 110 characters).
🪛 Ruff (0.14.14)
.github/actions/changelog-generator/generate_changelog.py
[error] 54-54: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 56-56: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 89-89: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 91-91: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 140-141: try-except-continue detected, consider logging the exception
(S112)
[warning] 140-140: Do not catch blind exception: Exception
(BLE001)
[warning] 161-161: Abstract raise to an inner function
(TRY301)
[warning] 161-161: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 163-163: Consider moving this statement to an else block
(TRY300)
[warning] 165-165: Do not catch blind exception: Exception
(BLE001)
[error] 255-255: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 256-256: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 283-283: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (19)
.github/actions/changelog-generator/test_generate_changelog.py (9)
24-42: Good coverage for env var validation.Covers present, missing, and empty cases cleanly.
45-80: PR detail parsing tests look solid.Nice coverage for defaults and label extraction.
82-112: Diff retrieval and truncation tests are clear.Validates both normal and oversized diffs.
114-141: Commit parsing tests are on point.Covers message truncation and commit limit.
143-183: Linked-issue extraction tests look good.Covers keyword patterns, max count, and empty body.
185-283: Gemini integration tests are thorough.Covers model selection, config usage, and error paths.
284-345: Prompt construction tests are comprehensive.Good coverage for labels, commits, issues, and truncation.
347-375: Comment posting tests look good.Covers success and non-201 error behavior.
377-378: Main guard is fine..github/actions/changelog-generator/generate_changelog.py (10)
1-35: Module header/imports look good.
37-43: Env var guard is clean and direct.
46-63: GitHub API helper is straightforward.
66-78: PR details extraction looks good.
81-99: Diff fetch + truncation is handled well.
102-111: Commit parsing logic is clear.
170-242: Prompt assembly is well structured.
245-261: PR comment posting logic looks good.
263-283: Main flow is clear and cohesive.
286-287: Entry point is fine.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2b1b98f to
d0d513d
Compare
|
A comment by code rabbit hasn't been resolved, apply the changes if you think theyre worth addressing, or just mention why you dont agree to it by replying to it |
9efcef1 to
5f1333c
Compare
| How It Works | ||
| ------------ | ||
|
|
||
| 1. A maintainer approves a PR or comments ``@openwisp-bot changelog`` |
There was a problem hiding this comment.
Let's make sure the following points are true, covered by unit tests and documented:
- If PR is approved and
@openwisp-bot changelogwas explicitly called after the latest changes were pushed, don't do anything - If PR is approved and
@openwisp-bot changelogwas not explicitly called after the latest changes were pushed, the action is triggered only if the PR title contains:[feature,[fixor[change- I think we can skip this for [deps as they don't require much change log writing - If
@openwisp-bot changelogis called, it will execute regardless of the PR title
BTW: approved by who? Anyone? No, it should be triggered only when approved by a member of the @openwisp organization.
There was a problem hiding this comment.
the maintainer of the repo, yes that I have done. Basically who has the write access to the openwisp's repos.
Line 43 - 48 in the .github/workflows/reusable-changelog-bot.py has this logic
There was a problem hiding this comment.
yes 3rd point has been implemented, the changeling will be generated regardless of PR title, but I will also add a test case which covers this case for safe side.
-
if I understand point 2 correctly you want the log to be generated automatically without mentioning @openwisp-bot changelog if the PR title contains feature, fix, change ?
-
I will have to fix the logic to follow point 1, that was a nice catch!
| freezegun | ||
| pytest | ||
| pytest-mock | ||
| google-genai |
There was a problem hiding this comment.
There was a problem hiding this comment.
This is still here even though setup.py has been updated.
| pytest.main( | ||
| [ | ||
| os.path.join("openwisp_utils/releaser/tests"), | ||
| os.path.join(".github/actions/changelog-generator"), |
There was a problem hiding this comment.
anything specific which you want to me to look in #562 in relation to runtests.py file?
If not, yes I am reading the changes to understand the PR in general
github-project-automation
Bot
moved this to To do (general)
in OpenWISP Contributor's Board
github-project-automation
Bot
moved this to In progress
in OpenWISP Priorities for next releases
github-project-automation
Bot
moved this from To do (general)
to In progress
in OpenWISP Contributor's Board
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/actions/changelog-generator/generate_changelog.py:
- Around line 66-78: The PR body returned by get_pr_details can be null which
later causes get_linked_issues to crash when calling .lower(); update
get_pr_details to coalesce the PR body to an empty string (e.g., use
pr_data.get("body") or "") when constructing the returned dict so "body" is
never None, and optionally add a defensive check in get_linked_issues to handle
empty or non-string bodies before calling .lower(); reference get_pr_details and
get_linked_issues when applying the change.
- Around line 175-247: The prompt in build_prompt does not supply a canonical
repository/PR URL so the LLM may hallucinate links; update build_prompt to read
the PR URL from pr_details (e.g., pr_details['html_url'] or a repo_url derived
from pr_details) and inject that into the prompt so the RST link rule references
the real URL variable, and update the RULE 4 example/reference to use that
variable (e.g., replace <repo_url> with the injected pr_html_url) so the model
always uses the provided canonical PR URL when rendering `#<number> <...>`
links.
In @.github/workflows/reusable-changelog-bot.yml:
- Around line 39-55: Add a review-approval gate to the existing check-permission
step: use the GitHub API to list PR reviews for the pull request (e.g., via
github.rest.pulls.listReviews using context.repo and context.issue.number),
compute a boolean hasApproval by checking if any review.state === 'APPROVED',
and require hasApproval along with hasAccess before proceeding; if not approved
call core.setFailed('Pull request must be approved to trigger changelog
generation') (retain existing hasAccess logic and variable names like hasAccess,
hasApproval, and the step id check-permission to locate where to insert the
check).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/actions/changelog-generator/action.yml.github/actions/changelog-generator/generate_changelog.py.github/actions/changelog-generator/test_generate_changelog.py.github/workflows/changelog-bot.yml.github/workflows/reusable-changelog-bot.ymldocs/developer/changelog-bot.rstrequirements-test.txtruntests.py
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/developer/changelog-bot.rst
- .github/workflows/changelog-bot.yml
- runtests.py
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/changelog-generator/test_generate_changelog.py (1)
.github/actions/changelog-generator/generate_changelog.py (9)
build_prompt(175-247)call_gemini(146-172)get_env_or_exit(37-43)get_linked_issues(114-143)get_pr_commits(102-111)get_pr_details(66-78)get_pr_diff(81-99)post_github_comment(250-265)main(268-288)
🪛 Ruff (0.14.14)
.github/actions/changelog-generator/generate_changelog.py
[error] 54-54: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 56-56: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 89-89: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 91-91: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 140-142: try-except-continue detected, consider logging the exception
(S112)
[warning] 140-140: Do not catch blind exception: Exception
(BLE001)
[warning] 166-166: Abstract raise to an inner function
(TRY301)
[warning] 166-166: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 168-168: Consider moving this statement to an else block
(TRY300)
[warning] 170-170: Do not catch blind exception: Exception
(BLE001)
[error] 260-260: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 261-261: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 288-288: Avoid specifying long messages outside the exception class
(TRY003)
.github/actions/changelog-generator/test_generate_changelog.py
[warning] 10-10: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
[warning] 11-11: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
[warning] 13-13: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
⏰ 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). (12)
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | 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.12 | 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.11 | django~=4.2.0
🔇 Additional comments (19)
requirements-test.txt (1)
1-6: LGTM.
Adds the test dependency needed by the new generator tests..github/actions/changelog-generator/generate_changelog.py (8)
37-43: LGTM.
Straightforward env validation and exit behavior.
46-63: LGTM.
Clear error handling for GitHub API failures.
81-99: LGTM.
Diff truncation keeps prompt size bounded without failing the run.
102-111: LGTM.
Commit summary and 10‑item cap are sensible.
114-143: LGTM.
Issue extraction is clear and appropriately capped.
146-173: LGTM.
Good handling for empty responses and API errors.
250-265: LGTM.
Posting logic is straightforward and fails fast on non‑201 responses.
268-288: LGTM.
Clear orchestration of the end‑to‑end flow..github/actions/changelog-generator/action.yml (1)
1-45: LGTM.
Inputs are mapped cleanly to the generator’s expected environment variables..github/workflows/reusable-changelog-bot.yml (1)
56-84: LGTM.
Sparse checkout and the reaction acknowledgment look good..github/actions/changelog-generator/test_generate_changelog.py (8)
25-44: LGTM.
Covers success and failure paths for env handling.
46-81: LGTM.
Good coverage for PR details defaults and parsing.
83-113: LGTM.
Diff retrieval and truncation behavior are well tested.
115-142: LGTM.
Commit parsing and the 10‑item cap are validated.
144-184: LGTM.
Linked‑issue extraction cases are covered, including limits.
186-283: LGTM.
Gemini call behavior is thoroughly mocked and asserted.
285-347: LGTM.
Prompt construction tests cover labels, commits, issues, and truncation.
348-379: LGTM.
Posting behavior and non‑201 failures are validated.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
nemesifier
changed the title
5c11cbc to
0ac1017
Compare
0ac1017 to
57107b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/actions/changelog-generator/test_generate_changelog.py:
- Around line 4-22: Remove the unused "# noqa: E402" suppressions in the test
file: delete the E402 noqa markers on the unittest, unittest.mock, and
generate_changelog import lines (the top sys.path manipulation makes them
unnecessary). Update the imports to be plain (e.g., "import unittest", "from
unittest.mock import ...", and "from generate_changelog import ...") so Ruff no
longer reports RUF100; if you prefer keeping a suppression, instead add a
per-file Ruff ignore for E402 at the top of the file.
🧹 Nitpick comments (1)
.github/actions/changelog-generator/generate_changelog.py (1)
114-143: Make linked-issue selection deterministic.
Using a set makes issue selection order unpredictable; sorting before slicing keeps prompts stable.♻️ Proposed fix
- for issue_num in list(issue_numbers)[:3]: # Limit to 3 issues + for issue_num in sorted(issue_numbers, key=int)[:3]: # Limit to 3 issues
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/actions/changelog-generator/action.yml.github/actions/changelog-generator/generate_changelog.py.github/actions/changelog-generator/test_generate_changelog.py.github/workflows/changelog-bot.yml.github/workflows/reusable-changelog-bot.ymldocs/developer/changelog-bot.rstrequirements-test.txtruntests.py
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/developer/changelog-bot.rst
- .github/workflows/changelog-bot.yml
- runtests.py
- requirements-test.txt
- .github/actions/changelog-generator/action.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/changelog-generator/test_generate_changelog.py (1)
.github/actions/changelog-generator/generate_changelog.py (9)
build_prompt(175-250)call_gemini(146-172)get_env_or_exit(37-43)get_linked_issues(114-143)get_pr_commits(102-111)get_pr_details(66-78)get_pr_diff(81-99)post_github_comment(253-268)main(271-291)
🪛 Ruff (0.14.14)
.github/actions/changelog-generator/generate_changelog.py
[error] 54-54: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 56-56: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 89-89: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 91-91: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 140-142: try-except-continue detected, consider logging the exception
(S112)
[warning] 140-140: Do not catch blind exception: Exception
(BLE001)
[warning] 166-166: Abstract raise to an inner function
(TRY301)
[warning] 166-166: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 168-168: Consider moving this statement to an else block
(TRY300)
[warning] 170-170: Do not catch blind exception: Exception
(BLE001)
[error] 263-263: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 264-264: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 291-291: Avoid specifying long messages outside the exception class
(TRY003)
.github/actions/changelog-generator/test_generate_changelog.py
[warning] 10-10: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
[warning] 11-11: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
[warning] 13-13: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
⏰ 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.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~=4.2.0
- GitHub Check: Python==3.11 | 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.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.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.11 | django~=4.2.0
🔇 Additional comments (25)
.github/actions/changelog-generator/generate_changelog.py (11)
1-24: Clear module documentation.
Nicely documents usage, env vars, and changelog format expectations.
37-43: Solid env guard.
Straightforward validation and exit path.
46-63: API wrapper looks good.
Error handling and timeouts are clear.
66-78: PR details normalization is tidy.
Reasonable defaults and defensive coalescing.
81-99: Diff fetch with truncation is sensible.
Keeps prompt size bounded without hard-failing.
102-111: Commit parsing is clean.
Limits and message extraction are straightforward.
146-172: Response validation and error handling look fine.
Nice guard for empty responses.
175-250: Prompt assembly is well structured.
Clear sections and rules for consistent output.
253-268: Comment posting flow is straightforward.
Simple and clear response validation.
271-291: Main flow wiring looks good.
Good end-to-end sequence.
294-295: Entry-point hook is fine.
Standard and clear..github/actions/changelog-generator/test_generate_changelog.py (9)
25-44: Env handling tests are solid.
Covers presence, missing, and empty values.
46-81: PR detail tests look comprehensive.
Covers both populated and missing fields.
83-113: Diff tests cover truncation well.
Good validation of large diff handling.
115-142: Commit parsing tests are clear.
Limits and message extraction are validated.
144-184: Linked-issue extraction tests are thorough.
Good coverage of patterns and limits.
186-283: Gemini call tests are well mocked.
Nice coverage of config, errors, and outputs.
285-347: Prompt construction tests are clear.
Checks labels, commits, issues, and body truncation.
348-376: Comment posting tests look good.
Covers success and non‑201 failures.
378-379: Test runner hook is fine.
Standard unittest entry point..github/workflows/reusable-changelog-bot.yml (5)
1-20: Inputs and secrets are clearly defined.
Defaults and required secret look good.
21-33: Job guard and permissions look appropriate.
Clear trigger gating and scoped permissions.
35-37: PR number extraction is straightforward.
Simple and readable.
39-99: Pre-checks are comprehensive.
Covers merge status, recent changes, permissions, and approvals.
100-128: Checkout/reaction/generation sequence looks good.
Flow is clear and easy to follow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| system_instruction: str, | ||
| api_key: str, | ||
| model: str, | ||
| changelog_format: str = "rst", |
There was a problem hiding this comment.
WARNING: The changelog_format parameter is accepted by call_gemini but never used inside the function body. This is dead code that creates a misleading API — callers pass a format value that has no effect.
Either remove the parameter, or use it (e.g., to adjust max_output_tokens or the model config based on format).
| suspicious_patterns = [ | ||
| r"ignore\s+previous\s+instructions", | ||
| r"ignore_[a-z_]*instructions", | ||
| r"new\s+task", |
There was a problem hiding this comment.
WARNING: The pattern r"new\s+task" is overly broad and will cause false positives, rejecting legitimate changelog entries such as:
[feature] New task scheduler added[change] New task queue implementation
Consider tightening the pattern to match only injection-style phrasing, e.g., r"new\s+task\s*:" (requiring a colon), or remove it entirely since the system instruction already guards against this.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code or outside diff hunks:
Custom Instructions Checklist
Files Reviewed (6 files)
|
|
@coderabbitai are you happy? If so approve, if not summarize the outstanding issues. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of all relevant files before deciding. 🏁 Script executed: Length of output: 285 🏁 Script executed: Length of output: 885 🏁 Script executed: Length of output: 43479
All previously flagged issues are resolved ✅
Two minor non-blocking nits remain (not blocking approval):
[approve] 🧠 Learnings used |
nemesifier
changed the title
|
@coderabbitai why don't you approve the PR then? |
|
🧠 Learnings used✅ Actions performedComments resolved and changes approved. |
ebe23ea to
f2a71e2
Compare
github-project-automation
Bot
moved this from In progress
to Reviewer approved
in OpenWISP Priorities for next releases
nemesifier
changed the title
github-project-automation
Bot
moved this from Reviewer approved
to Done
in OpenWISP Priorities for next releases
github-project-automation
Bot
moved this from In progress
to Done
in OpenWISP Contributor's Board
Checklist
Reference to Existing Issue
Closes #523
Description of Changes
Changelog Bot Workflow
1. TRIGGER
@openwisp-bot changelogon it2. WORKFLOW ACTIVATION
changelog-bot.ymldetects the eventreusable-changelog-bot.yml3. ACKNOWLEDGMENT
4. ACTION SETUP
google-genaidependencyGITHUB_TOKEN,GEMINI_API_KEYPR_NUMBER,REPO_NAME,LLM_MODEL5. CHANGELOG GENERATION (generate_changelog.py)
6. POST COMMENT
\``rst` code blockCHANGES.rst7. TESTING
runtests.pyto include the test cases forgenerate_changlog.py.8. DOCUMENTATION
docs/developer/changelog-bot.rst