diff --git a/server/search.go b/server/search.go index 90a92c6a..f85e8e08 100644 --- a/server/search.go +++ b/server/search.go @@ -179,7 +179,17 @@ func buildSynonymMap(groups [][]string) map[string][]string { // computeIDF builds the inverse document frequency map from the tool corpus // and pre-computes each tool's token set (stored on toolWithIntegration.tokens). // Only words appearing in at least one tool are indexed. -// Formula: IDF(word) = log(totalTools / toolsContainingWord) +// +// Formula: IDF(word) = log(totalTools / toolsContainingWord) + 1 +// +// The +1 is smoothing. Plain log(N/df) collapses to 0 when a word appears in +// every tool (log(N/N) = log(1) = 0). In a small corpus — a single-integration +// deployment, or an org whose policy narrows the catalog to one tool — that +// makes every candidate score 0, and scoreTools drops all of them, so search +// returns nothing even though the query clearly matched. Smoothing guarantees +// a matched token always contributes a positive floor while preserving the +// relative ordering (a rarer word still outscores a common one). +// // Missing keys at score time are treated as 0.0 (word contributes nothing). func computeIDF(tools []toolWithIntegration) map[string]float64 { total := len(tools) @@ -208,7 +218,7 @@ func computeIDF(tools []toolWithIntegration) map[string]float64 { idf := make(map[string]float64, len(docFreq)) for word, count := range docFreq { - idf[word] = math.Log(float64(total) / float64(count)) + idf[word] = math.Log(float64(total)/float64(count)) + 1 } return idf } diff --git a/server/search_test.go b/server/search_test.go index 2770dc6b..e63acf05 100644 --- a/server/search_test.go +++ b/server/search_test.go @@ -100,14 +100,11 @@ func TestComputeIDF(t *testing.T) { assert.Greater(t, deleteIDF, listIDF, "rare word 'delete' should have higher IDF than common word 'list'") }) - t.Run("word in all tools has zero IDF", func(t *testing.T) { - // Every tool has some word in common — check if any universal words get IDF=0 - // "item" or "a" might appear in all, but let's verify the math - // log(N/N) = log(1) = 0 + t.Run("word in all tools still has positive IDF", func(t *testing.T) { + // Smoothing (log(N/df)+1) keeps universal words above zero so a + // matched tool never scores 0 and gets dropped from results. for word, val := range idf { - if val == 0.0 { - t.Logf("Word %q has IDF=0 (appears in all tools)", word) - } + assert.Greater(t, val, 0.0, "word %q must have positive IDF after smoothing", word) } }) @@ -116,12 +113,37 @@ func TestComputeIDF(t *testing.T) { assert.Equal(t, 0.0, idf["nonexistent_word_xyz"]) }) - t.Run("IDF formula is log(total/count)", func(t *testing.T) { + t.Run("IDF formula is log(total/count)+1", func(t *testing.T) { // "delete" appears in exactly 1 tool (c_delete_item) - // IDF = log(10/1) ≈ 2.302 + // IDF = log(10/1) + 1 ≈ 3.302 deleteIDF := idf["delete"] - expected := math.Log(10.0 / 1.0) - assert.InDelta(t, expected, deleteIDF, 0.01, "IDF for 'delete' should be log(10/1)") + expected := math.Log(10.0/1.0) + 1 + assert.InDelta(t, expected, deleteIDF, 0.01, "IDF for 'delete' should be log(10/1)+1") + }) +} + +// TestComputeIDF_SmallCorpus is the regression for the degenerate case that +// made search return nothing: when a word appears in every tool, plain +// log(N/df) is 0. Smoothing must keep it positive. +func TestComputeIDF_SmallCorpus(t *testing.T) { + t.Run("single tool", func(t *testing.T) { + tools := []toolWithIntegration{ + {Integration: "echo", Tool: mcp.ToolDefinition{Name: mcp.ToolName("echo_ping"), Description: "ping the server"}}, + } + idf := computeIDF(tools) + for word, val := range idf { + assert.Greater(t, val, 0.0, "single-tool corpus: %q must have positive IDF", word) + } + }) + + t.Run("word shared by all tools", func(t *testing.T) { + tools := []toolWithIntegration{ + {Integration: "a", Tool: mcp.ToolDefinition{Name: mcp.ToolName("a_list_item"), Description: "list item"}}, + {Integration: "b", Tool: mcp.ToolDefinition{Name: mcp.ToolName("b_get_item"), Description: "get item"}}, + } + idf := computeIDF(tools) + // "item" appears in both tools: df == N. Must not be zero. + assert.Greater(t, idf["item"], 0.0, "universal word must not collapse to zero") }) } diff --git a/server/server_test.go b/server/server_test.go index 38f0cdf8..10ea3ce5 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -597,6 +597,47 @@ func TestHandleSearch_Pagination(t *testing.T) { } } +// TestHandleSearch_SmallCatalogReturnsMatches is the end-to-end regression +// for the IDF smoothing fix. With a tiny catalog every query token appears +// in "every" tool, so unsmoothed TF-IDF scored all candidates 0 and search +// returned nothing. A matching query must now return the matching tools. +func TestHandleSearch_SmallCatalogReturnsMatches(t *testing.T) { + t.Run("single tool", func(t *testing.T) { + mi := &mockIntegration{ + name: "echo", + healthy: true, + tools: []mcp.ToolDefinition{ + {Name: mcp.ToolName("echo_ping"), Description: "ping the server"}, + }, + } + s := setupTestServer(mi) + + result, err := s.handleSearch(context.Background(), searchRequest(map[string]any{"query": "ping"})) + require.NoError(t, err) + resp := parseSearchResponse(t, result) + assert.Equal(t, 1, resp.Total, "single-tool catalog must still surface a matching tool") + assert.Contains(t, searchToolNames(t, resp), "echo_ping") + }) + + t.Run("two tools sharing a word", func(t *testing.T) { + mi := &mockIntegration{ + name: "widgets", + healthy: true, + tools: []mcp.ToolDefinition{ + {Name: mcp.ToolName("widgets_list_item"), Description: "list item"}, + {Name: mcp.ToolName("widgets_get_item"), Description: "get item"}, + }, + } + s := setupTestServer(mi) + + // "item" appears in both tools (df == N). Must still match both. + result, err := s.handleSearch(context.Background(), searchRequest(map[string]any{"query": "item"})) + require.NoError(t, err) + resp := parseSearchResponse(t, result) + assert.Equal(t, 2, resp.Total, "a word shared by all tools must still return them") + }) +} + func TestHandleSearch_QueryFiltersCombinedWithPagination(t *testing.T) { mi := &mockIntegration{ name: "testint",