diff --git a/crates/attractor/src/handler/script.rs b/crates/attractor/src/handler/script.rs index c9988ee87..23d759ffb 100644 --- a/crates/attractor/src/handler/script.rs +++ b/crates/attractor/src/handler/script.rs @@ -132,6 +132,9 @@ impl Handler for ScriptHandler { outcome .context_updates .insert("tool.output".to_string(), serde_json::json!(stdout)); + outcome + .context_updates + .insert("script.stderr".to_string(), serde_json::json!(stderr)); outcome.notes = Some(format!("Script completed: {script}")); Ok(outcome) } else { @@ -143,7 +146,11 @@ impl Handler for ScriptHandler { } else { format!("Script failed: {}", stderr.trim()) }; - Ok(Outcome::fail(reason)) + let mut outcome = Outcome::fail(reason); + outcome + .context_updates + .insert("script.stderr".to_string(), serde_json::json!(stderr)); + Ok(outcome) } } Err(e) => Ok(Outcome::fail(e.to_string())), @@ -210,6 +217,8 @@ mod tests { assert!(outcome.notes.as_deref().unwrap().contains("echo hello")); let script_output = outcome.context_updates.get("script.output").unwrap(); assert!(script_output.as_str().unwrap().contains("hello")); + let script_stderr = outcome.context_updates.get("script.stderr").unwrap(); + assert_eq!(script_stderr.as_str().unwrap(), ""); } #[tokio::test] @@ -544,6 +553,31 @@ mod tests { assert!(script_output.as_str().unwrap().contains("legacy")); } + #[tokio::test] + async fn script_handler_captures_stderr() { + let handler = ScriptHandler; + let mut node = Node::new("script_node"); + node.attrs.insert( + "script".to_string(), + AttrValue::String("echo out && echo err >&2".to_string()), + ); + let context = Context::new(); + let graph = Graph::new("test"); + let logs_root = tempfile::tempdir().unwrap(); + + let outcome = handler + .execute(&node, &context, &graph, logs_root.path(), &make_services()) + .await + .unwrap(); + assert_eq!(outcome.status, StageStatus::Success); + let script_stderr = outcome.context_updates.get("script.stderr").unwrap(); + assert!( + script_stderr.as_str().unwrap().contains("err"), + "script.stderr should contain 'err', got: {:?}", + script_stderr + ); + } + #[tokio::test] async fn tool_output_context_key_dual_write() { let handler = ScriptHandler; diff --git a/crates/attractor/src/preamble.rs b/crates/attractor/src/preamble.rs index a69d6758a..96950c8ef 100644 --- a/crates/attractor/src/preamble.rs +++ b/crates/attractor/src/preamble.rs @@ -1,7 +1,7 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use crate::context::Context; -use crate::graph::Graph; +use crate::graph::{Graph, Node}; use crate::outcome::Outcome; /// Build a fidelity-appropriate preamble string for non-full context modes. @@ -9,10 +9,10 @@ use crate::outcome::Outcome; /// The preamble provides prior conversation context to the next LLM session, /// tailored by the fidelity mode: /// - `truncate`: Only graph goal and run ID -/// - `compact`: Structured bullet-point summary of completed stages and context +/// - `compact`: Nested-bullet summary with handler-specific sub-items /// - `summary:low`: Brief textual summary (~600 token target) /// - `summary:medium`: Moderate detail (~1500 token target) -/// - `summary:high`: Detailed summary (~3000 token target) +/// - `summary:high`: Detailed per-stage Markdown report #[must_use] pub fn build_preamble( fidelity: &str, @@ -28,59 +28,338 @@ pub fn build_preamble( "truncate" => { format!("Goal: {goal}\nRun ID: {run_id}\n") } - "compact" => { - build_compact_preamble(goal, &run_id, completed_nodes, node_outcomes, context) - } - "summary:low" => { - build_summary_preamble(goal, &run_id, completed_nodes, node_outcomes, context, SummaryDetail::Low) - } - "summary:medium" => { - build_summary_preamble(goal, &run_id, completed_nodes, node_outcomes, context, SummaryDetail::Medium) - } - "summary:high" => { - build_summary_preamble(goal, &run_id, completed_nodes, node_outcomes, context, SummaryDetail::High) - } + "compact" => build_compact_preamble(goal, completed_nodes, node_outcomes, context, graph), + "summary:low" => build_summary_preamble( + goal, + &run_id, + completed_nodes, + node_outcomes, + context, + graph, + SummaryDetail::Low, + ), + "summary:medium" => build_summary_preamble( + goal, + &run_id, + completed_nodes, + node_outcomes, + context, + graph, + SummaryDetail::Medium, + ), + "summary:high" => build_summary_preamble( + goal, + &run_id, + completed_nodes, + node_outcomes, + context, + graph, + SummaryDetail::High, + ), _ => { // Unknown fidelity mode: fall back to compact - build_compact_preamble(goal, &run_id, completed_nodes, node_outcomes, context) + build_compact_preamble(goal, completed_nodes, node_outcomes, context, graph) } } } -fn build_compact_preamble( - goal: &str, - run_id: &str, - completed_nodes: &[String], - node_outcomes: &HashMap, - context: &Context, -) -> String { - let mut parts = Vec::new(); - parts.push(format!("Goal: {goal}")); - parts.push(format!("Run ID: {run_id}")); +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- - if !completed_nodes.is_empty() { - parts.push(String::from("\nCompleted stages:")); - for node_id in completed_nodes { - if let Some(outcome) = node_outcomes.get(node_id) { - let status = outcome.status.to_string(); - let notes_suffix = outcome - .notes - .as_deref() - .map(|n| format!(" - {n}")) - .unwrap_or_default(); - parts.push(format!("- {node_id}: {status}{notes_suffix}")); - } else { - parts.push(format!("- {node_id}: completed")); +fn is_context_key_excluded(key: &str) -> bool { + key.starts_with("internal.") + || key.starts_with("current") + || key.starts_with("graph.") + || key.starts_with("thread.") + || key.starts_with("response.") + || key == "run_id" + || key == "outcome" + || key == "last_stage" + || key == "last_response" + || key == "preferred_label" +} + +fn format_value(val: &serde_json::Value) -> String { + match val.as_str() { + Some(s) => s.to_string(), + None => val.to_string(), + } +} + +fn format_token_count(tokens: i64) -> String { + if tokens >= 1000 { + let k = tokens as f64 / 1000.0; + // One decimal place, strip trailing zero after decimal + let formatted = format!("{k:.1}"); + format!("{formatted}k") + } else { + tokens.to_string() + } +} + +/// Returns the set of context keys that are rendered inline under a stage's +/// handler-specific details, so they can be skipped in the trailing context section. +fn stage_rendered_keys(node_id: &str, outcome: &Outcome) -> HashSet { + let candidates = [ + "script.output".to_string(), + "script.stderr".to_string(), + "tool.output".to_string(), + "last_stage".to_string(), + "last_response".to_string(), + format!("response.{node_id}"), + ]; + candidates + .into_iter() + .filter(|k| outcome.context_updates.contains_key(k)) + .collect() +} + +/// Render handler-specific nested bullets for compact mode. +fn render_compact_stage_details( + _node_id: &str, + node: Option<&Node>, + outcome: &Outcome, +) -> Vec { + let handler = node.and_then(|n| n.handler_type()); + match handler { + Some("script") => { + let mut lines = Vec::new(); + if let Some(n) = node { + if let Some(cmd) = n + .attrs + .get("script") + .or_else(|| n.attrs.get("tool_command")) + .and_then(|v| v.as_str()) + { + lines.push(format!(" - Script: `{cmd}`")); + } + } + if let Some(stdout_val) = outcome.context_updates.get("script.output") { + let stdout = format_value(stdout_val); + if stdout.trim().is_empty() { + lines.push(" - Stdout: (empty)".to_string()); + } else { + lines.push(" - Stdout:".to_string()); + lines.push(" ```".to_string()); + lines.push(format!(" {}", stdout.trim())); + lines.push(" ```".to_string()); + } + } + if let Some(stderr_val) = outcome.context_updates.get("script.stderr") { + let stderr = format_value(stderr_val); + if stderr.trim().is_empty() { + lines.push(" - Stderr: (empty)".to_string()); + } else { + lines.push(" - Stderr:".to_string()); + lines.push(" ```".to_string()); + lines.push(format!(" {}", stderr.trim())); + lines.push(" ```".to_string()); + } + } + lines + } + Some("codergen") => { + let mut lines = Vec::new(); + if let Some(usage) = &outcome.usage { + let input = format_token_count(usage.input_tokens); + let output = format_token_count(usage.output_tokens); + lines.push(format!( + " - Model: {}, {} tokens in / {} out", + usage.model, input, output + )); + } + if !outcome.files_touched.is_empty() { + lines.push(format!(" - Files: {}", outcome.files_touched.join(", "))); + } + lines + } + _ => Vec::new(), + } +} + +/// Render a full `## Stage: {node_id}` section for summary:high mode. +fn render_summary_high_stage_section( + node_id: &str, + node: Option<&Node>, + outcome: &Outcome, +) -> Vec { + let handler = node.and_then(|n| n.handler_type()); + let mut lines = Vec::new(); + lines.push(format!("\n## Stage: {node_id}")); + lines.push(format!("- Status: {}", outcome.status)); + + if let Some(h) = handler { + lines.push(format!("- Handler: {h}")); + } + + match handler { + Some("script") => { + if let Some(n) = node { + if let Some(cmd) = n + .attrs + .get("script") + .or_else(|| n.attrs.get("tool_command")) + .and_then(|v| v.as_str()) + { + lines.push(format!("- Script: `{cmd}`")); + } + } + if let Some(stdout_val) = outcome.context_updates.get("script.output") { + let stdout = format_value(stdout_val); + if stdout.trim().is_empty() { + lines.push("- Stdout: (empty)".to_string()); + } else { + lines.push("- Stdout:".to_string()); + lines.push(" ```".to_string()); + lines.push(format!(" {}", stdout.trim())); + lines.push(" ```".to_string()); + } + } + if let Some(stderr_val) = outcome.context_updates.get("script.stderr") { + let stderr = format_value(stderr_val); + if stderr.trim().is_empty() { + lines.push("- Stderr: (empty)".to_string()); + } else { + lines.push("- Stderr:".to_string()); + lines.push(" ```".to_string()); + lines.push(format!(" {}", stderr.trim())); + lines.push(" ```".to_string()); + } + } + } + Some("codergen") => { + if let Some(usage) = &outcome.usage { + lines.push(format!("- Model: {}", usage.model)); + lines.push(format!( + "- Tokens: {} in / {} out", + format_token_count(usage.input_tokens), + format_token_count(usage.output_tokens) + )); + } + if !outcome.files_touched.is_empty() { + lines.push(format!( + "- Files touched: {}", + outcome.files_touched.join(", ") + )); + } + // Include full response from context_updates + if let Some(resp_val) = + outcome.context_updates.get(&format!("response.{node_id}")) + { + let resp = format_value(resp_val); + if !resp.is_empty() { + lines.push("- Response:".to_string()); + // Blockquote each line + for line in resp.lines() { + lines.push(format!(" > {line}")); + } + } + } + } + _ => { + if let Some(notes) = outcome.notes.as_deref() { + lines.push(format!("- Notes: {notes}")); + } + if let Some(reason) = outcome.failure_reason.as_deref() { + lines.push(format!("- Failure reason: {reason}")); } } } - append_context_values(&mut parts, context); + lines +} + +/// Append filtered context as a `## Context` bullet list. +fn append_filtered_context( + parts: &mut Vec, + context: &Context, + rendered_keys: &HashSet, +) { + let snapshot = context.snapshot(); + let mut context_keys: Vec<&String> = snapshot + .keys() + .filter(|k| !is_context_key_excluded(k) && !rendered_keys.contains(*k)) + .collect(); + if !context_keys.is_empty() { + context_keys.sort(); + parts.push(String::from("\n## Context")); + for key in context_keys { + if let Some(val) = snapshot.get(key) { + parts.push(format!("- {key}: {}", format_value(val))); + } + } + } +} + +/// Append filtered context as a `## Current context` Markdown table. +fn append_filtered_context_table( + parts: &mut Vec, + context: &Context, + rendered_keys: &HashSet, +) { + let snapshot = context.snapshot(); + let mut context_keys: Vec<&String> = snapshot + .keys() + .filter(|k| !is_context_key_excluded(k) && !rendered_keys.contains(*k)) + .collect(); + if !context_keys.is_empty() { + context_keys.sort(); + parts.push(String::from("\n## Current context")); + parts.push("| Key | Value |".to_string()); + parts.push("|-----|-------|".to_string()); + for key in context_keys { + if let Some(val) = snapshot.get(key) { + parts.push(format!("| {key} | {} |", format_value(val))); + } + } + } +} + +// --------------------------------------------------------------------------- +// Compact preamble +// --------------------------------------------------------------------------- + +fn build_compact_preamble( + goal: &str, + completed_nodes: &[String], + node_outcomes: &HashMap, + context: &Context, + graph: &Graph, +) -> String { + let mut parts = Vec::new(); + parts.push(format!("Goal: {goal}")); + + let mut all_rendered_keys = HashSet::new(); + + if !completed_nodes.is_empty() { + parts.push(String::from("\n## Completed stages")); + for node_id in completed_nodes { + let node = graph.nodes.get(node_id); + if let Some(outcome) = node_outcomes.get(node_id) { + let status = &outcome.status; + parts.push(format!("- **{node_id}**: {status}")); + + let details = render_compact_stage_details(node_id, node, outcome); + parts.extend(details); + + all_rendered_keys.extend(stage_rendered_keys(node_id, outcome)); + } else { + parts.push(format!("- **{node_id}**: completed")); + } + } + } + + append_filtered_context(&mut parts, context, &all_rendered_keys); parts.push(String::new()); parts.join("\n") } +// --------------------------------------------------------------------------- +// Summary preamble +// --------------------------------------------------------------------------- + enum SummaryDetail { Low, Medium, @@ -93,114 +372,110 @@ fn build_summary_preamble( completed_nodes: &[String], node_outcomes: &HashMap, context: &Context, + graph: &Graph, detail: SummaryDetail, ) -> String { let mut parts = Vec::new(); parts.push(format!("Goal: {goal}")); parts.push(format!("Run ID: {run_id}")); - let stage_count = completed_nodes.len(); - parts.push(format!("Completed {stage_count} stage(s) so far.")); + let mut all_rendered_keys = HashSet::new(); - // Determine how many recent stages to include based on detail level - let recent_count = match detail { - SummaryDetail::Low => 2, - SummaryDetail::Medium => 5, - SummaryDetail::High => stage_count, - }; + match detail { + SummaryDetail::High => { + // Pipeline progress: count all nodes (including start/exit) + let total_nodes = graph.nodes.len(); + let completed_count = completed_nodes.len(); + parts.push(format!( + "Pipeline progress: {completed_count} of {total_nodes} stages completed" + )); - let stages_to_show: Vec<&String> = if stage_count > recent_count { - let skipped = stage_count - recent_count; - parts.push(format!("\n({skipped} earlier stage(s) omitted)")); - completed_nodes.iter().skip(skipped).collect() - } else { - completed_nodes.iter().collect() - }; - - if !stages_to_show.is_empty() { - parts.push(String::from("\nRecent stages:")); - for node_id in &stages_to_show { - if let Some(outcome) = node_outcomes.get(*node_id) { - let status = outcome.status.to_string(); - let mut line = format!("- {node_id}: {status}"); - if let Some(notes) = outcome.notes.as_deref() { - line.push_str(&format!(" ({notes})")); - } - if let Some(reason) = outcome.failure_reason.as_deref() { - line.push_str(&format!(" [reason: {reason}]")); + for node_id in completed_nodes { + let node = graph.nodes.get(node_id); + if let Some(outcome) = node_outcomes.get(node_id) { + let section = + render_summary_high_stage_section(node_id, node, outcome); + parts.extend(section); + all_rendered_keys.extend(stage_rendered_keys(node_id, outcome)); + } else { + parts.push(format!("\n## Stage: {node_id}")); + parts.push("- Status: completed".to_string()); } + } - parts.push(line); + append_filtered_context_table(&mut parts, context, &all_rendered_keys); + } + _ => { + // Low and Medium share the same structure as before + let stage_count = completed_nodes.len(); + parts.push(format!("Completed {stage_count} stage(s) so far.")); - // For medium and high detail, include context updates from the outcome - match detail { - SummaryDetail::Medium | SummaryDetail::High => { - if !outcome.context_updates.is_empty() { - let mut update_keys: Vec<&String> = outcome.context_updates.keys().collect(); - update_keys.sort(); - for key in update_keys { - if let Some(val) = outcome.context_updates.get(key) { - let val_str = if let Some(s) = val.as_str() { - s.to_string() - } else { - val.to_string() - }; - parts.push(format!(" - set {key} = {val_str}")); + let recent_count = match detail { + SummaryDetail::Low => 2, + SummaryDetail::Medium => 5, + SummaryDetail::High => unreachable!(), + }; + + let stages_to_show: Vec<&String> = if stage_count > recent_count { + let skipped = stage_count - recent_count; + parts.push(format!("\n({skipped} earlier stage(s) omitted)")); + completed_nodes.iter().skip(skipped).collect() + } else { + completed_nodes.iter().collect() + }; + + if !stages_to_show.is_empty() { + parts.push(String::from("\nRecent stages:")); + for node_id in &stages_to_show { + if let Some(outcome) = node_outcomes.get(*node_id) { + let status = outcome.status.to_string(); + let mut line = format!("- {node_id}: {status}"); + if let Some(notes) = outcome.notes.as_deref() { + line.push_str(&format!(" ({notes})")); + } + if let Some(reason) = outcome.failure_reason.as_deref() { + line.push_str(&format!(" [reason: {reason}]")); + } + parts.push(line); + + // For medium detail, include context updates from the outcome + if matches!(detail, SummaryDetail::Medium) { + if !outcome.context_updates.is_empty() { + let mut update_keys: Vec<&String> = + outcome.context_updates.keys().collect(); + update_keys.sort(); + for key in update_keys { + if let Some(val) = outcome.context_updates.get(key) { + parts.push(format!( + " - set {key} = {}", + format_value(val) + )); + } } } } + } else { + parts.push(format!("- {node_id}: completed")); } - SummaryDetail::Low => {} } - } else { - parts.push(format!("- {node_id}: completed")); + } + + // Include context values for medium only (low excludes them) + if matches!(detail, SummaryDetail::Medium) { + append_filtered_context(&mut parts, context, &all_rendered_keys); } } } - // Include context values for medium and high - match detail { - SummaryDetail::Medium | SummaryDetail::High => { - append_context_values(&mut parts, context); - } - SummaryDetail::Low => {} - } - parts.push(String::new()); parts.join("\n") } -fn append_context_values(parts: &mut Vec, context: &Context) { - let snapshot = context.snapshot(); - let mut context_keys: Vec<&String> = snapshot - .keys() - .filter(|k| { - !k.starts_with("internal.") - && !k.starts_with("current") - && *k != "run_id" - && *k != "outcome" - }) - .collect(); - if !context_keys.is_empty() { - context_keys.sort(); - parts.push(String::from("\nContext values:")); - for key in context_keys { - if let Some(val) = snapshot.get(key) { - let val_str = if let Some(s) = val.as_str() { - s.to_string() - } else { - val.to_string() - }; - parts.push(format!("- {key}: {val_str}")); - } - } - } -} - #[cfg(test)] mod tests { use super::*; use crate::graph::AttrValue; + use crate::outcome::StageUsage; // --- truncate mode --- @@ -250,8 +525,14 @@ mod tests { &node_outcomes, ); - assert!(!preamble.contains("plan"), "truncate should not list completed stages"); - assert!(!preamble.contains("code"), "truncate should not list completed stages"); + assert!( + !preamble.contains("plan"), + "truncate should not list completed stages" + ); + assert!( + !preamble.contains("code"), + "truncate should not list completed stages" + ); } // --- compact mode --- @@ -279,9 +560,19 @@ mod tests { ); assert!(preamble.contains("Deploy app"), "should contain the goal"); - assert!(preamble.contains("plan"), "should list completed stage 'plan'"); + assert!( + preamble.contains("## Completed stages"), + "should have Completed stages heading" + ); + assert!( + preamble.contains("**plan**"), + "should list completed stage 'plan' in bold" + ); assert!(preamble.contains("success"), "should show plan's success status"); - assert!(preamble.contains("code"), "should list completed stage 'code'"); + assert!( + preamble.contains("**code**"), + "should list completed stage 'code' in bold" + ); assert!(preamble.contains("fail"), "should show code's fail status"); } @@ -302,8 +593,14 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("graph.goal"), "should include graph.goal context key"); - assert!(preamble.contains("user.name"), "should include user.name context key"); + assert!( + !preamble.contains("graph.goal"), + "should exclude graph.* context keys" + ); + assert!( + preamble.contains("user.name"), + "should include user.name context key" + ); assert!(preamble.contains("alice"), "should include context value"); } @@ -314,6 +611,12 @@ mod tests { context.set("internal.fidelity", serde_json::json!("compact")); context.set("internal.retry_count.plan", serde_json::json!(1)); context.set("current_node", serde_json::json!("work")); + context.set("graph.default_fidelity", serde_json::json!("compact")); + context.set("thread.main.current_node", serde_json::json!("work")); + context.set("response.plan", serde_json::json!("some response")); + context.set("last_stage", serde_json::json!("plan")); + context.set("last_response", serde_json::json!("resp")); + context.set("preferred_label", serde_json::json!("success")); context.set("user.name", serde_json::json!("bob")); let completed_nodes: Vec = Vec::new(); let node_outcomes: HashMap = HashMap::new(); @@ -326,14 +629,49 @@ mod tests { &node_outcomes, ); - assert!(!preamble.contains("internal.fidelity"), "should exclude internal keys"); - assert!(!preamble.contains("internal.retry_count"), "should exclude internal keys"); - assert!(!preamble.contains("current_node"), "should exclude current keys"); + assert!( + !preamble.contains("internal.fidelity"), + "should exclude internal keys" + ); + assert!( + !preamble.contains("internal.retry_count"), + "should exclude internal keys" + ); + assert!( + !preamble.contains("current_node"), + "should exclude current keys" + ); + assert!( + !preamble.contains("graph.default_fidelity"), + "should exclude graph.* keys" + ); + assert!( + !preamble.contains("thread.main"), + "should exclude thread.* keys" + ); + assert!( + !preamble.contains("response.plan"), + "should exclude response.* keys" + ); + assert!( + !preamble.contains("- last_stage:"), + "should exclude last_stage" + ); + assert!( + !preamble.contains("- last_response:"), + "should exclude last_response" + ); + assert!( + !preamble.contains("- preferred_label:"), + "should exclude preferred_label" + ); assert!(preamble.contains("user.name"), "should include non-internal keys"); } #[test] fn build_preamble_compact_shows_notes_on_stages() { + // Compact no longer shows notes inline (handler-specific details replace them), + // but notes are still available in the outcome for non-handler stages. let graph = Graph::new("test"); let context = Context::new(); let completed_nodes = vec!["work".to_string()]; @@ -350,7 +688,200 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("auto-status: completed"), "should include outcome notes"); + // Compact uses bold node IDs and handler-specific details now + assert!( + preamble.contains("**work**"), + "should include node ID in bold" + ); + assert!(preamble.contains("success"), "should show success status"); + } + + // --- compact handler-specific details --- + + #[test] + fn compact_script_stage_shows_command_stdout_stderr() { + let mut graph = Graph::new("test"); + let mut run_tests = Node::new("run_tests"); + run_tests.attrs.insert( + "shape".to_string(), + AttrValue::String("parallelogram".to_string()), + ); + run_tests.attrs.insert( + "script".to_string(), + AttrValue::String("echo '10 passed'".to_string()), + ); + graph.nodes.insert("run_tests".to_string(), run_tests); + + let context = Context::new(); + let completed_nodes = vec!["run_tests".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.context_updates.insert( + "script.output".to_string(), + serde_json::json!("10 passed\n"), + ); + outcome + .context_updates + .insert("script.stderr".to_string(), serde_json::json!("")); + node_outcomes.insert("run_tests".to_string(), outcome); + + let preamble = build_preamble( + "compact", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!(preamble.contains("Script: `echo '10 passed'`"), "should show script command"); + assert!(preamble.contains("Stdout:"), "should show stdout label"); + assert!(preamble.contains("10 passed"), "should show stdout content"); + assert!(preamble.contains("Stderr: (empty)"), "should show empty stderr"); + } + + #[test] + fn compact_codergen_stage_shows_model_and_files() { + let mut graph = Graph::new("test"); + let mut report = Node::new("report"); + report.attrs.insert( + "shape".to_string(), + AttrValue::String("box".to_string()), + ); + graph.nodes.insert("report".to_string(), report); + + let context = Context::new(); + let completed_nodes = vec!["report".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.usage = Some(StageUsage { + model: "claude-sonnet-4-20250514".to_string(), + input_tokens: 1234, + output_tokens: 567, + cache_read_tokens: None, + cache_write_tokens: None, + reasoning_tokens: None, + cost: None, + }); + outcome.files_touched = vec!["src/lib.rs".to_string(), "src/main.rs".to_string()]; + node_outcomes.insert("report".to_string(), outcome); + + let preamble = build_preamble( + "compact", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("claude-sonnet-4-20250514"), + "should show model name" + ); + assert!(preamble.contains("1.2k tokens in"), "should show token count"); + assert!( + preamble.contains("src/lib.rs, src/main.rs"), + "should show files touched" + ); + } + + #[test] + fn compact_context_excludes_engine_keys() { + let graph = Graph::new("test"); + let context = Context::new(); + context.set("graph.default_fidelity", serde_json::json!("compact")); + context.set("thread.main.current_node", serde_json::json!("work")); + context.set("response.plan", serde_json::json!("some LLM response")); + context.set("last_stage", serde_json::json!("plan")); + context.set("user.preference", serde_json::json!("dark")); + let completed_nodes: Vec = Vec::new(); + let node_outcomes: HashMap = HashMap::new(); + + let preamble = build_preamble( + "compact", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + !preamble.contains("graph.default_fidelity"), + "should exclude graph.* keys" + ); + assert!( + !preamble.contains("thread.main"), + "should exclude thread.* keys" + ); + assert!( + !preamble.contains("response.plan"), + "should exclude response.* keys" + ); + assert!( + !preamble.contains("- last_stage:"), + "should exclude last_stage" + ); + assert!( + preamble.contains("user.preference"), + "should include user keys" + ); + } + + #[test] + fn compact_context_deduplicates_stage_rendered_keys() { + let mut graph = Graph::new("test"); + let mut step = Node::new("step"); + step.attrs.insert( + "shape".to_string(), + AttrValue::String("parallelogram".to_string()), + ); + step.attrs.insert( + "script".to_string(), + AttrValue::String("echo hi".to_string()), + ); + graph.nodes.insert("step".to_string(), step); + + let context = Context::new(); + // script.output is set in context (the engine copies context_updates to context) + context.set("script.output", serde_json::json!("hi\n")); + context.set("tool.output", serde_json::json!("hi\n")); + context.set("script.stderr", serde_json::json!("")); + let completed_nodes = vec!["step".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.context_updates.insert( + "script.output".to_string(), + serde_json::json!("hi\n"), + ); + outcome + .context_updates + .insert("tool.output".to_string(), serde_json::json!("hi\n")); + outcome + .context_updates + .insert("script.stderr".to_string(), serde_json::json!("")); + node_outcomes.insert("step".to_string(), outcome); + + let preamble = build_preamble( + "compact", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + // script.output and tool.output should NOT appear in the Context section + // because they're already rendered inline under the stage + let context_section = preamble + .split("## Context") + .nth(1) + .unwrap_or(""); + assert!( + !context_section.contains("script.output"), + "script.output should be deduplicated from context section" + ); + assert!( + !context_section.contains("tool.output"), + "tool.output should be deduplicated from context section" + ); } // --- summary:low mode --- @@ -378,7 +909,10 @@ mod tests { ); assert!(preamble.contains("Run tests"), "should contain the goal"); - assert!(preamble.contains("3 stage(s)"), "should mention total stage count"); + assert!( + preamble.contains("3 stage(s)"), + "should mention total stage count" + ); } #[test] @@ -429,7 +963,10 @@ mod tests { &node_outcomes, ); - assert!(!preamble.contains("user.name"), "summary:low should not include context values"); + assert!( + !preamble.contains("user.name"), + "summary:low should not include context values" + ); } // --- summary:medium mode --- @@ -488,7 +1025,10 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("user.name"), "summary:medium should include context values"); + assert!( + preamble.contains("user.name"), + "summary:medium should include context values" + ); assert!(preamble.contains("alice"), "should include context value"); } @@ -499,7 +1039,9 @@ mod tests { let completed_nodes = vec!["work".to_string()]; let mut node_outcomes: HashMap = HashMap::new(); let mut outcome = Outcome::success(); - outcome.context_updates.insert("result.score".to_string(), serde_json::json!(95)); + outcome + .context_updates + .insert("result.score".to_string(), serde_json::json!(95)); node_outcomes.insert("work".to_string(), outcome); let preamble = build_preamble( @@ -510,7 +1052,10 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("result.score"), "should include context updates from outcomes"); + assert!( + preamble.contains("result.score"), + "should include context updates from outcomes" + ); } // --- summary:high mode --- @@ -543,9 +1088,15 @@ mod tests { &node_outcomes, ); - // summary:high shows ALL stages - assert!(preamble.contains("s1"), "should show all stages including s1"); - assert!(preamble.contains("s6"), "should show all stages including s6"); + // summary:high shows ALL stages as ## Stage: headings + assert!( + preamble.contains("## Stage: s1"), + "should show all stages including s1" + ); + assert!( + preamble.contains("## Stage: s6"), + "should show all stages including s6" + ); assert!(!preamble.contains("omitted"), "should not omit any stages"); } @@ -565,7 +1116,10 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("connection timeout"), "should include failure reason"); + assert!( + preamble.contains("connection timeout"), + "should include failure reason" + ); } #[test] @@ -573,6 +1127,7 @@ mod tests { let graph = Graph::new("test"); let context = Context::new(); context.set("graph.goal", serde_json::json!("Build")); + context.set("user.name", serde_json::json!("alice")); let completed_nodes: Vec = Vec::new(); let node_outcomes: HashMap = HashMap::new(); @@ -584,7 +1139,258 @@ mod tests { &node_outcomes, ); - assert!(preamble.contains("graph.goal"), "should include context values"); + assert!( + !preamble.contains("graph.goal"), + "should exclude graph.* from context" + ); + // Table format for summary:high + assert!( + preamble.contains("| user.name |"), + "should include context values as table" + ); + } + + // --- summary:high handler-specific --- + + #[test] + fn summary_high_produces_stage_sections() { + let graph = Graph::new("test"); + let context = Context::new(); + let completed_nodes = vec!["start".to_string(), "work".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + node_outcomes.insert("start".to_string(), Outcome::success()); + node_outcomes.insert("work".to_string(), Outcome::success()); + + let preamble = build_preamble( + "summary:high", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("## Stage: start"), + "should have stage heading for start" + ); + assert!( + preamble.contains("## Stage: work"), + "should have stage heading for work" + ); + } + + #[test] + fn summary_high_script_stage_full_detail() { + let mut graph = Graph::new("test"); + let mut run_tests = Node::new("run_tests"); + run_tests.attrs.insert( + "shape".to_string(), + AttrValue::String("parallelogram".to_string()), + ); + run_tests.attrs.insert( + "script".to_string(), + AttrValue::String("make test".to_string()), + ); + graph.nodes.insert("run_tests".to_string(), run_tests); + + let context = Context::new(); + let completed_nodes = vec!["run_tests".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.context_updates.insert( + "script.output".to_string(), + serde_json::json!("All tests passed\n"), + ); + outcome.context_updates.insert( + "script.stderr".to_string(), + serde_json::json!("warning: unused var\n"), + ); + node_outcomes.insert("run_tests".to_string(), outcome); + + let preamble = build_preamble( + "summary:high", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("## Stage: run_tests"), + "should have stage heading" + ); + assert!(preamble.contains("Handler: script"), "should show handler"); + assert!( + preamble.contains("Script: `make test`"), + "should show script command" + ); + assert!( + preamble.contains("All tests passed"), + "should include stdout" + ); + assert!( + preamble.contains("warning: unused var"), + "should include stderr" + ); + } + + #[test] + fn summary_high_codergen_stage_with_response_preview() { + let mut graph = Graph::new("test"); + let mut report = Node::new("report"); + report.attrs.insert( + "shape".to_string(), + AttrValue::String("box".to_string()), + ); + graph.nodes.insert("report".to_string(), report); + + let context = Context::new(); + let completed_nodes = vec!["report".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.usage = Some(StageUsage { + model: "claude-sonnet-4-20250514".to_string(), + input_tokens: 1500, + output_tokens: 300, + cache_read_tokens: None, + cache_write_tokens: None, + reasoning_tokens: None, + cost: None, + }); + outcome.files_touched = vec!["src/lib.rs".to_string()]; + outcome.context_updates.insert( + "response.report".to_string(), + serde_json::json!("The tests all pass successfully."), + ); + node_outcomes.insert("report".to_string(), outcome); + + let preamble = build_preamble( + "summary:high", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("## Stage: report"), + "should have stage heading" + ); + assert!( + preamble.contains("Handler: codergen"), + "should show handler" + ); + assert!( + preamble.contains("Model: claude-sonnet-4-20250514"), + "should show model" + ); + assert!(preamble.contains("1.5k in"), "should show formatted tokens"); + assert!( + preamble.contains("Files touched: src/lib.rs"), + "should show files" + ); + assert!( + preamble.contains("The tests all pass"), + "should include response" + ); + } + + #[test] + fn summary_high_context_as_table() { + let graph = Graph::new("test"); + let context = Context::new(); + context.set("user.name", serde_json::json!("alice")); + context.set("custom.key", serde_json::json!("value")); + let completed_nodes: Vec = Vec::new(); + let node_outcomes: HashMap = HashMap::new(); + + let preamble = build_preamble( + "summary:high", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("## Current context"), + "should have context table heading" + ); + assert!( + preamble.contains("| Key | Value |"), + "should have table header" + ); + assert!( + preamble.contains("| user.name | alice |"), + "should have context row" + ); + } + + #[test] + fn summary_high_pipeline_progress_count() { + let mut graph = Graph::new("test"); + // Create 4 nodes total (including start/exit) + let start = Node::new("start"); + graph.nodes.insert("start".to_string(), start); + let work = Node::new("work"); + graph.nodes.insert("work".to_string(), work); + let test = Node::new("test"); + graph.nodes.insert("test".to_string(), test); + let exit = Node::new("exit"); + graph.nodes.insert("exit".to_string(), exit); + + let context = Context::new(); + let completed_nodes = vec!["start".to_string(), "work".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + node_outcomes.insert("start".to_string(), Outcome::success()); + node_outcomes.insert("work".to_string(), Outcome::success()); + + let preamble = build_preamble( + "summary:high", + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("2 of 4 stages completed"), + "should show pipeline progress with total node count, got:\n{preamble}" + ); + } + + // --- format_token_count --- + + #[test] + fn format_token_count_formatting() { + assert_eq!(format_token_count(500), "500"); + assert_eq!(format_token_count(999), "999"); + assert_eq!(format_token_count(1000), "1.0k"); + assert_eq!(format_token_count(1234), "1.2k"); + assert_eq!(format_token_count(1500), "1.5k"); + assert_eq!(format_token_count(10000), "10.0k"); + } + + // --- is_context_key_excluded --- + + #[test] + fn is_context_key_excluded_checks() { + assert!(is_context_key_excluded("internal.fidelity")); + assert!(is_context_key_excluded("internal.retry_count.plan")); + assert!(is_context_key_excluded("current_node")); + assert!(is_context_key_excluded("current.preamble")); + assert!(is_context_key_excluded("graph.default_fidelity")); + assert!(is_context_key_excluded("graph.goal")); + assert!(is_context_key_excluded("thread.main.current_node")); + assert!(is_context_key_excluded("response.plan")); + assert!(is_context_key_excluded("run_id")); + assert!(is_context_key_excluded("outcome")); + assert!(is_context_key_excluded("last_stage")); + assert!(is_context_key_excluded("last_response")); + assert!(is_context_key_excluded("preferred_label")); + assert!(!is_context_key_excluded("user.name")); + assert!(!is_context_key_excluded("custom.key")); + assert!(!is_context_key_excluded("script.output")); } // --- unknown fidelity mode --- @@ -611,7 +1417,10 @@ mod tests { // Should behave like compact: include goal and stages assert!(preamble.contains("Test fallback"), "should contain the goal"); - assert!(preamble.contains("step1"), "should list completed stages like compact"); + assert!( + preamble.contains("step1"), + "should list completed stages like compact" + ); } // --- empty state --- @@ -631,6 +1440,9 @@ mod tests { &node_outcomes, ); - assert!(!preamble.contains("Completed stages"), "should not show stages header when empty"); + assert!( + !preamble.contains("Completed stages"), + "should not show stages header when empty" + ); } } diff --git a/crates/attractor/tests/integration.rs b/crates/attractor/tests/integration.rs index 58e08634e..75e4a2ab7 100644 --- a/crates/attractor/tests/integration.rs +++ b/crates/attractor/tests/integration.rs @@ -4599,7 +4599,7 @@ async fn fidelity_compact_preamble_includes_completed_stages_and_context() { "compact preamble should contain the goal" ); assert!( - step_b_preamble.contains("Completed stages:"), + step_b_preamble.contains("## Completed stages"), "compact preamble should include completed stages section" ); assert!( @@ -4672,10 +4672,17 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { let preambles_med = captures_med.preambles.lock().unwrap(); let med_preamble = &preambles_med[1].1; - // summary:medium should include "Context values:" section (graph.goal is always set) + // summary:medium should include stage details (unlike summary:low which omits them) assert!( - med_preamble.contains("Context values:"), - "summary:medium preamble should include context values section" + med_preamble.contains("step_a"), + "summary:medium preamble should include completed stage step_a" + ); + // Verify medium and low differ: medium shows more recent stages + let preambles_low = captures_low.preambles.lock().unwrap(); + let low_preamble = &preambles_low[1].1; + assert!( + !low_preamble.contains("## Context"), + "summary:low preamble should not include context section" ); } @@ -6729,11 +6736,12 @@ async fn run_fidelity_prompt_pipeline(fidelity: &str) -> String { async fn fidelity_prompt_compact() { let prompt = run_fidelity_prompt_pipeline("compact").await; - // Preamble should contain goal, completed stages, and context values + // Preamble should contain goal, completed stages with handler details, and context assert!(prompt.contains("Validate the build"), "compact: should contain goal"); - assert!(prompt.contains("Completed stages:"), "compact: should list completed stages"); - assert!(prompt.contains("run_tests"), "compact: should mention run_tests node"); - assert!(prompt.contains("Context values:"), "compact: should include context values section"); + assert!(prompt.contains("## Completed stages"), "compact: should list completed stages"); + assert!(prompt.contains("**run_tests**"), "compact: should mention run_tests node in bold"); + assert!(prompt.contains("Script:"), "compact: should show script sub-item for run_tests"); + assert!(prompt.contains("Stdout:"), "compact: should show stdout sub-item for run_tests"); // Original prompt at the end assert!( @@ -6779,7 +6787,7 @@ async fn fidelity_prompt_summary_medium() { // summary:medium includes goal, stages, and context values assert!(prompt.contains("Validate the build"), "summary:medium: should contain goal"); assert!(prompt.contains("run_tests"), "summary:medium: should mention run_tests"); - assert!(prompt.contains("Context values:"), "summary:medium: should include context values"); + assert!(prompt.contains("## Context"), "summary:medium: should include context section"); // Original prompt at the end assert!( @@ -6792,10 +6800,11 @@ async fn fidelity_prompt_summary_medium() { async fn fidelity_prompt_summary_high() { let prompt = run_fidelity_prompt_pipeline("summary:high").await; - // summary:high includes goal, all stages, context values + // summary:high includes goal, all stages as ## Stage headings assert!(prompt.contains("Validate the build"), "summary:high: should contain goal"); - assert!(prompt.contains("run_tests"), "summary:high: should mention run_tests"); - assert!(prompt.contains("Context values:"), "summary:high: should include context values"); + assert!(prompt.contains("## Stage: run_tests"), "summary:high: should have stage heading for run_tests"); + assert!(prompt.contains("## Stage: start"), "summary:high: should have stage heading for start"); + assert!(prompt.contains("Pipeline progress:"), "summary:high: should show pipeline progress"); // Original prompt at the end assert!(