Sonnet Review
[NIT] The TestBuildLineToPositionMap_MultiHunk test verifies expected positions but doesn't check len(fileMap) the way TestBuildLineToPositionMap_SingleHunk does. Adding a length check would make the test stricter and catch any spurious extra entries.
[MINOR] The \ prefix check used to skip \ No newline at end of file markers (strings.HasPrefix(line, "\\")) would also inadvertently skip any other backslash-prefixed diff metadata lines. This is unlikely to be a problem in practice since unified diffs don't use backslash-prefixed lines for anything other than this marker, but a more precise check like strings.HasPrefix(line, "\\ ") (with trailing space) or matching the full string would be more defensive.
[NIT] The doc comment uses → (Unicode arrow) in the Returns line. This is fine for readability but slightly inconsistent with stdlib conventions which typically use plain ASCII (->) in godoc. Not a real issue.
[NIT] The json struct tags on ReviewComment and other types are present but there's no indication these types will be directly JSON-serialized by the vcs package itself — they're internal domain types exchanged between adapters and core logic. Adding json tags suggests these might be serialized to wire format, which could be misleading. If these types are purely internal, the tags add no value and could be omitted. This is a design question rather than a bug.
[NIT] The types.go file is missing a package doc comment or any reference back to the main package-level documentation. Since interfaces.go carries the package doc (// Package vcs defines...), this is fine per Go convention (only one file needs the package comment), but it's worth noting the package doc lives in interfaces.go rather than a dedicated doc.go, which is a slightly non-idiomatic placement for packages that will grow to have many files.
[MINOR] The compile-time interface check var _ vcs.Client = (*gitea.Client)(nil) is gated behind //go:build phase2 which means it never runs in normal CI. Its purpose is to document the gap, which is fine, but the comment in the PR description mentions a separate vcs/check.go with //go:build ignore — the actual implementation uses a _test.go file with //go:build phase2 instead. This is a reasonable choice (test files are safer than build-ignored files), but the PR description is slightly inaccurate about the mechanism. Not a code problem, just a documentation inconsistency between the PR description and the actual implementation.
[NIT] The file has no trailing newline after the last line (var _ Client = (*gitea.Client)(nil)). gofmt enforces a trailing newline; this may cause a gofmt check failure if one is run. Verify the file ends with a newline character.
[NIT] The anonymous struct for the Head field works but embedding a named struct would be slightly cleaner and more testable (allows construction via named literal without field-name lookup). This is a minor style preference; anonymous embedded structs in this pattern are common in JSON-mapping code.
[NIT] The anonymous struct for the User field inside Review has the same pattern. Same note as above — minor, acceptable for JSON-mapping types.
[MINOR] The package doc comment in types.go duplicates the one in interfaces.go almost verbatim. Only one file in a package should carry the package doc comment; the others should have no package comment or just package vcs with no preceding comment. The authoritative doc comment is on interfaces.go — remove the comment block from types.go.
[NIT] PostReview returns (*Review, error). Posting a review on most platforms (including Gitea and GitHub) returns a created review object, so this is reasonable. However, ListReviews returns []Review (value slice) while PostReview returns *Review (pointer). Consider making the return consistent — either []Review and Review, or []*Review and *Review — to avoid callers having to dereference in one case but not the other.
[NIT] The instruction 'Run go build -v . inside this file to see the documented gaps' is slightly misleading — you can't run a build inside a file. The intended instruction is probably 'Remove the //go:build ignore tag and run go build ./vcs/' or 'Run go build -tags ignore ./vcs/'. A clearer instruction avoids future confusion.
[MINOR] ReviewComment.CommitID is described as 'the commit SHA the comment is anchored to', but for Gitea (Phase 2 adapter) this will always need to be translated/populated. There is no enforcement that CommitID is non-empty. Consider noting in the doc comment whether CommitID is required or optional, and what the zero-value means, so adapter authors know what invariant to maintain.
[MINOR] TestBuildPatternPaths duplicates the path-building logic from fetchPatterns as a local closure rather than testing fetchPatterns directly. If the production logic in fetchPatterns changes, this test won't catch it. The test is testing a copy of the logic, not the function itself. Consider extracting buildPatternPaths as a package-level function and testing that instead, or writing an integration-style test that exercises fetchPatterns with a mock client.
[NIT] The Determine version step hardcodes REPO as 'rodin/review-bot' for the download source. On GitHub, this repo may not exist or be accessible, meaning the Install review-bot step would fail for GitHub Actions users trying to use this composite action from the public mirror. This is likely intentional (the composite action is primarily for Gitea) but deserves a comment.