[NIT] The commit-ID selection logic ('Use CommitID from the first comment that has one') applies it to all inline comments in the review, but GitHub actually requires every inline comment to be on the same commit. This is correct behavior, but the comment implies it's a best-effort choice when it should be a guaranteed invariant. Worth adding a note that the caller (main.go) is expected to set CommitID to pr.Head.SHA on all comments.
[MINOR] doJSONRequest does not retry on HTTP 429 like doRequest does. Since PostReview and DismissReview are write operations that can be rate-limited by GitHub, missing retry logic here could cause failures in high-traffic repos. Consider either adding retry logic or documenting that write operations are not retried.
[NIT] The --gitea-url alias registration uses *vcsURL as the default value at registration time. This works correctly because flag parsing hasn't happened yet, so *vcsURL holds the env-var-resolved default. However this is subtle — if the evaluation order ever changed (e.g., someone moves flag.Parse() above this line), the alias would silently use an empty string default instead. A brief comment noting this dependency on registration-before-parse order would help future readers.
[MINOR] In supersedeOldReviews, the GitHub case iterates over oldReviews and logs a warning on DismissReview failure but continues without returning an error. The function still returns nil after the loop even if all dismissals failed. This is intentional (best-effort dismissal) but the caller in main() calls os.Exit(1) on any error returned — so failures here are silently swallowed. Consider either returning a joined error for all failures or at minimum documenting the best-effort semantics in the function doc comment.
[MINOR] The --gitea-url backward-compatible alias is registered with flag.StringVar(vcsURL, "gitea-url", *vcsURL, ...). This dereferences *vcsURL at flag registration time to capture the current default, which is correct. However, the comment says 'whichever flag is set last by flag.Parse wins' — this is accurate but the behavior may surprise users if both --vcs-url and --gitea-url are passed simultaneously (last one wins rather than an error). This is a known limitation of the flag package alias approach and should ideally be documented in the flag usage or a warning emitted if both are set.
[MINOR] doJSONRequest is defined in github/reviews.go but is a general HTTP helper. If other files in the github package (e.g., the client file not shown in the diff) also define HTTP helpers, this may cause a naming conflict or duplication. Consider whether this belongs in a shared internal helper or the main client file. This is a structural concern rather than a correctness issue.
[NIT] The translateReviewEvent function has a case vcs.ReviewEventComment: return "COMMENT" followed by default: return "COMMENT". The default case is unreachable given the current vcs.ReviewEvent type but is harmless. No change needed; just noting it's redundant.
[NIT] Several test handlers use t.Fatalf inside the handler goroutine (e.g. t.Fatalf("expected POST, got %s", r.Method)). Calling t.Fatalf from a goroutine other than the test goroutine is technically not permitted per the testing package docs — it calls runtime.Goexit() on the wrong goroutine. The handler runs in the httptest server's goroutine, not the test's goroutine. For assertions in handlers that must stop the test, t.Errorf + return is safer. This is a pre-existing pattern in the codebase (not introduced here) so it's a low-risk nit, but new test files could use the safer t.Errorf + return pattern.
[NIT] The doRequestWithBody function only sets Content-Type: application/json when reqBody != nil. For the nil-body DELETE case (used by DeleteReview), no Content-Type is set, which is correct. However, the function's doc comment says "sets Content-Type to application/json" without qualifying the nil case. Minor doc improvement: "sets Content-Type to application/json when body is non-nil".
[NIT] DeleteReview calls doRequestWithBody with a nil body. The existing doGet wrapper (which calls doRequest → doRequestCore) would work just as well for a bodyless DELETE, but since DELETE with a body is unusual and doRequestWithBody(ctx, DELETE, url, nil) correctly produces a bodyless request (the nil-body branch in doRequestWithBody), this is functionally fine. Consider adding a brief comment clarifying that the nil is intentional and not an oversight (the existing inline comment already does this, so this is truly minor).
[MINOR] The --gitea-url alias registration uses flag.StringVar(vcsURL, "gitea-url", *vcsURL, ...) after vcsURL has already been parsed from env vars. This means the default for the alias is whatever vcsURL resolved to at registration time, not a live binding. In practice this works fine because flag.Parse() hasn't run yet, but it's subtly fragile: if the env-var chain for vcsURL ever produces an empty string, the alias and the primary flag start with different defaults. A comment explaining this would help future maintainers.
[MINOR] doJSONRequest is defined on *Client in this file but conceptually duplicates HTTP infrastructure that likely already exists elsewhere in the github package (the file references c.doGet, c.doRequest, c.allowInsecureHTTP, c.httpClient, c.token, userAgent, maxResponseBytes — all of which must live in another file). This is fine architecturally, but the HTTPS-enforcement check inside doJSONRequest is inconsistent with doGet/doRequest which presumably handle their own security. If those methods don't enforce HTTPS, the security posture is uneven. If they do, the duplication is unnecessary. Worth ensuring the pattern is consistent across the package.
[MINOR] supersedeOldReviews uses a switch with a case "gitea": fall-through comment but doesn't actually use the Go fall-through mechanism — it just breaks out of the switch and continues below. This is correct behavior but the comment 'Fall through to Gitea-specific logic below the switch' is slightly misleading since Go switch cases don't fall through by default and no fallthrough keyword is used. The intent is clear but the comment could be reworded to 'continue to Gitea-specific logic below' to avoid confusion.
[NIT] The CommitID on the review request is taken from the first comment that has one. If the reviews have multiple comments with different CommitIDs (which shouldn't happen in practice but is structurally possible), the behavior is implicit. A brief comment stating 'all comments are expected to reference the same commit' would make the intent explicit.