Skip to content

Commit b369530

Browse files
authored
refactor: document strict vs. lenient content-item semantics in convertMapToCallToolResult (#7571)
From a semantic function clustering analysis, two refactoring items were identified for MCP content normalization. This PR addresses them. ## Changes - **Documentation (`mcp/tool_result.go`)** — Added a `// Note:` comment block in `convertMapToCallToolResult` explaining why the inline `[]interface{}` switch must **not** be replaced with `mcpresult.NormalizeContentItems`: ```go // Note: This switch intentionally returns an error when a non-map item is encountered // (strict semantics), unlike mcpresult.NormalizeContentItems which silently skips // non-map items (lenient semantics). The strict behavior here is required because this // function produces SDK-valid CallToolResult values... // Do NOT replace this switch with a call to mcpresult.NormalizeContentItems — the // two have deliberately different error-handling semantics. ``` - **Structural refactor (not done — blocked)** — Import graph audit confirmed `mcp` already imports `difc`, and `difc` is a caller of `mcpresult`. Merging `mcpresult` into `mcp` would introduce the cycle `mcp → difc → mcp`. Consolidation is deferred until the dependency is broken.
2 parents d61f00b + 147074e commit b369530

1 file changed

Lines changed: 8 additions & 0 deletions

File tree

internal/mcp/tool_result.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ func convertMapToCallToolResult(m map[string]interface{}) (*sdk.CallToolResult,
6060

6161
// Collect content items from either []interface{} (produced by json.Unmarshal) or
6262
// []map[string]interface{} (produced by helpers like BuildMCPTextResponse).
63+
//
64+
// Note: This switch intentionally returns an error when a non-map item is encountered
65+
// (strict semantics), unlike mcpresult.NormalizeContentItems which silently skips
66+
// non-map items (lenient semantics). The strict behavior here is required because this
67+
// function produces SDK-valid CallToolResult values; a non-map item indicates malformed
68+
// backend data that should surface as an error rather than be silently dropped.
69+
// Do NOT replace this switch with a call to mcpresult.NormalizeContentItems — the
70+
// two have deliberately different error-handling semantics.
6371
var items []map[string]interface{}
6472
switch v := contentVal.(type) {
6573
case []interface{}:

0 commit comments

Comments
 (0)