From 0d7ae73857ea7494a3f2f4dcc4f7e273400fb7ea Mon Sep 17 00:00:00 2001 From: "brynary-fabro[bot]" <265161896+brynary-fabro[bot]@users.noreply.github.com> Date: Sun, 15 Mar 2026 19:54:48 -0400 Subject: [PATCH] Random Edge Selection (#12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR introduces a `selection="random"` node attribute that enables weighted-random tiebreaking when choosing among candidate outgoing edges. The existing deterministic behavior (highest weight, then lexical node ID) remains the default. The cascade priority—conditions → preferred label → suggested next → unconditional → fallback—is unchanged; randomness only replaces the final pick-one-from-candidates step within each tier. A new `weighted_random` function handles the sampling, treating edges with weight ≤ 0 as weight 1, while a `pick_edge` dispatcher routes to either the random or deterministic strategy based on the node's `selection()` accessor. A validation rule (`RandomSelectionNoConditionsRule`) rejects nodes that combine `selection="random"` with conditional edges, since condition evaluation order would conflict with random selection. A companion rule (`SelectionValidRule`) warns on unrecognized selection values. Both are registered as built-in lint rules with appropriate error/warning severities and actionable fix suggestions. Documentation is updated in the transitions guide with a new "Random selection" section explaining the behavior and constraints, and the DOT language reference gains a `selection` row in the node attributes table. All changes were developed following red/green TDD cycles with comprehensive test coverage for the accessor, weighted random sampling, edge selection integration, and both validation rules. ### Fabro Details
Ran 7 stages in 17m 1s for $4.07 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 0s | – | 0 | | preflight_compile | 0s | – | 0 | | preflight_lint | 0s | – | 0 | | implement | 0s | $2.43 | 0 | | simplify | 0s | $1.63 | 0 | | verify | 0s | – | 0 | | **Total** | **17m 1s** | **$4.07** | **0** |
Ran ImplementAndSimplify.fabro (10 nodes and 13 edges) ```dot digraph ImplementAndSimplify { graph [ goal="Implement and simplify", model_stylesheet=" * { backend: api; model: claude-opus-4-6;} " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo clippy -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan."] simplify [label="Simplify", prompt="@prompts/simplify.md"] verify [label="Verify", shape=parallelogram, script="cargo clippy -- -D warnings 2>&1 && cargo test 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all clippy lint warnings and test failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=success"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=success"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=success"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify -> verify verify -> exit [condition="outcome=success"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- docs/reference/dot-language.mdx | 1 + docs/workflows/transitions.mdx | 19 +- lib/crates/fabro-workflows/src/engine.rs | 161 ++++++++++++++-- lib/crates/fabro-workflows/src/graph/types.rs | 5 + .../fabro-workflows/src/validation/rules.rs | 174 ++++++++++++++++++ 5 files changed, 346 insertions(+), 14 deletions(-) diff --git a/docs/reference/dot-language.mdx b/docs/reference/dot-language.mdx index 724132427..af32eb6da 100644 --- a/docs/reference/dot-language.mdx +++ b/docs/reference/dot-language.mdx @@ -189,6 +189,7 @@ Start nodes can also be identified by ID (`start` or `Start`). Exit nodes can be | `retry_target` | String | Node ID to jump to on retry | | `goal_gate` | Boolean | When `true`, workflow fails if this node doesn't succeed | | `auto_status` | Boolean | Auto-generate status updates | +| `selection` | String | Edge tiebreaking strategy: `deterministic` (default) or `random` (weighted-random). Cannot be combined with conditional edges. | ### Agent and prompt nodes diff --git a/docs/workflows/transitions.mdx b/docs/workflows/transitions.mdx index 90103bc89..9c31c7d3c 100644 --- a/docs/workflows/transitions.mdx +++ b/docs/workflows/transitions.mdx @@ -3,7 +3,7 @@ title: "Transitions" description: "How Fabro decides which node to execute next" --- -After each node finishes, Fabro must decide which edge to follow to the next node. This decision is fully deterministic — given the same outcome and context, Fabro always picks the same edge. Understanding the transition logic helps you design workflows that route reliably. +After each node finishes, Fabro must decide which edge to follow to the next node. This decision is deterministic by default — given the same outcome and context, Fabro always picks the same edge. Nodes can opt into [random selection](#random-selection) for weighted-random tiebreaking instead. Understanding the transition logic helps you design workflows that route reliably. ## How transitions work @@ -155,3 +155,20 @@ node -> fallback [weight=1] ``` If weights are equal, the edge with the lexicographically first target node ID is chosen. This makes the behavior fully deterministic. + +## Random selection + +By default, tiebreaking between candidate edges is deterministic (highest weight, then lexical node ID). Setting `selection="random"` on a node switches to weighted-random tiebreaking for its outgoing edges: + +```dot +picker [label="Pick path", selection="random"] + +picker -> path_a [weight=3] +picker -> path_b [weight=1] +``` + +In this example, `path_a` is chosen ~75% of the time and `path_b` ~25%. Edges with weight ≤ 0 are treated as weight 1. The cascade priority (conditions → preferred label → suggested next → unconditional → fallback) is unchanged — randomness only affects the pick-one-from-candidates step within each tier. + + +`selection="random"` cannot be combined with conditional edges on the same node. Validation rejects this combination because condition evaluation order would conflict with random selection. Use unconditional edges with weights instead. + diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs index e75a51940..ec1eff779 100644 --- a/lib/crates/fabro-workflows/src/engine.rs +++ b/lib/crates/fabro-workflows/src/engine.rs @@ -390,6 +390,46 @@ fn best_by_weight_then_lexical<'a>(edges: &[&'a Edge]) -> Option<&'a Edge> { Some(best) } +/// Pick a random edge using weighted-random selection. +/// Edges with `weight <= 0` are treated as weight 1 for probability calculation. +fn weighted_random<'a>(edges: &[&'a Edge]) -> Option<&'a Edge> { + if edges.is_empty() { + return None; + } + if edges.len() == 1 { + return Some(edges[0]); + } + let weights: Vec = edges + .iter() + .map(|e| { + let w = e.weight(); + if w <= 0 { + 1.0 + } else { + w as f64 + } + }) + .collect(); + let total: f64 = weights.iter().sum(); + let mut rng = rand::thread_rng(); + let mut roll: f64 = rng.gen_range(0.0..total); + for (i, &w) in weights.iter().enumerate() { + roll -= w; + if roll < 0.0 { + return Some(edges[i]); + } + } + Some(edges[edges.len() - 1]) +} + +/// Dispatch to the appropriate edge-picking strategy. +fn pick_edge<'a>(edges: &[&'a Edge], selection: &str) -> Option<&'a Edge> { + match selection { + "random" => weighted_random(edges), + _ => best_by_weight_then_lexical(edges), + } +} + /// Select the next edge from a node's outgoing edges (spec Section 3.3). #[must_use] /// Result of edge selection: the chosen edge and the reason it was selected. @@ -403,6 +443,7 @@ pub fn select_edge<'a>( outcome: &Outcome, context: &Context, graph: &'a Graph, + selection: &str, ) -> Option> { let edges = graph.outgoing_edges(node_id); if edges.is_empty() { @@ -419,7 +460,7 @@ pub fn select_edge<'a>( .copied() .collect(); if !condition_matched.is_empty() { - return best_by_weight_then_lexical(&condition_matched).map(|edge| EdgeSelection { + return pick_edge(&condition_matched, selection).map(|edge| EdgeSelection { edge, reason: "condition", }); @@ -459,14 +500,14 @@ pub fn select_edge<'a>( .copied() .collect(); if !unconditional.is_empty() { - return best_by_weight_then_lexical(&unconditional).map(|edge| EdgeSelection { + return pick_edge(&unconditional, selection).map(|edge| EdgeSelection { edge, reason: "unconditional", }); } // Fallback: any edge - best_by_weight_then_lexical(&edges).map(|edge| EdgeSelection { + pick_edge(&edges, selection).map(|edge| EdgeSelection { edge, reason: "fallback", }) @@ -1598,7 +1639,13 @@ impl WorkflowRunEngine { previous_node_id = Some(node.id.clone()); stage_index += 1; // Select next edge and continue - let selection = select_edge(&node.id, &Outcome::skipped(), &context, graph); + let selection = select_edge( + &node.id, + &Outcome::skipped(), + &context, + graph, + node.selection(), + ); if let Some(sel) = selection { current_node_id = sel.edge.to.clone(); incoming_edge = Some(sel.edge); @@ -1815,7 +1862,7 @@ impl WorkflowRunEngine { }); (None, Some(target.clone())) } else { - let selection = select_edge(&node.id, &outcome, &context, graph); + let selection = select_edge(&node.id, &outcome, &context, graph, node.selection()); if let Some(sel) = &selection { self.services.emitter.emit(&WorkflowRunEvent::EdgeSelected { from_node: node.id.clone(), @@ -2550,6 +2597,65 @@ mod tests { assert!(result.is_none()); } + // --- weighted_random tests --- + + #[test] + fn weighted_random_empty_returns_none() { + assert!(weighted_random(&[]).is_none()); + } + + #[test] + fn weighted_random_single_edge() { + let e = Edge::new("a", "b"); + let result = weighted_random(&[&e]).unwrap(); + assert_eq!(result.to, "b"); + } + + #[test] + fn weighted_random_zero_weight_all_selected() { + let e1 = Edge::new("a", "b"); + let e2 = Edge::new("a", "c"); + let edges = vec![&e1, &e2]; + let mut seen_b = false; + let mut seen_c = false; + for _ in 0..200 { + let pick = weighted_random(&edges).unwrap(); + if pick.to == "b" { + seen_b = true; + } + if pick.to == "c" { + seen_c = true; + } + } + assert!(seen_b, "expected target 'b' to be selected at least once"); + assert!(seen_c, "expected target 'c' to be selected at least once"); + } + + #[test] + fn weighted_random_high_weight_dominates() { + let mut heavy = Edge::new("a", "heavy"); + heavy + .attrs + .insert("weight".to_string(), AttrValue::Integer(100)); + let mut light = Edge::new("a", "light"); + light + .attrs + .insert("weight".to_string(), AttrValue::Integer(1)); + let edges = vec![&heavy, &light]; + let mut heavy_count = 0; + for _ in 0..500 { + let pick = weighted_random(&edges).unwrap(); + if pick.to == "heavy" { + heavy_count += 1; + } + } + let ratio = heavy_count as f64 / 500.0; + assert!( + ratio > 0.90, + "expected heavy edge to win >90% of the time, got {ratio:.2}" + ); + } + // --- select_edge tests --- fn make_graph_with_edges(edges: Vec) -> Graph { @@ -2571,7 +2677,7 @@ mod tests { let g = Graph::new("test"); let outcome = Outcome::success(); let context = Context::new(); - assert!(select_edge("a", &outcome, &context, &g).is_none()); + assert!(select_edge("a", &outcome, &context, &g, "deterministic").is_none()); } #[test] @@ -2579,7 +2685,7 @@ mod tests { let g = make_graph_with_edges(vec![Edge::new("a", "b")]); let outcome = Outcome::success(); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "b"); assert_eq!(sel.reason, "unconditional"); } @@ -2599,7 +2705,7 @@ mod tests { let g = make_graph_with_edges(vec![e1, e2]); let outcome = Outcome::success(); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "success_path"); assert_eq!(sel.reason, "condition"); } @@ -2620,7 +2726,7 @@ mod tests { let mut outcome = Outcome::success(); outcome.preferred_label = Some("Fix".to_string()); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "fix"); assert_eq!(sel.reason, "preferred_label"); } @@ -2633,7 +2739,7 @@ mod tests { let mut outcome = Outcome::success(); outcome.suggested_next_ids = vec!["path2".to_string()]; let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "path2"); assert_eq!(sel.reason, "suggested_next"); } @@ -2648,7 +2754,7 @@ mod tests { let g = make_graph_with_edges(vec![e1, e2]); let outcome = Outcome::success(); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "high"); assert_eq!(sel.reason, "unconditional"); } @@ -2660,7 +2766,7 @@ mod tests { let g = make_graph_with_edges(vec![e1, e2]); let outcome = Outcome::success(); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "alpha"); assert_eq!(sel.reason, "unconditional"); } @@ -2676,11 +2782,40 @@ mod tests { let g = make_graph_with_edges(vec![e_cond, e_uncond]); let outcome = Outcome::success(); let context = Context::new(); - let sel = select_edge("a", &outcome, &context, &g).unwrap(); + let sel = select_edge("a", &outcome, &context, &g, "deterministic").unwrap(); assert_eq!(sel.edge.to, "cond_path"); assert_eq!(sel.reason, "condition"); } + #[test] + fn select_edge_random_returns_some_edge() { + let e1 = Edge::new("a", "b"); + let e2 = Edge::new("a", "c"); + let g = make_graph_with_edges(vec![e1, e2]); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge("a", &outcome, &context, &g, "random").unwrap(); + assert!(sel.edge.to == "b" || sel.edge.to == "c"); + assert_eq!(sel.reason, "unconditional"); + } + + #[test] + fn select_edge_random_preferred_label_still_wins() { + let mut e1 = Edge::new("a", "approve"); + e1.attrs.insert( + "label".to_string(), + AttrValue::String("Approve".to_string()), + ); + let e2 = Edge::new("a", "other"); + let g = make_graph_with_edges(vec![e1, e2]); + let mut outcome = Outcome::success(); + outcome.preferred_label = Some("Approve".to_string()); + let context = Context::new(); + let sel = select_edge("a", &outcome, &context, &g, "random").unwrap(); + assert_eq!(sel.edge.to, "approve"); + assert_eq!(sel.reason, "preferred_label"); + } + // --- check_goal_gates tests --- #[test] diff --git a/lib/crates/fabro-workflows/src/graph/types.rs b/lib/crates/fabro-workflows/src/graph/types.rs index f1a3bdce2..d6cc8cb61 100644 --- a/lib/crates/fabro-workflows/src/graph/types.rs +++ b/lib/crates/fabro-workflows/src/graph/types.rs @@ -235,6 +235,11 @@ impl Node { self.str_attr("backend") } + #[must_use] + pub fn selection(&self) -> &str { + self.str_attr("selection").unwrap_or("deterministic") + } + /// Resolve the handler type for this node using explicit type or shape mapping. #[must_use] pub fn handler_type(&self) -> Option<&str> { diff --git a/lib/crates/fabro-workflows/src/validation/rules.rs b/lib/crates/fabro-workflows/src/validation/rules.rs index dbdc76539..194cf01c2 100644 --- a/lib/crates/fabro-workflows/src/validation/rules.rs +++ b/lib/crates/fabro-workflows/src/validation/rules.rs @@ -32,6 +32,8 @@ pub fn built_in_rules() -> Vec> { Box::new(StylesheetModelKnownRule), Box::new(UnresolvedFileRefRule), Box::new(ThreadIdRequiresFidelityFullRule), + Box::new(SelectionValidRule), + Box::new(RandomSelectionNoConditionsRule), ] } @@ -1090,6 +1092,76 @@ impl LintRule for ThreadIdRequiresFidelityFullRule { } } +// --- Rule 23: selection_valid (WARNING) --- + +struct SelectionValidRule; + +const VALID_SELECTIONS: &[&str] = &["deterministic", "random"]; + +impl LintRule for SelectionValidRule { + fn name(&self) -> &'static str { + "selection_valid" + } + + fn apply(&self, graph: &Graph) -> Vec { + let mut diagnostics = Vec::new(); + for node in graph.nodes.values() { + if let Some(sel) = node.attrs.get("selection").and_then(AttrValue::as_str) { + if !VALID_SELECTIONS.contains(&sel) { + diagnostics.push(Diagnostic { + rule: self.name().to_string(), + severity: Severity::Warning, + message: format!("Node '{}' has invalid selection mode '{sel}'", node.id), + node_id: Some(node.id.clone()), + edge: None, + fix: Some(format!("Use one of: {}", VALID_SELECTIONS.join(", "))), + }); + } + } + } + diagnostics + } +} + +// --- Rule 24: random_selection_no_conditions (ERROR) --- + +struct RandomSelectionNoConditionsRule; + +impl LintRule for RandomSelectionNoConditionsRule { + fn name(&self) -> &'static str { + "random_selection_no_conditions" + } + + fn apply(&self, graph: &Graph) -> Vec { + let mut diagnostics = Vec::new(); + for node in graph.nodes.values() { + if node.selection() != "random" { + continue; + } + let has_conditional = graph + .outgoing_edges(&node.id) + .iter() + .any(|e| e.condition().is_some_and(|c| !c.is_empty())); + if has_conditional { + diagnostics.push(Diagnostic { + rule: self.name().to_string(), + severity: Severity::Error, + message: format!( + "Node '{}' has selection=\"random\" but also has conditional edges; random selection and conditions cannot be combined", + node.id + ), + node_id: Some(node.id.clone()), + edge: None, + fix: Some( + "Remove the condition attributes from outgoing edges, or remove selection=\"random\" from the node".to_string(), + ), + }); + } + } + diagnostics + } +} + #[cfg(test)] mod tests { use super::*; @@ -3166,4 +3238,106 @@ mod tests { let d = rule.apply(&g); assert!(d.is_empty()); } + + // --- selection_valid rule tests --- + + #[test] + fn selection_valid_known_values() { + let mut g = minimal_graph(); + let mut node = Node::new("pick"); + node.attrs.insert( + "selection".to_string(), + AttrValue::String("random".to_string()), + ); + g.nodes.insert("pick".to_string(), node); + let rule = SelectionValidRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn selection_valid_unknown_value_warns() { + let mut g = minimal_graph(); + let mut node = Node::new("pick"); + node.attrs.insert( + "selection".to_string(), + AttrValue::String("randon".to_string()), + ); + g.nodes.insert("pick".to_string(), node); + let rule = SelectionValidRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Warning); + assert_eq!(d[0].node_id.as_deref(), Some("pick")); + } + + #[test] + fn selection_valid_no_attr_ok() { + let g = minimal_graph(); + let rule = SelectionValidRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + // --- random_selection_no_conditions rule tests --- + + #[test] + fn random_selection_no_conditions_clean() { + let mut g = minimal_graph(); + let mut node = Node::new("pick"); + node.attrs.insert( + "selection".to_string(), + AttrValue::String("random".to_string()), + ); + g.nodes.insert("pick".to_string(), node); + g.edges.push(Edge::new("pick", "start")); + g.edges.push(Edge::new("pick", "exit")); + let rule = RandomSelectionNoConditionsRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn random_selection_with_conditions_errors() { + let mut g = minimal_graph(); + let mut node = Node::new("pick"); + node.attrs.insert( + "selection".to_string(), + AttrValue::String("random".to_string()), + ); + g.nodes.insert("pick".to_string(), node); + let mut e = Edge::new("pick", "exit"); + e.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=success".to_string()), + ); + g.edges.push(e); + g.edges.push(Edge::new("pick", "start")); + let rule = RandomSelectionNoConditionsRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Error); + assert_eq!(d[0].node_id.as_deref(), Some("pick")); + } + + #[test] + fn deterministic_selection_with_conditions_ok() { + let mut g = minimal_graph(); + let mut node = Node::new("gate"); + node.attrs.insert( + "selection".to_string(), + AttrValue::String("deterministic".to_string()), + ); + g.nodes.insert("gate".to_string(), node); + let mut e = Edge::new("gate", "exit"); + e.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=success".to_string()), + ); + g.edges.push(e); + g.edges.push(Edge::new("gate", "start")); + let rule = RandomSelectionNoConditionsRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } }