Skip to content

Add server-side branch prefix search#110

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

Add server-side branch prefix search#110
HarshMN2345 wants to merge 17 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 replaces the REST-based listBranches implementation with a GitHub GraphQL call that accepts a $search parameter and passes it as the query argument on the refs connection. A defensive guard for null author fields in getLatestCommit is also included.

  • listBranches now issues a single GraphQL request using refs(query: $search) for server-side filtering, returns a flat array<string>, and clamps $perPage to [1, 100]; the $page parameter is accepted but not used in the new implementation.
  • The test suite drops the old page-1/page-2 pagination assertions and adds search-filter assertions, including one that asserts a non-prefix substring ('ranch') returns results — which conflicts with the PR title ("prefix search") and the docblock ("Substring filter"), leaving the intended matching semantics ambiguous.

Confidence Score: 4/5

The core implementation is functionally sound for basic search use cases, but the accepted $page parameter is silently ignored and the test makes assumptions about GitHub API matching semantics that contradict the PR's stated purpose.

The GraphQL query correctly fetches branches with optional search filtering and returns a flat array. However, the $page parameter is accepted in the signature but never read — callers expecting paged navigation will silently receive only the first page. The new test also asserts that a non-prefix substring ('ranch') returns results while the PR title and docblock are internally inconsistent about whether the feature is prefix or substring matching, making it unclear which behavior is intentional and whether the test will pass against the live API.

Both src/VCS/Adapter/Git/GitHub.php and tests/VCS/Adapter/GitHubTest.php warrant a closer look — the implementation ignores the $page argument and the test's matching-semantics assertion may not hold against the real GitHub API.

Important Files Changed

Filename Overview
src/VCS/Adapter/Git/GitHub.php Replaces REST-based listBranches with a GraphQL implementation supporting server-side search; also defensively guards against null author in getLatestCommit. The $page parameter is accepted but unused in the new GraphQL path, and the $search docblock says "Substring filter" while the PR title says "prefix search".
tests/VCS/Adapter/GitHubTest.php Removes old page-1/page-2 pagination tests and adds search-filter tests; a new assertion expects 'ranch' (a substring, not a prefix) to match 'branch-a'/'branch-b', which contradicts the PR title ("prefix search") and may fail against the real API.

Reviews (9): Last reviewed commit: "revert pagination" | 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
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