Skip to content

Fix/line number validation#31

Open
supasympa wants to merge 14 commits into
mainfrom
fix/line-number-validation
Open

Fix/line number validation#31
supasympa wants to merge 14 commits into
mainfrom
fix/line-number-validation

Conversation

@supasympa

Copy link
Copy Markdown
Contributor

No description provided.

supasympa added 14 commits May 7, 2026 21:12
Prevent 422 'Path could not be resolved' errors when creating review
comments on files that don't exist in the PR's diff (e.g., deleted files).

- Add ListPRFiles() to fetch PR's changed files
- Filter buildReviewComments() to only include files present in PR diff
- Add tests for groupFindings and buildReviewComments
- TestReport_RetryWithoutCommentsOnPositionError: verifies retry logic
- TestReport_FallsBackToStickyCommentWhenBothReviewAttemptsFail
- TestReport_SuccessfulReviewOnFirstTry
- TestReport_UsesPRFilesFilter

Also converts GitHubReporter.client to use interface for testability.
- Add GetPRFileDiffs() to fetch file diffs with line counts
- Add validateLine() to cap line numbers to actual changed lines
- buildReviewComments now validates line numbers against actual diff
- Skip comments for invalid/outsized line numbers instead of failing

This should fix 'Line could not be resolved' errors when AI reports
line numbers beyond the actual changes in the diff.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acig: pass | risk=low | 0 finding(s) | $0.0007

Severity Critic Title File
Verdict JSON
{"schema_version":"1","repo":"https://github.com/helloodokai/acig","sha":"41fe47113336938ffd735072c7057cbf25369648","base_sha":"main","risk":"low","decision":"pass","summary":"Decision: pass | Risk: low | Findings: 0 blocking, 0 high, 0 medium, 0 low, 0 info | Cost: $0.0007","findings":[],"critic_results":[{"critic":"perf_smell","model":"qwen3-coder:480b","findings":[],"cost_usd":0.00019878,"duration_ms":1365,"tokens_in":3274,"tokens_out":13},{"critic":"security_smell","model":"qwen3-coder:480b","findings":[],"cost_usd":0.00020082000000000002,"duration_ms":3329,"tokens_in":3308,"tokens_out":13},{"critic":"risk_classifier","model":"gpt-oss:20b","findings":null,"cost_usd":0.00009397,"duration_ms":16022,"tokens_in":3253,"tokens_out":2048},{"critic":"test_coverage_smell","model":"gpt-oss:20b","findings":null,"cost_usd":0.00009366999999999999,"duration_ms":16118,"tokens_in":3223,"tokens_out":2048},{"critic":"style_conformance","model":"gpt-oss:20b","findings":null,"cost_usd":0.00009353,"duration_ms":16450,"tokens_in":3209,"tokens_out":2048}],"total_cost_usd":0.00068077,"total_duration_ms":53284,"budget_remaining_usd":0.24931923,"generated_at":"2026-05-08T00:09:30.076574888Z"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant