elastic / elastic/ai-github-actions
[refactor-opportunist] Extract shared review-thread pagination helper for Claude PR scripts
- Dominant language
- Python
- Stars
- 11
- Forks
- 16
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 31
Description
## 🏗️ Refactor Proposal
**Summary:** Centralize GraphQL review-thread/comment pagination into one shared shell helper and migrate callers incrementally.
## Problem
`claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh` and `claude-workflows/pr-review/scripts/pr-existing-comments.sh` both implemented near-identical GraphQL pagination logic for:
- Pull request `reviewThreads` pagination
- Per-thread `comments` pagination
- Merging paginated results back into a single thread list
Concrete evidence:
- `claude-workflows/pr-review/scripts/pr-existing-comments.sh:61-227` still contains the full pagination implementation.
- Before this PoC, `claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh` had the same structure; this run migrated it to shared code (`claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh:27-47`).
- Churn signal: commit `707fc679b432715d2199fcc654ee07945cac75fe` changed both files together (`166` and `178` line edits), which is a strong co-change coupling indicator.
This duplication increases maintenance risk: GraphQL schema changes or pagination bug fixes must be updated in multiple scripts.
## Proposed Approach
Introduce a shared script (`claude-workflows/shared/scripts/review-thread-pagination.sh`) that exposes reusable functions:
- `aw_fetch_review_threads_page`
- `aw_fetch_thread_comments_page`
- `aw_fetch_all_review_threads`
Keep caller-specific behavior (field selection, filtering, output formatting) in each workflow script, and move only pagination/aggregation into the shared helper.
## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability.
**Files changed:**
- `claude-workflows/shared/scripts/review-thread-pagination.sh` (new)
- `claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh` (migrated to shared helper)
**Before → After:**
- Before: `gh-get-review-threads.sh` contained ~160 lines of pagination/query functions and merge loops.
- After: it defines only caller-specific fields and delegates to shared logic:
```bash
source "$SCRIPT_DIR/../../shared/scripts/review-thread-pagination.sh"
...
THREADS=$(aw_fetch_all_review_threads "$OWNER" "$REPO" "$PR_NUMBER" "$THREAD_FIELDS" "$COMMENT_FIELDS")
```
Shared implementation now lives in:
- `claude-workflows/shared/scripts/review-thread-pagination.sh:8-177`
**Verification:**
- `make lint` ✅
- `PATH="$HOME/.local/bin:$PATH" UV_CACHE_DIR=/tmp/gh-aw/agent/uv-cache make test` ✅ (`53 passed`)
- `make compile` ✅
## Incremental Rollout Plan
This refactor can be completed incrementally:
1. **Done (proved):** Extract helper and migrate `mention-in-pr` review-thread fetch script.
2. Migrate `claude-workflows/pr-review/scripts/pr-existing-comments.sh` to the same helper while preserving its extra GraphQL fields (`originalLine`, `diffSide`, `originalCommit`).
3. Add/extend pagination tests to cover both callers through the shared helper contract and remove redundant code paths.
## Risks and Mitigations
- **Risk:** Caller field-shape differences (e.g., `originalCommit`) could be lost during migration.
**Mitigation:** Keep caller-provided field blocks (`THREAD_FIELDS`/`COMMENT_FIELDS`) as explicit inputs.
- **Risk:** Behavior drift in output shape consumed by downstream scripts.
**Mitigation:** Preserve `.comments.nodes` output contract and verify with existing pagination tests.
## Evidence
- `claude-workflows/pr-review/scripts/pr-existing-comments.sh:61-227`
- `claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh:27-47`
- `claude-workflows/shared/scripts/review-thread-pagination.sh:8-177`
- `tests/test_review_thread_pagination.py:157-194`
- `git log --since="60 days ago" --stat -- claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh claude-workflows/pr-review/scripts/pr-existing-comments.sh` (notably commit `707fc679b432715d2199fcc654ee07945cac75fe`)
No matching open issue/PR was found for this specific shared-helper extraction.
> [!NOTE]
>
> 🔒 Integrity filter blocked 42 items
>
> The following items were blocked because they don't meet the GitHub integrity level.
>
> - [#321](https://github.com/elastic/ai-github-actions/issues/321) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#875](https://github.com/elastic/ai-github-actions/issues/875) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#575](https://github.com/elastic/ai-github-actions/issues/575) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#121](https://github.com/elastic/ai-github-actions/issues/121) `search_issues`: 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".
> - [#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".
> - [#511](https://github.com/elastic/ai-github-actions/pull/511) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#589](https://github.com/elastic/ai-github-actions/pull/589) `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".
> - [#676](https://github.com/elastic/ai-github-actions/issues/676) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#235](https://github.com/elastic/ai-github-actions/issues/235) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#295](https://github.com/elastic/ai-github-actions/issues/295) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#197](https://github.com/elastic/ai-github-actions/issues/197) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#334](https://github.com/elastic/ai-github-actions/issues/334) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#344](https://github.com/elastic/ai-github-actions/issues/344) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#187](https://github.com/elastic/ai-github-actions/issues/187) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - ... and 26 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/25678565455)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Assessment
This issue has not been assessed yet.