elastic / elastic/ai-github-actions

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

Open
#1,199 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
11
Forks
16
Avg merge
22h 9m
Merged PRs (30d)
31

Description

## 🏗️ 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:

```bash
# 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](https://github.com/elastic/ai-github-actions/issues/359) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#1128](https://github.com/elastic/ai-github-actions/pull/1128) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#600](https://github.com/elastic/ai-github-actions/pull/600) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#700](https://github.com/elastic/ai-github-actions/pull/700) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#764](https://github.com/elastic/ai-github-actions/pull/764) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#490](https://github.com/elastic/ai-github-actions/pull/490) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#937](https://github.com/elastic/ai-github-actions/pull/937) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#14](https://github.com/elastic/ai-github-actions/pull/14) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#400](https://github.com/elastic/ai-github-actions/pull/400) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#418](https://github.com/elastic/ai-github-actions/pull/418) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#546](https://github.com/elastic/ai-github-actions/pull/546) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#537](https://github.com/elastic/ai-github-actions/pull/537) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#19](https://github.com/elastic/ai-github-actions/pull/19) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#851](https://github.com/elastic/ai-github-actions/pull/851) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#608](https://github.com/elastic/ai-github-actions/pull/608) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#47](https://github.com/elastic/ai-github-actions/issues/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:
>
> ```yaml
> tools:
> github:
> min-integrity: approved # merged | approved | unapproved | none
> ```
>
>

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Refactor Opportunist](https://github.com/elastic/ai-github-actions/actions/runs/26766588943)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.