feat(gitlab): add option to publish /review output as a resolvable threads

[aivarsliepa]

Fixes #2534

What

Adds an opt-in GitLab setting to post the /review summary as a resolvable thread (discussion) instead of a plain note:

[gitlab]
publish_review_as_thread = true  # default: false

Resolvable threads let reviewers treat the AI review like any human review: resolve it when addressed, and have it resurface when the review is refreshed.

Behavior

• Scope: only the review's final comment is affected.
• With pr_reviewer.persistent_comment=true (default): the first run opens a thread; subsequent runs edit it in place and reopen (unresolve) it if it had been resolved.
• With pr_reviewer.persistent_comment=false: each run opens a new thread; previous review threads are untouched.
• Fallback: if thread creation fails, the review is published as a plain note

[qodo-free-for-open-source-projects]

PR Summary by Qodo

feat(gitlab): publish /review output as a resolvable thread

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Adds opt-in GitLab setting publish_review_as_thread to post the /review summary as a
 resolvable discussion thread instead of a plain note.
• With pr_reviewer.persistent_comment=true, subsequent runs edit the existing thread in place and
 reopen (unresolve) it if resolved; falls back to a plain note if thread creation fails.
• Extends the base GitProvider interface with should_publish_review_as_thread and
 unresolve_comment_thread hooks, and threads them through
 publish_comment/publish_persistent_comment.
• Adds documentation for the new config option and extensive unit tests covering thread creation,
 fallback, reopening, and persistent-comment interactions.

Diagram

sequenceDiagram
  participant Reviewer as PRReviewer
  participant Provider as GitProvider Base
  participant GitLab as GitLabProvider
  participant API as GitLab Discussions API
  Reviewer->>Provider: should_publish_review_as_thread()
  Provider->>GitLab: check GITLAB.PUBLISH_REVIEW_AS_THREAD
  Reviewer->>GitLab: publish_persistent_comment(as_thread=true)
  GitLab->>API: discussions.create(body)
  API-->>GitLab: discussion or error
  alt success
    GitLab-->>Reviewer: resolvable thread posted
  else failure
    GitLab->>API: notes.create(body)
    API-->>GitLab: plain note
    GitLab-->>Reviewer: fallback note posted
  end
  Reviewer->>GitLab: unresolve_comment_thread(comment)
  GitLab->>API: discussions.list / resolved=false / save

Loading

High-Level Assessment

The chosen approach—an opt-in flag with graceful fallback to plain notes on failure—is appropriate and low-risk. It reuses the existing publish_comment/publish_persistent_comment abstraction via an added as_thread kwarg rather than introducing parallel code paths, keeping the change contained to the GitLab provider while preserving default behavior for all other providers.

Files changed (7) +327 / -9

Enhancement (3) +52 / -9

git_provider.pyAdd thread-publishing hooks to base GitProvider +15/-4

Add thread-publishing hooks to base GitProvider

• Introduces default no-op should_publish_review_as_thread and unresolve_comment_thread methods, and adds an as_thread parameter to publish_persistent_comment/publish_persistent_comment_full that is forwarded to publish_comment and used to trigger thread reopening on update.

pr_agent/git_providers/git_provider.py

gitlab_provider.pyImplement resolvable-thread publishing for GitLab reviews +31/-3

Implement resolvable-thread publishing for GitLab reviews

• Adds should_publish_review_as_thread reading the new setting, extends publish_comment to create a GitLab discussion (with fallback to a plain note on failure) when as_thread is set, and implements unresolve_comment_thread to reopen a resolved discussion tied to a given comment.

pr_agent/git_providers/gitlab_provider.py

pr_reviewer.pyWire review publishing to opt-in thread mode +6/-2

Wire review publishing to opt-in thread mode

• Computes as_thread kwargs based on should_publish_review_as_thread and passes them only to the final review publish call (persistent or plain), leaving other comments unaffected.

pr_agent/tools/pr_reviewer.py

Tests (2) +262 / -0

test_gitlab_provider.pyAdd unit tests for GitLab thread publishing +202/-0

Add unit tests for GitLab thread publishing

• Adds comprehensive tests covering should_publish_review_as_thread config reflection, publish_comment thread creation/fallback/error propagation, temporary comment exclusion, persistent comment thread reopening, and unresolve_comment_thread behavior.

tests/unittest/test_gitlab_provider.py

test_pr_reviewer_core.pyAdd test verifying as_thread forwarding in PRReviewer.run +60/-0

Add test verifying as_thread forwarding in PRReviewer.run

• Adds a parametrized async test ensuring as_thread is forwarded only to the final review publish call when the provider opts in, and never to temporary progress comments.

tests/unittest/test_pr_reviewer_core.py

Documentation (1) +11 / -0

additional_configurations.mdDocument the new publish_review_as_thread GitLab setting +11/-0

Document the new publish_review_as_thread GitLab setting

• Adds a new documentation section explaining how to enable posting the /review summary as a resolvable GitLab thread, including interaction with persistent_comment.

docs/docs/usage-guide/additional_configurations.md

Other (1) +2 / -0

configuration.tomlAdd publish_review_as_thread default setting +2/-0

Add publish_review_as_thread default setting

• Adds the new gitlab.publish_review_as_thread configuration key defaulting to false.

pr_agent/settings/configuration.toml

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Trailing whitespace in comment 📘 Rule violation ⚙ Maintainability

Description

A new test comment line includes trailing whitespace, which can fail repository hygiene/pre-commit
checks and creates noisy diffs in future edits.

Code

tests/unittest/test_gitlab_provider.py[415]

+        # persistent_comment=true with an existing review thread: edit it in place 

Evidence

PR Compliance ID 16 requires basic repository hygiene (no trailing whitespace). The line at
tests/unittest/test_gitlab_provider.py:415 ends with an extra space after place, introducing
trailing whitespace.

AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity): AGENTS.md: Repository Hygiene Checks Must Pass (Trailing Whitespace, Final Newlines, TOML/YAML Validity)
tests/unittest/test_gitlab_provider.py[415-415]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new line in a test file contains trailing whitespace, violating repository hygiene checks.
## Issue Context
Trailing whitespace commonly fails pre-commit hooks and should be removed.
## Fix Focus Areas
- tests/unittest/test_gitlab_provider.py[415-415]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. String bool misparsed 🐞 Bug ≡ Correctness

Description

GitLabProvider.should_publish_review_as_thread() wraps the setting in bool(...), so an env override
like "false" becomes truthy and unexpectedly enables publishing the review as a thread. This is
plausible in production because AUTO_CAST_FOR_DYNACONF is forced to "false", so env-layer values
commonly arrive as strings rather than native booleans.

Code

pr_agent/git_providers/gitlab_provider.py[R513-514]

+    def should_publish_review_as_thread(self) -> bool:
+        return bool(get_settings().get("GITLAB.PUBLISH_REVIEW_AS_THREAD", False))

Evidence

AUTO_CAST_FOR_DYNACONF is explicitly set to "false", meaning configuration values sourced from env
overrides can remain strings; using bool("false") evaluates to True. The codebase already
documents/handles this exact string-coercion problem elsewhere with an explicit _as_bool helper.

pr_agent/git_providers/gitlab_provider.py[510-516]
pr_agent/log/init.py[1-4]
pr_agent/git_providers/utils.py[239-246]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[690-714]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`should_publish_review_as_thread()` currently returns `bool(get_settings().get(...))`. If the setting is sourced from an environment variable (or other string-based layer) and is set to the string `"false"`, Python treats it as truthy and the feature is enabled unexpectedly.
## Issue Context
This repo forces `AUTO_CAST_FOR_DYNACONF="false"`, so Dynaconf env overrides may arrive as strings; other code already contains defensive string-to-bool coercion.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[513-514]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[698-713]
- pr_agent/git_providers/utils.py[240-242]
- pr_agent/log/__init__.py[2-2]
## Implementation notes
- Replace `bool(...)` with explicit coercion:
- If value is `bool`, return it.
- If value is `str`, return `value.strip().lower() in ("1","true","yes","on")`.
- Otherwise fall back to the provided default.
- Consider extracting/reusing a small shared helper (the OpenRouter `_as_bool` pattern already exists).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. unresolve_comment_thread failure duplicates review ✓ Resolved 📘 Rule violation ☼ Reliability

Description

publish_persistent_comment_full() can post a new review comment after successfully editing the
existing one if unresolve_comment_thread() raises, because the broad outer except triggers the
fallback publish. This can cause duplicated review comments/threads and inconsistent
persistent-review behavior.

Code

pr_agent/git_providers/git_provider.py[R376-387]

           self.edit_comment(comment, pr_comment_updated)
+                    if as_thread:
+                        # Reopen the thread if it was resolved, so the developer revisits the updated review.
+                        self.unresolve_comment_thread(comment)
           if final_update_message:
               return self.publish_comment(
                   f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}")
           return comment
except Exception as e:
   get_logger().exception(f"Failed to update persistent review, error: {e}")
   pass
-        return self.publish_comment(pr_comment)
+        return self.publish_comment(pr_comment, **({'as_thread': True} if as_thread else {}))

Evidence

The Robust Error Handling rule requires graceful handling without creating incorrect side effects.
Here, an exception in unresolve_comment_thread() (after edit_comment() succeeds) is handled by
the outer except, which then publishes a new comment, potentially duplicating the review.

Rule 3: Robust Error Handling
pr_agent/git_providers/git_provider.py[376-387]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`publish_persistent_comment_full()` edits an existing persistent comment and then calls `unresolve_comment_thread(comment)`. If `unresolve_comment_thread` throws, the surrounding `try/except` catches it and falls back to publishing a *new* comment, which can duplicate the review output.
## Issue Context
This risk was introduced by adding the `as_thread` flow and calling `unresolve_comment_thread` inside the same `try` that controls the persistent-comment update and fallback behavior.
## Fix Focus Areas
- pr_agent/git_providers/git_provider.py[376-387]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Broad except Exception in GitLab 📘 Rule violation ⛨ Security

Description

New GitLab-thread publishing logic uses broad except Exception blocks that swallow unexpected
failures and proceed, which can hide API/response-shape bugs and make behavior hard to reason about.
Narrowing exception types and handling specific failure modes would better enforce
security/reliability boundaries for provider I/O.

Code

pr_agent/git_providers/gitlab_provider.py[R529-542]

+        if as_thread and not is_temporary:
+            try:
+                discussion = self.mr.discussions.create({'body': mr_comment})
+            except Exception as e:
+                get_logger().warning(f"Failed to publish comment as a thread, falling back to a note: {e}")
+            else:
+                # Return the underlying note so callers keep note-level semantics (edit/remove/url by id).
+                # The thread already exists here, so a failure must not fall back to a note
+                # (it would duplicate the review); return None instead.
+                try:
+                    return self.mr.notes.get(discussion.attributes['notes'][0]['id'])
+                except Exception as e:
+                    get_logger().warning(f"Published review thread but failed to fetch its note: {e}")
+                    return None

Evidence

The security-boundary rule requires avoiding broad exception masking. The new GitLab-thread code
catches all exceptions in both thread creation/note fetching and thread reopening, logging and
continuing rather than handling specific expected exceptions distinctly.

pr_agent/git_providers/gitlab_provider.py[529-542]
pr_agent/git_providers/gitlab_provider.py[552-567]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GitLabProvider.publish_comment(..., as_thread=True)` and `GitLabProvider.unresolve_comment_thread()` catch `Exception` broadly and continue/return, which can mask unexpected errors (e.g., unexpected payload shapes, attribute errors, programmer mistakes) and reduce diagnosability.
## Issue Context
These code paths are provider network operations and response parsing; compliance requires targeted error handling and avoiding broad exception masking where possible.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[529-542]
- pr_agent/git_providers/gitlab_provider.py[552-567]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (3)

5. Unguarded discussion.attributes['notes'][0] ✓ Resolved 📘 Rule violation ☼ Reliability

Description

When publishing a comment as a GitLab thread, the code assumes
discussion.attributes['notes'][0]['id'] always exists and is well-formed, so an unexpected or
empty notes payload can raise (IndexError/KeyError/TypeError) and abort publishing with no
graceful fallback. Because this exception can occur after the thread is created, it can also prevent
downstream cleanup (e.g., leaving temporary "Preparing review..." comments behind).

Code

pr_agent/git_providers/gitlab_provider.py[R529-536]

+        if as_thread and not is_temporary:
+            try:
+                discussion = self.mr.discussions.create({'body': mr_comment})
+            except Exception as e:
+                get_logger().warning(f"Failed to publish comment as a thread, falling back to a note: {e}")
+            else:
+                # Return the underlying note so callers keep note-level semantics (edit/remove/url by id).
+                return self.mr.notes.get(discussion.attributes['notes'][0]['id'])

Evidence

PR Compliance ID 3 calls for graceful handling of edge cases, but the new as_thread=True publish
path directly indexes discussion.attributes['notes'][0]['id'] and then calls mr.notes.get(...)
without validating the response shape, so a missing/empty/malformed notes payload can raise and
stop the flow. The try/exception handling only wraps discussions.create, meaning failures during
note-id extraction or note retrieval will propagate after the discussion already exists. Since
PRReviewer posts a temporary progress note and only calls remove_initial_comment() after
publishing completes, a publish-time exception can leave those temporary notes undeleted.

Rule 3: Robust Error Handling
pr_agent/git_providers/gitlab_provider.py[529-536]
pr_agent/git_providers/gitlab_provider.py[522-540]
pr_agent/tools/pr_reviewer.py[163-200]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GitLabProvider.publish_comment(..., as_thread=True)` assumes the created discussion always contains `attributes['notes'][0]['id']`; if `notes` is missing/empty or shaped differently, the code raises (KeyError/IndexError/TypeError) and review publishing fails. Because this can happen after the discussion is created and before the caller finishes, it can also prevent cleanup (e.g., leaving temporary "Preparing review..." comments behind).
## Issue Context
This is a (new) code path used for the final `/review` output when `GITLAB.PUBLISH_REVIEW_AS_THREAD=true`. Failures here should degrade gracefully without duplicating the review comment (e.g., avoid creating a second plain note as a fallback), and the caller (PRReviewer) currently removes temporary progress comments only after publishing succeeds.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[522-540]
- pr_agent/tools/pr_reviewer.py[163-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Thread reopen scans discussions ✓ Resolved 🐞 Bug ➹ Performance

Description

GitLabProvider.unresolve_comment_thread() fetches all MR discussions (get_all=True) and linearly
scans their attributes to find the discussion owning a note ID, adding a full discussions listing on
every persistent threaded review update. On discussion-heavy MRs this increases API traffic and can
noticeably slow review refreshes.

Code

pr_agent/git_providers/gitlab_provider.py[R546-555]

+    def unresolve_comment_thread(self, comment):
+        try:
+            for discussion in self.mr.discussions.list(get_all=True):
+                notes = discussion.attributes.get('notes', [])
+                if not any(note.get('id') == comment.id for note in notes):
+                    continue
+                if any(note.get('resolvable') and note.get('resolved') for note in notes):
+                    discussion.resolved = False
+                    discussion.save()
+                return

Evidence

The reopen implementation explicitly lists all discussions with get_all=True and scans for a
matching note id. The persistent threaded update path calls unresolve_comment_thread(comment)
immediately after editing the persistent comment when as_thread=True, so this extra full-list call
can occur on every refresh.

pr_agent/git_providers/gitlab_provider.py[546-558]
pr_agent/git_providers/git_provider.py[363-387]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`unresolve_comment_thread()` calls `mr.discussions.list(get_all=True)` and scans every discussion to find the one containing `comment.id`. This adds an O(N) scan plus potentially multiple paginated API calls each time the persistent review is updated as a thread.
### Issue Context
This is invoked from `GitProvider.publish_persistent_comment_full()` after editing the existing persistent review comment when `as_thread=True`.
### Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[546-558]
- pr_agent/git_providers/git_provider.py[363-387]
### Implementation notes
- Prefer a direct lookup path when possible (e.g., if the note object exposes a `discussion_id`/`discussion` reference, use `mr.discussions.get(...)` rather than listing all).
- If direct lookup is not possible, consider limiting the scan (stop early, avoid `get_all=True` if the API/library can search by note id, or cache the discussion id when creating the thread in the same run).
- Keep the existing soft-fail behavior (don’t raise from reopen attempts).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. publish_comment interface mismatch 🐞 Bug ⚙ Maintainability

Description

GitProvider.publish_persistent_comment(_full) forwards as_thread into publish_comment via
**kwargs even though the abstract publish_comment contract doesn't accept it, so calling persistent
publishing with as_thread=True on non-GitLab providers can raise TypeError at runtime. The
PRReviewer path avoids this today, but the base API is now easy to misuse and inconsistent across
providers.

Code

pr_agent/git_providers/git_provider.py[R349-356]

def publish_persistent_comment(self, pr_comment: str,
                        initial_header: str,
                        update_header: bool = True,
                        name='review',
-                                   final_update_message=True):
-        return self.publish_comment(pr_comment)
+                                   final_update_message=True,
+                                   as_thread: bool = False):
+        return self.publish_comment(pr_comment, **({'as_thread': True} if as_thread else {}))

Evidence

The base class declares publish_comment(self, pr_comment, is_temporary=False) but now forwards
as_thread via kwargs from publish_persistent_comment. A representative non-GitLab provider
(GitHubProvider) still defines publish_comment without as_thread, so it would raise if that
kwarg is ever passed to it.

pr_agent/git_providers/git_provider.py[338-388]
pr_agent/git_providers/github_provider.py[471-492]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The base `GitProvider.publish_persistent_comment()` and `publish_persistent_comment_full()` conditionally call `self.publish_comment(..., as_thread=True)`, but many providers (e.g., GitHub) implement `publish_comment(pr_comment, is_temporary=False)` without an `as_thread` parameter. If any caller passes `as_thread=True` against such a provider, it will raise `TypeError`.
### Issue Context
Current reviewer code only passes `as_thread` when `should_publish_review_as_thread()` is true (GitLab), but the base provider API now exposes `as_thread` and forwards it in a way that can break other providers.
### Fix Focus Areas
- pr_agent/git_providers/git_provider.py[338-388]
- pr_agent/git_providers/github_provider.py[471-492]
### Implementation notes
Choose one:
1) **Backwards-compatible forwarding**: In the base methods, attempt `publish_comment(..., as_thread=True)` and on `TypeError` fall back to `publish_comment(pr_comment)`.
2) **Normalize the interface**: Update the abstract `publish_comment` signature to accept `as_thread: bool = False` (or `**kwargs`) and update providers to ignore unsupported args.
Prefer (1) for minimal churn.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Single quotes in new code ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

New/modified Python code uses single-quoted string literals (e.g., {'as_thread': True}), which
diverges from the repository compliance requirement to prefer double quotes. This increases
stylistic inconsistency across the modified modules.

Code

pr_agent/tools/pr_reviewer.py[R184-186]

+            # Providers that support it (GitLab) can post the review's final comment as a resolvable thread.
+            # This intent applies to the review only - never to status comments or the output of other tools.
+            review_thread_kwargs = {'as_thread': True} if self.git_provider.should_publish_review_as_thread() else {}

Evidence

The lint/format compliance item explicitly calls for preferring double quotes in new/modified Python
code. The cited lines introduce new single-quoted string literals in the threaded-review
implementation.

...

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit dfba8fb

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit bf358a9

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 54e7c85

[aivarsliepa]

#2545 (comment)

1: this was fixed

3: Intentional: these handlers keep publishing degradable (fall back to a note / skip the reopen) instead of failing the review, and match the codebase convention for provider I/O. Narrowing would turn unanticipated errors (e.g. unwrapped network exceptions) into unpublished reviews, and callers catch everything anyway - nothing would surface louder.

6: as_thread is only forwarded when should_publish_review_as_thread() is true, which only GitLab overrides. The conditional kwarg keeps other providers' signatures untouched, matching how provider specific params are handled elsewhere (thread_context, disable_fallback). Option 1 would mask genuine TypeError from inside publish_comment and risk double posting.

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 3a1aa7b

[IsmaelMartinez]

Went through this against main; the approach looks sound to me.

unresolve_comment_thread only pays for the discussions.list(get_all=True) scan when the note reports itself resolved, and a missing resolvable/resolved attribute falls through to the scan rather than silently skipping the unresolve — the safe direction. And since get_issue_comments is mr.notes.list(), notes created inside a discussion are still matched by the persistent-comment header lookup, so the persistent_comment=true path keeps working.

One scoping note for whoever reviews: this covers the review summary comment only, so the inline half of #2534 (and #2453) stays open. The cross-run fingerprints merged in #2424 are the primitive for that side, if someone picks it up.

[IsmaelMartinez]

@ColtHands asked me to take a closer look (#2534). Design and tests hold up: the kwargs injection keeps providers without as_thread untouched, the "updated to latest commit" status message correctly stays a plain note, and there is a test for an unresolve failure not falling through to the duplicating fallback.

One behaviour worth documenting. Turning publish_review_as_thread on for an MR that already has a persistent review leaves that review a plain note forever: publish_persistent_comment_full matches the existing comment by header and edits it in place, and a note cannot be promoted to a discussion. Only MRs whose first review runs after the flag is set get a thread.

Converting an existing one would mean deleting and re-posting, and GitLab refuses to delete a note that has replies (the failure mode #2404 addresses), so a documented caveat looks better than a migration path here.

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit f14b01f

[aivarsliepa]

@IsmaelMartinez thanks for review!
Added docs as per your suggestion (f14b01f)

[IsmaelMartinez]

Docs read right, and that's exactly the caveat I meant. LGTM. Full unit suite passes on the branch (1592).

@naorpeled this is ready from my side.

[naorpeled]

Amazing work @aivarsliepa !!!
Thanks for this!

Also @IsmaelMartinez thanks for joining in the review process ❤️