Release v4.4.3 - #69
Conversation
📝 WalkthroughWalkthroughRetry callback handling and wait-state updates were simplified, package metadata was advanced to 4.4.3, and broad synchronous, asynchronous, edge-case, and fast-path tests were added to enforce 100% coverage. ChangesRetry behavior and coverage
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (7)
tests/test_coverage_gaps.py (1)
665-708: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese two cases duplicate
tests/test_edge_cases.py.
test_sync_loop_try_again_positive_wait/test_sync_loop_try_again_zero_waitmirrorTestAsyncLoopTryAgain(test_edge_cases.py Lines 1419-1461) andTestFastPathTryAgain(test_fast_path.py Lines 477-519) sync/async pairs. Since this PR explicitly removes overlap fromtest_coverage_gaps.py, consider keeping the TryAgain cases in one file.🤖 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/test_coverage_gaps.py` around lines 665 - 708, Remove the duplicate test_sync_loop_try_again_positive_wait and test_sync_loop_try_again_zero_wait cases from test_coverage_gaps.py. Keep the existing TryAgain coverage in the established test_edge_cases.py and test_fast_path.py suites unchanged.tests/test_edge_cases.py (1)
950-953: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the no-op contract.
reraise()is expected to return silently when the attempt holds no exception; make that explicit so a future behavior change (e.g. raisingRetryError) fails the test.assert err.reraise() is None🤖 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/test_edge_cases.py` around lines 950 - 953, Update test_retry_error_reraise_no_exception to explicitly assert that err.reraise() returns None when Attempt has no exception, preserving the no-op contract and failing if it starts raising or returning another value.pyproject.toml (1)
74-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider raising
fail_underto match the new 100% policy.
AGENTS.mdstep 8 now mandates a 100% TOTAL, but the automated gate is still 95, so a regression won't fail CI.♻️ Suggested change
[tool.coverage.report] -fail_under = 95 +fail_under = 100 show_missing = true🤖 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 `@pyproject.toml` around lines 74 - 77, Update the [tool.coverage.report] fail_under threshold from 95 to 100 so the automated coverage gate enforces the 100% TOTAL policy, while preserving the existing show_missing and skip_empty settings.tests/test_backon_sync.py (1)
254-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWiden
giveupannotations whereNoneis supported.
_make_retry_withalready accepts_Predicate[Exception] | None, butDecider.on_exceptionand other publicon_exceptionsignatures still default to a concrete predicate and rejectNonein the type, even though the implementation and tests rely ongiveup=None. Add| Noneto those signatures so the supported behavior is type-checked.🤖 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/test_backon_sync.py` around lines 254 - 263, Update Decider.on_exception and every public on_exception signature to annotate giveup as _Predicate[Exception] | None, matching _make_retry_with and the existing giveup=None behavior. Preserve the current default and runtime handling while widening only the type annotations.tests/test_fast_path.py (3)
963-981: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an awaitable no-op
sleepin async tests.
sleep=lambda s: Noneis only safe here because the computed wait is exactly0.0and never awaited; any change in wait computation turns these intoTypeErrorinstead of a meaningful assertion failure. Reuse the_noopcoroutine already defined at 928-929.Also applies to: 999-1030
🤖 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/test_fast_path.py` around lines 963 - 981, Replace the synchronous sleep lambda in test_try_again_stop_fires_async and the async tests at the referenced later range with the existing _noop awaitable coroutine. Keep the retry parameters and assertions unchanged.
563-682: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
_OneShotWaithelper.The same one-shot wait stub is redefined four times in this class (563-591, 593-621, 623-651, 653-681) and three more times in
TestFastPathMore(836, 927, 999). Hoist a single module-level helper (parametrizing the returned interval) and use it everywhere.♻️ Sketch
class _OneShotWait: def __init__(self, interval=0.0): self._interval = interval self._called = False def __call__(self, **kw): return _OneShotWait(self._interval) def next(self, send=None): if self._called: raise StopIteration self._called = True return self._interval🤖 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/test_fast_path.py` around lines 563 - 682, Extract the repeated _OneShotWait definitions into one module-level helper, preserving the current one-shot StopIteration behavior and allowing the returned interval to be configured via an interval parameter. Replace all four local definitions in the shown retry tests and the three corresponding definitions in TestFastPathMore with the shared helper, passing any required interval values.
353-451: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssertions don't verify the slow path was taken.
These tests only assert the exception propagates, which also holds on the fast path, so a regression in
_is_fast_pathrouting wouldn't fail them.callsintest_on_success_forces_slow_path(391-396) andtest_on_giveup_forces_slow_pathis collected but never asserted. Consider asserting_is_fast_path(...) is Falsefor the given config, or asserting exact handler invocation counts (e.g.on_giveupfires once).🤖 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/test_fast_path.py` around lines 353 - 451, Strengthen the slow-path tests in test_rate_limit_alone_forces_slow_path, test_on_success_forces_slow_path, test_on_backoff_forces_slow_path, and test_on_giveup_forces_slow_path so they verify slow-path-specific behavior rather than only exception propagation. Assert the relevant callback invocation counts, including the collected calls in the success and give-up tests, or directly validate _is_fast_path(...) is False for each configuration.
🤖 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 `@CHANGELOG.md`:
- Line 4: Update the CHANGELOG entry describing the ParamSpec stub in _typing.py
to state that it is the fallback for Python versions below 3.10, so it is
unreachable on Python 3.10 and newer while remaining reachable on supported
Python 3.9.
In `@tests/test_edge_cases.py`:
- Around line 1077-1099: Update test_on_exception_async_gen_disabled so
backon.disable() is paired with a try/finally block, placing the existing
generator iteration and assertions in the protected section and restoring global
state with backon.enable() in finally. Apply the same cleanup structure to the
related test covering lines 1101-1121, preserving its existing assertions and
behavior.
In `@tests/test_fast_path.py`:
- Around line 753-772: Update test_fast_inner_wait_gen_kwargs_none_async and any
other async _retry_fast_async_inner tests lacking a sleep override to inject a
no-op or very small async sleep while retaining jitter=None, so retries avoid
the wait generator’s default interval and each test stays under 100ms.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 74-77: Update the [tool.coverage.report] fail_under threshold from
95 to 100 so the automated coverage gate enforces the 100% TOTAL policy, while
preserving the existing show_missing and skip_empty settings.
In `@tests/test_backon_sync.py`:
- Around line 254-263: Update Decider.on_exception and every public on_exception
signature to annotate giveup as _Predicate[Exception] | None, matching
_make_retry_with and the existing giveup=None behavior. Preserve the current
default and runtime handling while widening only the type annotations.
In `@tests/test_coverage_gaps.py`:
- Around line 665-708: Remove the duplicate
test_sync_loop_try_again_positive_wait and test_sync_loop_try_again_zero_wait
cases from test_coverage_gaps.py. Keep the existing TryAgain coverage in the
established test_edge_cases.py and test_fast_path.py suites unchanged.
In `@tests/test_edge_cases.py`:
- Around line 950-953: Update test_retry_error_reraise_no_exception to
explicitly assert that err.reraise() returns None when Attempt has no exception,
preserving the no-op contract and failing if it starts raising or returning
another value.
In `@tests/test_fast_path.py`:
- Around line 963-981: Replace the synchronous sleep lambda in
test_try_again_stop_fires_async and the async tests at the referenced later
range with the existing _noop awaitable coroutine. Keep the retry parameters and
assertions unchanged.
- Around line 563-682: Extract the repeated _OneShotWait definitions into one
module-level helper, preserving the current one-shot StopIteration behavior and
allowing the returned interval to be configured via an interval parameter.
Replace all four local definitions in the shown retry tests and the three
corresponding definitions in TestFastPathMore with the shared helper, passing
any required interval values.
- Around line 353-451: Strengthen the slow-path tests in
test_rate_limit_alone_forces_slow_path, test_on_success_forces_slow_path,
test_on_backoff_forces_slow_path, and test_on_giveup_forces_slow_path so they
verify slow-path-specific behavior rather than only exception propagation.
Assert the relevant callback invocation counts, including the collected calls in
the success and give-up tests, or directly validate _is_fast_path(...) is False
for each configuration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 72c36cbb-f04d-4faa-b50d-4ccc8869ee4f
📒 Files selected for processing (12)
AGENTS.mdCHANGELOG.mdbackon/_common.pybackon/_decorator.pybackon/_retry/_decide.pybackon/_retry/_helpers.pybackon/_typing.pypyproject.tomltests/test_backon_sync.pytests/test_coverage_gaps.pytests/test_edge_cases.pytests/test_fast_path.py
| ## 4.4.3 - 2026-07-24 | ||
|
|
||
| - Remove dead code: 5 unreachable `if state.outcome is not None` guards in `_decide.py`, 2 `return True` dead-code statements in `_decorator.py` and `_helpers.py`, 1 always-true `if user_handlers is not None` guard in `_common.py`. | ||
| - Add `# pragma: no cover` to the `ParamSpec` stub in `_typing.py` (unreachable on Python 3.13+). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Inaccurate rationale. The stub is the < 3.10 fallback, so it's unreachable on 3.10+ (not just 3.13+) and still reachable on Python 3.9, which the project supports.
📝 Suggested wording
-- Add `# pragma: no cover` to the `ParamSpec` stub in `_typing.py` (unreachable on Python 3.13+).
+- Add `# pragma: no cover` to the `ParamSpec` stub in `_typing.py` (only executed on Python 3.9).📝 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.
| - Add `# pragma: no cover` to the `ParamSpec` stub in `_typing.py` (unreachable on Python 3.13+). | |
| - Add `# pragma: no cover` to the `ParamSpec` stub in `_typing.py` (only executed on Python 3.9). |
🤖 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 `@CHANGELOG.md` at line 4, Update the CHANGELOG entry describing the ParamSpec
stub in _typing.py to state that it is the fallback for Python versions below
3.10, so it is unreachable on Python 3.10 and newer while remaining reachable on
supported Python 3.9.
| async def test_on_exception_async_gen_disabled(self): | ||
| backon.disable() | ||
| calls = [] | ||
|
|
||
| @backon.on_exception( | ||
| backon.constant, | ||
| ValueError, | ||
| max_tries=3, | ||
| jitter=None, | ||
| interval=0.01, | ||
| sleep=lambda s: None, | ||
| logger=None, | ||
| ) | ||
| async def gen(): | ||
| calls.append(1) | ||
| yield 42 | ||
|
|
||
| result = [] | ||
| async for item in gen(): | ||
| result.append(item) | ||
| assert result == [42] | ||
| assert len(calls) == 1 | ||
| backon.enable() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wrap backon.disable() in try/finally to avoid leaking global state.
If any assertion (or the generator iteration) fails, backon.enable() never runs and every subsequent test in the session sees retries disabled. The equivalent tests in tests/test_fast_path.py (Lines 685-693) already use try/finally.
🛡️ Proposed fix (sync case; apply the same shape to the async one)
def test_on_exception_sync_gen_disabled(self):
backon.disable()
- calls = []
- ...
- result = list(gen())
- assert result == [42]
- assert len(calls) == 1
- backon.enable()
+ try:
+ calls = []
+ ...
+ result = list(gen())
+ assert result == [42]
+ assert len(calls) == 1
+ finally:
+ backon.enable()Also applies to: 1101-1121
🤖 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/test_edge_cases.py` around lines 1077 - 1099, Update
test_on_exception_async_gen_disabled so backon.disable() is paired with a
try/finally block, placing the existing generator iteration and assertions in
the protected section and restoring global state with backon.enable() in
finally. Apply the same cleanup structure to the related test covering lines
1101-1121, preserving its existing assertions and behavior.
| async def test_fast_inner_wait_gen_kwargs_none_async(self): | ||
| from backon._retry._fast import _retry_fast_async_inner | ||
|
|
||
| calls = [] | ||
|
|
||
| async def target(): | ||
| calls.append(1) | ||
| if len(calls) < 2: | ||
| raise ValueError("fail") | ||
| return "ok" | ||
|
|
||
| result = await _retry_fast_async_inner( | ||
| target, | ||
| backon.constant, | ||
| condition=backon.retry_if_exception_type(ValueError), | ||
| max_tries=3, | ||
| jitter=None, | ||
| wait_gen_kwargs=None, | ||
| ) | ||
| assert result == "ok" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Async test sleeps for the wait generator's default interval.
Unlike its sync counterpart (695-714), this test passes no sleep, so _retry_fast_async_inner falls back to asyncio.sleep and the single retry waits constant's default interval — well over the 100ms per-test budget. Same applies to the awaited default in any other async fast-inner test without a sleep override.
🐛 Proposed fix
+ async def _noop(s):
+ pass
+
result = await _retry_fast_async_inner(
target,
backon.constant,
condition=backon.retry_if_exception_type(ValueError),
max_tries=3,
jitter=None,
+ sleep=_noop,
wait_gen_kwargs=None,
)As per coding guidelines, "keep every test under 100ms using jitter=None and small intervals".
📝 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.
| async def test_fast_inner_wait_gen_kwargs_none_async(self): | |
| from backon._retry._fast import _retry_fast_async_inner | |
| calls = [] | |
| async def target(): | |
| calls.append(1) | |
| if len(calls) < 2: | |
| raise ValueError("fail") | |
| return "ok" | |
| result = await _retry_fast_async_inner( | |
| target, | |
| backon.constant, | |
| condition=backon.retry_if_exception_type(ValueError), | |
| max_tries=3, | |
| jitter=None, | |
| wait_gen_kwargs=None, | |
| ) | |
| assert result == "ok" | |
| async def test_fast_inner_wait_gen_kwargs_none_async(self): | |
| from backon._retry._fast import _retry_fast_async_inner | |
| calls = [] | |
| async def target(): | |
| calls.append(1) | |
| if len(calls) < 2: | |
| raise ValueError("fail") | |
| return "ok" | |
| async def _noop(s): | |
| pass | |
| result = await _retry_fast_async_inner( | |
| target, | |
| backon.constant, | |
| condition=backon.retry_if_exception_type(ValueError), | |
| max_tries=3, | |
| jitter=None, | |
| sleep=_noop, | |
| wait_gen_kwargs=None, | |
| ) | |
| assert result == "ok" |
🤖 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/test_fast_path.py` around lines 753 - 772, Update
test_fast_inner_wait_gen_kwargs_none_async and any other async
_retry_fast_async_inner tests lacking a sleep override to inject a no-op or very
small async sleep while retaining jitter=None, so retries avoid the wait
generator’s default interval and each test stays under 100ms.
Source: Coding guidelines
1 participant
4.4.3 - 2026-07-24
if state.outcome is not Noneguards in_decide.py, 2return Truedead-code statements in_decorator.pyand_helpers.py, 1 always-trueif user_handlers is not Noneguard in_common.py.# pragma: no coverto theParamSpecstub in_typing.py(unreachable on Python 3.13+).test_coverage_gaps.py.Summary by CodeRabbit
New Features
Bug Fixes
TryAgainflows.Documentation