Fix incremental compare-diff request for CI-triggered reviews #71

New issue

Closed

opened 2026-05-04 10:19:23 -07:00 by jwilger · 0 comments

jwilger commented

2026-05-04 10:19:23 -07:00

Owner

Problem

CI-triggered reviews are now reaching POST /reviews/ci, but the orchestrator logs show incremental compare-diff lookup failing before falling back to the full PR diff:

incremental review: fetching compare diff repo="jwilger/auto_review" pr=70 previous=f82852bd54475c63c832a72221f74bdcb5512c97 current=4202056c2357ae0c337126570e081f489041a6c7
compare_diff failed; falling back to full diff error=API error 404: {"message":"The target couldn't be found.","url":"https://git.johnwilger.com/api/swagger","errors":["could not find '4202056c2357ae0c337126570e081f489041a6c7.diff' to be a commit, branch or tag in the head repository jwilger/auto_review"]}

The fallback lets the review complete, but it defeats incremental review efficiency and creates noisy logs.

Expected behavior

When both previous and current review SHAs are valid PR head commits, incremental compare-diff should fetch the relevant diff without falling back.

Direction

Investigate how ar-orchestrator builds the Forgejo compare-diff request for incremental reviews. Verify whether it is using an invalid Forgejo compare API shape, wrong owner/repo context, raw .diff path construction, or a head/base ordering issue.

Acceptance

Add a focused regression test for incremental compare-diff URL/request construction or Forgejo client behavior.
Fix the compare-diff call so valid previous/current PR head SHAs do not produce the observed 404.
Preserve the existing full-diff fallback for genuinely missing commits or Forgejo failures.
Logs distinguish expected fallback cases from request-construction bugs where possible.

Notes

Observed while validating PR #70 / issue #43. Review still posted successfully via fallback (review_id=796, findings=1).

## Problem CI-triggered reviews are now reaching `POST /reviews/ci`, but the orchestrator logs show incremental compare-diff lookup failing before falling back to the full PR diff: ```text incremental review: fetching compare diff repo="jwilger/auto_review" pr=70 previous=f82852bd54475c63c832a72221f74bdcb5512c97 current=4202056c2357ae0c337126570e081f489041a6c7 compare_diff failed; falling back to full diff error=API error 404: {"message":"The target couldn't be found.","url":"https://git.johnwilger.com/api/swagger","errors":["could not find '4202056c2357ae0c337126570e081f489041a6c7.diff' to be a commit, branch or tag in the head repository jwilger/auto_review"]} ``` The fallback lets the review complete, but it defeats incremental review efficiency and creates noisy logs. ## Expected behavior When both previous and current review SHAs are valid PR head commits, incremental compare-diff should fetch the relevant diff without falling back. ## Direction Investigate how `ar-orchestrator` builds the Forgejo compare-diff request for incremental reviews. Verify whether it is using an invalid Forgejo compare API shape, wrong owner/repo context, raw `.diff` path construction, or a head/base ordering issue. ## Acceptance - Add a focused regression test for incremental compare-diff URL/request construction or Forgejo client behavior. - Fix the compare-diff call so valid previous/current PR head SHAs do not produce the observed 404. - Preserve the existing full-diff fallback for genuinely missing commits or Forgejo failures. - Logs distinguish expected fallback cases from request-construction bugs where possible. ## Notes Observed while validating PR #70 / issue #43. Review still posted successfully via fallback (`review_id=796`, `findings=1`).

jwilger referenced this issue from a pull request that will close it,

2026-05-04 12:46:16 -07:00

fix(forgejo): use web compare diff route #73

jwilger closed this issue

2026-05-04 12:51:37 -07:00

jwilger referenced this issue from a commit

2026-05-04 12:51:38 -07:00

fix(forgejo): use web compare diff route (#73)