[MINOR] The type assertion client.(*gitea.Adapter) inside supersedeOldReviews couples this package-level function to the gitea package's internal concrete type. The comment acknowledges this is intentional and guarded, but if a decorator/wrapper is ever added around the adapter, this silently breaks. A cleaner long-term design would be to add a Gitea() GiteaUnderlying interface method or pass underlying directly rather than asserting. This is acceptable for now given the comment explains the rationale, but worth tracking.
[NIT] The comment on the page-limit warning says 'The loop naturally exits after this iteration since page increments past maxReviewPages.' This is slightly misleading: the loop condition is page <= maxReviewPages, so when page == maxReviewPages and this warning fires, the loop body completes and then page++ makes it maxReviewPages+1, failing the condition. The behavior is correct, but the comment could be clearer: 'The loop exits on the next iteration check since page will be incremented to maxReviewPages+1.'
[NIT] There is a stray blank line before // findAllOwnReviews returns all non-superseded reviews matching the sentinel. after the extractSentinelName function body closing brace. Minor cosmetic issue.
[MINOR] The supersedeOldReviews function accepts vcsURL string but for the GitHub case it is never used. For the default case in the outer switch it returns an error, so the parameter is effectively only used in the gitea path. This is fine functionally, but could cause confusion — a comment explaining the parameter is gitea-only would help.
[MINOR] The type assertion client.(*gitea.Adapter) in supersedeOldReviews is fragile. If the gitea provider is ever initialized differently (e.g. a wrapper or decorator), this assertion silently falls through to the error. The function already receives provider string — this is a valid guard, but the assertion is load-bearing for the gitea path and there's no test covering the !ok failure branch. A doc comment noting that the assertion is expected to always succeed given the prior provider switch (enforced by the caller) would make this safer to maintain.
[NIT] The VCS provider validation switch comes after setupLogger but before validateReviewerName. The ordering is sensible but provider validation could reasonably be done before logging setup since it's a hard prerequisite. Current ordering is fine — just noting the implicit ordering contract.
[NIT] The default case in the VCS client factory switch (lines ~188-191) sets fmt.Fprintf(os.Stderr, ...) and os.Exit(1). This is unreachable dead code since the same provider values are validated in the earlier switch at line 109-115. It's defensive coding which is fine, but it's worth noting in a comment that this case cannot be reached given the prior validation.
[MINOR] In ListReviews, the warning log at page == maxReviewPages is placed after the inner loop that appended reviews from the final page, but the break after the len(reviews) < reviewsPerPage check means the warning only fires when the full page was received AND we hit the limit. This is intentional but the condition order (check truncation before the page-limit log) makes it slightly non-obvious. No functional bug, but the log fires on the iteration where we exit via page == maxReviewPages after the loop body completes — which is correct. A minor ordering note.
[NIT] The DeleteReview method is implemented but appears to have no callers (GitHub supersede uses DismissReview). This is fine as part of satisfying the vcs.Client interface if that interface requires it, but if it's an extra method not in the interface, it adds untested surface area. The conformance test confirms vcs.Client is satisfied, so if DeleteReview is in the interface this is fine; otherwise it's unused public API.
[MAJOR] doJSONRequest duplicates ~100 lines of retry logic that already exists in doRequest. Both functions implement identical 429-retry-with-backoff loops, HTTPS validation, and Retry-After header parsing. This violates DRY and doubles the maintenance burden — a future change to retry semantics must be applied in two places. The preferred approach would be to make doRequest accept an optional body (io.Reader) or to extract the shared retry scaffolding into a single private helper that both GET and write operations call.
[MINOR] c.SetRetryBackoff(...) return value is not checked in TestDoJSONRequest_429Retry. The call on line 613 ignores the error. All other tests in the file use if err := c.SetRetryBackoff(...); err != nil { t.Fatalf(...) }. Should be consistent.
[NIT] Import block has "bytes" and "encoding/json" before "context" — goimports/gofmt ordering puts context first within the stdlib group. Minor formatting inconsistency.
[MINOR] supersedeOldReviews takes provider and vcsURL as plain strings rather than deriving them from the client value. This means the provider switch inside the function can diverge from the actual client type at runtime — e.g., passing provider="gitea" with a *github.Client would fall through to the type-assertion that expects *gitea.Adapter and return a confusing error. A type switch on client directly (or a provider-specific supersede method on the interface) would be more robust and remove the stringly-typed dispatch.
[NIT] findOwnReview is defined but no longer called anywhere in main.go after this refactor — findAllOwnReviews replaced it. Dead code should be removed or the function should be unexported with a //nolint if intentionally kept for future use.
[MINOR] ListReviews paginates up to maxReviewPages=100 pages of 100 reviews each (10,000 reviews). For typical PRs this is fine, but the cap is silent — if a PR somehow exceeds 10,000 reviews the function returns a truncated list with no warning. Consider logging a warning when page == maxReviewPages && len(reviews) == reviewsPerPage to indicate potential truncation, consistent with how other bounded loops in the codebase behave.
[MINOR] The supersedeOldReviews function accepts provider as a plain string and then type-asserts client.(*gitea.Adapter) for the gitea case. This couples the supersede logic to the concrete Gitea type rather than going through the interface. The type assertion failure path returns an error (good), but the design creates a hidden coupling: adding a new provider that also needs Gitea-style supersede would require modifying this function. Consider whether DismissReview-style behavior should be part of vcs.Client or whether the Gitea-specific path should be documented as intentionally exceptional.