Skip to content

fix: annotate wait_base.__radd__ as taking the int seed from sum() - #677

Merged
mergify[bot] merged 2 commits into
mainfrom
devs/jd/chore/mypy-strictness/annotate-wait-base-radd-taking-int-seed-sum--ad1bcef4
Aug 5, 2026
Merged

fix: annotate wait_base.__radd__ as taking the int seed from sum()#677
mergify[bot] merged 2 commits into
mainfrom
devs/jd/chore/mypy-strictness/annotate-wait-base-radd-taking-int-seed-sum--ad1bcef4

Conversation

@jd

@jd jd commented Aug 5, 2026

Copy link
Copy Markdown
Owner

__radd__ was annotated other: wait_base, which is the one type it can
never receive: wait_base.__add__ always succeeds, so Python never falls
back to the reflected operator for two wait strategies. The only caller is
sum(), which seeds its accumulator with the int 0 — hence the
type: ignore[comparison-overlap] on other == 0, and the dead
return self branch behind it that --warn-unreachable flags.

Annotate the parameter as int and return NotImplemented for a non-zero
left operand, so 5 + wait_fixed(1) raises TypeError at the addition
rather than building a wait_combine that explodes later when called.

With the signature corrected, mypy infers sum([...]) over wait strategies
on its own, so four type: ignore[list-item] comments in
test_wait_arbitrary_sum are no longer needed.

Depends-On: #683

@jd

jd commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

This pull request is part of a Mergify stack:

# Pull Request Link
1 ci: run on pull requests targeting any base branch #683
2 fix: annotate wait_base.radd as taking the int seed from sum() #677 👈
3 chore(mypy): enable the strictness options strict leaves off #678
4 refactor: drop always-true truthiness checks and enable truthy-bool #679
5 refactor: declare BaseAction's REPR_FIELDS and NAME as ClassVar #680
6 test: fix possibly-undefined and deprecated call sites #681
7 refactor: mark overridden methods with @OverRide #682

jd added 2 commits August 5, 2026 17:38
`on.pull_request.branches` filters on the *base* branch, not the head. With
it pinned to `main`, only a pull request merging directly into `main` ran
CI -- every stacked pull request, whose base is the branch below it in the
stack, got no test, lint or mypy run at all and showed nothing but the
Mergify checks.

Drop the filter so CI runs for every pull request regardless of base.

Change-Id: I1c211a106a57b9f4986bec4b76c0680c1a0f1b61
`__radd__` was annotated `other: wait_base`, which is the one type it can
never receive: `wait_base.__add__` always succeeds, so Python never falls
back to the reflected operator for two wait strategies. The only caller is
`sum()`, which seeds its accumulator with the int 0 — hence the
`type: ignore[comparison-overlap]` on `other == 0`, and the dead
`return self` branch behind it that `--warn-unreachable` flags.

Annotate the parameter as `int` and return `NotImplemented` for a non-zero
left operand, so `5 + wait_fixed(1)` raises `TypeError` at the addition
rather than building a `wait_combine` that explodes later when called.

With the signature corrected, mypy infers `sum([...])` over wait strategies
on its own, so four `type: ignore[list-item]` comments in
`test_wait_arbitrary_sum` are no longer needed.

Change-Id: Iad1bcef4412d6afc1a6f1335970cfbfcc4a34fcb
@jd
jd force-pushed the devs/jd/chore/mypy-strictness/annotate-wait-base-radd-taking-int-seed-sum--ad1bcef4 branch from b02c344 to ae08acf Compare August 5, 2026 15:39
@jd
jd changed the base branch from main to devs/jd/chore/mypy-strictness/run-pull-reqs-targeting-base-branch--1c211a10 August 5, 2026 15:39
@jd

jd commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

Revision history

# Type Changes Reason Date
1 initial b02c344 2026-08-05 15:39 UTC
2 rebase b02c344 → ae08acf (rebase only) 2026-08-05 15:39 UTC

Base automatically changed from devs/jd/chore/mypy-strictness/run-pull-reqs-targeting-base-branch--1c211a10 to main August 5, 2026 15:41
@mergify mergify Bot added the queued label Aug 5, 2026
@mergify

mergify Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 1 minute 2 seconds in the queue, including 25 seconds running CI.

Required conditions to merge

@mergify
mergify Bot merged commit 7c3c45c into main Aug 5, 2026
9 checks passed
@mergify
mergify Bot deleted the devs/jd/chore/mypy-strictness/annotate-wait-base-radd-taking-int-seed-sum--ad1bcef4 branch August 5, 2026 15:42
@mergify mergify Bot removed the queued label Aug 5, 2026
mergify Bot pushed a commit that referenced this pull request Aug 6, 2026
)

Three runtime regressions from the mypy-strictness stack (#679, #677), all
A/B verified against a2af454. None were caught by the test suite, and all
three only fire once a retry actually happens, so happy-path callers see
nothing until production.

1. `Retrying(wait=None)`, `wait=0` and `wait=sum([])` now raise
   `TypeError: 'NoneType' object is not callable` from inside `iter()`.
   #679 removed the `if self.wait:` guard on the grounds that `truthy-bool`
   proved it always true -- but that only holds for the *declared* type.
   Untyped callers pass `None`/`0` to mean "no wait", and `sum([])` over an
   empty strategy list returns the int 0. The `TypeError` is raised outside
   the attempt's try/except, so it escapes uncaught and destroys the caller's
   real exception. Restore the guard with a local `truthy-bool` suppression
   explaining why the "impossible" branch is reachable. `before`, `after`,
   `before_sleep` and `retry_error_callback` all kept their `is not None`
   guards; `wait` was the only one dropped.

2. `plain_callable + wait_strategy` now raises `TypeError`. `WaitBaseT`
   admits plain callables, and a function has no `__add__`, so this went
   through `wait_base.__radd__` -- which #677 narrowed to `int`. Widen it to
   `WaitBaseT | int` and build the `wait_combine` again. `5 + strategy` is
   still rejected, now via `NotImplemented` rather than a combination that
   fails later. The parameter stays `int` rather than `Literal[0]` because
   typeshed's `sum()` protocol requires `__radd__(x: int)`; narrowing it
   would make every `sum()` over strategies need a `type: ignore`.

3. `wait_combine.__call__` passed the state as `retry_state=`, which crashes
   on any `WaitBaseT` callable whose parameter has another name. Pass it
   positionally, as `BaseRetrying._run_wait` already does.

Adds regression tests for all three plus the async path -- the original
change shipped with no test covering any of them.

Change-Id: Ia40c32a22cdac09cbfba4280fb0a7f0c2cb7b7e0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant