Sonnet Review Bot sonnet-review-bot
  • Joined on 2026-04-24
sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 23:25:52 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Removing the blank line between CommitID and Comments fields reduces readability in what is an exported, public-API struct. The blank line was providing visual grouping between the scalar fields (Body, Event, CommitID) and the slice field (Comments). This is purely cosmetic but the change is a minor readability regression with no functional benefit.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 23:25:52 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] TestAdapter_PostReview_CommitID and TestAdapter_PostReview_CommitID_Threading (added later in the file) both test largely the same behaviour — that req.CommitID is forwarded to the underlying client and echoed back in the returned review. Having two tests for the same assertion is minor duplication; one could be removed or merged.

sonnet-review-bot commented on pull request rodin/review-bot#119 2026-05-13 23:03:57 +00:00
fix(github): consolidate review.go and identity.go into reviews.go (#116)

[NIT] The truncated boolean is only ever set to true inside the loop when page == maxPages (after the short-page guard). The loop condition page <= maxPages combined with the page == maxPages check at the end means the flag is always set on the final iteration when the page was full. This is correct but slightly subtle — a brief inline comment explaining why the outer if page == maxPages fires only when the last page was full (and the earlier break didn't fire) would help future readers. Low impact given the existing comment is close.

sonnet-review-bot commented on pull request rodin/review-bot#119 2026-05-13 23:03:57 +00:00
fix(github): consolidate review.go and identity.go into reviews.go (#116)

[NIT] Import ordering: fmt is listed before encoding/json, breaking the standard goimports ordering (stdlib imports should be alphabetical within their group). gofmt/goimports would reorder these. Minor since CI passes, but worth fixing for consistency with the style pattern.

sonnet-review-bot commented on pull request rodin/review-bot#119 2026-05-13 22:36:59 +00:00
fix(github): consolidate review.go and identity.go into reviews.go (#116)

[MINOR] The truncation warning is emitted after appending the last page's reviews, but only when page == maxReviewPages. If the final page happens to be exactly full (len(responses) == reviewsPerPage) and is also the last real page of data, the loop will make one more request that returns 0 results and break cleanly — the warning fires incorrectly in that case. Consider moving the warning outside the loop, after the loop exits, conditioned on page > maxReviewPages or by tracking whether the loop was cut short by the page cap rather than by an empty response.

sonnet-review-bot commented on pull request rodin/review-bot#119 2026-05-13 22:36:59 +00:00
fix(github): consolidate review.go and identity.go into reviews.go (#116)

[NIT] The pagination loop will always make one extra request (the one that returns 0 results) when the total review count is an exact multiple of reviewsPerPage. This is the standard approach and not a bug, but worth noting that GitHub's Link header could be used to avoid the extra round-trip if this ever becomes performance-sensitive.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:30:14 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Minor inconsistent alignment: CommitID string has extra whitespace padding (string ) before the json tag compared to the other fields. This is a gofmt style deviation that should be cleaned up.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:24:40 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] The comment // CommitID from vcs.ReviewComment is intentionally not forwarded is now slightly misleading in context: the review-level CommitID (req.CommitID) IS forwarded, only the per-comment CommitID is not. The comment could be tightened to // Per-comment CommitID is not forwarded: Gitea has no per-comment commit_id field. to avoid confusion.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:24:40 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Minor struct field alignment inconsistency: CommitID string is followed by two spaces before json:... while neighboring fields use one tab. This is a gofmt artifact from the diff display, but worth confirming gofmt was run on the final file. The full file content shows correctly formatted alignment, so this is likely just diff rendering.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:15:02 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Minor formatting inconsistency: Event and CommitID fields have inconsistent alignment in the struct literal — Event uses tab-separated alignment with double tabs while CommitID aligns differently. This is a gofmt concern and shouldn't be an issue if gofmt is enforced, but the raw diff shows misaligned tabs between Event ReviewEvent and CommitID string. Low impact since gofmt normalises this.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:15:02 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] The conflict detection logic between req.CommitID and per-comment CommitIDs works correctly but uses a subtle implicit assumption: the initial payload.CommitID = req.CommitID means the loop's payload.CommitID == "" check is only true when req.CommitID was also empty. This is correct but slightly hard to follow at a glance — a brief inline comment like // req.CommitID already applied above on the empty-check branch would improve readability.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:04:28 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Missing blank line between TestAdapter_PostReview_CommitID and TestAdapter_PostReview_WithComments_PositionTranslation. Minor style inconsistency — all other test functions in the file have a blank line separator.

sonnet-review-bot commented on pull request rodin/review-bot#118 2026-05-13 21:04:28 +00:00
feat(vcs): add CommitID to ReviewRequest (#115)

[NIT] Minor formatting inconsistency: Event and CommitID have trailing whitespace-padded alignment (Event ReviewEvent json:"event" and `CommitID string `json:"commit_id,omitempty") that differs from the fields above (Body string). gofmt should normalize this, but it's worth verifying the file passes gofmt -l.