Skip to content

Add server-side branch prefix search#110

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

Add server-side branch prefix search#110
HarshMN2345 wants to merge 22 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

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the REST-based listBranches implementation with a GraphQL query that supports prefix-filtered search via the refs(query:) argument and cursor-based pagination for pages beyond the first. It also makes author fields optional in getLatestCommit to handle commits whose GitHub account has been removed.

  • listBranches now issues a GraphQL refs query with an optional query variable for server-side prefix filtering; a separate cursor-preflight query is used when $page > 1 to derive the correct after cursor.
  • getLatestCommit removes the hard requirement for avatar_url and html_url on the author object, returning empty strings when those fields are absent instead of throwing.

Confidence Score: 3/5

Two defects in the changed code need fixing before this is safe to merge: the cursor preflight query breaks for pages 4+ and the new substring test will fail against the real API.

The cursor query passes ($page - 1) * $perPage as first; with $perPage capped at 50 this exceeds GitHub GraphQL's hard limit of 100 starting at page 4, silently returning empty results. Additionally, the new integration test asserts that refs(query: 'ranch') returns branches containing the string 'ranch', but GitHub's refs(query:) is prefix-only — the test will fail against the real API on every run.

Both src/VCS/Adapter/Git/GitHub.php (cursor first limit) and tests/VCS/Adapter/GitHubTest.php (incorrect substring assertion) need attention before merging.

Important Files Changed

Filename Overview
src/VCS/Adapter/Git/GitHub.php Rewrites listBranches to use GraphQL with cursor-based pagination and prefix search; also makes author fields optional in getLatestCommit. The cursor query's first variable can exceed GitHub's 100-item limit for pages 4+, silently returning empty results. Docblock incorrectly states the perPage ceiling as 100 instead of 50.
tests/VCS/Adapter/GitHubTest.php Adds search assertions to testListBranchesPagination; the new $substringSearch assertion incorrectly expects prefix-only refs(query: 'ranch') to return branch names containing 'ranch', which will fail against the real GitHub API.

Reviews (12): Last reviewed commit: "cap perPage at 50 to avoid GraphQL 100-i..." | Re-trigger Greptile

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 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 thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment thread tests/VCS/Adapter/GitHubTest.php Outdated
Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment on lines +770 to +819
return array_values(array_map(fn ($branch) => $branch['name'] ?? '', $responseBody));
$edges = $refs['edges'] ?? [];
$pageInfo = $refs['pageInfo'] ?? [];
$hasNext = (bool) ($pageInfo['hasNextPage'] ?? false);

return [
'items' => array_map(fn ($edge) => $edge['node']['name'] ?? '', $edges),
'hasNext' => $hasNext,
'nextCursor' => $hasNext ? ($pageInfo['endCursor'] ?? null) : null,
];

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.

Keep old response structure - no hasNext, no cursor

Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
Comment on lines +762 to +763
if ($page > 1) {
for ($i = 1; $i < $page; $i++) {

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.

Lets not loop

Comment thread src/VCS/Adapter/Git/GitHub.php Outdated
/**
* @return array{names: array<string>, endCursor: string|null}
*/
private function fetchBranchPage(string $owner, string $repositoryName, int $perPage, ?string $after, string $search): array

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.

Lets avoid helper method

Comment thread src/VCS/Adapter/Git/GitHub.php
@HarshMN2345 HarshMN2345 changed the title Add server-side branch prefix search and cursor pagination for GitHub Add server-side branch prefix search Jun 5, 2026
'variables' => [
'owner' => $owner,
'name' => $repositoryName,
'first' => ($page - 1) * $perPage,

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.

P1 Cursor query first value exceeds GitHub's GraphQL limit for pages 4+

The cursor query passes ($page - 1) * $perPage as first. With $perPage clamped to a maximum of 50, $page = 4 yields first = 150, which exceeds GitHub GraphQL's hard limit of 100 for any connection field. GitHub returns a validation error; the code at line 783 treats a non-array response as "no next page" and returns [], so every request for page 4 or higher silently returns an empty result instead of the actual data.

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.

2 participants