feat(gitlab): batch-publish committable code suggestions as one review - #2635
Merged
IsmaelMartinez merged 5 commits intoAug 26, 2026
Conversation
commitable_code_suggestions posts each suggestion as its own live GitLab discussion the moment it's created, so every suggestion fires its own notification/email. GitHub already avoids this by batching suggestions into a single create_review() call (see The-PR-Agent#361); GitLab had no equivalent. Add gitlab.publish_code_suggestions_as_review (default false): when enabled, suggestions are queued as GitLab draft notes and published together with a single draft_notes.bulk_publish() call, mirroring the GitLab UI's "start a review" flow. Suggestions remain fully committable either way - this only changes delivery timing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
PR Summary by QodoGitLab: optionally batch-publish committable suggestions via draft notes
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Contributor
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Addresses two valid findings from the Qodo automated review on this PR: - publish_code_suggestions() unconditionally called mr.draft_notes.bulk_publish() whenever publish_code_suggestions_as_review was enabled, even if this invocation queued zero drafts (e.g. an empty or all-failed-to-parse suggestion list). That could publish unrelated drafts already pending on the MR from a previous run. send_inline_comment now reports whether it actually created something, and bulk_publish is only called when at least one draft was queued this call. - The persistent-inline-comments dedup scanner for GitLab only scanned discussions and plain notes, missing draft notes queued by the new batching mode. A suggestion whose draft is still pending (e.g. after a bulk-publish failure) would not be recognized on a later run and could be re-queued as a duplicate. iter_existing_inline_comment_bodies now also scans mr.draft_notes.list(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the review - going through all 4 findings: Fixed (2, 4):
Not applying as suggested (1, 3), with reasoning:
|
Comment on lines
+645
to
+648
| if as_draft: | ||
| self.mr.draft_notes.create({'note': body, 'position': pos_obj}) | ||
| else: | ||
| self.mr.discussions.create({'body': body, 'position': pos_obj}) |
Contributor
There was a problem hiding this comment.
2. Draft failure drops suggestions 🐞 Bug ☼ Reliability
When as_draft=True, send_inline_comment() uses mr.draft_notes.create() for both the primary inline comment and the fallback general note, and never falls back to the previous live discussions.create()/notes.create() behavior if draft-note creation fails. If the draft-notes endpoint is unsupported/unavailable or consistently errors (e.g., 403/404), batch mode can fail to post suggestions at all.
Agent Prompt
### Issue description
In batch mode, `send_inline_comment(..., as_draft=True)` posts via `mr.draft_notes.create()` in both the primary (inline) and fallback (general note) paths. If draft-note creation fails due to environment/API constraints, the code retries another draft-note creation in the fallback path and ultimately returns `False` without attempting to post a live discussion/note. This can cause suggestions to be silently dropped for users who enable batch mode but cannot use draft notes.
### Issue Context
The pre-PR behavior (`as_draft=False`) used `mr.discussions.create()` for inline and `mr.notes.create()` for fallback. Batch mode should degrade gracefully back to that behavior when draft notes are not usable.
### Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[643-717]
### Suggested fix approach
- When `as_draft=True` and `mr.draft_notes.create(...)` raises, detect “drafts unavailable” style failures (or conservatively any exception) and retry using the live endpoints:
- Primary retry: `mr.discussions.create({'body': body, 'position': pos_obj})`
- Fallback retry: `mr.notes.create({'body': body_fallback, 'position': fallback_position})`
- Ensure `publish_code_suggestions()` does not call `bulk_publish()` if the run fell back to live comments (or track draft-vs-live separately).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
Code review by qodo was updated up to the latest commit f2a4e00 |
…lk-publish on real MR state Addresses two more findings from the Qodo review round on this PR: - "Draft failure drops suggestions": when as_draft=True, both the primary and fallback comment-creation attempts used mr.draft_notes.create(). If draft notes are unsupported/erroring outright for an MR (older self-hosted GitLab, permission gap), every suggestion in a batch run was silently dropped instead of posting live as they would with the flag off. The create+fallback logic is now extracted into _create_suggestion_note(); if a suggestion's draft attempt fails completely, send_inline_comment retries the same suggestion live instead of giving up on it. - "Pending drafts never published": the previous fix gated bulk_publish() on whether *this* call created a draft, but persistent_inline_comments dedup can now also match markers embedded in still-pending drafts (e.g. left over from an earlier bulk_publish failure). A run where every suggestion is skipped as a duplicate of one of those pending drafts would never call bulk_publish(), leaving them stuck forever. publish_code_suggestions() now checks the MR's actual pending drafts (mr.draft_notes.list()) instead of in-memory tracking, so it keeps retrying a stuck publish even when this run queued nothing new. Extended tests for both scenarios, plus a stateful draft_notes test fixture (create/list/bulk_publish behave like the real API) instead of asserting on call counts alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
Second round - the 2 new findings, both valid, fixed: Fixed:
Also switched the test fixture's Carried over from the previous round, still declined for the same reasons already given (not re-litigating in full here, see the earlier reply above):
|
Contributor
|
Code review by qodo was updated up to the latest commit 533f2b7 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| score = original_suggestion.get('score', 7) | ||
|
|
||
| if hasattr(self, 'main_language'): | ||
| language = self.main_language |
| if hasattr(self, 'main_language'): | ||
| language = self.main_language | ||
| else: | ||
| language = '' |
# Conflicts: # pr_agent/algo/inline_comment_dedup.py
…ebug block Where draft notes are unavailable, every suggestion has already degraded to a live discussion, so a failing draft_notes.list() must not log them as pending drafts needing a manual publish from the UI.
Contributor
Code Review by Qodo
1. Publishes unrelated pending drafts
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1158
to
+1159
| if pending: | ||
| self.mr.draft_notes.bulk_publish() |
Contributor
There was a problem hiding this comment.
7. Publishes unrelated pending drafts 🐞 Bug ≡ Correctness
bulk_publish() acts on the MR's entire pending draft collection, so when this run queues a suggestion it also publishes any unrelated manual or earlier drafts owned by the same GitLab user. The pending check only avoids this when the collection is empty; it does not isolate the drafts created by this batch.
Agent Prompt
## Issue description
Batch publication currently publishes every pending draft on the MR, including unrelated drafts from the same GitLab user.
## Issue Context
GitLab's MR-level `bulk_publish()` is not scoped to the notes created by this invocation. Track and distinguish PR-Agent-owned drafts, and do not invoke the MR-wide operation when unrelated drafts are pending; use a safe fallback when isolation cannot be guaranteed.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[1142-1159]
- tests/unittest/test_gitlab_batch_publish_suggestions.py[170-181]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1153
to
+1157
| except Exception as e: | ||
| # Draft notes are unusable on this instance/token; send_inline_comment has | ||
| # already degraded every suggestion to a live comment, so nothing is pending. | ||
| get_logger().warning(f"Could not list draft notes for MR {self.id_mr}: {e}") | ||
| pending = [] |
Contributor
There was a problem hiding this comment.
8. List failure strands drafts 🐞 Bug ☼ Reliability
If draft creation succeeds but the subsequent draft_notes.list() call fails, the handler sets pending to an empty list and skips bulk_publish(). Those queued suggestions remain invisible to reviewers even though publish_code_suggestions() returns success; the comment claiming every suggestion already degraded to a live comment is only true when creation, not listing, failed.
Agent Prompt
## Issue description
A transient failure while listing draft notes suppresses publication of drafts that were already created successfully.
## Issue Context
Track whether this invocation successfully queued drafts independently of the listing request. On listing failure, either safely attempt publication/retry it or return a failure that accurately signals the unpublished batch; do not replace an unknown state with an empty collection.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[1136-1159]
- tests/unittest/test_gitlab_batch_publish_suggestions.py[97-111]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1166
to
+1169
| get_logger().exception( | ||
| f"Failed to bulk-publish draft code-suggestion notes for MR {self.id_mr}; they remain " | ||
| f"as pending drafts, visible only to the posting user, until published manually from " | ||
| f"the GitLab UI or by a subsequent successful run: {e}") |
Contributor
There was a problem hiding this comment.
9. Publish failure reports success 🐞 Bug ☼ Reliability
When bulk_publish() fails, the exception is only logged and publish_code_suggestions() still returns True, so callers do not run their existing retry path while every queued suggestion remains visible only to the posting user. A transient final API failure therefore silently suppresses the whole review until an unrelated future run or manual action occurs.
Agent Prompt
## Issue description
A failed bulk publication is reported as successful to callers even though no reviewer can see the queued suggestions.
## Issue Context
Retry the bulk-publication operation safely and/or return an accurate failure result after exhausting retries. Ensure caller retries cannot enqueue duplicate drafts, especially when persistent inline deduplication is disabled.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[1160-1172]
- pr_agent/tools/pr_code_suggestions.py[685-689]
- tests/unittest/test_gitlab_batch_publish_suggestions.py[157-167]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
IsmaelMartinez
approved these changes
Aug 26, 2026
IsmaelMartinez
left a comment
IsmaelMartinez
left a comment
Collaborator
There was a problem hiding this comment.
Approving. I pushed the rebase and both fixes myself rather than send you round again, each on its own commit, and refreshed the description.
The batching holds up under test, and the third-round rework is the right shape: reading the MR's real pending-draft list rather than an in-memory flag means a stuck publish gets retried on the next run instead of stranding drafts. I am content with both findings you declined. Opt-in is right when drafts stay invisible until published, and the return-True argument stands now that the retry exists.
The one thing worth knowing: where draft notes are unavailable, every suggestion correctly degraded to a live comment and then the pending-draft listing failed too, so the handler reported them as stuck drafts needing a manual publish. They were already live. That is the fix in the second commit. The conflict itself was a single keep-both hunk from #2651, not #2774.
Thanks for turning three rounds around in a day, and sorry it then sat a fortnight.
| p.mr.draft_notes.bulk_publish.assert_called_once() | ||
|
|
||
|
|
||
| def test_empty_suggestions_does_not_bulk_publish_unrelated_pending_drafts(): |
Collaborator
There was a problem hiding this comment.
One note for later, no change needed: this does not quite prove what the comment above it claims. The fixture starts with no pending drafts, so it only shows that nothing-pending means no publish. Seed one unrelated pending draft and an empty run does publish it, because the gate is the MR's real state. That is the trade you chose in round two and I think it is the right one, but the name reads as a guarantee it is not making.
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
3 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.
Summary
commitable_code_suggestionson GitLab posts each suggestion as its own live discussion the moment it's created, so every suggestion fires its own notification (and email, if configured). GitHub already avoids this:GithubProvider.publish_code_suggestions()batches everything into a singlecreate_review(comments=[...])call, which is exactly what #361 asked for and fixed - but only on the GitHub side. GitLab never got an equivalent.This adds an opt-in
gitlab.publish_code_suggestions_as_reviewsetting (defaultfalse, so existing behavior is unchanged). When enabled,GitLabProvider.publish_code_suggestions():mr.draft_notes.create(...), already supported by the pinnedpython-gitlab==8.3.0) instead of an immediate live discussion,mr.draft_notes.bulk_publish()call once the loop finishes - the same mechanism GitLab's own "start a review" UI flow uses.Suggestions remain fully committable either way; this only changes delivery timing/notification volume. The existing position-rejected fallback path (general note when a suggestion can't be anchored to a diff line) also switches to a draft note in this mode, so nothing leaks out as a live comment ahead of the batch.
One change is not behind the new flag:
pr_agent/algo/inline_comment_dedup.pynow also scans pending draft notes on the GitLab path. Without it, a marker sitting in a draft left over from a run whose bulk-publish failed would be invisible to the dedup scan and the suggestion would be re-posted as a duplicate once published. That path is still gated behind the existingconfig.persistent_inline_comments(defaultfalse).Naming follows the existing
publish_X_as_Yconvention already used in this file (publish_review_as_thread) and elsewhere (publish_description_as_comment,publish_as_check_run).I searched existing issues/PRs for prior art on this before starting - #361 is the closest (GitHub-side version of the same complaint, already fixed) - and found nothing open for the GitLab side.
Test plan
tests/unittest/test_gitlab_batch_publish_suggestions.py, 8 tests: flag off leaves behaviour unchanged, flag on queues drafts and bulk-publishes once, the position-rejected fallback uses a draft note rather than a live note, a bulk-publish failure is caught and does not propagate, a total draft failure degrades every suggestion to a live comment, an empty suggestion list does not publish, an all-failed list does not publish, and a still-pending draft's marker is seen by the dedup scan.tests/unittest/test_inline_comment_dedup.pyto cover the new GitLab draft-note scan.ruff checkon the touched provider file: no new findings relative to main.commitable_code_suggestions=trueandpublish_code_suggestions_as_review=trueDocs
Added a "Batch-publishing committable suggestions on GitLab" section to
docs/docs/tools/improve.md, next to the existing "Persistent inline comments" section.