diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/constants.rs index 81f80b8efa..eef0addb63 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/constants.rs @@ -18,6 +18,8 @@ pub(crate) const RULE_FIX_AVAILABLE_NOTE: &str = pub(crate) const RULES_NATIVE_API_VERSION: i64 = 1; /// File name of the custom-rule result cache under the project cache directory. pub(crate) const CUSTOM_RULES_CACHE_FILE: &str = "custom-rules.json"; +/// Bulk cache subdirectory of a selected run's whole-project pass, kept apart from its slice's. +pub(crate) const WHOLE_PROJECT_CACHE_DIRECTORY: &str = "whole-project"; /// Suffix of a custom host scratch directory abandoned by a cancelled run. pub(crate) const CUSTOM_HOST_ABANDONED_SUFFIX: &str = ".abandoned"; /// Marker file whose presence tells split custom hosts the run was cancelled. diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/custom_evaluation.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/custom_evaluation.rs index b137b22c4d..1d6cf5b789 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/custom_evaluation.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/custom_evaluation.rs @@ -35,7 +35,7 @@ use crate::bindings::_helpers::rules::custom_rule_evidence::custom_rule_implemen use crate::bindings::_helpers::rules::custom_rule_evidence::custom_rule_import_closure; use crate::bindings::_helpers::rules::fact_encoder::{Encoded, reproducible}; use crate::bindings::_helpers::rules::fact_replay::{FactReplay, is_tree_fact}; -use crate::bindings::_helpers::rules::rule_run::rules_bulk_cache_path; +use crate::bindings::_helpers::rules::rule_run::bulk_cache_file; const PROJECT_SUBJECT: &str = "project"; const MODEL_SUBJECT: &str = "model"; @@ -49,6 +49,8 @@ pub(crate) struct CustomRun<'a, 'py> { pub(crate) verify_determinism: bool, pub(crate) implementation_fingerprints: HashMap, pub(crate) cancel: HostCancel, + /// The bulk cache subdirectory of the result cache, the bulk cache itself when `None`. + pub(crate) cache_directory: Option<&'a str>, } /// Reuse results whose recorded fact reads are unchanged and evaluate only the rest. @@ -88,8 +90,9 @@ pub(crate) fn evaluate_custom_rules_cached<'py>( rows.push((code.clone(), identity, subject_value(rule)?)); } let cache_path: Option = if enabled { - Some(rules_bulk_cache_path( - run.project_dir.extract()?, + Some(bulk_cache_file( + &run.project_dir.extract::()?, + run.cache_directory, CUSTOM_RULES_CACHE_FILE, )) } else { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs index d069c5a342..7edea16723 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs @@ -24,7 +24,6 @@ use sqlbuild_rules::models::{ Declaration, DeclarationMember, EvaluateRequest, Fault, Model, ModelRow, ModelRowsContext, RowsEvaluation, RowsRequest, RulesConfig, }; -use sqlbuild_rules::rule_cache::main::rules_bulk_cache_path::rules_bulk_cache_path; use crate::bindings::_helpers::boundary::keyword_request::Request; use crate::bindings::_helpers::boundary::panics::{compiler_guard, value_error}; @@ -42,6 +41,7 @@ use crate::bindings::_helpers::rules::config_json::asdict_json; use crate::bindings::_helpers::rules::early_builtin_rules::EarlyBuiltinRules; use crate::bindings::_helpers::rules::fact_replay::{project_attribute, project_models}; use crate::bindings::_helpers::rules::findings::{FindingClasses, finding_fault}; +use crate::bindings::_helpers::rules::rule_run::bulk_cache_file; use crate::bindings::_helpers::rules::selection::rules_error; use crate::bindings::_helpers::rules::sql_test_objects::{scenario_fact, sql_test_fact}; use crate::bindings::_helpers::scopes::scope_reports::metadata_projection_value; @@ -157,9 +157,9 @@ pub(crate) fn selectionless_rules_request( custom_payloads: &Bound<'_, PyAny>, ) -> PyResult { let py: Python<'_> = config.py(); - let header: PyResult = dialect - .into_bound_py_any(py) - .and_then(|dialect| request_header(config, project_dir, &dialect, (None, custom_payloads))); + let header: PyResult = dialect.into_bound_py_any(py).and_then(|dialect| { + request_header(config, project_dir, &dialect, (None, custom_payloads, None)) + }); as_rules_error( py, header.and_then(|header| { @@ -178,12 +178,16 @@ pub(crate) fn selectionless_rules_request( ) } -/// The request's version, paths, dialect, config, custom rules and, when given, scope index. +/// The request's version, paths, dialect, config, custom rules, cache files and scope index. fn request_header( config: &Bound<'_, PyAny>, project_dir: &Bound<'_, PyAny>, dialect: &Bound<'_, PyAny>, - (project, custom_payloads): (Option<&Bound<'_, PyAny>>, &Bound<'_, PyAny>), + (project, custom_payloads, cache_directory): ( + Option<&Bound<'_, PyAny>>, + &Bound<'_, PyAny>, + Option<&str>, + ), ) -> PyResult { let py: Python<'_> = config.py(); let resolved: Bound<'_, PyAny> = project_dir.call_method0("resolve")?; @@ -202,7 +206,7 @@ fn request_header( header["custom_rules"] = plain_value(custom_payloads)?; header["rules_cache_path"] = Value::String(path_text( py, - rules_bulk_cache_path(&resolved_path, NATIVE_RULES_CACHE_FILE), + bulk_cache_file(&resolved_path, cache_directory, NATIVE_RULES_CACHE_FILE), )?); Ok(header) } @@ -216,11 +220,19 @@ fn built_request( let config: Bound<'_, PyAny> = options.get("config")?; let project_dir: Bound<'_, PyAny> = options.get("project_dir")?; let dialect: Bound<'_, PyAny> = options.get("dialect")?; + let cache_directory: Option = match options.optional("cache_directory")? { + Some(directory) if !directory.is_none() => Some(directory.extract()?), + _ => None, + }; let header: Value = request_header( &config, &project_dir, &dialect, - (Some(&project), &options.get("custom_payloads")?), + ( + Some(&project), + &options.get("custom_payloads")?, + cache_directory.as_deref(), + ), )?; let (public_enums, public_constants) = declaration_rows( PyTuple::new( @@ -241,7 +253,11 @@ fn built_request( current_code_identity(py)?, path_text( py, - rules_bulk_cache_path(&unresolved, NATIVE_RULES_MEMO_FILE), + bulk_cache_file( + &unresolved, + cache_directory.as_deref(), + NATIVE_RULES_MEMO_FILE, + ), )?, )) } else { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_dispatch.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_dispatch.rs index 1681736db9..824501ea1d 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_dispatch.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_dispatch.rs @@ -11,7 +11,7 @@ use pyo3::exceptions::{PyKeyboardInterrupt, PyRuntimeError}; use pyo3::prelude::{ Bound, Py, PyAny, PyAnyMethods, PyErr, PyModule, PyModuleMethods, PyResult, Python, }; -use pyo3::types::{PyDict, PyTuple}; +use pyo3::types::{PyDict, PyTuple, PyTupleMethods}; use pyo3::{IntoPyObjectExt, pyfunction, wrap_pyfunction}; use sqlbuild_host::entries::models::UserCodeHost; @@ -23,12 +23,14 @@ use crate::bindings::_helpers::compiled_project::project::NativeCompiledProject; use crate::bindings::_helpers::native_host::host; use crate::bindings::_helpers::records::construct::new_record_with; use crate::bindings::_helpers::records::fields::field; +use crate::bindings::_helpers::records::fields::replaced; use crate::bindings::_helpers::records::ids::RecordId; use crate::bindings::_helpers::rules::catalogue::{ resolved_ignore_selectors, rule_catalogue, select_rules, }; use crate::bindings::_helpers::rules::constants::RULES_MEMO_REUSES; use crate::bindings::_helpers::rules::constants::TYPE_PROOF_RULE_CODES; +use crate::bindings::_helpers::rules::constants::WHOLE_PROJECT_CACHE_DIRECTORY; use crate::bindings::_helpers::rules::custom_evaluation::{ CustomRun, evaluate_custom_rules_cached, outcome, }; @@ -37,12 +39,14 @@ use crate::bindings::_helpers::rules::custom_rule_evidence::verify_custom_rules; use crate::bindings::_helpers::rules::early_builtin_rules::{ EarlyBuiltinRules, EarlyRulesReport, take_early_rules, }; +use crate::bindings::_helpers::rules::fact_replay::project_models; +use crate::bindings::_helpers::rules::results::rule_finding; use crate::bindings::_helpers::rules::rows::finalize_rule_findings_rows; use crate::bindings::_helpers::rules::rows::{ NativeRulesRequest, build_rules_request, evaluate_rules_request, evaluate_rules_request_with, rules_request_over, selectionless_rules_request, }; -use crate::bindings::_helpers::rules::rule_plan::plan_rule_run; +use crate::bindings::_helpers::rules::rule_plan::{plan_rule_run, whole_project_codes}; use crate::bindings::_helpers::rules::rule_run::finish_rule_run; use crate::bindings::_helpers::rules::selection::{rule_selected, selected_rule_codes}; use crate::bindings::_helpers::rules::sql_rules::run_planned_sql_rules; @@ -119,6 +123,37 @@ struct NativeRun<'py> { verify_determinism: bool, initial_findings: Option>, cancel: HostCancel, + /// Which of the selected custom rules this evaluation runs. + custom_scope: CustomScope, +} + +/// The custom rules one evaluation of a selected run runs, and where it caches. +enum CustomScope { + /// Every selected custom rule, caching in the bulk cache. + All, + /// The slice of a selection: every selected custom rule except these project-wide ones. + Slice(Vec), + /// The whole-project pass of a selection: only these, caching in its own directory. + WholeProject(Vec), +} + +impl CustomScope { + /// Whether this evaluation runs the selected custom rule `code`. + fn runs(&self, code: &String) -> bool { + match self { + Self::All => true, + Self::Slice(skipped) => !skipped.contains(code), + Self::WholeProject(codes) => codes.contains(code), + } + } + + /// The bulk cache subdirectory this evaluation caches in, the bulk cache itself when `None`. + fn cache_directory(&self) -> Option<&'static str> { + match self { + Self::WholeProject(_) => Some(WHOLE_PROJECT_CACHE_DIRECTORY), + Self::All | Self::Slice(_) => None, + } + } } /// How a native evaluation obtains its custom-rule outcome. @@ -164,6 +199,7 @@ fn evaluate_project_rules<'py>( verify_determinism, initial_findings: None, cancel: HostCancel::default(), + custom_scope: CustomScope::All, }; evaluate_native(&run, (false, None), CustomSource::Deferred, None) } @@ -188,6 +224,7 @@ fn evaluate_native_rules<'py>( verify_determinism: arguments.flag("verify_determinism")?, initial_findings: (!initial_findings.is_none()).then_some(initial_findings), cancel: HostCancel::default(), + custom_scope: CustomScope::All, }; evaluate_native( &run, @@ -223,6 +260,7 @@ fn evaluate_custom_rules<'py>( fingerprints.extract()? }, cancel: HostCancel::default(), + cache_directory: None, }) } @@ -245,6 +283,7 @@ pub(crate) fn evaluate_rules<'py>( KeywordArguments::read(py, "evaluate_rules", options, EVALUATE_RULES_OPTIONS)?; let graph: Bound<'py, PyAny> = arguments.value("graph")?; let dialect: Bound<'py, PyAny> = arguments.value("dialect")?; + let graph_project: Bound<'py, PyAny> = field(&graph, "project")?; let plan: Bound<'py, PyAny> = plan_rule_run(Some(&keywords( py, &[ @@ -258,6 +297,7 @@ pub(crate) fn evaluate_rules<'py>( ), ], )?))?; + let whole_codes: Vec = whole_project_codes(&plan)?; let run: NativeRun<'py> = NativeRun { project: field(&plan, "project")?, config: field(&plan, "config")?, @@ -267,6 +307,7 @@ pub(crate) fn evaluate_rules<'py>( verify_determinism: false, initial_findings: None, cancel: HostCancel::default(), + custom_scope: CustomScope::Slice(whole_codes.clone()), }; let payloads: Bound<'py, PyAny> = field(&plan, "custom_payloads")?; let custom: Option = start_custom_rules(&run, &payloads)?; @@ -279,7 +320,7 @@ pub(crate) fn evaluate_rules<'py>( ]; let native_dialect: String = run.dialect.clone(); let native_cancel: HostCancel = run.cancel.clone(); - let selectionless: bool = custom.is_none() && field(&plan, "selected")?.len()? == 0; + let selectionless: bool = custom.is_none() && field(&run.config, "select")?.len()? == 0; let early: Option = take_early_rules(); let early_report: Option> = early.as_ref().map(EarlyBuiltinRules::report); let native_thread: Option = if selectionless { @@ -300,6 +341,7 @@ pub(crate) fn evaluate_rules<'py>( verify_determinism: false, initial_findings: None, cancel: native_cancel, + custom_scope: CustomScope::All, }; evaluate_native( &run, @@ -331,9 +373,115 @@ pub(crate) fn evaluate_rules<'py>( } let sql_result: Bound<'py, PyAny> = sql_result?; let result: Bound<'py, PyAny> = result?; + let result: Bound<'py, PyAny> = if whole_codes.is_empty() { + result + } else { + with_whole_project_rules( + (&plan, &whole_codes), + (&graph_project, &payloads), + (&run, &result), + )? + }; finish_rule_run(&plan, &sql_result, &result) } +/// `codes` and every selected custom rule, which the custom-rule coverage rule checks. +fn with_custom_codes(plan: &Bound<'_, PyAny>, codes: &[String]) -> PyResult> { + let mut select: Vec = codes.to_vec(); + for rule in field(plan, "selected")?.try_iter()? { + let rule: Bound<'_, PyAny> = rule?; + let code: String = field(&rule, "code")?.extract()?; + if field(&rule, "custom")?.is_truthy()? && !codes.contains(&code) { + select.push(code); + } + } + Ok(select) +} + +/// `sliced` with `codes` evaluated over the whole project as compile does, kept to the slice. +fn with_whole_project_rules<'py>( + (plan, codes): (&Bound<'py, PyAny>, &[String]), + (project, payloads): (&Bound<'py, PyAny>, &Bound<'py, PyAny>), + (slice, sliced): (&NativeRun<'py>, &Bound<'py, PyAny>), +) -> PyResult> { + let py: Python<'py> = plan.py(); + let select: Vec = with_custom_codes(plan, codes)?; + let whole: NativeRun<'py> = NativeRun { + project: project.clone(), + config: replaced( + &field(plan, "effective_config")?, + &[ + ("select", PyTuple::new(py, select)?.into_any()), + ("ignore", PyTuple::empty(py).into_any()), + ], + )?, + project_dir: slice.project_dir.clone(), + catalogue: slice.catalogue.clone(), + dialect: slice.dialect.clone(), + verify_determinism: false, + initial_findings: None, + cancel: slice.cancel.clone(), + custom_scope: CustomScope::WholeProject(codes.to_vec()), + }; + let evaluated: PyResult> = evaluate_native( + &whole, + (true, Some(payloads.clone())), + CustomSource::Deferred, + None, + ); + cancel_on_interrupt(py, &evaluated, &whole.cancel); + let evaluated: Bound<'py, PyAny> = evaluated?; + let finalized: Bound<'py, PyTuple> = finalize_rule_findings_rows( + &whole.config, + &whole.project_dir, + codes.to_vec(), + field(&evaluated, "findings")? + .try_iter()? + .collect::>()?, + )?; + let project_wide: HashSet<&str> = codes.iter().map(String::as_str).collect(); + let mut findings: Vec> = Vec::new(); + for finding in field(sliced, "findings")?.try_iter()? { + let finding: Bound<'py, PyAny> = finding?; + if !project_wide.contains(field(&finding, "code")?.extract::()?.as_str()) { + findings.push(finding); + } + } + let selected: Bound<'py, PyAny> = field(plan, "model_paths")?; + let models: HashSet = project_models(project)? + .into_iter() + .map(|model| model.posix) + .collect(); + for finding in finalized.iter() { + let path: String = rule_finding(&finding)?.path; + if !models.contains(&path) || selected.contains(&path)? { + findings.push(finding); + } + } + let total = |name: &str| -> PyResult> { + (field(sliced, name)?.extract::()? + field(&evaluated, name)?.extract::()?) + .into_bound_py_any(py) + }; + new_record_with( + py, + RecordId::RulesResult, + &[ + ("findings", PyTuple::new(py, findings)?.into_any()), + ( + "evaluated_models", + project_models(&slice.project)? + .len() + .into_bound_py_any(py)?, + ), + ("cache_hits", total("cache_hits")?), + ("cache_misses", total("cache_misses")?), + ("built_in_ms", total("built_in_ms")?), + ("custom_ms", total("custom_ms")?), + ("custom_cpu_ms", total("custom_cpu_ms")?), + ], + ) +} + /// Evaluate built-in rules natively, or from `early` when identical, beside the custom rules. fn evaluate_native<'py>( run: &NativeRun<'py>, @@ -497,7 +645,7 @@ fn request_options<'py>( for code in TYPE_PROOF_RULE_CODES { include_type_proof = include_type_proof || rule_selected(&run.config, code)?; } - keywords( + let options: Bound<'py, PyDict> = keywords( py, &[ ("project", run.project.clone()), @@ -516,7 +664,11 @@ fn request_options<'py>( include_type_proof.into_bound_py_any(py)?, ), ], - ) + )?; + if let Some(directory) = run.custom_scope.cache_directory() { + options.set_item("cache_directory", directory)?; + } + Ok(options) } /// The request `evaluate_rules` would build for `plan`, reading model facts from `facts`. @@ -534,6 +686,7 @@ pub(crate) fn speculative_rules_request<'py>( verify_determinism: false, initial_findings: None, cancel: HostCancel::default(), + custom_scope: CustomScope::All, }; rules_request_over( &request_options(&run, &field(plan, "custom_payloads")?)?, @@ -556,8 +709,10 @@ fn start_custom_rules( let mut rules: Vec> = Vec::new(); for rule in run.catalogue.try_iter()? { let rule: Bound<'_, PyAny> = rule?; + let code: String = field(&rule, "code")?.extract()?; if field(&rule, "custom")?.is_truthy()? - && selected.contains(&field(&rule, "code")?.extract::()?) + && selected.contains(&code) + && run.custom_scope.runs(&code) { rules.push(rule.unbind()); } @@ -587,6 +742,7 @@ fn start_custom_rules( let dialect: String = run.dialect.clone(); let verify_determinism: bool = run.verify_determinism; let cancel: HostCancel = run.cancel.clone(); + let cache_directory: Option<&'static str> = run.custom_scope.cache_directory(); PhaseThread::spawn(CUSTOM_THREAD, move || { Python::attach(|py| { let [project, config, project_dir] = inputs; @@ -599,6 +755,7 @@ fn start_custom_rules( verify_determinism, implementation_fingerprints: fingerprints, cancel, + cache_directory, }) .map(Bound::unbind) }) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_plan.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_plan.rs index abce3d3f30..070528ed50 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_plan.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_plan.rs @@ -142,6 +142,21 @@ pub(crate) fn rule_run_plan<'py>( ) } +/// The selected project-wide codes a model selection still evaluates over the whole project. +pub(crate) fn whole_project_codes(plan: &Bound<'_, PyAny>) -> PyResult> { + let mut codes: Vec = Vec::new(); + if field(plan, "model_paths")?.is_none() { + return Ok(codes); + } + for rule in field(plan, "selected")?.try_iter()? { + let rule: Bound<'_, PyAny> = rule?; + if field(&rule, "project_wide")?.is_truthy()? { + codes.push(field(&rule, "code")?.extract()?); + } + } + Ok(codes) +} + /// The relative paths of the selected models, or `None` when the run evaluates every model. fn selected_model_paths<'py>( graph: &Bound<'py, PyAny>, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_run.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_run.rs index 3050ceeae5..0c59a06ca2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_run.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/rule_run.rs @@ -1,7 +1,7 @@ //! The finished result of one rules run, the bulk cache locations and freed-memory release. use std::collections::BTreeSet; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use pyo3::exceptions::PyException; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; @@ -20,6 +20,7 @@ use crate::bindings::_helpers::records::ids::RecordId; use crate::bindings::_helpers::rules::fact_replay::project_models; use crate::bindings::_helpers::rules::results::rule_finding; use crate::bindings::_helpers::rules::rows::finalize_rule_findings_rows; +use crate::bindings::_helpers::rules::rule_plan::whole_project_codes; const PATHLIB_MODULE: &str = "pathlib"; const CTYPES_MODULE: &str = "ctypes"; @@ -61,9 +62,13 @@ pub(crate) fn finish_rule_run<'py>( result: &Bound<'py, PyAny>, ) -> PyResult> { let py: Python<'py> = plan.py(); + let whole_project: Vec = whole_project_codes(plan)?; let mut evaluated_codes: Vec = Vec::new(); for rule in record_field(plan, "selected")?.try_iter()? { - evaluated_codes.push(record_field(&rule?, "code")?.extract()?); + let code: String = record_field(&rule?, "code")?.extract()?; + if !whole_project.contains(&code) { + evaluated_codes.push(code); + } } let mut combined: Vec> = Vec::new(); for findings in [ @@ -149,6 +154,18 @@ pub(crate) fn rules_bulk_cache_path(project_dir: PathBuf, file_name: &str) -> Pa bulk_cache_path(&project_dir, file_name) } +/// One bulk Rules cache file, inside `directory` of the bulk cache when given. +pub(crate) fn bulk_cache_file( + project_dir: &Path, + directory: Option<&str>, + file_name: &str, +) -> PathBuf { + match directory { + Some(directory) => bulk_cache_path(project_dir, directory).join(file_name), + None => bulk_cache_path(project_dir, file_name), + } +} + /// Hand freed glibc heap pages back to the OS; a no-op elsewhere or without `malloc_trim`. pub(crate) fn freed_memory_released(py: Python<'_>) -> PyResult<()> { if !cfg!(target_os = "linux") { diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md index 18b0c99bc1..c74480221a 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md @@ -24,6 +24,11 @@ compiler path and reject Rule findings before opening a warehouse connection. `sqb rules run` is intentionally focused. Use it to inspect one exact code or family, but run the normal compiler before treating the complete project as valid. +With `--select`, model-subject Rules evaluate only the selected models. Project-subject Rules, which +compare models with each other, still evaluate the whole project with every exception, as +`sqb compile` does, and then report only their findings on selected models and on files that are not +models. A selection never reports a project-subject finding that `sqb compile` would not. + ## Dependency-aware caching Rules cache under `target/rules-cache-native-v1`. A cache identity includes the Rule code, implementation and diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 8becb121ff..81d32c72aa 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -298,6 +298,7 @@ def build_rules_request( initial_findings: tuple[Finding, ...], custom_payloads: list[dict[str, object]], include_type_proof: bool, + cache_directory: str | None = None, ) -> NativeRulesRequest: ... def evaluate_rules_request(request: NativeRulesRequest) -> NativeRulesEvaluation: ... def emit_orjson_report(report: object) -> str: ... diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py index 4510bfa822..b6c548ef83 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py @@ -279,3 +279,27 @@ class PublishedLayerCase: rules_toml: str expected_findings: tuple[tuple[str, str], ...] expected_exit: int + + +@dataclass(frozen=True) +class SelectedProjectWideRuleCase: + """One selected plan whose project-wide Rules must agree with a whole-project compile.""" + + description: str + rules_toml: str + selection: tuple[str, ...] + expected_compile_findings: tuple[tuple[str, str], ...] + expected_plan_findings: tuple[tuple[str, str], ...] + expected_plan_exit: int + + +@dataclass(frozen=True) +class SelectedRuleCacheCase: + """One compile, selected plan and edited recompile that must share the built-in Rules cache.""" + + description: str + rules_toml: str + selection: tuple[str, ...] + edited_path: str + expected_entries: tuple[str, ...] + expected_recompile_cache_hits: int diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py index 5e480c12ff..7cfc9fb7f4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py @@ -675,3 +675,14 @@ def write_model_files(*, project_dir: Path, toml: str, files: tuple[tuple[str, s path: Path = project_dir / relative path.parent.mkdir(parents=True, exist_ok=True) path.write_text(text, encoding="utf-8") + + +def duckdb_rules_project_toml(*, project_dir: Path, rules_toml: str) -> str: + """Return a plannable DuckDB project's `sqlbuild_project.toml` with the given `[rules]` body.""" + + return ( + 'name = "orders"\nadapter = "duckdb"\ndefault_target = "dev"\n\n' + f'[connection]\ndatabase = "{project_dir / "orders.duckdb"}"\n\n' + '[targets.dev]\nschema = "dev"\n\n' + f"[rules]\n{rules_toml}" + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_selected_project_wide_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_selected_project_wide_rules.py new file mode 100644 index 0000000000..dda87ec3f5 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_selected_project_wide_rules.py @@ -0,0 +1,182 @@ +"""E2E coverage of project-wide Rules on a selected plan, which must agree with compile.""" + +from __future__ import annotations + +import json +import re +import subprocess +from pathlib import Path + +import pytest + +from sqlbuild._native import rules_bulk_cache_path +from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( + SelectedProjectWideRuleCase, + SelectedRuleCacheCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( + compile_diagnostics, + duckdb_rules_project_toml, + rule_cache_hits, + run_compile_cli, + run_unevaluated_rules_cli, + write_model_files, +) + +_FINDING_LINE: re.Pattern[str] = re.compile(r"^(\S+):\d+:\d+ \[(SQBR\w+)\] ", re.MULTILINE) +_RETURNS_PATH: str = "models/sales/mart/sales__mart__returns__finance.sql" +_REFUNDS_PATH: str = "models/sales/mart/sales__mart__refunds__audit.sql" +_ORDERS_PATH: str = "models/sales/mart/sales__mart__orders.sql" +_CUSTOMERS_PATH: str = "models/sales/mart/sales__mart__customers.sql" +_CONSUMER_PATH: str = "models/finance/staging/finance__stg_v__orders.sql" +_FILES: tuple[tuple[str, str], ...] = ( + ( + "models/sales/intermediate/clean/sales__int_clean__orders.sql", + "MODEL (description 'Clean orders.');\nSELECT 1 AS order_id\n", + ), + (_ORDERS_PATH, "MODEL (description 'Published orders.');\nSELECT 1 AS order_id\n"), + ( + _CUSTOMERS_PATH, + "MODEL (description 'Published customers.');\n" + 'SELECT o.order_id FROM __ref("sales__int_clean__orders") AS o\n', + ), + ( + "models/sales/mart/sales__mart__products.sql", + "MODEL (description 'Published products.');\nSELECT 1 AS order_id\n", + ), + ( + _RETURNS_PATH, + "MODEL (description 'Returns for finance.');\n" + 'SELECT o.order_id FROM __ref("sales__int_clean__orders") AS o\n', + ), + (_REFUNDS_PATH, "MODEL (description 'Refunds for audit.');\nSELECT 1 AS order_id\n"), + ( + _CONSUMER_PATH, + "MODEL (description 'Finance view of sales orders.');\n" + 'SELECT r.order_id FROM __ref("sales__mart__returns__finance") AS r\n' + 'JOIN __ref("sales__mart__refunds__audit") AS f ON r.order_id = f.order_id\n' + 'JOIN __ref("sales__mart__orders") AS o ON r.order_id = o.order_id\n', + ), +) +_PRODUCTS_PATH: str = "models/sales/mart/sales__mart__products.sql" +_CACHE_FILES: tuple[tuple[str, str], ...] = ( + (_ORDERS_PATH, "MODEL (description 'Published orders.');\nSELECT 1 AS order_id\n"), + (_CUSTOMERS_PATH, "MODEL (description 'Published customers.');\nSELECT 1 AS customer_id\n"), + (_PRODUCTS_PATH, "MODEL (description 'Published products.');\nSELECT 1 AS product_id\n"), +) +_QUALIFIED_IGNORES: str = ( + "\n[[rules.rule_ignores]]\n" + 'rules = ["SQBRPROJECT107"]\n' + f'paths = ["{_RETURNS_PATH}", "{_REFUNDS_PATH}"]\n' + 'reason = "Reviewed qualified variants."\n' +) + + +@pytest.mark.parametrize( + "test_case", + [ + SelectedProjectWideRuleCase( + description="a slice whose own majority differs keeps the whole-project shape", + rules_toml='select = ["SQBRPROJECT107"]\n' + _QUALIFIED_IGNORES, + selection=("+finance__stg_v__orders",), + expected_compile_findings=(), + expected_plan_findings=(), + expected_plan_exit=0, + ), + SelectedProjectWideRuleCase( + description="findings on models outside the slice are left out", + rules_toml='select = ["SQBRPROJECT107", "SQBRMODEL101"]\n', + selection=("sales__mart__returns__finance",), + expected_compile_findings=( + ("SQBRMODEL101", _CONSUMER_PATH), + ("SQBRMODEL101", _CUSTOMERS_PATH), + ("SQBRMODEL101", _RETURNS_PATH), + ("SQBRPROJECT107", _REFUNDS_PATH), + ("SQBRPROJECT107", _RETURNS_PATH), + ), + expected_plan_findings=( + ("SQBRMODEL101", _RETURNS_PATH), + ("SQBRPROJECT107", _RETURNS_PATH), + ), + expected_plan_exit=1, + ), + ], + ids=lambda case: case.description, +) +def test_given_selection_when_planning_then_project_wide_rules_match_whole_project_compile( + test_case: SelectedProjectWideRuleCase, + tmp_path: Path, +) -> None: + write_model_files( + project_dir=tmp_path, + toml=duckdb_rules_project_toml(project_dir=tmp_path, rules_toml=test_case.rules_toml), + files=_FILES, + ) + + compiled: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + plans: tuple[subprocess.CompletedProcess[str], ...] = tuple( + run_unevaluated_rules_cli(tmp_path, "plan", "--select", *test_case.selection) + for _ in ("cold", "warm") + ) + + compile_findings: tuple[tuple[str, str], ...] = tuple( + sorted({(str(item["code"]), str(item["path"])) for item in compile_diagnostics(compiled)}) + ) + assert compile_findings == test_case.expected_compile_findings, compiled.stdout + for planned in plans: + plan_output: str = planned.stdout + planned.stderr + plan_findings: tuple[tuple[str, str], ...] = tuple( + sorted({(code, path) for path, code in _FINDING_LINE.findall(plan_output)}) + ) + assert plan_findings == test_case.expected_plan_findings, plan_output + assert planned.returncode == test_case.expected_plan_exit, plan_output + assert "Traceback" not in plan_output, plan_output + + +@pytest.mark.parametrize( + "test_case", + [ + SelectedRuleCacheCase( + description="a selected plan keeps compile's model-rule cache for the next compile", + rules_toml='select = ["SQBRPROJECT107", "SQBRMODEL101"]\n', + selection=("sales__mart__orders",), + edited_path=_PRODUCTS_PATH, + expected_entries=(_CUSTOMERS_PATH, _ORDERS_PATH, _PRODUCTS_PATH), + expected_recompile_cache_hits=2, + ), + ], + ids=lambda case: case.description, +) +def test_given_compile_cache_when_planning_selection_then_compile_and_plan_share_model_rules( + test_case: SelectedRuleCacheCase, + tmp_path: Path, +) -> None: + write_model_files( + project_dir=tmp_path, + toml=duckdb_rules_project_toml(project_dir=tmp_path, rules_toml=test_case.rules_toml), + files=_CACHE_FILES, + ) + native_cache: Path = rules_bulk_cache_path(project_dir=tmp_path, file_name="native.json") + + compiled: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + after_compile: dict[str, object] = json.loads(native_cache.read_text(encoding="utf-8")) + planned: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "plan", "--select", *test_case.selection + ) + after_plan: dict[str, object] = json.loads(native_cache.read_text(encoding="utf-8")) + edited: Path = tmp_path / test_case.edited_path + _ = edited.write_text( + edited.read_text(encoding="utf-8").replace("SELECT 1", "SELECT 2"), encoding="utf-8" + ) + recompiled: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + assert planned.returncode == 0, planned.stdout + planned.stderr + assert recompiled.returncode == 0, recompiled.stdout + recompiled.stderr + assert tuple(sorted(after_compile["entries"])) == test_case.expected_entries # type: ignore[arg-type] + assert after_plan == after_compile + assert rule_cache_hits(recompiled) == test_case.expected_recompile_cache_hits, recompiled.stdout + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) 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 e123c6fd33..e44a6298cc 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -2649,7 +2649,7 @@ def two_models(*, project: Project, ctx: RuleContext) -> list[Finding]: full_exit: int = main(["--project-dir", str(tmp_path), "compile", "--json"]) full: dict[str, object] = json.loads(capsys.readouterr().out) - assert focused_exit == 1 + assert focused_exit == test_case.expected_exit_code assert full_exit == test_case.expected_exit_code assert full["diagnostics"] == [] diff --git a/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx b/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx index dbc194d6ca..8f5d93a55d 100644 --- a/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx +++ b/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx @@ -21,6 +21,11 @@ compiler path and reject Rule findings before opening a warehouse connection. `sqb rules run` is intentionally focused. Use it to inspect one exact code or family, but run the normal compiler before treating the complete project as valid. +With `--select`, model-subject Rules evaluate only the selected models. Project-subject Rules, which +compare models with each other, still evaluate the whole project with every exception, as +`sqb compile` does, and then report only their findings on selected models and on files that are not +models. A selection never reports a project-subject finding that `sqb compile` would not. + ## Dependency-aware caching Rules cache under `target/rules-cache-native-v1`. A cache identity includes the Rule code, implementation and