diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs new file mode 100644 index 000000000..8eaed4e33 --- /dev/null +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs @@ -0,0 +1,64 @@ +//! Every current receipt against the parser, whether or not a catalogued case cites it (decision D141). + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::check::_helpers::receipts::snowflake_syntax; +use crate::check::models::{CaseResult, LocalVerdict, SnowflakeSyntax}; +use crate::formats::models::{Receipt, SyntaxCoverage}; + +/// Whether the parser's verdict contradicts what Snowflake's parser did with the same statement. +fn contradicts(snowflake: SnowflakeSyntax, local: &LocalVerdict) -> bool { + match snowflake { + SnowflakeSyntax::Parsed => !matches!(local, LocalVerdict::Accepted { .. }), + SnowflakeSyntax::Refused => matches!(local, LocalVerdict::Accepted { .. }), + } +} + +/// The receipt case ids the parser contradicts by SQL, setup and session, and those without SQL. +pub(crate) fn receipt_contradictions( + coverage: &SyntaxCoverage, + cases: &BTreeMap<&str, CaseResult>, + parse: &dyn Fn(&str) -> LocalVerdict, +) -> (BTreeSet, BTreeSet) { + let mut statements: BTreeMap<(&str, &str, &str), Vec<&Receipt>> = BTreeMap::new(); + for receipt in &coverage.receipts { + statements + .entry(( + receipt.sql_sha256.as_str(), + receipt.setup_sha256.as_str(), + receipt.session_sha256.as_str(), + )) + .or_default() + .push(receipt); + } + let mut judged: BTreeMap<&str, LocalVerdict> = cases + .values() + .map(|case| (case.sql_sha256.as_str(), case.local.clone())) + .collect(); + let case_sql: BTreeMap<&str, &str> = coverage + .cases + .values() + .flatten() + .map(|case| (case.sql_sha256.as_str(), case.sql.as_str())) + .collect(); + let mut contradicted: BTreeSet = BTreeSet::new(); + let mut unjudged: BTreeSet = BTreeSet::new(); + for ((sql_sha256, _, _), receipts) in &statements { + let Some(snowflake) = snowflake_syntax(receipts) else { + continue; + }; + let sql: Option<&str> = case_sql + .get(sql_sha256) + .copied() + .or_else(|| coverage.receipt_sql.get(*sql_sha256).map(String::as_str)); + let Some(sql) = sql else { + unjudged.extend(receipts.iter().map(|receipt| receipt.case_id.clone())); + continue; + }; + let local: &LocalVerdict = judged.entry(sql_sha256).or_insert_with(|| parse(sql)); + if contradicts(snowflake, local) { + contradicted.extend(receipts.iter().map(|receipt| receipt.case_id.clone())); + } + } + (contradicted, unjudged) +} diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs index d774bbdd0..e800470e9 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod contradictions; pub(crate) mod fingerprints; pub(crate) mod forms; pub(crate) mod join; diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs index 165c40a34..1ece2b1cf 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs @@ -63,7 +63,7 @@ fn agreement(phase: Phase, local: &LocalVerdict, terminates: bool) -> Result Option { +pub(crate) fn snowflake_syntax(statement_receipts: &[&Receipt]) -> Option { let verdicts: BTreeSet> = statement_receipts .iter() .filter(|receipt| receipt.measure.is_none()) diff --git a/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs b/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs index c0779d0e1..05fee1098 100644 --- a/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs @@ -2,6 +2,7 @@ use std::collections::BTreeSet; +use crate::check::_helpers::contradictions::receipt_contradictions; use crate::check::_helpers::forms::form_result; use crate::check::_helpers::join::join; use crate::check::_helpers::references::reference_problems; @@ -31,10 +32,14 @@ pub fn check_coverage( forms.push(form_result(form, family, &context)); } } + let (contradicted, unjudged): (BTreeSet, BTreeSet) = + receipt_contradictions(coverage, &joined.cases, parse); CheckResult { parser_tree: parser_tree.map(str::to_owned), forms, unexcluded_pages: unexcluded_pages(&coverage.sources, &excluded_sources), problems: reference_problems(coverage, &joined.forms), + receipt_contradictions: contradicted, + unjudged_receipts: unjudged, } } diff --git a/crates/sqlquack-syntax-coverage/src/check/models.rs b/crates/sqlquack-syntax-coverage/src/check/models.rs index 7c6eb3b2e..9f944763e 100644 --- a/crates/sqlquack-syntax-coverage/src/check/models.rs +++ b/crates/sqlquack-syntax-coverage/src/check/models.rs @@ -162,6 +162,10 @@ pub struct CheckResult { pub unexcluded_pages: Vec, /// References between files that do not resolve; any one fails the check. pub problems: Vec, + /// Receipt case ids whose statement the parser contradicts, cited by a form or not (D141). + pub receipt_contradictions: BTreeSet, + /// Receipt case ids whose SQL no case or run report gives, so no verdict can be compared. + pub unjudged_receipts: BTreeSet, } /// The files joined for checking, with every case already judged. diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs index 2c6bff14e..c841cb1b1 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs @@ -433,3 +433,17 @@ pub(crate) fn move_form(root: &Path, id: &str) { &format!("{declared}anchor = \"Syntax, edited\""), ); } + +/// Adds a receipt recording `fields` for `retired-1`, a case id no form cites, of `sql`. +pub(crate) fn retired_receipt(root: &Path, sql: &str, fields: Value) { + let case: String = case_line("retired-1", "sf.query.select.o001", sql, &[]); + append_receipt(root, &with_fields(&receipt_line(&case, "accepted"), fields)); +} + +/// Writes the run report beside the receipts, listing `sql` among its inputs. +pub(crate) fn report_input(root: &Path, sql: &str) { + let report: Value = serde_json::json!({ + "inputs": [{"sql": sql, "sql_sha256": sha256(sql)}], + }); + write(root, "receipts/run-1.report.json", &report.to_string()); +} diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs index 7d835c496..b75a3b1db 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs @@ -4,10 +4,12 @@ use crate::check::tests::helpers::{ OTHER_TREE, SETUP, TREE, append_receipt, case_in_zone, case_line, crashing, crashing_on_select_from, drop_lines, exclusion_review, form, hanging, move_form, not_submitted, note_obligation, obligation, opaque, pin_definitions, pin_fingerprints, read, receipt_line, - rejecting, replace, reshaped, rewrite_select_17, select_17, select_17_setup_sha256, sha256, - structural, unobservable, verified_tree, with_fields, write, + rejecting, replace, report_input, reshaped, retired_receipt, rewrite_select_17, select_17, + select_17_setup_sha256, sha256, structural, unobservable, verified_tree, with_fields, write, +}; +use crate::check::tests::test_types::{ + AccountStateTestCase, CheckTestCase, Parameters, ReceiptCheckTestCase, }; -use crate::check::tests::test_types::{AccountStateTestCase, CheckTestCase, Parameters}; use crate::formats::main::load_coverage::load_coverage; #[test] @@ -1169,3 +1171,73 @@ fn given_a_receipt_with_account_state_when_checking_then_only_its_case_carries_i ); } } + +#[test] +fn given_receipts_no_form_cites_when_checking_then_each_is_judged_against_the_parser() { + let test_cases = [ + ReceiptCheckTestCase { + description: "a retired case Snowflake parsed, which the parser now rejects", + mutate: |root| { + retired_receipt(root, "SELECT 2", serde_json::json!({})); + report_input(root, "SELECT 2"); + }, + parse: rejecting, + expected_contradictions: vec!["retired-1"], + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired case Snowflake parsed, which the parser accepts", + mutate: |root| { + retired_receipt(root, "SELECT 2", serde_json::json!({})); + report_input(root, "SELECT 2"); + }, + parse: structural, + expected_contradictions: Vec::new(), + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired syntax error with errno 1003, which the parser accepts", + mutate: |root| { + retired_receipt( + root, + "SELECT 2", + serde_json::json!({"phase": "syntax_error", "outcome": "error", "errno": 1003}), + ); + report_input(root, "SELECT 2"); + }, + parse: structural, + expected_contradictions: vec!["retired-1"], + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired case whose SQL no case or report gives", + mutate: |root| retired_receipt(root, "SELECT 2", serde_json::json!({})), + parse: rejecting, + expected_contradictions: Vec::new(), + expected_unjudged: vec!["retired-1"], + }, + ]; + for test_case in test_cases { + let directory = tempfile::tempdir().expect("tempdir"); + verified_tree(directory.path()); + (test_case.mutate)(directory.path()); + let coverage = load_coverage(directory.path()).expect("loads"); + let check = check_coverage(&coverage, Some(TREE), &test_case.parse); + let contradicted: Vec<&str> = check + .receipt_contradictions + .iter() + .map(String::as_str) + .filter(|id| *id == "retired-1") + .collect(); + let unjudged: Vec<&str> = check.unjudged_receipts.iter().map(String::as_str).collect(); + assert_eq!( + (contradicted, unjudged), + ( + test_case.expected_contradictions, + test_case.expected_unjudged + ), + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs b/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs index 012001a06..18f28edcf 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs @@ -29,3 +29,12 @@ pub(crate) struct AccountStateTestCase { /// Each case id with its parameter names and values, in case-id order. pub(crate) expected_cases: Vec<(&'static str, Parameters<'static>)>, } + +/// Receipts on a case id no form cites, and the receipt verdicts the checker must then give (D141). +pub(crate) struct ReceiptCheckTestCase { + pub(crate) description: &'static str, + pub(crate) mutate: fn(&Path), + pub(crate) parse: fn(&str) -> LocalVerdict, + pub(crate) expected_contradictions: Vec<&'static str>, + pub(crate) expected_unjudged: Vec<&'static str>, +} diff --git a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs index 408efc4b5..20d09709c 100644 --- a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs +++ b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs @@ -339,8 +339,16 @@ fn read_snapshot(path: &Path) -> Result { /// Lines naming each non-empty list of the diff. fn diff_lines(diff: &SnapshotDiff) -> Vec { - let lists: [(&str, &Vec); 10] = [ + let lists: [(&str, &Vec); 12] = [ ("REGRESSION forms lost", &diff.forms_lost), + ( + "REGRESSION receipts the parser now contradicts, cited by a form or not (D141)", + &diff.receipts_contradicted, + ), + ( + "DOCUMENTED (D113) receipts the parser now contradicts, for the auditor", + &diff.documented_receipts_contradicted, + ), ("REGRESSION obligations lost", &diff.obligations_lost), ( "REGRESSION Snowflake-parsed cases the parser now rejects", @@ -360,11 +368,28 @@ fn diff_lines(diff: &SnapshotDiff) -> Vec { ("forms gained", &diff.forms_gained), ("obligations gained", &diff.obligations_gained), ]; - lists + let mut lines: Vec = lists .iter() .filter(|(_, ids)| !ids.is_empty()) .map(|(label, ids)| format!("{label} ({}): {}", ids.len(), ids.join(" "))) - .collect() + .collect(); + let counted: [(&str, usize); 2] = [ + ( + "receipts the parser already contradicted, not failing", + diff.receipts_still_contradicted.len(), + ), + ( + "receipts without SQL to judge, not failing", + diff.unjudged_receipts.len(), + ), + ]; + lines.extend( + counted + .iter() + .filter(|(_, count)| *count > 0) + .map(|(label, count)| format!("{label}: {count} (listed in --json)")), + ); + lines } /// BASE against HEAD, or against the current tree; fails on a regression or catalogue change. @@ -389,6 +414,7 @@ pub(crate) fn diff( let failing: bool = [ &diff.forms_lost, &diff.obligations_lost, + &diff.receipts_contradicted, &diff.parsed_cases_refused, &diff.syntax_error_cases_accepted, &diff.catalogue_files_changed, diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs index 6ba6c7c3c..5734c2425 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs @@ -6,13 +6,16 @@ use std::path::{Path, PathBuf}; use serde_json::Value; use crate::formats::_helpers::files::read_text; -use crate::formats::_helpers::hashes::is_sha256; -use crate::formats::constants::{REPORT_RECEIPT_SHA256, REPORT_WITHDRAWN}; +use crate::formats::_helpers::hashes::{is_sha256, sha256_hex}; +use crate::formats::constants::{REPORT_INPUTS, REPORT_RECEIPT_SHA256, REPORT_WITHDRAWN}; use crate::formats::models::Receipt; -/// The receipt line hashes the run reports withdraw, and every problem reading them. -pub(crate) fn withdrawn_receipts(reports: &[PathBuf]) -> (BTreeSet, Vec) { +/// The receipt line hashes the run reports withdraw, the SQL of their inputs by hash, and every problem. +pub(crate) fn read_reports( + reports: &[PathBuf], +) -> (BTreeSet, BTreeMap, Vec) { let mut withdrawn: BTreeSet = BTreeSet::new(); + let mut statements: BTreeMap = BTreeMap::new(); let mut problems: Vec = Vec::new(); for path in reports { let report: Value = match read_text(path).and_then(|(text, _)| { @@ -38,8 +41,21 @@ pub(crate) fn withdrawn_receipts(reports: &[PathBuf]) -> (BTreeSet, Vec< None => problems.push(withdrawn_problem(path)), } } + let inputs: &[Value] = report[REPORT_INPUTS].as_array().map_or(&[], Vec::as_slice); + for input in inputs { + match (input["sql"].as_str(), input["sql_sha256"].as_str()) { + (Some(sql), Some(hash)) if sha256_hex(sql.as_bytes()) == hash => { + statements.insert(hash.to_owned(), sql.to_owned()); + } + (Some(_), Some(hash)) => problems.push(format!( + "{}: input {hash} is not the SHA-256 of its sql", + path.display() + )), + _ => {} + } + } } - (withdrawn, problems) + (withdrawn, statements, problems) } fn withdrawn_problem(path: &Path) -> String { diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs index 6a622233f..17c1d2a27 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs @@ -11,7 +11,7 @@ use crate::formats::_helpers::records::{ check_obligation, check_reading, check_receipt, check_restatements, check_review, check_source, check_unobservable, }; -use crate::formats::_helpers::retired::{retire, withdrawn_receipts}; +use crate::formats::_helpers::retired::{read_reports, retire}; use crate::formats::constants::{ ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, LIMITS_FILE, MARKS, MARKS_DIR, OBLIGATIONS_DIR, RECEIPT_REPORT_SUFFIX, RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, @@ -22,7 +22,7 @@ use crate::formats::models::{ Obligation, ObligationsFile, Reading, Receipt, Review, ReviewsFile, Source, SourcesFile, Unobservable, UnobservableFile, }; -use crate::formats::types::FamilyCatalogue; +use crate::formats::types::{FamilyCatalogue, LoadedReceipts}; /// What one section of the tree yields. pub(crate) struct Section { @@ -206,8 +206,8 @@ pub(crate) fn load_marks(root: &Path) -> Section> { Section::of(records, declared, problems) } -/// Every receipts file, less the receipts superseded or withdrawn by the run reports beside them. -pub(crate) fn load_receipts(root: &Path) -> Section<(Vec, Vec)> { +/// Every receipts file, less those superseded or withdrawn, with the SQL its run reports give by hash. +pub(crate) fn load_receipts(root: &Path) -> Section { let (paths, others) = listed(&root.join(RECEIPTS_DIR), "jsonl"); let mut problems: Vec = others .iter() @@ -226,11 +226,11 @@ pub(crate) fn load_receipts(root: &Path) -> Section<(Vec, Vec)> ); published.extend(receipts); } - let (withdrawn, report_problems) = withdrawn_receipts(&reports); + let (withdrawn, statements, report_problems) = read_reports(&reports); problems.extend(report_problems); let (kept, retired, retire_problems) = retire(published, &withdrawn); problems.extend(retire_problems); - Section::of((kept, retired), Vec::new(), problems) + Section::of((kept, retired, statements), Vec::new(), problems) } /// `unobservable/.toml`: optional, and each obligation listed at most once. @@ -316,7 +316,7 @@ pub(crate) struct Sections { pub(crate) catalogue: Section, pub(crate) obligations: Section>>, pub(crate) cases: Section>>, - pub(crate) receipts: Section<(Vec, Vec)>, + pub(crate) receipts: Section, pub(crate) reviews: Section>, pub(crate) unobservable: Section<(Vec, Vec)>, pub(crate) readings: Section>, diff --git a/crates/sqlquack-syntax-coverage/src/formats/constants.rs b/crates/sqlquack-syntax-coverage/src/formats/constants.rs index 0ddf80c2c..9113fa9be 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/constants.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/constants.rs @@ -72,6 +72,8 @@ pub const SNAPSHOT_FORMAT: &str = "sqlquack-syntax-coverage-snapshot/1"; pub const BASELINE_FILE: &str = "baseline.json"; /// The run-report list of receipt lines the importer withdrew, each by `receipt_sha256`. pub const REPORT_WITHDRAWN: &str = "withdrawn"; +/// The run-report list of statements the importer read, each with `sql` and `sql_sha256`. +pub const REPORT_INPUTS: &str = "inputs"; /// The field of a withdrawn entry naming the receipt line's SHA-256. pub const REPORT_RECEIPT_SHA256: &str = "receipt_sha256"; /// The importer's run report beside each receipts file. diff --git a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs index 92afa166c..b9827eace 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs @@ -35,6 +35,7 @@ pub fn load_coverage(root: &Path) -> Result> { cases: sections.cases.records, receipts: sections.receipts.records.0, retired_receipts: sections.receipts.records.1, + receipt_sql: sections.receipts.records.2, reviews: sections.reviews.records, unobservable: sections.unobservable.records.0, diagnostics: sections.unobservable.records.1, diff --git a/crates/sqlquack-syntax-coverage/src/formats/models.rs b/crates/sqlquack-syntax-coverage/src/formats/models.rs index 1090e3a6e..59df56c6a 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/models.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/models.rs @@ -564,6 +564,8 @@ pub struct SyntaxCoverage { pub receipts: Vec, /// SHA-256 of each published receipt line that is superseded or withdrawn. pub retired_receipts: Vec, + /// The SQL of every statement a run report read, by its SHA-256, cited by a case or not. + pub receipt_sql: BTreeMap, pub reviews: Vec, pub unobservable: Vec, pub diagnostics: Vec, diff --git a/crates/sqlquack-syntax-coverage/src/formats/types.rs b/crates/sqlquack-syntax-coverage/src/formats/types.rs index 5e1fef53b..af59e907d 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/types.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/types.rs @@ -2,7 +2,10 @@ use std::collections::BTreeMap; -use crate::formats::models::{Definition, Form}; +use crate::formats::models::{Definition, Form, Receipt}; /// Each family's forms and definitions, in file order. pub(crate) type FamilyCatalogue = BTreeMap, Vec)>; + +/// The current receipts, the retired receipt line hashes, and the run reports' SQL by its hash. +pub(crate) type LoadedReceipts = (Vec, Vec, BTreeMap); diff --git a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs index 35eeae8fa..fdd36711e 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs @@ -25,7 +25,33 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff { .intersection(&head.syntax_error_cases_accepted) .cloned() .collect(); + let empty: BTreeSet = BTreeSet::new(); + let head_contradicted: &BTreeSet = + head.receipt_contradictions.as_ref().unwrap_or(&empty); + let new_contradictions: Vec = match &base.receipt_contradictions { + Some(base_contradicted) => head_contradicted + .difference(base_contradicted) + .cloned() + .collect(), + None => Vec::new(), + }; SnapshotDiff { + receipts_contradicted: new_contradictions + .iter() + .filter(|id| !head.documented_cases.contains(*id)) + .cloned() + .collect(), + documented_receipts_contradicted: new_contradictions + .iter() + .filter(|id| head.documented_cases.contains(*id)) + .cloned() + .collect(), + receipts_still_contradicted: head_contradicted + .iter() + .filter(|id| !new_contradictions.contains(id)) + .cloned() + .collect(), + unjudged_receipts: head.unjudged_receipts.iter().flatten().cloned().collect(), catalogue_files_changed: paths .into_iter() .filter(|path| base.catalogue_files.get(*path) != head.catalogue_files.get(*path)) diff --git a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs index 003a8e2d0..32f0ee050 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs @@ -65,5 +65,7 @@ pub fn snapshot( syntax_error_cases_rejected: by_snowflake(check, SnowflakeSyntax::Refused, false), syntax_error_cases_accepted: by_snowflake(check, SnowflakeSyntax::Refused, true), documented_cases: coverage.documented_cases.clone(), + receipt_contradictions: Some(check.receipt_contradictions.clone()), + unjudged_receipts: Some(check.unjudged_receipts.clone()), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/models.rs b/crates/sqlquack-syntax-coverage/src/report/models.rs index 6f78a8192..dbd6ef07f 100644 --- a/crates/sqlquack-syntax-coverage/src/report/models.rs +++ b/crates/sqlquack-syntax-coverage/src/report/models.rs @@ -73,6 +73,12 @@ pub struct Snapshot { /// Cases the catalogue marks documented (D113): their acceptance against a syntax error is intended. #[serde(default)] pub documented_cases: BTreeSet, + /// Every receipt case id whose statement the parser contradicts, cited or not; none before D141. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub receipt_contradictions: Option>, + /// Receipt case ids whose SQL no case or run report gives; none before D141. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unjudged_receipts: Option>, } /// One snapshot against a later one; coverage and catalogue changes kept apart. @@ -95,4 +101,12 @@ pub struct SnapshotDiff { pub syntax_error_cases_accepted: Vec, /// The same flips on cases marked documented (D113): listed for the auditor, not failing (D135). pub documented_cases_accepted: Vec, + /// Receipts the head contradicts and the base did not, less documented ones: regressions (D141). + pub receipts_contradicted: Vec, + /// The same on receipts of cases marked documented (D113): listed, not failing (D135). + pub documented_receipts_contradicted: Vec, + /// Receipts both contradict, or every head contradiction when the base has no receipt verdicts. + pub receipts_still_contradicted: Vec, + /// Receipts the head cannot judge because no case or run report gives their SQL. + pub unjudged_receipts: Vec, } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs index 830ecd882..5f538264f 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs @@ -34,6 +34,8 @@ pub(crate) fn base() -> Snapshot { syntax_error_cases_rejected: BTreeSet::from(["sf.query.select.k3".to_owned()]), syntax_error_cases_accepted: BTreeSet::new(), documented_cases: BTreeSet::new(), + receipt_contradictions: Some(BTreeSet::new()), + unjudged_receipts: Some(BTreeSet::new()), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs index 8650825d4..3f7e3d11b 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs @@ -1,8 +1,10 @@ +use std::collections::BTreeSet; + use crate::check::models::FormState; use crate::report::main::diff_snapshots::diff_snapshots; use crate::report::models::{Snapshot, SnapshotDiff}; use crate::report::tests::helpers::{base, ids}; -use crate::report::tests::test_types::DiffTestCase; +use crate::report::tests::test_types::{BaseDiffTestCase, DiffTestCase}; #[test] fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_are_named() { @@ -105,6 +107,42 @@ fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_ar }, expected_diff: SnapshotDiff::default(), }, + DiffTestCase { + description: "a receipt the head newly contradicts, cited by a form or not, is a regression", + head: |head| Snapshot { + receipt_contradictions: Some(BTreeSet::from(["retired-1".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + receipts_contradicted: ids(&["retired-1"]), + ..SnapshotDiff::default() + }, + }, + DiffTestCase { + description: "a documented case's receipt newly contradicted is listed apart (D135)", + head: |mut head| { + head.documented_cases + .insert("sf.query.select.k3".to_owned()); + head.receipt_contradictions = + Some(BTreeSet::from(["sf.query.select.k3".to_owned()])); + head + }, + expected_diff: SnapshotDiff { + documented_receipts_contradicted: ids(&["sf.query.select.k3"]), + ..SnapshotDiff::default() + }, + }, + DiffTestCase { + description: "a receipt without SQL to judge is listed, not a regression", + head: |head| Snapshot { + unjudged_receipts: Some(BTreeSet::from(["retired-2".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + unjudged_receipts: ids(&["retired-2"]), + ..SnapshotDiff::default() + }, + }, DiffTestCase { description: "a verified form removed from the catalogue is also lost", head: |mut head| { @@ -123,3 +161,39 @@ fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_ar assert_eq!(diff, test_case.expected_diff, "{}", test_case.description); } } + +#[test] +fn given_a_base_without_receipt_verdicts_when_diffing_then_no_contradiction_is_new() { + let test_cases = [ + BaseDiffTestCase { + description: "every head contradiction is existing against a base from before D141", + base: |base| Snapshot { + receipt_contradictions: None, + ..base + }, + head: |head| Snapshot { + receipt_contradictions: Some(BTreeSet::from(["retired-1".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + receipts_still_contradicted: ids(&["retired-1"]), + ..SnapshotDiff::default() + }, + }, + BaseDiffTestCase { + description: "a head from before D141 lists no receipts at all", + base: |base| base, + head: |head| Snapshot { + receipt_contradictions: None, + unjudged_receipts: None, + ..head + }, + expected_diff: SnapshotDiff::default(), + }, + ]; + for test_case in test_cases { + let diff: SnapshotDiff = + diff_snapshots(&(test_case.base)(base()), &(test_case.head)(base())); + assert_eq!(diff, test_case.expected_diff, "{}", test_case.description); + } +} diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs index ec1e10877..842e7491b 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs @@ -10,6 +10,14 @@ pub(crate) struct DiffTestCase { pub(crate) expected_diff: SnapshotDiff, } +/// A change to both snapshots, and the diff it must produce. +pub(crate) struct BaseDiffTestCase { + pub(crate) description: &'static str, + pub(crate) base: fn(Snapshot) -> Snapshot, + pub(crate) head: fn(Snapshot) -> Snapshot, + pub(crate) expected_diff: SnapshotDiff, +} + /// Receipts and expectations set by `mutate`, a base and a head parser, and what the diff must flag. pub(crate) struct ParserChangeTestCase { pub(crate) description: &'static str, diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 0cdfa6ef4..6c1ad6957 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -416,6 +416,24 @@ tree, so any parser commit makes its verified forms stale until it is renewed; a fingerprints goes stale only for the forms whose cases now parse differently. Either shows up as a regression. +## Receipt contradictions (decision D141) + +The case lists above only reach receipts through the catalogue: a receipt whose case id no form +cites, such as a retired or hand-written case, never counts. A snapshot therefore also records +`receipt_contradictions`: every current receipt, cited or not, whose statement the parser +contradicts. Statements are grouped by SQL, setup and session hashes and judged as above: the parser +rejects a statement Snowflake parsed, or accepts one Snowflake refused as syntax. The SQL comes from +the case that names the hash or from the `inputs` of the run reports beside the receipts, each +checked against its hash. A receipt neither gives is listed in `unjudged_receipts` and judged by +nothing. `diff` fails on a receipt the head contradicts and the base did not. A receipt of a case +marked `documented (D113)` is listed on its own line instead (decision D135). Receipts both sides +contradict, and every head contradiction when the base snapshot predates these fields, are listed in +the JSON without failing. + +The tracked baseline and receipts are a subset of the evidence, so a merge gate takes both snapshots +on the full catalogue and evidence: `snapshot` with the target branch's binary, `snapshot` with the +head's binary, then `diff BASE HEAD`. + ## Translation `syntax-coverage translation --sqlquack [--policy sqlquack.toml] [--family ]