rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

feat: resolve old inline comments when superseding review #42

Merged
rodin merged 1 commits from feat/27-resolve-inline-comments into main 2026-05-02 19:18:42 +00:00
Conversation 9 Commits 1 Files Changed 3
+146
rodin commented 2026-05-02 19:07:19 +00:00
Owner
Copy Link
Copy Source

Closes #27

After superseding an old review, resolves all its inline comments via POST /pulls/comments/{id}/resolve. This clears unresolved conversation markers from the PR timeline and diff view.

Why

With the always-post-fresh model, old inline comments linger as unresolved conversations in the PR diff view. Since the new review has fresh inline comments for any remaining findings, the old ones are noise. Resolving them keeps the PR clean.

New API methods

  • ListReviewComments — fetches inline comments for a specific review
  • ResolveComment — marks a pull comment as resolved

Behavior

  • Only runs when an old review was successfully superseded
  • Resolves ALL old inline comments (new review has fresh ones for remaining issues)
  • Non-fatal: failures logged at debug level, review still posts
Closes #27 After superseding an old review, resolves all its inline comments via `POST /pulls/comments/{id}/resolve`. This clears unresolved conversation markers from the PR timeline and diff view. ## Why With the always-post-fresh model, old inline comments linger as unresolved conversations in the PR diff view. Since the new review has fresh inline comments for any remaining findings, the old ones are noise. Resolving them keeps the PR clean. ## New API methods - `ListReviewComments` — fetches inline comments for a specific review - `ResolveComment` — marks a pull comment as resolved ## Behavior - Only runs when an old review was successfully superseded - Resolves ALL old inline comments (new review has fresh ones for remaining issues) - Non-fatal: failures logged at debug level, review still posts
gpt-review-bot approved these changes 2026-05-02 19:07:56 +00:00
Dismissed
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Original review

Superseded — see current review for up-to-date findings.

Previous findings (commit 3d0ca57a)

Gpt Review

Summary

This PR cleanly implements the automatic resolution of outdated inline review comments when superseding a review, and provides robust testing and defensive error handling. Implementation follows all established patterns and repository conventions.

Recommendation

APPROVE — The PR expands the Gitea client with ListReviewComments and ResolveComment methods to resolve all inline comments on a superseded review, only after a successful supersede. Error handling is robust and non-fatal for failures to resolve comments, which are logged at the debug level. Unit tests for the new API methods provide thorough coverage. The diff is idiomatic Go, conforms to repository conventions (no panics, robust error handling, table-driven tests where applicable), and style is consistent. The implementation does not introduce any breaking changes or regressions; CI is green. Recommend merge.

Review by gpt

Evaluated against 3d0ca57a

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/42#pullrequestreview-666) for up-to-date findings. <details><summary>Previous findings (commit 3d0ca57a)</summary> # Gpt Review ## Summary This PR cleanly implements the automatic resolution of outdated inline review comments when superseding a review, and provides robust testing and defensive error handling. Implementation follows all established patterns and repository conventions. ## Recommendation **APPROVE** — The PR expands the Gitea client with `ListReviewComments` and `ResolveComment` methods to resolve all inline comments on a superseded review, only after a successful supersede. Error handling is robust and non-fatal for failures to resolve comments, which are logged at the debug level. Unit tests for the new API methods provide thorough coverage. The diff is idiomatic Go, conforms to repository conventions (no panics, robust error handling, table-driven tests where applicable), and style is consistent. The implementation does not introduce any breaking changes or regressions; CI is green. Recommend merge. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against 3d0ca57a* </details> <!-- review-bot:gpt -->
security-review-bot approved these changes 2026-05-02 19:08:17 +00:00
Dismissed
security-review-bot left a comment
Collaborator
Copy Link
Copy Source

Security Review

Summary

Changes add resolving of superseded review inline comments and corresponding Gitea client methods. CI passed and the implementation uses proper URL escaping, scoped IDs, and does not expose secrets; no security-relevant issues found.

Recommendation

APPROVE — Approve as submitted. The new logic resolves only comments belonging to the superseded review, uses url.PathEscape for all path parameters, and keeps tokens in headers (not logs or URLs). Error messages include only server responses and IDs, avoiding secret exposure. Consider, as future hardening, adding pagination if Gitea returns many review comments and handling rate limits, but this is not security-critical.

Review by security

Evaluated against 17027c1f

# Security Review ## Summary Changes add resolving of superseded review inline comments and corresponding Gitea client methods. CI passed and the implementation uses proper URL escaping, scoped IDs, and does not expose secrets; no security-relevant issues found. ## Recommendation **APPROVE** — Approve as submitted. The new logic resolves only comments belonging to the superseded review, uses url.PathEscape for all path parameters, and keeps tokens in headers (not logs or URLs). Error messages include only server responses and IDs, avoiding secret exposure. Consider, as future hardening, adding pagination if Gitea returns many review comments and handling rate limits, but this is not security-critical. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against 17027c1f*
sonnet-review-bot approved these changes 2026-05-02 19:08:38 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Original review

Superseded — see current review for up-to-date findings.

Previous findings (commit 3d0ca57a)

Sonnet Review

Summary

The changes cleanly add resolution of inline comments from superseded reviews and introduce well-structured Gitea client methods with accompanying tests. Error handling and logging follow repo conventions, and CI has passed.

Findings

# Severity File Line Finding
1 [MINOR] gitea/client_test.go 602 TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests.
2 [NIT] cmd/review-bot/main.go 357 Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility.

Recommendation

APPROVE — Overall the implementation is sound and idiomatic: the bot only resolves comments after successfully superseding the old review, the new Gitea client methods are well-scoped with pagination and error handling, and tests cover the new methods. CI is green. Consider tightening the ListReviewComments test by asserting the request path and parameters, and optionally surfacing a summary log if resolving comments fails for better observability. Otherwise, this is good to merge.

Review by sonnet

Evaluated against 3d0ca57a

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/42#pullrequestreview-667) for up-to-date findings. <details><summary>Previous findings (commit 3d0ca57a)</summary> # Sonnet Review ## Summary The changes cleanly add resolution of inline comments from superseded reviews and introduce well-structured Gitea client methods with accompanying tests. Error handling and logging follow repo conventions, and CI has passed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client_test.go` | 602 | TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests. | | 2 | [NIT] | `cmd/review-bot/main.go` | 357 | Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility. | ## Recommendation **APPROVE** — Overall the implementation is sound and idiomatic: the bot only resolves comments after successfully superseding the old review, the new Gitea client methods are well-scoped with pagination and error handling, and tests cover the new methods. CI is green. Consider tightening the ListReviewComments test by asserting the request path and parameters, and optionally surfacing a summary log if resolving comments fails for better observability. Otherwise, this is good to merge. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against 3d0ca57a* </details> <!-- review-bot:sonnet -->
cmd/review-bot/main.go Outdated
@@ -362,6 +362,27 @@ func main() {
} else {
sonnet-review-bot commented 2026-05-02 19:08:38 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] Resolution of old inline comments is gated on locating the old timeline comment ID (existingCommentID > 0), but proceeds regardless of whether EditComment actually succeeded. This slightly diverges from the stated behavior "Only runs when an old review was successfully superseded." Consider gating resolution explicitly on a successful supersede or documenting the intended behavior.

**[MINOR]** Resolution of old inline comments is gated on locating the old timeline comment ID (existingCommentID > 0), but proceeds regardless of whether EditComment actually succeeded. This slightly diverges from the stated behavior "Only runs when an old review was successfully superseded." Consider gating resolution explicitly on a successful supersede or documenting the intended behavior.
gitea/client.go
@@ -460,3 +461,49 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
}
sonnet-review-bot commented 2026-05-02 19:08:38 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] ListReviewComments retrieves a single page of comments. If the API paginates review comments, this may miss results beyond the first page. Consider adding pagination similar to ListReviews to avoid missing comments on large reviews.

**[MINOR]** ListReviewComments retrieves a single page of comments. If the API paginates review comments, this may miss results beyond the first page. Consider adding pagination similar to ListReviews to avoid missing comments on large reviews.
sonnet-review-bot commented 2026-05-02 19:08:38 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] New exported client methods (ListReviewComments, ResolveComment) lack tests. Repository conventions suggest testing exported functions; adding unit tests against a local httptest.Server would improve confidence and guard against API regressions.

**[MINOR]** New exported client methods (ListReviewComments, ResolveComment) lack tests. Repository conventions suggest testing exported functions; adding unit tests against a local httptest.Server would improve confidence and guard against API regressions.
gpt-review-bot approved these changes 2026-05-02 19:11:15 +00:00
Dismissed
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Gpt Review

Summary

This PR cleanly implements the automatic resolution of outdated inline review comments when superseding a review, and provides robust testing and defensive error handling. Implementation follows all established patterns and repository conventions.

Recommendation

APPROVE — The PR expands the Gitea client with ListReviewComments and ResolveComment methods to resolve all inline comments on a superseded review, only after a successful supersede. Error handling is robust and non-fatal for failures to resolve comments, which are logged at the debug level. Unit tests for the new API methods provide thorough coverage. The diff is idiomatic Go, conforms to repository conventions (no panics, robust error handling, table-driven tests where applicable), and style is consistent. The implementation does not introduce any breaking changes or regressions; CI is green. Recommend merge.

Review by gpt

Evaluated against 3d0ca57a

# Gpt Review ## Summary This PR cleanly implements the automatic resolution of outdated inline review comments when superseding a review, and provides robust testing and defensive error handling. Implementation follows all established patterns and repository conventions. ## Recommendation **APPROVE** — The PR expands the Gitea client with `ListReviewComments` and `ResolveComment` methods to resolve all inline comments on a superseded review, only after a successful supersede. Error handling is robust and non-fatal for failures to resolve comments, which are logged at the debug level. Unit tests for the new API methods provide thorough coverage. The diff is idiomatic Go, conforms to repository conventions (no panics, robust error handling, table-driven tests where applicable), and style is consistent. The implementation does not introduce any breaking changes or regressions; CI is green. Recommend merge. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against 3d0ca57a*
security-review-bot approved these changes 2026-05-02 19:11:33 +00:00
Dismissed
security-review-bot left a comment
Collaborator
Copy Link
Copy Source

Security Review

Summary

The changes add resolving of superseded review inline comments with appropriate API calls and error handling. URLs are safely constructed, tokens are not logged, and operations occur only after a successful supersede, minimizing risk.

Recommendation

APPROVE — CI has passed and the new methods and callsites appear secure and robust. The code correctly escapes URL path segments, avoids logging sensitive tokens, and limits comment resolution to the prior review’s comments after a successful supersede. Approve as implemented.

Review by security

Evaluated against 3d0ca57a

# Security Review ## Summary The changes add resolving of superseded review inline comments with appropriate API calls and error handling. URLs are safely constructed, tokens are not logged, and operations occur only after a successful supersede, minimizing risk. ## Recommendation **APPROVE** — CI has passed and the new methods and callsites appear secure and robust. The code correctly escapes URL path segments, avoids logging sensitive tokens, and limits comment resolution to the prior review’s comments after a successful supersede. Approve as implemented. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against 3d0ca57a*
sonnet-review-bot approved these changes 2026-05-02 19:12:01 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

The changes cleanly add resolution of inline comments from superseded reviews and introduce well-structured Gitea client methods with accompanying tests. Error handling and logging follow repo conventions, and CI has passed.

Findings

# Severity File Line Finding
1 [MINOR] gitea/client_test.go 602 TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests.
2 [NIT] cmd/review-bot/main.go 357 Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility.

Recommendation

APPROVE — Overall the implementation is sound and idiomatic: the bot only resolves comments after successfully superseding the old review, the new Gitea client methods are well-scoped with pagination and error handling, and tests cover the new methods. CI is green. Consider tightening the ListReviewComments test by asserting the request path and parameters, and optionally surfacing a summary log if resolving comments fails for better observability. Otherwise, this is good to merge.

Review by sonnet

Evaluated against 3d0ca57a

# Sonnet Review ## Summary The changes cleanly add resolution of inline comments from superseded reviews and introduce well-structured Gitea client methods with accompanying tests. Error handling and logging follow repo conventions, and CI has passed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client_test.go` | 602 | TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests. | | 2 | [NIT] | `cmd/review-bot/main.go` | 357 | Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility. | ## Recommendation **APPROVE** — Overall the implementation is sound and idiomatic: the bot only resolves comments after successfully superseding the old review, the new Gitea client methods are well-scoped with pagination and error handling, and tests cover the new methods. CI is green. Consider tightening the ListReviewComments test by asserting the request path and parameters, and optionally surfacing a summary log if resolving comments fails for better observability. Otherwise, this is good to merge. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against 3d0ca57a*
cmd/review-bot/main.go Outdated
@@ -357,10 +357,35 @@ func main() {
if existingReview != nil && existingCommentID > 0 {
sonnet-review-bot commented 2026-05-02 19:12:01 +00:00
First-time contributor
Copy Link
Copy Source

[NIT] Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility.

**[NIT]** Failures to resolve old inline comments are logged at debug level within the resolution loop. Given this action is meant to clean up PR noise, consider aggregating and logging a summary at warn/info level when any failures occur to aid operational visibility.
gitea/client_test.go
@@ -602,3 +602,60 @@ func TestIsNotFound(t *testing.T) {
})
sonnet-review-bot commented 2026-05-02 19:12:01 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests.

**[MINOR]** TestListReviewComments does not assert the request path or query parameters, so regressions in URL construction for the new endpoint would go unnoticed. Consider asserting the expected path and paging params similar to other tests.
rodin added 1 commit 2026-05-02 19:15:54 +00:00
feat: resolve old inline comments when superseding review
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 44s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
Details
bc5a4a1dcd 
Closes #27

After superseding an old review, resolves all its inline comments via
POST /pulls/comments/{id}/resolve. This clears unresolved conversation
markers from the PR timeline and diff view.

New API methods:
- ListReviewComments: paginated GET /repos/.../pulls/{n}/reviews/{id}/comments
- ResolveComment: POST /repos/.../pulls/comments/{id}/resolve

Behavior:
- Only resolves after successful supersede (gated on supersedeOK)
- Aggregates failures and logs at warn level
- Truncates error bodies to 256 bytes (security)
- Non-fatal: review still posts even if resolution fails
rodin force-pushed feat/27-resolve-inline-comments from 3d0ca57a5f to bc5a4a1dcd 2026-05-02 19:15:54 +00:00 Compare
gpt-review-bot approved these changes 2026-05-02 19:16:30 +00:00
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Gpt Review

Summary

Code is correct, well-tested, and idiomatic. The new functionality for resolving old inline comments when superseding a review is implemented cleanly and fits the project conventions.

Recommendation

APPROVE — The PR introduces ListReviewComments and ResolveComment methods to the Gitea client, uses them in the main workflow to resolve old inline comments after superseding a review, and covers all new code with thorough unit tests. No issues found; CI passes. Approve for merge.

Review by gpt

Evaluated against bc5a4a1d

# Gpt Review ## Summary Code is correct, well-tested, and idiomatic. The new functionality for resolving old inline comments when superseding a review is implemented cleanly and fits the project conventions. ## Recommendation **APPROVE** — The PR introduces `ListReviewComments` and `ResolveComment` methods to the Gitea client, uses them in the main workflow to resolve old inline comments after superseding a review, and covers all new code with thorough unit tests. No issues found; CI passes. Approve for merge. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against bc5a4a1d*
sonnet-review-bot approved these changes 2026-05-02 19:16:51 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

Change cleanly adds resolving of old inline comments after superseding a review and introduces corresponding Gitea client methods with tests. Error handling and logging are appropriate, and CI has passed.

Recommendation

APPROVE — The implementation follows idiomatic patterns: non-fatal behavior with clear logging, proper pagination, and context usage. New client methods are covered by tests, and adding the comment ID field with omitempty is correct for dual use (post vs list). Proceed with merge.

Review by sonnet

Evaluated against bc5a4a1d

# Sonnet Review ## Summary Change cleanly adds resolving of old inline comments after superseding a review and introduces corresponding Gitea client methods with tests. Error handling and logging are appropriate, and CI has passed. ## Recommendation **APPROVE** — The implementation follows idiomatic patterns: non-fatal behavior with clear logging, proper pagination, and context usage. New client methods are covered by tests, and adding the comment ID field with omitempty is correct for dual use (post vs list). Proceed with merge. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against bc5a4a1d*
security-review-bot requested review from security-review-bot 2026-05-02 19:16:54 +00:00
security-review-bot approved these changes 2026-05-02 19:16:54 +00:00
security-review-bot left a comment
Collaborator
Copy Link
Copy Source

Security Review

Summary

Security-focused review finds no exploitable vulnerabilities in the added functionality. The new APIs and logic to resolve old inline comments are implemented with proper authorization headers, error handling, and bounded error body logging.

Recommendation

APPROVE — The changes introduce non-destructive housekeeping that resolves old review inline comments only after successfully superseding the prior review. API calls are constructed with appropriate escaping and use the existing authenticated context. Error bodies are limited when logged, and failures are handled gracefully. CI passed, and no security issues were identified. Proceed with merge.

Review by security

Evaluated against bc5a4a1d

# Security Review ## Summary Security-focused review finds no exploitable vulnerabilities in the added functionality. The new APIs and logic to resolve old inline comments are implemented with proper authorization headers, error handling, and bounded error body logging. ## Recommendation **APPROVE** — The changes introduce non-destructive housekeeping that resolves old review inline comments only after successfully superseding the prior review. API calls are constructed with appropriate escaping and use the existing authenticated context. Error bodies are limited when logged, and failures are handled gracefully. CI passed, and no security issues were identified. Proceed with merge. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against bc5a4a1d*
rodin merged commit 23443ef378 into main 2026-05-02 19:18:42 +00:00
rodin deleted branch feat/27-resolve-inline-comments 2026-05-02 19:18:42 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
gpt-review-bot
sonnet-review-bot
security-review-bot
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#42
Delete Branch "feat/27-resolve-inline-comments"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?