Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: list_discussions sort by updatedAt & createdAt, return updatedAt and author#690

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

Merged

Conversation

@tommaso-moro
Copy link
Contributor

@tommaso-morotommaso-moro commentedJul 16, 2025
edited by LuluBeatson
Loading

Addresses#670

Overview
This PR expands the functionality of thelist_discussions tool. It adds

  • TheupdatedAt andauthor.login (i.e. username) fields to the discussions payload in thelist_discussions tool
  • The ability to order discussions by field (updated_at andcreated_at) and direction (ascending order, descending order)

Refactoring
The PR includes extensive refactoring of thelist_discussions tool to handle optional parameters in our graphql queries. This was needed because the githubv4 library requires GraphQL queries to be defined using Go struct tags that must be known at compile time, so we could not dynamically construct query strings at runtime based on user input.

The changes made include

  • Introduced reusableDiscussionFragment struct to avoid field duplication
  • Created 4 pre-defined query variants:
    • BasicNoOrder: No optional parameters (uses GitHub API's defaults)
    • BasicWithOrder: Only ordering parameters
    • WithCategoryNoOrder: Only category filtering
    • WithCategoryAndOrder: Both category and ordering
  • Added runtime query selection logic to choose the appropriate variant based on user input

Tests
I updated the tests indiscussions_test.go to match the new queries, and to test the new ordering functionality with different sorting combinations.

Demo

Simple "list discussions" promptScreenshot 2025-07-17 at 17 37 47
Ordering: from most recently created to least recently createdScreenshot 2025-07-17 at 17 43 14
Ordering: from least recently updated to most recently updatedScreenshot 2025-07-17 at 17 44 05
Less explicit prompt: show me discussions from last 10 daysScreenshot 2025-07-17 at 17 47 09
Ordering with weaker model (GPT-4o)Screenshot 2025-07-17 at 17 59 42

Note: this PR adds author data to the payload of thelist_discussions tool but doesn't add support for filtering by author when listing discussions because that is not supported on the discussions field in the API, and it would need to be achieved in a separate search tool.

tommaso-moroand others added17 commitsJuly 14, 2025 23:03
@tommaso-morotommaso-moro marked this pull request as ready for reviewJuly 17, 2025 17:03
CopilotAI review requested due to automatic review settingsJuly 17, 2025 17:03
@tommaso-morotommaso-moro requested a review froma team as acode ownerJuly 17, 2025 17:03
Copy link
Contributor

CopilotAI left a 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 thelist_discussions tool by adding ordering capabilities and expanding the discussion payload to include author information and updated timestamps. The changes enable users to sort discussions by creation or update date in ascending or descending order, while maintaining backward compatibility with existing functionality.

  • AddedupdatedAt andauthor.login fields to the discussion payload
  • Implemented ordering support withorderBy anddirection parameters
  • Refactored GraphQL query handling to support optional parameters through pre-defined query variants

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

FileDescription
pkg/github/discussions.goAdded new query struct types, ordering logic, and GraphQL query selection based on parameters
pkg/github/discussions_test.goUpdated test data and added comprehensive test cases for ordering functionality
README.mdUpdated documentation to reflect new ordering parameters

@LuluBeatsonLuluBeatson changed the titleTommy/expand-list-discussions-toolfeat: list_discussions sort by updatedAt & createdAt, return updatedAt and authorJul 21, 2025
@LuluBeatsonLuluBeatson self-assigned thisJul 21, 2025
@LuluBeatsonLuluBeatson added the enhancementNew feature or request labelJul 21, 2025
@LuluBeatson
Copy link
Contributor

Thanks@tommaso-moro for taking on this issue and refactoring the ListDiscussion GraphQL queries.

I have mergedmain into this branch, bringing GraphQL pagination to Discussion tools.

Plain List
list the last 20 discussions in facebook/react
image
List with Pagination
list all discussions in facebook/react with page size 3...Yes fetch the next page too
imageimage
List with Pagination & Reverse CreatedAt Sorting
list all discussions in facebook/react with page size 3 in reverse order of creation
image
List with Pagination & UpdatedAt Sorting
list all discussions in facebook/react with page size 3 in order of last update
image

@mattdholloway
Copy link
Contributor

mattdholloway commentedJul 22, 2025
edited
Loading

This looks great! The only potential issue I can see is with the following user flow:

  1. Gets page 1 with a given sort order eg. ordered by time created
  2. UsesendCursor from that response
  3. Makes another request with a new sort order

A potential improvement might be to force the pagination to be restarted when the sort order is changed so that the results are consistent. Perhaps updating the tool description forendCursor with a warning to the effect of 'this becomes invalid if the sort order changes' might help?

Copy link
Contributor

@mattdhollowaymattdholloway left a comment

Choose a reason for hiding this comment

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

lgtm

@LuluBeatsonLuluBeatson merged commita939565 intogithub:mainJul 24, 2025
10 checks passed
nickytonline pushed a commit to nickytonline/github-mcp-http that referenced this pull requestOct 4, 2025
…t and author (github#690)* added updatedAt and Author (aka User) login to query and payload* added initial support for orderby and direction* sort by created at instead of updated at by default* remove unused code* refactor to map to most suitable query based on user inputs at runtime* updated readme with new description* restore original categoryID code, simplify vars management* quick fix* update tests to account for recent changes (author login, updated at date)* use switch statement for better readability* remove comment* linting* refactored logic, simplified switch statement* linting* use original queries from discussions list tool for testing* linting* remove logging* Complete merge by re-introducing pagination to ListDiscussions* fix unit tests* refactor: less repetitive interface---------Co-authored-by: LuluBeatson <lulubeatson@github.com>Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

@mattdhollowaymattdhollowaymattdholloway approved these changes

@omgitsadsomgitsadsAwaiting requested review from omgitsads

Assignees

@LuluBeatsonLuluBeatson

Labels

enhancementNew feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Discussions toolset: add author.login, updated_at in response; and orderBy in parameters

3 participants

@tommaso-moro@LuluBeatson@mattdholloway

[8]ページ先頭

©2009-2025 Movatter.jp