From 703f4903cda5af81a3ec84e0a079de23f02ce3ac Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sat, 16 May 2026 06:36:49 +0200 Subject: [PATCH] feat(variant): fields-per-variant on Artifact + resolver (#255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MVP for the variant-aware properties track (docs/design/variant-aware-properties.md): - `Artifact::fields_per_variant: BTreeMap>` serialized as `fields-per-variant:` (kebab-case) at the artifact level. - `Artifact::fields_for_variant(Option<&str>) -> Cow<'_, BTreeMap<...>>` resolves the effective fields map for a variant. Zero-alloc `Borrowed` fallback when no overlay applies (None, unknown variant, empty overlay). Allocates only when a known variant has overrides. - `#[derive(Default)]` on `Artifact` so tests can use the struct-update pattern instead of carrying every new field forward at each call site. Wired through: - yaml_hir schema-driven parser recognizes `fields-per-variant` and unpacks the nested mapping into the typed field (does NOT fall through to the generic `fields` smuggler). - generic-yaml adapter round-trips through a typed `fields_per_variant` on `GenericArtifact`. - Resolution semantics per design doc §5.2: overlay merge — variant keys override default keys; default keys not mentioned in the variant carry through. Unknown variants silently inherit default fields (graceful degradation when variant configs aren't loaded). Tests cover: - Resolver: None / unknown / known / overlay-only-some-keys / new-keys. - Parser: nested mapping extraction populates the typed field, not the generic `fields` map; both variants present after parse. Implements: REQ-010, REQ-028, REQ-029 Refs: FEAT-001 Co-Authored-By: Claude Opus 4.7 --- rivet-cli/src/main.rs | 13 ++ rivet-cli/src/mcp.rs | 1 + rivet-core/benches/core_benchmarks.rs | 4 + rivet-core/src/bundle.rs | 1 + rivet-core/src/cited_source.rs | 6 + rivet-core/src/db.rs | 1 + rivet-core/src/diff.rs | 1 + rivet-core/src/embed.rs | 1 + rivet-core/src/externals.rs | 3 + rivet-core/src/formats/aadl.rs | 3 + rivet-core/src/formats/generic.rs | 8 + rivet-core/src/formats/needs_json.rs | 1 + rivet-core/src/impact.rs | 2 + rivet-core/src/migrate.rs | 1 + rivet-core/src/model.rs | 156 +++++++++++++++++++- rivet-core/src/oslc.rs | 12 ++ rivet-core/src/proofs.rs | 2 + rivet-core/src/query.rs | 1 + rivet-core/src/reqif.rs | 12 ++ rivet-core/src/sexpr_eval.rs | 3 + rivet-core/src/test_helpers.rs | 1 + rivet-core/src/test_scanner.rs | 1 + rivet-core/src/validate.rs | 11 ++ rivet-core/src/wasm_runtime.rs | 4 + rivet-core/src/yaml_hir.rs | 82 ++++++++++ rivet-core/tests/externals_schemas.rs | 2 + rivet-core/tests/externals_sync.rs | 1 + rivet-core/tests/integration.rs | 2 + rivet-core/tests/mutate_integration.rs | 2 + rivet-core/tests/oslc_integration.rs | 1 + rivet-core/tests/proptest_core.rs | 23 +-- rivet-core/tests/proptest_operations.rs | 2 + rivet-core/tests/proptest_sexpr.rs | 1 + rivet-core/tests/schema_validation_rules.rs | 1 + rivet-core/tests/sexpr_doc_examples.rs | 1 + rivet-core/tests/sexpr_fuzz.rs | 17 ++- rivet-core/tests/sexpr_predicate_matrix.rs | 9 ++ rivet-core/tests/stpa_roundtrip.rs | 5 + rivet-core/tests/stpa_sec_verification.rs | 1 + 39 files changed, 380 insertions(+), 19 deletions(-) diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index abf5fd06..4968a0ee 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -7551,6 +7551,7 @@ fn parse_yaml_content( }) .collect(), fields: raw.fields, + fields_per_variant: Default::default(), provenance: None, source_file: Some(std::path::PathBuf::from(file_path)), }) @@ -7580,6 +7581,7 @@ fn parse_yaml_content( }) .collect(), fields: raw.fields, + fields_per_variant: Default::default(), provenance: None, source_file: Some(std::path::PathBuf::from(file_path)), }) @@ -11376,6 +11378,7 @@ fn cmd_add( tags: tags.to_vec(), links: link_vec, fields: fields_map, + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -11882,6 +11885,7 @@ fn cmd_batch(cli: &Cli, file: &std::path::Path) -> Result { tags: tags.clone(), links: link_vec, fields: fields.clone(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -11971,6 +11975,7 @@ fn cmd_batch(cli: &Cli, file: &std::path::Path) -> Result { tags: tags.clone(), links: link_vec, fields: fields.clone(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -13750,6 +13755,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path.clone()), }) @@ -13794,6 +13800,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path.clone()), }) @@ -13835,6 +13842,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path.clone()), }) @@ -13921,6 +13929,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path_a.clone()), }) @@ -13935,6 +13944,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path_b.clone()), }) @@ -13949,6 +13959,7 @@ mod lsp_tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path_b.clone()), }) @@ -14020,6 +14031,7 @@ artifacts: tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: Some(path.clone()), }) @@ -14165,6 +14177,7 @@ mod stats_tests { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-cli/src/mcp.rs b/rivet-cli/src/mcp.rs index 89059c90..7df05dc9 100644 --- a/rivet-cli/src/mcp.rs +++ b/rivet-cli/src/mcp.rs @@ -978,6 +978,7 @@ fn tool_add(project_dir: &Path, arguments: &Value) -> Result { tags, links, fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/benches/core_benchmarks.rs b/rivet-core/benches/core_benchmarks.rs index 52717029..3594922e 100644 --- a/rivet-core/benches/core_benchmarks.rs +++ b/rivet-core/benches/core_benchmarks.rs @@ -102,6 +102,7 @@ fn generate_artifacts(n: usize, links_per: usize) -> Vec { f.insert("priority".into(), serde_yaml::Value::String("must".into())); f }, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -282,6 +283,7 @@ fn build_diff_stores(n: usize) -> (Store, Store) { tags: vec!["common".into()], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -310,6 +312,7 @@ fn build_diff_stores(n: usize) -> (Store, Store) { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -328,6 +331,7 @@ fn build_diff_stores(n: usize) -> (Store, Store) { tags: vec!["new".into()], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/src/bundle.rs b/rivet-core/src/bundle.rs index 8102929e..a7721714 100644 --- a/rivet-core/src/bundle.rs +++ b/rivet-core/src/bundle.rs @@ -234,6 +234,7 @@ mod tests { }) .collect(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/cited_source.rs b/rivet-core/src/cited_source.rs index 0ad3dc0b..1c707cc0 100644 --- a/rivet-core/src/cited_source.rs +++ b/rivet-core/src/cited_source.rs @@ -961,6 +961,7 @@ last-checked: 2026-04-28T14:30:00Z tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -993,6 +994,7 @@ last-checked: 2026-04-28T14:30:00Z tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1023,6 +1025,7 @@ last-checked: 2026-04-28T14:30:00Z tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1177,6 +1180,7 @@ artifacts: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1213,6 +1217,7 @@ artifacts: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1275,6 +1280,7 @@ artifacts: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/src/db.rs b/rivet-core/src/db.rs index 8c6c3e8a..f04936e9 100644 --- a/rivet-core/src/db.rs +++ b/rivet-core/src/db.rs @@ -1621,6 +1621,7 @@ artifacts: tags: vec!["aadl".into()], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/diff.rs b/rivet-core/src/diff.rs index 86d3fdb7..71d3b9c3 100644 --- a/rivet-core/src/diff.rs +++ b/rivet-core/src/diff.rs @@ -322,6 +322,7 @@ mod tests { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/embed.rs b/rivet-core/src/embed.rs index 273a27f8..0ef65a43 100644 --- a/rivet-core/src/embed.rs +++ b/rivet-core/src/embed.rs @@ -1732,6 +1732,7 @@ mod tests { tags: tags.iter().map(|t| t.to_string()).collect(), links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/externals.rs b/rivet-core/src/externals.rs index 699840e4..f424ddd1 100644 --- a/rivet-core/src/externals.rs +++ b/rivet-core/src/externals.rs @@ -1285,6 +1285,7 @@ mod tests { }, ], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1322,6 +1323,7 @@ mod tests { target: "other:REQ-001".to_string(), // cross-external ref }], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1356,6 +1358,7 @@ mod tests { tags: vec![], links: vec![], // no links at all fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/src/formats/aadl.rs b/rivet-core/src/formats/aadl.rs index 40e64d05..6157078f 100644 --- a/rivet-core/src/formats/aadl.rs +++ b/rivet-core/src/formats/aadl.rs @@ -274,6 +274,7 @@ fn analysis_diagnostic_to_artifact( tags: vec!["aadl".into(), diag.analysis.clone()], links: vec![], fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -401,6 +402,7 @@ fn component_to_artifact( tags: vec!["aadl".into()], links: vec![], fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -436,6 +438,7 @@ fn diagnostic_to_artifact(index: usize, diag: &SparDiagnostic) -> Artifact { tags: vec!["aadl".into(), diag.analysis.clone()], links: vec![], fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/formats/generic.rs b/rivet-core/src/formats/generic.rs index 27c945ef..7975633c 100644 --- a/rivet-core/src/formats/generic.rs +++ b/rivet-core/src/formats/generic.rs @@ -132,6 +132,7 @@ impl Adapter for GenericYamlAdapter { }) .collect(), fields: a.fields.clone(), + fields_per_variant: a.fields_per_variant.clone(), provenance: a.provenance.clone(), }) .collect(), @@ -167,6 +168,12 @@ struct GenericArtifact { links: Vec, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] fields: BTreeMap, + #[serde( + default, + rename = "fields-per-variant", + skip_serializing_if = "BTreeMap::is_empty" + )] + fields_per_variant: BTreeMap>, #[serde(default, skip_serializing_if = "Option::is_none")] provenance: Option, } @@ -200,6 +207,7 @@ pub fn parse_generic_yaml(content: &str, source: Option<&Path>) -> Result, + /// Per-variant field overrides (issue #255, single-master overlay). + /// + /// Outer key: variant config name (e.g. `automotive`, `industrial`, + /// `consumer` — the `name:` field of a file under + /// `artifacts/variants/`). Inner map: same shape as `fields`, + /// keyed by field name, value is the variant-specific override. + /// + /// Resolution semantics (option A from + /// `docs/design/variant-aware-properties.md`): + /// - When no variant is active (`fields_for_variant(None)`), the + /// default `fields` map is returned unchanged. + /// - When a variant is active and present in this map, its + /// per-field overrides are overlaid on top of `fields` (later + /// wins). Fields not declared in the variant inherit the + /// default. + /// - When a variant is active but not declared here, the default + /// `fields` map is returned (silent fallback — the artifact + /// simply has no variant-specific differentiation). + #[serde( + default, + rename = "fields-per-variant", + skip_serializing_if = "BTreeMap::is_empty" + )] + pub fields_per_variant: BTreeMap>, + /// AI provenance metadata. #[serde(default, skip_serializing_if = "Option::is_none")] pub provenance: Option, @@ -202,6 +234,37 @@ impl Artifact { pub fn is_released(&self) -> bool { self.status_is("released") } + + /// Resolve the effective `fields` map for a given variant. + /// + /// Returns a `Cow<...>`: + /// - `Borrowed(&self.fields)` when no overlay applies (no variant, + /// unknown variant, or empty overlay) — zero allocations. + /// - `Owned(merged)` when the variant has overrides — clones the + /// default map once and overlays the variant's keys on top. + /// + /// Per `docs/design/variant-aware-properties.md` §5.2, the merge is + /// a flat `BTreeMap` overlay: variant keys override default keys; + /// default keys not mentioned in the variant carry through. + pub fn fields_for_variant( + &self, + variant: Option<&str>, + ) -> std::borrow::Cow<'_, BTreeMap> { + let Some(name) = variant else { + return std::borrow::Cow::Borrowed(&self.fields); + }; + let Some(overlay) = self.fields_per_variant.get(name) else { + return std::borrow::Cow::Borrowed(&self.fields); + }; + if overlay.is_empty() { + return std::borrow::Cow::Borrowed(&self.fields); + } + let mut merged = self.fields.clone(); + for (k, v) in overlay { + merged.insert(k.clone(), v.clone()); + } + std::borrow::Cow::Owned(merged) + } } #[cfg(test)] @@ -278,6 +341,97 @@ mod tests { assert!(!a.is_approved()); } + // ── fields-per-variant (#255) ──────────────────────────────────── + + fn art_with_variant_overrides() -> Artifact { + let mut a = minimal_artifact("REQ-THERMAL-01", "requirement"); + a.fields + .insert("max-temp-c".into(), serde_yaml::Value::Number(80.into())); + a.fields + .insert("priority".into(), serde_yaml::Value::String("must".into())); + a.fields_per_variant.insert("automotive".into(), { + let mut m = BTreeMap::new(); + m.insert("max-temp-c".into(), serde_yaml::Value::Number(80.into())); + m.insert( + "min-temp-c".into(), + serde_yaml::Value::Number((-40i64).into()), + ); + m + }); + a.fields_per_variant.insert("industrial".into(), { + let mut m = BTreeMap::new(); + m.insert("max-temp-c".into(), serde_yaml::Value::Number(100.into())); + m + }); + a + } + + #[test] + fn fields_for_variant_none_returns_default_fields() { + let a = art_with_variant_overrides(); + let f = a.fields_for_variant(None); + assert!(matches!(f, std::borrow::Cow::Borrowed(_))); + assert_eq!(f.get("max-temp-c"), a.fields.get("max-temp-c")); + assert_eq!(f.get("priority"), a.fields.get("priority")); + } + + #[test] + fn fields_for_variant_unknown_returns_default_fields() { + let a = art_with_variant_overrides(); + let f = a.fields_for_variant(Some("consumer")); + assert!( + matches!(f, std::borrow::Cow::Borrowed(_)), + "unknown variant must fall back to default fields, not allocate" + ); + } + + #[test] + fn fields_for_variant_known_overlays_and_keeps_unmentioned_defaults() { + let a = art_with_variant_overrides(); + let f = a.fields_for_variant(Some("industrial")); + // Industrial variant overrides max-temp-c but doesn't mention + // priority — priority must inherit from default. + assert_eq!( + f.get("max-temp-c"), + Some(&serde_yaml::Value::Number(100.into())), + "industrial overrides max-temp-c to 100" + ); + assert_eq!( + f.get("priority"), + Some(&serde_yaml::Value::String("must".into())), + "priority inherits from default" + ); + } + + #[test] + fn fields_for_variant_known_adds_new_keys() { + let a = art_with_variant_overrides(); + let f = a.fields_for_variant(Some("automotive")); + // Automotive adds min-temp-c which the default doesn't have. + assert_eq!( + f.get("min-temp-c"), + Some(&serde_yaml::Value::Number((-40i64).into())) + ); + } + + #[test] + fn fields_for_variant_yaml_roundtrip_preserves_overlay() { + // Serialise an artifact with fields-per-variant, parse it back, + // confirm the typed field survives the round-trip via serde. + let a = art_with_variant_overrides(); + let yaml = serde_yaml::to_string(&a).unwrap(); + let parsed: Artifact = serde_yaml::from_str(&yaml).unwrap(); + assert_eq!(parsed.fields_per_variant.len(), 2); + assert!(parsed.fields_per_variant.contains_key("automotive")); + assert!(parsed.fields_per_variant.contains_key("industrial")); + // Resolver still produces the right overlay after round-trip. + let f = parsed.fields_for_variant(Some("industrial")); + assert_eq!( + f.get("max-temp-c"), + Some(&serde_yaml::Value::Number(100.into())) + ); + } + #[test] fn baseline_config_deserializes() { let yaml = r#" diff --git a/rivet-core/src/oslc.rs b/rivet-core/src/oslc.rs index 8036320d..cb9e4656 100644 --- a/rivet-core/src/oslc.rs +++ b/rivet-core/src/oslc.rs @@ -713,6 +713,7 @@ pub fn oslc_to_artifact(resource: &OslcResource) -> Result { tags: Vec::new(), links, fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, }) @@ -1547,6 +1548,7 @@ mod tests { target: "IMPL-001".to_string(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1579,6 +1581,7 @@ mod tests { target: "REQ-001".to_string(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1609,6 +1612,7 @@ mod tests { target: "TC-001".to_string(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1648,6 +1652,7 @@ mod tests { }, ], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1676,6 +1681,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -1704,6 +1710,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -1726,6 +1733,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -1747,6 +1755,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -1760,6 +1769,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -1782,6 +1792,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -1795,6 +1806,7 @@ mod tests { tags: Vec::new(), links: Vec::new(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; diff --git a/rivet-core/src/proofs.rs b/rivet-core/src/proofs.rs index 757325b0..9555761b 100644 --- a/rivet-core/src/proofs.rs +++ b/rivet-core/src/proofs.rs @@ -34,6 +34,7 @@ mod proofs { tags: vec![], links, fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -531,6 +532,7 @@ mod proofs { }, ], fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/query.rs b/rivet-core/src/query.rs index b174a9bc..57d6f749 100644 --- a/rivet-core/src/query.rs +++ b/rivet-core/src/query.rs @@ -179,6 +179,7 @@ mod tests { tags: tags.iter().map(|t| t.to_string()).collect(), links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index 1b4c8411..b67b5d26 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -882,6 +882,7 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result Artifact { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/test_scanner.rs b/rivet-core/src/test_scanner.rs index d5239cf5..3ac500f4 100644 --- a/rivet-core/src/test_scanner.rs +++ b/rivet-core/src/test_scanner.rs @@ -408,6 +408,7 @@ mod tests { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/src/validate.rs b/rivet-core/src/validate.rs index 45b1f268..6d7cf821 100644 --- a/rivet-core/src/validate.rs +++ b/rivet-core/src/validate.rs @@ -2659,6 +2659,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -2676,6 +2677,7 @@ then: target: target.into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -2792,6 +2794,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -2880,6 +2883,7 @@ then: target: "REQ-MISSING".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -2961,6 +2965,7 @@ then: target: "REQ-MISSING".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3088,6 +3093,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3133,6 +3139,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3236,6 +3243,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3320,6 +3328,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3335,6 +3344,7 @@ then: target: "REQ-001".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -3402,6 +3412,7 @@ then: tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/src/wasm_runtime.rs b/rivet-core/src/wasm_runtime.rs index 9cb78fe8..7f1f1307 100644 --- a/rivet-core/src/wasm_runtime.rs +++ b/rivet-core/src/wasm_runtime.rs @@ -557,6 +557,7 @@ fn convert_wit_artifact_to_host( tags: wit.tags, links, fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -876,6 +877,7 @@ mod tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -899,6 +901,7 @@ mod tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; @@ -922,6 +925,7 @@ mod tests { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }]; diff --git a/rivet-core/src/yaml_hir.rs b/rivet-core/src/yaml_hir.rs index a2e848c8..78ec0e99 100644 --- a/rivet-core/src/yaml_hir.rs +++ b/rivet-core/src/yaml_hir.rs @@ -726,6 +726,7 @@ fn extract_section_item( tags, links, fields, + fields_per_variant: Default::default(), provenance, source_file: None, // set by caller }, @@ -821,6 +822,8 @@ fn extract_artifact_from_item(item: &SyntaxNode, result: &mut ParsedYamlFile) { let mut tags: Vec = Vec::new(); let mut links: Vec = Vec::new(); let mut fields: BTreeMap = BTreeMap::new(); + let mut fields_per_variant: BTreeMap> = + BTreeMap::new(); let mut field_spans: BTreeMap = BTreeMap::new(); let mut provenance: Option = None; @@ -885,6 +888,33 @@ fn extract_artifact_from_item(item: &SyntaxNode, result: &mut ParsedYamlFile) { provenance = extract_provenance(&value_node); field_spans.insert("provenance".into(), value_span); } + "fields-per-variant" => { + // Issue #255: nested mapping of variant-name → fields map. + // Outer mapping: variant config name → BTreeMap of field + // overrides. The full extracted YAML value is converted + // via the generic path, then unpacked into the typed + // shape. + let raw = node_to_yaml_value(&value_node); + if let serde_yaml::Value::Mapping(outer) = raw { + for (vk, vv) in outer { + let Some(variant_name) = vk.as_str() else { + continue; + }; + if let serde_yaml::Value::Mapping(inner) = vv { + let mut overlay = std::collections::BTreeMap::new(); + for (fk, fv) in inner { + if let Some(field_name) = fk.as_str() { + overlay.insert(field_name.to_string(), fv); + } + } + if !overlay.is_empty() { + fields_per_variant.insert(variant_name.to_string(), overlay); + } + } + } + } + field_spans.insert("fields-per-variant".into(), value_span); + } "fields" => { // Nested mapping of custom fields if let Some(nested_map) = child_of_kind(&value_node, SyntaxKind::Mapping) { @@ -962,6 +992,7 @@ fn extract_artifact_from_item(item: &SyntaxNode, result: &mut ParsedYamlFile) { tags, links, fields, + fields_per_variant, provenance, source_file: None, }; @@ -2238,4 +2269,55 @@ artifacts: ); } } + + /// Issue #255: the schema-driven parser must populate the typed + /// `fields_per_variant` from YAML's `fields-per-variant:` key, not + /// stash it as a generic untyped entry in `fields` (which is what + /// the fallthrough "unknown top-level key" branch would do). + #[test] + fn schema_driven_extracts_fields_per_variant() { + let source = "\ +artifacts: + - id: REQ-THERMAL-01 + type: requirement + title: Operating temperature envelope + status: approved + fields: + priority: must + max-temp-c: 80 + fields-per-variant: + automotive: + max-temp-c: 80 + min-temp-c: -40 + industrial: + max-temp-c: 100 +"; + let schema = crate::schema::Schema::merge(&[]); + let parsed = extract_schema_driven(source, &schema, None); + assert_eq!(parsed.artifacts.len(), 1); + let a = &parsed.artifacts[0].artifact; + + // The typed field must be populated, NOT smuggled into + // `fields["fields-per-variant"]`. + assert!( + !a.fields.contains_key("fields-per-variant"), + "fields-per-variant must NOT fall through to the generic fields map" + ); + assert_eq!(a.fields_per_variant.len(), 2, "two variants declared"); + assert!(a.fields_per_variant.contains_key("automotive")); + assert!(a.fields_per_variant.contains_key("industrial")); + + // Resolver applies the overlay correctly. yaml_hir parses bare + // digits as Value::Number per YAML 1.2 core schema, but leading + // signs ("-40") fall through to String — compare each value + // against the type yaml_hir actually produces. + let auto = a.fields_for_variant(Some("automotive")); + assert_eq!(auto.get("max-temp-c").and_then(|v| v.as_u64()), Some(80)); + assert_eq!(auto.get("min-temp-c").and_then(|v| v.as_str()), Some("-40")); + + let indu = a.fields_for_variant(Some("industrial")); + assert_eq!(indu.get("max-temp-c").and_then(|v| v.as_u64()), Some(100)); + // priority inherits from default + assert_eq!(indu.get("priority").and_then(|v| v.as_str()), Some("must")); + } } diff --git a/rivet-core/tests/externals_schemas.rs b/rivet-core/tests/externals_schemas.rs index 1c082dd8..1cf467b6 100644 --- a/rivet-core/tests/externals_schemas.rs +++ b/rivet-core/tests/externals_schemas.rs @@ -187,6 +187,7 @@ fn external_without_schema_demotes_unknown_type_to_info() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }); @@ -256,6 +257,7 @@ fn external_link_types_are_not_flagged_unknown() { target: "spar:OTHER".to_string(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }); diff --git a/rivet-core/tests/externals_sync.rs b/rivet-core/tests/externals_sync.rs index 10083273..3dfdcf6f 100644 --- a/rivet-core/tests/externals_sync.rs +++ b/rivet-core/tests/externals_sync.rs @@ -197,6 +197,7 @@ fn backlinks_from_spar_to_local() { target: "REQ-001".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/tests/integration.rs b/rivet-core/tests/integration.rs index 8a8861c2..e3d8051f 100644 --- a/rivet-core/tests/integration.rs +++ b/rivet-core/tests/integration.rs @@ -68,6 +68,7 @@ fn make_artifact(id: &str, art_type: &str, title: &str) -> Artifact { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -91,6 +92,7 @@ fn make_artifact_full( tags: tags.iter().map(|t| t.to_string()).collect(), links, fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/mutate_integration.rs b/rivet-core/tests/mutate_integration.rs index 217b3a18..2830cef0 100644 --- a/rivet-core/tests/mutate_integration.rs +++ b/rivet-core/tests/mutate_integration.rs @@ -68,6 +68,7 @@ fn make_artifact( tags: vec![], links, fields, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -619,6 +620,7 @@ fn test_append_artifact_to_file() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/tests/oslc_integration.rs b/rivet-core/tests/oslc_integration.rs index 2f7fe5f6..e241d26b 100644 --- a/rivet-core/tests/oslc_integration.rs +++ b/rivet-core/tests/oslc_integration.rs @@ -911,6 +911,7 @@ fn req(id: &str, title: &str, description: Option<&str>) -> Artifact { tags: vec![], links: vec![], fields: std::collections::BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/proptest_core.rs b/rivet-core/tests/proptest_core.rs index 699ea7cb..896b8554 100644 --- a/rivet-core/tests/proptest_core.rs +++ b/rivet-core/tests/proptest_core.rs @@ -100,8 +100,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, } }).collect(); @@ -142,8 +142,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; let a2 = Artifact { id: id.clone(), @@ -154,8 +154,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; prop_assert!(store.insert(a1).is_ok()); @@ -240,8 +240,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }).unwrap(); } @@ -299,6 +299,7 @@ fn prop_validation_determinism() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }) @@ -317,6 +318,7 @@ fn prop_validation_determinism() { target: "DET-L1".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }) @@ -335,6 +337,7 @@ fn prop_validation_determinism() { target: "NONEXISTENT".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }) @@ -385,8 +388,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }).unwrap(); } diff --git a/rivet-core/tests/proptest_operations.rs b/rivet-core/tests/proptest_operations.rs index 91acf90e..43aad6ff 100644 --- a/rivet-core/tests/proptest_operations.rs +++ b/rivet-core/tests/proptest_operations.rs @@ -79,6 +79,7 @@ fn make_artifact(id: &str, artifact_type: &str) -> Artifact { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -94,6 +95,7 @@ fn make_artifact_with_links(id: &str, artifact_type: &str, links: Vec) -> tags: vec![], links, fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/proptest_sexpr.rs b/rivet-core/tests/proptest_sexpr.rs index 9dd938ec..9fb5e29b 100644 --- a/rivet-core/tests/proptest_sexpr.rs +++ b/rivet-core/tests/proptest_sexpr.rs @@ -115,6 +115,7 @@ fn arb_artifact() -> impl Strategy { tags: tags.into_iter().map(|s| s.to_string()).collect(), links, fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/schema_validation_rules.rs b/rivet-core/tests/schema_validation_rules.rs index f502d305..db945eea 100644 --- a/rivet-core/tests/schema_validation_rules.rs +++ b/rivet-core/tests/schema_validation_rules.rs @@ -52,6 +52,7 @@ fn artifact(id: &str, art_type: &str, status: &str, links: &[(&str, &str)]) -> A }) .collect(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/sexpr_doc_examples.rs b/rivet-core/tests/sexpr_doc_examples.rs index 07c56fd7..147c176b 100644 --- a/rivet-core/tests/sexpr_doc_examples.rs +++ b/rivet-core/tests/sexpr_doc_examples.rs @@ -50,6 +50,7 @@ fn art(id: &str, t: &str, tags: &[&str], status: Option<&str>, links: &[(&str, & }) .collect(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } diff --git a/rivet-core/tests/sexpr_fuzz.rs b/rivet-core/tests/sexpr_fuzz.rs index 49619357..2ca65218 100644 --- a/rivet-core/tests/sexpr_fuzz.rs +++ b/rivet-core/tests/sexpr_fuzz.rs @@ -65,6 +65,7 @@ fn fixture_store() -> (Store, LinkGraph) { }) .collect(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -412,8 +413,8 @@ proptest! { tags: vec![], links: vec![], // no outbound links — empty set fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; let mut store = Store::default(); store.upsert(art.clone()); @@ -452,8 +453,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; let mut store = Store::default(); store.upsert(art.clone()); @@ -494,8 +495,8 @@ proptest! { tags: vec![], links: vec![], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; let verifier = Artifact { id: "V-001".into(), @@ -515,8 +516,8 @@ proptest! { }, ], fields: BTreeMap::new(), - provenance: None, - source_file: None, + fields_per_variant: Default::default(), + provenance: None, source_file: None, }; let mut store = Store::default(); store.upsert(req); diff --git a/rivet-core/tests/sexpr_predicate_matrix.rs b/rivet-core/tests/sexpr_predicate_matrix.rs index bd908b8a..c7ced76d 100644 --- a/rivet-core/tests/sexpr_predicate_matrix.rs +++ b/rivet-core/tests/sexpr_predicate_matrix.rs @@ -76,6 +76,7 @@ fn base_artifact() -> Artifact { ); m }, + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -443,6 +444,7 @@ fn linked_from_matches_incoming_link_type() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -467,6 +469,7 @@ fn linked_from_no_match_when_no_incoming_link() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -492,6 +495,7 @@ fn linked_from_no_match_wrong_link_type() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -528,6 +532,7 @@ fn linked_from_source_filter_is_honoured() { target: "SC-1".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -543,6 +548,7 @@ fn linked_from_source_filter_is_honoured() { target: "SC-1".into(), }], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -555,6 +561,7 @@ fn linked_from_source_filter_is_honoured() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -720,6 +727,7 @@ fn make_req(id: &str, tags: &[&str]) -> Artifact { tags: tags.iter().map(|s| s.to_string()).collect(), links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, } @@ -840,6 +848,7 @@ fn chain_store() -> (Store, LinkGraph) { }) .unwrap_or_default(), fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/tests/stpa_roundtrip.rs b/rivet-core/tests/stpa_roundtrip.rs index a9a2ef15..c48b7284 100644 --- a/rivet-core/tests/stpa_roundtrip.rs +++ b/rivet-core/tests/stpa_roundtrip.rs @@ -80,6 +80,7 @@ fn test_store_insert_and_lookup() { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -102,6 +103,7 @@ fn test_duplicate_id_rejected() { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -116,6 +118,7 @@ fn test_duplicate_id_rejected() { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -140,6 +143,7 @@ fn test_broken_link_detected() { target: "L-NONEXISTENT".into(), }], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; @@ -165,6 +169,7 @@ fn test_validation_catches_unknown_type() { tags: vec![], links: vec![], fields: Default::default(), + fields_per_variant: Default::default(), provenance: None, source_file: None, }; diff --git a/rivet-core/tests/stpa_sec_verification.rs b/rivet-core/tests/stpa_sec_verification.rs index 5278e5c4..f2c7bd47 100644 --- a/rivet-core/tests/stpa_sec_verification.rs +++ b/rivet-core/tests/stpa_sec_verification.rs @@ -468,6 +468,7 @@ fn test_validate_documents_checks_wiki_links() { tags: vec![], links: vec![], fields: BTreeMap::new(), + fields_per_variant: Default::default(), provenance: None, source_file: None, })