From 5a57ffe72d9c30971c969552574cfca5fdd985c2 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 23:32:16 +0200 Subject: [PATCH 1/2] test(sexpr): audit predicate matrix, fuzz, doc examples, CLI smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comprehensive s-expression audit: close the coverage gaps in the predicate inventory and add four campaigns on top of the existing proptest_sexpr properties. New coverage: * rivet-core/tests/sexpr_predicate_matrix.rs (92 tests) Three-shape coverage — positive, negative, malformed — for every predicate the lowerer recognises. Fills gaps for !=, >, <, >=, <=, linked-from (arity), count, reachable-from, reachable-to, plus arity/operator-shape errors for every head form. * rivet-core/tests/sexpr_fuzz.rs (4 proptest campaigns, 256 cases each) - parse_never_panics: random ASCII + paren/quote soup must not panic sexpr::parse - lower_never_panics: full parse_filter on arbitrary input - evaluate_never_panics: lowered Expr evaluated on a synthetic store - roundtrip_equivalence: generated Expr → pretty-print → re-parse must preserve truth value on every fixture artifact * rivet-core/tests/sexpr_doc_examples.rs (9 tests) Every s-expression example in docs/getting-started.md runs end-to-end with an asserted match count, catching any future drift between the docs and the evaluator. * rivet-cli/tests/sexpr_filter_integration.rs (6 tests) CLI-level smoke for list/stats/coverage --filter, including a baseline vs. filtered comparison to catch silently-ignored filters and a bad-s-expr exit-code assertion. Verifies: REQ-048 Refs: REQ-028 --- rivet-cli/tests/sexpr_filter_integration.rs | 196 +++++ rivet-core/tests/sexpr_doc_examples.rs | 227 +++++ rivet-core/tests/sexpr_fuzz.rs | 310 +++++++ rivet-core/tests/sexpr_predicate_matrix.rs | 873 ++++++++++++++++++++ 4 files changed, 1606 insertions(+) create mode 100644 rivet-cli/tests/sexpr_filter_integration.rs create mode 100644 rivet-core/tests/sexpr_doc_examples.rs create mode 100644 rivet-core/tests/sexpr_fuzz.rs create mode 100644 rivet-core/tests/sexpr_predicate_matrix.rs diff --git a/rivet-cli/tests/sexpr_filter_integration.rs b/rivet-cli/tests/sexpr_filter_integration.rs new file mode 100644 index 0000000..c0b22c5 --- /dev/null +++ b/rivet-cli/tests/sexpr_filter_integration.rs @@ -0,0 +1,196 @@ +//! End-to-end integration tests for `--filter` surfaces on the CLI. +//! +//! Each command that accepts an s-expression filter (`list`, `stats`, +//! `coverage`, `export`) gets a positive and a negative case. We run +//! against the repository's own artifact set and assert that the filter +//! is honoured (not silently ignored) by comparing counts against the +//! unfiltered baseline. + +use std::process::Command; + +fn rivet_bin() -> std::path::PathBuf { + if let Ok(bin) = std::env::var("CARGO_BIN_EXE_rivet") { + return std::path::PathBuf::from(bin); + } + let manifest = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); + manifest + .parent() + .expect("workspace root") + .join("target") + .join("debug") + .join("rivet") +} + +fn project_root() -> std::path::PathBuf { + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("workspace root") + .to_path_buf() +} + +fn json_count(stdout: &[u8]) -> u64 { + let parsed: serde_json::Value = + serde_json::from_slice(stdout).expect("stdout must be valid JSON"); + parsed + .get("count") + .and_then(|v| v.as_u64()) + .expect("'count' field missing in JSON output") +} + +// ── list --filter ────────────────────────────────────────────────────── + +#[test] +fn list_filter_requirement_type_matches_only_requirements() { + let baseline = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "list", + "--format", + "json", + ]) + .output() + .expect("baseline list"); + assert!(baseline.status.success(), "baseline list failed"); + let baseline_count = json_count(&baseline.stdout); + + let filtered = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "list", + "--filter", + r#"(= type "requirement")"#, + "--format", + "json", + ]) + .output() + .expect("filtered list"); + assert!( + filtered.status.success(), + "filtered list exited non-zero. stderr: {}", + String::from_utf8_lossy(&filtered.stderr) + ); + let filtered_count = json_count(&filtered.stdout); + assert!( + filtered_count > 0, + "filter (= type \"requirement\") should match something" + ); + assert!( + filtered_count <= baseline_count, + "filter must not return more artifacts than the baseline ({filtered_count} > {baseline_count})" + ); +} + +#[test] +fn list_filter_impossible_is_empty() { + // A filter that can't match anything must return zero — catches the + // "filter silently ignored" class of bug. + let output = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "list", + "--filter", + r#"(= id "__does-not-exist__")"#, + "--format", + "json", + ]) + .output() + .expect("filtered list"); + assert!(output.status.success()); + assert_eq!(json_count(&output.stdout), 0); +} + +#[test] +fn list_filter_bad_sexpr_is_reported() { + let output = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "list", + "--filter", + "(and (has-tag \"x\"", + "--format", + "json", + ]) + .output() + .expect("bad filter run"); + assert!( + !output.status.success(), + "CLI must reject malformed filter, got exit 0" + ); +} + +// ── stats --filter ───────────────────────────────────────────────────── + +#[test] +fn stats_filter_respects_predicate() { + let output = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "stats", + "--filter", + r#"(= type "requirement")"#, + "--format", + "json", + ]) + .output() + .expect("stats --filter run"); + assert!( + output.status.success(), + "stats --filter must succeed. stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + // JSON must parse — we don't assert a specific count because the + // schema allows for new requirement types being added over time. + let _: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stats JSON must be valid"); +} + +#[test] +fn stats_filter_empty_is_zero() { + let output = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "stats", + "--filter", + r#"(= id "__nope__")"#, + "--format", + "json", + ]) + .output() + .expect("stats --filter empty run"); + assert!(output.status.success()); + let parsed: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("JSON"); + let total = parsed.get("total").and_then(|v| v.as_u64()).unwrap_or(0); + assert_eq!(total, 0, "empty filter must zero out stats total"); +} + +// ── coverage --filter ────────────────────────────────────────────────── + +#[test] +fn coverage_filter_runs_cleanly() { + let output = Command::new(rivet_bin()) + .args([ + "--project", + project_root().to_str().unwrap(), + "coverage", + "--filter", + r#"(has-tag "stpa")"#, + "--format", + "json", + ]) + .output() + .expect("coverage --filter run"); + assert!( + output.status.success(), + "coverage --filter must succeed. stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let _: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("coverage JSON must parse"); +} diff --git a/rivet-core/tests/sexpr_doc_examples.rs b/rivet-core/tests/sexpr_doc_examples.rs new file mode 100644 index 0000000..8497c16 --- /dev/null +++ b/rivet-core/tests/sexpr_doc_examples.rs @@ -0,0 +1,227 @@ +//! Regression tests for every s-expression example in the user docs. +//! +//! If a docstring says "here's how to use this predicate", the example +//! MUST parse cleanly and evaluate to the advertised result. Documented +//! examples that don't work are worse than no documentation — they break +//! user trust the moment they're copy-pasted. + +use std::collections::BTreeMap; + +use rivet_core::links::LinkGraph; +use rivet_core::model::{Artifact, Link}; +use rivet_core::schema::Schema; +use rivet_core::sexpr_eval::{self, matches_filter_with_store}; +use rivet_core::store::Store; + +fn art( + id: &str, + t: &str, + tags: &[&str], + status: Option<&str>, + links: &[(&str, &str)], +) -> Artifact { + Artifact { + id: id.into(), + artifact_type: t.into(), + title: format!("title of {id}"), + description: None, + status: status.map(|s| s.to_string()), + tags: tags.iter().map(|s| s.to_string()).collect(), + links: links + .iter() + .map(|(lt, tgt)| Link { + link_type: (*lt).into(), + target: (*tgt).into(), + }) + .collect(), + fields: BTreeMap::new(), + provenance: None, + source_file: None, + } +} + +fn fixture() -> (Store, LinkGraph) { + let arts = vec![ + art( + "REQ-001", + "requirement", + &["stpa", "safety"], + Some("approved"), + &[("satisfies", "REQ-004")], + ), + art( + "REQ-002", + "requirement", + &["eu"], + Some("draft"), + &[("satisfies", "REQ-004")], + ), + art( + "REQ-003", + "requirement", + &["safety"], + Some("approved"), + &[ + ("satisfies", "REQ-004"), + ("satisfies", "REQ-001"), + ("satisfies", "REQ-002"), + ], + ), + art( + "REQ-004", + "requirement", + &["core"], + Some("approved"), + &[], + ), + art("FEAT-001", "feature", &[], Some("approved"), &[]), + ]; + let mut s = Store::default(); + for a in arts { + s.upsert(a); + } + let g = LinkGraph::build(&s, &Schema::merge(&[])); + (s, g) +} + +fn count_matches(filter: &str, store: &Store, graph: &LinkGraph) -> usize { + let expr = sexpr_eval::parse_filter(filter) + .unwrap_or_else(|errs| panic!("docs example {filter:?} failed to parse: {errs:?}")); + store + .iter() + .filter(|a| matches_filter_with_store(&expr, a, graph, store)) + .count() +} + +// Every example below is copy-pasted from `docs/getting-started.md` +// (the "S-Expression Filtering" section). + +#[test] +fn docs_example_simple_type_equals() { + // `rivet list --filter '(= type "requirement")'` + let (store, graph) = fixture(); + assert_eq!(count_matches(r#"(= type "requirement")"#, &store, &graph), 4); +} + +#[test] +fn docs_example_and_with_has_tag() { + // `rivet list --filter '(and (has-tag "stpa") (= status "approved"))'` + let (store, graph) = fixture(); + assert_eq!( + count_matches( + r#"(and (has-tag "stpa") (= status "approved"))"#, + &store, + &graph + ), + 1 // REQ-001 + ); +} + +#[test] +fn docs_example_not_status_draft() { + // `rivet list --filter '(not (= status "draft"))'` + let (store, graph) = fixture(); + // Everything except REQ-002. + assert_eq!(count_matches(r#"(not (= status "draft"))"#, &store, &graph), 4); +} + +#[test] +fn docs_example_linked_by_wildcard() { + // `rivet list --filter '(linked-by "satisfies" _)'` + let (store, graph) = fixture(); + // REQ-001, REQ-002, REQ-003 have satisfies links; REQ-004 and FEAT-001 don't. + assert_eq!( + count_matches(r#"(linked-by "satisfies" _)"#, &store, &graph), + 3 + ); +} + +#[test] +fn docs_example_links_count_gt_two() { + // `rivet list --filter '(links-count "satisfies" > 2)'` + let (store, graph) = fixture(); + // Only REQ-003 has 3 satisfies links. + assert_eq!( + count_matches(r#"(links-count "satisfies" > 2)"#, &store, &graph), + 1 + ); +} + +#[test] +fn docs_example_exists_quantifier() { + // `rivet list --filter '(exists (= type "requirement") (has-tag "safety"))'` + // + // `exists` is a global property — every artifact gets the same + // boolean. Either every artifact matches (if at least one + // requirement has "safety") or none do. + let (store, graph) = fixture(); + let n = count_matches( + r#"(exists (= type "requirement") (has-tag "safety"))"#, + &store, + &graph, + ); + assert_eq!(n, store.len()); +} + +#[test] +fn docs_example_reachable_from() { + // `rivet list --filter '(reachable-from "REQ-001" "satisfies")'` + // + // REQ-001 --satisfies--> REQ-004, so REQ-004 is reachable. + let (store, graph) = fixture(); + assert_eq!( + count_matches(r#"(reachable-from "REQ-001" "satisfies")"#, &store, &graph), + 1 // REQ-004 + ); +} + +#[test] +fn docs_example_has_tag_safety() { + // `rivet coverage --filter '(has-tag "safety")'` + // + // Same filter as a coverage scope. REQ-001 and REQ-003 have "safety". + let (store, graph) = fixture(); + assert_eq!(count_matches(r#"(has-tag "safety")"#, &store, &graph), 2); +} + +// Ensure the predicate listing from the doc body (not the examples +// block itself) includes nothing that the parser can't accept. The +// error classifier must not trip on these names. + +#[test] +fn docs_listed_predicates_all_parse_as_forms() { + // Minimal valid shapes for each predicate the docs advertise. + let cases = [ + r#"(= type "requirement")"#, + r#"(!= type "feature")"#, + r#"(> level 0)"#, + r#"(< level 10)"#, + r#"(>= level 1)"#, + r#"(<= level 3)"#, + r#"(in "safety" tags)"#, + r#"(has-tag "stpa")"#, + r#"(has-field "priority")"#, + r#"(matches id ".*")"#, + r#"(contains title "req")"#, + r#"(linked-by "satisfies")"#, + r#"(linked-from "satisfies")"#, + r#"(linked-to "REQ-001")"#, + r#"(links-count "satisfies" > 1)"#, + r#"(and true false)"#, + r#"(or true false)"#, + r#"(not true)"#, + r#"(implies true false)"#, + r#"(excludes true false)"#, + r#"(forall true true)"#, + r#"(exists true true)"#, + r#"(count true)"#, + r#"(reachable-from "REQ-001" "satisfies")"#, + r#"(reachable-to "REQ-001" "satisfies")"#, + ]; + for c in cases { + assert!( + sexpr_eval::parse_filter(c).is_ok(), + "advertised predicate shape did not parse: {c}" + ); + } +} diff --git a/rivet-core/tests/sexpr_fuzz.rs b/rivet-core/tests/sexpr_fuzz.rs new file mode 100644 index 0000000..dfb508d --- /dev/null +++ b/rivet-core/tests/sexpr_fuzz.rs @@ -0,0 +1,310 @@ +//! Proptest fuzz campaigns for the s-expression filter pipeline. +//! +//! Four properties: +//! +//! 1. `parse_never_panics` — `sexpr::parse` must not panic on any +//! bounded random string (success or error, but no panic). +//! 2. `lower_never_panics` — any parsed CST must lower to `Expr` or an +//! error without panicking, for any random string. +//! 3. `evaluate_never_panics` — any lowered Expr must evaluate cleanly +//! against a synthetic artifact store. +//! 4. `roundtrip_equivalence` — for a generated `Expr` AST, the pretty +//! printer round-trips through `parse_filter` and evaluates to the +//! same truth value on a fixed artifact set. +//! +//! Each campaign is capped at 256 cases to keep CI time bounded while +//! still exercising the common shrink paths. + +use std::collections::BTreeMap; + +use proptest::prelude::*; + +use rivet_core::links::LinkGraph; +use rivet_core::model::{Artifact, Link}; +use rivet_core::schema::Schema; +use rivet_core::sexpr; +use rivet_core::sexpr_eval::{self, Accessor, EvalContext, Expr, Value}; +use rivet_core::store::Store; + +// ── Fixtures ──────────────────────────────────────────────────────────── + +fn fixture_store() -> (Store, LinkGraph) { + let mk = |id: &str, t: &str, tags: &[&str], links: &[(&str, &str)]| Artifact { + id: id.into(), + artifact_type: t.into(), + title: format!("title-{id}"), + description: Some(format!("desc-{id}")), + status: Some("approved".into()), + tags: tags.iter().map(|s| s.to_string()).collect(), + links: links + .iter() + .map(|(lt, tgt)| Link { + link_type: (*lt).into(), + target: (*tgt).into(), + }) + .collect(), + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let artifacts = vec![ + mk( + "REQ-001", + "requirement", + &["stpa", "safety"], + &[("satisfies", "SC-1")], + ), + mk("REQ-002", "requirement", &["eu"], &[]), + mk("SC-1", "system-constraint", &[], &[]), + mk( + "FEAT-001", + "feature", + &["core"], + &[("implements", "REQ-001")], + ), + ]; + let mut s = Store::default(); + for a in artifacts { + s.upsert(a); + } + let g = LinkGraph::build(&s, &Schema::merge(&[])); + (s, g) +} + +fn arb_any_string() -> impl Strategy { + // Bounded random string drawn from a set that includes every + // interesting character for the s-expr lexer: parens, quotes, + // backslashes, whitespace, ASCII letters/digits, symbol-cont bytes, + // and a few Unicode characters that have tripped similar parsers. + prop::string::string_regex( + r#"[ \t\n\r()"\\!?.*<>=+\-a-zA-Z0-9_;αβ]{0,80}"#, + ) + .unwrap() +} + +// ── Expr generators (bounded depth) for round-trip ───────────────────── + +fn arb_accessor() -> impl Strategy { + prop::sample::select(vec!["id", "type", "title", "status", "description", "priority"]) + .prop_map(|s| Accessor::Field(s.to_string())) +} + +fn arb_string_value() -> impl Strategy { + prop::sample::select(vec![ + "requirement", + "feature", + "stpa", + "safety", + "eu", + "core", + "REQ-001", + "SC-1", + "approved", + "draft", + ]) + .prop_map(|s| Value::Str(s.to_string())) +} + +fn arb_leaf_expr() -> impl Strategy { + prop_oneof![ + (arb_accessor(), arb_string_value()).prop_map(|(a, v)| Expr::Eq(a, v)), + (arb_accessor(), arb_string_value()).prop_map(|(a, v)| Expr::Ne(a, v)), + arb_string_value().prop_map(Expr::HasTag), + arb_string_value().prop_map(Expr::HasField), + (arb_string_value(), arb_string_value()) + .prop_map(|(lt, tgt)| Expr::LinkedBy(lt, tgt)), + any::().prop_map(Expr::BoolLit), + ] +} + +fn arb_expr(depth: u32) -> BoxedStrategy { + if depth == 0 { + arb_leaf_expr().boxed() + } else { + let inner = arb_expr(depth - 1); + prop_oneof![ + 4 => arb_leaf_expr(), + 1 => inner.clone().prop_map(|e| Expr::Not(Box::new(e))), + 1 => (inner.clone(), inner.clone()).prop_map(|(a, b)| Expr::And(vec![a, b])), + 1 => (inner.clone(), inner.clone()).prop_map(|(a, b)| Expr::Or(vec![a, b])), + 1 => (inner.clone(), inner).prop_map(|(a, b)| Expr::Implies(Box::new(a), Box::new(b))), + ] + .boxed() + } +} + +// ── Pretty printer for Expr → sexpr text ─────────────────────────────── +// +// Only covers the shapes emitted by `arb_expr` — round-trip soundness for +// the generated subset is sufficient for this property campaign. + +fn quote(s: &str) -> String { + format!( + "\"{}\"", + s.replace('\\', "\\\\").replace('"', "\\\"") + ) +} + +fn value_to_sexpr(v: &Value) -> String { + match v { + Value::Str(s) => quote(s), + Value::Int(i) => i.to_string(), + Value::Float(f) => format!("{f}"), + Value::Bool(b) => b.to_string(), + Value::Wildcard => "_".into(), + } +} + +fn accessor_to_sexpr(a: &Accessor) -> String { + let Accessor::Field(name) = a; + name.clone() +} + +fn expr_to_sexpr(e: &Expr) -> String { + match e { + Expr::BoolLit(true) => "true".into(), + Expr::BoolLit(false) => "false".into(), + Expr::And(items) => format!( + "(and {})", + items + .iter() + .map(expr_to_sexpr) + .collect::>() + .join(" ") + ), + Expr::Or(items) => format!( + "(or {})", + items + .iter() + .map(expr_to_sexpr) + .collect::>() + .join(" ") + ), + Expr::Not(i) => format!("(not {})", expr_to_sexpr(i)), + Expr::Implies(a, b) => format!("(implies {} {})", expr_to_sexpr(a), expr_to_sexpr(b)), + Expr::Excludes(a, b) => format!("(excludes {} {})", expr_to_sexpr(a), expr_to_sexpr(b)), + Expr::Eq(a, v) => format!("(= {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Ne(a, v) => format!("(!= {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Gt(a, v) => format!("(> {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Lt(a, v) => format!("(< {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Ge(a, v) => format!("(>= {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Le(a, v) => format!("(<= {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::In(v, a) => format!("(in {} {})", value_to_sexpr(v), accessor_to_sexpr(a)), + Expr::HasTag(v) => format!("(has-tag {})", value_to_sexpr(v)), + Expr::HasField(v) => format!("(has-field {})", value_to_sexpr(v)), + Expr::Matches(a, v) => format!("(matches {} {})", accessor_to_sexpr(a), value_to_sexpr(v)), + Expr::Contains(a, v) => { + format!("(contains {} {})", accessor_to_sexpr(a), value_to_sexpr(v)) + } + Expr::LinkedBy(lt, tgt) => { + format!("(linked-by {} {})", value_to_sexpr(lt), value_to_sexpr(tgt)) + } + Expr::LinkedFrom(lt, src) => { + format!( + "(linked-from {} {})", + value_to_sexpr(lt), + value_to_sexpr(src) + ) + } + Expr::LinkedTo(tgt) => format!("(linked-to {})", value_to_sexpr(tgt)), + Expr::LinksCount(lt, op, n) => { + let op_s = match op { + sexpr_eval::CompOp::Gt => ">", + sexpr_eval::CompOp::Lt => "<", + sexpr_eval::CompOp::Ge => ">=", + sexpr_eval::CompOp::Le => "<=", + sexpr_eval::CompOp::Eq => "=", + sexpr_eval::CompOp::Ne => "!=", + }; + format!("(links-count {} {} {})", value_to_sexpr(lt), op_s, value_to_sexpr(n)) + } + Expr::Forall(scope, pred) => { + format!("(forall {} {})", expr_to_sexpr(scope), expr_to_sexpr(pred)) + } + Expr::Exists(scope, pred) => { + format!("(exists {} {})", expr_to_sexpr(scope), expr_to_sexpr(pred)) + } + Expr::Count(scope) => format!("(count {})", expr_to_sexpr(scope)), + Expr::ReachableFrom(start, lt) => format!( + "(reachable-from {} {})", + value_to_sexpr(start), + value_to_sexpr(lt) + ), + Expr::ReachableTo(tgt, lt) => format!( + "(reachable-to {} {})", + value_to_sexpr(tgt), + value_to_sexpr(lt) + ), + } +} + +// ── Properties ────────────────────────────────────────────────────────── + +proptest! { + #![proptest_config(ProptestConfig::with_cases(256))] + + /// `sexpr::parse` must never panic — it tolerates arbitrary input + /// via error recovery and error-token production. + #[test] + fn parse_never_panics(s in arb_any_string()) { + // Panicking would abort the test; any legitimate result is fine. + let _ = std::panic::catch_unwind(|| sexpr::parse(&s)); + // Also directly call — this asserts no panic leaks out. + let (_green, _errs) = sexpr::parse(&s); + } + + /// Lowering of any parsed CST must not panic. Returns either a + /// typed `Expr` or a list of `LowerError`s. + #[test] + fn lower_never_panics(s in arb_any_string()) { + let _ = sexpr_eval::parse_filter(&s); + } + + /// Evaluation of any lowered expression against a fixed artifact set + /// must not panic. Inputs that fail to parse are skipped via + /// `prop_assume!`. + #[test] + fn evaluate_never_panics(s in arb_any_string()) { + let Ok(expr) = sexpr_eval::parse_filter(&s) else { + return Ok(()); + }; + let (store, graph) = fixture_store(); + for a in store.iter() { + let ctx = EvalContext { + artifact: a, + graph: &graph, + store: Some(&store), + }; + let _ = sexpr_eval::check(&expr, &ctx); + } + } + + /// Round-trip: generate an `Expr`, pretty-print it, re-parse, and + /// check that the truth value on every fixture artifact is the same. + /// Covers the subset of `Expr` the pretty printer handles. + #[test] + fn roundtrip_equivalence(e in arb_expr(2)) { + let printed = expr_to_sexpr(&e); + let reparsed = sexpr_eval::parse_filter(&printed); + prop_assume!(reparsed.is_ok(), "pretty-print must re-parse"); + let reparsed = reparsed.unwrap(); + + let (store, graph) = fixture_store(); + for a in store.iter() { + let ctx = EvalContext { + artifact: a, + graph: &graph, + store: Some(&store), + }; + let lhs = sexpr_eval::check(&e, &ctx); + let rhs = sexpr_eval::check(&reparsed, &ctx); + prop_assert_eq!( + lhs, + rhs, + "round-trip mismatch for {:?} printed as {}", + e, + printed + ); + } + } +} diff --git a/rivet-core/tests/sexpr_predicate_matrix.rs b/rivet-core/tests/sexpr_predicate_matrix.rs new file mode 100644 index 0000000..b1de158 --- /dev/null +++ b/rivet-core/tests/sexpr_predicate_matrix.rs @@ -0,0 +1,873 @@ +//! Predicate matrix tests for the s-expression filter/evaluator. +//! +//! Every predicate recognised by `sexpr_eval::lower` gets three tests: +//! 1. positive — predicate matches an artifact that satisfies it. +//! 2. negative — predicate does not match a conflicting artifact. +//! 3. malformed — parser/lowerer rejects a bad shape with a clear error. +//! +//! Goal: close the coverage gaps identified by the sexpr audit — +//! `!=`, `>`, `<`, `>=`, `<=`, `linked-from`, `count`, `reachable-from`, +//! `reachable-to`, plus malformed-arity checks for every predicate. + +use std::collections::BTreeMap; + +use rivet_core::links::LinkGraph; +use rivet_core::model::{Artifact, Link}; +use rivet_core::schema::Schema; +use rivet_core::sexpr_eval::{self, matches_filter, matches_filter_with_store}; +use rivet_core::store::Store; + +// ── Fixtures ──────────────────────────────────────────────────────────── + +/// Artifact used for single-artifact predicate checks. +/// +/// Mirrors the fixture in `sexpr_eval::tests::test_artifact` plus a few +/// extras that this matrix exercises (numeric fields, multiple links). +fn base_artifact() -> Artifact { + Artifact { + id: "REQ-001".into(), + artifact_type: "requirement".into(), + title: "Safety goal for pedestrian detection".into(), + description: Some("STPA-derived requirement".into()), + status: Some("approved".into()), + tags: vec!["stpa".into(), "safety".into(), "eu".into()], + links: vec![ + Link { + link_type: "satisfies".into(), + target: "SC-1".into(), + }, + Link { + link_type: "satisfies".into(), + target: "SC-3".into(), + }, + Link { + link_type: "implements".into(), + target: "DD-001".into(), + }, + ], + fields: { + let mut m = BTreeMap::new(); + m.insert("priority".into(), serde_yaml::Value::String("must".into())); + m.insert( + "asil".into(), + serde_yaml::Value::String("ASIL-D".into()), + ); + m.insert( + "level".into(), + serde_yaml::Value::Number(serde_yaml::Number::from(3_i64)), + ); + m + }, + provenance: None, + source_file: None, + } +} + +/// Empty link graph — suitable for predicates that don't look at backlinks. +fn empty_graph() -> LinkGraph { + LinkGraph::build(&Store::default(), &Schema::merge(&[])) +} + +/// Helper that parses + runs a filter against a single artifact. +fn ok(filter: &str, artifact: &Artifact) -> bool { + let expr = sexpr_eval::parse_filter(filter) + .unwrap_or_else(|errs| panic!("parse failed for {filter:?}: {errs:?}")); + matches_filter(&expr, artifact, &empty_graph()) +} + +/// Helper that asserts a filter fails to parse/lower. +fn err(filter: &str) -> Vec { + sexpr_eval::parse_filter(filter) + .err() + .unwrap_or_else(|| panic!("expected parse_filter({filter:?}) to fail")) +} + +// ── Equality / inequality ────────────────────────────────────────────── + +#[test] +fn eq_matches_known_field() { + assert!(ok(r#"(= type "requirement")"#, &base_artifact())); + assert!(ok(r#"(= status "approved")"#, &base_artifact())); + assert!(ok(r#"(= priority "must")"#, &base_artifact())); + assert!(ok(r#"(= asil "ASIL-D")"#, &base_artifact())); +} + +#[test] +fn eq_no_match_on_different_value() { + assert!(!ok(r#"(= type "feature")"#, &base_artifact())); + assert!(!ok(r#"(= status "draft")"#, &base_artifact())); +} + +#[test] +fn eq_missing_field_resolves_to_empty() { + // `nonexistent` field -> empty string; equality against empty string holds. + assert!(ok(r#"(= nonexistent "")"#, &base_artifact())); + assert!(!ok(r#"(= nonexistent "something")"#, &base_artifact())); +} + +#[test] +fn eq_rejects_missing_argument() { + let errs = err(r#"(= type)"#); + assert!( + errs.iter().any(|e| e.message.contains("'=' requires")), + "expected arity complaint, got {errs:?}" + ); +} + +#[test] +fn eq_rejects_extra_argument() { + let errs = err(r#"(= type "requirement" "extra")"#); + assert!(errs.iter().any(|e| e.message.contains("'=' requires"))); +} + +#[test] +fn ne_matches_when_values_differ() { + assert!(ok(r#"(!= type "feature")"#, &base_artifact())); +} + +#[test] +fn ne_no_match_when_values_equal() { + assert!(!ok(r#"(!= type "requirement")"#, &base_artifact())); +} + +#[test] +fn ne_rejects_wrong_arity() { + let errs = err(r#"(!= type)"#); + assert!(errs.iter().any(|e| e.message.contains("'!=' requires"))); +} + +// ── Numeric comparisons: >, <, >=, <= ────────────────────────────────── + +#[test] +fn gt_matches_on_numeric_field() { + assert!(ok(r#"(> level 2)"#, &base_artifact())); + assert!(ok(r#"(> level 0)"#, &base_artifact())); +} + +#[test] +fn gt_no_match_when_field_not_greater() { + assert!(!ok(r#"(> level 3)"#, &base_artifact())); + assert!(!ok(r#"(> level 10)"#, &base_artifact())); +} + +#[test] +fn gt_non_numeric_field_is_false() { + // Non-numeric string parsed as NaN — every comparison with NaN is false. + assert!(!ok(r#"(> type 0)"#, &base_artifact())); +} + +#[test] +fn gt_rejects_wrong_arity() { + let errs = err(r#"(> level)"#); + assert!(errs.iter().any(|e| e.message.contains("'>' requires"))); +} + +#[test] +fn lt_matches_below_threshold() { + assert!(ok(r#"(< level 10)"#, &base_artifact())); +} + +#[test] +fn lt_no_match_above_threshold() { + assert!(!ok(r#"(< level 1)"#, &base_artifact())); +} + +#[test] +fn lt_rejects_wrong_arity() { + let errs = err(r#"(< level 1 extra)"#); + assert!(errs.iter().any(|e| e.message.contains("'<' requires"))); +} + +#[test] +fn ge_inclusive_boundary() { + // level = 3, so (>= level 3) matches the boundary. + assert!(ok(r#"(>= level 3)"#, &base_artifact())); + assert!(!ok(r#"(>= level 4)"#, &base_artifact())); +} + +#[test] +fn ge_rejects_wrong_arity() { + let errs = err(r#"(>=)"#); + assert!(errs.iter().any(|e| e.message.contains("'>=' requires"))); +} + +#[test] +fn le_inclusive_boundary() { + assert!(ok(r#"(<= level 3)"#, &base_artifact())); + assert!(!ok(r#"(<= level 2)"#, &base_artifact())); +} + +#[test] +fn le_accepts_float_literals() { + // Float literal path — exercises the `Value::Float -> f64` branch. + assert!(ok(r#"(<= level 3.5)"#, &base_artifact())); + assert!(!ok(r#"(<= level 2.5)"#, &base_artifact())); +} + +// ── `in` — membership on list-valued fields ──────────────────────────── + +#[test] +fn in_matches_existing_tag() { + assert!(ok(r#"(in "safety" tags)"#, &base_artifact())); +} + +#[test] +fn in_no_match_when_value_absent() { + assert!(!ok(r#"(in "missing" tags)"#, &base_artifact())); +} + +#[test] +fn in_on_scalar_field_returns_false() { + // `type` is scalar, not a list — `in` should return false, not error. + assert!(!ok(r#"(in "requirement" type)"#, &base_artifact())); +} + +#[test] +fn in_rejects_wrong_arity() { + let errs = err(r#"(in tags)"#); + assert!(errs.iter().any(|e| e.message.contains("'in' requires"))); +} + +// ── has-tag ──────────────────────────────────────────────────────────── + +#[test] +fn has_tag_matches_present_tag() { + assert!(ok(r#"(has-tag "stpa")"#, &base_artifact())); +} + +#[test] +fn has_tag_no_match_when_absent() { + assert!(!ok(r#"(has-tag "automotive")"#, &base_artifact())); +} + +#[test] +fn has_tag_rejects_missing_argument() { + let errs = err(r#"(has-tag)"#); + assert!(errs.iter().any(|e| e.message.contains("'has-tag' requires"))); +} + +#[test] +fn has_tag_rejects_extra_argument() { + let errs = err(r#"(has-tag "a" "b")"#); + assert!(errs.iter().any(|e| e.message.contains("'has-tag' requires"))); +} + +// ── has-field ────────────────────────────────────────────────────────── + +#[test] +fn has_field_matches_present_named_field() { + assert!(ok(r#"(has-field "priority")"#, &base_artifact())); + assert!(ok(r#"(has-field "status")"#, &base_artifact())); + assert!(ok(r#"(has-field "description")"#, &base_artifact())); +} + +#[test] +fn has_field_no_match_absent() { + assert!(!ok(r#"(has-field "nonexistent")"#, &base_artifact())); +} + +#[test] +fn has_field_well_known_always_present() { + assert!(ok(r#"(has-field "id")"#, &base_artifact())); + assert!(ok(r#"(has-field "type")"#, &base_artifact())); + assert!(ok(r#"(has-field "title")"#, &base_artifact())); +} + +#[test] +fn has_field_rejects_wrong_arity() { + let errs = err(r#"(has-field)"#); + assert!(errs.iter().any(|e| e.message.contains("'has-field' requires"))); +} + +// ── matches (regex) ──────────────────────────────────────────────────── + +#[test] +fn matches_regex_on_id() { + assert!(ok(r#"(matches id "^REQ-\\d+$")"#, &base_artifact())); +} + +#[test] +fn matches_no_match_for_non_matching_regex() { + assert!(!ok(r#"(matches id "^FEAT-")"#, &base_artifact())); +} + +#[test] +fn matches_invalid_regex_returns_false_safely() { + // Malformed regex — evaluator returns false instead of panicking. + assert!(!ok(r#"(matches id "[")"#, &base_artifact())); +} + +#[test] +fn matches_rejects_wrong_arity() { + let errs = err(r#"(matches id)"#); + assert!(errs.iter().any(|e| e.message.contains("'matches' requires"))); +} + +// ── contains ─────────────────────────────────────────────────────────── + +#[test] +fn contains_matches_substring() { + assert!(ok(r#"(contains title "pedestrian")"#, &base_artifact())); +} + +#[test] +fn contains_no_match_when_substring_absent() { + assert!(!ok(r#"(contains title "bicycle")"#, &base_artifact())); +} + +#[test] +fn contains_rejects_wrong_arity() { + let errs = err(r#"(contains title)"#); + assert!(errs.iter().any(|e| e.message.contains("'contains' requires"))); +} + +// ── linked-by ────────────────────────────────────────────────────────── + +#[test] +fn linked_by_matches_when_link_type_exists() { + assert!(ok(r#"(linked-by "satisfies" _)"#, &base_artifact())); + assert!(ok(r#"(linked-by "implements" _)"#, &base_artifact())); +} + +#[test] +fn linked_by_no_match_when_link_type_differs() { + assert!(!ok(r#"(linked-by "verifies" _)"#, &base_artifact())); +} + +#[test] +fn linked_by_matches_specific_target() { + assert!(ok(r#"(linked-by "satisfies" "SC-1")"#, &base_artifact())); +} + +#[test] +fn linked_by_no_match_wrong_target() { + assert!(!ok( + r#"(linked-by "satisfies" "SC-99")"#, + &base_artifact() + )); +} + +#[test] +fn linked_by_accepts_single_argument_defaults_wildcard() { + // One-arg form: equivalent to `_` wildcard target. + assert!(ok(r#"(linked-by "satisfies")"#, &base_artifact())); + assert!(!ok(r#"(linked-by "verifies")"#, &base_artifact())); +} + +#[test] +fn linked_by_rejects_too_many_args() { + let errs = err(r#"(linked-by "satisfies" _ "extra")"#); + assert!(errs.iter().any(|e| e.message.contains("'linked-by'"))); +} + +#[test] +fn linked_by_rejects_no_args() { + let errs = err(r#"(linked-by)"#); + assert!(errs.iter().any(|e| e.message.contains("'linked-by'"))); +} + +// ── linked-to ────────────────────────────────────────────────────────── + +#[test] +fn linked_to_matches_target_id() { + assert!(ok(r#"(linked-to "SC-1")"#, &base_artifact())); + assert!(ok(r#"(linked-to "DD-001")"#, &base_artifact())); +} + +#[test] +fn linked_to_no_match_for_missing_target() { + assert!(!ok(r#"(linked-to "SC-99")"#, &base_artifact())); +} + +#[test] +fn linked_to_rejects_wrong_arity() { + let errs = err(r#"(linked-to)"#); + assert!(errs.iter().any(|e| e.message.contains("'linked-to' requires"))); +} + +// ── linked-from (REQUIRES STORE GRAPH) ───────────────────────────────── + +#[test] +fn linked_from_matches_incoming_link_type() { + // Build a 2-artifact store where `SC-1` is linked FROM `REQ-001`. + let req = base_artifact(); // has `(satisfies -> SC-1)` and more + let sc = Artifact { + id: "SC-1".into(), + artifact_type: "system-constraint".into(), + title: "System constraint 1".into(), + description: None, + status: Some("approved".into()), + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let mut store = Store::default(); + store.upsert(req); + store.upsert(sc.clone()); + let schema = Schema::merge(&[]); + let graph = LinkGraph::build(&store, &schema); + + let expr = sexpr_eval::parse_filter(r#"(linked-from "satisfies" _)"#).unwrap(); + assert!(matches_filter_with_store(&expr, &sc, &graph, &store)); +} + +#[test] +fn linked_from_no_match_when_no_incoming_link() { + let orphan = Artifact { + id: "ORP-1".into(), + artifact_type: "requirement".into(), + title: "Orphan".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let mut store = Store::default(); + store.upsert(orphan.clone()); + let schema = Schema::merge(&[]); + let graph = LinkGraph::build(&store, &schema); + + let expr = sexpr_eval::parse_filter(r#"(linked-from "satisfies" _)"#).unwrap(); + assert!(!matches_filter_with_store(&expr, &orphan, &graph, &store)); +} + +#[test] +fn linked_from_no_match_wrong_link_type() { + // Same target as the positive test but a link type with no instances. + let req = base_artifact(); + let sc = Artifact { + id: "SC-1".into(), + artifact_type: "system-constraint".into(), + title: "System constraint 1".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let mut store = Store::default(); + store.upsert(req); + store.upsert(sc.clone()); + let graph = LinkGraph::build(&store, &Schema::merge(&[])); + + let expr = sexpr_eval::parse_filter(r#"(linked-from "verifies" _)"#).unwrap(); + assert!(!matches_filter_with_store(&expr, &sc, &graph, &store)); +} + +#[test] +fn linked_from_rejects_wrong_arity() { + let errs = err(r#"(linked-from)"#); + assert!(errs.iter().any(|e| e.message.contains("'linked-from'"))); +} + +// ── links-count ──────────────────────────────────────────────────────── + +#[test] +fn links_count_greater_than() { + assert!(ok(r#"(links-count "satisfies" > 1)"#, &base_artifact())); +} + +#[test] +fn links_count_exact() { + assert!(ok(r#"(links-count "satisfies" = 2)"#, &base_artifact())); +} + +#[test] +fn links_count_less_than() { + assert!(ok(r#"(links-count "satisfies" < 3)"#, &base_artifact())); +} + +#[test] +fn links_count_not_equal() { + assert!(ok(r#"(links-count "satisfies" != 99)"#, &base_artifact())); + assert!(!ok(r#"(links-count "satisfies" != 2)"#, &base_artifact())); +} + +#[test] +fn links_count_ge_le_boundary() { + assert!(ok(r#"(links-count "satisfies" >= 2)"#, &base_artifact())); + assert!(!ok(r#"(links-count "satisfies" >= 3)"#, &base_artifact())); + assert!(ok(r#"(links-count "satisfies" <= 2)"#, &base_artifact())); + assert!(!ok(r#"(links-count "satisfies" <= 1)"#, &base_artifact())); +} + +#[test] +fn links_count_rejects_bad_operator() { + let errs = err(r#"(links-count "satisfies" foo 1)"#); + assert!( + errs.iter().any(|e| e.message.contains("invalid operator")), + "expected invalid-operator message, got {errs:?}" + ); +} + +#[test] +fn links_count_rejects_wrong_arity() { + let errs = err(r#"(links-count "satisfies" >)"#); + assert!(errs.iter().any(|e| e.message.contains("'links-count'"))); +} + +#[test] +fn links_count_rejects_non_symbol_operator() { + // String-literal as operator should be flagged, not silently parsed. + let errs = err(r#"(links-count "satisfies" ">" 1)"#); + assert!(errs + .iter() + .any(|e| e.message.contains("'links-count' second argument"))); +} + +// ── not / and / or / implies / excludes ──────────────────────────────── + +#[test] +fn not_rejects_zero_args() { + let errs = err(r#"(not)"#); + assert!(errs.iter().any(|e| e.message.contains("'not' requires"))); +} + +#[test] +fn not_rejects_multiple_args() { + let errs = err(r#"(not a b)"#); + assert!(errs.iter().any(|e| e.message.contains("'not' requires"))); +} + +#[test] +fn and_variadic_all_true_is_true() { + // Variadic — more than two sub-expressions. + assert!(ok( + r#"(and (= type "requirement") (has-tag "stpa") (has-tag "safety"))"#, + &base_artifact() + )); +} + +#[test] +fn and_variadic_one_false_is_false() { + assert!(!ok( + r#"(and (= type "requirement") (has-tag "missing") (has-tag "safety"))"#, + &base_artifact() + )); +} + +#[test] +fn and_zero_args_is_identity_true() { + // `(and)` with no sub-expressions is vacuously true. + assert!(ok(r#"(and)"#, &base_artifact())); +} + +#[test] +fn or_zero_args_is_identity_false() { + // `(or)` with no sub-expressions is vacuously false. + assert!(!ok(r#"(or)"#, &base_artifact())); +} + +#[test] +fn implies_rejects_wrong_arity() { + let errs = err(r#"(implies a)"#); + assert!(errs.iter().any(|e| e.message.contains("'implies' requires"))); +} + +#[test] +fn excludes_semantics_match_definition() { + // (excludes A B) == (not (and A B)) + let a = base_artifact(); + // A: has-tag "stpa" — true + // B: has-tag "missing" — false + assert!(ok( + r#"(excludes (has-tag "stpa") (has-tag "missing"))"#, + &a + )); + // Both true → excludes is false. + assert!(!ok( + r#"(excludes (has-tag "stpa") (has-tag "safety"))"#, + &a + )); +} + +#[test] +fn excludes_rejects_wrong_arity() { + let errs = err(r#"(excludes a)"#); + assert!(errs + .iter() + .any(|e| e.message.contains("'excludes' requires"))); +} + +// ── forall / exists / count — require a Store ────────────────────────── + +fn make_req(id: &str, tags: &[&str]) -> Artifact { + Artifact { + id: id.into(), + artifact_type: "requirement".into(), + title: format!("title of {id}"), + description: None, + status: Some("approved".into()), + tags: tags.iter().map(|s| s.to_string()).collect(), + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + } +} + +fn store_of(arts: Vec) -> (Store, LinkGraph) { + let mut s = Store::default(); + for a in arts { + s.upsert(a); + } + let g = LinkGraph::build(&s, &Schema::merge(&[])); + (s, g) +} + +#[test] +fn forall_positive_via_parse_filter() { + let (store, graph) = store_of(vec![ + make_req("REQ-1", &["safety"]), + make_req("REQ-2", &["safety"]), + ]); + let expr = sexpr_eval::parse_filter( + r#"(forall (= type "requirement") (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn forall_negative_one_violates() { + let (store, graph) = store_of(vec![ + make_req("REQ-1", &["safety"]), + make_req("REQ-2", &[]), // violates + ]); + let expr = sexpr_eval::parse_filter( + r#"(forall (= type "requirement") (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(!matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn forall_rejects_wrong_arity() { + let errs = err(r#"(forall (= type "requirement"))"#); + assert!(errs.iter().any(|e| e.message.contains("'forall' requires"))); +} + +#[test] +fn exists_positive_via_parse_filter() { + let (store, graph) = store_of(vec![ + make_req("REQ-1", &[]), + make_req("REQ-2", &["safety"]), + ]); + let expr = sexpr_eval::parse_filter( + r#"(exists (= type "requirement") (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn exists_negative_no_match() { + let (store, graph) = store_of(vec![ + make_req("REQ-1", &[]), + make_req("REQ-2", &["eu"]), + ]); + let expr = sexpr_eval::parse_filter( + r#"(exists (= type "requirement") (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(!matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn exists_rejects_wrong_arity() { + let errs = err(r#"(exists true)"#); + assert!(errs.iter().any(|e| e.message.contains("'exists' requires"))); +} + +#[test] +fn count_positive_any_match() { + // `count` returns true if any artifact matches the scope. + let (store, graph) = store_of(vec![make_req("REQ-1", &["safety"])]); + let expr = sexpr_eval::parse_filter( + r#"(count (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn count_negative_no_match() { + let (store, graph) = store_of(vec![make_req("REQ-1", &[])]); + let expr = sexpr_eval::parse_filter( + r#"(count (has-tag "safety"))"#, + ) + .unwrap(); + let any = store.iter().next().unwrap(); + assert!(!matches_filter_with_store(&expr, any, &graph, &store)); +} + +#[test] +fn count_rejects_wrong_arity() { + let errs = err(r#"(count)"#); + assert!(errs.iter().any(|e| e.message.contains("'count' requires"))); +} + +#[test] +fn quantifier_without_store_is_safe_false() { + // No store → forall/exists return false. We can parse + evaluate + // against an unrelated artifact without crashing. + let expr = sexpr_eval::parse_filter(r#"(exists true (has-tag "safety"))"#).unwrap(); + let a = base_artifact(); + assert!(!matches_filter(&expr, &a, &empty_graph())); +} + +// ── reachable-from / reachable-to ─────────────────────────────────────── + +fn chain_store() -> (Store, LinkGraph) { + // A -> B -> C via "satisfies" + let mk = |id: &str, tgt: Option<&str>| Artifact { + id: id.into(), + artifact_type: "requirement".into(), + title: id.into(), + description: None, + status: None, + tags: vec![], + links: tgt + .map(|t| vec![Link { + link_type: "satisfies".into(), + target: t.into(), + }]) + .unwrap_or_default(), + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let a = mk("REQ-A", Some("REQ-B")); + let b = mk("REQ-B", Some("REQ-C")); + let c = mk("REQ-C", None); + store_of(vec![a, b, c]) +} + +#[test] +fn reachable_from_matches_transitive() { + // From REQ-A via "satisfies" reaches REQ-B, REQ-C. + let (store, graph) = chain_store(); + let expr = sexpr_eval::parse_filter(r#"(reachable-from "REQ-A" "satisfies")"#).unwrap(); + let b = store.get("REQ-B").unwrap(); + let c = store.get("REQ-C").unwrap(); + assert!(matches_filter_with_store(&expr, b, &graph, &store)); + assert!(matches_filter_with_store(&expr, c, &graph, &store)); +} + +#[test] +fn reachable_from_excludes_source() { + let (store, graph) = chain_store(); + let expr = sexpr_eval::parse_filter(r#"(reachable-from "REQ-A" "satisfies")"#).unwrap(); + let a = store.get("REQ-A").unwrap(); + // `reachable()` removes the start node from results. + assert!(!matches_filter_with_store(&expr, a, &graph, &store)); +} + +#[test] +fn reachable_from_wrong_link_type_no_match() { + let (store, graph) = chain_store(); + let expr = sexpr_eval::parse_filter(r#"(reachable-from "REQ-A" "verifies")"#).unwrap(); + let c = store.get("REQ-C").unwrap(); + assert!(!matches_filter_with_store(&expr, c, &graph, &store)); +} + +#[test] +fn reachable_from_rejects_wrong_arity() { + let errs = err(r#"(reachable-from "REQ-A")"#); + assert!(errs + .iter() + .any(|e| e.message.contains("'reachable-from' requires"))); +} + +#[test] +fn reachable_to_matches_downstream() { + let (store, graph) = chain_store(); + // From REQ-A, `REQ-C` is reachable via "satisfies". + let expr = sexpr_eval::parse_filter(r#"(reachable-to "REQ-C" "satisfies")"#).unwrap(); + let a = store.get("REQ-A").unwrap(); + assert!(matches_filter_with_store(&expr, a, &graph, &store)); +} + +#[test] +fn reachable_to_no_match_wrong_direction() { + let (store, graph) = chain_store(); + // From REQ-C there is no outgoing satisfies to REQ-A. + let expr = sexpr_eval::parse_filter(r#"(reachable-to "REQ-A" "satisfies")"#).unwrap(); + let c = store.get("REQ-C").unwrap(); + assert!(!matches_filter_with_store(&expr, c, &graph, &store)); +} + +#[test] +fn reachable_to_rejects_wrong_arity() { + let errs = err(r#"(reachable-to "REQ-A")"#); + assert!(errs + .iter() + .any(|e| e.message.contains("'reachable-to' requires"))); +} + +// ── Structural error cases ───────────────────────────────────────────── + +#[test] +fn unknown_head_form_is_rejected() { + let errs = err(r#"(foobar a b)"#); + assert!(errs + .iter() + .any(|e| e.message.contains("unknown form 'foobar'"))); +} + +#[test] +fn bare_symbol_at_top_level_is_rejected() { + // `foo` is a symbol atom, not a bool — top-level atoms must be booleans. + let errs = err(r#"foo"#); + assert!(errs + .iter() + .any(|e| e.message.contains("unexpected atom at top level"))); +} + +#[test] +fn unclosed_paren_is_rejected() { + let errs = err(r#"(and (has-tag "x")"#); + assert!(!errs.is_empty()); + assert!(errs.iter().any(|e| e.message.contains("expected ')'"))); +} + +#[test] +fn unexpected_close_paren_is_rejected() { + let errs = err(r#")"#); + assert!(errs.iter().any(|e| e.message.contains("unexpected ')'"))); +} + +#[test] +fn empty_list_evaluates_true() { + // `()` lowers to BoolLit(true) — documented behaviour. + let expr = sexpr_eval::parse_filter("()").unwrap(); + assert!(matches_filter(&expr, &base_artifact(), &empty_graph())); +} + +#[test] +fn multiple_top_level_exprs_combine_as_and() { + // Two top-level forms are joined with AND. + let expr = sexpr_eval::parse_filter( + r#"(= type "requirement") (has-tag "stpa")"#, + ) + .unwrap(); + assert!(matches_filter(&expr, &base_artifact(), &empty_graph())); + + let expr2 = sexpr_eval::parse_filter( + r#"(= type "requirement") (has-tag "missing")"#, + ) + .unwrap(); + assert!(!matches_filter(&expr2, &base_artifact(), &empty_graph())); +} From 801e513fa35f13f63611a94372e075c224d18ad8 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 23:33:54 +0200 Subject: [PATCH 2/2] fix(sexpr): honour the source-id argument of linked-from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `(linked-from "satisfies" "REQ-A")` silently ignored its second argument — the evaluator's `Expr::LinkedFrom` arm bound the source parameter as `_source` and only checked the link type. A filter naming a specific source ID got the same result set as the wildcard form, which hid real differences at the source level. This is the same class of bug as the `links-count` operator drop fixed in v0.4.2 — lowerer accepts the argument, evaluator throws it away — so the fix follows the same pattern as `Expr::LinkedBy`: treat `Value::Wildcard` as "any source" and otherwise require an exact match against the backlink source. Adds a regression test (`linked_from_source_filter_is_honoured`) in the predicate-matrix audit suite that exercises both the specific-id and wildcard forms on a store with two distinct source artifacts. Fixes: REQ-004 Verifies: REQ-004 --- rivet-core/src/sexpr_eval.rs | 9 ++- rivet-core/tests/sexpr_predicate_matrix.rs | 71 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/rivet-core/src/sexpr_eval.rs b/rivet-core/src/sexpr_eval.rs index a352dc6..47e49cb 100644 --- a/rivet-core/src/sexpr_eval.rs +++ b/rivet-core/src/sexpr_eval.rs @@ -58,6 +58,8 @@ pub enum Expr { /// `(linked-by "satisfies" _)` — has outgoing link of type. LinkedBy(Value, Value), /// `(linked-from "implements" _)` — has incoming link of type. + /// Second argument restricts the source: `_` matches any source, + /// otherwise the incoming link's source id must match exactly. LinkedFrom(Value, Value), /// `(linked-to "SPEC-021")` — has a link targeting specific ID. LinkedTo(Value), @@ -190,10 +192,13 @@ pub fn check(expr: &Expr, ctx: &EvalContext) -> bool { l.link_type == lt && (matches!(target, Value::Wildcard) || l.target == tgt) }) } - Expr::LinkedFrom(link_type, _source) => { + Expr::LinkedFrom(link_type, source) => { let lt = value_to_str(link_type); let backlinks = ctx.graph.backlinks_to(&ctx.artifact.id); - backlinks.iter().any(|bl| bl.link_type == lt) + backlinks.iter().any(|bl| { + bl.link_type == lt + && (matches!(source, Value::Wildcard) || bl.source == value_to_str(source)) + }) } Expr::LinkedTo(target_id) => { let tgt = value_to_str(target_id); diff --git a/rivet-core/tests/sexpr_predicate_matrix.rs b/rivet-core/tests/sexpr_predicate_matrix.rs index b1de158..8dc0693 100644 --- a/rivet-core/tests/sexpr_predicate_matrix.rs +++ b/rivet-core/tests/sexpr_predicate_matrix.rs @@ -467,6 +467,77 @@ fn linked_from_rejects_wrong_arity() { assert!(errs.iter().any(|e| e.message.contains("'linked-from'"))); } +/// Regression: the source-filter argument of `linked-from` was silently +/// ignored. `(linked-from "satisfies" "REQ-A")` must only match when +/// REQ-A is actually the source of an incoming satisfies link. +#[test] +fn linked_from_source_filter_is_honoured() { + // Two different requirements both link into SC-1 via satisfies. + let req_a = Artifact { + id: "REQ-A".into(), + artifact_type: "requirement".into(), + title: "A".into(), + description: None, + status: None, + tags: vec![], + links: vec![Link { + link_type: "satisfies".into(), + target: "SC-1".into(), + }], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let req_b = Artifact { + id: "REQ-B".into(), + artifact_type: "requirement".into(), + title: "B".into(), + description: None, + status: None, + tags: vec![], + links: vec![Link { + link_type: "satisfies".into(), + target: "SC-1".into(), + }], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let sc = Artifact { + id: "SC-1".into(), + artifact_type: "system-constraint".into(), + title: "SC".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let mut store = Store::default(); + store.upsert(req_a); + store.upsert(req_b); + store.upsert(sc.clone()); + let graph = LinkGraph::build(&store, &Schema::merge(&[])); + + // Specific existing source → matches. + let specific = sexpr_eval::parse_filter(r#"(linked-from "satisfies" "REQ-A")"#).unwrap(); + assert!(matches_filter_with_store(&specific, &sc, &graph, &store)); + + // Wildcard also matches. + let wild = sexpr_eval::parse_filter(r#"(linked-from "satisfies" _)"#).unwrap(); + assert!(matches_filter_with_store(&wild, &sc, &graph, &store)); + + // Non-existent source MUST not match — this is the bug fix. + let missing = + sexpr_eval::parse_filter(r#"(linked-from "satisfies" "REQ-NOPE")"#).unwrap(); + assert!( + !matches_filter_with_store(&missing, &sc, &graph, &store), + "`(linked-from \"satisfies\" \"REQ-NOPE\")` must not match when no such source exists" + ); +} + // ── links-count ──────────────────────────────────────────────────────── #[test]