Skip to content

Commit 7877d8d

Browse files
mattdhollowaymattdholloway
committed
update pagination to remove from discussions
1 parent 5d795ec commit 7877d8d

File tree

3 files changed

+68
-43
lines changed

3 files changed

+68
-43
lines changed

‎pkg/github/discussions.go‎

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
3333
mcp.WithString("category",
3434
mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."),
3535
),
36-
WithUnifiedPagination(),
36+
WithCursorPagination(),
3737
),
3838
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
3939
// Required params
@@ -53,7 +53,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
5353
}
5454

5555
// Get pagination parameters and convert to GraphQL format
56-
pagination, err := OptionalPaginationParams(request)
56+
pagination, err := OptionalCursorPaginationParams(request)
5757
if err != nil {
5858
return nil, err
5959
}
@@ -305,7 +305,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
305305
mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")),
306306
mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")),
307307
mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")),
308-
WithUnifiedPagination(),
308+
WithCursorPagination(),
309309
),
310310
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
311311
// Decode params
@@ -319,22 +319,21 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
319319
}
320320

321321
// Get pagination parameters and convert to GraphQL format
322-
pagination, err := OptionalPaginationParams(request)
322+
pagination, err := OptionalCursorPaginationParams(request)
323323
if err != nil {
324324
return nil, err
325325
}
326326

327327
// Check if pagination parameters were explicitly provided
328-
_, pageProvided := request.GetArguments()["page"]
329328
_, perPageProvided := request.GetArguments()["perPage"]
330-
paginationExplicit := pageProvided || perPageProvided
329+
paginationExplicit := perPageProvided
331330

332331
paginationParams, err := pagination.ToGraphQLParams()
333332
if err != nil {
334333
return nil, err
335334
}
336335

337-
// Use default of 100 if pagination was not explicitly provided
336+
// Use default of 30 if pagination was not explicitly provided
338337
if !paginationExplicit {
339338
defaultFirst := int32(DefaultGraphQLPageSize)
340339
paginationParams.First = &defaultFirst
@@ -419,7 +418,6 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
419418
mcp.Required(),
420419
mcp.Description("Repository name"),
421420
),
422-
WithUnifiedPagination(),
423421
),
424422
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
425423
// Decode params
@@ -431,28 +429,6 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
431429
return mcp.NewToolResultError(err.Error()), nil
432430
}
433431

434-
// Get pagination parameters and convert to GraphQL format
435-
pagination, err := OptionalPaginationParams(request)
436-
if err != nil {
437-
return nil, err
438-
}
439-
440-
// Check if pagination parameters were explicitly provided
441-
_, pageProvided := request.GetArguments()["page"]
442-
_, perPageProvided := request.GetArguments()["perPage"]
443-
paginationExplicit := pageProvided || perPageProvided
444-
445-
paginationParams, err := pagination.ToGraphQLParams()
446-
if err != nil {
447-
return nil, err
448-
}
449-
450-
// Use default of 100 if pagination was not explicitly provided
451-
if !paginationExplicit {
452-
defaultFirst := int32(DefaultGraphQLPageSize)
453-
paginationParams.First = &defaultFirst
454-
}
455-
456432
client, err := getGQLClient(ctx)
457433
if err != nil {
458434
return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil
@@ -472,18 +448,13 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
472448
EndCursor githubv4.String
473449
}
474450
TotalCount int
475-
} `graphql:"discussionCategories(first: $first, after: $after)"`
451+
} `graphql:"discussionCategories(first: $first)"`
476452
} `graphql:"repository(owner: $owner, name: $repo)"`
477453
}
478454
vars := map[string]interface{}{
479455
"owner": githubv4.String(params.Owner),
480456
"repo": githubv4.String(params.Repo),
481-
"first": githubv4.Int(*paginationParams.First),
482-
}
483-
if paginationParams.After != nil {
484-
vars["after"] = githubv4.String(*paginationParams.After)
485-
} else {
486-
vars["after"] = (*githubv4.String)(nil)
457+
"first": githubv4.Int(25),
487458
}
488459
if err := client.Query(ctx, &q, vars); err != nil {
489460
return mcp.NewToolResultError(err.Error()), nil

‎pkg/github/discussions_test.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,14 +353,13 @@ func Test_GetDiscussionComments(t *testing.T) {
353353

354354
func Test_ListDiscussionCategories(t *testing.T) {
355355
// Use exact string query that matches implementation output
356-
qListCategories := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first, after: $after){nodes{id,name},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
356+
qListCategories := "query($first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first){nodes{id,name},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
357357

358358
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
359359
vars := map[string]interface{}{
360360
"owner": "owner",
361361
"repo": "repo",
362-
"first": float64(30),
363-
"after": (*string)(nil),
362+
"first": float64(25),
364363
}
365364

366365
mockResp := githubv4mock.DataResponse(map[string]any{
@@ -397,9 +396,6 @@ func Test_ListDiscussionCategories(t *testing.T) {
397396

398397
text := getTextResult(t, result).Text
399398

400-
// Debug: print the actual JSON response
401-
t.Logf("JSON response: %s", text)
402-
403399
var response struct {
404400
Categories []map[string]string `json:"categories"`
405401
PageInfo struct {

‎pkg/github/server.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,21 @@ func WithUnifiedPagination() mcp.ToolOption {
212212
}
213213
}
214214

215+
// WithCursorPagination adds only cursor-based pagination parameters to a tool (no page parameter).
216+
func WithCursorPagination() mcp.ToolOption {
217+
return func(tool *mcp.Tool) {
218+
mcp.WithNumber("perPage",
219+
mcp.Description("Results per page for pagination (min 1, max 100)"),
220+
mcp.Min(1),
221+
mcp.Max(100),
222+
)(tool)
223+
224+
mcp.WithString("after",
225+
mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."),
226+
)(tool)
227+
}
228+
}
229+
215230
type PaginationParams struct {
216231
Page int
217232
PerPage int
@@ -243,6 +258,49 @@ func OptionalPaginationParams(r mcp.CallToolRequest) (PaginationParams, error) {
243258
}, nil
244259
}
245260

261+
// OptionalCursorPaginationParams returns the "perPage" and "after" parameters from the request,
262+
// without the "page" parameter, suitable for cursor-based pagination only.
263+
func OptionalCursorPaginationParams(r mcp.CallToolRequest) (CursorPaginationParams, error) {
264+
perPage, err := OptionalIntParamWithDefault(r, "perPage", 30)
265+
if err != nil {
266+
return CursorPaginationParams{}, err
267+
}
268+
after, err := OptionalParam[string](r, "after")
269+
if err != nil {
270+
return CursorPaginationParams{}, err
271+
}
272+
return CursorPaginationParams{
273+
PerPage: perPage,
274+
After: after,
275+
}, nil
276+
}
277+
278+
type CursorPaginationParams struct {
279+
PerPage int
280+
After string
281+
}
282+
283+
// ToGraphQLParams converts cursor pagination parameters to GraphQL-specific parameters.
284+
func (p CursorPaginationParams) ToGraphQLParams() (GraphQLPaginationParams, error) {
285+
if p.PerPage > 100 {
286+
return GraphQLPaginationParams{}, fmt.Errorf("perPage value %d exceeds maximum of 100", p.PerPage)
287+
}
288+
if p.PerPage < 0 {
289+
return GraphQLPaginationParams{}, fmt.Errorf("perPage value %d cannot be negative", p.PerPage)
290+
}
291+
first := int32(p.PerPage)
292+
293+
var after *string
294+
if p.After != "" {
295+
after = &p.After
296+
}
297+
298+
return GraphQLPaginationParams{
299+
First: &first,
300+
After: after,
301+
}, nil
302+
}
303+
246304
type GraphQLPaginationParams struct {
247305
First *int32
248306
After *string

0 commit comments

Comments
 (0)