diff --git a/src/graph/state.rs b/src/graph/state.rs index e16f16f..824a8fe 100644 --- a/src/graph/state.rs +++ b/src/graph/state.rs @@ -320,6 +320,16 @@ pub(super) fn template_root_keys(template: &str) -> Vec { .collect() } +// Whether `s` is exactly one `{{key}}` reference and nothing else, after the +// same trim `interpolate_raw` applies, so this accepts exactly the strings the +// runtime resolves as a pure reference. +pub(super) fn is_lone_template(s: &str) -> bool { + let s = s.trim(); + TEMPLATE_VAR_RE + .find(s) + .is_ok_and(|m| m.is_some_and(|m| m.start() == 0 && m.end() == s.len())) +} + fn value_to_string(value: &Value) -> String { match value { Value::String(s) => s.clone(), @@ -931,6 +941,38 @@ mod tests { assert_eq!(result, json!("v")); } + #[test] + fn is_lone_template_cases() { + for accepted in [ + "{{budget}}", + "{{cfg.limits[0]}}", + " {{budget}} ", + "\n{{k}}\t", + ] { + assert!( + is_lone_template(accepted), + "{accepted:?} should be a lone template" + ); + } + for rejected in [ + "", + "4", + "{{}}", + "{{ key }}", + "n={{k}}", + "{{k}}x", + "{{a}} {{b", + "{{a}}{{b}}", + "{{a-b}}", + "{{k", + ] { + assert!( + !is_lone_template(rejected), + "{rejected:?} must not be a lone template" + ); + } + } + #[test] fn interpolate_raw_inner_spaces_treated_as_mixed() { let manager = manager_with(&[("k", json!("v"))]); diff --git a/src/graph/validator.rs b/src/graph/validator.rs index e325f86..c6e5097 100644 --- a/src/graph/validator.rs +++ b/src/graph/validator.rs @@ -1,4 +1,4 @@ -use super::state::template_root_keys; +use super::state::{is_lone_template, template_root_keys}; use super::types::{ConcurrencyCap, Graph, NextTargets, Node, NodeType}; use crate::client::{Model, ModelType}; use crate::config; @@ -557,12 +557,12 @@ impl GraphValidator { for (node_id, node) in &graph.nodes { if let NodeType::Map(m) = &node.node_type && let Some(ConcurrencyCap::Template(t)) = &m.max_concurrency - && !contains_template(t) + && !is_lone_template(t) { result.error(ValidationError::with_node( node_id, "map node's `max_concurrency` is a string but not a template; write \ - an integer or a `{{key}}` template", + an integer or exactly one `{{key}}` with nothing around it", )); } } @@ -654,12 +654,13 @@ impl GraphValidator { )); } // A missing entry is reported by `validate_node_references`. - let mut members: Vec = branch_subgraph(graph, &m.branch).into_iter().collect(); - if members.is_empty() { + let mut member_ids: Vec = + branch_subgraph(graph, &m.branch).into_iter().collect(); + if member_ids.is_empty() { continue; } - members.sort(); - let members: Vec<(&str, &Node)> = members + member_ids.sort(); + let members: Vec<(&str, &Node)> = member_ids .iter() .filter_map(|id| graph.get_node(id).map(|n| (id.as_str(), n))) .collect(); @@ -1156,11 +1157,6 @@ fn primary_templated_fields(node: &Node) -> Vec { } } -fn contains_template(s: &str) -> bool { - s.find("{{") - .is_some_and(|open| s[open + 2..].contains("}}")) -} - fn node_state_updates_map(node: &Node) -> Option<&std::collections::HashMap> { match &node.node_type { NodeType::Llm(n) => n.state_updates.as_ref(), @@ -3628,6 +3624,52 @@ mod tests { ); } + #[test] + fn map_max_concurrency_malformed_templates_error() { + for cap in ["{{}}", "{{ key }}", "n={{k}}", "{{a}} {{b", "{{a-b}}"] { + let map = map_with_cap("m", "br", Some("end"), cap); + let branch = llm_with_state_updates("br", &[("output", "{{output}}")], None); + let graph = graph_with( + vec![("m", map), ("br", branch), ("end", end_node("end"))], + "m", + ); + + let result = validator().validate(&graph); + + assert!( + result.errors.iter().any(|e| e.message + == "map node's `max_concurrency` is a string but not a template; write \ + an integer or exactly one `{{key}}` with nothing around it" + && e.node_id.as_deref() == Some("m")), + "cap {cap:?} should fail the whole-string template check: {:?}", + result.errors + ); + } + } + + #[test] + fn map_max_concurrency_lone_template_forms_pass() { + for cap in ["{{budget}}", "{{cfg.limits[0]}}", " {{budget}} "] { + let map = map_with_cap("m", "br", Some("end"), cap); + let branch = llm_with_state_updates("br", &[("output", "{{output}}")], None); + let graph = graph_with( + vec![("m", map), ("br", branch), ("end", end_node("end"))], + "m", + ); + + let result = validator().validate(&graph); + + assert!( + !result + .errors + .iter() + .any(|e| e.message.contains("is a string but not a template")), + "cap {cap:?} is a lone template and must pass: {:?}", + result.errors + ); + } + } + #[test] fn map_max_concurrency_template_reading_sibling_write_errors() { let mut start = end_node("start"); diff --git a/src/parsers/bash.rs b/src/parsers/bash.rs index 019eaec..040a156 100644 --- a/src/parsers/bash.rs +++ b/src/parsers/bash.rs @@ -19,7 +19,7 @@ pub fn generate_bash_declarations( debug!("Building script at '{tool_file:?}'"); let (build_script, declarations) = build_bash_tool(&src, file_name) - .with_context(|| format!("Failed to build script at '{tools_file_path:?}'"))?; + .with_context(|| format!("Failed to build script '{file_name}'"))?; function::write_file_atomic(tools_file_path, &build_script, Some(0o755)) .with_context(|| format!("Failed to write built script to '{tools_file_path:?}'"))?;