[MINOR] The commitStatusResponse.State field is parsed from the API response but never used — only the individual Statuses slice entries are iterated. This is a minor dead field. Either use State to short-circuit when overall state is known, or remove it from the struct to avoid confusion about its purpose.
[NIT] The doRequest method has accept string as the last parameter. Per Go idiom (style.md receiver naming / api-conventions), named parameters for Accept headers are fine, but the parameter name accept conflicts with the common Go pattern of using accept only in request context. Not a correctness issue — purely style.
[MINOR] After resp.Body.Close() is called for a successful response, if io.ReadAll returns an error, the body is already closed which is correct, but the success path reads up to maxResponseBytes+1 bytes via io.LimitReader. This is the intentional over-read trick to detect exceeding the limit, which is correct. However, on the error path for non-2xx responses (line 245), errBody, _ := io.ReadAll(...) silently discards the read error. While acceptable in practice since it's for error body capture, the convention in this codebase is to check all error returns. This is minor since the worst case is an empty error body.
[NIT] The GetFileContentAtRef method is placed in pr.go but it's also the implementation delegate for GetFileContent in files.go. Conceptually it belongs in files.go alongside the other file-related methods. The split works because Go doesn't care which file a method is in, but for navigability (per the style.md pattern of organizing by concept) moving it to files.go would be cleaner.
[MINOR] The validated map stores depth as the value but is described as storing 'the maximum depth at which it was previously checked.' The current code stores the current visit depth, not the maximum. If a node is first visited at depth 10 and later at depth 5, validated[node] becomes 5 — the shorter path. The short-circuit condition depth <= prevDepth then allows re-traversal at depth 6 even though we already checked at depth 10. In practice this isn't a security issue (the alias depth bypass test covers the real attack vector), but the comment and variable name ('validated' implying completeness) are slightly misleading. Consider renaming to visitedAtDepth and updating the comment to say 'the last depth at which this node was validated' rather than 'maximum'.
[MINOR] The go 1.26.2 in go.mod is a pre-release/future version (Go 1.26 has not been released). This may cause go mod tidy to behave unexpectedly on stable Go toolchains and suggests the module was initialized with an unstable toolchain. This is pre-existing and not introduced by this PR, but worth flagging.
[NIT] The HTML comment <!-- Deviation from step 1+3 for go-yaml migration: see #91 for rationale. --> is reasonable, but as a comment inside Markdown it won't render visibly in most Markdown renderers — it'll be invisible. If this rationale needs to be discoverable for future reviewers, a visible footnote or a link in the table row might be more effective. That said, hiding it is arguably intentional to keep the table clean.
[NIT] TestYAMLEmptyFileRejection has a redundant if err != nil guard before checking err.Error() — since the previous block already verified err != nil, the outer guard is unnecessary. This is a style nit that doesn't affect correctness.
[MINOR] The MappingValueNode case visits both Key and Value at depth+1 relative to the MappingValueNode's own depth. Since MappingValueNode is itself visited at depth+1 from its parent MappingNode, keys end up at depth+2 from the mapping. This asymmetry between key depth and value depth means scalar keys consume an extra depth level relative to what might be intuitive, but it's consistent and the tests validate the behavior works. Worth a brief comment explaining that keys consume a depth level intentionally.
[NIT] The comment on unmarshalYAMLWithDepthLimit says 'Multi-document YAML files are rejected to prevent confusing behavior where additional documents are silently ignored.' but the function header also mentions 'strict field checking'. The doc comment could be tightened to mention all three concerns (depth, multi-doc, strict fields) in the opening summary rather than leaving strict-field checking implicit.
[MINOR] The validated map stores the depth at which a node was last validated, but depth <= prevDepth as the short-circuit condition means a node visited at depth 3 that was previously validated at depth 5 would be skipped — which is correct (deeper previous validation is more conservative). However the comment says 'validated it at the same or deeper effective depth' which is slightly ambiguous. The logic is correct but 'same or shallower current depth compared to the previous validation depth' would be clearer.
[NIT] TestDoRequest_429RetryAfterHeader and TestDoRequest_RetryAfterDoesNotMutateBackoff are gated behind testing.Short() but TestDoRequest_429RetryAfterHTTPDate (which waits ~2 seconds) is not. This test will slow down non-short test runs. Consider adding testing.Short() skip to TestDoRequest_429RetryAfterHTTPDate as well for consistency.
[NIT] The escapePath function removes dot-segments silently. The doc comment acknowledges this and explains it's intentional, which is good. But the test case {"../etc/passwd", "etc/passwd"} documents that a path traversal attempt is silently resolved to etc/passwd rather than returning an error. Depending on threat model, callers may want to know the path was modified. Since this is intentional and documented, this is a NIT-level observation for a future design consideration.
[NIT] In GetCommitStatuses, the check run pagination loop uses checkPage as the variable name while the PR files loop uses page. Naming is internally consistent within each function, but the inconsistency between the two sibling pagination loops is a minor style nit.
[MINOR] The GetPullRequestFiles function is documented to return nil (not empty slice) when there are no changed files, which is correct for nil-safe ranging. However, GetCommitStatuses similarly returns a nil result slice when there are no statuses (via var result []vcs.CommitStatus plus no appends), but this is not documented in the function comment. Minor documentation inconsistency.
[MINOR] The doc comment on defaultCheckRedirect says it "strips the Authorization header on cross-host redirects or protocol downgrades (HTTPS→HTTP) to prevent credential leakage, while still following the redirect." However, a protocol downgrade from HTTPS to HTTP is a genuine security issue — stripping the header and still following is debatable. Consider returning an error on HTTPS→HTTP downgrade rather than silently following. This is a design choice that has security implications, not a bug per se, but worth flagging.
[MINOR] The response body limit check uses >= instead of >. io.LimitReader reads at most maxResponseBytes bytes, so len(body) == maxResponseBytes could mean the response is exactly 10 MiB (not truncated) or it was truncated at the limit. Using > is impossible here since LimitReader caps at the limit. The check should be == maxResponseBytes to be semantically correct, or the limit should be maxResponseBytes+1 to distinguish exactly-limit from truncated. The current code will error on exactly 10 MiB legitimate responses.