Skip to content

[refactor-opportunist] Extract shared PR review-thread pagination helperย #1199

Description

@github-actions

๐Ÿ—๏ธ Refactor Proposal

Summary: Consolidate duplicated GraphQL review-thread pagination logic into a shared shell helper, then migrate both PR-review and mention-in-PR scripts to use it.

Problem

Two production scripts independently implement nearly identical paginated GraphQL fetch logic for review threads and nested comments, which increases maintenance cost and drift risk:

  • claude-workflows/pr-review/scripts/pr-existing-comments.sh (previously had inline fetch_review_threads_page, fetch_thread_comments_page, and pagination loops)
  • claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh (still has the same structure)

Concrete evidence:

  • claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh:27-180 contains duplicate page-fetch + cursor loops (THREADS_AFTER, HAS_NEXT_THREADS, nested comment pagination).
  • claude-workflows/pr-review/scripts/pr-existing-comments.sh had the same logic before the PoC extraction (now replaced by helper call at :23-25 and :64-79).
  • Co-change signal: git log --since="60 days ago" -- claude-workflows/pr-review/scripts/pr-existing-comments.sh claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh shows shared fix commit 707fc67 (โ€œpaginate GraphQL review thread fetches in PR helper scriptsโ€).

Proposed Approach

Create a shared helper that encapsulates:

  1. GraphQL query construction for first/after pagination calls.
  2. Cursor loop across reviewThreads pages.
  3. Nested cursor loop across per-thread comments pages.

Keep each workflow script as a thin adapter that supplies only field selections/output shaping and workflow-specific filtering behavior.

Proof of Concept

I partially implemented this refactor on one representative slice (pr-review) to verify viability.

Files changed:

  • claude-workflows/scripts/gh-fetch-review-threads.sh (new shared helper)
  • claude-workflows/pr-review/scripts/pr-existing-comments.sh (migrated to helper)

Before โ†’ After:

  • Before: pr-existing-comments.sh carried ~170 lines of inline pagination/query logic.
  • After: the script sources a shared helper and passes field sets:
# claude-workflows/pr-review/scripts/pr-existing-comments.sh:23-25
SCRIPT_DIR="$(cd "$(dirname "\$\{BASH_SOURCE[0]}")" && pwd)"
source "\$\{SCRIPT_DIR}/../../scripts/gh-fetch-review-threads.sh"

# :64-79
THREADS="$(aw_fetch_review_threads_with_comments "$OWNER" "$REPO_NAME" "$PR_NUMBER" "$THREAD_FIELDS" "$COMMENT_FIELDS")"

Shared abstraction now lives in:

  • claude-workflows/scripts/gh-fetch-review-threads.sh:3-184

Verification:

  • make lint โœ…
  • UV_CACHE_DIR=/tmp/gh-aw/agent/uv-cache PATH="/tmp/gh-aw/agent/uvenv/bin:$PATH" make test โœ… (53 passed)

Incremental Rollout Plan

This refactor can be completed incrementally:

  1. Done (PoC): Extract helper and migrate pr-existing-comments.sh.
  2. Next slice: Migrate claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh to the shared helper while preserving JSON output contract.
  3. Remaining work: Add focused regression coverage for helper-driven behavior in both workflows and remove any residual duplicated pagination code.

Risks and Mitigations

  • Risk: Output-shape regressions in consumer scripts.
    Mitigation: Preserve existing field lists per caller and keep tests asserting exact structure.
  • Risk: Over-generalized helper interface becomes hard to read.
    Mitigation: Keep helper scope strictly to pagination/fetch; leave filtering/formatting in caller scripts.

Evidence

  • claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh:27-180
  • claude-workflows/pr-review/scripts/pr-existing-comments.sh:23-25,64-79
  • claude-workflows/scripts/gh-fetch-review-threads.sh:3-184
  • Commit: 707fc67 (recent co-change fix across both scripts)

Note

๐Ÿ”’ Integrity filter blocked 26 items

The following items were blocked because they don't meet the GitHub integrity level.

  • #359 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1128 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #600 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #700 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #764 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #490 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #937 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #14 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #400 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #418 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #546 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #537 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #19 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #851 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #608 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #47 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • ... and 10 more items

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

What is this? | From workflow: Trigger Refactor Opportunist

Give us feedback! React with ๐Ÿš€ if perfect, ๐Ÿ‘ if helpful, ๐Ÿ‘Ž if not.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions