Skip to content

Commit 86a41c2

Browse files
kerobbikerobbi
committed
Merge branch 'main' into kerobbi/response-optimisation
2 parents de12f8f + ee3ab7b commit 86a41c2

File tree

3 files changed

+153
-67
lines changed

3 files changed

+153
-67
lines changed

‎pkg/github/minimal_types.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,18 @@ type MinimalCommitFile struct {
7979
Changes int `json:"changes,omitempty"`
8080
}
8181

82+
// MinimalPRFile represents a file changed in a pull request.
83+
// Compared to MinimalCommitFile, it includes the patch diff and previous filename for renames.
84+
type MinimalPRFile struct {
85+
Filename string `json:"filename"`
86+
Status string `json:"status,omitempty"`
87+
Additions int `json:"additions,omitempty"`
88+
Deletions int `json:"deletions,omitempty"`
89+
Changes int `json:"changes,omitempty"`
90+
Patch string `json:"patch,omitempty"`
91+
PreviousFilename string `json:"previous_filename,omitempty"`
92+
}
93+
8294
// MinimalCommit is the trimmed output type for commit objects.
8395
type MinimalCommit struct {
8496
SHA string `json:"sha"`
@@ -600,6 +612,57 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
600612
return minimalCommit
601613
}
602614

615+
// MinimalPageInfo contains pagination cursor information.
616+
type MinimalPageInfo struct {
617+
HasNextPage bool `json:"has_next_page"`
618+
HasPreviousPage bool `json:"has_previous_page"`
619+
StartCursor string `json:"start_cursor,omitempty"`
620+
EndCursor string `json:"end_cursor,omitempty"`
621+
}
622+
623+
// MinimalReviewComment is the trimmed output type for PR review comment objects.
624+
type MinimalReviewComment struct {
625+
Body string `json:"body,omitempty"`
626+
Path string `json:"path"`
627+
Line *int `json:"line,omitempty"`
628+
Author string `json:"author,omitempty"`
629+
CreatedAt string `json:"created_at,omitempty"`
630+
UpdatedAt string `json:"updated_at,omitempty"`
631+
HTMLURL string `json:"html_url"`
632+
}
633+
634+
// MinimalReviewThread is the trimmed output type for PR review thread objects.
635+
type MinimalReviewThread struct {
636+
IsResolved bool `json:"is_resolved"`
637+
IsOutdated bool `json:"is_outdated"`
638+
IsCollapsed bool `json:"is_collapsed"`
639+
Comments []MinimalReviewComment `json:"comments"`
640+
TotalCount int `json:"total_count"`
641+
}
642+
643+
// MinimalReviewThreadsResponse is the trimmed output for a paginated list of PR review threads.
644+
type MinimalReviewThreadsResponse struct {
645+
ReviewThreads []MinimalReviewThread `json:"review_threads"`
646+
TotalCount int `json:"total_count"`
647+
PageInfo MinimalPageInfo `json:"page_info"`
648+
}
649+
650+
func convertToMinimalPRFiles(files []*github.CommitFile) []MinimalPRFile {
651+
result := make([]MinimalPRFile, 0, len(files))
652+
for _, f := range files {
653+
result = append(result, MinimalPRFile{
654+
Filename: f.GetFilename(),
655+
Status: f.GetStatus(),
656+
Additions: f.GetAdditions(),
657+
Deletions: f.GetDeletions(),
658+
Changes: f.GetChanges(),
659+
Patch: f.GetPatch(),
660+
PreviousFilename: f.GetPreviousFilename(),
661+
})
662+
}
663+
return result
664+
}
665+
603666
// convertToMinimalBranch converts a GitHub API Branch to MinimalBranch
604667
func convertToMinimalBranch(branch *github.Branch) MinimalBranch {
605668
return MinimalBranch{
@@ -608,3 +671,61 @@ func convertToMinimalBranch(branch *github.Branch) MinimalBranch {
608671
Protected: branch.GetProtected(),
609672
}
610673
}
674+
675+
func convertToMinimalReviewThreadsResponse(query reviewThreadsQuery) MinimalReviewThreadsResponse {
676+
threads := query.Repository.PullRequest.ReviewThreads
677+
678+
minimalThreads := make([]MinimalReviewThread, 0, len(threads.Nodes))
679+
for _, thread := range threads.Nodes {
680+
minimalThreads = append(minimalThreads, convertToMinimalReviewThread(thread))
681+
}
682+
683+
return MinimalReviewThreadsResponse{
684+
ReviewThreads: minimalThreads,
685+
TotalCount: int(threads.TotalCount),
686+
PageInfo: MinimalPageInfo{
687+
HasNextPage: bool(threads.PageInfo.HasNextPage),
688+
HasPreviousPage: bool(threads.PageInfo.HasPreviousPage),
689+
StartCursor: string(threads.PageInfo.StartCursor),
690+
EndCursor: string(threads.PageInfo.EndCursor),
691+
},
692+
}
693+
}
694+
695+
func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread {
696+
comments := make([]MinimalReviewComment, 0, len(thread.Comments.Nodes))
697+
for _, c := range thread.Comments.Nodes {
698+
comments = append(comments, convertToMinimalReviewComment(c))
699+
}
700+
701+
return MinimalReviewThread{
702+
IsResolved: bool(thread.IsResolved),
703+
IsOutdated: bool(thread.IsOutdated),
704+
IsCollapsed: bool(thread.IsCollapsed),
705+
Comments: comments,
706+
TotalCount: int(thread.Comments.TotalCount),
707+
}
708+
}
709+
710+
func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment {
711+
m := MinimalReviewComment{
712+
Body: string(c.Body),
713+
Path: string(c.Path),
714+
Author: string(c.Author.Login),
715+
HTMLURL: c.URL.String(),
716+
}
717+
718+
if c.Line != nil {
719+
line := int(*c.Line)
720+
m.Line = &line
721+
}
722+
723+
if !c.CreatedAt.IsZero() {
724+
m.CreatedAt = c.CreatedAt.Format(time.RFC3339)
725+
}
726+
if !c.UpdatedAt.IsZero() {
727+
m.UpdatedAt = c.UpdatedAt.Format(time.RFC3339)
728+
}
729+
730+
return m
731+
}

‎pkg/github/pullrequests.go‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,9 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo
291291
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request files", resp, body), nil
292292
}
293293

294-
r, err := json.Marshal(files)
295-
if err != nil {
296-
return nil, fmt.Errorf("failed to marshal response: %w", err)
297-
}
294+
minimalFiles := convertToMinimalPRFiles(files)
298295

299-
return utils.NewToolResultText(string(r)), nil
296+
return MarshalledTextResult(minimalFiles), nil
300297
}
301298

302299
// GraphQL types for review threads query
@@ -410,24 +407,7 @@ func GetPullRequestReviewComments(ctx context.Context, gqlClient *githubv4.Clien
410407
}
411408
}
412409

413-
// Build response with review threads and pagination info
414-
response := map[string]any{
415-
"reviewThreads": query.Repository.PullRequest.ReviewThreads.Nodes,
416-
"pageInfo": map[string]any{
417-
"hasNextPage": query.Repository.PullRequest.ReviewThreads.PageInfo.HasNextPage,
418-
"hasPreviousPage": query.Repository.PullRequest.ReviewThreads.PageInfo.HasPreviousPage,
419-
"startCursor": string(query.Repository.PullRequest.ReviewThreads.PageInfo.StartCursor),
420-
"endCursor": string(query.Repository.PullRequest.ReviewThreads.PageInfo.EndCursor),
421-
},
422-
"totalCount": int(query.Repository.PullRequest.ReviewThreads.TotalCount),
423-
}
424-
425-
r, err := json.Marshal(response)
426-
if err != nil {
427-
return nil, fmt.Errorf("failed to marshal response: %w", err)
428-
}
429-
430-
return utils.NewToolResultText(string(r)), nil
410+
return MarshalledTextResult(convertToMinimalReviewThreadsResponse(query)), nil
431411
}
432412

433413
func GetPullRequestReviews(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {

‎pkg/github/pullrequests_test.go‎

Lines changed: 29 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,15 +1229,15 @@ func Test_GetPullRequestFiles(t *testing.T) {
12291229
textContent := getTextResult(t, result)
12301230

12311231
// Unmarshal and verify the result
1232-
var returnedFiles []*github.CommitFile
1232+
var returnedFiles []MinimalPRFile
12331233
err = json.Unmarshal([]byte(textContent.Text), &returnedFiles)
12341234
require.NoError(t, err)
12351235
assert.Len(t, returnedFiles, len(tc.expectedFiles))
12361236
for i, file := range returnedFiles {
1237-
assert.Equal(t, *tc.expectedFiles[i].Filename, *file.Filename)
1238-
assert.Equal(t, *tc.expectedFiles[i].Status, *file.Status)
1239-
assert.Equal(t, *tc.expectedFiles[i].Additions, *file.Additions)
1240-
assert.Equal(t, *tc.expectedFiles[i].Deletions, *file.Deletions)
1237+
assert.Equal(t, tc.expectedFiles[i].GetFilename(), file.Filename)
1238+
assert.Equal(t, tc.expectedFiles[i].GetStatus(), file.Status)
1239+
assert.Equal(t, tc.expectedFiles[i].GetAdditions(), file.Additions)
1240+
assert.Equal(t, tc.expectedFiles[i].GetDeletions(), file.Deletions)
12411241
}
12421242
})
12431243
}
@@ -1619,45 +1619,35 @@ func Test_GetPullRequestComments(t *testing.T) {
16191619
},
16201620
expectError: false,
16211621
validateResult: func(t *testing.T, textContent string) {
1622-
var result map[string]any
1622+
var result MinimalReviewThreadsResponse
16231623
err := json.Unmarshal([]byte(textContent), &result)
16241624
require.NoError(t, err)
16251625

1626-
// Validate response structure
1627-
assert.Contains(t, result, "reviewThreads")
1628-
assert.Contains(t, result, "pageInfo")
1629-
assert.Contains(t, result, "totalCount")
1630-
16311626
// Validate review threads
1632-
threads := result["reviewThreads"].([]any)
1633-
assert.Len(t, threads, 1)
1627+
assert.Len(t, result.ReviewThreads, 1)
16341628

1635-
thread := threads[0].(map[string]any)
1636-
assert.Equal(t, "RT_kwDOA0xdyM4AX1Yz", thread["ID"])
1637-
assert.Equal(t, false, thread["IsResolved"])
1638-
assert.Equal(t, false, thread["IsOutdated"])
1639-
assert.Equal(t, false, thread["IsCollapsed"])
1629+
thread := result.ReviewThreads[0]
1630+
assert.Equal(t, false, thread.IsResolved)
1631+
assert.Equal(t, false, thread.IsOutdated)
1632+
assert.Equal(t, false, thread.IsCollapsed)
16401633

16411634
// Validate comments within thread
1642-
comments := thread["Comments"].(map[string]any)
1643-
commentNodes := comments["Nodes"].([]any)
1644-
assert.Len(t, commentNodes, 2)
1635+
assert.Len(t, thread.Comments, 2)
16451636

16461637
// Validate first comment
1647-
comment1 := commentNodes[0].(map[string]any)
1648-
assert.Equal(t, "PRRC_kwDOA0xdyM4AX1Y0", comment1["ID"])
1649-
assert.Equal(t, "This looks good", comment1["Body"])
1650-
assert.Equal(t, "file1.go", comment1["Path"])
1638+
comment1 := thread.Comments[0]
1639+
assert.Equal(t, "This looks good", comment1.Body)
1640+
assert.Equal(t, "file1.go", comment1.Path)
1641+
assert.Equal(t, "reviewer1", comment1.Author)
16511642

16521643
// Validate pagination info
1653-
pageInfo := result["pageInfo"].(map[string]any)
1654-
assert.Equal(t, false, pageInfo["hasNextPage"])
1655-
assert.Equal(t, false, pageInfo["hasPreviousPage"])
1656-
assert.Equal(t, "cursor1", pageInfo["startCursor"])
1657-
assert.Equal(t, "cursor2", pageInfo["endCursor"])
1644+
assert.Equal(t, false, result.PageInfo.HasNextPage)
1645+
assert.Equal(t, false, result.PageInfo.HasPreviousPage)
1646+
assert.Equal(t, "cursor1", result.PageInfo.StartCursor)
1647+
assert.Equal(t, "cursor2", result.PageInfo.EndCursor)
16581648

16591649
// Validate total count
1660-
assert.Equal(t, float64(1), result["totalCount"])
1650+
assert.Equal(t, 1, result.TotalCount)
16611651
},
16621652
},
16631653
{
@@ -1761,27 +1751,22 @@ func Test_GetPullRequestComments(t *testing.T) {
17611751
expectError: false,
17621752
lockdownEnabled: true,
17631753
validateResult: func(t *testing.T, textContent string) {
1764-
var result map[string]any
1754+
var result MinimalReviewThreadsResponse
17651755
err := json.Unmarshal([]byte(textContent), &result)
17661756
require.NoError(t, err)
17671757

17681758
// Validate that only maintainer comment is returned
1769-
threads := result["reviewThreads"].([]any)
1770-
assert.Len(t, threads, 1)
1759+
assert.Len(t, result.ReviewThreads, 1)
17711760

1772-
thread := threads[0].(map[string]any)
1773-
comments := thread["Comments"].(map[string]any)
1761+
thread := result.ReviewThreads[0]
17741762

17751763
// Should only have 1 comment (maintainer) after filtering
1776-
assert.Equal(t, float64(1), comments["TotalCount"])
1777-
1778-
commentNodes := comments["Nodes"].([]any)
1779-
assert.Len(t, commentNodes, 1)
1764+
assert.Equal(t, 1, thread.TotalCount)
1765+
assert.Len(t, thread.Comments, 1)
17801766

1781-
comment := commentNodes[0].(map[string]any)
1782-
author := comment["Author"].(map[string]any)
1783-
assert.Equal(t, "maintainer", author["Login"])
1784-
assert.Equal(t, "Maintainer review comment", comment["Body"])
1767+
comment := thread.Comments[0]
1768+
assert.Equal(t, "maintainer", comment.Author)
1769+
assert.Equal(t, "Maintainer review comment", comment.Body)
17851770
},
17861771
},
17871772
}

0 commit comments

Comments
 (0)