feat(mcp): get_node surfaces fill_ref + stroke_ref from var_table
`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.
This commit is contained in:
parent
7f73278c6c
commit
8b3bdcdfd5
|
|
@ -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<String> 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();
|
||||
|
|
|
|||
|
|
@ -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<u64, NodeRecord> = 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<u64, NodeRecord>,
|
||||
) {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue