diff --git a/src/graph/llm.rs b/src/graph/llm.rs index 4be1ef2..a409469 100644 --- a/src/graph/llm.rs +++ b/src/graph/llm.rs @@ -262,7 +262,7 @@ async fn run_with_retries( /// Whether `turn` (0-based) is the final turn the node's cap allows. A cap of 0 means no cap. pub(crate) fn is_last_turn(turn: u32, max_iterations: u32) -> bool { - max_iterations > 0 && turn + 1 == max_iterations + max_iterations > 0 && turn == max_iterations - 1 } async fn run_chat_loop( @@ -768,6 +768,12 @@ mod tests { assert!(!is_last_turn(u32::MAX, 0)); } + #[test] + fn is_last_turn_at_u32_max_does_not_overflow() { + assert!(is_last_turn(u32::MAX - 1, u32::MAX)); + assert!(!is_last_turn(u32::MAX, u32::MAX)); + } + /// The turn-top abort check runs before any client is created, so an /// already-aborted graph never issues a model call and the node fails /// with the abort reason rather than a connection error. diff --git a/src/graph/map.rs b/src/graph/map.rs index 7c15f9d..16651a5 100644 --- a/src/graph/map.rs +++ b/src/graph/map.rs @@ -1,5 +1,5 @@ use super::executor::{PeerRetireGuard, StepContext, StepResult, TaskCancelGuard, step}; -use super::state::StateManager; +use super::state::{StateManager, is_lone_template}; use super::types::{ConcurrencyCap, Graph, MapNode, Node, NodeType}; use super::validator::branch_subgraph; use crate::config::{RenderMode, RequestContext}; @@ -324,7 +324,8 @@ fn wants_peer_identity(node: &Node) -> bool { /// accepted; anything else is the author's bug and surfaces as an error /// rather than being clamped. A literal 0 is rejected the same way, so a /// graph loaded with validation off fails here instead of silently running -/// one item at a time. Only the absent-cap default is clamped. +/// one item at a time, and a string that is not exactly one `{{key}}` is +/// rejected before interpolation for the same reason. Only the absent-cap default is clamped. fn resolve_max_concurrency( node: &MapNode, state: &StateManager, @@ -340,6 +341,13 @@ fn resolve_max_concurrency( Some(ConcurrencyCap::Template(t)) => t, }; + if !is_lone_template(template) { + bail!( + "map node '{node_id}': max_concurrency \"{template}\" is a string but not a template; \ + write an integer or exactly one `{{{{key}}}}` with nothing around it" + ); + } + let value = state .interpolate_raw(template) .with_context(|| format!("map node '{node_id}': evaluating `max_concurrency` template"))?; @@ -662,6 +670,28 @@ mod tests { ); assert!(chain.contains("budget"), "{chain}"); } + + #[test] + fn resolve_max_concurrency_rejects_non_template_string() { + let state = state_with("budget", Value::from(3)); + let cap = Some(ConcurrencyCap::Template("4".into())); + let msg = resolve(cap, &state).unwrap_err().to_string(); + assert!( + msg.contains( + "is a string but not a template; write an integer or exactly one `{{key}}` \ + with nothing around it" + ), + "{msg}" + ); + assert!(msg.contains("map node 'm': max_concurrency \"4\""), "{msg}"); + } + + #[test] + fn resolve_max_concurrency_accepts_padded_template() { + let state = state_with("budget", Value::from(3)); + let cap = Some(ConcurrencyCap::Template(" {{budget}} ".into())); + assert_eq!(resolve(cap, &state).unwrap(), 3); + } } #[cfg(test)] diff --git a/src/graph/state.rs b/src/graph/state.rs index 824a8fe..6a1c21d 100644 --- a/src/graph/state.rs +++ b/src/graph/state.rs @@ -320,14 +320,24 @@ 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. +// Whether `s` is exactly one `{{key}}` reference and nothing else (after the +// same trim `interpolate_raw` applies) whose path parses with the grammar +// `get_nested_value` uses, so this accepts exactly the strings the runtime +// can resolve 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())) + let Ok(Some(m)) = TEMPLATE_VAR_RE.captures(s) else { + return false; + }; + let whole = m.get(0).expect("group 0 always present"); + if whole.start() != 0 || whole.end() != s.len() { + return false; + } + let path = m.get(1).expect("template regex has one group").as_str(); + let mut parts = path.split('.'); + let first = parts.next().and_then(split_indices); + first.is_some_and(|(name, _)| !name.is_empty()) + && parts.all(|part| split_indices(part).is_some()) } fn value_to_string(value: &Value) -> String { @@ -973,6 +983,28 @@ mod tests { } } + #[test] + fn is_lone_template_matches_runtime_path_grammar() { + for accepted in [ + "{{a.b[0][1].c}}", + "{{a[0][1]}}", + "{{a.}}", + "{{a..b}}", + "{{a.[0]}}", + ] { + assert!( + is_lone_template(accepted), + "{accepted:?} resolves at run time and must be accepted" + ); + } + for rejected in ["{{limits[foo]}}", "{{a[]}}", "{{a[0}}", "{{.a}}"] { + assert!( + !is_lone_template(rejected), + "{rejected:?} never resolves at run time and must be rejected" + ); + } + } + #[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 c6e5097..c19c855 100644 --- a/src/graph/validator.rs +++ b/src/graph/validator.rs @@ -3647,6 +3647,27 @@ mod tests { } } + #[test] + fn map_max_concurrency_non_numeric_index_errors() { + let map = map_with_cap("m", "br", Some("end"), "{{limits[foo]}}"); + 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")), + "a non-numeric index can never resolve and must fail at load: {:?}", + result.errors + ); + } + #[test] fn map_max_concurrency_lone_template_forms_pass() { for cap in ["{{budget}}", "{{cfg.limits[0]}}", " {{budget}} "] {