Add commitID parameter to gitea.Client.PostReview so the review is anchored to the specific commit SHA that was evaluated, rather than relying on Gitea to default to HEAD.
Changes
gitea/client.go — Added commitID string parameter to PostReview. Included as commit_id,omitempty in the JSON payload so empty values are not sent.
cmd/review-bot/main.go — Pass evaluatedSHA (already computed from pr.Head.Sha) to PostReview.
Tests — Updated all callers, added TestPostReview_EmptyCommitID_OmittedFromPayload to verify omitempty behavior.
Why
Without explicit commit anchoring, if the PR head advances between analysis and posting, the review targets a different commit than what was evaluated. The stale check prevents most races, but explicit anchoring expresses intent clearly in the API call.
## Summary
Add `commitID` parameter to `gitea.Client.PostReview` so the review is anchored to the specific commit SHA that was evaluated, rather than relying on Gitea to default to HEAD.
## Changes
1. **`gitea/client.go`** — Added `commitID string` parameter to `PostReview`. Included as `commit_id,omitempty` in the JSON payload so empty values are not sent.
2. **`cmd/review-bot/main.go`** — Pass `evaluatedSHA` (already computed from `pr.Head.Sha`) to `PostReview`.
3. **Tests** — Updated all callers, added `TestPostReview_EmptyCommitID_OmittedFromPayload` to verify omitempty behavior.
## Why
Without explicit commit anchoring, if the PR head advances between analysis and posting, the review targets a different commit than what was evaluated. The stale check prevents most races, but explicit anchoring expresses intent clearly in the API call.
Closes #107
rodin
added the wip label 2026-05-13 17:29:20 +00:00
Add commitID parameter to gitea.Client.PostReview so the review is
anchored to the specific commit that was evaluated. The caller
(cmd/review-bot) already computes evaluatedSHA from pr.Head.Sha;
this wires it through to the Gitea API payload.
When commitID is empty, omitempty drops it from the JSON and Gitea
defaults to the current PR head (backward-compatible).
Closes#107
rodin
removed the wip label 2026-05-13 17:29:25 +00:00
A clean, well-scoped change that adds commitID as an explicit parameter to PostReview, passes evaluatedSHA from the call site, and verifies omitempty behaviour with a targeted test. CI passes, all callers updated consistently.
Findings
#
Severity
File
Line
Finding
1
[NIT]
gitea/client.go
283
The doc comment on PostReview still says event should be "APPROVED" or "REQUEST_CHANGES", but the integration test and production code also pass "COMMENT". The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g. event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT").
Recommendation
APPROVE — The change is correct and minimal. The commitID parameter is properly threaded through, omitempty is tested, and CI is green. Approve as-is; the single nit about the doc comment is cosmetic and doesn't affect correctness.
~~Original review~~
**Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/112#pullrequestreview-3341) for up-to-date findings.
<details><summary>Previous findings (commit e59c2bc8)</summary>
# Sonnet Review
## Summary
A clean, well-scoped change that adds `commitID` as an explicit parameter to `PostReview`, passes `evaluatedSHA` from the call site, and verifies `omitempty` behaviour with a targeted test. CI passes, all callers updated consistently.
## Findings
| # | Severity | File | Line | Finding |
|---|----------|------|------|--------|
| 1 | [NIT] | `gitea/client.go` | 283 | The doc comment on `PostReview` still says `event should be "APPROVED" or "REQUEST_CHANGES"`, but the integration test and production code also pass `"COMMENT"`. The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g. `event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT"`). |
## Recommendation
**APPROVE** — The change is correct and minimal. The `commitID` parameter is properly threaded through, `omitempty` is tested, and CI is green. Approve as-is; the single nit about the doc comment is cosmetic and doesn't affect correctness.
---
*Review by sonnet*
<!-- review-bot:sonnet -->
---
*Evaluated against e59c2bc8*
</details>
<!-- review-bot:sonnet -->
[NIT] The doc comment on PostReview still says event should be "APPROVED" or "REQUEST_CHANGES", but the integration test and production code also pass "COMMENT". The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g. event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT").
**[NIT]** The doc comment on `PostReview` still says `event should be "APPROVED" or "REQUEST_CHANGES"`, but the integration test and production code also pass `"COMMENT"`. The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g. `event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT"`).
security-review-bot
requested review from security-review-bot 2026-05-13 17:30:22 +00:00
Security-focused review of the changes shows no new vulnerabilities introduced. Adding commit_id to PostReview payload anchors reviews to a specific SHA without increasing attack surface.
Recommendation
APPROVE — CI passed and the changes are limited to passing an explicit commit SHA to Gitea via a JSON-encoded field with omitempty semantics. This does not introduce injection, authorization, or other security risks. Proceed with merge.
~~Original review~~
**Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/112#pullrequestreview-3343) for up-to-date findings.
<details><summary>Previous findings (commit e59c2bc8)</summary>
# Security Review
## Summary
Security-focused review of the changes shows no new vulnerabilities introduced. Adding commit_id to PostReview payload anchors reviews to a specific SHA without increasing attack surface.
## Recommendation
**APPROVE** — CI passed and the changes are limited to passing an explicit commit SHA to Gitea via a JSON-encoded field with omitempty semantics. This does not introduce injection, authorization, or other security risks. Proceed with merge.
---
*Review by security*
<!-- review-bot:security -->
---
*Evaluated against e59c2bc8*
</details>
<!-- review-bot:security -->
gpt-review-bot
approved these changes 2026-05-13 17:30:37 +00:00
Changes cleanly add an explicit commitID to PostReview, update all call sites, and include a test to verify omitempty behavior. The implementation is straightforward, preserves error handling conventions, and CI passed.
Recommendation
APPROVE — The explicit commit anchoring via commit_id is a solid improvement for correctness under concurrent PR updates. Call sites and tests are correctly updated, and the omitempty test guards against sending empty values. No issues found in the changes; proceed with merge. As a general note for future work, ensure the event values used align with the Gitea API semantics across all call sites.
~~Original review~~
**Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/112#pullrequestreview-3344) for up-to-date findings.
<details><summary>Previous findings (commit e59c2bc8)</summary>
# Gpt Review
## Summary
Changes cleanly add an explicit commitID to PostReview, update all call sites, and include a test to verify omitempty behavior. The implementation is straightforward, preserves error handling conventions, and CI passed.
## Recommendation
**APPROVE** — The explicit commit anchoring via commit_id is a solid improvement for correctness under concurrent PR updates. Call sites and tests are correctly updated, and the omitempty test guards against sending empty values. No issues found in the changes; proceed with merge. As a general note for future work, ensure the event values used align with the Gitea API semantics across all call sites.
---
*Review by gpt*
<!-- review-bot:gpt -->
---
*Evaluated against e59c2bc8*
</details>
<!-- review-bot:gpt -->
rodin
added the wip label 2026-05-13 17:35:19 +00:00
None — diff looks clean. The commitID parameter is correctly threaded through all call sites, the omitempty tag prevents empty values from being serialized, and a dedicated test verifies that behavior. The integration test and main.go call sites are updated consistently.
Phase 2: Prior Review Verification
Finding
Reviewer
Status
Notes
[NIT] Doc comment lists only APPROVED/REQUEST_CHANGES but COMMENT is also used
All three bot reviewers approved. The only outstanding item is a cosmetic NIT about a pre-existing doc comment inaccuracy — not introduced by this PR and not blocking. The change is correct, minimal, and well-tested.
## Self-Review: PR #112
Self-review against e59c2bc83162b38c48abfe8789434f68bbb08524
### Phase 1: Independent Findings
None — diff looks clean. The `commitID` parameter is correctly threaded through all call sites, the `omitempty` tag prevents empty values from being serialized, and a dedicated test verifies that behavior. The integration test and main.go call sites are updated consistently.
### Phase 2: Prior Review Verification
| Finding | Reviewer | Status | Notes |
|---------|----------|--------|-------|
| [NIT] Doc comment lists only APPROVED/REQUEST_CHANGES but COMMENT is also used | sonnet-review-bot | STILL PRESENT | Pre-existing inaccuracy; NIT-severity, reviewer approved regardless |
### Assessment: ✅ Clean
All three bot reviewers approved. The only outstanding item is a cosmetic NIT about a pre-existing doc comment inaccuracy — not introduced by this PR and not blocking. The change is correct, minimal, and well-tested.
rodin
removed the wip label 2026-05-13 17:36:42 +00:00
rodin
added the wip label 2026-05-13 17:40:23 +00:00
Source: sonnet-review-bot inline comment on gitea/client.go line ~283 Issue: Doc comment on PostReview says event should be "APPROVED" or "REQUEST_CHANGES" but code also uses "COMMENT". Fix: Update the doc comment to say event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT".
Implementation
Edit gitea/client.go — update the doc comment on PostReview to list all three valid event values.
Run go test ./... to confirm no regressions.
Commit and push to review-bot-issue-107 branch.
Reply to inline comment id 19793 acknowledging the fix.
## Fix Plan against e59c2bc83162b38c48abfe8789434f68bbb08524:
### Finding 1: NIT — PostReview doc comment inaccuracy (comment id 19793)
**Source:** sonnet-review-bot inline comment on `gitea/client.go` line ~283
**Issue:** Doc comment on `PostReview` says `event should be "APPROVED" or "REQUEST_CHANGES"` but code also uses `"COMMENT"`.
**Fix:** Update the doc comment to say `event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT"`.
### Implementation
1. Edit `gitea/client.go` — update the doc comment on `PostReview` to list all three valid event values.
2. Run `go test ./...` to confirm no regressions.
3. Commit and push to `review-bot-issue-107` branch.
4. Reply to inline comment id 19793 acknowledging the fix.
This PR adds a commitID string parameter to PostReview to anchor reviews to a specific commit SHA, using omitempty so empty values are not sent. The implementation is correct, all callers are updated, the new test for omitempty behavior is well-written, and CI is passing.
Findings
#
Severity
File
Line
Finding
1
[NIT]
gitea/client.go
284
The anonymous struct payload field alignment uses extra whitespace (CommitID string \json:...`vsBody string `json:...``) that is not gofmt-canonical. This is cosmetic but gofmt would normalize it. Likely a non-issue if gofmt is enforced in CI, but worth keeping consistent.
Recommendation
APPROVE — The change is small, correct, and well-tested. The commitID parameter is placed before comments in the signature, which is a reasonable ordering (metadata before optional extras). The omitempty tag correctly handles the empty-string case. CI passes. The single nit about struct field alignment spacing is cosmetic and not a blocker. Approve.
# Sonnet Review
## Summary
This PR adds a `commitID string` parameter to `PostReview` to anchor reviews to a specific commit SHA, using `omitempty` so empty values are not sent. The implementation is correct, all callers are updated, the new test for omitempty behavior is well-written, and CI is passing.
## Findings
| # | Severity | File | Line | Finding |
|---|----------|------|------|--------|
| 1 | [NIT] | `gitea/client.go` | 284 | The anonymous struct payload field alignment uses extra whitespace (`CommitID string \`json:...\`` vs `Body string \`json:...\``) that is not gofmt-canonical. This is cosmetic but gofmt would normalize it. Likely a non-issue if gofmt is enforced in CI, but worth keeping consistent. |
## Recommendation
**APPROVE** — The change is small, correct, and well-tested. The `commitID` parameter is placed before `comments` in the signature, which is a reasonable ordering (metadata before optional extras). The `omitempty` tag correctly handles the empty-string case. CI passes. The single nit about struct field alignment spacing is cosmetic and not a blocker. Approve.
---
*Review by sonnet*
<!-- review-bot:sonnet -->
---
*Evaluated against 028d4694*
security-review-bot
requested review from security-review-bot 2026-05-13 17:43:09 +00:00
Changes cleanly add an explicit commit_id to review submissions and pass the evaluated SHA from the PR, improving correctness without introducing new security risks. CI has passed and the new tests cover the omitempty behavior and updated call sites.
Recommendation
APPROVE — The update anchors reviews to a specific commit via commit_id and is implemented safely with proper JSON marshaling and omission when empty. Calls now pass the previously fetched pr.Head.Sha, and a stale-head check remains in place. No security vulnerabilities are introduced by these changes. As a defense-in-depth enhancement (optional), you could validate commitID format (e.g., hex SHA) before sending, but this is not required for security. Proceed with merge.
# Security Review
## Summary
Changes cleanly add an explicit commit_id to review submissions and pass the evaluated SHA from the PR, improving correctness without introducing new security risks. CI has passed and the new tests cover the omitempty behavior and updated call sites.
## Recommendation
**APPROVE** — The update anchors reviews to a specific commit via commit_id and is implemented safely with proper JSON marshaling and omission when empty. Calls now pass the previously fetched pr.Head.Sha, and a stale-head check remains in place. No security vulnerabilities are introduced by these changes. As a defense-in-depth enhancement (optional), you could validate commitID format (e.g., hex SHA) before sending, but this is not required for security. Proceed with merge.
---
*Review by security*
<!-- review-bot:security -->
---
*Evaluated against 028d4694*
gpt-review-bot
approved these changes 2026-05-13 17:43:19 +00:00
Solid, focused change adding an explicit commitID to PostReview and propagating it through callers. Tests were updated comprehensively, including a new check for omitempty behavior, and CI passed.
Recommendation
APPROVE — The explicit commit anchoring via commit_id is a good improvement and is implemented cleanly with proper JSON omitempty semantics. Call sites are correctly updated, including main and integration tests, and additional tests validate new behavior. Given CI success and no issues detected in the diff, this is ready to merge.
# Gpt Review
## Summary
Solid, focused change adding an explicit commitID to PostReview and propagating it through callers. Tests were updated comprehensively, including a new check for omitempty behavior, and CI passed.
## Recommendation
**APPROVE** — The explicit commit anchoring via commit_id is a good improvement and is implemented cleanly with proper JSON omitempty semantics. Call sites are correctly updated, including main and integration tests, and additional tests validate new behavior. Given CI success and no issues detected in the diff, this is ready to merge.
---
*Review by gpt*
<!-- review-bot:gpt -->
---
*Evaluated against 028d4694*
rodin
added the wip label 2026-05-13 17:50:02 +00:00
None — diff looks clean. The commitID string parameter is correctly threaded through all call sites (main.go passes evaluatedSHA, integration test and existing tests pass ""). The omitempty tag correctly prevents serialization of empty strings. The new TestPostReview_EmptyCommitID_OmittedFromPayload test validates this behavior via raw JSON inspection. The doc comment was updated in this PR to include "COMMENT" as a valid event value. No correctness, error-handling, or coverage gaps found.
Phase 2: Prior Review Verification
Finding
Reviewer
Status
Notes
NIT: doc comment missing "COMMENT" as valid event value
sonnet-review-bot (review 3325)
✅ Resolved
Fixed in 028d469: doc comment now lists all three valid event values (APPROVED, REQUEST_CHANGES, COMMENT)
NIT: struct field alignment spacing not gofmt-canonical
sonnet-review-bot (review 3341)
🚫 Invalid
The diff shows standard Go struct literal alignment with tab-aligned field tags — this is idiomatic and gofmt-normalized. The reviewer appears to have been misled by rendered whitespace in the diff view; the actual source uses tabs per gofmt convention
Assessment: ✅ Clean
All prior reviewer findings are resolved or invalid. The change is small, focused, and correct: it adds commitID as an explicit parameter to anchor reviews to a specific commit SHA, uses omitempty to avoid sending empty values, updates all callers consistently, and adds a targeted test for the omitempty behavior. Ready to merge.
## Self-Review: PR #112
Self-review against 028d46942af0c605d1057510455ac4de38af31d2
### Phase 1: Independent Findings
None — diff looks clean. The `commitID string` parameter is correctly threaded through all call sites (`main.go` passes `evaluatedSHA`, integration test and existing tests pass `""`). The `omitempty` tag correctly prevents serialization of empty strings. The new `TestPostReview_EmptyCommitID_OmittedFromPayload` test validates this behavior via raw JSON inspection. The doc comment was updated in this PR to include `"COMMENT"` as a valid event value. No correctness, error-handling, or coverage gaps found.
### Phase 2: Prior Review Verification
| Finding | Reviewer | Status | Notes |
|---------|----------|--------|-------|
| NIT: doc comment missing `"COMMENT"` as valid event value | sonnet-review-bot (review 3325) | ✅ Resolved | Fixed in 028d469: doc comment now lists all three valid event values (APPROVED, REQUEST_CHANGES, COMMENT) |
| NIT: struct field alignment spacing not gofmt-canonical | sonnet-review-bot (review 3341) | 🚫 Invalid | The diff shows standard Go struct literal alignment with tab-aligned field tags — this is idiomatic and gofmt-normalized. The reviewer appears to have been misled by rendered whitespace in the diff view; the actual source uses tabs per gofmt convention |
### Assessment: ✅ Clean
All prior reviewer findings are resolved or invalid. The change is small, focused, and correct: it adds `commitID` as an explicit parameter to anchor reviews to a specific commit SHA, uses `omitempty` to avoid sending empty values, updates all callers consistently, and adds a targeted test for the omitempty behavior. Ready to merge.
rodin
removed the wip label 2026-05-13 17:51:26 +00:00
aweiker
was assigned by rodin2026-05-13 17:55:51 +00:00
rodin
added the ready label 2026-05-13 17:55:51 +00:00
aweiker
approved these changes 2026-05-13 18:07:36 +00:00
aweiker
merged commit b26514714f into main2026-05-13 18:07:39 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
sonnet-review-bot
security-review-bot
gpt-review-bot
aweiker
Summary
Add
commitIDparameter togitea.Client.PostReviewso the review is anchored to the specific commit SHA that was evaluated, rather than relying on Gitea to default to HEAD.Changes
gitea/client.go— AddedcommitID stringparameter toPostReview. Included ascommit_id,omitemptyin the JSON payload so empty values are not sent.cmd/review-bot/main.go— PassevaluatedSHA(already computed frompr.Head.Sha) toPostReview.TestPostReview_EmptyCommitID_OmittedFromPayloadto verify omitempty behavior.Why
Without explicit commit anchoring, if the PR head advances between analysis and posting, the review targets a different commit than what was evaluated. The stale check prevents most races, but explicit anchoring expresses intent clearly in the API call.
Closes #107
Original reviewSuperseded — see current review for up-to-date findings.
Previous findings (commit
e59c2bc8)Sonnet Review
Summary
A clean, well-scoped change that adds
commitIDas an explicit parameter toPostReview, passesevaluatedSHAfrom the call site, and verifiesomitemptybehaviour with a targeted test. CI passes, all callers updated consistently.Findings
gitea/client.goPostReviewstill saysevent should be "APPROVED" or "REQUEST_CHANGES", but the integration test and production code also pass"COMMENT". The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g.event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT").Recommendation
APPROVE — The change is correct and minimal. The
commitIDparameter is properly threaded through,omitemptyis tested, and CI is green. Approve as-is; the single nit about the doc comment is cosmetic and doesn't affect correctness.Review by sonnet
Evaluated against
e59c2bc8@@ -277,3 +280,4 @@CommitID: commitID,Comments: comments,}[NIT] The doc comment on
PostReviewstill saysevent should be "APPROVED" or "REQUEST_CHANGES", but the integration test and production code also pass"COMMENT". The comment is a pre-existing inaccuracy but the PR touches this function's doc block and is a good opportunity to fix it (e.g.event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT").Original reviewSuperseded — see current review for up-to-date findings.
Previous findings (commit
e59c2bc8)Security Review
Summary
Security-focused review of the changes shows no new vulnerabilities introduced. Adding commit_id to PostReview payload anchors reviews to a specific SHA without increasing attack surface.
Recommendation
APPROVE — CI passed and the changes are limited to passing an explicit commit SHA to Gitea via a JSON-encoded field with omitempty semantics. This does not introduce injection, authorization, or other security risks. Proceed with merge.
Review by security
Evaluated against
e59c2bc8Original reviewSuperseded — see current review for up-to-date findings.
Previous findings (commit
e59c2bc8)Gpt Review
Summary
Changes cleanly add an explicit commitID to PostReview, update all call sites, and include a test to verify omitempty behavior. The implementation is straightforward, preserves error handling conventions, and CI passed.
Recommendation
APPROVE — The explicit commit anchoring via commit_id is a solid improvement for correctness under concurrent PR updates. Call sites and tests are correctly updated, and the omitempty test guards against sending empty values. No issues found in the changes; proceed with merge. As a general note for future work, ensure the event values used align with the Gitea API semantics across all call sites.
Review by gpt
Evaluated against
e59c2bc8Self-Review: PR #112
Self-review against
e59c2bc831Phase 1: Independent Findings
None — diff looks clean. The
commitIDparameter is correctly threaded through all call sites, theomitemptytag prevents empty values from being serialized, and a dedicated test verifies that behavior. The integration test and main.go call sites are updated consistently.Phase 2: Prior Review Verification
Assessment: ✅ Clean
All three bot reviewers approved. The only outstanding item is a cosmetic NIT about a pre-existing doc comment inaccuracy — not introduced by this PR and not blocking. The change is correct, minimal, and well-tested.
Fix Plan against
e59c2bc831:Finding 1: NIT — PostReview doc comment inaccuracy (comment id 19793)
Source: sonnet-review-bot inline comment on
gitea/client.goline ~283Issue: Doc comment on
PostReviewsaysevent should be "APPROVED" or "REQUEST_CHANGES"but code also uses"COMMENT".Fix: Update the doc comment to say
event should be one of "APPROVED", "REQUEST_CHANGES", or "COMMENT".Implementation
gitea/client.go— update the doc comment onPostReviewto list all three valid event values.go test ./...to confirm no regressions.review-bot-issue-107branch.Fixed in
028d469: updated the doc comment to list all three valid event values: APPROVED, REQUEST_CHANGES, and COMMENT.Sonnet Review
Summary
This PR adds a
commitID stringparameter toPostReviewto anchor reviews to a specific commit SHA, usingomitemptyso empty values are not sent. The implementation is correct, all callers are updated, the new test for omitempty behavior is well-written, and CI is passing.Findings
gitea/client.goCommitID string \json:...`vsBody string `json:...``) that is not gofmt-canonical. This is cosmetic but gofmt would normalize it. Likely a non-issue if gofmt is enforced in CI, but worth keeping consistent.Recommendation
APPROVE — The change is small, correct, and well-tested. The
commitIDparameter is placed beforecommentsin the signature, which is a reasonable ordering (metadata before optional extras). Theomitemptytag correctly handles the empty-string case. CI passes. The single nit about struct field alignment spacing is cosmetic and not a blocker. Approve.Review by sonnet
Evaluated against
028d4694Security Review
Summary
Changes cleanly add an explicit commit_id to review submissions and pass the evaluated SHA from the PR, improving correctness without introducing new security risks. CI has passed and the new tests cover the omitempty behavior and updated call sites.
Recommendation
APPROVE — The update anchors reviews to a specific commit via commit_id and is implemented safely with proper JSON marshaling and omission when empty. Calls now pass the previously fetched pr.Head.Sha, and a stale-head check remains in place. No security vulnerabilities are introduced by these changes. As a defense-in-depth enhancement (optional), you could validate commitID format (e.g., hex SHA) before sending, but this is not required for security. Proceed with merge.
Review by security
Evaluated against
028d4694Gpt Review
Summary
Solid, focused change adding an explicit commitID to PostReview and propagating it through callers. Tests were updated comprehensively, including a new check for omitempty behavior, and CI passed.
Recommendation
APPROVE — The explicit commit anchoring via commit_id is a good improvement and is implemented cleanly with proper JSON omitempty semantics. Call sites are correctly updated, including main and integration tests, and additional tests validate new behavior. Given CI success and no issues detected in the diff, this is ready to merge.
Review by gpt
Evaluated against
028d4694Self-Review: PR #112
Self-review against
028d46942aPhase 1: Independent Findings
None — diff looks clean. The
commitID stringparameter is correctly threaded through all call sites (main.gopassesevaluatedSHA, integration test and existing tests pass""). Theomitemptytag correctly prevents serialization of empty strings. The newTestPostReview_EmptyCommitID_OmittedFromPayloadtest validates this behavior via raw JSON inspection. The doc comment was updated in this PR to include"COMMENT"as a valid event value. No correctness, error-handling, or coverage gaps found.Phase 2: Prior Review Verification
"COMMENT"as valid event value028d469: doc comment now lists all three valid event values (APPROVED, REQUEST_CHANGES, COMMENT)Assessment: ✅ Clean
All prior reviewer findings are resolved or invalid. The change is small, focused, and correct: it adds
commitIDas an explicit parameter to anchor reviews to a specific commit SHA, usesomitemptyto avoid sending empty values, updates all callers consistently, and adds a targeted test for the omitempty behavior. Ready to merge.