feat(github): paginate flare_git list actions instead of truncating at 30 - #233
Conversation
…t 30 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughGitHub list operations now use a shared pagination helper that aggregates pages into JSON arrays. Issues, pull requests, releases, and workflow runs were updated to use this flow, with workflow-run responses extracting the ChangesGitHub pagination
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ListOperation
participant Client
participant GitHubAPI
ListOperation->>Client: call get_paginated with path and extractor
Client->>GitHubAPI: request each page
GitHubAPI-->>Client: return page JSON
Client->>Client: extract and aggregate items
Client-->>ListOperation: return aggregated array
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/github/client.rs (1)
104-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd an integration test for the pagination loop.
The current tests cover
as_arrayonly; they do not verify query preservation,page/per_pagegeneration, multi-page aggregation, the 100-item boundary, or later-page error propagation. A mocked two-page response would cover the behavior introduced here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/github/client.rs` around lines 104 - 129, Add an integration test for GitHubClient::get_paginated using a mocked two-page endpoint: verify existing query parameters are preserved, each request includes the expected per_page=100 and page values, exactly 100 items on the first page triggers a second request, results from both pages are aggregated, and an error from a later page is propagated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/github/client.rs`:
- Around line 104-129: Add an integration test for GitHubClient::get_paginated
using a mocked two-page endpoint: verify existing query parameters are
preserved, each request includes the expected per_page=100 and page values,
exactly 100 items on the first page triggers a second request, results from both
pages are aggregated, and an error from a later page is propagated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c2c352f8-aaf3-49f0-802d-10b5fc8922b7
📒 Files selected for processing (5)
src/github/actions.rssrc/github/client.rssrc/github/issues.rssrc/github/pulls.rssrc/github/releases.rs
…, #233) (#290) * skill CLI: add skill crate dep, migrate serde_yaml→serde_yml, wire skill subcommand Agentflare-Agent: 1 Agentflare-Branch: skill-detect-worktree * skill CLI: wire skill subcommand into command dispatch (#233) Agentflare-Agent: claude-code_2-1-216_agent Agentflare-Branch: skill-detect-worktree * skill-detect: intent classifier, MCP skill_detect tool, session/prompt injection hooks (#231) Agentflare-Agent: claude-code_2-1-216_agent Agentflare-Branch: skill-detect-worktree * skill CLI: use crate::paths::home() for testability, add install/registry tests (#233) Agentflare-Agent: claude-code_2-1-216_agent Agentflare-Branch: skill-detect-worktree * skill-detect: semantic re-rank via MiniLM ONNX pipeline, per-turn skill injection, clippy cleanup (#231) Agentflare-Agent: claude-code_2-1-216_agent Agentflare-Branch: skill-detect-worktree * fix(skill-registry): drop unsound serde_yml, run cargo fmt serde_yml has an unsound RUSTSEC-2025-0068 advisory (segfault risk in its Serializer) and its upstream project is archived; cargo-deny flags the 0.0.13 this crate's unconstrained "0.0" requirement resolved to. Switched to serde_yaml_ng, a maintained drop-in fork, for frontmatter parsing. Also applies the cargo fmt fix CI flagged on types.rs. Agentflare-Agent: claude-code_2-1-216_agent Agentflare-Branch: item-231-233-skill-detect
Summary
Follow-up to the #221 review. The
flare_gitlist actions (pr_list,issue_list,release_list,run_list) hit GitHub's list endpoints with noper_page/pagination, so they silently returned only the first 30 items — on a busy repo the tail is dropped with no indication.This adds a single pagination helper on the client and routes the four list endpoints through it:
pulls::list,issues::list,releases::list→ bare-array endpoints, use the sharedclient::as_arrayextractor.actions::list_runs→{ "workflow_runs": [...] }envelope, uses a localworkflow_runsextractor. (parse_runswas replaced by that extractor; its two tests were updated in place.)Query strings already present on the path (e.g.
?state=open,?branch=x) are preserved — the helper picks?vs&based on whether the base path already contains a query.Test plan
cargo test --workspace(27 github tests pass, incl. newas_array_*/workflow_runs_*)cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedanticcargo fmt --checkNotes for reviewers
PER_PAGE"; a run of exactlyPER_PAGEitems costs one extra request that returns empty, then stops — correct, just one extra round-trip. Non-array / missing-key responses degrade to an empty page (extractors default tovec![]), so a malformed page ends pagination rather than looping.Vec<PullRequest>, etc.); callers and MCP output formats are untouched. Behavior change is strictly "returns all items instead of the first 30".Link to Devin session: https://app.devin.ai/sessions/1d454eb984eb4b71a45d2bd42793cb80
Requested by: @getappz
Summary by CodeRabbit
New Features
Bug Fixes