Skip to content

fix: restore lexical search semantics for search_issues #3243

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lorenzozanee wants to merge 1 commit into from
Closed

fix: restore lexical search semantics for search_issues #3243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/github/__toolsnaps__/search_issues.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"readOnlyHint": true,
"title": "Search issues"
},
"description": "Search issues using natural-language semantic matching. Best for conceptual or paraphrased queries (e.g. \"login fails after password reset\"). Already scoped to is:issue.",
"description": "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue",
"inputSchema": {
"properties": {
"fields": {
Expand Down Expand Up @@ -64,7 +64,7 @@
"type": "number"
},
"query": {
"description": "The search query, as natural language. When the user gives alternative wordings, include them as plain words rather than joining them with OR.",
"description": "Search query using GitHub issues search syntax",
"type": "string"
},
"repo": {
Expand Down Expand Up @@ -95,4 +95,4 @@
"type": "object"
},
"name": "search_issues"
}
}
13 changes: 4 additions & 9 deletions pkg/github/issues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1816,15 +1816,10 @@ const (
)

// SearchIssues creates a tool to search for issues.
func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool {
cfg := newToolConfig(opts)

// Semantic is the default; however as it is not available on GHES, we fall back to
// lexical search for that host type.
mode := searchModeSemantic
if cfg.hostType == utils.HostTypeGHES {
mode = searchModeLexical
}
func SearchIssues(t translations.TranslationHelperFunc, _ ...ToolOption) inventory.ServerTool {
// Use GitHub's lexical issues-search contract on every supported host. Semantic
// matching does not preserve exact keyword and qualifier semantics.
mode := searchModeLexical

toolDescription := searchIssuesSemanticDescription
queryDescription := searchIssuesSemanticQueryDescription
Expand Down
110 changes: 68 additions & 42 deletions pkg/github/issues_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,22 @@ func Test_SearchIssues(t *testing.T) {
},
},
}
keywordSearchResult := &github.IssuesSearchResult{
Total: github.Ptr(1),
IncompleteResults: github.Ptr(true),
Issues: []*github.Issue{
{
Number: github.Ptr(44),
Title: github.Ptr("Transport timeout"),
Body: github.Ptr("The transport retries after a timeout"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/modelcontextprotocol/python-sdk/issues/44"),
User: &github.User{
Login: github.Ptr("user3"),
},
},
},
}

tests := []struct {
name string
Expand All @@ -1129,18 +1145,37 @@ func Test_SearchIssues(t *testing.T) {
expectedResult *github.IssuesSearchResult
expectedErrMsg string
}{
{
name: "keyword search preserves matching results and truncation",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue repo:modelcontextprotocol/python-sdk transport",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, keywordSearchResult),
),
}),
requestArgs: map[string]any{
"query": "repo:modelcontextprotocol/python-sdk transport",
},
expectError: false,
expectedResult: keywordSearchResult,
},
{
name: "successful issues search with all parameters",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue repo:owner/repo is:open",
"sort": "created",
"order": "desc",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue repo:owner/repo is:open",
"sort": "created",
"order": "desc",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1162,12 +1197,11 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "repo:test-owner/test-repo is:issue is:open",
"sort": "created",
"order": "asc",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "repo:test-owner/test-repo is:issue is:open",
"sort": "created",
"order": "asc",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1189,10 +1223,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue bug",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue bug",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1211,10 +1244,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue feature",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue feature",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand Down Expand Up @@ -1244,10 +1276,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1265,10 +1296,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue repo:github/github-mcp-server critical",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue repo:github/github-mcp-server critical",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1288,10 +1318,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue repo:octocat/Hello-World bug",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue repo:octocat/Hello-World bug",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1309,10 +1338,9 @@ func Test_SearchIssues(t *testing.T) {
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand All @@ -1333,7 +1361,6 @@ func Test_SearchIssues(t *testing.T) {
"q": "is:issue field.priority:P1",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"advanced_search": "true",
},
).andThen(
Expand All @@ -1347,15 +1374,14 @@ func Test_SearchIssues(t *testing.T) {
expectedResult: mockSearchResult,
},
{
name: "semantic search sets search_type",
name: "lexical search omits search_type",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:issue is:open",
"page": "1",
"per_page": "30",
"search_type": "semantic",
"q": "is:issue is:open",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
Expand Down
10 changes: 5 additions & 5 deletions pkg/github/search_semantic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,20 @@ func Test_searchIssuesTool_descriptionMatchesEngine(t *testing.T) {

// The description has to describe the engine the host will actually use.
// Steering a lexical-only host toward paraphrased natural language is actively misleading.
semantic := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeDotcom))
dotcom := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeDotcom))
lexical := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeGHES))

require.Equal(t, "search_issues", semantic.Tool.Name)
require.Equal(t, "search_issues", dotcom.Tool.Name)
require.Equal(t, "search_issues", lexical.Tool.Name)

assert.Equal(t, searchIssuesSemanticDescription, semantic.Tool.Description)
assert.Equal(t, searchIssuesLexicalDescription, dotcom.Tool.Description)
assert.Equal(t, searchIssuesLexicalDescription, lexical.Tool.Description)

semanticSchema, ok := semantic.Tool.InputSchema.(*jsonschema.Schema)
dotcomSchema, ok := dotcom.Tool.InputSchema.(*jsonschema.Schema)
require.True(t, ok)
lexicalSchema, ok := lexical.Tool.InputSchema.(*jsonschema.Schema)
require.True(t, ok)

assert.Equal(t, searchIssuesSemanticQueryDescription, semanticSchema.Properties["query"].Description)
assert.Equal(t, searchIssuesLexicalQueryDescription, dotcomSchema.Properties["query"].Description)
assert.Equal(t, searchIssuesLexicalQueryDescription, lexicalSchema.Properties["query"].Description)
}
10 changes: 4 additions & 6 deletions pkg/http/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,10 +923,8 @@ func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTo
return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx), nil
}

// TestStaticInventoryAppliesHostCapabilities guards against HTTP deployments
// silently getting dotcom behaviour. ServerConfig.Host can point at GHES, where
// semantic issue search 403s, so the static inventory has to classify the host
// rather than fall through to the zero value.
// TestStaticInventoryAppliesHostCapabilities guards the search contract exposed
// by HTTP deployments for both dotcom and enterprise hosts.
func TestStaticInventoryAppliesHostCapabilities(t *testing.T) {
t.Parallel()

Expand All @@ -938,12 +936,12 @@ func TestStaticInventoryAppliesHostCapabilities(t *testing.T) {
{
name: "empty host defaults to dotcom",
host: "",
wantDescription: "semantic",
wantDescription: "lexical",
},
{
name: "dotcom",
host: "https://github.com",
wantDescription: "semantic",
wantDescription: "lexical",
},
{
name: "GHES falls back to lexical",
Expand Down
4 changes: 2 additions & 2 deletions pkg/http/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ func TestInitGlobalToolScopeMapUsesHost(t *testing.T) {
want string
}{
{
name: "dotcom uses semantic search",
name: "dotcom uses lexical search",
hostType: utils.HostTypeDotcom,
want: "Search issues using natural-language semantic matching. Best for conceptual or paraphrased queries (e.g. \"login fails after password reset\"). Already scoped to is:issue.",
want: "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue",
},
{
name: "GHES uses lexical search",
Expand Down