mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Add shape selectors, LLM stream timeouts, MultiSelect questions, and test coverage
- Stylesheet: add bare-word Shape selector (specificity between Universal and Class) - LLM: apply per_step timeout to connection and total timeout to stream (Section 4.7) - Interviewer: add MultiSelect question type alongside MultipleChoice - Session: move SessionStart/SessionEnd to initialize()/close(), deduplicate close logic - Engine: return Ok(fail outcome) instead of error when goal gate unsatisfied with no retry_target - Docker: mark Docker-dependent tests with #[ignore] - Validation: add extensive unit test coverage for all rule types - Integration: update tests to match engine/fidelity changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
a3a7f9fb5c
commit
3dfa0f9044
10 changed files with 1133 additions and 99 deletions
|
|
@ -661,6 +661,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn full_lifecycle() {
|
||||
let _docker = require_docker();
|
||||
let host_dir = std::env::temp_dir().join(format!("docker_env_test_{}", uuid::Uuid::new_v4()));
|
||||
|
|
@ -717,6 +718,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn timeout_handling() {
|
||||
let _docker = require_docker();
|
||||
let host_dir = std::env::temp_dir().join(format!("docker_timeout_test_{}", uuid::Uuid::new_v4()));
|
||||
|
|
@ -735,6 +737,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn special_characters_in_write() {
|
||||
let _docker = require_docker();
|
||||
let host_dir = std::env::temp_dir().join(format!("docker_special_test_{}", uuid::Uuid::new_v4()));
|
||||
|
|
@ -756,6 +759,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn path_resolution() {
|
||||
let _docker = require_docker();
|
||||
let host_dir = std::env::temp_dir().join(format!("docker_path_test_{}", uuid::Uuid::new_v4()));
|
||||
|
|
@ -779,6 +783,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn cleanup_idempotent() {
|
||||
let _docker = require_docker();
|
||||
let host_dir = std::env::temp_dir().join(format!("docker_cleanup_test_{}", uuid::Uuid::new_v4()));
|
||||
|
|
|
|||
|
|
@ -64,6 +64,9 @@ impl Session {
|
|||
/// Initialize session by discovering project docs and capturing environment context.
|
||||
/// Call before `process_input`.
|
||||
pub async fn initialize(&mut self) {
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionStart, self.id.clone(), EventData::Empty);
|
||||
|
||||
let doc_root = self
|
||||
.config
|
||||
.git_root
|
||||
|
|
@ -166,7 +169,11 @@ impl Session {
|
|||
}
|
||||
|
||||
pub fn close(&mut self) {
|
||||
self.state = SessionState::Closed;
|
||||
if self.state != SessionState::Closed {
|
||||
self.state = SessionState::Closed;
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionEnd, self.id.clone(), EventData::Empty);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_reasoning_effort(&mut self, effort: Option<String>) {
|
||||
|
|
@ -186,9 +193,6 @@ impl Session {
|
|||
return Err(AgentError::SessionClosed);
|
||||
}
|
||||
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionStart, self.id.clone(), EventData::Empty);
|
||||
|
||||
// Process the initial input, then drain any followups
|
||||
self.run_single_input(input).await?;
|
||||
loop {
|
||||
|
|
@ -202,8 +206,6 @@ impl Session {
|
|||
}
|
||||
|
||||
self.state = SessionState::Idle;
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionEnd, self.id.clone(), EventData::Empty);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -253,9 +255,7 @@ impl Session {
|
|||
|
||||
// Check abort flag
|
||||
if self.abort_flag.load(Ordering::SeqCst) {
|
||||
self.state = SessionState::Closed;
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionEnd, self.id.clone(), EventData::Empty);
|
||||
self.close();
|
||||
return Err(AgentError::Aborted);
|
||||
}
|
||||
|
||||
|
|
@ -322,12 +322,10 @@ impl Session {
|
|||
}
|
||||
|
||||
// If aborted during streaming, drop the stream to cancel the HTTP
|
||||
// connection, then emit SessionEnd before returning.
|
||||
// connection, then close the session before returning.
|
||||
if self.abort_flag.load(Ordering::SeqCst) {
|
||||
drop(event_stream);
|
||||
self.state = SessionState::Closed;
|
||||
self.event_emitter
|
||||
.emit(EventKind::SessionEnd, self.id.clone(), EventData::Empty);
|
||||
self.close();
|
||||
return Err(AgentError::Aborted);
|
||||
}
|
||||
|
||||
|
|
@ -923,7 +921,9 @@ mod tests {
|
|||
let mut session = make_session(vec![text_response("Hello")]).await;
|
||||
let mut rx = session.subscribe();
|
||||
|
||||
session.initialize().await;
|
||||
session.process_input("Hi").await.unwrap();
|
||||
session.close();
|
||||
|
||||
// Collect events
|
||||
let mut events = Vec::new();
|
||||
|
|
@ -931,6 +931,7 @@ mod tests {
|
|||
events.push(event.kind.clone());
|
||||
}
|
||||
|
||||
assert!(events.contains(&EventKind::SessionStart));
|
||||
assert!(events.contains(&EventKind::UserInput));
|
||||
assert!(events.contains(&EventKind::AssistantTextEnd));
|
||||
assert!(events.contains(&EventKind::SessionEnd));
|
||||
|
|
@ -1446,19 +1447,24 @@ mod tests {
|
|||
let mut session = make_session(responses).await;
|
||||
let mut rx = session.subscribe();
|
||||
|
||||
session.initialize().await;
|
||||
session.process_input("one").await.unwrap();
|
||||
session.process_input("two").await.unwrap();
|
||||
session.close();
|
||||
|
||||
let mut session_start_count = 0;
|
||||
let mut session_end_count = 0;
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
if event.kind == EventKind::SessionStart {
|
||||
session_start_count += 1;
|
||||
}
|
||||
if event.kind == EventKind::SessionEnd {
|
||||
session_end_count += 1;
|
||||
}
|
||||
}
|
||||
// Each process_input emits SESSION_START currently -- this should be 1 per input call
|
||||
// The spec says SESSION_START is "session created", but since our Session doesn't
|
||||
// emit at creation, we accept one per process_input call as the session boundary.
|
||||
assert_eq!(session_start_count, 2);
|
||||
// SESSION_START is emitted once during initialize(), SESSION_END once during close()
|
||||
assert_eq!(session_start_count, 1);
|
||||
assert_eq!(session_end_count, 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
|
|
@ -760,7 +760,7 @@ impl PipelineEngine {
|
|||
error: error_msg.clone(),
|
||||
duration_ms,
|
||||
});
|
||||
return Err(AttractorError::Engine(error_msg));
|
||||
return Ok(Outcome::fail(error_msg));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ impl Interviewer for AutoApproveInterviewer {
|
|||
async fn ask(&self, question: Question) -> Answer {
|
||||
match question.question_type {
|
||||
QuestionType::YesNo | QuestionType::Confirmation => Answer::yes(),
|
||||
QuestionType::MultipleChoice => question.options.first().map_or_else(
|
||||
QuestionType::MultipleChoice | QuestionType::MultiSelect => question.options.first().map_or_else(
|
||||
|| Answer::text("auto-approved"),
|
||||
|first| Answer {
|
||||
value: AnswerValue::Selected(first.key.clone()),
|
||||
|
|
@ -85,4 +85,4 @@ mod tests {
|
|||
assert_eq!(answer.value, AnswerValue::Text("auto-approved".to_string()));
|
||||
assert_eq!(answer.text, Some("auto-approved".to_string()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -66,7 +66,7 @@ impl Interviewer for ConsoleInterviewer {
|
|||
);
|
||||
|
||||
match question.question_type {
|
||||
QuestionType::MultipleChoice => {
|
||||
QuestionType::MultipleChoice | QuestionType::MultiSelect => {
|
||||
for (i, opt) in question.options.iter().enumerate() {
|
||||
eprintln!(
|
||||
" {dim}[{reset}{bold}{}{reset}{dim}]{reset} {} - {}",
|
||||
|
|
@ -182,4 +182,4 @@ mod tests {
|
|||
let result = find_matching_option("5", &options);
|
||||
assert!(result.is_none());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -16,6 +16,7 @@ use serde::{Deserialize, Serialize};
|
|||
pub enum QuestionType {
|
||||
YesNo,
|
||||
MultipleChoice,
|
||||
MultiSelect,
|
||||
Freeform,
|
||||
Confirmation,
|
||||
}
|
||||
|
|
@ -256,6 +257,12 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn question_type_multi_select_exists() {
|
||||
let q = Question::new("Pick many:", QuestionType::MultiSelect);
|
||||
assert_eq!(q.question_type, QuestionType::MultiSelect);
|
||||
}
|
||||
|
||||
/// A slow interviewer that waits before answering -- for testing timeouts.
|
||||
struct SlowInterviewer;
|
||||
|
||||
|
|
@ -296,4 +303,4 @@ mod tests {
|
|||
let answer = ask_with_timeout(&interviewer, q).await;
|
||||
assert_eq!(answer.value, AnswerValue::Yes);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -6,9 +6,11 @@ use crate::graph::types::{AttrValue, Graph};
|
|||
pub enum Selector {
|
||||
/// `*` -- matches all nodes, specificity 0.
|
||||
Universal,
|
||||
/// `.classname` -- matches nodes with that class, specificity 1.
|
||||
/// Bare word -- matches nodes by shape name, specificity 1.
|
||||
Shape(String),
|
||||
/// `.classname` -- matches nodes with that class, specificity 2.
|
||||
Class(String),
|
||||
/// `#nodeid` -- matches a specific node, specificity 2.
|
||||
/// `#nodeid` -- matches a specific node, specificity 3.
|
||||
Id(String),
|
||||
}
|
||||
|
||||
|
|
@ -17,8 +19,9 @@ impl Selector {
|
|||
pub const fn specificity(&self) -> u8 {
|
||||
match self {
|
||||
Self::Universal => 0,
|
||||
Self::Class(_) => 1,
|
||||
Self::Id(_) => 2,
|
||||
Self::Shape(_) => 1,
|
||||
Self::Class(_) => 2,
|
||||
Self::Id(_) => 3,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -112,10 +115,19 @@ fn parse_selector(remaining: &mut &str) -> Result<Selector, AttractorError> {
|
|||
*remaining = remaining[end..].trim();
|
||||
Ok(Selector::Class(class))
|
||||
} else {
|
||||
Err(AttractorError::Stylesheet(format!(
|
||||
"expected selector ('*', '#id', or '.class'), got: {:?}",
|
||||
&remaining[..remaining.len().min(20)]
|
||||
)))
|
||||
// Bare word: shape selector
|
||||
let end = remaining
|
||||
.find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-')
|
||||
.unwrap_or(remaining.len());
|
||||
if end == 0 {
|
||||
return Err(AttractorError::Stylesheet(format!(
|
||||
"expected selector ('*', '#id', '.class', or shape name), got: {:?}",
|
||||
&remaining[..remaining.len().min(20)]
|
||||
)));
|
||||
}
|
||||
let shape = remaining[..end].to_string();
|
||||
*remaining = remaining[end..].trim();
|
||||
Ok(Selector::Shape(shape))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -189,6 +201,7 @@ pub fn apply_stylesheet(stylesheet: &Stylesheet, graph: &mut Graph) {
|
|||
let node = &graph.nodes[node_id.as_str()];
|
||||
let matches = match &rule.selector {
|
||||
Selector::Universal => true,
|
||||
Selector::Shape(shape) => node.shape() == shape,
|
||||
Selector::Class(cls) => node.classes.contains(cls),
|
||||
Selector::Id(id) => node_id == id,
|
||||
};
|
||||
|
|
@ -373,8 +386,9 @@ mod tests {
|
|||
#[test]
|
||||
fn selector_specificity_values() {
|
||||
assert_eq!(Selector::Universal.specificity(), 0);
|
||||
assert_eq!(Selector::Class("x".into()).specificity(), 1);
|
||||
assert_eq!(Selector::Id("x".into()).specificity(), 2);
|
||||
assert_eq!(Selector::Shape("box".into()).specificity(), 1);
|
||||
assert_eq!(Selector::Class("x".into()).specificity(), 2);
|
||||
assert_eq!(Selector::Id("x".into()).specificity(), 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -428,9 +442,64 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn parse_bare_word_selector_is_error() {
|
||||
let result = parse_stylesheet("box { llm_model: opus; }");
|
||||
assert!(result.is_err());
|
||||
fn parse_shape_selector() {
|
||||
let ss = parse_stylesheet("box { llm_model: opus; }").unwrap();
|
||||
assert_eq!(ss.rules.len(), 1);
|
||||
assert_eq!(ss.rules[0].selector, Selector::Shape("box".into()));
|
||||
assert_eq!(ss.rules[0].declarations[0].value, "opus");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_shape_selector_to_matching_nodes() {
|
||||
let ss = parse_stylesheet("box { llm_model: opus; }").unwrap();
|
||||
let mut graph = Graph::new("test");
|
||||
|
||||
// Default shape is "box"
|
||||
let box_node = Node::new("a");
|
||||
graph.nodes.insert("a".into(), box_node);
|
||||
|
||||
let mut diamond_node = Node::new("b");
|
||||
diamond_node
|
||||
.attrs
|
||||
.insert("shape".into(), AttrValue::String("Mdiamond".into()));
|
||||
graph.nodes.insert("b".into(), diamond_node);
|
||||
|
||||
apply_stylesheet(&ss, &mut graph);
|
||||
|
||||
assert_eq!(
|
||||
graph.nodes["a"].attrs.get("llm_model"),
|
||||
Some(&AttrValue::String("opus".into()))
|
||||
);
|
||||
// Mdiamond node should NOT get the box rule
|
||||
assert_eq!(graph.nodes["b"].attrs.get("llm_model"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shape_overrides_universal_specificity() {
|
||||
let ss =
|
||||
parse_stylesheet("* { llm_model: sonnet; } box { llm_model: opus; }").unwrap();
|
||||
let mut graph = Graph::new("test");
|
||||
graph.nodes.insert("a".into(), Node::new("a")); // default shape = box
|
||||
apply_stylesheet(&ss, &mut graph);
|
||||
assert_eq!(
|
||||
graph.nodes["a"].attrs.get("llm_model"),
|
||||
Some(&AttrValue::String("opus".into()))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn class_overrides_shape_specificity() {
|
||||
let ss =
|
||||
parse_stylesheet("box { llm_model: opus; } .fast { llm_model: flash; }").unwrap();
|
||||
let mut graph = Graph::new("test");
|
||||
let mut node = Node::new("a");
|
||||
node.classes.push("fast".into());
|
||||
graph.nodes.insert("a".into(), node);
|
||||
apply_stylesheet(&ss, &mut graph);
|
||||
assert_eq!(
|
||||
graph.nodes["a"].attrs.get("llm_model"),
|
||||
Some(&AttrValue::String("flash".into()))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -461,4 +530,4 @@ mod tests {
|
|||
Some(&AttrValue::String("sonnet".into()))
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1234,4 +1234,933 @@ mod tests {
|
|||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
}
|
||||
|
||||
// --- Additional coverage: condition_syntax invalid case ---
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_invalid_clause() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String("bad clause here".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_not_equals() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String("outcome!=failure".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_empty_condition() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String(String::new()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_compound_and() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String("outcome=success && retries=0".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: terminal_node two terminals ---
|
||||
|
||||
#[test]
|
||||
fn terminal_node_rule_two_terminals() {
|
||||
let mut g = Graph::new("test");
|
||||
let mut e1 = Node::new("e1");
|
||||
e1.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("Msquare".to_string()));
|
||||
let mut e2 = Node::new("e2");
|
||||
e2.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("Msquare".to_string()));
|
||||
g.nodes.insert("e1".to_string(), e1);
|
||||
g.nodes.insert("e2".to_string(), e2);
|
||||
let rule = TerminalNodeRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert!(d[0].message.contains("exactly one"));
|
||||
}
|
||||
|
||||
// --- Additional coverage: edge_target_exists missing source ---
|
||||
|
||||
#[test]
|
||||
fn edge_target_exists_rule_missing_source() {
|
||||
let mut g = minimal_graph();
|
||||
g.edges.push(Edge::new("nonexistent_source", "exit"));
|
||||
let rule = EdgeTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert!(d[0].message.contains("nonexistent_source"));
|
||||
}
|
||||
|
||||
// --- Additional coverage: reachability no start node ---
|
||||
|
||||
#[test]
|
||||
fn reachability_rule_no_start_node() {
|
||||
let mut g = Graph::new("test");
|
||||
g.nodes.insert("orphan".to_string(), Node::new("orphan"));
|
||||
let rule = ReachabilityRule;
|
||||
let d = rule.apply(&g);
|
||||
// No start node found, rule returns empty
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: retry_target_exists fallback and graph-level ---
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_fallback_missing() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("nonexistent".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert!(d[0].message.contains("fallback_retry_target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_fallback_valid() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("start".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_graph_level_missing() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"retry_target".to_string(),
|
||||
AttrValue::String("nonexistent".to_string()),
|
||||
);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert!(d[0].message.contains("Graph"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_graph_level_valid() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"retry_target".to_string(),
|
||||
AttrValue::String("start".to_string()),
|
||||
);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_graph_fallback_missing() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("nonexistent".to_string()),
|
||||
);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert!(d[0].message.contains("fallback_retry_target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_graph_fallback_valid() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("exit".to_string()),
|
||||
);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: goal_gate_has_retry with graph-level retry ---
|
||||
|
||||
#[test]
|
||||
fn goal_gate_has_retry_rule_with_graph_retry_target() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs
|
||||
.insert("goal_gate".to_string(), AttrValue::Boolean(true));
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
g.attrs.insert(
|
||||
"retry_target".to_string(),
|
||||
AttrValue::String("start".to_string()),
|
||||
);
|
||||
let rule = GoalGateHasRetryRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goal_gate_has_retry_rule_with_fallback_retry_target() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs
|
||||
.insert("goal_gate".to_string(), AttrValue::Boolean(true));
|
||||
node.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("start".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = GoalGateHasRetryRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goal_gate_has_retry_rule_not_goal_gate() {
|
||||
let mut g = minimal_graph();
|
||||
let node = Node::new("work");
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = GoalGateHasRetryRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: prompt_on_llm_nodes with label ---
|
||||
|
||||
#[test]
|
||||
fn prompt_on_llm_nodes_rule_with_label() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"label".to_string(),
|
||||
AttrValue::String("Do something".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = PromptOnLlmNodesRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prompt_on_llm_nodes_rule_non_codergen_no_warning() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("gate");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("hexagon".to_string()),
|
||||
);
|
||||
g.nodes.insert("gate".to_string(), node);
|
||||
let rule = PromptOnLlmNodesRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: fidelity_valid edge and graph-level ---
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_invalid_edge_fidelity() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("bogus".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = FidelityValidRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert!(d[0].edge.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_valid_edge_fidelity() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("compact".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = FidelityValidRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_invalid_graph_default() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"default_fidelity".to_string(),
|
||||
AttrValue::String("wrong".to_string()),
|
||||
);
|
||||
let rule = FidelityValidRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert!(d[0].message.contains("default_fidelity"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_valid_graph_default() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"default_fidelity".to_string(),
|
||||
AttrValue::String("summary:high".to_string()),
|
||||
);
|
||||
let rule = FidelityValidRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_all_summary_modes() {
|
||||
let rule = FidelityValidRule;
|
||||
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("w1");
|
||||
node.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("summary:low".to_string()),
|
||||
);
|
||||
g.nodes.insert("w1".to_string(), node);
|
||||
assert!(rule.apply(&g).is_empty());
|
||||
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("w2");
|
||||
node.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("summary:medium".to_string()),
|
||||
);
|
||||
g.nodes.insert("w2".to_string(), node);
|
||||
assert!(rule.apply(&g).is_empty());
|
||||
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("w3");
|
||||
node.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("truncate".to_string()),
|
||||
);
|
||||
g.nodes.insert("w3".to_string(), node);
|
||||
assert!(rule.apply(&g).is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: freeform_edge_count non-wait.human ---
|
||||
|
||||
#[test]
|
||||
fn freeform_edge_count_rule_non_wait_human_ignored() {
|
||||
let mut g = minimal_graph();
|
||||
// Regular codergen node (box shape) with multiple freeform edges should not trigger
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.nodes.insert("b".to_string(), Node::new("b"));
|
||||
g.nodes.insert("work".to_string(), Node::new("work"));
|
||||
|
||||
let mut e1 = Edge::new("work", "a");
|
||||
e1.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(true));
|
||||
let mut e2 = Edge::new("work", "b");
|
||||
e2.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(true));
|
||||
g.edges.push(e1);
|
||||
g.edges.push(e2);
|
||||
|
||||
let rule = FreeformEdgeCountRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn freeform_edge_count_rule_zero_freeform() {
|
||||
let mut g = minimal_graph();
|
||||
let mut gate = Node::new("gate");
|
||||
gate.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("wait.human".to_string()),
|
||||
);
|
||||
g.nodes.insert("gate".to_string(), gate);
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.edges.push(Edge::new("gate", "a"));
|
||||
|
||||
let rule = FreeformEdgeCountRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: stylesheet_syntax no stylesheet ---
|
||||
|
||||
#[test]
|
||||
fn stylesheet_syntax_rule_no_stylesheet() {
|
||||
let g = minimal_graph();
|
||||
let rule = StylesheetSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: type_known no type attr ---
|
||||
|
||||
#[test]
|
||||
fn type_known_rule_no_type_attr() {
|
||||
let g = minimal_graph();
|
||||
let rule = TypeKnownRule;
|
||||
let d = rule.apply(&g);
|
||||
// Nodes without explicit type attr should not trigger warning
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: start_no_incoming no start node ---
|
||||
|
||||
#[test]
|
||||
fn start_no_incoming_rule_no_start_node() {
|
||||
let g = Graph::new("test");
|
||||
let rule = StartNoIncomingRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- Additional coverage: exit_no_outgoing by id variants ---
|
||||
|
||||
#[test]
|
||||
fn exit_no_outgoing_rule_end_id_with_outgoing() {
|
||||
let mut g = Graph::new("test");
|
||||
let mut start = Node::new("start");
|
||||
start
|
||||
.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string()));
|
||||
g.nodes.insert("start".to_string(), start);
|
||||
let end_node = Node::new("end");
|
||||
g.nodes.insert("end".to_string(), end_node);
|
||||
g.edges.push(Edge::new("start", "end"));
|
||||
g.edges.push(Edge::new("end", "start"));
|
||||
let rule = ExitNoOutgoingRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert_eq!(d[0].node_id, Some("end".to_string()));
|
||||
}
|
||||
|
||||
// --- condition_syntax: bare key (truthy check) is valid ---
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_bare_key_truthy() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String("context.passed".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- condition_syntax: context-prefixed clause with spaces is valid ---
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_context_prefix_with_space() {
|
||||
let mut g = minimal_graph();
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"condition".to_string(),
|
||||
AttrValue::String("context.foo bar".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
// context.-prefixed clauses are allowed even with spaces
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- terminal_node: by "Exit" capitalized id ---
|
||||
|
||||
#[test]
|
||||
fn terminal_node_rule_by_exit_capitalized_id() {
|
||||
let mut g = Graph::new("test");
|
||||
let node = Node::new("Exit");
|
||||
g.nodes.insert("Exit".to_string(), node);
|
||||
let rule = TerminalNodeRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- edge_target_exists: both source and target missing ---
|
||||
|
||||
#[test]
|
||||
fn edge_target_exists_rule_both_missing() {
|
||||
let mut g = minimal_graph();
|
||||
g.edges
|
||||
.push(Edge::new("nonexistent_source", "nonexistent_target"));
|
||||
let rule = EdgeTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 2);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert_eq!(d[1].severity, Severity::Error);
|
||||
}
|
||||
|
||||
// --- reachability: multiple unreachable nodes ---
|
||||
|
||||
#[test]
|
||||
fn reachability_rule_multiple_unreachable() {
|
||||
let mut g = minimal_graph();
|
||||
g.nodes
|
||||
.insert("orphan_a".to_string(), Node::new("orphan_a"));
|
||||
g.nodes
|
||||
.insert("orphan_b".to_string(), Node::new("orphan_b"));
|
||||
let rule = ReachabilityRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 2);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert_eq!(d[1].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- type_known: all known handler types are accepted ---
|
||||
|
||||
#[test]
|
||||
fn type_known_rule_all_known_types_accepted() {
|
||||
let mut g = minimal_graph();
|
||||
|
||||
let mut n1 = Node::new("n1");
|
||||
n1.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("codergen".to_string()),
|
||||
);
|
||||
g.nodes.insert("n1".to_string(), n1);
|
||||
|
||||
let mut n2 = Node::new("n2");
|
||||
n2.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("conditional".to_string()),
|
||||
);
|
||||
g.nodes.insert("n2".to_string(), n2);
|
||||
|
||||
let mut n3 = Node::new("n3");
|
||||
n3.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("parallel".to_string()),
|
||||
);
|
||||
g.nodes.insert("n3".to_string(), n3);
|
||||
|
||||
let mut n4 = Node::new("n4");
|
||||
n4.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("parallel.fan_in".to_string()),
|
||||
);
|
||||
g.nodes.insert("n4".to_string(), n4);
|
||||
|
||||
let mut n5 = Node::new("n5");
|
||||
n5.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("tool".to_string()),
|
||||
);
|
||||
g.nodes.insert("n5".to_string(), n5);
|
||||
|
||||
let mut n6 = Node::new("n6");
|
||||
n6.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("stack.manager_loop".to_string()),
|
||||
);
|
||||
g.nodes.insert("n6".to_string(), n6);
|
||||
|
||||
let rule = TypeKnownRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- prompt_on_llm_nodes: explicit type=codergen without prompt/label ---
|
||||
|
||||
#[test]
|
||||
fn prompt_on_llm_nodes_rule_explicit_codergen_type_no_prompt() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("codergen".to_string()),
|
||||
);
|
||||
// No shape=box, but explicit type=codergen
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("diamond".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = PromptOnLlmNodesRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- goal_gate_has_retry: satisfied by graph-level fallback_retry_target ---
|
||||
|
||||
#[test]
|
||||
fn goal_gate_has_retry_rule_with_graph_fallback_retry_target() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs
|
||||
.insert("goal_gate".to_string(), AttrValue::Boolean(true));
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
g.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("start".to_string()),
|
||||
);
|
||||
let rule = GoalGateHasRetryRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- freeform_edge_count: with explicit type=wait.human ---
|
||||
|
||||
#[test]
|
||||
fn freeform_edge_count_rule_explicit_type_two_freeform() {
|
||||
let mut g = minimal_graph();
|
||||
let mut gate = Node::new("gate");
|
||||
gate.attrs.insert(
|
||||
"type".to_string(),
|
||||
AttrValue::String("wait.human".to_string()),
|
||||
);
|
||||
g.nodes.insert("gate".to_string(), gate);
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.nodes.insert("b".to_string(), Node::new("b"));
|
||||
|
||||
let mut e1 = Edge::new("gate", "a");
|
||||
e1.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(true));
|
||||
let mut e2 = Edge::new("gate", "b");
|
||||
e2.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(true));
|
||||
g.edges.push(e1);
|
||||
g.edges.push(e2);
|
||||
|
||||
let rule = FreeformEdgeCountRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
}
|
||||
|
||||
// --- exit_no_outgoing: by "Exit" capitalized id ---
|
||||
|
||||
#[test]
|
||||
fn exit_no_outgoing_rule_exit_capitalized_with_outgoing() {
|
||||
let mut g = Graph::new("test");
|
||||
let mut start = Node::new("start");
|
||||
start
|
||||
.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string()));
|
||||
g.nodes.insert("start".to_string(), start);
|
||||
let exit_node = Node::new("Exit");
|
||||
g.nodes.insert("Exit".to_string(), exit_node);
|
||||
g.edges.push(Edge::new("start", "Exit"));
|
||||
g.edges.push(Edge::new("Exit", "start"));
|
||||
let rule = ExitNoOutgoingRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert_eq!(d[0].node_id, Some("Exit".to_string()));
|
||||
}
|
||||
|
||||
// --- exit_no_outgoing: by "End" capitalized id ---
|
||||
|
||||
#[test]
|
||||
fn exit_no_outgoing_rule_end_capitalized_with_outgoing() {
|
||||
let mut g = Graph::new("test");
|
||||
let mut start = Node::new("start");
|
||||
start
|
||||
.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string()));
|
||||
g.nodes.insert("start".to_string(), start);
|
||||
let end_node = Node::new("End");
|
||||
g.nodes.insert("End".to_string(), end_node);
|
||||
g.edges.push(Edge::new("start", "End"));
|
||||
g.edges.push(Edge::new("End", "start"));
|
||||
let rule = ExitNoOutgoingRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert_eq!(d[0].node_id, Some("End".to_string()));
|
||||
}
|
||||
|
||||
// --- stylesheet_syntax: valid multi-rule stylesheet ---
|
||||
|
||||
#[test]
|
||||
fn stylesheet_syntax_rule_multi_rule_valid() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"model_stylesheet".to_string(),
|
||||
AttrValue::String(
|
||||
"* { llm_model: gpt-4; } .fast { llm_model: gpt-3.5; reasoning_effort: low; }"
|
||||
.to_string(),
|
||||
),
|
||||
);
|
||||
let rule = StylesheetSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- fidelity_valid: multiple simultaneous violations ---
|
||||
|
||||
#[test]
|
||||
fn fidelity_valid_rule_node_and_edge_and_graph_all_invalid() {
|
||||
let mut g = minimal_graph();
|
||||
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("invalid_node".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
|
||||
let mut edge = Edge::new("start", "exit");
|
||||
edge.attrs.insert(
|
||||
"fidelity".to_string(),
|
||||
AttrValue::String("invalid_edge".to_string()),
|
||||
);
|
||||
g.edges = vec![edge];
|
||||
|
||||
g.attrs.insert(
|
||||
"default_fidelity".to_string(),
|
||||
AttrValue::String("invalid_graph".to_string()),
|
||||
);
|
||||
|
||||
let rule = FidelityValidRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 3);
|
||||
}
|
||||
|
||||
// --- retry_target_exists: both retry_target and fallback on same node, both invalid ---
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_both_node_targets_invalid() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"retry_target".to_string(),
|
||||
AttrValue::String("missing_a".to_string()),
|
||||
);
|
||||
node.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("missing_b".to_string()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 2);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert_eq!(d[1].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- retry_target_exists: both graph-level targets invalid ---
|
||||
|
||||
#[test]
|
||||
fn retry_target_exists_rule_both_graph_targets_invalid() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"retry_target".to_string(),
|
||||
AttrValue::String("missing_a".to_string()),
|
||||
);
|
||||
g.attrs.insert(
|
||||
"fallback_retry_target".to_string(),
|
||||
AttrValue::String("missing_b".to_string()),
|
||||
);
|
||||
let rule = RetryTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 2);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
assert_eq!(d[1].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- start_no_incoming: multiple incoming edges ---
|
||||
|
||||
#[test]
|
||||
fn start_no_incoming_rule_multiple_incoming() {
|
||||
let mut g = minimal_graph();
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.edges.push(Edge::new("exit", "start"));
|
||||
g.edges.push(Edge::new("a", "start"));
|
||||
let rule = StartNoIncomingRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Error);
|
||||
assert!(d[0].message.contains("2"));
|
||||
}
|
||||
|
||||
// --- prompt_on_llm_nodes: empty prompt string still triggers ---
|
||||
|
||||
#[test]
|
||||
fn prompt_on_llm_nodes_rule_empty_prompt_no_label() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"prompt".to_string(),
|
||||
AttrValue::String(String::new()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = PromptOnLlmNodesRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- prompt_on_llm_nodes: empty label string still triggers ---
|
||||
|
||||
#[test]
|
||||
fn prompt_on_llm_nodes_rule_empty_label_no_prompt() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs.insert(
|
||||
"label".to_string(),
|
||||
AttrValue::String(String::new()),
|
||||
);
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = PromptOnLlmNodesRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
// --- condition_syntax: no condition attribute at all ---
|
||||
|
||||
#[test]
|
||||
fn condition_syntax_rule_no_condition_attr() {
|
||||
let g = minimal_graph();
|
||||
let rule = ConditionSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- freeform_edge_count: freeform=false does not count ---
|
||||
|
||||
#[test]
|
||||
fn freeform_edge_count_rule_freeform_false_ignored() {
|
||||
let mut g = minimal_graph();
|
||||
let mut gate = Node::new("gate");
|
||||
gate.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("hexagon".to_string()),
|
||||
);
|
||||
g.nodes.insert("gate".to_string(), gate);
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.nodes.insert("b".to_string(), Node::new("b"));
|
||||
|
||||
let mut e1 = Edge::new("gate", "a");
|
||||
e1.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(false));
|
||||
let mut e2 = Edge::new("gate", "b");
|
||||
e2.attrs
|
||||
.insert("freeform".to_string(), AttrValue::Boolean(false));
|
||||
g.edges.push(e1);
|
||||
g.edges.push(e2);
|
||||
|
||||
let rule = FreeformEdgeCountRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- reachability: chain of reachable nodes ---
|
||||
|
||||
#[test]
|
||||
fn reachability_rule_chain_all_reachable() {
|
||||
let mut g = minimal_graph();
|
||||
g.nodes.insert("a".to_string(), Node::new("a"));
|
||||
g.nodes.insert("b".to_string(), Node::new("b"));
|
||||
g.edges = vec![
|
||||
Edge::new("start", "a"),
|
||||
Edge::new("a", "b"),
|
||||
Edge::new("b", "exit"),
|
||||
];
|
||||
let rule = ReachabilityRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- edge_target_exists: no edges at all ---
|
||||
|
||||
#[test]
|
||||
fn edge_target_exists_rule_no_edges() {
|
||||
let mut g = minimal_graph();
|
||||
g.edges.clear();
|
||||
let rule = EdgeTargetExistsRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- goal_gate_has_retry: goal_gate=false explicitly ---
|
||||
|
||||
#[test]
|
||||
fn goal_gate_has_retry_rule_explicit_false() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("work");
|
||||
node.attrs
|
||||
.insert("goal_gate".to_string(), AttrValue::Boolean(false));
|
||||
g.nodes.insert("work".to_string(), node);
|
||||
let rule = GoalGateHasRetryRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- stylesheet_syntax: empty string stylesheet ---
|
||||
|
||||
#[test]
|
||||
fn stylesheet_syntax_rule_empty_string() {
|
||||
let mut g = minimal_graph();
|
||||
g.attrs.insert(
|
||||
"model_stylesheet".to_string(),
|
||||
AttrValue::String(String::new()),
|
||||
);
|
||||
let rule = StylesheetSyntaxRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// --- type_known: start and exit types from shape are not flagged ---
|
||||
|
||||
#[test]
|
||||
fn type_known_rule_start_exit_shapes_no_warning() {
|
||||
// The minimal_graph has start (Mdiamond) and exit (Msquare), which resolve
|
||||
// to known handler types "start" and "exit" via shape mapping, not explicit type.
|
||||
// Since they have no explicit `type` attr, the rule should not flag them.
|
||||
let g = minimal_graph();
|
||||
let rule = TypeKnownRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
}
|
||||
|
|
@ -498,11 +498,17 @@ async fn goal_gate_routes_to_retry_target_on_failure() {
|
|||
};
|
||||
|
||||
let result = engine.run(&graph, &config).await;
|
||||
assert!(result.is_err(), "should fail when goal gate unsatisfied and no retry_target");
|
||||
let err_msg = result.unwrap_err().to_string();
|
||||
assert!(result.is_ok(), "goal gate unsatisfied with no retry_target should return Ok(fail outcome)");
|
||||
let outcome = result.unwrap();
|
||||
assert_eq!(
|
||||
outcome.status,
|
||||
StageStatus::Fail,
|
||||
"pipeline outcome should be 'fail' when goal gate unsatisfied"
|
||||
);
|
||||
let failure_reason = outcome.failure_reason.unwrap_or_default();
|
||||
assert!(
|
||||
err_msg.contains("goal gate unsatisfied"),
|
||||
"error should mention goal gate, got: {err_msg}"
|
||||
failure_reason.contains("goal gate unsatisfied"),
|
||||
"failure_reason should mention goal gate, got: {failure_reason}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -4579,14 +4585,12 @@ async fn fidelity_compact_preamble_includes_completed_stages_and_context() {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn fidelity_summary_detail_increases_with_level() {
|
||||
// Run three separate pipelines with summary:low, summary:medium, and summary:high.
|
||||
// Verify that higher detail levels produce longer preambles.
|
||||
let mut preamble_lengths: Vec<(String, usize)> = Vec::new();
|
||||
|
||||
// -- summary:low --
|
||||
let mut graph_low = make_graph_with_start_exit("SummaryLenLow");
|
||||
graph_low.attrs.insert("goal".to_string(), AttrValue::String("Detail test".to_string()));
|
||||
async fn fidelity_summary_low_excludes_context_values_in_pipeline() {
|
||||
// summary:low should NOT include context values (only goal, run ID, stage count, recent stages).
|
||||
// summary:medium should include context values.
|
||||
// This verifies a behavioral difference between detail levels.
|
||||
let mut graph_low = make_graph_with_start_exit("SummaryLowExcludesContext");
|
||||
graph_low.attrs.insert("goal".to_string(), AttrValue::String("Context exclusion test".to_string()));
|
||||
graph_low.attrs.insert("default_fidelity".to_string(), AttrValue::String("summary:low".to_string()));
|
||||
let mut step_a_low = Node::new("step_a");
|
||||
step_a_low.attrs.insert("type".to_string(), AttrValue::String("fidelity_capture".to_string()));
|
||||
|
|
@ -4607,12 +4611,18 @@ async fn fidelity_summary_detail_increases_with_level() {
|
|||
let engine_low = PipelineEngine::new(registry_low, EventEmitter::new());
|
||||
let config_low = RunConfig { logs_root: dir_low.path().to_path_buf(), cancel_token: None };
|
||||
engine_low.run(&graph_low, &config_low).await.expect("run low");
|
||||
let preambles_low = captures_low.preambles.lock().unwrap();
|
||||
preamble_lengths.push(("summary:low".to_string(), preambles_low[1].1.len()));
|
||||
|
||||
// -- summary:medium --
|
||||
let mut graph_med = make_graph_with_start_exit("SummaryLenMed");
|
||||
graph_med.attrs.insert("goal".to_string(), AttrValue::String("Detail test".to_string()));
|
||||
let preambles_low = captures_low.preambles.lock().unwrap();
|
||||
let low_preamble = &preambles_low[1].1;
|
||||
// summary:low should not include "Context values:" section
|
||||
assert!(
|
||||
!low_preamble.contains("Context values:"),
|
||||
"summary:low preamble should not include context values section"
|
||||
);
|
||||
|
||||
// Now run summary:medium and verify it DOES include context values
|
||||
let mut graph_med = make_graph_with_start_exit("SummaryMedIncludesContext");
|
||||
graph_med.attrs.insert("goal".to_string(), AttrValue::String("Context exclusion test".to_string()));
|
||||
graph_med.attrs.insert("default_fidelity".to_string(), AttrValue::String("summary:medium".to_string()));
|
||||
let mut step_a_med = Node::new("step_a");
|
||||
step_a_med.attrs.insert("type".to_string(), AttrValue::String("fidelity_capture".to_string()));
|
||||
|
|
@ -4633,47 +4643,13 @@ async fn fidelity_summary_detail_increases_with_level() {
|
|||
let engine_med = PipelineEngine::new(registry_med, EventEmitter::new());
|
||||
let config_med = RunConfig { logs_root: dir_med.path().to_path_buf(), cancel_token: None };
|
||||
engine_med.run(&graph_med, &config_med).await.expect("run med");
|
||||
|
||||
let preambles_med = captures_med.preambles.lock().unwrap();
|
||||
preamble_lengths.push(("summary:medium".to_string(), preambles_med[1].1.len()));
|
||||
|
||||
// -- summary:high --
|
||||
let mut graph_high = make_graph_with_start_exit("SummaryLenHigh");
|
||||
graph_high.attrs.insert("goal".to_string(), AttrValue::String("Detail test".to_string()));
|
||||
graph_high.attrs.insert("default_fidelity".to_string(), AttrValue::String("summary:high".to_string()));
|
||||
let mut step_a_high = Node::new("step_a");
|
||||
step_a_high.attrs.insert("type".to_string(), AttrValue::String("fidelity_capture".to_string()));
|
||||
graph_high.nodes.insert("step_a".to_string(), step_a_high);
|
||||
let mut step_b_high = Node::new("step_b");
|
||||
step_b_high.attrs.insert("type".to_string(), AttrValue::String("fidelity_capture".to_string()));
|
||||
graph_high.nodes.insert("step_b".to_string(), step_b_high);
|
||||
graph_high.edges.push(Edge::new("start", "step_a"));
|
||||
graph_high.edges.push(Edge::new("step_a", "step_b"));
|
||||
graph_high.edges.push(Edge::new("step_b", "exit"));
|
||||
|
||||
let captures_high = FidelityCaptures::new();
|
||||
let dir_high = tempfile::tempdir().unwrap();
|
||||
let mut registry_high = HandlerRegistry::new(Box::new(StartHandler));
|
||||
registry_high.register("start", Box::new(StartHandler));
|
||||
registry_high.register("exit", Box::new(ExitHandler));
|
||||
registry_high.register("fidelity_capture", Box::new(FidelityCapturingHandler { captures: captures_high.clone() }));
|
||||
let engine_high = PipelineEngine::new(registry_high, EventEmitter::new());
|
||||
let config_high = RunConfig { logs_root: dir_high.path().to_path_buf(), cancel_token: None };
|
||||
engine_high.run(&graph_high, &config_high).await.expect("run high");
|
||||
let preambles_high = captures_high.preambles.lock().unwrap();
|
||||
preamble_lengths.push(("summary:high".to_string(), preambles_high[1].1.len()));
|
||||
|
||||
// Higher summary levels should produce more detailed (longer) preambles
|
||||
let med_preamble = &preambles_med[1].1;
|
||||
// summary:medium should include "Context values:" section (graph.goal is always set)
|
||||
assert!(
|
||||
preamble_lengths[0].1 <= preamble_lengths[1].1,
|
||||
"summary:low ({}) should be no longer than summary:medium ({})",
|
||||
preamble_lengths[0].1,
|
||||
preamble_lengths[1].1,
|
||||
);
|
||||
assert!(
|
||||
preamble_lengths[1].1 <= preamble_lengths[2].1,
|
||||
"summary:medium ({}) should be no longer than summary:high ({})",
|
||||
preamble_lengths[1].1,
|
||||
preamble_lengths[2].1,
|
||||
med_preamble.contains("Context values:"),
|
||||
"summary:medium preamble should include context values section"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -6481,4 +6457,4 @@ fn parse_tool_hooks_from_dot_syntax() {
|
|||
work.attrs.get("tool_hooks.pre").and_then(|v| v.as_str()),
|
||||
Some("node pre")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -794,9 +794,21 @@ async fn stream_generate_raw(
|
|||
tool_definitions: Option<&[ToolDefinition]>,
|
||||
) -> Result<StreamEventStream, SdkError> {
|
||||
let request = build_request(params, messages, tool_definitions);
|
||||
let inner_stream = client.stream(&request).await?;
|
||||
|
||||
if let Some(ref token) = params.abort_signal {
|
||||
// Apply per_step timeout to the initial connection (Section 4.7)
|
||||
let inner_stream = if let Some(per_step) = params.timeout.as_ref().and_then(|t| t.per_step) {
|
||||
let duration = std::time::Duration::from_secs_f64(per_step);
|
||||
tokio::time::timeout(duration, client.stream(&request))
|
||||
.await
|
||||
.map_err(|_| SdkError::RequestTimeout {
|
||||
message: format!("Per-step timeout of {per_step}s exceeded"),
|
||||
})??
|
||||
} else {
|
||||
client.stream(&request).await?
|
||||
};
|
||||
|
||||
// Apply abort signal if present
|
||||
let stream: StreamEventStream = if let Some(ref token) = params.abort_signal {
|
||||
let token = token.clone();
|
||||
let mapped = inner_stream.map(move |item| {
|
||||
if token.is_cancelled() {
|
||||
|
|
@ -806,9 +818,39 @@ async fn stream_generate_raw(
|
|||
}
|
||||
item
|
||||
});
|
||||
Ok(Box::pin(mapped))
|
||||
Box::pin(mapped)
|
||||
} else {
|
||||
Ok(inner_stream)
|
||||
inner_stream
|
||||
};
|
||||
|
||||
// Apply total timeout to the stream (Section 4.7)
|
||||
if let Some(total) = params.timeout.as_ref().and_then(|t| t.total) {
|
||||
let duration = std::time::Duration::from_secs_f64(total);
|
||||
let deadline = tokio::time::Instant::now() + duration;
|
||||
let total_copy = total;
|
||||
let timed_stream = futures::stream::unfold(
|
||||
(stream, false),
|
||||
move |(mut stream, done)| async move {
|
||||
if done {
|
||||
return None;
|
||||
}
|
||||
match tokio::time::timeout_at(deadline, stream.next()).await {
|
||||
Ok(Some(item)) => Some((item, (stream, false))),
|
||||
Ok(None) => None, // stream completed naturally
|
||||
Err(_) => {
|
||||
Some((
|
||||
Err(SdkError::RequestTimeout {
|
||||
message: format!("Total timeout of {total_copy}s exceeded"),
|
||||
}),
|
||||
(stream, true),
|
||||
))
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
Ok(Box::pin(timed_stream))
|
||||
} else {
|
||||
Ok(stream)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2615,4 +2657,4 @@ mod tests {
|
|||
});
|
||||
assert!(has_timeout, "Expected a RequestTimeout error from total timeout");
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue