From 8b3bdcdfd56c8bf0d962e1eb8f8b8bd35e810545 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Thu, 14 May 2026 18:27:46 +0800 Subject: [PATCH] feat(mcp): get_node surfaces fill_ref + stroke_ref from var_table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NodeRecord` grew two fields so LLM clients can tell when a node's colour follows a variable instead of a literal. Without this, the client can only see the resolved hex — it doesn't know whether to bump the variable (theme-wide ripple) or write a per-node override. `crates/openpencil-shell-core/src/mcp/tools.rs`: - `NodeRecord { fill_ref: String, stroke_ref: String }`. Empty string when the node doesn't use a variable for that paint channel. Empty (not omitted) so clients can probe with a single `out.get("fill_ref") == Some(&"")` instead of having to handle the missing-key case as well. - `get_node_snapshot` + `walk_node` thread the document's `var_table` through the tree walk and look up `var_table.fill_refs[node.id]` + `var_table.stroke_refs[ node.id]` per node. The lookup is O(log n) BTreeMap, so the full-document walk stays O(n log n) — fine for typical documents (≤ thousands of nodes); LLM call frequency is far below paint frequency anyway. - The `get_node` `call()` payload adds the two new keys alongside the existing kind / name / bounds / parent_id. Test (1 added, 281 shell-core total, 33 mcp module total): - `get_node_surfaces_fill_and_stroke_refs` — installs `fill_refs[11] = "color-primary"` + `stroke_refs[11] = "color-accent"` on the sample doc, calls `get_node` with node_id=11, asserts the payload carries the variable names. A second call with node_id=12 (no ref mapping) asserts both fields are empty strings (not absent), pinning the API contract. The MCP read-side now covers the full graph that paint-time `$ref` substitution uses: list_variables → choose a variable; list_pages → choose a page; get_document_info → counts; get_selection → current focus; get_node(id) → kind, bounds, parent, and which paint channels follow which variable. --- crates/openpencil-shell-core/src/mcp.rs | 40 +++++++++++++++++++ crates/openpencil-shell-core/src/mcp/tools.rs | 26 +++++++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/crates/openpencil-shell-core/src/mcp.rs b/crates/openpencil-shell-core/src/mcp.rs index d24188592..a4ba0f00c 100644 --- a/crates/openpencil-shell-core/src/mcp.rs +++ b/crates/openpencil-shell-core/src/mcp.rs @@ -538,6 +538,46 @@ mod tests { } } + #[test] + fn get_node_surfaces_fill_and_stroke_refs() { + // When a node's fill / stroke colour follows a variable + // (`var_table.fill_refs[id] = "color-primary"`), the + // `get_node` payload must expose the variable name so LLM + // clients can pick "bump the variable" vs "override the + // node's literal colour" without re-walking the document. + use crate::document::{Document, NodeId}; + let mut doc = Document::sample(); + doc.var_table + .fill_refs + .insert(NodeId::new(11), "color-primary".into()); + doc.var_table + .stroke_refs + .insert(NodeId::new(11), "color-accent".into()); + let tool = get_node_snapshot(&doc); + let mut args = BTreeMap::new(); + args.insert("node_id".into(), "11".into()); + match tool.call(&args) { + ToolOutcome::Ok(out) => { + assert_eq!(out.get("fill_ref"), Some(&"color-primary".to_string())); + assert_eq!(out.get("stroke_ref"), Some(&"color-accent".to_string())); + } + other => panic!("expected Ok, got {other:?}"), + } + // Nodes WITHOUT a ref-mapping must return empty strings + // (not omit the field) so clients can probe with a single + // `out.get("fill_ref") == Some("")` instead of branching + // on Option AND Option<&str>. + let mut args2 = BTreeMap::new(); + args2.insert("node_id".into(), "12".into()); + match tool.call(&args2) { + ToolOutcome::Ok(out) => { + assert_eq!(out.get("fill_ref"), Some(&String::new())); + assert_eq!(out.get("stroke_ref"), Some(&String::new())); + } + other => panic!("expected Ok, got {other:?}"), + } + } + #[test] fn get_node_errors_on_non_numeric_arg() { let doc = crate::document::Document::sample(); diff --git a/crates/openpencil-shell-core/src/mcp/tools.rs b/crates/openpencil-shell-core/src/mcp/tools.rs index 4a82a3df2..dddbf898b 100644 --- a/crates/openpencil-shell-core/src/mcp/tools.rs +++ b/crates/openpencil-shell-core/src/mcp/tools.rs @@ -189,6 +189,13 @@ pub struct NodeRecord { pub width: i32, pub height: i32, pub parent_id: u64, + /// Name of the variable driving this node's fill colour at + /// paint time, if any. Empty when the node's fill is a literal + /// colour. LLM clients use this to decide whether to bump the + /// variable (theme-wide change) or write a per-node override. + pub fill_ref: String, + /// Stroke parallel to `fill_ref`. + pub stroke_ref: String, } impl McpTool for GetNode { @@ -228,6 +235,8 @@ impl McpTool for GetNode { out.insert("width".into(), rec.width.to_string()); out.insert("height".into(), rec.height.to_string()); out.insert("parent_id".into(), rec.parent_id.to_string()); + out.insert("fill_ref".into(), rec.fill_ref.clone()); + out.insert("stroke_ref".into(), rec.stroke_ref.clone()); ToolOutcome::Ok(out) } } @@ -236,7 +245,7 @@ pub fn get_node_snapshot(doc: &crate::document::Document) -> GetNode { let mut nodes: BTreeMap = BTreeMap::new(); for page in &doc.pages { for node in &page.children { - walk_node(node, 0, &mut nodes); + walk_node(node, 0, &doc.var_table, &mut nodes); } } GetNode { nodes } @@ -245,6 +254,7 @@ pub fn get_node_snapshot(doc: &crate::document::Document) -> GetNode { fn walk_node( node: &crate::document::Node, parent_id: u64, + var_table: &crate::document::VariableTable, out: &mut BTreeMap, ) { let bounds = node.aggregate_bounds(); @@ -259,6 +269,16 @@ fn walk_node( crate::document::NodeKind::Path => "path", crate::document::NodeKind::Other(_) => "other", }; + let fill_ref = var_table + .fill_refs + .get(&node.id) + .cloned() + .unwrap_or_default(); + let stroke_ref = var_table + .stroke_refs + .get(&node.id) + .cloned() + .unwrap_or_default(); out.insert( node.id.raw(), NodeRecord { @@ -269,10 +289,12 @@ fn walk_node( width: bounds.size.x as i32, height: bounds.size.y as i32, parent_id, + fill_ref, + stroke_ref, }, ); for child in &node.children { - walk_node(child, node.id.raw(), out); + walk_node(child, node.id.raw(), var_table, out); } }