Skip to content

fix(suggestions): remove stale progress comments - #2648

Merged
IsmaelMartinez merged 2 commits into
The-PR-Agent:mainfrom
TLA020:fix/remove-progress-comment-no-suggestions
Aug 21, 2026
Merged

fix(suggestions): remove stale progress comments#2648
IsmaelMartinez merged 2 commits into
The-PR-Agent:mainfrom
TLA020:fix/remove-progress-comment-no-suggestions

Conversation

@TLA020

@TLA020 TLA020 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes the /improve progress placeholder when a run produces no suggestions and quiet output is enabled.

Scope

All providers. The cleanup uses the exact temporary comment returned by the current run. It does not perform broad cleanup when no handle is available, so unrelated temporary comments are left untouched.

Default behavior remains unchanged when publish_output_no_suggestions is enabled: the placeholder still becomes the no-suggestions result.

Validation

  • 14 focused suggestion tests pass.
  • 1,874 unit tests pass, with 1 skipped and 1 expected failure.
  • Ruff passes on the focused test file.
  • git diff --check passes.

@github-actions github-actions Bot added the bug label Aug 13, 2026
@qodo-code-review

qodo-code-review Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Test docstring exceeds line limit ✓ Resolved 📘 Rule violation ⚙ Maintainability ⭐ New
Description
The added docstring on line 199 exceeds the repository's configured 120-character Ruff maximum for
Python source lines. This leaves the changed test file noncompliant with the line-length
requirement.
Code

tests/unittest/test_pr_code_suggestions_core.py[R199-200]

+    """The progress placeholder is only overwritten on the paths that publish something, so a run that stays
+    quiet must remove it explicitly instead of leaving it behind saying work is in progress."""
Evidence
The checklist requires every logical line in changed Python files, including comments and
docstrings, to be at most 120 characters. The added explanatory docstring spans lines 199-200, with
its first physical line exceeding that limit.

Rule 2694655: Enforce 120-character maximum line length in Python source per Ruff config
tests/unittest/test_pr_code_suggestions_core.py[199-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
The newly added test docstring exceeds the configured 120-character Python line limit.

## Issue Context
Ruff is configured with a 120-character maximum, and this finding is limited to the newly added test lines.

## Fix Focus Areas
- tests/unittest/test_pr_code_suggestions_core.py[199-200]

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


2. Quiet cleanup removes unrelated comments ✓ Resolved 🐞 Bug ≡ Correctness
Description
When this run has no progress_response, the new quiet-path cleanup invokes
remove_initial_comment(), which providers implement by deleting every tracked temporary comment
rather than only this run's placeholder. An overlapping run or another tool using temporary comments
can therefore lose its in-progress status comment.
Code

pr_agent/tools/pr_code_suggestions.py[R237-238]

+            # The progress placeholder is part of the mechanics rather than the output. Clean it up here too, or a
+            # quiet run leaves it behind saying work is in progress.
Evidence
The added quiet-path block reaches remove_initial_comment() when progress_response is absent.
The unsupported-GFM branch creates a temporary comment without storing its return value, and
provider implementations perform broad temporary-comment deletion.

pr_agent/tools/pr_code_suggestions.py[117-118]
pr_agent/tools/pr_code_suggestions.py[237-242]
pr_agent/git_providers/bitbucket_provider.py[426-444]
pr_agent/git_providers/github_provider.py[850-862]

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 quiet no-suggestions path falls back to `remove_initial_comment()`, but provider implementations remove all temporary comments they track. This can delete another operation's progress comment.

## Issue Context
The unsupported-GFM branch creates a temporary comment without assigning it to `self.progress_response`; the new cleanup therefore uses the broad fallback. Provider implementations such as Bitbucket's iterate all tracked temporary comments, while GitHub removes all PR comments marked temporary.

## Fix Focus Areas
- pr_agent/tools/pr_code_suggestions.py[237-242]
- pr_agent/git_providers/bitbucket_provider.py[426-444]
- pr_agent/git_providers/github_provider.py[850-862]

Retain the current run's temporary comment handle/identifier and remove only that comment, or add a provider API for scoped temporary-comment cleanup.

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


3. Progress cleanup comment exceeds limit ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The added comment on line 237 exceeds the repository's configured 120-character Ruff limit, causing
the modified Python source to violate the line-length requirement.
Code

pr_agent/tools/pr_code_suggestions.py[237]

+            # the progress placeholder is part of the mechanics rather than part of the output, so it is cleaned up
Evidence
The changed source contains a long comment at pr_agent/tools/pr_code_suggestions.py:237; the
repository configuration sets Ruff's maximum line length to 120.

Rule 2694655: Enforce 120-character maximum line length in Python source per Ruff config
pr_agent/tools/pr_code_suggestions.py[237-237]

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 added explanatory comment exceeds the configured 120-character Python line limit.

## Issue Context
Ruff is configured with `line-length = 120`; keep the explanation while splitting it across multiple comment lines.

## Fix Focus Areas
- pr_agent/tools/pr_code_suggestions.py[237-237]

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


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

Grey Divider

Context sources
✅ Compliance rules (platform): 34 rules
Review mode: 🚀 Fast: This is a localized cleanup fix in one execution path with focused tests, avoiding security, API, migration, concurrency, and other high-risk areas.

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit d1b2717

Results up to commit 472b3ad 🚀 Fast


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)


Remediation recommended
1. Progress cleanup comment exceeds limit ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The added comment on line 237 exceeds the repository's configured 120-character Ruff limit, causing
the modified Python source to violate the line-length requirement.
Code

pr_agent/tools/pr_code_suggestions.py[237]

+            # the progress placeholder is part of the mechanics rather than part of the output, so it is cleaned up
Evidence
The changed source contains a long comment at pr_agent/tools/pr_code_suggestions.py:237; the
repository configuration sets Ruff's maximum line length to 120.

Rule 2694655: Enforce 120-character maximum line length in Python source per Ruff config
pr_agent/tools/pr_code_suggestions.py[237-237]

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 added explanatory comment exceeds the configured 120-character Python line limit.

## Issue Context
Ruff is configured with `line-length = 120`; keep the explanation while splitting it across multiple comment lines.

## Fix Focus Areas
- pr_agent/tools/pr_code_suggestions.py[237-237]

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


Results up to commit ce9d7e2 🚀 Fast


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)


Remediation recommended
1. Quiet cleanup removes unrelated comments ✓ Resolved 🐞 Bug ≡ Correctness
Description
When this run has no progress_response, the new quiet-path cleanup invokes
remove_initial_comment(), which providers implement by deleting every tracked temporary comment
rather than only this run's placeholder. An overlapping run or another tool using temporary comments
can therefore lose its in-progress status comment.
Code

pr_agent/tools/pr_code_suggestions.py[R237-238]

+            # The progress placeholder is part of the mechanics rather than the output. Clean it up here too, or a
+            # quiet run leaves it behind saying work is in progress.
Evidence
The added quiet-path block reaches remove_initial_comment() when progress_response is absent.
The unsupported-GFM branch creates a temporary comment without storing its return value, and
provider implementations perform broad temporary-comment deletion.

pr_agent/tools/pr_code_suggestions.py[117-118]
pr_agent/tools/pr_code_suggestions.py[237-242]
pr_agent/git_providers/bitbucket_provider.py[426-444]
pr_agent/git_providers/github_provider.py[850-862]

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 quiet no-suggestions path falls back to `remove_initial_comment()`, but provider implementations remove all temporary comments they track. This can delete another operation's progress comment.

## Issue Context
The unsupported-GFM branch creates a temporary comment without assigning it to `self.progress_response`; the new cleanup therefore uses the broad fallback. Provider implementations such as Bitbucket's iterate all tracked temporary comments, while GitHub removes all PR comments marked temporary.

## Fix Focus Areas
- pr_agent/tools/pr_code_suggestions.py[237-242]
- pr_agent/git_providers/bitbucket_provider.py[426-444]
- pr_agent/git_providers/github_provider.py[850-862]

Retain the current run's temporary comment handle/identifier and remove only that comment, or add a provider API for scoped temporary-comment cleanup.

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


Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix stale progress comment when /improve finds no suggestions

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove the “Work in progress…” placeholder on quiet no-suggestions runs
• Preserve existing behavior when “no suggestions” output is published
• Add unit tests covering both progress and temporary comment cleanup paths
Diagram

graph TD
  U["Reviewer runs /improve"] --> T["PRCodeSuggestions"] --> D{"Publish no-suggestions output?"}
  T --> S["Settings"] --> D
  D -->|"Yes"| E["Edit/publish comment"] --> C[("PR comments")]
  D -->|"No"| R["Remove placeholder"] --> C
  T --> P["GitProvider API"] --> E
  P --> R
  subgraph Legend
    direction LR
    _proc["Process"] ~~~ _dec{"Decision"} ~~~ _db[("Comments storage")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize placeholder cleanup in a finally/guard block
  • ➕ Reduces risk of future early-return paths skipping cleanup
  • ➕ Keeps comment lifecycle responsibilities in one place
  • ➖ Slight refactor risk if multiple publish paths rely on nuanced behavior
  • ➖ May require re-auditing other publishing branches for ordering semantics
2. Always create a durable progress comment ID and standardize removal via one method
  • ➕ Eliminates dual-mode handling (progress_response vs initial comment)
  • ➕ Simplifies tests and provider behavior expectations
  • ➖ Broader change across providers/callers
  • ➖ Not necessary for this narrowly scoped bug

Recommendation: The PR’s approach is the right minimal fix: explicitly remove the placeholder on the quiet no-suggestions path while preserving existing published-output behavior. Consider a later small refactor to centralize cleanup (e.g., via a finally block) to prevent similar regressions from new early returns.

Files changed (2) +61 / -0

Bug fix (1) +6 / -0
pr_code_suggestions.pyExplicitly remove placeholder comment on quiet no-suggestions runs +6/-0

Explicitly remove placeholder comment on quiet no-suggestions runs

• When output publishing is disabled for no-suggestions cases, the tool now cleans up the progress placeholder instead of leaving a stale “Work in progress…” comment. It removes the progress comment if available, otherwise removes the initial temporary comment.

pr_agent/tools/pr_code_suggestions.py

Tests (1) +55 / -0
test_pr_code_suggestions_core.pyAdd tests for placeholder cleanup in no-suggestions scenarios +55/-0

Add tests for placeholder cleanup in no-suggestions scenarios

• Adds a fixture to toggle publish_output_no_suggestions and new async tests verifying that quiet no-suggestions runs remove either the progress comment or initial temporary comment and publish nothing. Also asserts that the default publishing behavior still overwrites the progress comment with the no-suggestions message.

tests/unittest/test_pr_code_suggestions_core.py

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 17a371d

@TLA020
TLA020 force-pushed the fix/remove-progress-comment-no-suggestions branch from 17a371d to 4794ca9 Compare August 13, 2026 15:43
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 4794ca9

@TLA020
TLA020 force-pushed the fix/remove-progress-comment-no-suggestions branch from 4794ca9 to f076920 Compare August 14, 2026 17:02
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@TLA020
TLA020 force-pushed the fix/remove-progress-comment-no-suggestions branch from f076920 to 6b62dff Compare August 17, 2026 18:21
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 6b62dff

@TLA020 TLA020 changed the title fix(suggestions): remove the progress comment when a run finds nothing fix(suggestions): remove stale progress comments Aug 17, 2026
@qodo-code-review

qodo-code-review Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving and merging. Good catch that the non-GFM branch never stored progress_response, so the temporary "Preparing suggestions..." comment could never be removed afterwards.

Verified: full suite green at 1,874, and reverting pr_code_suggestions.py while keeping your tests fails 2 of the 14.

One heads-up: #2649 touches the same test file and will conflict once this lands. Could you rebase it on main?

@IsmaelMartinez
IsmaelMartinez merged commit 91d94c2 into The-PR-Agent:main Aug 21, 2026
5 checks passed
@TLA020

TLA020 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Approving and merging. Good catch that the non-GFM branch never stored progress_response, so the temporary "Preparing suggestions..." comment could never be removed afterwards.

Verified: full suite green at 1,874, and reverting pr_code_suggestions.py while keeping your tests fails 2 of the 14.

One heads-up: #2649 touches the same test file and will conflict once this lands. Could you rebase it on main?

Thanks i will :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants