Skip to content

Commit f11bf9a

Browse files
SamMorrowDrumsCopilot
SamMorrowDrums
and
Copilot
committed
fix: add minimal_output opt-out and Accept header test for code search
Address PR review feedback: 1. Add minimal_output parameter (default: true) to search_code, matching the pattern from search_repositories. When false, returns the full GitHub API CodeSearchResult for backward compatibility. 2. Add Accept header assertion to tests via a new withHeaders() helper on partialMock, verifying the text-match Accept header is actually requested (not just mocked in the response). 3. Add test case for minimal_output=false path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7073022 commit f11bf9a

5 files changed

Lines changed: 123 additions & 46 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,7 @@ The following sets of tools are available:
12731273

12741274
- **search_code** - Search code
12751275
- **Required OAuth Scopes**: `repo`
1276+
- `minimal_output`: Return minimal repository information (default: true). When false, returns full GitHub API repository objects. (boolean, optional)
12761277
- `order`: Sort order for results (string, optional)
12771278
- `page`: Page number for pagination (min 1) (number, optional)
12781279
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)

‎pkg/github/__toolsnaps__/search_code.snap‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
"description": "Fast and precise code search across ALL GitHub repositories using GitHub's native search engine. Best for finding exact symbols, functions, classes, or specific code patterns.",
77
"inputSchema": {
88
"properties": {
9+
"minimal_output": {
10+
"default": true,
11+
"description": "Return minimal repository information (default: true). When false, returns full GitHub API repository objects.",
12+
"type": "boolean"
13+
},
914
"order": {
1015
"description": "Sort order for results",
1116
"enum": [

‎pkg/github/helper_test.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,15 @@ func expectRequestBody(t *testing.T, expectedRequestBody any) *partialMock {
220220
type partialMock struct {
221221
t *testing.T
222222

223-
expectedPath string
224-
expectedQueryParams map[string]string
225-
expectedRequestBody any
223+
expectedPath string
224+
expectedQueryParams map[string]string
225+
expectedRequestBody any
226+
expectedHeaderContains map[string]string
227+
}
228+
229+
func (p *partialMock) withHeaders(headers map[string]string) *partialMock {
230+
p.expectedHeaderContains = headers
231+
return p
226232
}
227233

228234
func (p *partialMock) andThen(responseHandler http.HandlerFunc) http.HandlerFunc {
@@ -247,6 +253,12 @@ func (p *partialMock) andThen(responseHandler http.HandlerFunc) http.HandlerFunc
247253
require.Equal(p.t, p.expectedRequestBody, unmarshaledRequestBody)
248254
}
249255

256+
if p.expectedHeaderContains != nil {
257+
for k, v := range p.expectedHeaderContains {
258+
require.Contains(p.t, r.Header.Get(k), v, "expected header %q to contain %q", k, v)
259+
}
260+
}
261+
250262
responseHandler(w, r)
251263
}
252264
}

‎pkg/github/search.go‎

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,11 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool {
234234
Description: "Sort order for results",
235235
Enum: []any{"asc", "desc"},
236236
},
237+
"minimal_output": {
238+
Type: "boolean",
239+
Description: "Return minimal repository information (default: true). When false, returns full GitHub API repository objects.",
240+
Default: json.RawMessage(`true`),
241+
},
237242
},
238243
Required: []string{"query"},
239244
}
@@ -268,6 +273,10 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool {
268273
if err != nil {
269274
return utils.NewToolResultError(err.Error()), nil, nil
270275
}
276+
minimalOutput, err := OptionalBoolParamWithDefault(args, "minimal_output", true)
277+
if err != nil {
278+
return utils.NewToolResultError(err.Error()), nil, nil
279+
}
271280

272281
opts := &github.SearchOptions{
273282
Sort: sort,
@@ -302,29 +311,37 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool {
302311
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search code", resp, body), nil, nil
303312
}
304313

305-
minimalItems := make([]MinimalCodeResult, 0, len(result.CodeResults))
306-
for _, code := range result.CodeResults {
307-
item := MinimalCodeResult{
308-
Name: code.GetName(),
309-
Path: code.GetPath(),
310-
SHA: code.GetSHA(),
311-
TextMatches: code.TextMatches,
312-
}
313-
if code.Repository != nil {
314-
item.Repository = code.Repository.GetFullName()
314+
var r []byte
315+
if minimalOutput {
316+
minimalItems := make([]MinimalCodeResult, 0, len(result.CodeResults))
317+
for _, code := range result.CodeResults {
318+
item := MinimalCodeResult{
319+
Name: code.GetName(),
320+
Path: code.GetPath(),
321+
SHA: code.GetSHA(),
322+
TextMatches: code.TextMatches,
323+
}
324+
if code.Repository != nil {
325+
item.Repository = code.Repository.GetFullName()
326+
}
327+
minimalItems = append(minimalItems, item)
315328
}
316-
minimalItems = append(minimalItems, item)
317-
}
318329

319-
minimalResult := &MinimalCodeSearchResult{
320-
TotalCount: result.GetTotal(),
321-
IncompleteResults: result.GetIncompleteResults(),
322-
Items: minimalItems,
323-
}
330+
minimalResult := &MinimalCodeSearchResult{
331+
TotalCount: result.GetTotal(),
332+
IncompleteResults: result.GetIncompleteResults(),
333+
Items: minimalItems,
334+
}
324335

325-
r, err := json.Marshal(minimalResult)
326-
if err != nil {
327-
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
336+
r, err = json.Marshal(minimalResult)
337+
if err != nil {
338+
return utils.NewToolResultErrorFromErr("failed to marshal minimal response", err), nil, nil
339+
}
340+
} else {
341+
r, err = json.Marshal(result)
342+
if err != nil {
343+
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
344+
}
328345
}
329346

330347
return utils.NewToolResultText(string(r)), nil, nil

‎pkg/github/search_test.go‎

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ func Test_SearchCode(t *testing.T) {
422422
assert.Contains(t, schema.Properties, "order")
423423
assert.Contains(t, schema.Properties, "perPage")
424424
assert.Contains(t, schema.Properties, "page")
425+
assert.Contains(t, schema.Properties, "minimal_output")
425426
assert.ElementsMatch(t, schema.Required, []string{"query"})
426427

427428
// Setup mock search results
@@ -455,13 +456,18 @@ func Test_SearchCode(t *testing.T) {
455456
},
456457
}
457458

459+
textMatchAcceptHeader := map[string]string{
460+
"Accept": "text-match",
461+
}
462+
458463
tests := []struct {
459464
name string
460465
mockedClient *http.Client
461466
requestArgs map[string]any
462467
expectError bool
463468
expectedResult *github.CodeSearchResult
464469
expectedErrMsg string
470+
minimalOutput bool
465471
}{
466472
{
467473
name: "successful code search with all parameters",
@@ -472,7 +478,7 @@ func Test_SearchCode(t *testing.T) {
472478
"order": "desc",
473479
"page": "1",
474480
"per_page": "30",
475-
}).andThen(
481+
}).withHeaders(textMatchAcceptHeader).andThen(
476482
mockResponse(t, http.StatusOK, mockSearchResult),
477483
),
478484
}),
@@ -485,6 +491,7 @@ func Test_SearchCode(t *testing.T) {
485491
},
486492
expectError: false,
487493
expectedResult: mockSearchResult,
494+
minimalOutput: true,
488495
},
489496
{
490497
name: "code search with minimal parameters",
@@ -493,7 +500,7 @@ func Test_SearchCode(t *testing.T) {
493500
"q": "fmt.Println language:go",
494501
"page": "1",
495502
"per_page": "30",
496-
}).andThen(
503+
}).withHeaders(textMatchAcceptHeader).andThen(
497504
mockResponse(t, http.StatusOK, mockSearchResult),
498505
),
499506
}),
@@ -502,6 +509,26 @@ func Test_SearchCode(t *testing.T) {
502509
},
503510
expectError: false,
504511
expectedResult: mockSearchResult,
512+
minimalOutput: true,
513+
},
514+
{
515+
name: "code search with full output",
516+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
517+
GetSearchCode: expectQueryParams(t, map[string]string{
518+
"q": "fmt.Println language:go",
519+
"page": "1",
520+
"per_page": "30",
521+
}).withHeaders(textMatchAcceptHeader).andThen(
522+
mockResponse(t, http.StatusOK, mockSearchResult),
523+
),
524+
}),
525+
requestArgs: map[string]any{
526+
"query": "fmt.Println language:go",
527+
"minimal_output": false,
528+
},
529+
expectError: false,
530+
expectedResult: mockSearchResult,
531+
minimalOutput: false,
505532
},
506533
{
507534
name: "search code fails",
@@ -546,30 +573,45 @@ func Test_SearchCode(t *testing.T) {
546573
require.NoError(t, err)
547574
require.False(t, result.IsError)
548575

549-
// Parse the result and get the text content if no error
550576
textContent := getTextResult(t, result)
551577

552-
// Unmarshal and verify the minimal result
553-
var returnedResult MinimalCodeSearchResult
554-
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
555-
require.NoError(t, err)
556-
assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount)
557-
assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults)
558-
assert.Len(t, returnedResult.Items, len(tc.expectedResult.CodeResults))
559-
for i, code := range returnedResult.Items {
560-
assert.Equal(t, tc.expectedResult.CodeResults[i].GetName(), code.Name)
561-
assert.Equal(t, tc.expectedResult.CodeResults[i].GetPath(), code.Path)
562-
assert.Equal(t, tc.expectedResult.CodeResults[i].GetSHA(), code.SHA)
563-
assert.Equal(t, tc.expectedResult.CodeResults[i].Repository.GetFullName(), code.Repository)
564-
}
578+
if tc.minimalOutput {
579+
// Unmarshal and verify the minimal result
580+
var returnedResult MinimalCodeSearchResult
581+
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
582+
require.NoError(t, err)
583+
assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount)
584+
assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults)
585+
assert.Len(t, returnedResult.Items, len(tc.expectedResult.CodeResults))
586+
for i, code := range returnedResult.Items {
587+
assert.Equal(t, tc.expectedResult.CodeResults[i].GetName(), code.Name)
588+
assert.Equal(t, tc.expectedResult.CodeResults[i].GetPath(), code.Path)
589+
assert.Equal(t, tc.expectedResult.CodeResults[i].GetSHA(), code.SHA)
590+
assert.Equal(t, tc.expectedResult.CodeResults[i].Repository.GetFullName(), code.Repository)
591+
}
565592

566-
// Verify text matches are included when present
567-
if len(tc.expectedResult.CodeResults[0].TextMatches) > 0 {
568-
require.NotEmpty(t, returnedResult.Items[0].TextMatches)
569-
assert.Equal(t,
570-
tc.expectedResult.CodeResults[0].TextMatches[0].GetFragment(),
571-
returnedResult.Items[0].TextMatches[0].GetFragment(),
572-
)
593+
// Verify text matches are included when present
594+
if len(tc.expectedResult.CodeResults[0].TextMatches) > 0 {
595+
require.NotEmpty(t, returnedResult.Items[0].TextMatches)
596+
assert.Equal(t,
597+
tc.expectedResult.CodeResults[0].TextMatches[0].GetFragment(),
598+
returnedResult.Items[0].TextMatches[0].GetFragment(),
599+
)
600+
}
601+
} else {
602+
// Verify the full result is returned
603+
var returnedResult github.CodeSearchResult
604+
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
605+
require.NoError(t, err)
606+
assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total)
607+
assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults)
608+
assert.Len(t, returnedResult.CodeResults, len(tc.expectedResult.CodeResults))
609+
for i, code := range returnedResult.CodeResults {
610+
assert.Equal(t, *tc.expectedResult.CodeResults[i].Name, *code.Name)
611+
assert.Equal(t, *tc.expectedResult.CodeResults[i].Path, *code.Path)
612+
assert.Equal(t, *tc.expectedResult.CodeResults[i].SHA, *code.SHA)
613+
assert.Equal(t, *tc.expectedResult.CodeResults[i].Repository.FullName, *code.Repository.FullName)
614+
}
573615
}
574616
})
575617
}

0 commit comments

Comments
 (0)