- Notifications
You must be signed in to change notification settings - Fork3k
projects: add server instructions#1352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
projects: add server instructions#1352
Conversation
There was a problem hiding this comment.
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 enhances the GitHub Projects V2 integration by adding advanced pagination support, improving response structures, refining query capabilities, and updating test coverage to match the new implementation. The changes focus on providing more robust pagination handling and clearer field filtering capabilities.
Key changes:
- Added cursor-based pagination support (
after,beforeparameters) withpageInfoin responses - Changed response format to include
pageInfometadata alongside data arrays - Updated field filtering to exclude special/system data types from responses
- Refactored field ID handling from array to comma-separated string format
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/github/projects.go | Core implementation: adds pagination options extraction, pageInfo builder, filterSpecialTypes function, updated response structures with pageInfo, and comprehensive query documentation |
| pkg/github/projects_test.go | Test updates: modified mock data to include required fields (node_id), updated assertions to validate new response structure with pageInfo, simplified field query parameter handling |
| pkg/github/toolsnaps/*.snap | Snapshot updates: documents new pagination parameters and updated descriptions for tool schemas |
| README.md | Documentation updates: adds pagination parameters and extensive query syntax documentation for projects tools |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
63bbdf0 to1235e58Compare
kerobbi left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The technical improvements here lgtm! The only issues I can see are related to descriptions/prompts being quite verbose, and overly focused on technical details.
I have added a couple of comments just for better clarity, although they are all related to the same root concern.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
kerobbi left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Left some comments mainly regarding reducing/optimising context usage and avoiding duplicate instructions.
I also suggested removing a lot of the query related instructions, as in general we've been finding that the model is able to construct pretty good queries using the GitHub syntax. I am curious, have you found that the model has not been able to construct queries well? If so, was it happening even for simple queries or for more complex projects ones?
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
stephenotalora commentedNov 7, 2025
Thanks,@kerobbi! I appreciate the feedback,@tmelliottjr is out until Monday, but I’m reviewing your comments in the meantime and will address them accordingly 🙇🏼 |
2ba80b8 to5cb0758Comparestephenotalora commentedNov 7, 2025
@tmelliottjr just an fyi, recent push is to address a number of conflicts with main |
| }, | ||
| "per_page": { | ||
| "description":"Number of resultsper page (max100, default: 30)", | ||
| "description":"Resultsper page (max50)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why is that different than the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I observed the models often usingper_page: 100. Depending on the content in the payload and how many/what types of fields are requested this can result in some incredibly large response payloads. This was in service of trying to minimize that.
This along with encouraging the correct usage ofquery will hopefully reduce scenarios where truncation occurs.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
pkg/github/instructions.go Outdated
| Self-check before returning: | ||
| - Paginated fully | ||
| - Dedupe by id/node_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why do we even need to dedupe by id? If we're listing items by id they should contain unique items already.
8f6adde to976ff67Comparetmelliottjr commentedNov 11, 2025
Closed in favor of#1393 |
This PR updates the tool/argument descriptions for projects related tools to better enable tool selection and project item filter.
Additionally, this PR introduces pagination for
list_*operations to enable effective querying.