Skip to content

Promote Develop to Main - #991

Merged
ptr727 merged 2 commits into
mainfrom
develop
Aug 25, 2026
Merged

Promote Develop to Main#991
ptr727 merged 2 commits into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Promotes #986 (issues #985, #973) to main.

What

Review

PR #986 review loop: CodeRabbit and qodo reviewed, three findings
raised and closed (two fixed, in cbb1bbc; one declined with evidence
of pre-existing precedent in the file, resolved with authorization).
Copilot's own review account is in the fleet's known repo-wide quota-
exhausted state (confirmed live via the fixed wait itself, in 6s
rather than the ~45-minute blind poll the unfixed version would have
spent), so this proceeded on the other two reviewers' coverage per
standing precedent.

Summary by CodeRabbit

  • New Features

    • Review status now indicates when thread results are incomplete, including marked thread and unresolved counts.
    • Added guidance to use the paginated reply path when additional review threads are available.
    • Copilot activity searches can expand from the latest 20 to the latest 100 pull requests when no activity is found.
  • Bug Fixes

    • Improved handling of empty Copilot history and missing bot identifiers.
    • Added coverage for paginated threads and expanded history searches.

## What

`copilot_history` reads the Copilot reviewer's own review and comment
history from the repository's 20 most-recently-updated pull requests
(`HISTORY_PRS`), feeding both `wait`'s auto-request bot id and its
repo-wide quota signal. An outage that outlasts that window leaves
every one of those pull requests silent, so both readings fall back to
blind polling for the rest of the outage with no way to tell that
outage apart from a repository that has simply never seen a Copilot
review.

Reproduced today on PRs #981-984: the fast exit-46/47 quota detection
worked for the first two waits, then the 20-PR window emptied out and
every wait after it fell back to a full ~2700s blind poll before
reporting `PENDING`.

Fixes #985: `copilot_history` now retries once at a wider
`HISTORY_PRS_WIDE` (100, GitHub's own per-connection ceiling) whenever
the narrow window comes back with nothing at all, and only then, so
the ordinary case still costs one call. `Q_BOT_ID` takes its PR count
as a GraphQL variable instead of a baked-in literal, so the narrow and
wide reads share one query text.

Also addresses #973 in the same pass, since it sits in the same
digest/`Q_FULL` code the widen fix touches: `reviewThreads(first:100)`
carried no `hasNextPage` tracking at all, so a pull request with more
than 100 review threads silently undercounted `threads=`/`unresolved=`
with no signal anything was cut. `threads=` now prints a trailing `+`
and a `THREADS TRUNCATED` block names the gap, rather than a full
cursor-paginated read of the connection (the heavier of the two fixes
the issue suggested), since the tracked reviewers on this repository
have never come close to 100 open threads on one pull request.

## Verification

- `python3 -m unittest discover -s scripts/tests`: 846 passed (7 new,
  covering the widen retry at both the `copilot_history` and `wait`
  CLI level, and the truncation guard at both the `threads_truncated`
  and `digest` level).
- `uvx ruff@latest format --check .` / `check .`: clean.
- `uvx mypy@latest`: clean.
- `python3 scripts/prose_lint.py scripts/pr_review.py
scripts/tests/test_pr_review.py`: clean.
- `python3 scripts/repo_gate.py --check eol` / `--check sha-pin`: clean.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Review summaries now indicate when results are incomplete due to
thread limits.
* Repository-wide history checks automatically search a broader
pull-request range when recent activity is not found.
  * Status messages clearly report the search ranges being checked.

* **Bug Fixes**
* Improved detection and reporting of incomplete review-thread results.
* More reliably identifies available Copilot history across pull
requests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Improve Copilot History Fallback and Flag Truncated Threads

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Widen empty Copilot history lookbacks from 20 to 100 pull requests.
• Flag truncated review-thread counts instead of silently underreporting unresolved work.
• Add unit and wait-path coverage for fallback and truncation behavior.
Diagram

graph TD
  CLI["PR Review CLI"] --> FULL["Full PR Query"] --> DIGEST["Digest Counts"]
  DIGEST -->|truncated| WARN["Truncation Warning"]
  CLI --> NARROW["20 PR History"] --> EMPTY{"Activity Found?"}
  EMPTY -->|no| WIDE["100 PR History"] --> WAIT["Wait Signals"]
  EMPTY -->|yes| WAIT
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Always query 100 pull requests
  • ➕ Removes fallback branching
  • ➕ Finds older Copilot activity in one request
  • ➖ Expands every ordinary history request unnecessarily
  • ➖ Fetches substantially more nested review and comment data
2. Fully paginate review threads
  • ➕ Produces exact thread and unresolved counts
  • ➕ Eliminates manual follow-up for pull requests exceeding 100 threads
  • ➖ Adds GraphQL round trips to the common digest path
  • ➖ Duplicates the existing paginated traversal used by reply

Recommendation: Keep the conditional 20-to-100 history fallback and explicit thread-truncation warning. They fix the observed failures while preserving single-call behavior for normal history reads and avoiding expensive thread pagination in the digest path; the existing reply traversal remains the appropriate mechanism when exact thread data is required.

Files changed (2) +201 / -27

Bug fix (1) +89 / -25
pr_review.pyAdd widened Copilot history fallback and thread truncation reporting +89/-25

Add widened Copilot history fallback and thread truncation reporting

• Parameterizes the Copilot history GraphQL query and retries once with 100 recent pull requests when the 20-PR window contains no activity. It also reads review-thread pagination metadata, marks incomplete summary counts with a trailing plus sign, and emits a detailed truncation warning.

scripts/pr_review.py

Tests (1) +112 / -2
test_pr_review.pyCover history widening and truncated thread summaries +112/-2

Cover history widening and truncated thread summaries

• Extends test payloads with review-thread pagination state and verifies digest markers for truncated and complete pages. Adds unit and end-to-end wait coverage for narrow-window success, widened fallback, both-window emptiness, and bot auto-request behavior.

scripts/tests/test_pr_review.py

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

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: 1f3a2fc8-4fb2-498d-a4d2-345527da88e4

📥 Commits

Reviewing files that changed from the base of the PR and between 9cfd2be and 88edbdb.

📒 Files selected for processing (1)
  • scripts/pr_review.py

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


📝 Walkthrough

Walkthrough

The review flow detects truncated review threads and marks related counts. Copilot reviewer lookup widens repository history from 20 to 100 pull requests when the narrow search finds no activity.

Changes

Review status flow

Layer / File(s) Summary
Thread truncation and digest reporting
scripts/pr_review.py, scripts/tests/test_pr_review.py
The pull-request query exposes reviewThreads.pageInfo.hasNextPage. Digest counts receive a + marker when threads are truncated, and the digest emits THREADS TRUNCATED. Tests cover truncated and complete pages.
Widened Copilot history lookup
scripts/pr_review.py, scripts/tests/test_pr_review.py
History queries accept configurable widths. An empty narrow result triggers one 100-pull-request search. Tests cover widening, existing narrow results, empty results, bot discovery, and review requests.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 88edb

The changes improve reviewer-activity fallback handling and make review-thread truncation visible; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.37% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the pull request objective: promoting the Develop changes to Main. It is concise and specific.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

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

@qodo-code-review

qodo-code-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Polling message uses semicolon ✓ Resolved 📜 Skill insight ✧ Quality
Description
The newly authored polling status prose uses a semicolon to join clauses. The checklist requires
prose punctuation to avoid semicolons.
Code

scripts/pr_review.py[659]

+            "requests to read the reviewer bot id from, so nothing was requested here; "
Relevance

●●● Strong

Semicolon removal is a trivial deterministic prose-style fix, and repository prose rules favor
correcting it.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2826756 prohibits semicolons in agent-authored prose. The added user-facing string
contains here; polling only.

scripts/pr_review.py[659-660]
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 polling status message uses a semicolon as prose punctuation.

## Issue Context
Split the clauses into sentences or use another ASCII punctuation form.

## Fix Focus Areas
- scripts/pr_review.py[659-660]

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


2. HISTORY_PRS_WIDE comment overlong ✗ Dismissed 📜 Skill insight ⚙ Maintainability
Description
The new constant is preceded by a three-line prose block, although comments must default to one line
and permit only a second line for a genuine constraint. This makes the implementation commentary
more verbose than the checklist allows.
Code

scripts/pr_review.py[R335-337]

+# The one-time widened retry `copilot_history` reaches for where HISTORY_PRS carries no Copilot activity at all, rather than reverting every caller to blind polling for the rest of an outage that outlasts it (#985).
+# GitHub's own connection ceiling for a single `first`, the same reason FILES_WINDOW and CHECKS_WINDOW hold it.
+# Reaching further back needs cursor pagination, which this best-effort signal is not worth paying for, so one wider try is where this stops.
Relevance

●●● Strong

Recent PR #982 accepted the same three-line comment reduction rule in repository scripts.

PR-#982

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2826677 requires comments to be one line by default and allows only a second line
for genuine constraints. The added block at lines 335-337 uses three full comment lines to explain
the widened retry, GitHub limit, and pagination tradeoff.

scripts/pr_review.py[335-337]
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 `HISTORY_PRS_WIDE` explanation is a three-line prose comment block, exceeding the allowed concise comment structure.

## Issue Context
Keep only the essential reason that cannot be expressed by the constant and surrounding code.

## Fix Focus Areas
- scripts/pr_review.py[335-337]

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


3. Truncation message uses semicolon ✓ Resolved 📜 Skill insight ✧ Quality
Description
The new THREADS TRUNCATED explanatory prose joins instructions with a semicolon. This violates the
rule against semicolons as prose punctuation.
Code

scripts/pr_review.py[1667]

+            "open. `threads=` and `unresolved=` above undercount; read the rest with "
Relevance

●●● Strong

Semicolon removal is a trivial deterministic prose-style fix, and repository prose rules favor
correcting it.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2826756 prohibits semicolons in agent-authored prose. The added warning contains
undercount; read the rest.

scripts/pr_review.py[1667-1668]
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 truncation warning uses a semicolon as prose punctuation.

## Issue Context
Use a period and begin a new sentence for the instruction to read remaining threads.

## Fix Focus Areas
- scripts/pr_review.py[1667-1668]

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



Informational

4. Comment embeds issue context ✗ Dismissed 📜 Skill insight ✧ Quality
Description
The HISTORY_PRS_WIDE comment embeds the task-specific #985 issue reference. That context belongs
in the PR description or history rather than inline code commentary.
Code

scripts/pr_review.py[335]

+# The one-time widened retry `copilot_history` reaches for where HISTORY_PRS carries no Copilot activity at all, rather than reverting every caller to blind polling for the rest of an outage that outlasts it (#985).
Relevance

●● Moderate

No close rejection precedent for issue references in comments; the finding is subjective despite the
stated rule.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2827092 prohibits task- or PR-specific references in comments. The added comment
explicitly ends with (#985).

scripts/pr_review.py[335-335]
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 inline comment references issue `#985`, tying the code explanation to the current task context.

## Issue Context
Retain only durable reasoning that remains understandable without the originating ticket.

## Fix Focus Areas
- scripts/pr_review.py[335-335]

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


5. copilot_history docstring exposes internals ✗ Dismissed 📜 Skill insight ✧ Quality
Description
The expanded docstring describes the two-call algorithm, outage history, and cost rationale instead
of limiting itself to the function's behavior contract. These implementation details make callers
depend conceptually on mechanics that may change.
Code

scripts/pr_review.py[R525-528]

+    Read at HISTORY_PRS first and, only where that comes back with nothing at all, read again at
+    the wider HISTORY_PRS_WIDE. A narrow window emptying out is the ordinary case, a repository
+    whose most recent activity genuinely carries none of the reviewer's, and costs nothing beyond
+    the one call either caller below was always going to make. It stops being ordinary once an
Relevance

●● Moderate

History includes mixed docstring-style outcomes, but no close precedent rejecting algorithm-focused
contracts in this file.

PR-#959

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2827096 requires docstrings to focus on behavior contracts rather than
implementation details. The added text explains the narrow call, wider second call, ordinary-case
cost, and outage mechanism.

scripts/pr_review.py[525-534]
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 `copilot_history` docstring documents internal traversal and retry mechanics rather than the stable caller-facing behavior.

## Issue Context
State what history is returned and its ordering. Move any indispensable implementation rationale to a concise inline comment near the retry.

## Fix Focus Areas
- scripts/pr_review.py[525-534]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 70 rules
✅ Skills: 5 invoked
  comment-and-doc-style
  dotnet-codestyle
  python-codestyle
  shell-codestyle
  workflow-ci-contract
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread scripts/pr_review.py
Comment thread scripts/pr_review.py
Comment thread scripts/pr_review.py
Comment thread scripts/pr_review.py Outdated
Comment thread scripts/pr_review.py Outdated
qodo flagged two semicolons in #986's new prose (PR #991's promotion
review) that violate the fleet's no-semicolon-in-prose rule
(comment-and-doc-style). Recast each as two sentences:

- `request_copilot_review`'s no-bot-id fallback message
- the `THREADS TRUNCATED` digest block

Not squashed into #986 since it already merged; this is a small
follow-up onto develop.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Style**
  * Improved punctuation in diagnostic messages for clearer readability.
  * Clarified the thread-truncation warning and pagination instruction.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@ptr727
ptr727 merged commit 7b21025 into main Aug 25, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant