[NIT] The State field doc comment lists APPROVE as a valid value, but the GitHub API returns APPROVED (not APPROVE) and the Gitea adapter maps to the same. The translation in translateGitHubReviewState only handles CHANGES_REQUESTED → REQUEST_CHANGES and COMMENTED → COMMENT; APPROVED passes through as-is. So the canonical state for an approved review is APPROVED (with a D), not APPROVE. The doc comment should say APPROVED to match what callers will actually see.
[NIT] In GetPullRequestDiff, the HTTP security check (rejecting plain HTTP) is duplicated inline rather than delegated to doRequest. This is a third copy of the same pattern (also in doRequest and doRequestWithBody). Consider extracting the scheme check into a private helper checkHTTPS(url string) error to eliminate the repetition.
[MINOR] The diffLineRanges type alias leaks a dependency on gitea.DiffLineRanges into the shared VCS client file. The comment acknowledges this, but it means any file importing cmd/review-bot/vcs_client.go implicitly depends on the gitea package even for GitHub-only workflows. A cleaner approach would be to have ParseDiffNewLines return an interface with a Contains(file string, line int) bool method, which would break the gitea coupling entirely.
[NIT] The translateGitHubReviewState comment mentions COMMENTED as a state that translates to COMMENT, but the GitHub API actually uses COMMENTED as the state on the stored review object when the event was COMMENT. The translation is correct, but the doc comment on PostReview says 'event strings (APPROVE, REQUEST_CHANGES, COMMENT)' — COMMENT here is the event while COMMENTED is the state, which are different fields. The code handles this correctly but the terminology in the comments could be clearer.
[NIT] inlineCommentInfo.NewPosition is named NewPosition (matching Gitea's field name) rather than something neutral like Line or Position. Since this is a shared adapter type in main, a more neutral name would be cleaner and wouldn't expose the Gitea naming convention in the abstraction layer.
[MINOR] The diffLineRanges type alias leaks gitea.DiffLineRanges into the main package even though the comment acknowledges this is only used by the Gitea adapter. This means the main package still carries a direct compile-time dependency on the gitea package through this alias, partially undermining the abstraction goal. An alternative would be to define an opaque diffChecker interface with a single Contains(file string, line int) bool method, which would remove the gitea dependency from giteaExtendedClient entirely and make the interface truly VCS-agnostic.
[MINOR] fetchPatterns type-asserts to *giteaClientVCSAdapter to access GetAllFilesInPath, which is not on the vcsClient interface. This is a known limitation documented with a warning log, but it means pattern fetching silently does nothing for GitHub. If GitHub pattern fetching is a planned feature, GetAllFilesInPath (or an equivalent recursive listing) should be added to vcsClient. If it's intentionally Gitea-only, a comment explaining the long-term plan would reduce future confusion.
[MINOR] There are two consecutive blank lines between GetPullRequest and GetPullRequestFiles (visible in the diff around the function boundary). gofmt normally allows at most one blank line between top-level declarations. This is a trivial formatting issue but violates the canonical formatting pattern.
[MINOR] The translateGitHubReviewState comment says COMMENTED maps to COMMENT, but the GitHub API actually uses COMMENTED (not CHANGES_REQUESTED) for comment-only reviews in list responses. This translation looks correct as written, but it's worth a test case for COMMENTED -> COMMENT in TestListReviews to document and lock in this behavior, since it's distinct from the CHANGES_REQUESTED -> REQUEST_CHANGES case that is already tested.
[NIT] TestPostReview_ConflictingCommitIDs creates a NewClient("tok", "https://api.github.com") without AllowInsecureHTTPForTest(). This is correct (it never makes a network request), but the test will fail if network is unavailable and the sentinel check somehow escapes. More importantly, this test correctly validates client-side validation, so no issue — just noting the intentional lack of a test server is fine here.
[NIT] getFileContentAtRef uses filepath as a parameter name, which shadows the standard library package path/filepath. While filepath is not imported in this file, the naming is potentially confusing. The gitea client likely uses a different name; consider filePath or path for consistency.
[MINOR] ErrNotSupported is declared but never used anywhere in the codebase (the giteaExtClient type assertion in main.go gates calls cleanly without needing this error). Either use it (e.g., have githubVCSAdapter return it from a hypothetical future extension method) or remove it to avoid dead code. The comment on the type says 'GitHub implementations return ErrNotSupported for those' but githubVCSAdapter doesn't implement giteaExtClient at all, so this sentinel serves no purpose currently.
[MINOR] The URL heuristic for VCS type detection hardcodes 'github.concur.com' as a special case alongside 'github.com'. This company-specific hostname leaks internal infrastructure details into open-source code and is brittle — any other GHES instance won't be detected. Since VCS_TYPE env var is the recommended path and the comment acknowledges this is a fallback for manual invocations, consider dropping the hardcoded GHES heuristic and just matching 'github.com', or document the pattern more generically (e.g., check for '/api/v3' in the URL).
[NIT] The comment on githubVCSAdapter.PostReview mentions that 'NewPosition from gitea diff parsing gives absolute line numbers, which will not match GitHub's position values' — this is accurate but the code proceeds to pass them anyway with Line+Side. Consider adding a TODO noting that proper GitHub diff hunk position mapping is needed for reliable inline comments, rather than just noting the limitation in a comment.
[NIT] In TestGetFileContent_Base64WithNewlines, the variable encoded is declared and then immediately suppressed with _ = encoded. The variable is unused because the test hardcodes the JSON string inline. Either remove the encoded variable declaration entirely or use it to construct the response body.
[MINOR] The giteaExtClient interface exposes []gitea.ReviewComment (a concrete gitea package type) through its ListReviewComments method signature. This breaks the abstraction layer — the interface in the consumer package (cmd/review-bot) now directly depends on the gitea package's internal types. Since giteaExtClient is Gitea-specific by design, this is pragmatically acceptable, but it's worth noting as a design smell. If a second Gitea-compatible VCS were added, the return type would need to change.
[MINOR] The init() function is used here to parse CIDRs and populate blockedCIDRs. Per the package-design pattern, init() is appropriate for registration-style setup, and this usage is acceptable. However, the parse errors are deferred to test time rather than panicking at startup. Since these are hard-coded literals that can never fail at runtime (barring a programming error), an immediate panic in init() would be more idiomatic (as used in regexp.MustCompile for compile-time-known values). The current approach is a conscious design choice documented in the code, so this is minor — but it is a deviation from the 'Must pattern' documented in api-conventions.md.