fix(ai-review): don't fail the whole review when inline lines can't anchor#99
Merged
Merged
Conversation
…nchor GitHub rejects an entire pulls.createReview call with 422 "Line could not be resolved" when any comment targets a line outside the PR diff, common for unsupported languages where the semantic diff falls back to whitespace filtering and line numbers no longer map to diff positions. ClearPR let that 422 fail the review and post a "Review failed" comment, losing all findings (surfaced by ClearPR reviewing its own markdown docs PR). Now postInlineComments returns whether the comments anchored. On a 422 it degrades instead of throwing, and the summary lists the findings as text (severity, path:line, body) so nothing is lost. Other errors still propagate. Adds statusCode to GitHubApiError for precise detection and a summary-builder test for the fallback path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (found by dogfooding)
ClearPR reviewed its own docs PR (#98, markdown) and posted "Review failed: Unprocessable Entity: 'Line could not be resolved'" three times instead of a review.
pulls.createReviewposts all inline comments in one call; if any comment'slineisn't part of the PR diff, GitHub 422s the entire review. For unsupported languages (markdown here) the semantic diff falls back to whitespace filtering, so the LLM's line numbers don't map to diff positions. ClearPR let the 422 propagate, which posted a failure comment and dropped every finding (the summary only shows counts, not bodies).Fix
ReviewPosterPort.postInlineCommentsnow returnsboolean, true if anchored, false if GitHub couldn't resolve the lines. Other errors still throw.422(GitHubApiError.statusCode, newly exposed) and degrades with a warning instead of failing.BuildReviewSummaryUseCaselists each finding as text ([severity] path:line body) so findings are preserved. Refactored into a helper to stay under the complexity gate.Result
Markdown / unsupported-language PRs get a real review summary with findings instead of a "Review failed" comment. Inline anchoring still works for supported languages.
Tests
New summary-builder test for the unanchored path; updated the e2e fake poster. Full suite 175/175, tsc/eslint/complexity/build clean.
Note
Future improvement (not here): post per-comment so resolvable lines still anchor even when some don't.