feat: implement fallback behavior for compare API 404 errors in DataM…#323
feat: implement fallback behavior for compare API 404 errors in DataM…#323miroslavpojer wants to merge 11 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/compare_mode.md`:
- Around line 131-134: The fenced log example in the compare mode documentation
is missing a language tag, which triggers markdownlint MD040. Update the fenced
block around the compare output so it uses a simple fence language such as text
or log, and keep the content under the same documentation section that
references repo.compare and the fallback warning.
In `@release_notes_generator/data/miner.py`:
- Around line 111-122: The fallback in miner’s compare flow is too broad because
`self._safe_call(repo.compare)` loses the failure reason, so `comparison is
None` currently triggers on auth, rate-limit, and transient errors as well as
missing refs. Update the `comparison` handling in `Miner` to preserve or inspect
the compare error and only enter the `repo.get_commit` fallback when the compare
failure is specifically a verified not-found case for the target tag; otherwise
surface the error and do not downgrade to a single-commit release note path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c06740a-9452-4591-b9df-88f1d1d96701
📒 Files selected for processing (3)
docs/features/compare_mode.mdrelease_notes_generator/data/miner.pytests/unit/release_notes_generator/data/test_miner.py
There was a problem hiding this comment.
Pull request overview
Implements a compare-mode fallback so the action can continue when repo.compare(from_tag, to_tag) fails, aiming to reduce CI race-condition failures around tag availability.
Changes:
- Added compare-mode fallback in
DataMiner._handle_compare_mode()that resolves the target ref viarepo.get_commit(to_tag)whencompare()yields no result. - Added unit tests for the fallback path and for the “fallback also fails” exit path.
- Documented the new fallback behavior in compare mode docs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
release_notes_generator/data/miner.py |
Adds fallback behavior when compare returns None, logging a warning and attempting to resolve the target ref to a commit. |
tests/unit/release_notes_generator/data/test_miner.py |
Adds two new tests covering fallback success and fallback failure. |
docs/features/compare_mode.md |
Documents the fallback strategy and expected behavior/logging. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/unit/release_notes_generator/data/test_miner.py (1)
309-309: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStop patching
DataMinerinternals in the tests.These assignments couple the suite to
_safe_calland_rate_limiterinstead of the public constructor/collaborator seams. Prefer passing a controllable limiter fixture or patching the decorator at the module boundary. As per coding guidelines, "Must not access private members (names starting with_) of the class under test directly in tests".Also applies to: 336-336, 359-359, 408-408, 458-458, 481-481, 609-609, 791-792, 836-837, 876-877, 906-907
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/release_notes_generator/data/test_miner.py` at line 309, Stop assigning to DataMiner private members in the tests; replace direct use of _safe_call and _rate_limiter with public setup points. Update the affected test cases in test_miner.py to inject a controllable limiter or patch the decorator at the module boundary, using DataMiner’s constructor/collaborator seams instead of mutating internals.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@release_notes_generator/data/miner.py`:
- Around line 147-158: The fallback in miner.py’s compare flow is using
repo.default_branch instead of the requested target ref, which changes
release-note scope. Update the comparison recovery logic in the branch around
comparison, repo.get_branch, and repo.compare so that when the target tag is
missing it resolves the latest commit for the requested target ref/tag rather
than defaulting to repo.default_branch, and then retries compare with that
resolved commit SHA.
- Around line 157-159: The retry path in the compare flow is missing the same
exception handling as the initial `repo.compare()` call, so failures from the
fallback can escape uncaught. Move the retry compare in `Miner` into the same
`try`/`except` handling used for the first compare, or add equivalent handling
around the `_rate_limiter(repo.compare)` retry. Make sure timeouts and non-404
`GithubException` on the retry still log the error and exit with the same
`SystemExit` behavior as the original path.
---
Nitpick comments:
In `@tests/unit/release_notes_generator/data/test_miner.py`:
- Line 309: Stop assigning to DataMiner private members in the tests; replace
direct use of _safe_call and _rate_limiter with public setup points. Update the
affected test cases in test_miner.py to inject a controllable limiter or patch
the decorator at the module boundary, using DataMiner’s constructor/collaborator
seams instead of mutating internals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d6ae028-fca1-4b5d-aab3-cfe2c74b7b22
📒 Files selected for processing (2)
release_notes_generator/data/miner.pytests/unit/release_notes_generator/data/test_miner.py
…ult branch commit
…irectly and improve error handling
|
Closing this PR as the solution is too complex and in wrong direction. |
Solution
When the compare API fails with a 404 (target tag doesn't exist yet), the action now gracefully falls back to fetching the latest commit SHA of the target tag. A warning is logged to indicate the fallback occurred. This solves race-condition scenarios in CI/CD pipelines where the release notes generator runs before the tag is fully created.
Release Notes
Files Changed
release_notes_generator/data/miner.py— Fallback logic in_handle_compare_mode()tests/unit/release_notes_generator/data/test_miner.py— 2 new fallback testsdocs/features/compare_mode.md— Fallback behavior documentationCloses #322
Summary by CodeRabbit