Skip to content

Add boundaries to pagination - #86

Merged
SamMorrowDrums merged 4 commits into
mainfrom
wm/boundaries
Apr 3, 2025
Merged

Add boundaries to pagination#86
SamMorrowDrums merged 4 commits into
mainfrom
wm/boundaries

Conversation

@williammartin

Copy link
Copy Markdown
Collaborator

Description

Fixes#80

Reviewer Notes

There are no tests for this, also, it's not clear to me why search_repositories and list_commits have no minimum bounds on page but I'm doing the minimum to match anthropic right now.

CopilotAI review requested due to automatic review settings April 3, 2025 20:00

CopilotAI left a comment

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.

Pull Request Overview

This PR adds boundaries to pagination parameters to ensure that the "per_page" values remain between 1 and 100 and that the "page" value is at least 1.

  • Enforces minimum and maximum limits on the "per_page" parameter for code search, user search, and issues search.
  • Enforces a minimum bound on the "page" parameter in each function.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

FileDescription
pkg/github/search.goAdded mcp.Min and mcp.Max validations for pagination options in searchCode and searchUsers functions.
pkg/github/issues.goAdded mcp.Min and mcp.Max validations for pagination options in searchIssues function.
Comments suppressed due to low confidence (2)

pkg/github/search.go:91

  • The new pagination boundary validations in this file are not covered by tests. Consider adding test cases to ensure the validations behave as expected.
mcp.Min(1),

pkg/github/issues.go:167

  • The pagination validation changes in this file lack test coverage. It is recommended to add tests verifying that the minimum and maximum boundaries are enforced correctly.
mcp.Min(1),

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@SamMorrowDrumsSamMorrowDrums left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

wow-oh

SamMorrowDrums
SamMorrowDrums previously approved these changes Apr 3, 2025
Base automatically changed from wm-kw/enum-strings to mainApril 3, 2025 21:31
@SamMorrowDrums
SamMorrowDrums dismissed their stale reviewApril 3, 2025 21:31

The base branch was changed.

@SamMorrowDrums
SamMorrowDrums merged commit 87d4407 into mainApr 3, 2025
@SamMorrowDrums
SamMorrowDrums deleted the wm/boundaries branch April 3, 2025 21:34
Sign up for freeto 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.

Various tools are missing pagination property bounds or are not using matching case

3 participants

@williammartin@SamMorrowDrums