[MINOR] The Retry-After override only applies when attempt < len(backoff), which means on the last retryable attempt (attempt == maxRetryAttempts-2 == 1, with default backoff len 2) the cap and assignment do apply. However if the server returns 429 on attempt 1 (second attempt) and len(backoff) == 2, then attempt(1) < len(backoff)(2) is true and backoff[1] is set — but attempt 1 is the last attempt that will retry (attempt 2 would be maxRetryAttempts-1 == 2 which is NOT < maxRetryAttempts-1), so the stored backoff[1] is never consumed. This is a logic gap: the Retry-After value is parsed and stored but the delay is never applied on the final retry cycle. This is a subtle correctness issue but low impact since it only means the cap is computed unnecessarily on the last attempt.
[NIT] w.Write(...) return values are ignored in test handlers throughout. This is standard for httptest handlers, but adding //nolint or accepting the convention is fine. Not a real issue.
[NIT] The doRequest signature has accept string as a named parameter after reqURL string. Per the style patterns, prefer consistent parameter naming. Minor: the internal copy of backoff via append([]time.Duration(nil), c.retryBackoff...) is idiomatic but slightly obscure; slices.Clone (Go 1.21+) would be clearer if the project targets that version.
[NIT] The APIError.Body field is exported and documented as storing up to 64 KiB of raw response body. However Error() truncates to 200 bytes. The truncation happens after newline sanitization, meaning the sanitized string could still be 214+ chars. More importantly, truncation is applied to the local body variable but the field e.Body is not truncated — callers accessing err.(*APIError).Body directly get the full 64 KiB. This is documented in the comment, so it's intentional, but the ordering (truncate then sanitize) means a body that is exactly 200 bytes of content followed by newlines won't have those newlines sanitized if they fall within the first 200 chars. This is correct as-is but worth noting.
[MINOR] The http field name shadows the standard net/http package import within method bodies. While Go resolves this correctly (the import is referenced as http.Client, http.StatusNotFound, etc. at package level), naming a struct field http is a recognized source of confusion and a go vet / staticcheck warning in some configurations. Prefer httpClient or hc as the field name.
[MINOR] The Retry-After header override only applies when attempt < len(backoff). With maxRetryAttempts = 3 and the default backoff slice of length 2, this works for attempts 0 and 1 (covering both possible retries). But if SetRetryBackoff is called with an empty slice []time.Duration{}, the Retry-After delay is computed but never applied, silently falling back to a zero delay. The comment in the code says the delay is set via backoff[attempt] = delay, but if backoff is empty this branch is skipped. This is a subtle edge case that could confuse callers of SetRetryBackoff.
[MINOR] When delay == 0 (e.g., HTTP-date in the past, or Retry-After: 0), the timer is never created and the goroutine proceeds without sleeping — correct. However, when context is already cancelled and delay is 0, the cancelled context won't be checked before the next http.NewRequestWithContext call. This is fine because http.NewRequestWithContext will itself fail fast on a cancelled context, but it does mean a 429 response with a past HTTP-date on a cancelled context won't return ctx.Err() cleanly from the sleep select — it will instead return an HTTP transport error wrapped with 'do request'. The TestDoRequest_ContextCanceled test sets a non-zero backoff which masks this. Consider adding a select { case <-ctx.Done(): return nil, ctx.Err(); default: } guard at the top of each loop iteration, or simply accept the current behavior as the wrapped error is still propagated.
[MINOR] asAPIError is a helper used only by IsNotFound and IsUnauthorized. These two callers could directly use errors.As inline (2 lines each), making asAPIError unnecessary indirection. By the 'internal/ Packages' pattern, if it's only used locally unexported it's fine, but it adds no abstraction value here. NIT-level — not a real problem.
[NIT] Tests like TestDoRequest_429_RetryAfter_IntegerSeconds set Retry-After: 1 on the server but then override backoff to {0, 0}. The Retry-After: 1 value from the server will override backoff[0] to 1s (via parseRetryAfter), meaning the test will actually sleep for 1 second. This contradicts the intent of SetRetryBackoff([]time.Duration{0, 0}) to make the test fast. The test passes but takes ~1s longer than expected. Consider setting Retry-After: 0 in the server handler for this test, or verify this is intentional.
[NIT] w.Write([]byte(...)) return values are silently ignored in test handlers. This is idiomatic in test code, but some linters flag it. Adding //nolint or using fmt.Fprint is not necessary — just noting it follows the project's test style.
[MINOR] In doGetLimited, the overflow guard if limitBytes <= 0 { limitBytes = math.MaxInt64 } is dead code given the caller's pre-check (maxSize == math.MaxInt64 routes to the unlimited path before this function is ever called). The comment on the clamp says it handles maxBytes == math.MaxInt64, but that case never reaches doGetLimited. The guard is harmless and provides defense-in-depth, but the comment is misleading since it describes a case that is already excluded by the caller.
[NIT] The test uses defer server.Close() inside a table-driven t.Run loop. This is fine here since each subtest gets its own server, but it's worth noting that t.Cleanup(server.Close) would be more idiomatic per the testing patterns (ensures cleanup after the subtest completes, not just when the closure returns). Not a bug since both are equivalent in this pattern.
[MINOR] The MaxDiffSize field documentation says 'Set to any negative value (or math.MaxInt64) to disable the limit' but the convention for disabling is inconsistent. Negative values and math.MaxInt64 both map to the unlimited path, but the doc comment grouping math.MaxInt64 alongside negative values is slightly surprising since math.MaxInt64 is positive. Consider clarifying the doc: 'Set to a negative value to disable the limit. math.MaxInt64 is also treated as disabled to avoid overflow edge cases.'
[NIT] The error message returned on size exceeded wraps ErrDiffTooLarge correctly with %w, but the message says 'response is larger than %d bytes' which is technically inaccurate — the response is at least maxBytes+1 bytes. A slightly more accurate phrasing would be 'response exceeds %d bytes'. Minor readability nit.
[MINOR] doGetLimited duplicates the entire retry loop from doGet (backoff initialization, delay logic, request construction, error handling, retry conditions). This is ~60 lines of duplicated logic. A cleaner design would accept an optional response-body transformer/limiter function in a single internal method, or factor the retry machinery into a shared helper that accepts a per-attempt callback. As-is, any future fix to the retry logic (e.g., adding a new retriable error condition, changing log fields) must be applied in two places.
[NIT] The test server handler ignores write errors from w.Write([]byte(largeDiff)). This is idiomatic for test handlers and not a bug, but for completeness the existing codebase pattern is the same so this is consistent.