From cd4a2186e895885a7dfff56953634cda4395130e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 15:20:10 +0000 Subject: [PATCH] coverage: judge every receipt against the parser and fail on new contradictions (D141) The snapshot's case lists only reach receipts through catalogue forms, so a receipt on a retired or uncited case id never counted. The check now judges every current receipt by its statement (SQL, setup and session), with SQL from the citing case or from the hash-checked inputs of the run reports, and records receipt_contradictions and unjudged_receipts in the snapshot. diff fails on a contradiction the base did not have; documented (D113) ones go on their own non-failing line (D135), and existing ones are listed without failing. On catalogue 8c9639e7 and evidence a82d976b, 4203044e -> cbe394ec reports 53 new contradictions, including all 27 the D118 audit of #68 found, plus the 68 documented flips on their own line. --- .../src/check/_helpers/contradictions.rs | 64 +++++++++++++++ .../src/check/_helpers/mod.rs | 1 + .../src/check/_helpers/receipts.rs | 2 +- .../src/check/main/check_coverage.rs | 5 ++ .../src/check/models.rs | 4 + .../src/check/tests/helpers.rs | 14 ++++ .../src/check/tests/test_check_coverage.rs | 78 ++++++++++++++++++- .../src/check/tests/test_types.rs | 9 +++ .../src/cli/_helpers/commands.rs | 32 +++++++- .../src/formats/_helpers/retired.rs | 26 +++++-- .../src/formats/_helpers/sections.rs | 14 ++-- .../src/formats/constants.rs | 2 + .../src/formats/main/load_coverage.rs | 1 + .../src/formats/models.rs | 2 + .../src/formats/types.rs | 5 +- .../src/report/main/diff_snapshots.rs | 26 +++++++ .../src/report/main/snapshot.rs | 2 + .../src/report/models.rs | 14 ++++ .../src/report/tests/helpers.rs | 2 + .../src/report/tests/test_diff_snapshots.rs | 76 +++++++++++++++++- .../src/report/tests/test_types.rs | 8 ++ docs/syntax-coverage.md | 18 +++++ 22 files changed, 384 insertions(+), 21 deletions(-) create mode 100644 crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs 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 ]