From a80f29924dbe060566c5d94c04bd6b31615e1982 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:31:32 +0200 Subject: [PATCH 1/6] fix(reqif): round-trip AI provenance via rivet:* string attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReqIF importer set `provenance: None` unconditionally and the exporter never emitted any provenance data — every AI-provenance field was ABSENT on Path 2 per the fidelity analysis in docs/design/polarion-reqif-fidelity.md. Define a stable ReqIF AttributeDefinition scheme (five `rivet:*` string attributes: created-by, model, session-id, timestamp, reviewed-by) and round-trip the full `Provenance` struct. Absence on import stays `None` for backward compatibility with files that don't carry rivet metadata. Adds two regression tests: - test_provenance_roundtrip — all five fields survive - test_provenance_absent_stays_none — backward-compat contract Fixes: REQ-025 --- rivet-core/src/reqif.rs | 178 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 176 insertions(+), 2 deletions(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index b32b840..0caba43 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -26,7 +26,7 @@ use serde::{Deserialize, Serialize}; use crate::adapter::{Adapter, AdapterConfig, AdapterSource}; use crate::error::Error; -use crate::model::{Artifact, Link}; +use crate::model::{Artifact, Link, Provenance}; // ── ReqIF XML structures ──────────────────────────────────────────────── // @@ -465,6 +465,24 @@ const ATTR_DEF_STATUS: &str = "ATTR-STATUS"; const ATTR_DEF_TAGS: &str = "ATTR-TAGS"; const ATTR_DEF_ARTIFACT_TYPE: &str = "ATTR-ARTIFACT-TYPE"; +// Provenance attribute definition identifiers and long-names. +// +// These expose rivet's AI-provenance metadata over ReqIF as a stable +// convention: five string attributes prefixed with `rivet:`. Other tools +// that don't know about rivet will ignore the unknown attribute names. +// Files that don't carry them round-trip as `provenance: None`. +const ATTR_DEF_PROV_CREATED_BY: &str = "ATTR-RIVET-CREATED-BY"; +const ATTR_DEF_PROV_MODEL: &str = "ATTR-RIVET-MODEL"; +const ATTR_DEF_PROV_SESSION_ID: &str = "ATTR-RIVET-SESSION-ID"; +const ATTR_DEF_PROV_TIMESTAMP: &str = "ATTR-RIVET-TIMESTAMP"; +const ATTR_DEF_PROV_REVIEWED_BY: &str = "ATTR-RIVET-REVIEWED-BY"; + +const PROV_LONG_CREATED_BY: &str = "rivet:created-by"; +const PROV_LONG_MODEL: &str = "rivet:model"; +const PROV_LONG_SESSION_ID: &str = "rivet:session-id"; +const PROV_LONG_TIMESTAMP: &str = "rivet:timestamp"; +const PROV_LONG_REVIEWED_BY: &str = "rivet:reviewed-by"; + // ── Adapter ───────────────────────────────────────────────────────────── pub struct ReqIfAdapter { @@ -655,6 +673,12 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result = None; let mut reqif_name: Option = None; let mut reqif_text: Option = None; + // Rivet AI-provenance attributes. + let mut prov_created_by: Option = None; + let mut prov_model: Option = None; + let mut prov_session_id: Option = None; + let mut prov_timestamp: Option = None; + let mut prov_reviewed_by: Option = None; if let Some(values) = &obj.values { for av in &values.string_values { @@ -703,6 +727,32 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result { + if !av.the_value.is_empty() { + prov_created_by = Some(av.the_value.clone()); + } + } + PROV_LONG_MODEL => { + if !av.the_value.is_empty() { + prov_model = Some(av.the_value.clone()); + } + } + PROV_LONG_SESSION_ID => { + if !av.the_value.is_empty() { + prov_session_id = Some(av.the_value.clone()); + } + } + PROV_LONG_TIMESTAMP => { + if !av.the_value.is_empty() { + prov_timestamp = Some(av.the_value.clone()); + } + } + PROV_LONG_REVIEWED_BY => { + if !av.the_value.is_empty() { + prov_reviewed_by = Some(av.the_value.clone()); + } + } _ => { fields.insert( attr_name.to_string(), @@ -770,6 +820,17 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result) -> Result ReqIfRoot { }, ]; + // Rivet AI-provenance attribute definitions. Emitted on every + // SpecObjectType so tools that preserve attribute ordering don't + // drop them; values are only set per-SpecObject when the source + // Artifact carries provenance. + for (ident, long_name) in [ + (ATTR_DEF_PROV_CREATED_BY, PROV_LONG_CREATED_BY), + (ATTR_DEF_PROV_MODEL, PROV_LONG_MODEL), + (ATTR_DEF_PROV_SESSION_ID, PROV_LONG_SESSION_ID), + (ATTR_DEF_PROV_TIMESTAMP, PROV_LONG_TIMESTAMP), + (ATTR_DEF_PROV_REVIEWED_BY, PROV_LONG_REVIEWED_BY), + ] { + string_attrs.push(AttributeDefinitionString { + identifier: ident.into(), + long_name: Some(long_name.into()), + datatype_ref: Some(DatatypeRef { + datatype_ref: DATATYPE_STRING_ID.into(), + }), + }); + } + for fname in &field_names { string_attrs.push(AttributeDefinitionString { identifier: format!("ATTR-{fname}"), @@ -964,6 +1045,30 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { }, ]; + // Emit rivet AI-provenance when present. Fields with `None` + // values are skipped — only the non-empty metadata survives. + if let Some(p) = &a.provenance { + let entries: [(&str, Option<&str>); 5] = [ + (ATTR_DEF_PROV_CREATED_BY, Some(p.created_by.as_str())), + (ATTR_DEF_PROV_MODEL, p.model.as_deref()), + (ATTR_DEF_PROV_SESSION_ID, p.session_id.as_deref()), + (ATTR_DEF_PROV_TIMESTAMP, p.timestamp.as_deref()), + (ATTR_DEF_PROV_REVIEWED_BY, p.reviewed_by.as_deref()), + ]; + for (ident, val) in entries { + if let Some(v) = val { + if !v.is_empty() { + string_values.push(AttributeValueString { + the_value: v.to_string(), + definition: AttrDefinitionRef { + attr_def_ref: ident.into(), + }, + }); + } + } + } + } + for (key, value) in &a.fields { let val_str = match value { serde_yaml::Value::String(s) => s.clone(), @@ -1275,4 +1380,73 @@ mod tests { // Unmapped types pass through unchanged assert_eq!(arts[1].artifact_type, "section"); } + + /// Provenance must round-trip through ReqIF. Rivet's AI-provenance + /// metadata is encoded as five `rivet:*` string attributes on every + /// SpecObject. Absence on the way in → `None` on the way out. + /// + /// Regression test for the bug documented in + /// `docs/design/polarion-reqif-fidelity.md` row "provenance.*" where every + /// provenance field was ABSENT on Path 2. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_provenance_roundtrip() { + let art = Artifact { + id: "REQ-PROV".into(), + artifact_type: "requirement".into(), + title: "Provenance carrier".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: Some(Provenance { + created_by: "ai-assisted".into(), + model: Some("claude-opus-4-7".into()), + session_id: Some("s-1234".into()), + timestamp: Some("2026-04-19T12:34:56Z".into()), + reviewed_by: Some("alice".into()), + }), + source_file: None, + }; + + let adapter = ReqIfAdapter::new(); + let config = AdapterConfig::default(); + let bytes = adapter.export(&[art.clone()], &config).unwrap(); + let re = adapter + .import(&AdapterSource::Bytes(bytes), &config) + .unwrap(); + + assert_eq!(re.len(), 1); + assert_eq!(re[0].provenance, art.provenance); + } + + /// Files without any provenance attributes parse back to `None` — the + /// backward-compatibility contract. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_provenance_absent_stays_none() { + let art = Artifact { + id: "REQ-NOPROV".into(), + artifact_type: "requirement".into(), + title: "No provenance".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + let adapter = ReqIfAdapter::new(); + let config = AdapterConfig::default(); + let bytes = adapter.export(&[art], &config).unwrap(); + let re = adapter + .import(&AdapterSource::Bytes(bytes), &config) + .unwrap(); + assert_eq!(re.len(), 1); + assert!(re[0].provenance.is_none()); + } } From 389d8ccdf713dede591a6763d997963a33796fa1 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:34:15 +0200 Subject: [PATCH 2/6] fix(reqif): emit typed fields via canonical strings, not Debug-form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exporter wrote `format!(\"{other:?}\")` for any non-string `serde_yaml::Value` in `Artifact.fields`, producing XML payloads like `\"Bool(true)\"` and `\"Sequence [String(\\\"a\\\")]\"` — not something any ReqIF consumer (Polarion, DOORS, StrictDoc) could parse. Replace with `encode_field_value` doing explicit per-variant conversion: - `Bool` → `\"true\"` / `\"false\"` - `Number` → decimal string - `Sequence` / `Mapping` → JSON (well-known, reversible, and a YAML subset so it survives re-parse) - `Null` → attribute omitted - `Tagged` → recurse on inner value The importer mirrors this with `decode_field_value`: best-effort JSON recovery for content that unambiguously looks structured (leading `[`, `{`, `true`, `false`, or a digit/sign); anything else stays a string. Adds two regression tests: - test_non_string_fields_roundtrip — bool/int/float/list round-trip and the raw XML contains no `Bool(`, `Number(`, or `Sequence [` fragments. - test_null_field_dropped_on_export — null fields are omitted, not emitted as empty attributes. Fixes: REQ-025 --- rivet-core/src/reqif.rs | 202 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 188 insertions(+), 14 deletions(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index 0caba43..4eceb48 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -754,10 +754,7 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result { - fields.insert( - attr_name.to_string(), - serde_yaml::Value::String(av.the_value.clone()), - ); + fields.insert(attr_name.to_string(), decode_field_value(&av.the_value)); } } } @@ -1070,16 +1067,14 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } for (key, value) in &a.fields { - let val_str = match value { - serde_yaml::Value::String(s) => s.clone(), - other => format!("{other:?}"), - }; - string_values.push(AttributeValueString { - the_value: val_str, - definition: AttrDefinitionRef { - attr_def_ref: format!("ATTR-{key}"), - }, - }); + if let Some(val_str) = encode_field_value(value) { + string_values.push(AttributeValueString { + the_value: val_str, + definition: AttrDefinitionRef { + attr_def_ref: format!("ATTR-{key}"), + }, + }); + } } SpecObject { @@ -1148,6 +1143,67 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } } +/// Encode a `serde_yaml::Value` as a ReqIF ATTRIBUTE-VALUE-STRING string. +/// +/// ReqIF 1.2's STRING attribute only carries text, so we apply explicit +/// type-aware conversions rather than Rust's `Debug` format (which emitted +/// gibberish like `"Bool(true)"` or `"Sequence [String(\"a\")]"`). +/// +/// Conventions: +/// - `String(s)` → `s` verbatim. +/// - `Bool(b)` → `"true"` / `"false"`. +/// - `Number(n)` → decimal string representation. +/// - `Sequence` → JSON array representation (lossless, reversible). +/// - `Mapping` → JSON object representation (ReqIF has no native map type). +/// - `Null` → attribute omitted (returns `None`). +/// - `Tagged(t)` → recurse into inner value, tag itself is not preserved. +fn encode_field_value(value: &serde_yaml::Value) -> Option { + match value { + serde_yaml::Value::Null => None, + serde_yaml::Value::String(s) => Some(s.clone()), + serde_yaml::Value::Bool(b) => Some(if *b { "true".into() } else { "false".into() }), + serde_yaml::Value::Number(n) => Some(n.to_string()), + serde_yaml::Value::Sequence(_) | serde_yaml::Value::Mapping(_) => { + // JSON is both a well-understood interchange form and a YAML + // subset, so the string remains valid input to serde_yaml on + // import. + serde_json::to_string(value).ok() + } + serde_yaml::Value::Tagged(t) => encode_field_value(&t.value), + } +} + +/// Attempt to recover the original `serde_yaml::Value` type from a ReqIF +/// ATTRIBUTE-VALUE-STRING written by `encode_field_value`. +/// +/// This is best-effort type recovery for round-trip fidelity: values that +/// unambiguously parse as JSON booleans, numbers, arrays, or objects are +/// reconstructed as the matching YAML variant. Anything that doesn't +/// parse — the common case for free-form text — is kept as a string. +fn decode_field_value(s: &str) -> serde_yaml::Value { + // Strings that happen to round-trip through JSON unchanged (plain text + // without a leading quote) must not be re-typed, so we only attempt + // JSON recovery for content that looks like a JSON scalar/compound. + let trimmed = s.trim_start(); + let looks_structured = trimmed.starts_with('{') + || trimmed.starts_with('[') + || trimmed == "true" + || trimmed == "false" + || trimmed + .chars() + .next() + .is_some_and(|c| c == '-' || c.is_ascii_digit()); + + if looks_structured { + if let Ok(v) = serde_json::from_str::(s) { + if let Ok(yaml_v) = serde_yaml::to_value(&v) { + return yaml_v; + } + } + } + serde_yaml::Value::String(s.to_string()) +} + /// Serialize a ReqIF document to XML bytes. pub fn serialize_reqif(root: &ReqIfRoot) -> Result, Error> { let xml_body = xml_to_string(root) @@ -1422,6 +1478,124 @@ mod tests { assert_eq!(re[0].provenance, art.provenance); } + /// Non-string `fields` values must round-trip without Rust-`Debug` + /// coercion. Regression test for `reqif.rs:968-970` flagged in + /// `docs/design/polarion-reqif-fidelity.md`: previously a bool field + /// emitted `"Bool(true)"` instead of `"true"`, a list emitted the + /// Rust-internal `Sequence[String("…")]` form. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_non_string_fields_roundtrip() { + let mut fields: BTreeMap = BTreeMap::new(); + fields.insert("safety-critical".into(), serde_yaml::Value::Bool(true)); + fields.insert( + "asil-level".into(), + serde_yaml::Value::Number(serde_yaml::Number::from(3i64)), + ); + fields.insert( + "confidence".into(), + serde_yaml::Value::Number(serde_yaml::Number::from(0.85f64)), + ); + fields.insert( + "aliases".into(), + serde_yaml::Value::Sequence(vec![ + serde_yaml::Value::String("req-a".into()), + serde_yaml::Value::String("req-b".into()), + ]), + ); + + let art = Artifact { + id: "REQ-FIELDS".into(), + artifact_type: "requirement".into(), + title: "Typed fields".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields: fields.clone(), + provenance: None, + source_file: None, + }; + + let adapter = ReqIfAdapter::new(); + let config = AdapterConfig::default(); + let bytes = adapter.export(&[art], &config).unwrap(); + + // The raw XML must not contain Rust `Debug` form artefacts like + // `Bool(`, `Number(`, or `Sequence[`. Those would indicate the bug + // has regressed. + let xml = std::str::from_utf8(&bytes).unwrap(); + assert!( + !xml.contains("Bool("), + "Debug-form Bool leaked into XML: {xml}" + ); + assert!( + !xml.contains("Sequence ["), + "Debug-form Sequence leaked into XML: {xml}" + ); + assert!(!xml.contains("Number("), "Debug-form Number leaked"); + + let re = adapter + .import(&AdapterSource::Bytes(bytes), &config) + .unwrap(); + assert_eq!(re.len(), 1); + assert_eq!( + re[0].fields.get("safety-critical"), + Some(&serde_yaml::Value::Bool(true)) + ); + // Integer equality via Number. + let asil = re[0].fields.get("asil-level").unwrap(); + if let serde_yaml::Value::Number(n) = asil { + assert_eq!(n.as_i64(), Some(3)); + } else { + panic!("asil-level lost its number type: {asil:?}"); + } + // Float equality. + let conf = re[0].fields.get("confidence").unwrap(); + if let serde_yaml::Value::Number(n) = conf { + assert!((n.as_f64().unwrap() - 0.85).abs() < 1e-9); + } else { + panic!("confidence lost its number type: {conf:?}"); + } + // Sequence recovered. + let aliases = re[0].fields.get("aliases").unwrap(); + if let serde_yaml::Value::Sequence(items) = aliases { + assert_eq!(items.len(), 2); + } else { + panic!("aliases did not round-trip as sequence: {aliases:?}"); + } + } + + /// Null field values are dropped (not emitted as empty attributes). + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_null_field_dropped_on_export() { + let mut fields: BTreeMap = BTreeMap::new(); + fields.insert("deprecated".into(), serde_yaml::Value::Null); + let art = Artifact { + id: "REQ-NULL".into(), + artifact_type: "requirement".into(), + title: "Null field".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields, + provenance: None, + source_file: None, + }; + let adapter = ReqIfAdapter::new(); + let bytes = adapter.export(&[art], &AdapterConfig::default()).unwrap(); + let re = adapter + .import(&AdapterSource::Bytes(bytes), &AdapterConfig::default()) + .unwrap(); + assert_eq!(re.len(), 1); + // Null is not present after round-trip (attribute omitted). + assert!(re[0].fields.get("deprecated").is_none()); + } + /// Files without any provenance attributes parse back to `None` — the /// backward-compatibility contract. // rivet: verifies REQ-025 From 6f1c0cb7c2d0670991a13a9b74236050af024cac Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:35:53 +0200 Subject: [PATCH 3/6] fix(reqif): encode tags as JSON array so commas and whitespace survive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tags were joined on `, ` at export and split on `,` at import, so any tag containing a comma (`\"safety, critical\"`) or leading whitespace was silently mangled on re-import — a silent corruption flagged in the fidelity scorecard. Switch to JSON array encoding on export (`[\"safety, critical\", \"plain\"]`). `decode_tags` auto-detects the form: values starting with `[` are parsed as JSON; everything else falls back to the legacy comma-split, keeping backward compatibility with older rivet exports and ReqIF files from other tools. Adds two regression tests: - test_tags_with_special_chars_roundtrip — commas, leading space, and quotes all survive export/import. - test_tags_legacy_comma_form_parses — the fallback path still works. Fixes: REQ-025 --- rivet-core/src/reqif.rs | 99 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 12 deletions(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index 4eceb48..5db1ce8 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -694,12 +694,7 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result { - tags = av - .the_value - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); + tags = decode_tags(&av.the_value); } "artifact-type" => { if !av.the_value.is_empty() { @@ -785,11 +780,7 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result { - tags = value - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); + tags = decode_tags(&value); } _ => { fields.insert(attr_name.to_string(), serde_yaml::Value::String(value)); @@ -1029,7 +1020,7 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { }, }, AttributeValueString { - the_value: a.tags.join(", "), + the_value: encode_tags(&a.tags), definition: AttrDefinitionRef { attr_def_ref: ATTR_DEF_TAGS.into(), }, @@ -1143,6 +1134,39 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } } +/// Encode `tags` as a JSON array string for ReqIF transport. +/// +/// The previous implementation joined with `", "` and split on `,` — any +/// tag containing a comma (e.g. `"safety, critical"`) or leading +/// whitespace got mangled on round-trip. JSON array form is predictable, +/// reversible, and uses a well-known escaping convention that all ReqIF +/// consumers understand as a plain string. +/// +/// For backward compatibility the importer also accepts the legacy +/// comma-joined form, so files produced by older rivet versions or other +/// tools keep working (see `decode_tags`). +fn encode_tags(tags: &[String]) -> String { + if tags.is_empty() { + return String::new(); + } + serde_json::to_string(tags).unwrap_or_default() +} + +/// Decode a tags attribute value. Preferred form is a JSON array; +/// falls back to comma-split for backward compatibility. +fn decode_tags(s: &str) -> Vec { + let trimmed = s.trim_start(); + if trimmed.starts_with('[') { + if let Ok(v) = serde_json::from_str::>(s) { + return v; + } + } + s.split(',') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() +} + /// Encode a `serde_yaml::Value` as a ReqIF ATTRIBUTE-VALUE-STRING string. /// /// ReqIF 1.2's STRING attribute only carries text, so we apply explicit @@ -1596,6 +1620,57 @@ mod tests { assert!(re[0].fields.get("deprecated").is_none()); } + /// Tags containing commas or leading whitespace must round-trip intact. + /// Regression for the bug at `reqif.rs:953-958` vs `reqif.rs:672-679`: + /// previously the exporter joined with `, ` and the importer split on + /// `,`, so any tag with a comma was silently split on re-import. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_tags_with_special_chars_roundtrip() { + let art = Artifact { + id: "REQ-TAGS".into(), + artifact_type: "requirement".into(), + title: "Tags with specials".into(), + description: None, + status: None, + tags: vec![ + "safety, critical".into(), // contains comma + " leading-space".into(), // leading whitespace + "plain".into(), + "with \"quotes\"".into(), + ], + links: vec![], + fields: BTreeMap::new(), + provenance: None, + source_file: None, + }; + + let adapter = ReqIfAdapter::new(); + let config = AdapterConfig::default(); + let bytes = adapter.export(&[art.clone()], &config).unwrap(); + let re = adapter + .import(&AdapterSource::Bytes(bytes), &config) + .unwrap(); + assert_eq!(re.len(), 1); + assert_eq!( + re[0].tags, art.tags, + "tags with commas/whitespace lost on round-trip" + ); + } + + /// Legacy comma-joined tags (from older rivet exports or other tools) + /// are still parsed correctly on import — backward-compat contract. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_tags_legacy_comma_form_parses() { + assert_eq!( + decode_tags("alpha, beta , gamma"), + vec!["alpha".to_string(), "beta".to_string(), "gamma".to_string()] + ); + } + /// Files without any provenance attributes parse back to `None` — the /// backward-compatibility contract. // rivet: verifies REQ-025 From d4fe254103a27eba9106323737f97c81503895dc Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:37:07 +0200 Subject: [PATCH 4/6] fix(reqif): stamp REQ-IF-HEADER CREATION-TIME with current UTC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header hardcoded `creation_time: None`, so every export emitted an empty CREATION-TIME — tools like Polarion's ReqIF importer have nothing to record against, and diffing two exports over time becomes impossible. Add `reqif_creation_timestamp()` which returns ISO-8601 UTC using the same `std::time::SystemTime` + civil_from_days algorithm already used by `export.rs::timestamp_now`, keeping the no-chrono dep contract. Adds regression test test_creation_time_is_stamped asserting the XML contains a non-empty `` in the 20-char ISO form. Fixes: REQ-025 --- rivet-core/src/reqif.rs | 67 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index 5db1ce8..fe77442 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -1111,7 +1111,7 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { req_if_header: ReqIfHeader { identifier: "rivet-export".into(), comment: Some("Generated by Rivet SDLC tool".into()), - creation_time: None, + creation_time: Some(reqif_creation_timestamp()), repository_id: None, req_if_tool_id: Some("rivet".into()), req_if_version: Some("1.2".into()), @@ -1134,6 +1134,38 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } } +/// Current UTC timestamp in ISO 8601 format, for the REQ-IF-HEADER +/// CREATION-TIME element. Inline implementation (no chrono/jiff dep) — +/// see `export.rs::timestamp_now` for the same algorithm used in HTML +/// export. Uses Howard Hinnant's civil_from_days algorithm for the +/// year/month/day breakdown; no leap-second handling. +fn reqif_creation_timestamp() -> String { + let now = std::time::SystemTime::now(); + let duration = now + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default(); + let secs = duration.as_secs(); + let days = secs / 86400; + let time_secs = secs % 86400; + let hours = time_secs / 3600; + let minutes = (time_secs % 3600) / 60; + let seconds = time_secs % 60; + let (year, month, day) = { + let days = days + 719_468; + let era = days / 146_097; + let doe = days - era * 146_097; + let yoe = (doe - doe / 1460 + doe / 36524 - doe / 146_096) / 365; + let y = yoe + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let d = doy - (153 * mp + 2) / 5 + 1; + let m = if mp < 10 { mp + 3 } else { mp - 9 }; + let y = if m <= 2 { y + 1 } else { y }; + (y, m, d) + }; + format!("{year:04}-{month:02}-{day:02}T{hours:02}:{minutes:02}:{seconds:02}Z") +} + /// Encode `tags` as a JSON array string for ReqIF transport. /// /// The previous implementation joined with `", "` and split on `,` — any @@ -1659,6 +1691,39 @@ mod tests { ); } + /// The exported REQ-IF-HEADER must carry a non-empty CREATION-TIME in + /// ISO-8601 UTC form. Regression for the hardcoded `creation_time: None` + /// flagged in the fidelity scorecard. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_creation_time_is_stamped() { + let arts = sample_artifacts(); + let adapter = ReqIfAdapter::new(); + let bytes = adapter.export(&arts, &AdapterConfig::default()).unwrap(); + let xml = std::str::from_utf8(&bytes).unwrap(); + + // Element must be present and non-empty. + assert!( + xml.contains(""), + "CREATION-TIME missing from exported XML: {xml}" + ); + assert!( + !xml.contains(""), + "CREATION-TIME is empty in exported XML" + ); + + // Re-parse and confirm the header field round-trips. + let root: ReqIfRoot = quick_xml::de::from_str(xml).unwrap(); + let ct = root.the_header.req_if_header.creation_time.as_deref(); + assert!(ct.is_some(), "creation_time deserialized as None"); + let ct = ct.unwrap(); + // ISO 8601 form: YYYY-MM-DDTHH:MM:SSZ (20 chars). + assert_eq!(ct.len(), 20, "creation_time not ISO 8601: {ct}"); + assert!(ct.ends_with('Z')); + assert!(ct.contains('T')); + } + /// Legacy comma-joined tags (from older rivet exports or other tools) /// are still parsed correctly on import — backward-compat contract. // rivet: verifies REQ-025 From eac5efa39739c66d46bbd07676398d4ef618b68d Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:43:27 +0200 Subject: [PATCH 5/6] fix(reqif): emit ENUMERATION datatype for schema allowed-values fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exporter always wrote a single `DATATYPE-DEFINITION-STRING` and mapped every field to a STRING attribute, silently flattening any `allowed-values` constraint the schema declared (e.g. severity: [catastrophic, critical, marginal, negligible]). Downstream tools that consume ReqIF lose the closed-enum semantics. Add `ReqIfAdapter::with_schema(schema)` and a new `build_reqif_with_schema(artifacts, Option<&Schema>)`; `build_reqif` now delegates to it with `None` for backward compatibility. When a schema is attached and the artifact's `ArtifactTypeDef.fields` declares `allowed-values`, the exporter emits: - one `DATATYPE-DEFINITION-ENUMERATION` per (artifact-type, field) pair - an `ATTRIBUTE-DEFINITION-ENUMERATION` on the matching SpecObjectType - an `ATTRIBUTE-VALUE-ENUMERATION` on each SpecObject whose value matches an allowed label; values outside the enum fall back to STRING so validate.rs can still flag them. The importer path is unchanged — it already recognised ATTRIBUTE-VALUE-ENUMERATION via the existing StrictDoc compatibility code — so the round-trip closes. Adds two regression tests: - test_schema_enum_field_emits_enumeration — round-trip through a real `Schema` with `allowed-values: [catastrophic, critical, …]`. - test_export_without_schema_stays_string — no unexpected ENUMERATION when no schema is attached. Schema.rs is not modified; the adapter only reads `FieldDef.allowed_values` via its public API. Fixes: REQ-025 --- rivet-core/src/reqif.rs | 299 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 287 insertions(+), 12 deletions(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index fe77442..c469052 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -27,6 +27,7 @@ use serde::{Deserialize, Serialize}; use crate::adapter::{Adapter, AdapterConfig, AdapterSource}; use crate::error::Error; use crate::model::{Artifact, Link, Provenance}; +use crate::schema::Schema; // ── ReqIF XML structures ──────────────────────────────────────────────── // @@ -487,14 +488,29 @@ const PROV_LONG_REVIEWED_BY: &str = "rivet:reviewed-by"; pub struct ReqIfAdapter { supported: Vec, + /// Optional schema used on export to emit `DATATYPE-DEFINITION-ENUMERATION` + /// for fields whose schema declares `allowed-values`. When `None`, the + /// exporter falls back to flat STRING attributes for all fields. + schema: Option, } impl ReqIfAdapter { pub fn new() -> Self { Self { supported: vec![], // accepts all types + schema: None, } } + + /// Attach a schema to drive enum-aware export. When artifacts carry + /// fields whose schema declares `allowed-values`, the exporter emits a + /// `DATATYPE-DEFINITION-ENUMERATION` and an `ATTRIBUTE-DEFINITION-ENUMERATION` + /// on the SpecObjectType, instead of a flat STRING attribute. Import is + /// unchanged — it already recognises ENUMERATION values. + pub fn with_schema(mut self, schema: Schema) -> Self { + self.schema = Some(schema); + self + } } impl Default for ReqIfAdapter { @@ -536,7 +552,7 @@ impl Adapter for ReqIfAdapter { } fn export(&self, artifacts: &[Artifact], _config: &AdapterConfig) -> Result, Error> { - let reqif = build_reqif(artifacts); + let reqif = build_reqif_with_schema(artifacts, self.schema.as_ref()); serialize_reqif(&reqif) } } @@ -885,8 +901,39 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result, + /// Allowed label strings (the schema's `allowed-values` array). + allowed: Vec, +} + /// Build a ReqIF document from Rivet artifacts. +/// +/// Shorthand for `build_reqif_with_schema(artifacts, None)` — emits flat +/// STRING attributes for every field, ignoring `allowed-values` constraints. pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { + build_reqif_with_schema(artifacts, None) +} + +/// Build a ReqIF document from Rivet artifacts, optionally consulting a +/// Schema to emit `DATATYPE-DEFINITION-ENUMERATION` constraints. +/// +/// When `schema` is `Some`, fields whose schema declares `allowed-values` +/// are emitted as `ATTRIBUTE-DEFINITION-ENUMERATION` on the SpecObjectType +/// and as `ATTRIBUTE-VALUE-ENUMERATION` on each SpecObject. Other fields +/// still use STRING attributes. When `schema` is `None`, all fields are +/// STRING (legacy behaviour). +pub fn build_reqif_with_schema(artifacts: &[Artifact], schema: Option<&Schema>) -> ReqIfRoot { // Collect unique artifact types and link types. let mut artifact_types: Vec = Vec::new(); let mut link_types: Vec = Vec::new(); @@ -912,14 +959,73 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } } - // Build DATATYPE-DEFINITION-STRING. + // Precompute schema-driven enum metadata per (artifact_type, field_name): + // enum_meta[(at, field)] = (datatype_id, attr_def_id, [enum_value_id, ...]) + // This is empty when `schema` is None or no enum fields apply. + let mut enum_meta: std::collections::BTreeMap<(String, String), EnumFieldMeta> = + std::collections::BTreeMap::new(); + if let Some(sch) = schema { + for at in &artifact_types { + let Some(atdef) = sch.artifact_types.get(at) else { + continue; + }; + for fdef in &atdef.fields { + let Some(allowed) = &fdef.allowed_values else { + continue; + }; + if allowed.is_empty() { + continue; + } + // Datatypes, attribute-defs, and enum-values need globally-unique + // identifiers. Namespacing with artifact-type keeps per-type + // allowed-values sets separate when the same field name appears + // on multiple types with different constraints. + let datatype_id = format!("DT-ENUM-{at}-{}", fdef.name); + let attr_def_id = format!("ATTR-ENUM-{at}-{}", fdef.name); + let value_ids: Vec = allowed + .iter() + .enumerate() + .map(|(i, _)| format!("{datatype_id}-V{i}")) + .collect(); + enum_meta.insert( + (at.clone(), fdef.name.clone()), + EnumFieldMeta { + datatype_id, + attr_def_id, + value_ids, + allowed: allowed.clone(), + }, + ); + } + } + } + + // Build DATATYPE-DEFINITION-STRING + optional ENUMERATION datatypes. + let mut enum_types: Vec = Vec::new(); + for meta in enum_meta.values() { + let values: Vec = meta + .allowed + .iter() + .zip(meta.value_ids.iter()) + .map(|(label, id)| EnumValue { + identifier: id.clone(), + long_name: Some(label.clone()), + }) + .collect(); + enum_types.push(DatatypeDefinitionEnumeration { + identifier: meta.datatype_id.clone(), + long_name: Some(format!("Enum-{}", meta.datatype_id)), + specified_values: Some(SpecifiedValues { values }), + }); + } + let datatypes = Datatypes { string_types: vec![DatatypeDefinitionString { identifier: DATATYPE_STRING_ID.into(), long_name: Some("String".into()), max_length: Some(65535), }], - enum_types: vec![], + enum_types, }; // Build SPEC-OBJECT-TYPEs — one per artifact type, each with standard @@ -973,14 +1079,24 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { }); } + let mut enum_attrs: Vec = Vec::new(); for fname in &field_names { - string_attrs.push(AttributeDefinitionString { - identifier: format!("ATTR-{fname}"), - long_name: Some(fname.clone()), - datatype_ref: Some(DatatypeRef { - datatype_ref: DATATYPE_STRING_ID.into(), - }), - }); + // Prefer ENUMERATION when the schema declares allowed-values + // for this (artifact-type, field) pair. + if let Some(meta) = enum_meta.get(&(at.clone(), fname.clone())) { + enum_attrs.push(AttributeDefinitionEnumeration { + identifier: meta.attr_def_id.clone(), + long_name: Some(fname.clone()), + }); + } else { + string_attrs.push(AttributeDefinitionString { + identifier: format!("ATTR-{fname}"), + long_name: Some(fname.clone()), + datatype_ref: Some(DatatypeRef { + datatype_ref: DATATYPE_STRING_ID.into(), + }), + }); + } } SpecObjectType { @@ -988,7 +1104,7 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { long_name: Some(at.clone()), spec_attributes: Some(SpecAttributes { string_attrs, - enum_attrs: vec![], + enum_attrs, }), } }) @@ -1057,7 +1173,30 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { } } + let mut enum_values: Vec = Vec::new(); for (key, value) in &a.fields { + if let Some(meta) = enum_meta.get(&(a.artifact_type.clone(), key.clone())) { + // Schema-driven ENUMERATION. The value must match one of + // the allowed labels; if it doesn't, fall back to a STRING + // attribute so the raw value isn't silently dropped. + let label = match value { + serde_yaml::Value::String(s) => s.clone(), + other => encode_field_value(other).unwrap_or_default(), + }; + if let Some(pos) = meta.allowed.iter().position(|a| a == &label) { + enum_values.push(AttributeValueEnumeration { + values: Some(EnumValueRefs { + refs: vec![meta.value_ids[pos].clone()], + }), + definition: EnumAttrDefinitionRef { + attr_def_ref: meta.attr_def_id.clone(), + }, + }); + continue; + } + // Out-of-enum value: emit as STRING with the original + // attribute name so downstream validate.rs can flag it. + } if let Some(val_str) = encode_field_value(value) { string_values.push(AttributeValueString { the_value: val_str, @@ -1077,7 +1216,7 @@ pub fn build_reqif(artifacts: &[Artifact]) -> ReqIfRoot { }), values: Some(Values { string_values, - enum_values: vec![], + enum_values, }), } }) @@ -1724,6 +1863,142 @@ mod tests { assert!(ct.contains('T')); } + /// When the schema declares `allowed-values` for a field, the exporter + /// must emit `DATATYPE-DEFINITION-ENUMERATION` plus + /// `ATTRIBUTE-DEFINITION-ENUMERATION` rather than a flat STRING. + /// Regression for the bug at `reqif.rs:871-874` flagged in + /// `docs/design/polarion-reqif-fidelity.md`: the exporter previously + /// never emitted any ENUMERATION, silently flattening closed-enum + /// schema constraints. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_schema_enum_field_emits_enumeration() { + use crate::schema::{ArtifactTypeDef, FieldDef, Schema}; + use std::collections::HashMap; + + let atdef = ArtifactTypeDef { + name: "hazard".into(), + description: "Safety hazard".into(), + fields: vec![FieldDef { + name: "severity".into(), + field_type: "string".into(), + required: false, + description: None, + allowed_values: Some(vec![ + "catastrophic".into(), + "critical".into(), + "marginal".into(), + "negligible".into(), + ]), + }], + link_fields: vec![], + aspice_process: None, + common_mistakes: vec![], + example: None, + yaml_section: None, + yaml_sections: vec![], + yaml_section_suffix: None, + shorthand_links: Default::default(), + }; + let mut at_map = HashMap::new(); + at_map.insert("hazard".to_string(), atdef); + let schema = Schema { + artifact_types: at_map, + link_types: HashMap::new(), + inverse_map: HashMap::new(), + traceability_rules: vec![], + conditional_rules: vec![], + }; + + let mut fields: BTreeMap = BTreeMap::new(); + fields.insert( + "severity".into(), + serde_yaml::Value::String("critical".into()), + ); + let art = Artifact { + id: "H-1".into(), + artifact_type: "hazard".into(), + title: "Runaway train".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields, + provenance: None, + source_file: None, + }; + + let adapter = ReqIfAdapter::new().with_schema(schema); + let bytes = adapter.export(&[art], &AdapterConfig::default()).unwrap(); + let xml = std::str::from_utf8(&bytes).unwrap(); + + // Must contain ENUMERATION elements — not just STRING. + assert!( + xml.contains("DATATYPE-DEFINITION-ENUMERATION"), + "no DATATYPE-DEFINITION-ENUMERATION in export: {xml}" + ); + assert!( + xml.contains("ATTRIBUTE-DEFINITION-ENUMERATION"), + "no ATTRIBUTE-DEFINITION-ENUMERATION in export: {xml}" + ); + assert!( + xml.contains("ATTRIBUTE-VALUE-ENUMERATION"), + "no ATTRIBUTE-VALUE-ENUMERATION in export: {xml}" + ); + // All allowed values must appear as ENUM-VALUE LONG-NAMEs. + for v in ["catastrophic", "critical", "marginal", "negligible"] { + assert!( + xml.contains(v), + "allowed value {v} missing from enum datatype" + ); + } + + // Import round-trip: the enum-valued field comes back as a string + // with the long-name of the referenced enum value. + let re = adapter + .import(&AdapterSource::Bytes(bytes), &AdapterConfig::default()) + .unwrap(); + assert_eq!(re.len(), 1); + assert_eq!( + re[0].fields.get("severity"), + Some(&serde_yaml::Value::String("critical".into())) + ); + } + + /// Without a schema, exports fall back to flat STRING attributes for + /// fields — backward compatibility with adapter callers that pre-date + /// `with_schema`. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_export_without_schema_stays_string() { + let mut fields: BTreeMap = BTreeMap::new(); + fields.insert( + "severity".into(), + serde_yaml::Value::String("critical".into()), + ); + let art = Artifact { + id: "H-1".into(), + artifact_type: "hazard".into(), + title: "Runaway train".into(), + description: None, + status: None, + tags: vec![], + links: vec![], + fields, + provenance: None, + source_file: None, + }; + let adapter = ReqIfAdapter::new(); + let bytes = adapter.export(&[art], &AdapterConfig::default()).unwrap(); + let xml = std::str::from_utf8(&bytes).unwrap(); + assert!( + !xml.contains("DATATYPE-DEFINITION-ENUMERATION"), + "unexpected ENUMERATION emitted without schema: {xml}" + ); + } + /// Legacy comma-joined tags (from older rivet exports or other tools) /// are still parsed correctly on import — backward-compat contract. // rivet: verifies REQ-025 From 131bee90a14fc2dbd70aeec5e5592e23dd0345e4 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 22 Apr 2026 06:57:26 +0200 Subject: [PATCH 6/6] fix(reqif): reject dangling SPEC-RELATION targets instead of phantom Links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the importer built `SpecRelation`s in a single pass: if the target SpecObject didn't exist in the file, the Link was still attached to the source artifact pointing at a missing ID. That phantom edge would later surface as a broken link in the LinkGraph, but the cause (a malformed ReqIF input) was silently lost. Two-pass import: 1. First pass (already present) collects every SpecObject ID into `artifact_ids`. 2. Relation pass now checks both source and target against that set; any mismatch is collected into `dangling` with the source/target/role triple. At the end, if `dangling` is non-empty the whole import is rejected with `Error::Adapter` listing every offending relation. This is more aggressive than `links.rs` (which keeps broken links as advisory data) because a ReqIF file is an atomic interchange unit — a dangling SPEC-OBJECT-REF means the file is malformed, not that the traceability store has a temporary gap. Adds two regression tests: - test_dangling_spec_relation_rejected — target points at a missing ID; error names the missing target and the link role. - test_dangling_source_rejected — source points at a missing ID. Fixes: REQ-025 --- rivet-core/src/reqif.rs | 124 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 123 insertions(+), 1 deletion(-) diff --git a/rivet-core/src/reqif.rs b/rivet-core/src/reqif.rs index c469052..d29d5d2 100644 --- a/rivet-core/src/reqif.rs +++ b/rivet-core/src/reqif.rs @@ -868,6 +868,14 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result = Vec::new(); for rel in &content.spec_relations.relations { let link_type = rel .relation_type_ref @@ -888,7 +896,26 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result {} (role={}): {}", + rel.source.spec_object_ref, + rel.target.spec_object_ref, + link_type, + match (source_idx.is_none(), !target_known) { + (true, true) => "source and target unknown", + (true, false) => "source unknown", + (false, true) => "target unknown", + (false, false) => unreachable!(), + } + )); + continue; + } + + if let Some(idx) = source_idx { artifacts[idx].links.push(Link { link_type, target: target_id.clone(), @@ -896,6 +923,14 @@ pub fn parse_reqif(xml: &str, type_map: &HashMap) -> Result + + + + + + + + + + + + + + SOT-req + + + + + SRT-traces-to + R-1 + DOES-NOT-EXIST + + + + +"#; + + let err = + parse_reqif(xml, &HashMap::new()).expect_err("dangling target should be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("dangling"), + "error should mention 'dangling': {msg}" + ); + assert!( + msg.contains("DOES-NOT-EXIST"), + "error should name the missing target: {msg}" + ); + assert!( + msg.contains("traces-to"), + "error should carry the link role: {msg}" + ); + } + + /// When the source of a SpecRelation doesn't exist, we also reject + /// the import rather than dropping the relation on the floor. + // rivet: verifies REQ-025 + #[test] + #[cfg_attr(miri, ignore)] + fn test_dangling_source_rejected() { + let xml = r#" + + + + + + + + + + + + + SOT-req + + + + + MISSING-SRC + R-1 + + + + +"#; + let err = parse_reqif(xml, &HashMap::new()).expect_err("dangling source rejected"); + assert!(err.to_string().contains("MISSING-SRC")); + } + /// Legacy comma-joined tags (from older rivet exports or other tools) /// are still parsed correctly on import — backward-compat contract. // rivet: verifies REQ-025