perf(mcp): move command list rows instead of cloning (#8844)

This commit is contained in:
Yongjip Kim
2026-10-23 17:38:23 -04:00
committed by GitHub
parent 08a606c7d1
commit 78013e2256
+32 -4
View File
@@ -412,6 +412,14 @@ fn strict_node(v: &mut Value, defs: &serde_json::Map<String, Value>, depth: usiz
}
}
/// Both the bare engine list and older bridge wrappers own their rows already.
fn into_command_rows(mut reply: Value) -> Vec<Value> {
match reply {
Value::Array(rows) => rows,
_ => reply.get_mut("commands").and_then(Value::as_array_mut).map(std::mem::take).unwrap_or_default(),
}
}
fn ok_json(v: &Value) -> CallToolResult {
let text = match json_bytes(v).and_then(|bytes| String::from_utf8(bytes).map_err(|error| AutomationError::Other(error.to_string()))) {
Ok(text) => text,
@@ -827,10 +835,7 @@ impl PhotocraftMcp {
};
let needle = p.filter.map(|f| f.to_lowercase());
let enabled_only = p.enabled_only.unwrap_or(false);
let items: Vec<Value> = all
.as_array()
.cloned()
.unwrap_or_else(|| all.get("commands").and_then(Value::as_array).cloned().unwrap_or_default())
let items: Vec<Value> = into_command_rows(all)
.into_iter()
.filter(|c| {
needle.as_ref().is_none_or(|n| photocraft_engine::commands::list_entry_matches(c, n))
@@ -1083,6 +1088,29 @@ pub fn render_document_png(doc: &photocraft_doc::Document, max_side: u32) -> Res
mod tests {
use super::*;
#[test]
fn command_rows_keep_bare_and_wrapped_replies_compatible() {
let rows = json!([{"id":"first","params":{"nested":[1,2]}}, {"id":"second","enabled":false}]);
assert_eq!(into_command_rows(rows.clone()), rows.as_array().unwrap().clone());
assert_eq!(into_command_rows(json!({"commands":rows,"other":"ignored"})), rows.as_array().unwrap().clone());
for reply in [json!(null), json!(false), json!(3), json!("invalid"), json!({}), json!({"commands":null}), json!({"commands":{}}), json!([])] {
assert!(into_command_rows(reply).is_empty());
}
}
#[test]
fn command_rows_reuse_the_owned_array_and_nested_strings() {
for wrapped in [false, true] {
let rows = json!([{"id":"first","params":"documentation".repeat(64)}]);
let array = rows.as_array().unwrap().as_ptr();
let text = rows[0]["params"].as_str().unwrap().as_ptr();
let reply = if wrapped { Value::Object([("commands".into(), rows)].into_iter().collect()) } else { rows };
let moved = into_command_rows(reply);
assert_eq!(moved.as_ptr(), array, "wrapped={wrapped}: move the original allocation");
assert_eq!(moved[0]["params"].as_str().unwrap().as_ptr(), text, "wrapped={wrapped}: retain nested allocations");
}
}
#[test]
fn strict_schema_inlines_refs_and_stops_on_recursive_definitions() {
// A definition that refers to itself: inlining stops at a depth instead of looping.