From f4b33d5cde60c416cbf5bfe636af433b1f3e9269 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Mon, 12 Oct 2026 09:05:38 +0100 Subject: [PATCH] perf(mcp): deduplicate operation schemas with shared definitions Share operation definitions between individual requests and batch steps, reducing the execute request schema by 39%. Place definitions at the tool input schema root so local references resolve correctly. Update the schema digest and smoke assertions. Verified with 45 MCP tests, the live stdio smoke test, and 228 schema validation comparisons. --- docs/automation/mcp_smoke.py | 26 +++++++++--- src/mcp.rs | 79 +++++++++++++++++++++++++++++++++++- src/mcp_ops.rs | 32 ++++++--------- 3 files changed, 111 insertions(+), 26 deletions(-) diff --git a/docs/automation/mcp_smoke.py b/docs/automation/mcp_smoke.py index ac6a0d74e..47d48f16a 100644 --- a/docs/automation/mcp_smoke.py +++ b/docs/automation/mcp_smoke.py @@ -53,8 +53,20 @@ def legacy(server: Path) -> None: assert names == TOOLS, names execute_request = definitions["ocs_execute"]["inputSchema"]["properties"]["request"] variants = execute_request.get("anyOf") or execute_request.get("oneOf") - assert len(variants) == 39 + # Resolve local references from the complete tool input schema. + root = definitions["ocs_execute"]["inputSchema"] + def resolve(variant): + if "$ref" not in variant: + return variant + ref = variant["$ref"] + assert ref.startswith("#/") + resolved = root + for part in ref[2:].split("/"): + resolved = resolved[part.replace("~1", "/").replace("~0", "~")] + return resolved + variants = [resolve(variant) for variant in variants] op_enum = execute_request["properties"]["op"]["enum"] + assert len(variants) == len(op_enum) for shipped in ("entities_create", "entities_delete", "entities_transform", "block_define", "block_delete", "file_identity", "xdata_set", "view_focus", "wblock", "plot", @@ -139,9 +151,12 @@ def modern(server: Path) -> None: "_meta": meta, }, }) - error = missing_cmd["result"]["structuredContent"] - assert error["code"] == "invalid_arguments" - assert "LINE 0,0 10,10" in error["error"] + # Tool errors are text content; the output schema covers successes only. + assert missing_cmd["result"]["isError"] is True + assert "structuredContent" not in missing_cmd["result"] + error = missing_cmd["result"]["content"][0]["text"] + assert "Missing cmd" in error + assert "LINE 0,0 10,10" in error invalid_mutation = request(process, { "jsonrpc": "2.0", @@ -154,7 +169,8 @@ def modern(server: Path) -> None: }, }) assert invalid_mutation["result"]["isError"] is True - assert "request_id" in invalid_mutation["result"]["structuredContent"]["error"] + assert "structuredContent" not in invalid_mutation["result"] + assert "request_id" in invalid_mutation["result"]["content"][0]["text"] rejected = request(process, { "jsonrpc": "2.0", diff --git a/src/mcp.rs b/src/mcp.rs index 4ffc27086..fc00e1189 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -31,7 +31,7 @@ const MODERN_PROTOCOL_VERSION: &str = "2026-07-28"; /// /// `block_delete` gained its optional `force` parameter. #[cfg(test)] -const TOOL_SCHEMA_DIGEST: &str = "c16572f139198d90807ede156370879106672ed97566c16ab6ae001c996059e8"; +const TOOL_SCHEMA_DIGEST: &str = "d597165afb46e8f067a77a3b827851b49be99d4739b15cf939b3be5b6c780bb1"; const MAX_REQUEST: usize = 1_048_576; const MAX_RESPONSE: u64 = 16 * 1024 * 1024; const CACHE_TTL_MS: u64 = 3_600_000; @@ -2252,6 +2252,17 @@ fn execute_request_schema() -> Value { crate::mcp_ops::execute_request_schema() } +// Local references resolve from the tool input schema root, not from request. +fn execute_input_schema() -> Value { + let mut schema = json!({"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"request":execute_request_schema(),"wait_seconds":{"type":"number","minimum":0,"maximum":60,"default":30,"description":"Total time to wait for completion before returning."},"response_detail":{"type":"string","enum":["compact","changed_entities","full"],"default":"compact","description":"compact returns only state needed for the next edit; changed_entities also returns current geometry for changed handles; full preserves the complete editor state."}},"required":["ocs_session_id","request"],"additionalProperties":false}); + let request = schema["properties"]["request"].as_object_mut().unwrap(); + let defs = request.remove("$defs").unwrap(); + let dialect = request.remove("$schema").unwrap(); + schema["$defs"] = defs; + schema["$schema"] = dialect; + schema +} + fn read_output_schema() -> Value { json!({ "type":"object", @@ -2297,7 +2308,7 @@ pub(crate) fn tool_definitions() -> Value { { "name":"ocs_execute", "description":"Execute semantic OCS actions, batch drafting, and mutations. Supports batch creation (entities_create), reference image underlays (embed_image), block definitions (block_define), and block reference insertions (INSERT). MANDATORY: For architectural drafting, all doors and windows must be inserted as CAD blocks (type: INSERT), never loose lines.", - "inputSchema":{"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"request":execute_request_schema(),"wait_seconds":{"type":"number","minimum":0,"maximum":60,"default":30,"description":"Total time to wait for completion before returning."},"response_detail":{"type":"string","enum":["compact","changed_entities","full"],"default":"compact","description":"compact returns only state needed for the next edit; changed_entities also returns current geometry for changed handles; full preserves the complete editor state."}},"required":["ocs_session_id","request"],"additionalProperties":false}, + "inputSchema":execute_input_schema(), "outputSchema":execute_output_schema(), "title":"Execute OCS action","annotations":{"readOnlyHint":false,"destructiveHint":true,"idempotentHint":true,"openWorldHint":false} }, @@ -3030,6 +3041,11 @@ mod tests { let find_variant = |op_name: &str| { request_variants .iter() + .map(|v| { + tools[2]["inputSchema"] + .pointer(v["$ref"].as_str().unwrap().strip_prefix('#').unwrap()) + .expect("operation reference resolves") + }) .find(|v| v["properties"]["op"]["enum"][0] == op_name) .cloned() .unwrap_or_else(|| panic!("variant for {op_name} exists")) @@ -4266,6 +4282,65 @@ mod tests { ); } + #[test] + fn execute_schema_shares_operations_and_resolves_local_references() { + fn check_refs(value: &Value, root: &Value) { + match value { + Value::Object(object) => { + if let Some(reference) = object.get("$ref") { + let pointer = reference.as_str().unwrap().strip_prefix('#').unwrap(); + assert!(root.pointer(pointer).is_some(), "unresolved {reference}"); + } + for child in object.values() { + check_refs(child, root); + } + } + Value::Array(array) => { + for child in array { + check_refs(child, root); + } + } + _ => {} + } + } + let schema = execute_input_schema(); + check_refs(&schema, &schema); + let standalone = execute_request_schema(); + check_refs(&standalone, &standalone); + assert!(schema["properties"]["request"].get("$defs").is_none()); + let variants = schema["properties"]["request"]["anyOf"].as_array().unwrap(); + let batch = schema["$defs"]["batch_step"]["anyOf"].as_array().unwrap(); + for (op, variant) in crate::mcp_ops::OPS.iter().zip(variants) { + assert_eq!(variant, &json!({"$ref":format!("#/$defs/op_{}", op.name)})); + assert_eq!(batch.contains(variant), op.batchable); + assert_eq!( + schema["$defs"][format!("op_{}", op.name)], + crate::mcp_ops::op_schema(op) + ); + } + + // Reconstruct the old duplicated layout using the same operation + // definitions, so the size comparison stays meaningful as ops evolve. + let mut old = standalone.clone(); + old["anyOf"] = json!(crate::mcp_ops::OPS + .iter() + .map(|op| crate::mcp_ops::op_schema(op)) + .collect::>()); + old["$defs"]["batch_step"]["anyOf"] = json!(crate::mcp_ops::OPS + .iter() + .filter(|op| op.batchable) + .map(|op| crate::mcp_ops::op_schema(op)) + .collect::>()); + old["$defs"] + .as_object_mut() + .unwrap() + .retain(|key, _| !key.starts_with("op_")); + let before = old.to_string().len(); + let after = standalone.to_string().len(); + eprintln!("Execute request schema: {before} -> {after} bytes"); + assert!(after * 100 < before * 70, "expected at least 30% reduction"); + } + #[test] fn schema_size_and_defs_structure() { let schema = execute_request_schema(); diff --git a/src/mcp_ops.rs b/src/mcp_ops.rs index b1692d546..80fd5407e 100644 --- a/src/mcp_ops.rs +++ b/src/mcp_ops.rs @@ -935,26 +935,20 @@ fn ty_schema(ty: &Ty) -> Value { } pub fn shared_defs() -> Value { - let batch_variants: Vec = OPS - .iter() - .filter(|op| op.batchable) - .map(|op| op_schema(op)) - .collect(); - - json!({ + let mut defs = json!({ "point": {"type": "array", "items": {"type": "number"}, "minItems": 2, "maxItems": 3}, "window": {"type": "array", "items": {"type": "number"}, "minItems": 4, "maxItems": 4}, "handle": {"type": "string", "pattern": "^(0[xX])?[0-9A-Fa-f]+$"}, - "batch_step": { - "type": "object", - "properties": { - "op": {"type": "string"}, - "document_id": {"type": "integer", "minimum": 0} - }, - "required": ["op"], - "anyOf": batch_variants - } - }) + "batch_step": batch_step_schema() + }); + for op in OPS { + defs[format!("op_{}", op.name)] = op_schema(op); + } + defs +} + +fn op_ref(op: &OpDef) -> Value { + json!({"$ref": format!("#/$defs/op_{}", op.name)}) } fn rules_text(op: &OpDef) -> String { @@ -996,7 +990,7 @@ pub fn op_schema(op: &OpDef) -> Value { } pub fn execute_request_schema() -> Value { - let variants: Vec = OPS.iter().map(|op| op_schema(op)).collect(); + let variants: Vec = OPS.iter().map(|op| op_ref(op)).collect(); let op_names: Vec<&str> = OPS.iter().map(|op| op.name).collect(); json!({ "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -1021,7 +1015,7 @@ pub fn batch_step_schema() -> Value { let batch_variants: Vec = OPS .iter() .filter(|op| op.batchable) - .map(|op| op_schema(op)) + .map(|op| op_ref(op)) .collect(); json!({