diff --git a/artifacts/requirements.yaml b/artifacts/requirements.yaml index 1d4f176a..f68b4424 100644 --- a/artifacts/requirements.yaml +++ b/artifacts/requirements.yaml @@ -8418,8 +8418,8 @@ artifacts: - id: REQ-320 type: requirement title: "No way to subset an embedded preset's rules, so an unmodelled process level renders as an empty row forever (#871)" - status: proposed - description: "Reported from scry. Part (1) of #871 — an empty-population rule rendering as 100.0% — was ALREADY FIXED in v0.34.0 by REQ-294 and verified on current main: both the per-rule table and the V-closure lines render `n/a%`, and `coverage --format json` emits `percentage: null` alongside `empty_scope: true`, which is exactly the shape the issue asks for. The reporter is on scry's vendored copy, which predates that release. Part (2) is real and unaddressed: the `aspice` preset is embedded and there is no way to subset its rules, so a project adopting it for SWE.1/SWE.6 necessarily inherits SWE.2/SWE.3/SWE.4 rows it can never satisfy. scry deliberately does not model those levels — its design intent lives in a parallel dev spine, and authoring a unit-verification artifact per absent detail-design element would be fabricated traceability, which is the outcome a coverage gate exists to prevent. Their workaround is the tell: a checked-in file declaring the unmodelled levels, gated in CI to fail both when an empty population is undeclared and when a declaration goes stale. That is a mechanism rivet should own, and it is the same shape as REQ-309 (`exempt-when-field` gave a DECLARED exemption a machine-readable home so it stops reading as an oversight) and REQ-313 (a declared-undischargeable criterion). Shape to consider: let a project declare in `rivet.yaml` which preset rules it does not model, with the declaration itself validated — an unmodelled level that later acquires artifacts should fail, so the declaration cannot go stale silently. Deliberately grouped with those two rather than solved standalone: three near-identical mechanisms for `this is declared, not forgotten` would be worse than one." + status: verified + description: "Reported from scry. Part (1) of #871 — an empty-population rule rendering as 100.0% — was ALREADY FIXED in v0.34.0 by REQ-294 and verified on current main: both the per-rule table and the V-closure lines render `n/a%`, and `coverage --format json` emits `percentage: null` alongside `empty_scope: true`, which is exactly the shape the issue asks for. The reporter is on scry's vendored copy, which predates that release. Part (2) is real and unaddressed: the `aspice` preset is embedded and there is no way to subset its rules, so a project adopting it for SWE.1/SWE.6 necessarily inherits SWE.2/SWE.3/SWE.4 rows it can never satisfy. scry deliberately does not model those levels — its design intent lives in a parallel dev spine, and authoring a unit-verification artifact per absent detail-design element would be fabricated traceability, which is the outcome a coverage gate exists to prevent. Their workaround is the tell: a checked-in file declaring the unmodelled levels, gated in CI to fail both when an empty population is undeclared and when a declaration goes stale. That is a mechanism rivet should own, and it is the same shape as REQ-309 (`exempt-when-field` gave a DECLARED exemption a machine-readable home so it stops reading as an oversight) and REQ-313 (a declared-undischargeable criterion). Shape to consider: let a project declare in `rivet.yaml` which preset rules it does not model, with the declaration itself validated — an unmodelled level that later acquires artifacts should fail, so the declaration cannot go stale silently. Deliberately grouped with those two rather than solved standalone: three near-identical mechanisms for `this is declared, not forgotten` would be worse than one. Shipped as coverage.unmodelled-rules in rivet.yaml: a list of rule plus reason, where the reason is REQUIRED because a declaration without one is indistinguishable from suppressing an inconvenient row. The declared rule STAYS in the report, annotated — hiding it is the failure this replaces, not the fix. Two ways a declaration can fail, and both are errors that fail the run: it is STALE when the rule now has source artifacts, so the project said it does not model something and then modelled it; and it is UNKNOWN when no such rule exists in the active schemas, because silently ignoring a typo would let a project believe it had declared something. The staleness half is the point, and is what was promised to the reporter: an exemption that outlives its reason is the same defect as the 100 percent it replaced, a number that stopped meaning what it says. Implemented additively as mark_unmodelled rather than as a parameter on compute_coverage, whose signature is public. Grouped with REQ-309 (exempt-when-field, artifact declares itself out of a rule population) and REQ-313 (declared-blocked criteria): three levels of the same idea, and this is the project-scope one. Negative-controlled at --lib scope: never detecting stale, ignoring an unknown rule, and not annotating each redden exactly one test." release: v0.36.0 provenance: created-by: ai-assisted diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index 8521842b..aef0ed12 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -8882,7 +8882,17 @@ fn cmd_coverage( graph = LinkGraph::build(&store, &schema); } - let report = coverage::compute_coverage(&store, &schema, &graph); + let mut report = coverage::compute_coverage(&store, &schema, &graph); + + // REQ-320 (#871 part 2): apply the project's unmodelled-rule declarations. + // The rules STAY in the report — hiding them is the failure this replaces. + // A declaration that no longer holds is an error: an exemption that + // outlives its reason is the same defect as the 100% it replaced. + let unmodelled_problems = match ctx.config.coverage.as_ref() { + Some(cov_cfg) => coverage::mark_unmodelled(&mut report, &cov_cfg.unmodelled_rules), + None => Vec::new(), + }; + let report = report; // #808: distinguish "no artifacts to score" (n/a) from "100% // coverage." A rule with total=0 emits `null` for both percentages; @@ -8916,6 +8926,10 @@ fn cmd_coverage( "percentage": pct_or_null(empty, e.percentage()), "accounted_percentage": pct_or_null(empty, e.accounted_percentage()), "uncovered_ids": e.uncovered_ids, + // REQ-320: present only when the project declared it, so a + // reader can tell an unmodelled level from an empty + // population nobody claimed. + "unmodelled": e.unmodelled, }) }) .collect(); @@ -8972,6 +8986,11 @@ fn cmd_coverage( "passed": passed, }); } + // REQ-320: a declaration that no longer holds is reported, named, and + // fails the run. + if !unmodelled_problems.is_empty() { + output["unmodelled_problems"] = serde_json::json!(unmodelled_problems); + } println!("{}", serde_json::to_string_pretty(&output).unwrap()); } else { let any_boundary = report.entries.iter().any(|e| e.external_boundary > 0); @@ -9148,6 +9167,19 @@ fn cmd_coverage( return Ok(false); } + // REQ-320: a declaration in `coverage.unmodelled-rules` that no longer + // holds fails the run. Reported before --fail-under so the specific + // diagnostic is not masked by a generic threshold message, and always — + // not only under a flag — because a stale exemption silently shrinks the + // denominator every later number is computed against. + if !unmodelled_problems.is_empty() { + eprintln!("\nerror: unmodelled-rule declaration(s) no longer hold:"); + for p in &unmodelled_problems { + eprintln!(" {}: {}", p.rule, p.message); + } + return Ok(false); + } + if let Some(&threshold) = fail_under { // Empty scope trivially "passes" the threshold check with legacy // math (overall = 100.0) but the user set a threshold to gate on diff --git a/rivet-cli/src/serve/variant.rs b/rivet-cli/src/serve/variant.rs index 61d7031e..9e8b9c37 100644 --- a/rivet-cli/src/serve/variant.rs +++ b/rivet-cli/src/serve/variant.rs @@ -393,6 +393,7 @@ mod tests { fn empty_cfg() -> ProjectConfig { ProjectConfig { + coverage: None, project: ProjectMetadata { name: "t".into(), version: None, diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index 5cb95a35..2caac673 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -10509,3 +10509,95 @@ fn check_verification_evidence_rejects_hollow_and_non_test_names() { "must exit non-zero when a step's evidence is hollow" ); } + +/// REQ-320 (#871 part 2): a project may declare a preset rule as one it does +/// not model, and that declaration must FAIL when it stops being true. +/// +/// Adopting `aspice` for SWE.1/SWE.6 drags in SWE.2/3/4 rows a project may +/// never intend to satisfy. Before this there was nowhere to say so, and an +/// unmodelled level rendered as an empty row forever — indistinguishable from +/// one someone forgot. +/// +/// The staleness half is the point. An exemption that outlives its reason is +/// the same defect as the 100% it replaced: a number that stopped meaning what +/// it says. So a declared-unmodelled rule that acquires a population is an +/// ERROR, not a quiet re-inclusion. +/// +/// rivet: verifies REQ-320 +#[test] +fn coverage_declares_unmodelled_rules_and_fails_when_stale() { + let write_project = |dir: &std::path::Path, declared: &str| { + std::fs::create_dir_all(dir.join("artifacts")).unwrap(); + std::fs::write( + dir.join("rivet.yaml"), + format!( + "project:\n name: p\n schemas: [common, dev]\n\ + sources:\n - path: artifacts\n format: generic-yaml\n\ + coverage:\n unmodelled-rules:\n - rule: {declared}\n \ + reason: modelled in a parallel spine, by design\n" + ), + ) + .unwrap(); + std::fs::write( + dir.join("artifacts/a.yaml"), + "artifacts:\n \ + - id: REQ-001\n type: requirement\n title: t\n status: draft\n", + ) + .unwrap(); + }; + let run = |dir: &std::path::Path| { + let out = Command::new(rivet_bin()) + .args([ + "--project", + dir.to_str().unwrap(), + "coverage", + "--format", + "json", + ]) + .output() + .expect("coverage"); + let text = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + (out.status.success(), text) + }; + + // ── Happy path: `decision-justification` has an empty population in this + // fixture (no design-decision artifacts), which is exactly the case the + // requirement is about — an unmodelled level, not a forgotten one. + let tmp_ok = tempfile::tempdir().expect("temp dir"); + write_project(tmp_ok.path(), "decision-justification"); + let (ok, text) = run(tmp_ok.path()); + assert!( + text.contains("decision-justification"), + "a declared rule must still be NAMED, not dropped — a hidden rule is \ + the failure this replaces; got:\n{text}" + ); + assert!( + text.contains("unmodelled"), + "the rule must be marked as declared-unmodelled so a reader can tell it \ + from an empty population nobody claimed; got:\n{text}" + ); + assert!( + ok, + "a truthful declaration must not fail the run; got:\n{text}" + ); + + // ── Stale: `requirement-verification` HAS a population here (REQ-001), so + // declaring it unmodelled is a claim contradicted by the artifacts. + let tmp_stale = tempfile::tempdir().expect("temp dir"); + write_project(tmp_stale.path(), "requirement-verification"); + let (stale_ok, stale_text) = run(tmp_stale.path()); + assert!( + stale_text.contains("stale") || stale_text.contains("no longer"), + "a declaration contradicted by a real population must say so; \ + got:\n{stale_text}" + ); + assert!( + !stale_ok, + "a stale unmodelled declaration must FAIL — an exemption that outlives \ + its reason is the defect it replaced; got:\n{stale_text}" + ); +} diff --git a/rivet-core/src/coverage.rs b/rivet-core/src/coverage.rs index 66360697..b40f8683 100644 --- a/rivet-core/src/coverage.rs +++ b/rivet-core/src/coverage.rs @@ -77,6 +77,11 @@ pub struct CoverageEntry { /// declaration. #[serde(default)] pub exempt: usize, + /// Set when the project declares it does not model this rule (REQ-320). + /// Carries the declared reason. The entry stays in the report — a hidden + /// rule is the failure this replaces, not the fix. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unmodelled: Option, /// Ids of the exempt sources, so a report can name them. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub exempt_ids: Vec, @@ -395,6 +400,7 @@ pub fn compute_coverage(store: &Store, schema: &Schema, graph: &LinkGraph) -> Co entries.push(CoverageEntry { exempt, exempt_ids, + unmodelled: None, rule_name: rule.name.clone(), description: rule.description.clone(), source_type: rule.source_type.clone(), @@ -465,9 +471,145 @@ fn terminates_at_external_anchor( // ── Tests ──────────────────────────────────────────────────────────────── +/// A declaration in `coverage.unmodelled-rules` that does not hold. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct UnmodelledProblem { + /// The declared rule name. + pub rule: String, + /// What is wrong with the declaration. + pub kind: UnmodelledProblemKind, + /// Reader-facing explanation. + pub message: String, +} + +/// Why a declaration fails. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum UnmodelledProblemKind { + /// The rule now has source artifacts, so the project DOES model it. + Stale, + /// No such rule in the active schemas — a typo declares nothing. + UnknownRule, +} + +/// Annotate `report` with the project's unmodelled-rule declarations and +/// return every declaration that does not hold (REQ-320). +/// +/// Additive rather than a parameter on [`compute_coverage`], whose signature is +/// public. +/// +/// Two failure kinds, and both matter for the same reason. A STALE declaration +/// is one the artifacts now contradict: the project said it does not model this +/// and then modelled it. An exemption that outlives its reason is the same +/// defect as the 100% it replaced — a number that stopped meaning what it says. +/// An UNKNOWN rule is a declaration that never applied to anything; silently +/// ignoring a typo would let a project believe it had declared something. +pub fn mark_unmodelled( + report: &mut CoverageReport, + declared: &[crate::model::UnmodelledRule], +) -> Vec { + let mut problems = Vec::new(); + for d in declared { + match report.entries.iter_mut().find(|e| e.rule_name == d.rule) { + None => problems.push(UnmodelledProblem { + rule: d.rule.clone(), + kind: UnmodelledProblemKind::UnknownRule, + message: format!( + "declared unmodelled but no rule named '{}' exists in the active schemas", + d.rule + ), + }), + Some(entry) => { + if entry.total > 0 { + problems.push(UnmodelledProblem { + rule: d.rule.clone(), + kind: UnmodelledProblemKind::Stale, + message: format!( + "declared unmodelled ({}) but {} source artifact(s) now match it — \ + the declaration is stale and no longer describes this project", + d.reason, entry.total + ), + }); + } + entry.unmodelled = Some(d.reason.clone()); + } + } + } + problems +} + #[cfg(test)] mod tests { use super::*; + + fn entry(name: &str, total: usize) -> CoverageEntry { + CoverageEntry { + rule_name: name.into(), + description: String::new(), + source_type: "requirement".into(), + link_type: "verifies".into(), + direction: CoverageDirection::Backward, + target_types: vec![], + covered: 0, + exempt: 0, + exempt_ids: vec![], + unmodelled: None, + external_boundary: 0, + external_boundary_ids: vec![], + total, + uncovered_ids: vec![], + } + } + + fn decl(rule: &str) -> crate::model::UnmodelledRule { + crate::model::UnmodelledRule { + rule: rule.into(), + reason: "modelled in a parallel spine".into(), + } + } + + /// A truthful declaration annotates the rule and keeps it in the report. + /// + /// rivet: verifies REQ-320 + #[test] + fn unmodelled_declaration_annotates_without_hiding() { + let mut r = CoverageReport { + entries: vec![entry("unmodelled-one", 0), entry("other", 3)], + }; + let problems = mark_unmodelled(&mut r, &[decl("unmodelled-one")]); + assert!(problems.is_empty(), "got {problems:?}"); + assert_eq!(r.entries.len(), 2, "the rule must NOT be dropped"); + assert!(r.entries[0].unmodelled.is_some()); + assert!(r.entries[1].unmodelled.is_none(), "only the declared rule"); + } + + /// A declaration the artifacts contradict is stale and must be reported. + /// + /// rivet: verifies REQ-320 + #[test] + fn unmodelled_declaration_goes_stale_when_population_appears() { + let mut r = CoverageReport { + entries: vec![entry("now-modelled", 4)], + }; + let problems = mark_unmodelled(&mut r, &[decl("now-modelled")]); + assert_eq!(problems.len(), 1, "got {problems:?}"); + assert_eq!(problems[0].kind, UnmodelledProblemKind::Stale); + assert!(problems[0].message.contains('4'), "name the count"); + } + + /// A typo declares nothing; silently ignoring it would let a project + /// believe it had declared something. + /// + /// rivet: verifies REQ-320 + #[test] + fn unmodelled_declaration_naming_no_rule_is_reported() { + let mut r = CoverageReport { + entries: vec![entry("real-rule", 0)], + }; + let problems = mark_unmodelled(&mut r, &[decl("typo-rule")]); + assert_eq!(problems.len(), 1, "got {problems:?}"); + assert_eq!(problems[0].kind, UnmodelledProblemKind::UnknownRule); + } use crate::schema::{Severity, TraceabilityRule}; use crate::test_helpers::{artifact_with_links, minimal_artifact, minimal_schema}; @@ -981,6 +1123,7 @@ mod tests { CoverageEntry { exempt: 0, exempt_ids: Vec::new(), + unmodelled: None, rule_name: rule.into(), description: String::new(), source_type: source.into(), diff --git a/rivet-core/src/lib.rs b/rivet-core/src/lib.rs index 730bb059..b224b748 100644 --- a/rivet-core/src/lib.rs +++ b/rivet-core/src/lib.rs @@ -1053,6 +1053,7 @@ mod tests { commits: None, release: None, externals: None, + coverage: None, baselines: None, docs_check: None, } diff --git a/rivet-core/src/model.rs b/rivet-core/src/model.rs index 4bacacb5..6cd04e48 100644 --- a/rivet-core/src/model.rs +++ b/rivet-core/src/model.rs @@ -1127,6 +1127,28 @@ pub struct ReleaseConfig { pub require: Option, } +/// Project-level coverage configuration (REQ-320). +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct CoverageConfig { + /// Rules the project declares it does not model. + #[serde(default, rename = "unmodelled-rules")] + pub unmodelled_rules: Vec, +} + +/// A rule a project declares out of scope, with the reason it is out of scope. +/// +/// The reason is REQUIRED. A declaration without one is indistinguishable from +/// suppressing an inconvenient row, which is the thing this must not become. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct UnmodelledRule { + /// Rule name, as it appears in `rivet coverage`. + pub rule: String, + /// Why this project does not model it. + pub reason: String, +} + /// Project configuration loaded from `rivet.yaml`. /// /// #808 uses a soft-warn (not `deny_unknown_fields`) for unknown @@ -1159,6 +1181,14 @@ pub struct ProjectConfig { /// External project dependencies for cross-repo linking. #[serde(default)] pub externals: Option>, + /// Rules this project deliberately does not model (REQ-320, #871). + /// + /// Adopting a preset for part of its scope drags in the rest of its rules, + /// and an unmodelled level then renders as an empty row forever — + /// indistinguishable from one nobody got round to. This is where a project + /// says which rows are absent BY INTENT. + #[serde(default)] + pub coverage: Option, /// Named baselines for scoped validation and coverage. /// Order matters: earlier baselines are cumulatively included in later ones. #[serde(default)] diff --git a/rivet-core/src/proofs.rs b/rivet-core/src/proofs.rs index 40c588e5..2f39c948 100644 --- a/rivet-core/src/proofs.rs +++ b/rivet-core/src/proofs.rs @@ -223,6 +223,7 @@ mod proofs { covered, exempt: 0, exempt_ids: vec![], + unmodelled: None, total, uncovered_ids: vec![], external_boundary: 0,