[NIT] The disabled-limit branch (maxSize < 0) wraps the error with 'fetch diff: %w' but the limited branch also wraps with the same prefix inside the return. This is fine, but worth noting that the error string convention is consistent between the two paths.
[MINOR] The doRequest method mutates the backoff slice in-place (backoff[attempt] = delay) when a Retry-After header is received. When c.retryBackoff is non-nil, backoff is a direct reference to it (not a copy), so this mutation modifies the Client's shared retryBackoff slice. The doc comment says the Client is safe for concurrent use, but concurrent requests could race on this write. Even for sequential use, the backoff values persist across calls. Either copy the backoff slice at the top of doRequest, or use a local variable for the delay rather than mutating the slice.
[MINOR] The context cancellation test (TestDoRequest_ContextCanceled) cancels the context before calling doGet, which means the first HTTP request itself will likely fail with a context error rather than the retry timer being cancelled. The timer-cancellation path (the select with timer.C and ctx.Done()) is not actually exercised. Consider cancelling the context after the first 429 response is received (e.g. via a channel signalled from the handler) to properly test the timer interruption path.
[MINOR] TestDoRequest_429_RetryAfter_IntegerSeconds contains a large block of commented-out reasoning text and dead code (the first srv is created, a client c is set up but never used, then srv.Close() is called prematurely). This leaves a closed server and an unused client in the test. The test logic is repeated via srv2/c2. The function should be cleaned up to remove the dead first half and just use the working second server directly. The comments read like unresolved thinking rather than explanatory documentation.
[NIT] TestDoRequest_429_RetryAfter_HTTPDate sets initial backoff to {0, 0} and a Retry-After of 1 second — this means the test will actually sleep for 1 second (the override sets backoff[0]=1s, and that delay is applied before attempt 1). This is a real 1-second test. For a fast test suite, consider using Retry-After: 0 or mocking the sleep mechanism, or accepting this as an intentional slow test and documenting it.
[NIT] Import ordering has "errors" before "context", which violates the standard goimports ordering (stdlib packages in alphabetical order). This would be caught by goimports/gofmt and should be fixed for consistency.
[NIT] The doRequest function signature uses a named parameter accept string rather than acceptHeader string or similar. The single-word name is fine, but worth noting the parameter shadows no built-in. Minor style nit — no action required.
[MINOR] doGetLimited duplicates roughly 80 lines of retry logic that already exists in doGet. The only difference is the size-limiting read on the success path. This violates DRY and means any future change to retry behavior (new backoff strategy, different log fields, new error classification) must be made in two places. A cleaner approach would be to extract the retry loop into a shared helper that accepts a response-body handler function, or to add an optional maxBytes int64 parameter to doGet (where 0 means unlimited). The current duplication is a maintenance hazard.
[MINOR] The maxSize < 0 check uses the negated condition to dispatch to doGet for the unlimited path. Since DefaultMaxDiffSize is already set when maxSize == 0, the check maxSize < 0 is correct, but the comment says "disabled" while the field doc says "-1 to disable". An explicit maxSize == -1 check (or const noLimit int64 = -1) would make the intent clearer and prevent confusion if someone passes e.g. -2.
[NIT] The w.Write return values are silently discarded in all test handlers. While harmless in httptest servers, the convention in this codebase (and in stdlib examples) is to at least acknowledge the return. Not a correctness issue.
[NIT] The --gitea-url alias comment block is long (12 lines) and explains a subtle flag.StringVar trick. Consider extracting the alias registration to a small helper or at minimum shortening the comment to the key invariant: flag.StringVar shares the pointer, so whichever flag is set last wins. The current comment is accurate but its length makes it easy to miss the ORDERING constraint.
[NIT] The ListReviews page-limit warning comment (// NOTE: This warning only fires when...) is placed after the if page == maxReviewPages block but before the loop's closing brace. The comment is correct but its placement after the action it describes is mildly confusing. Moving it above the if block would improve readability.
[MINOR] The default case in the VCS client initialization switch (default: fmt.Fprintf(os.Stderr, ...) ; os.Exit(1)) is unreachable dead code: the provider is already validated by the earlier switch at line 109-114 which also exits on invalid values. This is harmless but adds noise.
[MINOR] The supersedeOldReviews function uses a switch with a fall-through comment for gitea case (// Continue to Gitea-specific logic below the switch.) followed by a type assertion on the concrete *gitea.Adapter. This is idiomatic Go for provider dispatch, but the pattern leaks the concrete gitea type through the vcs.Client abstraction boundary in main.go. The existing comment acknowledges this, but a cleaner alternative would be to add a Supersede(ctx, ...) method to the vcs.Client interface or a separate GiteaSuperseder interface. Not a bug for now since the provider switch validates upfront, but a future decorator wrapping gitea.Adapter would silently break this.
[MINOR] There is a double blank line between the closing brace of the github case and the Gitea-specific code (after case "gitea": // Continue to Gitea-specific logic below the switch.). Minor formatting issue that gofmt would catch — verify this was intentional.