From 2332973af43a607f6e6cec196ec8e258d9184f13 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:20:54 +0100 Subject: [PATCH] fix(rules): enforce semantic model layers --- .../src/configuration/_helpers/loading.rs | 21 ++ crates/sqlbuild-rules-native/src/constants.rs | 6 + .../src/engine/tests/test_evaluation.rs | 152 +++++++++++++ .../src/engine/tests/test_types.rs | 12 ++ crates/sqlbuild-rules-native/src/models.rs | 12 ++ .../src/rules/_helpers/catalogue.rs | 24 ++- .../src/rules/_helpers/evaluation.rs | 88 ++++---- .../src/rules/_helpers/mod.rs | 1 + .../src/rules/_helpers/model_layers.rs | 203 ++++++++++++++++++ .../src/sql_lint/_helpers/additional.rs | 48 +++++ .../src/sql_lint/_helpers/engine.rs | 3 + .../src/sql_lint/models.rs | 3 + .../src/sql_lint/tests.rs | 2 + .../sql_lint/tests/test_empty_fixture_lint.rs | 61 ++++++ .../src/sql_lint/tests/test_types.rs | 7 + docs/model-layer-rules.md | 39 ++++ docs/native-lint-rule-matrix.md | 2 +- .../compile/_helpers/attachment/core.py | 11 +- .../_helpers/attachment/model_config.py | 4 + src/sqlbuild/compiler/compile/constants.py | 1 + src/sqlbuild/compiler/compile/models.py | 2 + src/sqlbuild/lint/_helpers/native_sql.py | 4 + src/sqlbuild/lint/main/run_lint.py | 27 ++- src/sqlbuild/lint/models.py | 1 + .../rule_engine/_helpers/engine/config.py | 9 + .../rule_engine/_helpers/engine/native.py | 2 + .../rule_engine/_helpers/engine/ruleset.py | 1 + .../rule_engine/_helpers/guidance/skills.py | 20 +- .../rule_engine/_helpers/run/rules.py | 17 +- src/sqlbuild/rule_engine/models.py | 10 + .../sqlbuild/cli/commands/main/test_format.py | 61 ++++++ .../sqlbuild/cli/commands/main/test_rules.py | 132 ++++++++++++ .../_helpers/engine/test_catalogue.py | 6 + 33 files changed, 933 insertions(+), 59 deletions(-) create mode 100644 crates/sqlbuild-rules-native/src/rules/_helpers/model_layers.rs create mode 100644 crates/sqlbuild-rules-native/src/sql_lint/tests/test_empty_fixture_lint.rs create mode 100644 docs/model-layer-rules.md diff --git a/crates/sqlbuild-rules-native/src/configuration/_helpers/loading.rs b/crates/sqlbuild-rules-native/src/configuration/_helpers/loading.rs index 00df6a0f8d..2ff74ddcfd 100644 --- a/crates/sqlbuild-rules-native/src/configuration/_helpers/loading.rs +++ b/crates/sqlbuild-rules-native/src/configuration/_helpers/loading.rs @@ -49,6 +49,7 @@ fn validate_raw(value: &toml::Value) -> Result<(), String> { "threshold_overrides", "rule_options", "rule_exceptions", + "graph_edge_exceptions", "rule_ignores", "select_star_allow", "domains", @@ -69,6 +70,10 @@ fn validate_raw(value: &toml::Value) -> Result<(), String> { } for (key, fields) in [ ("rule_exceptions", ["rule", "path", "reason"].as_slice()), + ( + "graph_edge_exceptions", + ["consumer", "dependency", "reason"].as_slice(), + ), ("rule_ignores", ["rules", "reason"].as_slice()), ("select_star_allow", ["paths", "reason"].as_slice()), ( @@ -197,6 +202,22 @@ pub(crate) fn validate(config: &RulesConfig) -> Result<(), String> { return Err(format!("invalid rule code: {}", exception.rule)); } } + let mut graph_edges: std::collections::BTreeSet<(&str, &str)> = + std::collections::BTreeSet::new(); + for exception in &config.graph_edge_exceptions { + if exception.consumer.trim().is_empty() + || exception.dependency.trim().is_empty() + || exception.reason.trim().is_empty() + { + return Err("rules.graph_edge_exceptions fields must be non-empty strings".into()); + } + if !graph_edges.insert((&exception.consumer, &exception.dependency)) { + return Err(format!( + "duplicate graph edge exception: {} -> {}", + exception.consumer, exception.dependency + )); + } + } for ignore in &config.rule_ignores { if ignore.rules.is_empty() || (ignore.paths.is_empty() && ignore.selectors.is_empty()) diff --git a/crates/sqlbuild-rules-native/src/constants.rs b/crates/sqlbuild-rules-native/src/constants.rs index 1552e3d84c..e562e1efde 100644 --- a/crates/sqlbuild-rules-native/src/constants.rs +++ b/crates/sqlbuild-rules-native/src/constants.rs @@ -10,6 +10,10 @@ pub(crate) const GENERIC_CASE_NAME: &str = "case"; pub(crate) const CUSTOM_RULE_NAMESPACE: &str = "XSQBR"; pub(crate) const TARGET_DIRECTORY: &str = "target"; pub(crate) const REFERENCE_KIND: &str = "ref"; +pub(crate) const MODEL_SCHEMA_CONFIG_KEY: &str = "schema"; +pub(crate) const STAGING_LAYER_DIRECTORY: &str = "staging"; +pub(crate) const INTERMEDIATE_LAYER_DIRECTORY: &str = "intermediate"; +pub(crate) const MART_LAYER_DIRECTORY: &str = "mart"; pub(crate) const SOURCE_REFERENCE_KIND: &str = "source"; pub(crate) const DBT_REFERENCE_KIND: &str = "dbt_ref"; pub(crate) const TABLE_FUNCTION_REFERENCE_KIND: &str = "table_function"; @@ -37,6 +41,8 @@ pub(crate) const INLINE_SQL_HOOK: &str = "inline_sql"; pub(crate) const MACRO_TEST_MODE: &str = "macro"; pub(crate) const OPEN_PAREN: &str = "("; pub(crate) const CLOSE_PAREN: &str = ")"; +pub(crate) const EMPTY_FIXTURE_QUERY_TOKEN_COUNT: usize = 6; +pub(crate) const ENCLOSING_CTE_TOKEN_COUNT: usize = 3; pub(crate) const OUTSIDE_KEY: &str = "outside"; pub(crate) const SQL_HOOK: &str = "sql"; pub(crate) const TABLE_FUNCTION_TEST_MODE: &str = "table_fn"; diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs index 6bddfc24fd..e6af01313f 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs @@ -6,6 +6,158 @@ use crate::engine::tests::helpers; use crate::engine::tests::test_types; use crate::rules::_helpers::evaluation::normalize_rules_sql; +#[test] +fn given_conflicting_model_identity_when_evaluating_layer_rules_then_reports_independent_findings() +-> Result<(), String> { + let test_cases = [test_types::ModelLayerRulesTestCase { + description: "conflicting model metadata reports every independent boundary", + expected_codes: &[ + "SQBRMODEL103", + "SQBRMODEL104", + "SQBRPROJECT101", + "SQBRPROJECT104", + "SQBRPROJECT105", + "SQBRPROJECT106", + ], + }]; + let project_dir = TempDir::new().map_err(|error| error.to_string())?; + let config = json!({ + "select": [ + "SQBRMODEL103", + "SQBRMODEL104", + "SQBRPROJECT101", + "SQBRPROJECT104", + "SQBRPROJECT105", + "SQBRPROJECT106" + ], + "cache": {"enabled": false} + }); + let mut request: Value = serde_json::from_str(&helpers::request(&project_dir, &config)) + .map_err(|error| error.to_string())?; + request["models"][0] = json!({ + "name": "commerce__mart_v__int_v_orders", + "relative_path": "models/commerce/intermediate/enriched/orders/commerce__mart_v__int_v_orders.sql", + "query_sql": "SELECT 1 AS order_id", + "authored_sql": "SELECT 1 AS order_id", + "config": {"materialized": "table", "schema": "staging"}, + "authored_config_keys": ["materialized", "schema"], + "logical_schema": "staging", + "references": [{"ref_kind": "ref", "ref_name": "commerce__mart__stg_orders"}] + }); + + for test_case in test_cases { + let result: Value = serde_json::from_str(&evaluate_json(&request.to_string())?) + .map_err(|error| error.to_string())?; + let codes: Vec<&str> = result["faults"] + .as_array() + .into_iter() + .flatten() + .filter_map(|fault| fault["code"].as_str()) + .collect(); + + assert_eq!(codes, test_case.expected_codes, "{}", test_case.description); + } + Ok(()) +} + +#[test] +fn given_internal_dependency_edges_when_evaluating_graph_rule_then_only_exact_live_exceptions_apply() +-> Result<(), String> { + let project_dir = TempDir::new().map_err(|error| error.to_string())?; + let models = json!([ + { + "name": "commerce__stg__orders", + "relative_path": "models/commerce/staging/commerce__stg__orders.sql", + "query_sql": "SELECT 1", + "authored_sql": "SELECT 1", + "references": [ + {"ref_kind": "ref", "ref_name": "commerce__int_enriched__orders"}, + {"ref_kind": "ref", "ref_name": "commerce__mart__customers"} + ] + }, + { + "name": "commerce__int_clean__orders", + "relative_path": "models/commerce/intermediate/clean/commerce__int_clean__orders.sql", + "query_sql": "SELECT 1", + "authored_sql": "SELECT 1", + "references": [{"ref_kind": "ref", "ref_name": "commerce__int_enriched__customers"}] + }, + { + "name": "commerce__mart__summary", + "relative_path": "models/commerce/mart/commerce__mart__summary.sql", + "query_sql": "SELECT 1", + "authored_sql": "SELECT 1", + "references": [{"ref_kind": "ref", "ref_name": "commerce__int_enriched__orders"}] + } + ]); + let test_cases = [ + test_types::GraphRuleTestCase { + description: "unexcluded internal inversions", + exceptions: json!([]), + expected_fault_count: 2, + expected_message_fragments: &[], + }, + test_types::GraphRuleTestCase { + description: "exact live exceptions", + exceptions: json!([ + { + "consumer": "commerce__stg__orders", + "dependency": "commerce__int_enriched__orders", + "reason": "Tracked cleanup" + }, + { + "consumer": "commerce__int_clean__orders", + "dependency": "commerce__int_enriched__customers", + "reason": "Tracked cleanup" + } + ]), + expected_fault_count: 0, + expected_message_fragments: &[], + }, + test_types::GraphRuleTestCase { + description: "stale exact exception", + exceptions: json!([{ + "consumer": "commerce__stg__orders", + "dependency": "commerce__int_clean__missing", + "reason": "Resolved cleanup" + }]), + expected_fault_count: 3, + expected_message_fragments: &["graph edge exception is stale"], + }, + ]; + for test_case in test_cases { + let config = json!({ + "select": ["SQBRGRAPH101"], + "graph_edge_exceptions": test_case.exceptions, + "cache": {"enabled": false} + }); + let mut request: Value = serde_json::from_str(&helpers::request(&project_dir, &config)) + .map_err(|error| error.to_string())?; + request["models"] = models.clone(); + let result: Value = serde_json::from_str(&evaluate_json(&request.to_string())?) + .map_err(|error| error.to_string())?; + let faults = result["faults"] + .as_array() + .ok_or("faults must be an array")?; + assert_eq!( + faults.len(), + test_case.expected_fault_count, + "{}", + test_case.description + ); + for fragment in test_case.expected_message_fragments { + assert!( + faults.iter().any(|fault| fault["message"] + .as_str() + .is_some_and(|message| message.contains(fragment))), + "{}", + test_case.description + ); + } + } + Ok(()) +} + #[test] fn given_repeated_native_evaluation_when_faulting_then_returns_deterministic_complete_facts() -> Result<(), String> { diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs index 46a4aa88db..997dceb20c 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs @@ -6,6 +6,18 @@ pub(crate) struct NativeEvaluationTestCase { pub(crate) expected_faults: Value, } +pub(crate) struct ModelLayerRulesTestCase { + pub(crate) description: &'static str, + pub(crate) expected_codes: &'static [&'static str], +} + +pub(crate) struct GraphRuleTestCase { + pub(crate) description: &'static str, + pub(crate) exceptions: Value, + pub(crate) expected_fault_count: usize, + pub(crate) expected_message_fragments: &'static [&'static str], +} + pub(crate) struct DynamicPivotEvaluationTestCase { pub(crate) description: &'static str, pub(crate) expected_faults: Value, diff --git a/crates/sqlbuild-rules-native/src/models.rs b/crates/sqlbuild-rules-native/src/models.rs index 29ba2d0c4d..dd7dce1ffc 100644 --- a/crates/sqlbuild-rules-native/src/models.rs +++ b/crates/sqlbuild-rules-native/src/models.rs @@ -65,6 +65,14 @@ pub(crate) struct RuleException { pub reason: String, } +#[derive(Clone, Debug, Deserialize, Serialize, Eq, Ord, PartialEq, PartialOrd)] +#[serde(deny_unknown_fields)] +pub(crate) struct GraphEdgeException { + pub consumer: String, + pub dependency: String, + pub reason: String, +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub(crate) struct RuleIgnore { @@ -135,6 +143,7 @@ pub(crate) struct RulesConfig { pub threshold_overrides: Vec, pub rule_options: BTreeMap>, pub rule_exceptions: Vec, + pub graph_edge_exceptions: Vec, pub rule_ignores: Vec, pub select_star_allow: Vec, pub domains: Vec, @@ -154,6 +163,7 @@ impl Default for RulesConfig { threshold_overrides: vec![], rule_options: BTreeMap::new(), rule_exceptions: vec![], + graph_edge_exceptions: vec![], rule_ignores: vec![], select_star_allow: vec![], domains: vec![], @@ -508,6 +518,8 @@ pub(crate) struct Model { pub query_sql: String, pub authored_sql: String, pub config: BTreeMap, + pub authored_config_keys: Vec, + pub logical_schema: Option, pub references: Vec, pub columns: Vec, pub dynamic_columns: Vec, diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/catalogue.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/catalogue.rs index ab85fd4158..75bf312128 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/catalogue.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/catalogue.rs @@ -33,6 +33,7 @@ macro_rules! rule { | "SQBRPROJECT202" | "SQBRPROJECT203" | "SQBRPROJECT204" + | "SQBRGRAPH101" | "SQBRTEST101" | "SQBRTEST102" | "SQBRTEST103" @@ -66,7 +67,14 @@ pub(crate) fn catalogue() -> Vec { "structure", "view-marker", "view materialization and model v marker must agree", - "Use stg_v/int_v/mart_v for a view, or change the materialization to match the non-view layer name.", + "Use stg_v/int_v/mart_v for a view, or change the materialization to match the non-view layer name. Keep _v only in the canonical layer segment.", + ), + rule!( + "SQBRMODEL104", + "structure", + "central-model-schema", + "model schemas must come from central path defaults", + "Place the model under the folder matching its semantic layer, then remove the schema declaration. Configure intentional schema differences centrally in sqlbuild_project.toml.", ), rule!( "SQBRGRAPH101", @@ -110,6 +118,20 @@ pub(crate) fn catalogue() -> Vec { "referenced model identifiers must follow rule naming grammar", "Rename the referenced model and this __ref to the rule model grammar.", ), + rule!( + "SQBRPROJECT105", + "repository", + "name-folder-alignment", + "model name layer must match its canonical folder", + "Make the model name and canonical layer folder agree. Choose the layer from the model's semantic responsibility, not from its dependencies.", + ), + rule!( + "SQBRPROJECT106", + "repository", + "name-schema-alignment", + "model name layer must match its effective logical schema", + "Correct the central path default or move the model to its intended semantic-layer folder. Do not add a model-local schema declaration.", + ), rule!( "SQBRCONTRACT101", "contracts", diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs index c2df5204a3..1636388d13 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs @@ -5,7 +5,7 @@ use crate::constants::{ use crate::models::{Declaration, EvaluateRequest, Fault, Model, RuleMetadata, RulesConfig}; use crate::rules::_helpers::domain_layout::folder_layer_details; use crate::rules::_helpers::{ - contract_name_types, dynamic_contracts, explicit_output_types, numeric_decisions, + contract_name_types, dynamic_contracts, explicit_output_types, model_layers, numeric_decisions, typed_contract_columns, }; use crate::rules::models::{ @@ -161,8 +161,8 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result) -> Result) -> Result) -> Vec { let faults = FaultCollector::default(); + if let Some(rule) = request.selected.get("SQBRGRAPH101") { + model_layers::internal_forward_refs(request, rule, &faults); + } if let Some(rule) = request.selected.get("SQBRDECLARATION201") { duplicate_enums(request.request, rule, &faults); } @@ -1039,14 +1048,15 @@ fn select_star( } #[derive(Clone)] -struct NameParts { +pub(super) struct NameParts { domain: String, - layer: String, + pub(super) layer: String, + subject: String, source: Option, is_view: bool, } -fn parse_name(name: &str) -> Option { +pub(super) fn parse_name(name: &str) -> Option { let parts: Vec<&str> = name.split("__").collect(); if !(3..=4).contains(&parts.len()) { return None; @@ -1072,6 +1082,7 @@ fn parse_name(name: &str) -> Option { Some(NameParts { domain: parts[0].into(), layer: parts[1].into(), + subject: parts[2].into(), source: parts.get(3).map(|value| (*value).into()), is_view: parts[1].ends_with("_v"), }) @@ -1098,40 +1109,6 @@ fn view_marker(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultColl } } -fn layer_order(layer: &str) -> usize { - match layer { - "stg" | "stg_v" => 0, - "int_clean" => 1, - "int_v" | "int_enriched" => 2, - _ => 3, - } -} - -fn forward_refs(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { - let Some(current) = parse_name(&parsed.model.name) else { - return; - }; - for reference in &parsed.model.references { - if reference.ref_kind != REFERENCE_KIND { - continue; - } - if let Some(upstream) = parse_name(&reference.ref_name) - && layer_order(&upstream.layer) > layer_order(¤t.layer) - { - faults.push(custom_fault!( - parsed.model, - rule, - None, - format!( - "{} reaches forward from {} to {} via {}", - parsed.model.name, current.layer, upstream.layer, reference.ref_name - ), - None, - )); - } - } -} - fn raw_qualified_tables(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { for query in all_queries(&parsed.query) { let Some(select) = root_select(query) else { @@ -1179,6 +1156,18 @@ fn name_grammar( faults: &FaultCollector, ) { if let Some(parts) = parse_name(&parsed.model.name) { + if let Some(marker) = duplicate_layer_marker(&parts.subject) { + faults.push(custom_fault!( + parsed.model, + rule, + None, + format!( + "model {:?} declares layer {:?} but its subject starts with a second layer marker {:?}", + parsed.model.name, parts.layer, marker + ), + Some("Rename the model so exactly one layer appears immediately after the domain. Do not preserve the previous layer inside the subject; resolve any resulting identity collision as a semantic design decision.".into()), + )); + } if !config.domains.is_empty() && !config.domains.contains(&parts.domain) { faults.push(custom_fault!( parsed.model, @@ -1209,6 +1198,21 @@ fn name_grammar( faults.push(custom_fault!(parsed.model, rule, None, message, None)); } +fn duplicate_layer_marker(subject: &str) -> Option<&'static str> { + [ + "int_enriched", + "int_clean", + "mart_v", + "stg_v", + "int_v", + "mart", + "stg", + "v", + ] + .into_iter() + .find(|marker| subject.starts_with(&format!("{marker}_"))) +} + fn folder_layer(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { let Some(parts) = parse_name(&parsed.model.name) else { return; @@ -1286,7 +1290,9 @@ fn source_token_rule( fn reference_name_rule(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { for reference in &parsed.model.references { - if reference.ref_kind == REFERENCE_KIND && parse_name(&reference.ref_name).is_none() { + let invalid = parse_name(&reference.ref_name) + .is_none_or(|parts| duplicate_layer_marker(&parts.subject).is_some()); + if reference.ref_kind == REFERENCE_KIND && invalid { faults.push(custom_fault!( parsed.model, rule, diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs index b2fa6832a4..dcb6787ba2 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod domain_layout; pub(crate) mod dynamic_contracts; pub(crate) mod evaluation; pub(crate) mod explicit_output_types; +pub(crate) mod model_layers; pub(crate) mod numeric_decisions; pub(crate) mod sql_test_rules; pub(crate) mod typed_contract_columns; diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/model_layers.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/model_layers.rs new file mode 100644 index 0000000000..9f9a42372d --- /dev/null +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/model_layers.rs @@ -0,0 +1,203 @@ +use std::collections::BTreeSet; + +use crate::constants::{ + INTERMEDIATE_LAYER_DIRECTORY, MART_LAYER_DIRECTORY, MODEL_SCHEMA_CONFIG_KEY, REFERENCE_KIND, + STAGING_LAYER_DIRECTORY, +}; +use crate::models::{Fault, Model, RuleMetadata}; +use crate::rules::_helpers::evaluation::parse_name; +use crate::rules::models::{FaultCollector, ProjectEvaluationRequest}; + +const NO_CANONICAL_LAYER_FOLDER: &str = "no canonical layer folder"; + +pub(crate) fn explicit_model_schema(model: &Model, rule: &RuleMetadata, faults: &FaultCollector) { + if model + .authored_config_keys + .iter() + .any(|key| key == MODEL_SCHEMA_CONFIG_KEY) + { + let schema = model + .config + .get(MODEL_SCHEMA_CONFIG_KEY) + .and_then(serde_json::Value::as_str) + .unwrap_or(""); + faults.push(model_fault( + model, + rule, + format!( + "model declares schema {schema:?} explicitly; model schemas must come from path defaults" + ), + None, + )); + } +} + +pub(crate) fn internal_forward_refs( + evaluation: &ProjectEvaluationRequest<'_>, + rule: &RuleMetadata, + faults: &FaultCollector, +) { + let configured: BTreeSet<(String, String)> = evaluation + .request + .config + .graph_edge_exceptions + .iter() + .map(|entry| (entry.consumer.clone(), entry.dependency.clone())) + .collect(); + let mut used: BTreeSet<(String, String)> = BTreeSet::new(); + for model in &evaluation.request.models { + let Some(current) = parse_name(&model.name) else { + continue; + }; + let Some(current_rank) = internal_layer_order(¤t.layer) else { + continue; + }; + for reference in &model.references { + if reference.ref_kind != REFERENCE_KIND { + continue; + } + let Some(upstream) = parse_name(&reference.ref_name) else { + continue; + }; + let Some(upstream_rank) = internal_layer_order(&upstream.layer) else { + continue; + }; + if upstream_rank <= current_rank { + continue; + } + let edge = (model.name.clone(), reference.ref_name.clone()); + if configured.contains(&edge) { + used.insert(edge); + continue; + } + faults.push(model_fault( + model, + rule, + format!( + "internal {} model {:?} depends on later internal {} model {:?}", + current.layer, model.name, upstream.layer, reference.ref_name + ), + Some("Keep the consumer in its semantic layer. Move the required logic into the same or an earlier internal layer, or expose it through an approved published mart interface. For temporary debt, configure this exact consumer/dependency edge with a reason; do not rename or move the consumer merely to satisfy this rule.".into()), + )); + } + } + for exception in configured.difference(&used) { + faults.push(Fault { + code: rule.code.clone(), + path: "sqlbuild_project.toml".into(), + line: 1, + column: 1, + message: format!( + "graph edge exception is stale: {} -> {}", + exception.0, exception.1 + ), + remediation: "Remove the stale graph_edge_exceptions entry.".into(), + }); + } +} + +pub(crate) fn name_folder_alignment(model: &Model, rule: &RuleMetadata, faults: &FaultCollector) { + let Some(parts) = parse_name(&model.name) else { + return; + }; + let expected = canonical_folder_for_layer(&parts.layer); + let actual = canonical_folder_in_path(&model.relative_path); + if actual != Some(expected) { + faults.push(model_fault( + model, + rule, + format!( + "model {:?} declares layer {:?} but is stored under {:?}; expected canonical folder {:?}", + model.name, + parts.layer, + actual.unwrap_or(NO_CANONICAL_LAYER_FOLDER), + expected + ), + None, + )); + } +} + +pub(crate) fn name_schema_alignment(model: &Model, rule: &RuleMetadata, faults: &FaultCollector) { + let Some(parts) = parse_name(&model.name) else { + return; + }; + let Some(actual) = model.logical_schema.as_deref() else { + return; + }; + let expected = logical_schema_for_layer(&parts.layer); + if !actual.eq_ignore_ascii_case(expected) { + faults.push(model_fault( + model, + rule, + format!( + "model {:?} resolves to schema {:?}, but layer {:?} requires schema {:?}", + model.name, actual, parts.layer, expected + ), + None, + )); + } +} + +fn internal_layer_order(layer: &str) -> Option { + match layer { + "stg" | "stg_v" => Some(0), + "int_clean" => Some(1), + "int_v" | "int_enriched" => Some(2), + _ => None, + } +} + +fn canonical_folder_for_layer(layer: &str) -> &'static str { + match layer { + "stg" | "stg_v" => STAGING_LAYER_DIRECTORY, + "int_clean" => "intermediate/clean", + "int_enriched" | "int_v" => "intermediate/enriched", + _ => MART_LAYER_DIRECTORY, + } +} + +fn canonical_folder_in_path(relative_path: &str) -> Option<&'static str> { + let parts: Vec<&str> = relative_path.split('/').collect(); + let parent = &parts[..parts.len().saturating_sub(1)]; + for (index, component) in parent.iter().enumerate().skip(1) { + if *component == STAGING_LAYER_DIRECTORY { + return Some(STAGING_LAYER_DIRECTORY); + } + if *component == MART_LAYER_DIRECTORY { + return Some(MART_LAYER_DIRECTORY); + } + if *component == INTERMEDIATE_LAYER_DIRECTORY { + return match parent.get(index + 1).copied() { + Some("clean") => Some("intermediate/clean"), + Some("enriched") => Some("intermediate/enriched"), + _ => None, + }; + } + } + None +} + +fn logical_schema_for_layer(layer: &str) -> &'static str { + match layer { + "stg" | "stg_v" => STAGING_LAYER_DIRECTORY, + "int_clean" | "int_enriched" | "int_v" => INTERMEDIATE_LAYER_DIRECTORY, + _ => MART_LAYER_DIRECTORY, + } +} + +fn model_fault( + model: &Model, + rule: &RuleMetadata, + message: String, + remediation: Option, +) -> Fault { + Fault { + code: rule.code.clone(), + path: model.relative_path.clone(), + line: 1, + column: 1, + message, + remediation: remediation.unwrap_or_else(|| rule.remediation.clone()), + } +} diff --git a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs index b2aa9e9b4c..78977fb896 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs @@ -2,6 +2,10 @@ use std::collections::{HashMap, HashSet}; use polyglot_sql::tokens::{Span, Token, TokenType}; +use crate::constants::{ + CLOSE_PAREN, EMPTY_FIXTURE_QUERY_TOKEN_COUNT, ENCLOSING_CTE_TOKEN_COUNT, OPEN_PAREN, + SQL_WILDCARD, +}; use crate::sql_lint::_helpers::engine::{ direct_indices, is_comment, is_layout, is_query_from, query_end, significant_after, significant_before, token_depths, @@ -49,6 +53,7 @@ struct SelectFactContext<'a> { dependency_identifiers: &'a HashSet, allows_ceremonial_select: bool, allows_dynamic_output_star: bool, + allows_empty_fixture_star: bool, } struct ProjectedStarContext<'a> { @@ -169,6 +174,7 @@ pub(super) fn collect_additional_facts( dependency_identifiers: options.dependency_identifiers, allows_ceremonial_select: options.allows_ceremonial_select, allows_dynamic_output_star: options.allows_dynamic_output_star, + allows_empty_fixture_star: options.allows_empty_fixture_star, }); facts.duplicate_output_aliases = select_facts.duplicate_output_aliases; facts.unaliased_calculations = select_facts.unaliased_calculations; @@ -277,6 +283,7 @@ fn collect_select_additional_facts(context: &SelectFactContext<'_>) -> Additiona &direct, from_position, ) + && !is_canonical_empty_input_fixture(context, select_index, end) && !has_irreducible_projected_star( &ProjectedStarContext { tokens, @@ -358,6 +365,47 @@ fn collect_select_additional_facts(context: &SelectFactContext<'_>) -> Additiona facts } +fn is_canonical_empty_input_fixture( + context: &SelectFactContext<'_>, + select_index: usize, + query_end: usize, +) -> bool { + if !context.allows_empty_fixture_star { + return false; + } + let significant_query: Vec<&str> = context.tokens[select_index..query_end] + .iter() + .filter(|token| !is_layout(token) && !is_comment(token)) + .map(|token| token.text.as_str()) + .collect(); + if significant_query.len() != EMPTY_FIXTURE_QUERY_TOKEN_COUNT + || !significant_query[0].eq_ignore_ascii_case("select") + || significant_query[1] != SQL_WILDCARD + || !significant_query[2].eq_ignore_ascii_case("from") + || !significant_query[3].eq_ignore_ascii_case("__empty_fixture") + || significant_query[4] != OPEN_PAREN + || significant_query[5] != CLOSE_PAREN + { + return false; + } + let previous: Vec<&Token> = context.tokens[..select_index] + .iter() + .filter(|token| !is_layout(token) && !is_comment(token)) + .rev() + .take(ENCLOSING_CTE_TOKEN_COUNT) + .collect(); + if previous.len() != ENCLOSING_CTE_TOKEN_COUNT + || previous[0].token_type != TokenType::LParen + || !previous[1].text.eq_ignore_ascii_case("as") + { + return false; + } + let cte_name = previous[2].text.trim_matches(['"', '`']); + ["__ref__", "__source__", "__seed__"] + .iter() + .any(|prefix| cte_name.starts_with(prefix)) +} + fn is_compiler_proven_dynamic_output_select( context: &SelectFactContext<'_>, select_index: usize, diff --git a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/engine.rs b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/engine.rs index 530e88f208..e505296172 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/engine.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/engine.rs @@ -323,6 +323,7 @@ struct FactBuildOptions<'a> { externally_referenced_ctes: &'a HashSet, allows_ceremonial_select: bool, allows_dynamic_output_star: bool, + allows_empty_fixture_star: bool, } pub(crate) fn lint_json_impl(request_json: &str) -> Result { @@ -383,6 +384,7 @@ pub(crate) fn lint(request: LintRequest) -> Result { externally_referenced_ctes: &externally_referenced_ctes, allows_ceremonial_select: request.allows_ceremonial_select, allows_dynamic_output_star: request.allows_dynamic_output_star, + allows_empty_fixture_star: request.allows_empty_fixture_star, }; let facts = build_facts(&statements, &tokens, &request.sql, &options); let context = DiagnosticContext { @@ -445,6 +447,7 @@ fn build_facts( dependency_identifiers: options.dependency_identifiers, allows_ceremonial_select: options.allows_ceremonial_select, allows_dynamic_output_star: options.allows_dynamic_output_star, + allows_empty_fixture_star: options.allows_empty_fixture_star, }, ); for expression in expressions { diff --git a/crates/sqlbuild-rules-native/src/sql_lint/models.rs b/crates/sqlbuild-rules-native/src/sql_lint/models.rs index 385588e5f6..9d5229419a 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/models.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/models.rs @@ -71,6 +71,8 @@ pub(crate) struct LintRequest { pub allows_ceremonial_select: bool, #[serde(default)] pub allows_dynamic_output_star: bool, + #[serde(default)] + pub allows_empty_fixture_star: bool, } #[derive(Debug)] @@ -79,6 +81,7 @@ pub(crate) struct AdditionalFactOptions<'a> { pub dependency_identifiers: &'a HashSet, pub allows_ceremonial_select: bool, pub allows_dynamic_output_star: bool, + pub allows_empty_fixture_star: bool, } #[derive(Debug, Serialize)] diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests.rs index 1a632265ba..8bf146e4e8 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests.rs @@ -1,5 +1,7 @@ #[path = "tests/test_batch_lint.rs"] mod batch_lint; +#[path = "tests/test_empty_fixture_lint.rs"] +mod empty_fixture_lint; #[path = "tests/helpers.rs"] mod helpers; #[path = "tests/test_sql_lint.rs"] diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_empty_fixture_lint.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_empty_fixture_lint.rs new file mode 100644 index 0000000000..b5500742b2 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_empty_fixture_lint.rs @@ -0,0 +1,61 @@ +use serde_json::{Value, json}; + +use crate::sql_lint::main::engine::lint_json; +use crate::sql_lint::tests::test_types; + +#[test] +fn given_empty_fixture_context_when_linting_projected_star_then_only_canonical_inputs_pass() +-> Result<(), String> { + let test_cases = [ + test_types::EmptyFixtureLintTestCase { + description: "canonical ref fixture", + sql: "WITH __ref__orders AS (SELECT * FROM __empty_fixture()) SELECT 1", + allows_empty_fixture_star: true, + expected_diagnostic_count: 0, + }, + test_types::EmptyFixtureLintTestCase { + description: "canonical source fixture", + sql: "WITH __source__web__orders AS (SELECT * FROM __empty_fixture()) SELECT 1", + allows_empty_fixture_star: true, + expected_diagnostic_count: 0, + }, + test_types::EmptyFixtureLintTestCase { + description: "ordinary SQL has no intrinsic exemption", + sql: "SELECT * FROM __empty_fixture()", + allows_empty_fixture_star: false, + expected_diagnostic_count: 1, + }, + test_types::EmptyFixtureLintTestCase { + description: "expected fixture remains controlled", + sql: "WITH __expected__orders AS (SELECT * FROM __empty_fixture()) SELECT 1", + allows_empty_fixture_star: true, + expected_diagnostic_count: 1, + }, + test_types::EmptyFixtureLintTestCase { + description: "additional projection is not canonical", + sql: "WITH __ref__orders AS (SELECT *, 1 AS extra FROM __empty_fixture()) SELECT 1", + allows_empty_fixture_star: true, + expected_diagnostic_count: 1, + }, + ]; + for test_case in test_cases { + let response = lint_json( + &json!({ + "version": 1, + "sql": test_case.sql, + "dialect": "snowflake", + "enabled_rules": ["SQBRSQL021"], + "allows_empty_fixture_star": test_case.allows_empty_fixture_star + }) + .to_string(), + )?; + let payload: Value = serde_json::from_str(&response).map_err(|error| error.to_string())?; + assert_eq!( + payload["diagnostics"].as_array().map_or(0, Vec::len), + test_case.expected_diagnostic_count, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_types.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_types.rs index 1ff58ad258..597c7ccc7e 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_types.rs @@ -5,6 +5,13 @@ pub(crate) struct LintTestCase { pub expected_anchors: &'static [(&'static str, &'static str)], } +pub(crate) struct EmptyFixtureLintTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) allows_empty_fixture_star: bool, + pub(crate) expected_diagnostic_count: usize, +} + pub(crate) struct BatchLintTestCase { pub(crate) description: &'static str, pub(crate) queries: &'static [&'static str], diff --git a/docs/model-layer-rules.md b/docs/model-layer-rules.md new file mode 100644 index 0000000000..b31c5ae74a --- /dev/null +++ b/docs/model-layer-rules.md @@ -0,0 +1,39 @@ +# Model layer rules + +SQLBuild model layers describe semantic responsibility, not the highest layer among a model's +dependencies. A model keeps its role when it imports a published interface. + +Model identities use exactly one authoritative layer: + +```text +____[__] +``` + +Supported layers and canonical folders are: + +| Layer | Folder | Logical schema | +| --- | --- | --- | +| `stg`, `stg_v` | `staging` | `staging` | +| `int_clean` | `intermediate/clean` | `intermediate` | +| `int_enriched`, `int_v` | `intermediate/enriched` | `intermediate` | +| `mart`, `mart_v` | `mart` | `mart` | + +`mart` and `mart_v` are published interfaces. Staging and intermediate layers are internal. The +internal dependency order is `stg`/`stg_v` → `int_clean` → `int_enriched`/`int_v`; dependencies on +published marts are outside that rank check. + +Schemas belong in central path defaults, not individual `MODEL()` declarations. View +materialization requires the matching `_v` layer, and `_v` appears only in the canonical layer +segment. + +Temporary internal dependency debt uses exact central exceptions: + +```toml +[[rules.graph_edge_exceptions]] +consumer = "orders__stg__daily_orders" +dependency = "orders__int_clean__customer_orders" +reason = "Remove after the shared normalization is moved upstream." +``` + +An exception matches one consumer/dependency edge. Wildcards are unsupported, and an entry that no +longer suppresses an internal inversion is reported as stale. diff --git a/docs/native-lint-rule-matrix.md b/docs/native-lint-rule-matrix.md index 8f8afcd920..a31532421b 100644 --- a/docs/native-lint-rule-matrix.md +++ b/docs/native-lint-rule-matrix.md @@ -40,7 +40,7 @@ Rewrite classifications describe formatter-owned transformations: **always**, ** | AM01 DISTINCT with GROUP BY | Core compiler | `SQBRSQL006` | always | SQLBuild reports only demonstrably redundant `DISTINCT`. | | AM02 bare UNION | Optional rule | `SQBRSQL008` | conditional | Use `UNION DISTINCT` only in dialects that accept the explicit spelling. | | AM03 mixed ORDER direction | Optional rule | `SQBRSQL026` | never | Diagnosis is safe; multi-expression authored edits remain conservative. | -| AM04 unknown output width | Optional rule | `SQBRSQL021` | never | Column enumeration needs schema evidence. | +| AM04 unknown output width | Optional rule | `SQBRSQL021` | never | Column enumeration needs schema evidence; canonical `__empty_fixture()` input CTEs in SQL tests are compiler-controlled. | | AM05 implicit JOIN type | Optional rule | `SQBRSQL025` | always | Conditioned plain `JOIN` is spelled `INNER JOIN`. | | AM06 mixed ordinal/name references | Optional rule | `SQBRSQL011` | never | Rewriting ordinals needs resolved projection names. | | AM07 set branch arity | Core compiler | `SQBRSQL020` | never | Invalid branch shape; intended schema requires input. | diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py index 10eba26326..30b56e3067 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py @@ -83,6 +83,7 @@ from sqlbuild.compiler.compile.constants import ( MACRO_CALL_PATTERN, MODEL_FULL_REFRESH_CONFIG_KEY, + MODEL_SCHEMA_CONFIG_KEY, ) from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.compile.models import ( @@ -586,8 +587,10 @@ def _build_model_inputs( ) expanded_config: CompileModelConfig = CompileModelConfig( values=hook_expansion.values, + model_header_keys=effective_config.model_header_keys, matched_path_default=effective_config.matched_path_default, logical_schema=effective_config.logical_schema, + layer_schema=effective_config.layer_schema, logical_database=effective_config.logical_database, time_travel_retention=effective_config.time_travel_retention, table_type=effective_config.table_type, @@ -1040,9 +1043,13 @@ def build_model_config(*, request: ModelConfigBuildRequest) -> CompileModelConfi ) else: model_resolved_values = layered_values - raw_logical_schema: object | None = model_resolved_values.get("schema") + raw_logical_schema: object | None = model_resolved_values.get(MODEL_SCHEMA_CONFIG_KEY) raw_logical_database: object | None = model_resolved_values.get("database") logical_schema: str | None = raw_logical_schema if isinstance(raw_logical_schema, str) else None + layer_schema_is_configured: bool = MODEL_SCHEMA_CONFIG_KEY in model_header_values or ( + matched_path_default is not None + and MODEL_SCHEMA_CONFIG_KEY in path_defaults[matched_path_default] + ) logical_database: str | None = ( raw_logical_database if isinstance(raw_logical_database, str) else None ) @@ -1096,8 +1103,10 @@ def build_model_config(*, request: ModelConfigBuildRequest) -> CompileModelConfi validate_model_config_has_no_macros(values=target_resolved_values) return CompileModelConfig( values=target_resolved_values, + model_header_keys=tuple(sorted(model_header_values)), matched_path_default=matched_path_default, logical_schema=logical_schema, + layer_schema=logical_schema if layer_schema_is_configured else None, logical_database=logical_database, time_travel_retention=retention, table_type=table_type, diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py index fd724c627f..67e92c85ed 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py @@ -330,8 +330,10 @@ def strip_model_header_metadata_from_config(config: CompileModelConfig) -> Compi return config return CompileModelConfig( values=filtered_values, + model_header_keys=config.model_header_keys, matched_path_default=config.matched_path_default, logical_schema=config.logical_schema, + layer_schema=config.layer_schema, logical_database=config.logical_database, time_travel_retention=config.time_travel_retention, table_type=config.table_type, @@ -556,8 +558,10 @@ def _merge_schema_tags( ) return CompileModelConfig( values=merged_values, + model_header_keys=config.model_header_keys, matched_path_default=config.matched_path_default, logical_schema=config.logical_schema, + layer_schema=config.layer_schema, logical_database=config.logical_database, time_travel_retention=config.time_travel_retention, table_type=config.table_type, diff --git a/src/sqlbuild/compiler/compile/constants.py b/src/sqlbuild/compiler/compile/constants.py index 5843706b0b..2bf88a5daa 100644 --- a/src/sqlbuild/compiler/compile/constants.py +++ b/src/sqlbuild/compiler/compile/constants.py @@ -136,3 +136,4 @@ COMPACT_ANALYSIS_RESPONSE_LENGTH: int = 2 COMPACT_ANALYSIS_FACT_LENGTH: int = 6 COMPACT_ANALYSIS_SOURCE_LENGTH: int = 3 +MODEL_SCHEMA_CONFIG_KEY: str = "schema" diff --git a/src/sqlbuild/compiler/compile/models.py b/src/sqlbuild/compiler/compile/models.py index aba90f651b..bc9e8571f3 100644 --- a/src/sqlbuild/compiler/compile/models.py +++ b/src/sqlbuild/compiler/compile/models.py @@ -201,8 +201,10 @@ class CompileModelConfig: """Pre-semantic effective config layers attached to a model input.""" values: dict[str, object] = field(default_factory=dict) + model_header_keys: tuple[str, ...] = field(default_factory=tuple) matched_path_default: str | None = None logical_schema: str | None = None + layer_schema: str | None = None logical_database: str | None = None time_travel_retention: ResolvedTimeTravelRetention = field( default_factory=ResolvedTimeTravelRetention diff --git a/src/sqlbuild/lint/_helpers/native_sql.py b/src/sqlbuild/lint/_helpers/native_sql.py index a47b599724..a18b081d4e 100644 --- a/src/sqlbuild/lint/_helpers/native_sql.py +++ b/src/sqlbuild/lint/_helpers/native_sql.py @@ -69,6 +69,7 @@ tuple[str, ...], bool, bool, + bool, ] type _NativeResult = dict[str, Any] | NativeLintError @@ -106,6 +107,8 @@ def run_native_sql_lint( payload["allows_ceremonial_select"] = True if body.allows_dynamic_output_star: payload["allows_dynamic_output_star"] = True + if body.allows_empty_fixture_star: + payload["allows_empty_fixture_star"] = True requests[cache_key] = payload response_cache: dict[_NativeCacheKey, _NativeResult] = _native_responses(requests=requests) @@ -146,6 +149,7 @@ def _native_cache_key(*, body: LintBody, config: LintConfig) -> _NativeCacheKey: body.externally_referenced_ctes, body.allows_ceremonial_select, body.allows_dynamic_output_star, + body.allows_empty_fixture_star, ) diff --git a/src/sqlbuild/lint/main/run_lint.py b/src/sqlbuild/lint/main/run_lint.py index 8c36a82172..afcc6c1500 100644 --- a/src/sqlbuild/lint/main/run_lint.py +++ b/src/sqlbuild/lint/main/run_lint.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import replace from pathlib import Path from sqlbuild.compiler.compile.constants import MODEL_DIRECTORY_NAME @@ -134,6 +135,7 @@ def _prepared_bodies( allows_ceremonial_select: bool = any( header.kind in {HEADER_KIND_TEST, HEADER_KIND_SCENARIO} for header in headers ) + allows_empty_fixture_star: bool = any(header.kind == HEADER_KIND_TEST for header in headers) body_start: int body_end: int for body_start, body_end in lint_body_ranges( @@ -143,17 +145,20 @@ def _prepared_bodies( project_dir=project_dir, ): bodies.append( - prepare_lint_body( - project_dir=project_dir, - file_path=file_path, - contents=contents, - body_start=body_start, - body_end=body_end, - context=context, - external_identifiers=external_identifiers, - allows_ceremonial_select=allows_ceremonial_select, - allows_dynamic_output_star=allows_dynamic_output_star, - compiled_expansion=compiled_expansion, + replace( + prepare_lint_body( + project_dir=project_dir, + file_path=file_path, + contents=contents, + body_start=body_start, + body_end=body_end, + context=context, + external_identifiers=external_identifiers, + allows_ceremonial_select=allows_ceremonial_select, + allows_dynamic_output_star=allows_dynamic_output_star, + compiled_expansion=compiled_expansion, + ), + allows_empty_fixture_star=allows_empty_fixture_star, ) ) return tuple(bodies) diff --git a/src/sqlbuild/lint/models.py b/src/sqlbuild/lint/models.py index ccebbe0b77..c0079f988a 100644 --- a/src/sqlbuild/lint/models.py +++ b/src/sqlbuild/lint/models.py @@ -46,6 +46,7 @@ class LintBody: externally_referenced_ctes: tuple[str, ...] = () allows_ceremonial_select: bool = False allows_dynamic_output_star: bool = False + allows_empty_fixture_star: bool = False @dataclass(frozen=True) diff --git a/src/sqlbuild/rule_engine/_helpers/engine/config.py b/src/sqlbuild/rule_engine/_helpers/engine/config.py index fb676628e2..3cb0774f4c 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/config.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/config.py @@ -12,6 +12,7 @@ from sqlbuild.rule_engine._helpers.engine.native import load_native_config from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.models import ( + GraphEdgeExclusion, LayoutConfig, RuleExemption, RuleIgnore, @@ -47,6 +48,14 @@ def load_rules_config(project_dir: Path) -> RulesConfig: ) for item in _tables(payload.get("rule_exceptions")) ), + graph_edge_exceptions=tuple( + GraphEdgeExclusion( + consumer=str(item["consumer"]), + dependency=str(item["dependency"]), + reason=str(item["reason"]), + ) + for item in _tables(payload.get("graph_edge_exceptions")) + ), rule_ignores=tuple( RuleIgnore( rules=_strings(item.get("rules")), diff --git a/src/sqlbuild/rule_engine/_helpers/engine/native.py b/src/sqlbuild/rule_engine/_helpers/engine/native.py index b850a6e9f1..fdbf9e72ca 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/native.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/native.py @@ -461,6 +461,8 @@ def _model_payload( "query_sql": model.query_sql, "authored_sql": model.authored_query_sql or model.authored_sql, "config": model.config.values, + "authored_config_keys": list(model.config.model_header_keys), + "logical_schema": model.config.layer_schema, "references": [ { "ref_kind": str(reference.ref_kind), diff --git a/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py b/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py index 4942d97ced..5df5356bce 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py @@ -90,6 +90,7 @@ def resolve_ruleset(*, config: RulesConfig, project_dir: Path) -> ResolvedRulese "sql_tests": config.sql_tests, "layout": config.layout, "exceptions": config.rule_exceptions, + "graph_edge_exceptions": config.graph_edge_exceptions, "ignores": config.rule_ignores, } fingerprint: str = hashlib.sha256( diff --git a/src/sqlbuild/rule_engine/_helpers/guidance/skills.py b/src/sqlbuild/rule_engine/_helpers/guidance/skills.py index f230a11609..7b29d67f74 100644 --- a/src/sqlbuild/rule_engine/_helpers/guidance/skills.py +++ b/src/sqlbuild/rule_engine/_helpers/guidance/skills.py @@ -48,7 +48,12 @@ def render_skills(*, config: RulesConfig, project_dir: Path) -> tuple[str, str]: } if effective: body.extend(("", f"Effective options: {effective}")) - if config.rule_exceptions or config.rule_ignores or config.select_star_allow: + if ( + config.rule_exceptions + or config.graph_edge_exceptions + or config.rule_ignores + or config.select_star_allow + ): body.extend( ( "", @@ -66,6 +71,11 @@ def render_skills(*, config: RulesConfig, project_dir: Path) -> tuple[str, str]: body.append( f"- Ignore `{','.join(entry.rules)}` at `{','.join(scopes)}`: {entry.reason}" ) + for entry in config.graph_edge_exceptions: + body.append( + f"- `SQBRGRAPH101` exact edge `{entry.consumer}` -> " + f"`{entry.dependency}`: {entry.reason}" + ) for entry in config.select_star_allow: body.append(f"- Lone-star allowance `{','.join(entry.paths)}`: {entry.reason}") if any( @@ -103,8 +113,14 @@ def _rule_example(*, rule: Rule) -> str: examples: dict[str, str] = { "SQBRMODEL101": 'WITH upstream AS (SELECT * FROM __ref("domain__stg__entity")), ...', "SQBRMODEL102": "SELECT id, status FROM final; use a lone SELECT * only when allowed.", - "SQBRGRAPH101": "stg -> int_clean -> int_enriched -> mart; skipping layers is valid.", + "SQBRMODEL103": "A view uses stg_v, int_v, or mart_v; a table uses a non-v layer.", + "SQBRMODEL104": "MODEL() omits schema; central path defaults provide it.", + "SQBRGRAPH101": ( + "Internal flow is stg -> int_clean -> int_enriched; marts are published interfaces." + ), "SQBRPROJECT101": "domain__int_clean__entity or domain__mart_v__entity.", + "SQBRPROJECT105": "A domain__int_enriched__entity model lives under intermediate/enriched.", + "SQBRPROJECT106": "A domain__stg__entity model resolves to the staging logical schema.", "SQBRCONTRACT101": ( "MODEL (contract enforced, columns (...)); declare authoritative columns." ), diff --git a/src/sqlbuild/rule_engine/_helpers/run/rules.py b/src/sqlbuild/rule_engine/_helpers/run/rules.py index 3149e33871..7e419e4806 100644 --- a/src/sqlbuild/rule_engine/_helpers/run/rules.py +++ b/src/sqlbuild/rule_engine/_helpers/run/rules.py @@ -93,7 +93,11 @@ def evaluate_rules( ) selected_project: CompiledProject = _selected_project(graph=graph, model_paths=model_paths) selected_config: RulesConfig = replace( - _selection_rules_config(config=effective_config, model_paths=model_paths), + _selection_rules_config( + config=effective_config, + model_paths=model_paths, + project=selected_project, + ), select=tuple(rule.code for rule in selected), ignore=(), ) @@ -210,15 +214,24 @@ def _prepare_sql_lint( def _selection_rules_config( - *, config: RulesConfig, model_paths: frozenset[str] | None + *, + config: RulesConfig, + model_paths: frozenset[str] | None, + project: CompiledProject, ) -> RulesConfig: if model_paths is None: return config + selected_model_names: frozenset[str] = frozenset(model.name for model in project.models) return replace( config, rule_exceptions=tuple( entry for entry in config.rule_exceptions if Path(entry.path).as_posix() in model_paths ), + graph_edge_exceptions=tuple( + entry + for entry in config.graph_edge_exceptions + if entry.consumer in selected_model_names + ), ) diff --git a/src/sqlbuild/rule_engine/models.py b/src/sqlbuild/rule_engine/models.py index 79cfa8c786..01308ba36b 100644 --- a/src/sqlbuild/rule_engine/models.py +++ b/src/sqlbuild/rule_engine/models.py @@ -226,6 +226,15 @@ class RuleExemption: reason: str +@dataclass(frozen=True) +class GraphEdgeExclusion: + """One exact, temporary internal-layer dependency exception.""" + + consumer: str + dependency: str + reason: str + + @dataclass(frozen=True) class RuleIgnore: rules: tuple[str, ...] @@ -276,6 +285,7 @@ class RulesConfig: threshold_overrides: tuple[ThresholdOverride, ...] = () rule_options: dict[str, dict[str, RuleOptionValue]] = field(default_factory=dict) rule_exceptions: tuple[RuleExemption, ...] = () + graph_edge_exceptions: tuple[GraphEdgeExclusion, ...] = () rule_ignores: tuple[RuleIgnore, ...] = () select_star_allow: tuple[SelectStarAllow, ...] = () domains: tuple[str, ...] = () diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py index bec3a98fa5..6f9d7b7ebf 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py @@ -13,6 +13,67 @@ ) +@pytest.mark.parametrize( + "test_case", + [ + FormatCompileIntegrationTestCase( + description="canonical empty input fixture remains compiler-compatible", + expected_literal="SELECT * FROM __empty_fixture()", + ) + ], + ids=lambda case: case.description, +) +def test_given_typed_null_input_when_fixture_formatting_then_canonical_empty_fixture_compiles( + test_case: FormatCompileIntegrationTestCase, + tmp_path: Path, +) -> None: + """Prove formatter output remains valid with uncontrolled-star lint selected.""" + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRSQL021"]\n', + encoding="utf-8", + ) + models: Path = tmp_path / "models" + models.mkdir() + (models / "stg_orders.sql").write_text( + "MODEL (\n" + ' description "Staged orders",\n' + " contract enforced,\n" + " columns (\n" + " order_id (type INTEGER, nullable true),\n" + " order_label (type VARCHAR, nullable true),\n" + " ),\n" + ");\n\n" + "SELECT CAST(1 AS INTEGER) AS order_id, CAST('new' AS VARCHAR) AS order_label\n", + encoding="utf-8", + ) + (models / "orders.sql").write_text( + 'MODEL (description "Orders");\n\nSELECT order_id, order_label FROM __ref("stg_orders")\n', + encoding="utf-8", + ) + test_file: Path = tmp_path / "tests" / "unit" / "test_orders.sql" + test_file.parent.mkdir(parents=True) + test_file.write_text( + "TEST();\n\n" + "WITH\n" + "__ref__stg_orders AS (\n" + " SELECT\n" + " CAST(NULL AS INTEGER) AS order_id,\n" + " CAST(NULL AS VARCHAR) AS order_label\n" + " WHERE FALSE\n" + "),\n" + "__expected__orders AS (SELECT 1 AS order_id, 'new' AS order_label)\n" + "SELECT 1\n", + encoding="utf-8", + ) + + format_exit: int = main(["--project-dir", str(tmp_path), "format", "--fixtures-only"]) + compile_exit: int = main(["--project-dir", str(tmp_path), "compile", "--no-cache"]) + + assert format_exit == 0 + assert compile_exit == 0 + assert test_case.expected_literal in test_file.read_text(encoding="utf-8") + + @pytest.mark.parametrize( "test_case", [ diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index 75a5d9e53d..670c9fefdd 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -19,6 +19,138 @@ ) +@pytest.mark.parametrize( + "test_case", + [ + RulePassIntegrationTestCase( + description="conflicting model metadata reports independent boundaries", + expected_exit_code=1, + expected_diagnostics=( + "SQBRMODEL103", + "SQBRMODEL104", + "SQBRPROJECT101", + "SQBRPROJECT104", + "SQBRPROJECT105", + "SQBRPROJECT106", + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_conflicting_model_layer_metadata_when_compiling_then_rules_report_each_boundary( + test_case: RulePassIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Exercise model-header provenance and layer alignment through the real compiler.""" + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "commerce"\nadapter = "duckdb"\n\n' + "[rules]\n" + 'select = ["SQBRMODEL103", "SQBRMODEL104", "SQBRPROJECT101", ' + '"SQBRPROJECT104", "SQBRPROJECT105", "SQBRPROJECT106"]\n', + encoding="utf-8", + ) + model_dir: Path = tmp_path / "models" / "commerce" / "intermediate" / "enriched" + model_dir.mkdir(parents=True) + (model_dir / "commerce__mart_v__int_v_orders.sql").write_text( + "MODEL (\n" + ' description "Published orders",\n' + " materialized table,\n" + " schema staging,\n" + ");\n\n" + 'SELECT order_id FROM __ref("commerce__mart__stg_orders")\n', + encoding="utf-8", + ) + published_dir: Path = tmp_path / "models" / "commerce" / "mart" + published_dir.mkdir() + (published_dir / "commerce__mart__stg_orders.sql").write_text( + 'MODEL (description "Orders");\nSELECT 1 AS order_id\n', encoding="utf-8" + ) + + exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + result: dict[str, object] = json.loads(capsys.readouterr().out) + diagnostics: list[dict[str, object]] = cast(list[dict[str, object]], result["diagnostics"]) + codes: set[object] = {diagnostic["code"] for diagnostic in diagnostics} + + assert exit_code == test_case.expected_exit_code + assert set(test_case.expected_diagnostics) <= codes + + +@pytest.mark.parametrize( + "test_case", + [ + RulesIntegrationTestCase( + description="live exact edge passes and stale edge fails", + expected_exit_code=0, + expected_code="SQBRGRAPH101", + ) + ], + ids=lambda case: case.description, +) +def test_given_exact_internal_edge_exception_when_compiling_then_live_edge_passes_and_stale_fails( + test_case: RulesIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Exercise exact graph exceptions and stale detection through TOML configuration.""" + project_file: Path = tmp_path / "sqlbuild_project.toml" + base_config = 'name = "commerce"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRGRAPH101"]\n\n' + live_exception = ( + "[[rules.graph_edge_exceptions]]\n" + 'consumer = "commerce__stg__orders"\n' + 'dependency = "commerce__int_enriched__orders"\n' + 'reason = "Tracked cleanup"\n' + ) + project_file.write_text(base_config + live_exception, encoding="utf-8") + models: Path = tmp_path / "models" + models.mkdir() + (models / "commerce__stg__orders.sql").write_text( + 'MODEL ();\nSELECT order_id FROM __ref("commerce__int_enriched__orders")\n', + encoding="utf-8", + ) + (models / "commerce__int_enriched__orders.sql").write_text( + "MODEL ();\nSELECT 1 AS order_id\n", encoding="utf-8" + ) + (models / "commerce__mart__summary.sql").write_text( + "MODEL ();\nSELECT 1 AS order_count\n", encoding="utf-8" + ) + + live_exit: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + live_result: dict[str, object] = json.loads(capsys.readouterr().out) + focused_exit: int = main( + [ + "--project-dir", + str(tmp_path), + "compile", + "--json", + "--no-cache", + "--select", + "commerce__mart__summary", + ] + ) + focused_result: dict[str, object] = json.loads(capsys.readouterr().out) + project_file.write_text( + base_config + + live_exception.replace( + 'dependency = "commerce__int_enriched__orders"', + 'dependency = "commerce__int_clean__missing"', + ), + encoding="utf-8", + ) + stale_exit: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + stale_result: dict[str, object] = json.loads(capsys.readouterr().out) + + assert live_exit == test_case.expected_exit_code + assert not cast(list[object], live_result["diagnostics"]) + assert focused_exit == 0 + assert not cast(list[object], focused_result["diagnostics"]) + assert stale_exit == 1 + assert any( + diagnostic["code"] == test_case.expected_code and "stale" in str(diagnostic["message"]) + for diagnostic in cast(list[dict[str, object]], stale_result["diagnostics"]) + ) + + @pytest.mark.parametrize( "test_case", [RulePassIntegrationTestCase("constant-backed numeric decision", 0, ())], diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py index 86731f001a..57ad62bd63 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py @@ -46,6 +46,8 @@ "SQBRPROJECT102", "SQBRPROJECT103", "SQBRPROJECT104", + "SQBRPROJECT105", + "SQBRPROJECT106", "SQBRPROJECT201", "SQBRPROJECT202", "SQBRPROJECT203", @@ -53,6 +55,7 @@ "SQBRMODEL101", "SQBRMODEL102", "SQBRMODEL103", + "SQBRMODEL104", "SQBRTEST101", "SQBRTEST102", "SQBRTEST103", @@ -68,6 +71,7 @@ "SQBRMODEL101", "SQBRMODEL102", "SQBRMODEL103", + "SQBRMODEL104", ) SQL_TEST_CODES: tuple[str, ...] = ( "SQBRTEST101", @@ -103,6 +107,8 @@ "SQBRPROJECT102", "SQBRPROJECT103", "SQBRPROJECT104", + "SQBRPROJECT105", + "SQBRPROJECT106", "SQBRPROJECT201", "SQBRPROJECT202", "SQBRPROJECT203",