Skip to content

fix(issue-loop): count every pull request citing an issue, not the newest 30 - #2340

Merged
developer0hye merged 2 commits into
mainfrom
fix/issue-loop-complete-pr-refs
Oct 19, 2026
Merged

developer0hye merged 2 commits into
mainfrom
fix/issue-loop-complete-pr-refs

Conversation

@developer0hye

@developer0hye developer0hye commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

File submission policy

  • No sample files or attachments are submitted.

Summary

The issue loop detects progress by counting merged PRs whose bodies cite an issue and finding its newest open citing PR. A newest-30 window can lose an older citer during a run, hiding a real milestone. The proposed search fallback also missed older PRs awaiting indexing and silently capped the count at 1,000.

Enumerate all pages of GitHub's repository PR connection for each state, then apply the existing issue-reference filter and deduplicate PR numbers. A failure on any page aborts the operation before a count is reported. --print-progress N exposes the same logic as a read-only diagnostic.

This PR originated in an unattended reflection run. The reflection proposer is prohibited from merging its own proposal; this revision follows the maintainer's separate request to review and repair the open PR queue.

Validation

  • Added both review counterexamples before the implementation change. The original code returned 0 instead of 1 for an older unindexed merge, and 1,000 instead of 1,001 for a larger history; both tests failed as expected.
  • All 90 issue-loop tests pass, including 14 progress tests covering multiple pages, newest-open selection, duplicate entries, citation boundaries, empty results, first-page failure, later-page failure, and failure while listing open PRs.
  • All 112 visual PR contract checker tests pass.
  • bash -n scripts/issue_loop.sh and git diff --check pass.
  • Live read-only validation matched a separately enumerated list of 1,205 merged and 15 open PRs, whose sizes matched GraphQL total counts. Issues 2217, 2056, 2224, and 1721 returned merged counts 3, 4, 2, and 26 respectively; issue 2056 selected open PR 2252. Each full diagnostic took about 12–13 seconds in this scoped check.
  • Rust runtime tests are not applicable: this change affects the issue-loop shell script, its Python tests, and documentation only.
  • Documentation freshness audit: PASS before the signed integration commit.

Related issue

None. This fixes the progress-detection failure documented by the original reflection report and the reproducible review counterexamples.

Visual impact

  • No rendered PDF change
  • Rendered PDF change or visual evidence added
  • Reason: no conversion code, fixtures, or PDF output changes.

Final performance pass

  • Status: Not applicable: no conversion runtime path changes.
  • Measurements: Not applicable; only issue-loop API enumeration changes.
  • Outcome and revalidation: the issue-loop behavioral suite and live read-only PR enumeration validate the changed path.

Checklist

  • Commits include a Signed-off-by line.
  • Final memory/speed pass is not applicable because conversion runtime is unchanged.
  • PR scope contains one root cause: incomplete progress counts.

…west 30

The loop reads a run's progress from the merged pull requests whose body
cites the issue (milestone when the count rises) and the open one citing
it (pr-open). Both read only the 30 most recently created pull requests,
so an older citing PR dropped out of the count whenever newer ones merged.

On 2026-10-19 #2217's second attempt merged #2321 while #2319 also
merged, pushing the older citing #2219 out of the window: the count read
2 before and after, the run was recorded as no-progress, and the loop
labeled #2217 autofix-blocked with a comment saying no pull request had
merged. The same window gave later prompts an undercounted or missing
preflight note (#2224's runs were told nothing about #2225).

Union the 30 newest with a body search for the issue number, keep the
`#N\b` filter, and fail on a gh error instead of reading zero. The
newest 30 still cover a just-merged PR the search index may lack.
`--print-progress N` prints what the loop counts, for tests and for
diagnosing a misclassified run.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
@developer0hye

developer0hye commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner Author

Two reproducible gaps remain in citing_prs, despite the current green CI. I reproduced both against this exact head using the PR's existing fake-GitHub harness, without changing the script.

  1. An older PR merged before search indexing catches up is still lost. progress(2217, [pr(1, "Related: #2217", indexed=False)] + unrelated(2, 40)) returns merged=0, although one merged PR cites the issue. The fallback contains the newest PRs by creation, so it does not cover an older PR that was just merged. This can still misclassify actual progress and eventually block an issue.
  2. The search cap silently undercounts. progress(2217, [pr(i, "Related: #2217") for i in range(1, 1002)]) returns merged=1000, although all 1,001 PRs cite the issue. The newest-30 fallback is already a subset of those search results. Treating a truncated successful response as a complete count preserves the failure mode the change is intended to remove.

Please add both behavioral cases and use an authoritative paginated PR enumeration, or another approach that establishes completeness without depending on search indexing or silently truncating. If completeness cannot be established, return an error rather than a definitive count. The existing tests for API failures and newest-open selection should remain intact.

The PR is useful and remains open for repair; I am holding the merge on these concrete counterexamples.


Update: both counterexamples are fixed in cc33dda4330169fcfc5f3ab2d871760961ef7b25. The implementation now traverses the repository PR connection with GraphQL pagination, without search-index dependence or a result cap, and aborts without partial output if any page fails. Both added tests failed on the original implementation and pass after the repair. All 90 issue-loop tests and 112 PR-contract checker tests pass; the documentation audit returned PASS. Live diagnostic results for four issues matched a separately enumerated list of 1,205 merged and 15 open PRs, verified against repository total counts.

The code findings above are resolved. The PR is ready for review and awaiting its exact-head CI; no merge has occurred.

@developer0hye
developer0hye marked this pull request as draft October 19, 2026 14:40
Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
@developer0hye
developer0hye marked this pull request as ready for review October 19, 2026 15:43
@developer0hye
developer0hye merged commit 1eb1e8a into main Oct 19, 2026
26 checks passed
@developer0hye
developer0hye deleted the fix/issue-loop-complete-pr-refs branch October 19, 2026 16:44
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