Skip to content

feat(review): make the review heading configurable - #2797

Merged
IsmaelMartinez merged 4 commits into
The-PR-Agent:mainfrom
oleksii-tumanov:feature/configurable-review-heading
Aug 26, 2026
Merged

feat(review): make the review heading configurable#2797
IsmaelMartinez merged 4 commits into
The-PR-Agent:mainfrom
oleksii-tumanov:feature/configurable-review-heading

Conversation

@oleksii-tumanov

@oleksii-tumanov oleksii-tumanov commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add pr_reviewer.review_heading while preserving the current visible heading by default.
  • Separate presentation from stable hidden full and incremental review identities.
  • Migrate legacy default-heading reviews in place and keep incremental review discovery compatible across GitHub, GitLab, and Azure DevOps, with persistent update parity for Bitbucket Cloud.

Compatibility

  • GitHub check runs and non-comment artifacts remain presentation-only.
  • Gitea stays on its existing legacy persistence path until feat(review): preserve findings across review reruns #2722 normalizes its comment payloads.
  • One stable marker represents one persistent /review channel; changing the heading does not create multiple named review channels.

Output example

review_heading = "Guideline Compliance Check" renders:

  • ## Guideline Compliance Check 🔍
  • ## Incremental Guideline Compliance Check 🔍

Tests

  • PYTHONPATH=. pytest -q tests/unittest — 2153 passed, 1 skipped, 1 xfailed
  • Focused review identity, reviewer, GitLab, Bitbucket, and Azure suite — 256 passed

Addresses the /review portion of #2038.

@github-actions github-actions Bot added the feature 💡 label Aug 25, 2026
Comment thread pr_agent/git_providers/azuredevops_provider.py Fixed
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Make PR review heading configurable with stable comment identities

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

Grey Divider

AI Description

• Add pr_reviewer.review_heading to customize visible full/incremental review titles.
• Introduce hidden review identity markers to keep persistent updates stable across heading changes.
• Update Git providers and tests to migrate legacy heading-based reviews safely.
Diagram

graph TD
  A["Settings: pr_reviewer.review_heading"] --> B["PRReviewer tool"] --> C["Markdown formatter (utils)"] --> D["Hidden identity marker"] --> E["GitProvider persistent publish"] --> F{{"VCS PR comments"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Persist provider-specific comment IDs
  • ➕ More robust than content matching; avoids any header/body parsing edge cases
  • ➖ Requires durable storage across runs/installations (DB/kv-store) or provider-specific metadata
  • ➖ Harder to support uniformly across self-hosted providers and auth modes
2. Use a structured marker block (e.g., JSON) instead of a single-line identity
  • ➕ Can encode versioning and additional metadata (channel, tool version, run id)
  • ➖ More verbose; higher risk of rendering artifacts or being edited by users
  • ➖ Still requires careful bounded matching to avoid false positives
3. Keep heading-prefix matching only, document constraints
  • ➕ Simplest implementation; minimal changes to provider logic
  • ➖ Breaks persistent updates when headings change
  • ➖ Higher risk of collisions with human comments that share similar headings

Recommendation: The chosen approach (stable, bounded HTML comment identities with legacy-prefix fallback) is the best trade-off for portability and backward compatibility. It decouples presentation from persistence without requiring external storage, and the bounded matching logic reduces false-positive adoption risk while enabling in-place migration of legacy comments.

Files changed (15) +728 / -102

Enhancement (7) +208 / -73
utils.pyAdd review heading formatter and stable identity helpers +66/-5

Add review heading formatter and stable identity helpers

• Introduces 'PRReviewIdentity' markers and helper functions to format headings from configuration, insert identity markers into comments, and match identities safely near the comment header. Updates markdown generation to use the configurable heading while keeping identity out of presentation.

pr_agent/algo/utils.py

azuredevops_provider.pySwitch Azure previous-review lookup to identity-aware matching +20/-10

Switch Azure previous-review lookup to identity-aware matching

• Replaces heading-prefix matching with identity-based identifiers for locating prior reviews. Extends persistent publish signature to accept identity markers and declares support for review comment identities.

pr_agent/git_providers/azuredevops_provider.py

bitbucket_provider.pyMigrate Bitbucket persistent review updates to identity markers +45/-19

Migrate Bitbucket persistent review updates to identity markers

• Adds identity insertion and identity-first selection of the comment to update, with fallback to legacy heading containment for migration. Preserves existing non-review persistent matching behavior when no identity is provided.

pr_agent/git_providers/bitbucket_provider.py

gitea_provider.pyPlumb identity parameters but keep Gitea on legacy persistence path +11/-2

Plumb identity parameters but keep Gitea on legacy persistence path

• Updates the method signature to accept identity parameters while explicitly deferring identity-based persistence until Gitea comment payloads are normalized.

pr_agent/git_providers/gitea_provider.py

github_provider.pyUse identity-aware matching for previous-review discovery and persistence +20/-10

Use identity-aware matching for previous-review discovery and persistence

• Updates previous review lookup to match stable identities (with legacy fallback identifiers) and forwards identity parameters through the persistent publish path. Declares identity support while preserving the check-run publication path as presentation-only.

pr_agent/git_providers/github_provider.py

gitlab_provider.pyAdopt identity-aware anchor note discovery for incremental reviews +28/-22

Adopt identity-aware anchor note discovery for incremental reviews

• Replaces prefix-based incremental anchor selection with identity-based matching, updates helper naming/docs accordingly, and forwards identity parameters through persistent publishing. Declares identity support for GitLab.

pr_agent/git_providers/gitlab_provider.py

pr_reviewer.pyPublish reviews with stable identities and legacy migration headers +18/-5

Publish reviews with stable identities and legacy migration headers

• For persistent full reviews, passes a stable identity marker and a legacy header to enable in-place migration of existing default-heading comments. For non-persistent publishing, injects identities when the provider supports identity-aware incremental discovery.

pr_agent/tools/pr_reviewer.py

Refactor (1) +52 / -27
git_provider.pyGeneralize persistent publishing to use identity markers with legacy fallback +52/-27

Generalize persistent publishing to use identity markers with legacy fallback

• Extends the base persistent-comment flow to inject identity markers and to locate the update target via identity/legacy header matching. Adds a capability flag ('supports_review_comment_identity') for provider-specific enablement.

pr_agent/git_providers/git_provider.py

Tests (5) +462 / -2
test_azure_devops_incremental.pyTest Azure previous-review matching with custom heading + identity marker +17/-0

Test Azure previous-review matching with custom heading + identity marker

• Adds coverage ensuring Azure identifies prior full reviews via the hidden identity marker even when the visible heading differs.

tests/unittest/test_azure_devops_incremental.py

test_bitbucket_provider.pyAdd Bitbucket persistent review migration and matching tests +88/-0

Add Bitbucket persistent review migration and matching tests

• Adds tests for migrating legacy heading-based persistent reviews to identity-based updates, preferring marked comments over legacy ones, and avoiding accidental matches outside the header region.

tests/unittest/test_bitbucket_provider.py

test_gitlab_provider.pyAdd GitLab persistence migration and identity-based previous-review tests +84/-2

Add GitLab persistence migration and identity-based previous-review tests

• Covers migration from legacy headings to identity markers, preference for marked comments, avoidance of human lookalike headings, and previous-review lookup using stable identities.

tests/unittest/test_gitlab_provider.py

test_pr_reviewer_core.pyValidate PRReviewer forwards identity and legacy header in persistent mode +65/-0

Validate PRReviewer forwards identity and legacy header in persistent mode

• Updates existing tests to assert identity parameters are passed in persistent publishing, and adds a parametrized test ensuring non-persistent publishes embed the correct identity when supported.

tests/unittest/test_pr_reviewer_core.py

test_review_comment_identity.pyAdd focused unit tests for heading formatting and identity semantics +208/-0

Add focused unit tests for heading formatting and identity semantics

• Introduces a new test module validating configurable headings, invalid-heading fallback, identity insertion/idempotency, bounded identity matching behavior, legacy compatibility, and provider forwarding rules (including check-run path behavior).

tests/unittest/test_review_comment_identity.py

Documentation (1) +4 / -0
review.mdDocument configurable 'review_heading' behavior and persistence semantics +4/-0

Document configurable 'review_heading' behavior and persistence semantics

• Adds documentation for the new 'pr_reviewer.review_heading' option, including full vs incremental rendered headings and the guarantee that changing the heading updates the same persistent comment (not a new channel).

docs/docs/tools/review.md

Other (1) +2 / -0
configuration.tomlAdd default 'review_heading' configuration value +2/-0

Add default 'review_heading' configuration value

• Introduces 'review_heading = "PR Reviewer Guide"' with a clarifying comment that identity tracking is separate from visible headings.

pr_agent/settings/configuration.toml

@qodo-code-review

qodo-code-review Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. DEFAULT_REVIEW_HEADING duplicates configuration ✓ Resolved 📘 Rule violation ⚙ Maintainability ⭐ New
Description
PR Reviewer Guide is embedded in business logic as a fallback while the same configurable default
is already defined in configuration.toml. This duplicates a user-configurable runtime value
instead of keeping the settings loader as its single source of truth.
Code

pr_agent/algo/utils.py[73]

+DEFAULT_REVIEW_HEADING = "PR Reviewer Guide"
Relevance

●●● Strong

Recent accepted precedent explicitly removes duplicated Python defaults when configuration.toml is
the settings source of truth.

PR-#2598
PR-#2528

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2694652 prohibits duplicating configurable runtime values as literals when a
settings loader is available. The Python constant at utils.py:73 duplicates the default declared
for pr_reviewer.review_heading in configuration.toml:112.

Rule 2694652: Do not hard-code configuration; load it from .pr_agent.toml or pr_agent/settings
pr_agent/algo/utils.py[73-79]
pr_agent/settings/configuration.toml[111-112]

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 configurable review-heading default is duplicated as a Python literal and in `configuration.toml`.

## Issue Context
Keep `pr_reviewer.review_heading` and its fallback sourced from the settings configuration rather than embedding the same user-facing value in business logic. Preserve fallback handling for invalid overrides without duplicating the literal.

## Fix Focus Areas
- pr_agent/algo/utils.py[73-90]
- pr_agent/settings/configuration.toml[111-112]

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


2. Legacy fallback overwrites human comments ✓ Resolved 🐞 Bug ≡ Correctness
Description
When no marked comment exists, Bitbucket Cloud selects the first comment containing the legacy
heading anywhere in its body, rather than identifying it as a review header. A human comment that
quotes or discusses ## PR Reviewer Guide 🔍 can therefore be edited in place and replaced with the
generated persistent review during migration.
Code

pr_agent/git_providers/bitbucket_provider.py[R414-417]

+                if comment_to_update is None and legacy_initial_header:
+                    comment_to_update = next(
+                        (comment for comment in comments if legacy_initial_header in comment.raw),
+                        None,
Relevance

●●● Strong

Recent Bitbucket precedents accept defensive correctness fixes preventing malformed or unintended
comment selection.

PR-#2504
PR-#2791

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PRReviewer explicitly supplies the legacy default review heading for migration, and the new
Bitbucket fallback tests that value with Python substring containment over every comment. The common
identity matcher used by the other providers instead treats a legacy header as a leading prefix,
demonstrating that arbitrary in-body occurrences are not a review identity.

pr_agent/tools/pr_reviewer.py[205-212]
pr_agent/git_providers/bitbucket_provider.py[405-418]
pr_agent/algo/utils.py[95-108]

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 Bitbucket Cloud legacy-migration fallback matches `legacy_initial_header` as an unrestricted substring, so it can adopt a human comment that contains the old review heading in its content and overwrite it.

## Issue Context
The stable-marker lookup above this fallback is bounded by `comment_matches_identity`; apply equivalently constrained review-header matching to the legacy fallback while preserving migration of real legacy review comments.

## Fix Focus Areas
- pr_agent/git_providers/bitbucket_provider.py[405-418]
- pr_agent/algo/utils.py[95-108]

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


3. Long review_heading docs line ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly added review_heading documentation is a single line well over 120 characters, violating
the repository’s 120-character max line length rule. This reduces readability and makes future edits
harder.
Code

docs/docs/tools/review.md[63]

+        <td>Visible base heading for review comments, without the Markdown prefix or the incremental label. For example, <code>review_heading = "Guideline Compliance Check"</code> renders <code>## Guideline Compliance Check 🔍</code> for a full review and <code>## Incremental Guideline Compliance Check 🔍</code> for an incremental review. On GitHub, GitLab, Azure DevOps, and Bitbucket Cloud, changing this value updates the same persistent review comment; it does not create a separate review channel. Default is <code>PR Reviewer Guide</code>.</td>
Relevance

●●● Strong

Recent accepted precedents enforce 120-character wrapping and documentation readability fixes.

PR-#2776
PR-#2424

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2694690 requires keeping modified source lines at or under 120 characters. The
added <td>...review_heading...</td> line is substantially longer than 120 characters.

Rule 2694690: Enforce maximum line length of 120 characters
docs/docs/tools/review.md[61-64]

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 `review_heading` documentation row is a single very long line that exceeds the 120-character maximum line length requirement.

## Issue Context
This is in `docs/docs/tools/review.md` within an HTML table row; you can safely split the `<td>...</td>` content across multiple physical lines (HTML will collapse whitespace), or insert `<br />` where appropriate.

## Fix Focus Areas
- docs/docs/tools/review.md[61-64]

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


View medium (1)
4. Non-imperative anchor-note comment ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
A newly updated comment in gitlab_provider.py is written as descriptive narration rather than
imperative phrasing. This violates the docstring/comment style requirement and reduces consistency
across the codebase.
Code

pr_agent/git_providers/gitlab_provider.py[R431-432]

+    # Anchor-note identities per incremental "kind". An incremental run looks for the most recent
+    # prior note matching any identity and uses its timestamp as the timeline anchor.
Relevance

●●● Strong

Recent accepted precedent explicitly requests imperative phrasing for newly added comments.

PR-#2791

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2694688 requires newly modified comments to use imperative phrasing. The updated
comment starts with a descriptive noun phrase (Anchor-note identities...) rather than an
imperative verb form.

Rule 2694688: Docstrings and comments must use imperative phrasing
pr_agent/git_providers/gitlab_provider.py[431-432]

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 modified comment uses descriptive phrasing instead of imperative phrasing.

## Issue Context
Compliance requires newly added/modified comments to use imperative phrasing (e.g., “Use …”, “Match …”, “Return …”).

## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[431-432]

ⓘ 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 small, localized fallback-default change in one utility path; it has behavioral impact but no high-risk area or multiple independent logic paths.

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit ff8a7c8

Results up to commit 12b77ec 🧠 Deep


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


Remediation recommended
1. Legacy fallback overwrites human comments ✓ Resolved 🐞 Bug ≡ Correctness
Description
When no marked comment exists, Bitbucket Cloud selects the first comment containing the legacy
heading anywhere in its body, rather than identifying it as a review header. A human comment that
quotes or discusses ## PR Reviewer Guide 🔍 can therefore be edited in place and replaced with the
generated persistent review during migration.
Code

pr_agent/git_providers/bitbucket_provider.py[R414-417]

+                if comment_to_update is None and legacy_initial_header:
+                    comment_to_update = next(
+                        (comment for comment in comments if legacy_initial_header in comment.raw),
+                        None,
Relevance

●●● Strong

Recent Bitbucket precedents accept defensive correctness fixes preventing malformed or unintended
comment selection.

PR-#2504
PR-#2791

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PRReviewer explicitly supplies the legacy default review heading for migration, and the new
Bitbucket fallback tests that value with Python substring containment over every comment. The common
identity matcher used by the other providers instead treats a legacy header as a leading prefix,
demonstrating that arbitrary in-body occurrences are not a review identity.

pr_agent/tools/pr_reviewer.py[205-212]
pr_agent/git_providers/bitbucket_provider.py[405-418]
pr_agent/algo/utils.py[95-108]

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 Bitbucket Cloud legacy-migration fallback matches `legacy_initial_header` as an unrestricted substring, so it can adopt a human comment that contains the old review heading in its content and overwrite it.

## Issue Context
The stable-marker lookup above this fallback is bounded by `comment_matches_identity`; apply equivalently constrained review-header matching to the legacy fallback while preserving migration of real legacy review comments.

## Fix Focus Areas
- pr_agent/git_providers/bitbucket_provider.py[405-418]
- pr_agent/algo/utils.py[95-108]

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


2. Long review_heading docs line ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The newly added review_heading documentation is a single line well over 120 characters, violating
the repository’s 120-character max line length rule. This reduces readability and makes future edits
harder.
Code

docs/docs/tools/review.md[63]

+        <td>Visible base heading for review comments, without the Markdown prefix or the incremental label. For example, <code>review_heading = "Guideline Compliance Check"</code> renders <code>## Guideline Compliance Check 🔍</code> for a full review and <code>## Incremental Guideline Compliance Check 🔍</code> for an incremental review. On GitHub, GitLab, Azure DevOps, and Bitbucket Cloud, changing this value updates the same persistent review comment; it does not create a separate review channel. Default is <code>PR Reviewer Guide</code>.</td>
Relevance

●●● Strong

Recent accepted precedents enforce 120-character wrapping and documentation readability fixes.

PR-#2776
PR-#2424

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2694690 requires keeping modified source lines at or under 120 characters. The
added <td>...review_heading...</td> line is substantially longer than 120 characters.

Rule 2694690: Enforce maximum line length of 120 characters
docs/docs/tools/review.md[61-64]

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 `review_heading` documentation row is a single very long line that exceeds the 120-character maximum line length requirement.

## Issue Context
This is in `docs/docs/tools/review.md` within an HTML table row; you can safely split the `<td>...</td>` content across multiple physical lines (HTML will collapse whitespace), or insert `<br />` where appropriate.

## Fix Focus Areas
- docs/docs/tools/review.md[61-64]

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


3. Non-imperative anchor-note comment ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
A newly updated comment in gitlab_provider.py is written as descriptive narration rather than
imperative phrasing. This violates the docstring/comment style requirement and reduces consistency
across the codebase.
Code

pr_agent/git_providers/gitlab_provider.py[R431-432]

+    # Anchor-note identities per incremental "kind". An incremental run looks for the most recent
+    # prior note matching any identity and uses its timestamp as the timeline anchor.
Relevance

●●● Strong

Recent accepted precedent explicitly requests imperative phrasing for newly added comments.

PR-#2791

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2694688 requires newly modified comments to use imperative phrasing. The updated
comment starts with a descriptive noun phrase (Anchor-note identities...) rather than an
imperative verb form.

Rule 2694688: Docstrings and comments must use imperative phrasing
pr_agent/git_providers/gitlab_provider.py[431-432]

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 modified comment uses descriptive phrasing instead of imperative phrasing.

## Issue Context
Compliance requires newly added/modified comments to use imperative phrasing (e.g., “Use …”, “Match …”, “Return …”).

## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[431-432]

ⓘ 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

Code review by qodo was updated up to the latest commit 83fe5ec

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@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.

This is careful work: marker-first discovery with the legacy fallback is the right shape, the body discloses the full scope, and coverage holds by construction, with 235 passing on the merged tree and collection failing everywhere once the source is reverted. Two sequencing points rather than code changes: #2795 landed this morning and conflicts with this in three files, so this needs a rebase over current main. And on the #2722 overlap, I propose this lands ahead, with #2722 folding its planned rebase, already queued behind #2724, on top; I have left a note on #2722 saying the same.

@oleksii-tumanov
oleksii-tumanov force-pushed the feature/configurable-review-heading branch from b070934 to 48ce593 Compare August 25, 2026 15:15
@oleksii-tumanov

oleksii-tumanov commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Done, I rebased this onto current main.
The focused suite (256 tests) and full unit suite (2,153 tests) pass locally.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 48ce593

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@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.

Thanks for the rebase. It merges clean on today's main, 2279 pass against 2250, and reverting the source behind your tests turns 18 assertions red.

On the #2722 overlap I said this should wait. That was wrong: there is no code dependency, and the conflict set is the same whichever lands first, so waiting would have moved work onto you rather than removed it.

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.

3 participants