Skip to content

Commit e428b7a

Browse files
committed
verify fixes are working
Signed-off-by: Huamin Chen <[email protected]>
1 parent 30a4032 commit e428b7a

File tree

2 files changed

+157
-9
lines changed

2 files changed

+157
-9
lines changed

src/semantic-router/pkg/extproc/test_utils_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func CreateTestConfig() *config.RouterConfig {
9595
Args []string `yaml:"args,omitempty"`
9696
Env map[string]string `yaml:"env,omitempty"`
9797
URL string `yaml:"url,omitempty"`
98-
ToolName string `yaml:"tool_name"`
98+
ToolName string `yaml:"tool_name,omitempty"`
9999
Threshold float32 `yaml:"threshold"`
100100
TimeoutSeconds int `yaml:"timeout_seconds,omitempty"`
101101
} `yaml:"mcp_category_model,omitempty"`
@@ -125,7 +125,7 @@ func CreateTestConfig() *config.RouterConfig {
125125
Args []string `yaml:"args,omitempty"`
126126
Env map[string]string `yaml:"env,omitempty"`
127127
URL string `yaml:"url,omitempty"`
128-
ToolName string `yaml:"tool_name"`
128+
ToolName string `yaml:"tool_name,omitempty"`
129129
Threshold float32 `yaml:"threshold"`
130130
TimeoutSeconds int `yaml:"timeout_seconds,omitempty"`
131131
}{

src/semantic-router/pkg/utils/classification/mcp_classifier_test.go

Lines changed: 155 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type MockMCPClient struct {
1919
callToolError error
2020
closeError error
2121
connected bool
22+
getToolsResult []mcp.Tool
2223
}
2324

2425
func (m *MockMCPClient) Connect() error {
@@ -46,7 +47,7 @@ func (m *MockMCPClient) Ping(ctx context.Context) error {
4647
}
4748

4849
func (m *MockMCPClient) GetTools() []mcp.Tool {
49-
return nil
50+
return m.getToolsResult
5051
}
5152

5253
func (m *MockMCPClient) GetResources() []mcp.Resource {
@@ -119,17 +120,164 @@ var _ = Describe("MCP Category Classifier", func() {
119120
})
120121
})
121122

122-
Context("when tool name is not specified", func() {
123-
It("should return error", func() {
123+
// Note: tool_name is now optional and will be auto-discovered if not specified.
124+
// The Init method will automatically discover classification tools from the MCP server
125+
// by calling discoverClassificationTool().
126+
127+
// Note: Full initialization test requires mocking NewClient and GetTools which is complex
128+
// In real tests, we'd need dependency injection for the client factory
129+
})
130+
131+
Describe("discoverClassificationTool", func() {
132+
BeforeEach(func() {
133+
mcpClassifier.client = mockClient
134+
mcpClassifier.config = cfg
135+
})
136+
137+
Context("when tool name is explicitly configured", func() {
138+
It("should use the configured tool name", func() {
139+
cfg.Classifier.MCPCategoryModel.ToolName = "my_classifier"
140+
err := mcpClassifier.discoverClassificationTool()
141+
Expect(err).ToNot(HaveOccurred())
142+
Expect(mcpClassifier.toolName).To(Equal("my_classifier"))
143+
})
144+
})
145+
146+
Context("when tool name is not configured", func() {
147+
BeforeEach(func() {
124148
cfg.Classifier.MCPCategoryModel.ToolName = ""
125-
err := mcpClassifier.Init(cfg)
149+
})
150+
151+
It("should discover classify_text tool", func() {
152+
mockClient.getToolsResult = []mcp.Tool{
153+
{Name: "some_other_tool", Description: "Other tool"},
154+
{Name: "classify_text", Description: "Classifies text into categories"},
155+
}
156+
err := mcpClassifier.discoverClassificationTool()
157+
Expect(err).ToNot(HaveOccurred())
158+
Expect(mcpClassifier.toolName).To(Equal("classify_text"))
159+
})
160+
161+
It("should discover classify tool", func() {
162+
mockClient.getToolsResult = []mcp.Tool{
163+
{Name: "classify", Description: "Classify text"},
164+
}
165+
err := mcpClassifier.discoverClassificationTool()
166+
Expect(err).ToNot(HaveOccurred())
167+
Expect(mcpClassifier.toolName).To(Equal("classify"))
168+
})
169+
170+
It("should discover categorize tool", func() {
171+
mockClient.getToolsResult = []mcp.Tool{
172+
{Name: "categorize", Description: "Categorize text"},
173+
}
174+
err := mcpClassifier.discoverClassificationTool()
175+
Expect(err).ToNot(HaveOccurred())
176+
Expect(mcpClassifier.toolName).To(Equal("categorize"))
177+
})
178+
179+
It("should discover categorize_text tool", func() {
180+
mockClient.getToolsResult = []mcp.Tool{
181+
{Name: "categorize_text", Description: "Categorize text into categories"},
182+
}
183+
err := mcpClassifier.discoverClassificationTool()
184+
Expect(err).ToNot(HaveOccurred())
185+
Expect(mcpClassifier.toolName).To(Equal("categorize_text"))
186+
})
187+
188+
It("should prioritize classify_text over other common names", func() {
189+
mockClient.getToolsResult = []mcp.Tool{
190+
{Name: "categorize", Description: "Categorize"},
191+
{Name: "classify_text", Description: "Main classifier"},
192+
{Name: "classify", Description: "Classify"},
193+
}
194+
err := mcpClassifier.discoverClassificationTool()
195+
Expect(err).ToNot(HaveOccurred())
196+
Expect(mcpClassifier.toolName).To(Equal("classify_text"))
197+
})
198+
199+
It("should prefer common names over pattern matching", func() {
200+
mockClient.getToolsResult = []mcp.Tool{
201+
{Name: "my_classification_tool", Description: "Custom classifier"},
202+
{Name: "classify", Description: "Built-in classifier"},
203+
}
204+
err := mcpClassifier.discoverClassificationTool()
205+
Expect(err).ToNot(HaveOccurred())
206+
Expect(mcpClassifier.toolName).To(Equal("classify"))
207+
})
208+
209+
It("should discover by pattern matching in name", func() {
210+
mockClient.getToolsResult = []mcp.Tool{
211+
{Name: "text_classification", Description: "Some description"},
212+
}
213+
err := mcpClassifier.discoverClassificationTool()
214+
Expect(err).ToNot(HaveOccurred())
215+
Expect(mcpClassifier.toolName).To(Equal("text_classification"))
216+
})
217+
218+
It("should discover by pattern matching in description", func() {
219+
mockClient.getToolsResult = []mcp.Tool{
220+
{Name: "analyze_text", Description: "Tool for text classification"},
221+
}
222+
err := mcpClassifier.discoverClassificationTool()
223+
Expect(err).ToNot(HaveOccurred())
224+
Expect(mcpClassifier.toolName).To(Equal("analyze_text"))
225+
})
226+
227+
It("should return error when no tools available", func() {
228+
mockClient.getToolsResult = []mcp.Tool{}
229+
err := mcpClassifier.discoverClassificationTool()
126230
Expect(err).To(HaveOccurred())
127-
Expect(err.Error()).To(ContainSubstring("tool name is not specified"))
231+
Expect(err.Error()).To(ContainSubstring("no tools available"))
232+
})
233+
234+
It("should return error when no classification tool found", func() {
235+
mockClient.getToolsResult = []mcp.Tool{
236+
{Name: "foo", Description: "Does foo"},
237+
{Name: "bar", Description: "Does bar"},
238+
}
239+
err := mcpClassifier.discoverClassificationTool()
240+
Expect(err).To(HaveOccurred())
241+
Expect(err.Error()).To(ContainSubstring("no classification tool found"))
242+
})
243+
244+
It("should handle case-insensitive pattern matching", func() {
245+
mockClient.getToolsResult = []mcp.Tool{
246+
{Name: "TextClassification", Description: "Classify documents"},
247+
}
248+
err := mcpClassifier.discoverClassificationTool()
249+
Expect(err).ToNot(HaveOccurred())
250+
Expect(mcpClassifier.toolName).To(Equal("TextClassification"))
251+
})
252+
253+
It("should match 'classif' in description (case-insensitive)", func() {
254+
mockClient.getToolsResult = []mcp.Tool{
255+
{Name: "my_tool", Description: "This tool performs Classification tasks"},
256+
}
257+
err := mcpClassifier.discoverClassificationTool()
258+
Expect(err).ToNot(HaveOccurred())
259+
Expect(mcpClassifier.toolName).To(Equal("my_tool"))
260+
})
261+
262+
It("should log available tools when none match", func() {
263+
mockClient.getToolsResult = []mcp.Tool{
264+
{Name: "tool1", Description: "Does something"},
265+
{Name: "tool2", Description: "Does another thing"},
266+
}
267+
err := mcpClassifier.discoverClassificationTool()
268+
Expect(err).To(HaveOccurred())
269+
Expect(err.Error()).To(ContainSubstring("tool1"))
270+
Expect(err.Error()).To(ContainSubstring("tool2"))
128271
})
129272
})
130273

131-
// Note: Full initialization test requires mocking NewClient which is complex
132-
// In real tests, we'd need dependency injection for the client factory
274+
// Test suite summary:
275+
// - Explicit configuration: ✓ (1 test)
276+
// - Common tool names discovery: ✓ (4 tests - classify_text, classify, categorize, categorize_text)
277+
// - Priority/precedence: ✓ (2 tests - classify_text first, common names over patterns)
278+
// - Pattern matching: ✓ (4 tests - name, description, case-insensitive)
279+
// - Error cases: ✓ (3 tests - no tools, no match, logging)
280+
// Total: 14 comprehensive tests for auto-discovery
133281
})
134282

135283
Describe("Close", func() {

0 commit comments

Comments
 (0)