Python README: state the retry contract the code actually implements

[jeremy]

The Python README's Retry Behavior section still described the pre-#486 contract. Three claims were wrong or incomplete — all the same failure, the contract changed and the README didn't follow.

What was wrong

The status set. "Retried on RateLimitError (429), NetworkError, and retryable ApiError (500, 502, 503, 504)" — 500, 502 and 504 have not been retried since #486. The gate is the operation's declared retry_on set, and it is exhaustive in both directions. A reader planning around this expected three attempts on a 500 and got one.

The attempt budget. "Controlled by config.max_retries" — #483 made it min(config.max_retries, the operation's declared max). Not hypothetical: 43 of 226 operations declare max 2 against a default cap of 3, so they retry once fewer than the README promised.

The error table read as a retry prediction. It was accurate — 500 really is ApiError(retryable=True) — but nothing said that retryable is a taxonomy hint rather than a statement about the transport. That gap is what makes the first bullet's error believable. SPEC.md:432 already draws this line; the README hadn't.

Verification

Everything here is measured, not read.

Claim	How confirmed
226/226 operations declare [429, 503]	parsed generated/metadata.json — one distinct set
Declared max is 3 (183 ops) or 2 (43 ops)	same parse — the ceiling is live, not theoretical
429 → RateLimitError, 500/502/503/504 → ApiError, all retryable=True; 418 False	executed error_from_response
500/502/504 get one attempt; 429/503 get three	TestDeclaredRetryStatuses [500-1] [502-1] [504-1] [429-3] [503-3]
A declared max of 2 beats a caller cap of 5	TestPerOperationRetryMax CapTwoOp-5-2
Nothing else broke	make py-check — 508 passed; make check-retry-metadata-parity green

One process note worth recording. My first probe passed an httpx.Response where error_from_response wants an int status. It fell through to the else branch and reported 500 as retryable=False — which, taken at face value, would have had me "correct" an accurate table into an inaccurate one. Re-run against the real signature it gives the opposite answer. The table was right; the probe was wrong.

Scope

Documentation only — no code change, and no change to the error taxonomy itself, which is correct as it stands. This is the last of the three defects found while auditing #486's nine unresolved review threads; the other two shipped in #500.

---

Summary by cubic

Update the Python README to match the SDK’s actual retry behavior and scope the rules to Basecamp API operations. Clarifies which statuses retry, how the retry cap works, and that client.authorization.get() follows error classification.

• Bug Fixes
  • Which statuses: Only operation-declared statuses retry; all ops declare 429 and 503; 500/502/504 are not retried. Scoped to Basecamp API operations.
  • Max retries: min(config.max_retries, operation's declared maximum); the declared max can only lower the cap.
  • retryable flag: A classification hint for your code, not a retry promise (e.g., 500 is retryable=True but not retried for API operations).
  • Authorization: client.authorization.get() is not a Basecamp API operation; it retries whatever retryable reports — including 500/502/504 — bounded only by config.max_retries.

^{Written for commit 8985a28. Summary will update on new commits.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7cc526306

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

This PR updates the Python SDK README’s “Retry Behavior” section to match the retry contract the Python transport actually enforces (operation-declared retry_on + per-operation retry ceiling), and clarifies that BasecampError.retryable is an error taxonomy hint rather than a transport guarantee.

Changes:

• Clarify that retried HTTP statuses are governed by the operation’s declared retry_on set (and that 500/502/504 are not retried for governed operations).
• Clarify that the effective attempt budget is min(config.max_retries, operation_declared_max).
• Clarify that retryable is informational for callers and does not predict whether the SDK will retry.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Comments suppressed due to low confidence (2)

python/README.md:453

• The “declared set is exhaustive” statement is accurate for governed API operations, but the SDK also issues requests without operation metadata (e.g., AuthorizationService / get_absolute) where status-based retries fall back to the error’s retryable classification. As written, this can read as a global promise for all SDK requests.

- **Which statuses** - Only the statuses an operation declares retryable. Every operation declares `429, 503`, and the declared set is exhaustive: a status outside it — **including 500, 502, and 504** — is surfaced to you on the first attempt rather than retried
- **GET requests** - Retried on the declared statuses above, plus `NetworkError`, which carries no HTTP status and so is governed by its error classification instead

python/README.md:459

• Per-operation retry maxima are only available when an operation id is provided; requests without operation metadata (e.g., AuthorizationService / get_absolute) are not bounded by an operation-declared maximum. The README line currently reads like the min() ceiling applies universally.

- **Max retries** - `min(config.max_retries, the operation's declared maximum)`. `config.max_retries` is a total attempt count including the initial request (default: 3 attempts; `0` means a single request with no retry). An operation declaring a lower maximum wins — the declared value can only lower the cap, never raise it

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.