Skip to content

fix(reqif): 6 fidelity bugs — provenance, typed fields, tags, creation-time, enum, dangling relations#175

Merged
avrabe merged 6 commits intomainfrom
fix/v041-reqif-fidelity
Apr 22, 2026
Merged

fix(reqif): 6 fidelity bugs — provenance, typed fields, tags, creation-time, enum, dangling relations#175
avrabe merged 6 commits intomainfrom
fix/v041-reqif-fidelity

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented Apr 22, 2026

Summary

Six small, focused fixes for the ReqIF adapter, closing the fidelity gaps documented in #169 (docs/design/polarion-reqif-fidelity.md). One commit per bug. Each commit: a failing regression test pre-fix, green post-fix, clippy clean.

# Bug Commit
1 Provenance unconditionally dropped on export + import 24b8059
2 Non-string fields values coerced via Rust Debug format e69e5fc
3 Tags round-trip broken for commas / leading whitespace 6e0d551
4 CREATION-TIME hardcoded to None on every export d032865
5 Only STRING datatype emitted; allowed-values schema flattened 848e267
6 Dangling SpecRelation targets silently create phantom Links 5feaed7

Scorecard — before / after

Counting the affected rows in the §2 fidelity table of docs/design/polarion-reqif-fidelity.md:

Row Before (PR #169 baseline) After this PR
fields["key"] (bool/number) LOSSY (true"Bool(true)") LOSSLESS
fields["key"] (list / mapping) LOSSY (Rust Debug-form) LOSSLESS (JSON-encoded)
tags (commas / whitespace) LOSSY (silently split on re-import) LOSSLESS (JSON array)
provenance.created_by ABSENT LOSSLESS
provenance.model ABSENT LOSSLESS
provenance.session_id ABSENT LOSSLESS
provenance.timestamp ABSENT LOSSLESS
provenance.reviewed_by ABSENT LOSSLESS
Schema allowed-values LOSSY (flattened) LOSSLESS when schema is attached via with_schema
Header CREATION-TIME LOSSY (empty) LOSSLESS (ISO 8601 UTC)
Dangling SPEC-RELATION targets silent dangle hard error with source/target/role triple

Path 2 (ReqIF) ReqIF-native fields move from 7 LOSSLESS / 10 LOSSY / 8 ABSENT → approximately 17 LOSSLESS / 3 LOSSY / 3 ABSENT for the rows this PR touches. The remaining ABSENT rows (source_file, schema traceability-rules, commit trailers, etc.) are out of scope and documented in the design note.

Test count

  • 5 baseline ReqIF unit tests passing before this PR.
  • 11 new regression tests (in rivet-core/src/reqif.rs tests module), one or more per fix:
    • test_provenance_roundtrip, test_provenance_absent_stays_none
    • test_non_string_fields_roundtrip, test_null_field_dropped_on_export
    • test_tags_with_special_chars_roundtrip, test_tags_legacy_comma_form_parses
    • test_creation_time_is_stamped
    • test_schema_enum_field_emits_enumeration, test_export_without_schema_stays_string
    • test_dangling_spec_relation_rejected, test_dangling_source_rejected
  • Full cargo test -p rivet-core green (592 lib tests + 203 integration).

Implementation notes

  • No changes to the off-limits files (db.rs, feature_model.rs, yaml_hir.rs, coverage.rs, schema.rs, validate.rs, verus/, proofs/, CI). Schema is read via its public API only.
  • No new dependencies. serde_json (already in the workspace) covers JSON encoding for fields/tags. UTC timestamp uses the same SystemTime + civil-from-days pattern already in export.rs::timestamp_now — no chrono/jiff.
  • The Adapter trait signature is untouched. Schema-aware export is opt-in via ReqIfAdapter::with_schema(schema); ReqIfAdapter::new() still behaves as before (flat STRING attributes). build_reqif(artifacts) is kept as a thin wrapper around build_reqif_with_schema(artifacts, None) for API stability.
  • Bug 6 rejects the whole ReqIF document when any relation is dangling rather than mirroring links.rs's advisory-broken-link pattern, because a ReqIF file is an atomic interchange unit.

Test plan

  • cargo test -p rivet-core --lib reqif — all 16 tests green
  • cargo test -p rivet-core — full suite green
  • cargo clippy -p rivet-core --lib --tests — clippy clean
  • Scorecard rows above match the actual XML output (spot-check by exporting a provenance-stamped artifact and grepping rivet:created-by)

Refs: REQ-025, #169

🤖 Generated with Claude Code

avrabe added 6 commits April 22, 2026 00:27
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
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
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
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 `<CREATION-TIME>` in the 20-char ISO form.

Fixes: REQ-025
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
…Links

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
@avrabe avrabe force-pushed the fix/v041-reqif-fidelity branch from 5feaed7 to 131bee9 Compare April 22, 2026 05:27
@avrabe avrabe merged commit 89cdb75 into main Apr 22, 2026
1 check passed
@avrabe avrabe deleted the fix/v041-reqif-fidelity branch April 22, 2026 05:32
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 131bee9 Previous: a24c1b9 Ratio
store_lookup/100 2237 ns/iter (± 54) 1679 ns/iter (± 10) 1.33
store_lookup/1000 25812 ns/iter (± 468) 19264 ns/iter (± 61) 1.34
traceability_matrix/1000 59926 ns/iter (± 1887) 40368 ns/iter (± 145) 1.48
query/100 804 ns/iter (± 15) 647 ns/iter (± 2) 1.24
query/1000 7684 ns/iter (± 391) 5527 ns/iter (± 12) 1.39
document_parse/100 178804 ns/iter (± 1236) 148581 ns/iter (± 857) 1.20
document_parse/1000 1687001 ns/iter (± 35671) 1375967 ns/iter (± 9026) 1.23

This comment was automatically generated by workflow using github-action-benchmark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant