elastic / elastic/ai-github-actions
[autonomy-atomicity] PR review pagination test couples data-path verification to summary wording
- Dominant language
- Python
- Stars
- 11
- Forks
- 16
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 31
Description
## Autonomy / Atomicity Findings
### 1. Pagination test is over-broad and tied to unrelated output text
**Category:** Over-broad test
**File(s):**
- `tests/test_review_thread_pagination.py` (lines 157-174)
- `claude-workflows/pr-review/scripts/pr-existing-comments.sh` (summary rendering at lines 263-286)
**Problem:**
The test named `test_pr_existing_comments_paginates_threads_and_comments` exercises pagination, but its assertions are coupled to presentation text generated by summary rendering:
- `assert "Existing review threads: 2 total" in result.stdout`
- `assert "1 with replies" in result.stdout`
Those strings come from the summary output path (`--summary`) in `pr-existing-comments.sh` rather than from pagination state directly. This means unrelated edits to summary wording/format (for example, changing phrasing or emoji markers) can fail a test that is supposed to validate pagination behavior. In parallel development, this creates unnecessary contention between:
- contributors changing review-summary UX/output copy, and
- contributors changing pagination logic.
**Suggested fix:**
Split responsibilities in tests so pagination correctness is asserted structurally and independently from summary text formatting.
A concrete approach:
1. Keep one test focused on pagination/data aggregation (e.g., assert thread count and per-thread comment counts from parsed output or a machine-readable mode).
2. Move summary wording checks to a separate formatting-contract test (or loosen to stable structural markers only).
## Suggested Actions
- [ ] Refactor `test_pr_existing_comments_paginates_threads_and_comments` to assert pagination behavior independently of summary copy.
- [ ] Add/adjust a separate test for summary formatting expectations to avoid cross-feature brittleness.
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Autonomy Atomicity Analyzer](https://github.com/elastic/ai-github-actions/actions/runs/32749478940)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Assessment
This issue has not been assessed yet.