Skip to content

Add server-side branch prefix search and cursor pagination for GitHub#110

Open
HarshMN2345 wants to merge 7 commits into
mainfrom
feature/github-branch-search
Open

Add server-side branch prefix search and cursor pagination for GitHub#110
HarshMN2345 wants to merge 7 commits into
mainfrom
feature/github-branch-search

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

Uses GraphQL refs query with query variable for prefix filtering and cursor-based pagination instead of fetching all branches client-side.

Uses GraphQL refs query with query variable for prefix filtering and
cursor-based pagination instead of fetching all branches client-side.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR migrates listBranches from a REST call to a GitHub GraphQL refs query, adding server-side search filtering and cursor-based pagination via a new $search parameter and a struct return type. It also fixes getLatestCommit to gracefully handle null author fields that occur on commits made by GitHub App installations.

  • listBranches now returns array{items, hasNext, nextCursor} instead of a flat array<string>; the null-repository case (data.repository: null) is correctly guarded with is_array() before any array access, and pagination state is read from GraphQL's own pageInfo rather than a client-side probe.
  • getLatestCommit no longer throws when author is null; instead commitAuthorAvatar and commitAuthorUrl fall back to empty strings, which is appropriate for App-installation commits.
  • Tests are updated throughout to assert the new struct shape and exercise three-page cursor navigation and search-filtered pagination.

Confidence Score: 4/5

Safe to merge with awareness that the adapter contract change is intentionally staged; no runtime bugs found in the current implementation.

The GraphQL implementation is logically correct — null-repository is guarded before any array access, pageInfo.hasNextPage is used directly so no extra round-trip or probe arithmetic is needed, and the getLatestCommit author-null fix is sound. Unresolved concerns from prior review discussion (documentation inconsistency between 'prefix' in the PHPDoc and 'substring' in the test comment, and the silent fallback when an integer $page > 1 is passed) are present in the merged code but do not cause runtime failures in typical usage.

src/VCS/Adapter/Git/GitHub.php — specifically the $search parameter documentation and the integer $page silent-fallback behaviour.

Important Files Changed

Filename Overview
src/VCS/Adapter/Git/GitHub.php Replaces REST-based listBranches with GraphQL cursor pagination and adds $search param; also fixes getLatestCommit to tolerate null author from App installations. Null-repository case is handled correctly via is_array() guard. Documentation inconsistency (PHPDoc says "Prefix filter" but code/test describes substring behaviour) and silent fallback for integer $page > 1 remain from prior review threads.
tests/VCS/Adapter/GitHubTest.php Tests updated to assert the new struct return shape; cursor-based pagination flow is fully exercised across three pages. Existing clone/owner/search tests moved earlier in the file. No functional test regressions observed.

Reviews (5): Last reviewed commit: "Remove client-side prefix filter — trust..." | Re-trigger Greptile

Comment thread src/VCS/Adapter/Git/GitHub.php
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment on lines +755 to +758
* @param int|string|null $page Pass 1 (or null) for the first page. For subsequent pages
* always pass the opaque cursor string from the previous nextCursor — GitHub uses
* cursor-based GraphQL pagination and has no concept of integer page offsets.
* Any integer value other than 1 is treated as page 1.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent fallback for integer $page values other than 1 may surprise callers

A caller passing $page = 3 silently gets the first page with no error or warning. Consider throwing an \InvalidArgumentException for integer values > 1, or explicitly marking those values as unsupported.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant