Trim implementation-detail docstrings Qodo flagged on PR #1068 by ptr727 · Pull Request #1069 · ptr727/ProjectTemplate

ptr727 · 2026-08-28T23:06:55Z

Follow-up to #1067, prompted by Qodo's round on the develop -> main promotion PR (#1068).

Qodo flagged three real style issues in content #1067 introduced:

marker_blocks' docstring recorded implementation history/mechanics rather than its return contract.
Two new test classes carried one-line class-summary docstrings, which comment-and-doc-style forbids regardless of length.
A test docstring used historical "previously read" framing instead of a present-tense invariant.

local-strict-review against this diff (dispatched before this push) caught three more real issues in my own first pass at fixing those three: an inaccurate mechanism claim in a rewritten docstring (said "word-boundaried", the actual mechanism is the required glyph), an inverted rationale in a new inline comment, and a dangling docstring cross-reference this diff itself created. All fixed, all covered by the existing 306-test suite passing unchanged.

All local gates pass: ruff check/format, mypy, the full scripts/tests suite, prose_lint.py --diff, repo_gate.py --check eol.

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Clarified documentation for shared review-comment block extraction and blockquote handling.
- Simplified documentation for suppressed review content.
Tests
- Refined test descriptions for review findings, including requirements for resolved-status indicators.

Three of Qodo's findings on PR #1068's re-review of the develop diff were real, about content #1067 introduced: - marker_blocks' docstring recorded PR #1067's own shape history and its line-scanning implementation rather than its return contract. Trimmed, the essential reasoning kept as a one-line inline comment. - TestCodeRabbitOutsideDiff and TestQodoOpenFindings still carried one-line class-summary docstrings, which comment-and-doc-style forbids regardless of length. Removed, matching most other test classes in this file. - A test docstring used historical 'previously read' change-framing instead of a present-tense invariant. Rewritten. local-strict-review against this diff (dispatched before this push) caught three more real issues in my own first pass at these same trims, now fixed too: - The rewritten badge test docstring claimed 'word-boundaried' matching, but QODO_BADGE requires the check-mark/cross glyph, not a word boundary. Corrected to name the actual mechanism. - A new inline comment gave an inverted reason for scanning line by line ('the heading can sit nested inside a summary'), when the actual reason is the opposite: the heading sits in a wrapper's body, not its summary, which is exactly why reading only the summary misses it. - suppressed_blocks' docstring still pointed readers to marker_blocks for 'the shape history behind it' after that history was trimmed out, a dangling reference this diff itself created. Removed the dangling clause. Two more of Qodo's findings, and both of CodeRabbit's findings on this same round, target .agents/skills/drive-pr and .agents/skills/local-strict-review, carried content from #1065 rather than #1067. Left for separate disposition: CodeRabbit's two are declined in the promotion PR thread (local-strict-review's own 'Disposing of Findings' section already requires disposition before a PR opens, contrary to the finding's claim; making that a hard publish-blocking gate would reverse a deliberate, documented design choice), and Qodo's AGENTS.md-duplication finding is deferred to a filed issue rather than redesigned mid-review.

qodo-code-review · 2026-08-28T23:07:26Z

PR Summary by Qodo

Trim PR review docstrings to match prose style rules

📝 Documentation🕐 Less than 5 minutes

AI Description

• Refocuses parser docstrings on contracts and keeps essential rationale beside the relevant scan.
• Removes forbidden test-class summaries and states the badge regression invariant in present tense.

High-Level Assessment

The current approach is optimal: retain contract-focused function docstrings, place the one necessary mechanism rationale beside the scan, and omit redundant test-class summaries. Moving this context into external documentation would separate it from the code it explains without improving maintainability.

Files changed (2) +10 / -28

Documentation (2) +10 / -28

pr_review.pyRefocus marker-block documentation on its contract+7/-22

Refocus marker-block documentation on its contract

• Removes implementation history and low-level mechanics from 'marker_blocks' documentation. Keeps the blockquote return guarantee in the docstring, preserves the wrapper-body scanning rationale inline, and repairs the shortened cross-reference from 'suppressed_blocks'.

scripts/pr_review.py

test_pr_review.pyAlign review parser tests with docstring style rules+3/-6

Align review parser tests with docstring style rules

• Removes prohibited one-line summary docstrings from two test classes. Rewrites the Qodo badge regression docstring as a present-tense invariant that accurately identifies the required glyph as the matching safeguard.

scripts/tests/test_pr_review.py

coderabbitai · 2026-08-28T23:08:02Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 729ee1ab-6b5c-4769-9162-c8e222616f96

📥 Commits

Reviewing files that changed from the base of the PR and between 6bfc8cd and 1501336.

📒 Files selected for processing (1)

scripts/tests/test_pr_review.py

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The PR clarifies shared marker-block parsing documentation and updates test documentation for resolved-badge detection. Runtime behavior remains unchanged.

Changes

Documentation clarifications

Layer / File(s)	Summary
Parser and test documentation `scripts/pr_review.py`, `scripts/tests/test_pr_review.py`	The parser documentation describes shared extraction, heading-only blockquote stripping, and line-by-line scanning. Test documentation clarifies glyph-based resolved-badge detection and reformats existing docstrings.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk:⚪ Minimal · up to 15013

This PR trims and corrects documentation comments without changing product behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: trimming implementation-detail docstrings flagged by Qodo in PR `#1068`.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-pr1068-review-findings

_{Comment @coderabbitai help to get the list of available commands.}

qodo-code-review · 2026-08-28T23:08:40Z

Code Review by Qodo

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

1. ~~Test invariant exceeds prose cap~~✓ Resolved🐞 Bug⚙ Maintainability

Description

The rewritten test invariant is a 29-word sentence, exceeding the repository's 25-word maximum. Its
multiple nested qualifications weaken the readability this docstring-only cleanup is intended to
improve.

Code

scripts/tests/test_pr_review.py[R1119-1121]

+ """The required glyph, not the bare word, keeps this script's own `isResolved`+ identifier, quoted in a finding's title, from being read as the badge and closing an+ open finding on its title."""

Relevance

●● Moderate
Style concerns are plausible, but history shows mixed treatment of prose wrapping and length rules.
PR-#959
PR-#991
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed branch text forms one 29-word sentence, and the repository's style skill explicitly
limits newly authored prose sentences to 25 words.
scripts/tests/test_pr_review.py[1118-1125]
.agents/skills/comment-and-doc-style/SKILL.md[103-114]

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 rewritten test docstring contains a 29-word sentence, above the repository's 25-word prose cap.
## Issue Context
Preserve the present-tense invariant that `QODO_BADGE` requires the glyph, so a title containing only `isResolved` remains open. Split or shorten the explanation without restoring historical framing.
## Fix Focus Areas
- scripts/tests/test_pr_review.py[1119-1121]

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

2. marker_blocks documents poll mechanics✓ Resolved📜 Skill insight✧ Quality

Description

The revised marker_blocks docstring still explains reviewThreads and thread-poll mechanics
instead of limiting itself to the function's return contract. This leaves implementation rationale
in a docstring that the PR specifically rewrites.

Code

scripts/pr_review.py[R1489-1491]

+ (CodeRabbit's outside-diff-range findings): both are a review body collapsing real findings+ into a block that raises no `reviewThreads` entry, so a thread poll alone reports a clean+ pass over either.

Relevance

●●● Strong
Directly matches the PR’s stated docstring-cleanup intent; implementation-mechanics rationale is
explicitly being removed.
PR-#991
PR-#982
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2827096 requires docstrings to describe behavior contracts rather than internal
implementation details. The added paragraph explains that blocks raise no reviewThreads entry and
how a thread poll reports them, which is operational rationale rather than the returned block
semantics.

scripts/pr_review.py[1489-1491]
Skill: python-codestyle

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 `marker_blocks` docstring describes review-thread polling mechanics rather than only behavior callers can rely on.
## Issue Context
The function's opening sentence and `strip_blockquote` statement already express its return contract and argument behavior. Move any necessary rationale to a concise inline comment or remove it.
## Fix Focus Areas
- scripts/pr_review.py[1489-1491]

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

3. ~~Wrapper comment is overlong~~✓ Resolved📜 Skill insight⚙ Maintainability

Description

The new marker_blocks inline rationale is a 34-word, multi-clause sentence that exceeds the
repository's 25-word maximum and short-comment requirement. Its repeated explanation of the wrapper
summary/body distinction makes the loop harder to scan and leaves this style-focused change
inconsistent with the project's comment-writing rules.

Code

scripts/pr_review.py[1504]

+ # Scanned line by line rather than read from a wrapper's own summary, since the heading can sit in the wrapper's body instead of its summary, where reading only the summary would miss it entirely.

Relevance

● Weak
Recent same-file precedent rejected a style-only request to condense an explanatory comment block.
PR-#991
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The branch adds a 34-word code comment, while compliance rule 2826677 applies to code comments, caps
newly authored prose sentences at 25 words, and requires concise, single-line comments by default.
The comment repeats the wrapper summary/body distinction across several clauses and states the
missed-heading outcome twice, demonstrating that it is longer and less direct than required.
scripts/pr_review.py[1504-1504]
scripts/pr_review.py[1501-1506]
.agents/skills/comment-and-doc-style/SKILL.md[103-114]
.agents/skills/comment-and-doc-style/SKILL.md[121-134]
Skill: comment-and-doc-style

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 `marker_blocks` inline rationale is a 34-word sentence, exceeding the repository's 25-word prose cap and the requirement for concise comments.
## Issue Context
Preserve the non-obvious rationale that scanning must include a wrapper body because a heading can occur there rather than in its summary, but express it more directly on one concise line or in short sentences.
## Fix Focus Areas
- scripts/pr_review.py[1504-1504]

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

Context sources

✅ Compliance rules (platform): 71 rules

✅ Skills: 5 invoked

comment-and-doc-style

dotnet-codestyle

python-codestyle

shell-codestyle

workflow-ci-contract

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Copilot

🟢 Approval recommended

The changes are documentation/comment scope adjustments that align with the repo’s docstring/comment contract focus and don’t alter runtime behavior.

Pull request overview

This PR follows up on Qodo-reported style issues by trimming/reshaping docstrings and test documentation so they describe stable behavior contracts rather than implementation history, and by removing disallowed one-line class docstrings in tests.

Changes:

Shorten marker_blocks’ docstring to focus on the behavioral contract and keep blockquote-stripping scope explicitly documented.
Remove one-line class-summary docstrings from two test classes.
Rephrase a test docstring into present-tense, contract-focused wording.

File summaries

File	Description
scripts/pr_review.py	Trims the `marker_blocks` docstring to contract-focused content and keeps the key `strip_blockquote` behavior documented.
scripts/tests/test_pr_review.py	Removes one-line class docstrings and updates a test docstring to present-tense, behavior-focused phrasing.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Qodo's round-2 findings on PR #1069 itself: - marker_blocks' docstring still explained the reviewThreads blind spot after two prior trims. Removed the rationale sentence entirely; suppressed_blocks and outside_diff_blocks each already carry their own copy of it, so nothing is lost. - The rewritten badge test's invariant sentence ran 29 (Qodo's count) to 32 (mine) words, over comment-and-doc-style's 25-word cap (ASD-STE100, prose_lint.py --check sentence-length). Split into two sentences under the cap.

Qodo caught this in the same round as the marker_blocks/prose-cap findings already fixed in d0ceaca, but on a comment that commit did not touch: 35 words on one line, over the 25-word cap. Split into two short sentences, same information, one per line per the file's own comment-wrap convention.

ptr727 · 2026-08-28T23:11:43Z

Answering Qodo's "Wrapper comment is overlong" finding (comment-only, no thread): fixed in 6bfc8cd. The 35-word inline comment near marker_blocks' scan loop is split into two sentences, one per line, both under the 25-word cap. Same information kept.

Copilot

🟡 Changes recommended

Newly added documentation text introduces comment/docstring style inconsistencies that should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 2
Review effort level: Lite

Copilot

🟢 Approval recommended

The changes are limited to docstrings/comments in the reviewed files and do not alter runtime behavior, with no correctness or contract issues introduced.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot's finding on one instance (test_pr_review.py, the isResolved-badge test docstring rewritten twice already this round) is real and matches the established convention every pre-existing multi-line docstring in this file already follows: closing """ on its own line, text never trailing on the same line as the last sentence. Swept the same fix across nine more instances this task's own earlier commits introduced with the same defect, rather than fixing only the one flagged.

Copilot

🟢 Approval recommended

The changes are documentation/comment-only, match the stated intent, and do not introduce behavioral or test-logic modifications.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

Follow-up to #1067/#1069, prompted by Copilot's round on the develop -> main promotion PR (#1068). Copilot flagged two real docstring/comment inaccuracies: - The `status` docstring's `cr_outside_diff` paragraph claimed it prints only once CodeRabbit has raised a finding, but `digest()` also prints it (as `cr_outside_diff=0+`) once the reviews window is truncated, regardless of what's currently visible. - `TestOtherReviewers`' docstring claimed "identity and commit only", contradicting its own rate-limit-marker tests, which read comment/review body content. `local-strict-review` against this diff (dispatched before this push) caught three follow-on issues in that same rewrite (a garden-path modifier, a reword that read as the opposite of what it meant, and a comment trim from the same review round that had dropped load-bearing rationale), all fixed. All covered by the existing 306-test suite passing unchanged. All local gates pass: `ruff check`/`format`, `mypy`, the full `scripts/tests` suite, `prose_lint.py --diff`, `repo_gate.py --check eol`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Bug Fixes** * CodeRabbit findings are now reported when the review window is truncated, even if no visible findings were detected. * Truncated results display the count as `0+` when applicable, making incomplete review results clearer. * **Documentation** * Clarified Qodo parser behavior and automated review-bot coverage. * Documented which review checks apply to different automated reviewers without changing parsing behavior.

CopilotAI lite review requested due to automatic review settings August 28, 2026 23:06

Copilotstarted reviewing on behalf of ptr727 August 28, 2026 23:07 View session

qodo-code-reviewBot reviewed Aug 28, 2026
View reviewed changes

Comment threadscripts/pr_review.py Outdated
Comment threadscripts/tests/test_pr_review.py Outdated

CopilotAI reviewed Aug 28, 2026
View reviewed changes

CopilotAI review requested due to automatic review settings August 28, 2026 23:10

Copilotstarted reviewing on behalf of ptr727 August 28, 2026 23:10 View session

CopilotAI reviewed Aug 28, 2026
View reviewed changes

Comment threadscripts/pr_review.py Outdated
Comment threadscripts/tests/test_pr_review.py Outdated

CopilotAI review requested due to automatic review settings August 28, 2026 23:14

Copilotstarted reviewing on behalf of ptr727 August 28, 2026 23:15 View session

CopilotAI reviewed Aug 28, 2026
View reviewed changes

CopilotAI review requested due to automatic review settings August 28, 2026 23:22

Copilotstarted reviewing on behalf of ptr727 August 28, 2026 23:22 View session

CopilotAI reviewed Aug 28, 2026
View reviewed changes

ptr727 merged commit 76c87cf into developAug 29, 2026
9 checks passed

ptr727 deleted the fix-pr1068-review-findings branch August 29, 2026 00:06

ptr727 mentioned this pull request Aug 29, 2026
Fix docstring drift Copilot caught on PR #1068 #1072
Merged

Navigation Menu

Trim implementation-detail docstrings Qodo flagged on PR #1068 - #1069

Trim implementation-detail docstrings Qodo flagged on PR #1068#1069
ptr727 merged 4 commits into
developfrom
fix-pr1068-review-findings

ptr727 commented Aug 28, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

Uh oh!

coderabbitaiBot commented Aug 28, 2026•
edited
Loading

Walkthrough

Changes

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026•
edited
Loading

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Uh oh!

ptr727 commented Aug 28, 2026

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ptr727 commented Aug 28, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

PR Summary by Qodo

Uh oh!

coderabbitaiBot commented Aug 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

ptr727 commented Aug 28, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ptr727 commented Aug 28, 2026•
edited by coderabbitai Bot
Loading

coderabbitaiBot commented Aug 28, 2026•
edited
Loading

qodo-code-reviewBot commented Aug 28, 2026•
edited
Loading