fix(gitlab): diff-aware anchors and note fallback for review posting - #124
Merged
Conversation
gitlab-post-review sent `new_line`-only positions for every RIGHT-side
comment, but GitLab requires both line numbers for unchanged context
lines and refuses lines outside the diff entirely, so those findings
400'd and the job hard-failed (seen on a customer MR where both
approved comments targeted untouched code).
- Build a per-file line index from the MR changes and shape each
position to the line's role: added -> new_line, removed -> old_line,
context -> both.
- Post comments whose line is not in the diff (or that GitLab refuses)
as plain MR notes instead of dropping them; only comments failing
both routes count as failures, and the job only hard-fails when
nothing reached the MR at all.
- Surface the GitLab error response body in GitlabApiError messages
("400 Bad Request" alone was undebuggable from CI logs).
- Track fallback counts in review_post_results.json and the sticky
tracking note.
- Tell Pass 1/Pass 2 prompts (GitHub + GitLab) that anchors must be
lines visible in the diff.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Contributor
|
Droid finished @factory-nizar's task —— View job The GitLab anchoring and fallback improvements look solid and well-tested. A few prompt/docs/tracking strings should be updated to match the new semantics (removed/LEFT anchors and what counts as "failed"). |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
…wording - validator prompt: mention removed/LEFT lines as valid anchors and what `side` means for the line anchor - ReviewPostOutcome.failed doc + tracking note: `failed` now means both inline and note fallback failed, not an anchoring refusal - fallbackNoteBody: use old_path for LEFT-side comments and drop the "outside the MR diff" claim (API refusals also land here) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
nikhil-factory
approved these changes
Aug 13, 2026
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
2 participants
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
On a customer MR,
gitlab-post-reviewfailed to post both approved findings with an opaqueGitLab API 400: Bad Request, then hard-failed the job.Root cause:
buildPosition()sendsnew_line-only positions for every RIGHT-side comment, but GitLab's discussions API requires:new_lineonlyold_lineonlyold_lineandnew_lineBoth findings targeted lines the MR never touched, so every payload shape was refused, and the
posted === 0guard turned two bad anchors into a red pipeline. On top of that, the actual GitLab error body (line_code must be a valid line code-style detail) was captured inGitlabApiError.bodybut never shown, making this undebuggable from CI logs.Fix
buildDiffIndex), and shape each position to the line's actual role in the diff — added →new_line, removed →old_line, context → both numbers.path:line, instead of being dropped. Only comments failing both routes count as failures.GitlabApiError.messagenow includes the response body, so the next report says why GitLab refused.linemust be visible in the diff and that out-of-diff findings should re-anchor to the nearest related changed line.review_post_results.jsonand render in the sticky tracking note.If the changes fetch itself fails, posting degrades gracefully to the previous anchor-as-given behavior (plus the note fallback).
Validation
bun test: 575 pass, 0 fail (new coverage:buildDiffIndexparsing, context/added/removed anchoring, out-of-diff fallback, note-fallback failure accounting, entrypoint fallback flow, error-body surfacing, tracking-note rendering).bun run typecheck: clean.Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
https://gitlab.com/factory-nizar/droid-review-anchor-e2e/-/merge_requests/1
Closes VAL-14