adding review comments grouped as threads

[tonytrg]

A review thread like structure seems much more natural for the LLM to understand and build context on seperate review threads as they are naturally structured.
Closes:
#1086

[Copilot]

Pull request overview

This PR refactors the get_review_comments method to use GitHub's GraphQL API instead of REST, introducing support for review threads. Review threads group logically related comments on the same code location, providing additional metadata like isResolved, isOutdated, and isCollapsed status.

Key changes:

• Migrates from REST API's flat comment list to GraphQL's hierarchical review threads structure
• Updates PullRequestRead function signature to accept a GraphQL client getter
• Implements cursor-based pagination for threads (30 default, 100 max) with 50 comments per thread limit
• Adapts lockdown mode filtering to work at the thread level while preserving thread structure

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/github/pullrequests.go	Adds GraphQL types for review threads and reimplements GetPullRequestReviewComments to use GraphQL API, including new lockdown filtering logic
pkg/github/tools.go	Updates PullRequestRead initialization to pass GraphQL client getter
pkg/github/pullrequests_test.go	Refactors test cases to use GraphQL mocks, adds lockdown package import, and updates all test instantiations of PullRequestRead
pkg/github/__toolsnaps__/pull_request_read.snap	Updates tool schema description to reflect new review threads behavior and pagination approach
README.md	Updates documentation for get_review_comments method to describe the new thread-based structure

[Copilot]

The comment "Default to 100 threads if not specified" is misleading. The gqlParams.First will always be non-nil and set to at least 30 (the default from OptionalPaginationParams), so this code path will never default to 100. The default is actually 30, not 100.

Consider either:

1. Removing this comment and the default assignment (just use perPage := *gqlParams.First)
2. Or updating the comment to clarify: "Use value from pagination params (default 30 via OptionalPaginationParams), max is 100 for GraphQL"

Suggested change

	// Default to 100 threads if not specified, max is 100 for GraphQL
	perPage := int32(100)
	if gqlParams.First != nil && *gqlParams.First > 0 {
	perPage = *gqlParams.First
	}
	// Use value from pagination params (default 30 via OptionalPaginationParams), max is 100 for GraphQL
	perPage := *gqlParams.First

[Copilot]

After lockdown filtering, threads may end up with zero comments but are still included in the response. Consider filtering out empty threads after comment filtering to provide cleaner output:

// After filtering comments, remove threads with no remaining comments
filteredThreads := make([]reviewThreadNode, 0, len(query.Repository.PullRequest.ReviewThreads.Nodes))
for i := range query.Repository.PullRequest.ReviewThreads.Nodes {
    thread := &query.Repository.PullRequest.ReviewThreads.Nodes[i]
    if len(thread.Comments.Nodes) > 0 {
        filteredThreads = append(filteredThreads, *thread)
    }
}
query.Repository.PullRequest.ReviewThreads.Nodes = filteredThreads

This would be consistent with how GetPullRequestReviews filters out entire reviews in lockdown mode.

Suggested change

	}
	}
	// After filtering comments, remove threads with no remaining comments
	filteredThreads := make([]reviewThreadNode, 0, len(query.Repository.PullRequest.ReviewThreads.Nodes))
	for i := range query.Repository.PullRequest.ReviewThreads.Nodes {
	thread := &query.Repository.PullRequest.ReviewThreads.Nodes[i]
	if len(thread.Comments.Nodes) > 0 {
	filteredThreads = append(filteredThreads, *thread)
	}
	}
	query.Repository.PullRequest.ReviewThreads.Nodes = filteredThreads

[Copilot]

The response structure has inconsistent JSON key casing. The top-level map uses camelCase keys ("reviewThreads", "pageInfo", "totalCount"), but the nested structs (reviewThreadNode, reviewCommentNode, pageInfoFragment) don't have JSON tags, so they'll marshal to PascalCase by default.

This creates inconsistent output like:

{
  "reviewThreads": [
    {
      "ID": "...",              // PascalCase from struct
      "IsResolved": false,      // PascalCase from struct
      ...
    }
  ],
  "pageInfo": { ... },          // camelCase from map key
  "totalCount": 1               // camelCase from map key
}

Consider adding JSON tags to the struct fields to maintain consistent camelCase throughout:

type reviewThreadNode struct {
    ID          githubv4.ID      `graphql:"id" json:"id"`
    IsResolved  githubv4.Boolean `graphql:"isResolved" json:"isResolved"`
    // ... etc
}

[Copilot]

The hardcoded limit of 50 comments per thread may silently truncate threads with more comments. Consider documenting this limitation in the tool description or adding a field to the response indicating if comments were truncated.

For example, you could add to the thread response:

"commentsTruncated": len(thread.Comments.Nodes) >= 50

Or document in the tool description: "Note: Each thread returns at most 50 comments. Threads with more comments will be truncated."