Summary
On GitLab, pr_code_suggestions.persistent_comment = true is supposed to keep a single ## PR Code Suggestions ✨ discussion that is edited in place across pushes. In practice, once that discussion has any reply (e.g. the author replied to address a suggestion, which makes the thread resolvable and is the only way it can be resolved on GitLab), every subsequent run of /improve leaves the original thread in place and creates a brand-new "Code Suggestions" discussion. The result is one duplicate suggestions thread per push on any MR where the author engages with the bot.
Reproduction
- Open an MR.
/improve posts discussion A with header ## PR Code Suggestions ✨ and marker <!-- {sha1} -->.
- Reply to A (any reply — required for GitLab to allow resolving the thread). Optionally resolve A.
- Push another commit so
/improve runs again.
- Expected: A's comment is edited in place with "Latest suggestions up to {sha2}" + "Previous suggestions up to commit {sha1}".
- Actual: a fresh discussion B appears containing exactly that merged body ("Latest suggestions up to {sha2}" + "Previous suggestions up to commit {sha1}"). A stays as-is.
Root cause
In pr_agent/tools/pr_code_suggestions.py:
- Around
pr_code_suggestions.py:107-110, when config.publish_output_progress=true (default) and is_auto_command=false (CLI invocations from CI), pr-agent posts a "Preparing suggestions..." note at the start of the run and stashes it as self.progress_response. On GitLab this immediately creates a fresh discussion (call it B).
- In
publish_persistent_comment_with_history at pr_code_suggestions.py:338-341, when both a previous persistent comment is found AND progress_response is set, the code does:
if progress_response: # publish to 'progress_response' comment, because it refreshes immediately
git_provider.edit_comment(progress_response, pr_comment_updated)
git_provider.remove_comment(comment)
comment = progress_response
So it edits the progress note B with the merged body and tries to delete the previous persistent comment A.
GitLabProvider.remove_comment (pr_agent/git_providers/gitlab_provider.py:761-765) calls comment.delete() inside a bare try/except that just logs. GitLab refuses to delete a note that has replies (HTTP 403), so when the author has interacted with A the delete fails silently. Net effect: A is left intact and B becomes the new persistent comment. On the next push the same thing happens, producing C, D, etc.
Both popular hypotheses are wrong, for reference:
- It is not a per-commit marker problem. Lookup is by
comment.body.startswith(initial_header) at pr_code_suggestions.py:278, where initial_header="## PR Code Suggestions ✨" is passed in from the caller (pr_code_suggestions.py:162) — a stable header.
- It is not that resolved discussions are filtered out of the listing.
GitLabProvider.get_issue_comments (gitlab_provider.py:789-790) calls self.mr.notes.list(get_all=True) which returns notes regardless of resolution state. The original is found — it just gets clobbered by the buggy edit/delete pairing.
Suggested fix
In publish_persistent_comment_with_history, when both a previous persistent comment and a progress_response exist, prefer editing the previously-found comment in place and removing the progress note, rather than the other way around. That keeps the stable thread stable across pushes and limits the silent-delete failure to a throwaway note that is safe to leave behind on error.
if progress_response:
git_provider.edit_comment(comment, pr_comment_updated)
git_provider.remove_comment(progress_response)
else:
git_provider.edit_comment(comment, pr_comment_updated)
return comment
Trade-off to be aware of
The current ordering is deliberate — the inline comment at line 338 reads # publish to 'progress_response' comment, because it refreshes immediately. The intent is that on GitLab a freshly-posted note renders in the UI more promptly than an edit of an older note. The proposed fix gives that up on the happy path (no replies) in exchange for stability when replies are present. In practice GitLab does update edited notes, so the UX cost is small, but it is a real behavioural change rather than a pure bug fix.
As a defence-in-depth, GitLabProvider.remove_comment could also detect the "cannot delete a note with replies" case and skip the delete cleanly rather than swallow the 403 — that change is purely additive and worth doing regardless.
A more conservative alternative would be to keep the current "edit the fresh note, delete the old one" path on GitHub (where deletion is reliable) and only flip it on GitLab, but that adds provider-specific branching in a generic helper. The simple swap above seems like a fair trade.
Workaround
Setting config.publish_output_progress = false (e.g. via env CONFIG__PUBLISH_OUTPUT_PROGRESS=false in CI) avoids the problem because progress_response is None and the code falls into the in-place edit path. The trade-off is losing the "Preparing suggestions..." indicator while the run is in flight.
Environment
- Provider: GitLab (gitlab.com)
- Auth: GitLab personal access token,
is_auto_command=false (CLI invocation from CI)
- Config:
pr_code_suggestions.persistent_comment=true, commitable_code_suggestions=false (also reproduces with true)
- pr-agent ref: reproduced at
009ba5a116c4d3273368a6dc53a4efdb7904d519, current main at time of filing
Summary
On GitLab,
pr_code_suggestions.persistent_comment = trueis supposed to keep a single## PR Code Suggestions ✨discussion that is edited in place across pushes. In practice, once that discussion has any reply (e.g. the author replied to address a suggestion, which makes the thread resolvable and is the only way it can be resolved on GitLab), every subsequent run of/improveleaves the original thread in place and creates a brand-new "Code Suggestions" discussion. The result is one duplicate suggestions thread per push on any MR where the author engages with the bot.Reproduction
/improveposts discussion A with header## PR Code Suggestions ✨and marker<!-- {sha1} -->./improveruns again.Root cause
In
pr_agent/tools/pr_code_suggestions.py:pr_code_suggestions.py:107-110, whenconfig.publish_output_progress=true(default) andis_auto_command=false(CLI invocations from CI), pr-agent posts a "Preparing suggestions..." note at the start of the run and stashes it asself.progress_response. On GitLab this immediately creates a fresh discussion (call it B).publish_persistent_comment_with_historyatpr_code_suggestions.py:338-341, when both a previous persistent comment is found ANDprogress_responseis set, the code does:So it edits the progress note B with the merged body and tries to delete the previous persistent comment A.
GitLabProvider.remove_comment(pr_agent/git_providers/gitlab_provider.py:761-765) callscomment.delete()inside a baretry/exceptthat just logs. GitLab refuses to delete a note that has replies (HTTP 403), so when the author has interacted with A the delete fails silently. Net effect: A is left intact and B becomes the new persistent comment. On the next push the same thing happens, producing C, D, etc.Both popular hypotheses are wrong, for reference:
comment.body.startswith(initial_header)atpr_code_suggestions.py:278, whereinitial_header="## PR Code Suggestions ✨"is passed in from the caller (pr_code_suggestions.py:162) — a stable header.GitLabProvider.get_issue_comments(gitlab_provider.py:789-790) callsself.mr.notes.list(get_all=True)which returns notes regardless of resolution state. The original is found — it just gets clobbered by the buggy edit/delete pairing.Suggested fix
In
publish_persistent_comment_with_history, when both a previous persistent comment and aprogress_responseexist, prefer editing the previously-found comment in place and removing the progress note, rather than the other way around. That keeps the stable thread stable across pushes and limits the silent-delete failure to a throwaway note that is safe to leave behind on error.Trade-off to be aware of
The current ordering is deliberate — the inline comment at line 338 reads
# publish to 'progress_response' comment, because it refreshes immediately. The intent is that on GitLab a freshly-posted note renders in the UI more promptly than an edit of an older note. The proposed fix gives that up on the happy path (no replies) in exchange for stability when replies are present. In practice GitLab does update edited notes, so the UX cost is small, but it is a real behavioural change rather than a pure bug fix.As a defence-in-depth,
GitLabProvider.remove_commentcould also detect the "cannot delete a note with replies" case and skip the delete cleanly rather than swallow the 403 — that change is purely additive and worth doing regardless.A more conservative alternative would be to keep the current "edit the fresh note, delete the old one" path on GitHub (where deletion is reliable) and only flip it on GitLab, but that adds provider-specific branching in a generic helper. The simple swap above seems like a fair trade.
Workaround
Setting
config.publish_output_progress = false(e.g. via envCONFIG__PUBLISH_OUTPUT_PROGRESS=falsein CI) avoids the problem becauseprogress_responseisNoneand the code falls into the in-place edit path. The trade-off is losing the "Preparing suggestions..." indicator while the run is in flight.Environment
is_auto_command=false(CLI invocation from CI)pr_code_suggestions.persistent_comment=true,commitable_code_suggestions=false(also reproduces withtrue)009ba5a116c4d3273368a6dc53a4efdb7904d519, currentmainat time of filing