From fabd65b08ab229f4870f607d88438038721e7129 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Mon, 13 Apr 2026 13:40:22 +0200 Subject: [PATCH] Fix: propagate edge-lookup errors from iterative traversal loop The retain-based loop swallowed catalog.lookup_edge_by_name errors by keeping the traversal for the next pass, where it could never succeed. This caused the no-progress break to fire, silently dropping the traversal and producing incorrect query results with missing joins. Replaced retain with a manual for-loop that propagates errors via ?. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/omnigraph-compiler/src/ir/lower.rs | 31 ++++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/omnigraph-compiler/src/ir/lower.rs b/crates/omnigraph-compiler/src/ir/lower.rs index 96025c1..fa906b8 100644 --- a/crates/omnigraph-compiler/src/ir/lower.rs +++ b/crates/omnigraph-compiler/src/ir/lower.rs @@ -243,19 +243,23 @@ fn lower_clauses( // until all traversals are consumed. let mut remaining: Vec<&Traversal> = traversals.to_vec(); while !remaining.is_empty() { - let before = remaining.len(); - remaining.retain(|traversal| { + let mut next_remaining = Vec::new(); + for traversal in &remaining { let src_bound = bound_vars.contains(&traversal.src); let dst_bound = bound_vars.contains(&traversal.dst); if !src_bound && !dst_bound { - return true; // keep for next pass + next_remaining.push(*traversal); + continue; } - // This traversal can be processed — emit the appropriate ops. - // (errors inside retain are not ergonomic, so we rely on the - // type-checker having validated edge names already.) - let Some(edge) = catalog.lookup_edge_by_name(&traversal.edge_name) else { - return true; // keep; will error on next pass or after loop - }; + + let edge = catalog + .lookup_edge_by_name(&traversal.edge_name) + .ok_or_else(|| { + crate::error::NanoError::Plan(format!( + "lowering traversal referenced missing edge '{}' after typecheck", + traversal.edge_name + )) + })?; let direction = type_ctx .traversals @@ -340,14 +344,11 @@ fn lower_clauses( bound_vars.insert(traversal.dst.clone()); } } - false // processed — remove from remaining - }); - if remaining.len() == before { - // No progress — remaining traversals reference no bound variables. - // Fall through; the traversals will be skipped (this would be a - // type-check error in practice). + } + if next_remaining.len() == remaining.len() { break; } + remaining = next_remaining; } // Lower explicit filters