Skip to content

Commit 75cd389

Browse files
committed
review comments
Signed-off-by: Huamin Chen <[email protected]>
1 parent d0284d3 commit 75cd389

File tree

3 files changed

+32
-9
lines changed

3 files changed

+32
-9
lines changed

src/semantic-router/pkg/connectivity/mcp/http_client.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import (
1313
)
1414

1515
const (
16-
// MCPProtocolVersion is the MCP protocol version supported by this implementation
16+
// MCPProtocolVersion is the MCP protocol version supported by this implementation.
17+
// This version is defined by the Model Context Protocol specification and must match
18+
// the version expected by MCP servers. The version "2024-11-05" represents the current
19+
// stable MCP specification release. This is not derived from mcp-go as the library
20+
// does not export a version constant; instead, it's specified by the protocol itself.
1721
MCPProtocolVersion = "2024-11-05"
1822
// MCPClientVersion is the version of this MCP client implementation
1923
MCPClientVersion = "1.0.0"

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import (
1818

1919
const (
2020
// DefaultMCPThreshold is the default confidence threshold for MCP classification.
21-
// A value of 0.5 is commonly used as it represents a neutral cutoff for binary or probabilistic classification,
22-
// meaning predictions with confidence above 50% are considered positive. Adjust as needed for your use case.
21+
// For multi-class classification, a value of 0.5 means that a predicted class must have more than 50% confidence
22+
// to be selected. Adjust this threshold as needed for your use case.
2323
DefaultMCPThreshold = 0.5
2424
)
2525

@@ -222,8 +222,7 @@ func createMCPCategoryInference(initializer MCPCategoryInitializer) MCPCategoryI
222222
// IsMCPCategoryEnabled checks if MCP-based category classification is properly configured
223223
func (c *Classifier) IsMCPCategoryEnabled() bool {
224224
return c.Config.Classifier.MCPCategoryModel.Enabled &&
225-
c.Config.Classifier.MCPCategoryModel.ToolName != "" &&
226-
c.mcpCategoryInitializer != nil
225+
c.Config.Classifier.MCPCategoryModel.ToolName != ""
227226
}
228227

229228
// initializeMCPCategoryClassifier initializes the MCP category classification model
@@ -232,6 +231,10 @@ func (c *Classifier) initializeMCPCategoryClassifier() error {
232231
return fmt.Errorf("MCP category classification is not properly configured")
233232
}
234233

234+
if c.mcpCategoryInitializer == nil {
235+
return fmt.Errorf("MCP category initializer is not set")
236+
}
237+
235238
if err := c.mcpCategoryInitializer.Init(c.Config); err != nil {
236239
return fmt.Errorf("failed to initialize MCP category classifier: %w", err)
237240
}
@@ -246,6 +249,10 @@ func (c *Classifier) classifyCategoryMCP(text string) (string, float64, error) {
246249
return "", 0.0, fmt.Errorf("MCP category classification is not properly configured")
247250
}
248251

252+
if c.mcpCategoryInference == nil {
253+
return "", 0.0, fmt.Errorf("MCP category inference is not initialized")
254+
}
255+
249256
// Create context with timeout
250257
ctx := context.Background()
251258
if c.Config.Classifier.MCPCategoryModel.TimeoutSeconds > 0 {
@@ -302,6 +309,10 @@ func (c *Classifier) classifyCategoryWithEntropyMCP(text string) (string, float6
302309
return "", 0.0, entropy.ReasoningDecision{}, fmt.Errorf("MCP category classification is not properly configured")
303310
}
304311

312+
if c.mcpCategoryInference == nil {
313+
return "", 0.0, entropy.ReasoningDecision{}, fmt.Errorf("MCP category inference is not initialized")
314+
}
315+
305316
// Create context with timeout
306317
ctx := context.Background()
307318
if c.Config.Classifier.MCPCategoryModel.TimeoutSeconds > 0 {

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,10 +330,9 @@ var _ = Describe("Classifier MCP Methods", func() {
330330
Expect(classifier.IsMCPCategoryEnabled()).To(BeFalse())
331331
})
332332

333-
It("should return false when initializer is nil", func() {
334-
classifier.mcpCategoryInitializer = nil
335-
Expect(classifier.IsMCPCategoryEnabled()).To(BeFalse())
336-
})
333+
// Note: IsMCPCategoryEnabled now only checks configuration, not runtime state.
334+
// Runtime checks (like initializer != nil) are handled separately in the actual
335+
// classification methods to maintain separation of concerns.
337336
})
338337

339338
Describe("classifyCategoryMCP", func() {
@@ -473,6 +472,15 @@ var _ = Describe("Classifier MCP Methods", func() {
473472
Expect(err.Error()).To(ContainSubstring("not properly configured"))
474473
})
475474
})
475+
476+
Context("when initializer is nil", func() {
477+
It("should return error", func() {
478+
classifier.mcpCategoryInitializer = nil
479+
err := classifier.initializeMCPCategoryClassifier()
480+
Expect(err).To(HaveOccurred())
481+
Expect(err.Error()).To(ContainSubstring("initializer is not set"))
482+
})
483+
})
476484
})
477485
})
478486

0 commit comments

Comments
 (0)