fix!: batch of refactor, sink, audit, CLI and rules fixes (#1202)

* fix: move and rename a model's unit tests with sqb rename and sqb mv

A model's own unit test files, whose every TEST block targets only that
model, now follow it: test_<model>[__<behavior>].sql files take the new
name, TEST (name "<model>__<behavior>") headers take the new subject, and
files in the folder mirroring the model move to the folder mirroring the
destination. The edits are verified in the scratch compile like every
other edit, reported in --dry-run, and written only when verification
passes.

* fix: list USING joins and keep lambda parameters in audit-argument column renames

An audit argument that joins on the renamed column with USING or NATURAL
is now listed for a manual edit instead of failing the verification
compile. The column analyser treats a lambda parameter named like the
column as its own scope, so references in the lambda body are no longer
renamed to the outer column.

* fix: resume lifecycle sink delivery after one slow call

A sink call over the one-second timeout used to block that sink for the
rest of the command, silently dropping every later event including run
and invocation outcomes. Delivery now resumes on a fresh worker once the
slow call returns; events dropped meanwhile are still counted, outcome
events are held and delivered instead of dropped, and the command warns
when the stall begins. The timeout clock only charges calls the delivery
thread has started and caps each dispatcher round's charge, so time
spent waiting for the GIL is not blamed on the sink. The exit warning now
points at sink health and the shutdown deadline instead of only the
shutdown timeout.

* fix!: reject generic audit arguments the audit's SQL never uses

An attached generic audit that passes an argument neither its SQL nor its
evidence SQL reads, including a misspelt one, now fails the compile with
an error naming the audit, the argument to remove and the definition
file. Such arguments used to be ignored, so the audit silently checked
something other than what the attachment declared.

BREAKING CHANGE: projects whose audit attachments pass arguments the
audit never reads must remove those arguments.

* docs: document the rendered audit SQL compile writes under target/compiled/audits

sqb compile already writes each attached generic audit's rendered SQL to
target/compiled/audits/generic/<resource>/ and removes it with the
attachment, but nothing said so. Document the compiled artifact layout
and cover the audit files, and their cleanup, with an end-to-end test.

* fix: print expected CLI errors whose text holds undecodable bytes

A selector with undecodable bytes made sqb compile crash with a Unicode
error instead of reporting S007, in text mode and with --json. Expected
error text now keeps lone surrogates as \xNN or \uXXXX escapes before it
is printed or put in the JSON report.

* fix(rules): stop treating # as a Snowflake line comment

The Rules scanner skipped Snowflake code after #, which Snowflake does
not treat as a comment (only -- and //), so dependency calls there were
missed. The scanner now matches the Snowflake tokenizer.

* fix(rules): report SELECT * in plain set-operation branches

SQBRMODEL102 only inspected a query's body when it was a single SELECT,
so a star in any branch of UNION, INTERSECT or EXCEPT went unreported.
The rule now checks every plain SELECT branch of each query body.

* test: pin the S101 missing-database message to one settings example

* fix: keep an audit's other options in the 64-bit option help

The help for an oversized minimum_samples or evidence_limit showed the
audit call with only the offending option, so copying it dropped the
audit's name, severity and other options. It now repeats the whole call
with only the offending value replaced.

* fix(rules): exempt each dynamic-pivot SELECT * branch of a set operation

SQBRMODEL102 now checks every set-operation branch, but the dynamic-pivot
exemption was decided once per query and never matched a set-operation
body, so a union of dynamic-pivot star branches was reported per branch
although a lone dynamic-pivot star is exempt. The exemption now applies
to each branch.

* fix: keep delivering lifecycle events when the stall warning cannot be written

A failing stderr write of the blocked-sink warning ended the dispatcher
thread, leaving queued events undelivered and uncounted. The warning is
now best-effort.

* fix: keep a unit test in its folder when a move would change what it resolves

Test SQL resolves macros, enums and constants from the test file's own
folder, so moving a model's test to the destination's mirrored folder
could hide a folder-scoped test declaration, failing verification. A
test file now moves only when every scoped declaration it uses resolves
to the same definition from the destination folder; otherwise it is
renamed in place and the output's new notes section names the file and
the declaration. The JSON output gains a notes array.

* style: satisfy the doc-comment, parameter and test-shape checks for the batch

* test: expect escaped errors for undecodable selectors

Two integration tests pinned the previous crash (a TypeError) for a selector with undecodable bytes. Expected CLI errors now print that text escaped, so the tests assert the escaped S007 error and a valid ASCII JSON report.

Author: kvlonge

crates/sqlbuild-analysis/src/column_references/_helpers/references.rs

@@ -346,17 +346,52 @@ impl Analysis<'_> {
         let Some(body) = select_body(expression) else {
             return;
         };
+        let shadowed: HashSet<(usize, usize)> = self.lambda_parameter_uses(body);
         for node in walk_in_scope(body, false) {
             let Expression::Column(column) = node else {
                 continue;
             };
             if column.name.name == STAR || column.name.name.to_lowercase() != self.column {
                 continue;
             }
+            if column.table.is_none()
+                && column
+                    .span
+                    .is_some_and(|span| shadowed.contains(&(span.start, span.end)))
+            {
+                continue;
+            }
             self.record_column(column, select, chain, role);
         }
     }
 
+    /// Spans of unqualified uses of a lambda parameter named like the column, which read the parameter.
+    fn lambda_parameter_uses(&self, body: &Expression) -> HashSet<(usize, usize)> {
+        let mut shadowed: HashSet<(usize, usize)> = HashSet::new();
+        for node in walk_in_scope(body, false) {
+            let Expression::Lambda(lambda) = node else {
+                continue;
+            };
+            if !lambda
+                .parameters
+                .iter()
+                .any(|parameter| parameter.name.to_lowercase() == self.column)
+            {
+                continue;
+            }
+            for inner in walk_in_scope(&lambda.body, false) {
+                if let Expression::Column(column) = inner
+                    && column.table.is_none()
+                    && column.name.name.to_lowercase() == self.column
+                    && let Some(span) = column.span
+                {
+                    shadowed.insert((span.start, span.end));
+                }
+            }
+        }
+        shadowed
+    }
+
     fn record_column(
         &mut self,
         column: &polyglot_sql::expressions::Column,

crates/sqlbuild-analysis/src/column_references/tests/test_column_references.rs

@@ -76,6 +76,21 @@ fn given_column_uses_when_analyzing_then_reports_rename_sites() -> Result<(), St
             expected_output_aliases: &[],
             expected_outputs: 0,
         },
+        ColumnReferenceTestCase {
+            description: "a lambda parameter named like the column shadows it in the body",
+            sql: "SELECT list_filter([amount], amount -> amount > 0) AS kept, \
+                  list_filter([1], lambda amount: amount > 0) AS other FROM orders_fact",
+            output_ctes: &[],
+            expected_parsed: true,
+            expected_names: &["amount"],
+            expected_scopes: &["root"],
+            expected_bare_projections: 0,
+            expected_aliases: &[],
+            expected_star_scopes: &[],
+            expected_joins: 0,
+            expected_output_aliases: &[],
+            expected_outputs: 0,
+        },
         ColumnReferenceTestCase {
             description: "output scopes report aliased and bare projections of every branch",
             sql: "WITH fixture AS (SELECT 1 AS amount UNION ALL SELECT 2 AS amount) \

crates/sqlbuild-attachments/src/audits/_helpers/rendering.rs

@@ -1,6 +1,8 @@
 //! Python's argument merge, SQL rendering and policy resolution for one attachment.
 
-use crate::audits::_helpers::parameters::{RenderStop, render_parameterized_sql};
+use crate::audits::_helpers::parameters::{
+    RenderStop, parameter_matches, render_parameterized_sql,
+};
 use crate::audits::_helpers::policies::{resolve_run_scope, resolve_severity};
 use crate::audits::constants::{MODEL_RESOURCE, SEED_RESOURCE, SOURCE_RESOURCE};
 use crate::audits::models::{ArgumentValue, AuditAttachment, PolicySource};
@@ -94,3 +96,36 @@ pub(crate) fn merged_arguments(
     }
     Ok(merged)
 }
+
+/// The error for the first explicit argument neither the audit SQL nor its evidence SQL reads.
+pub(crate) fn unused_argument(attachment: &AuditAttachment) -> Option<String> {
+    let mut used: Vec<&str> = parameter_matches(&attachment.sql_body)
+        .into_iter()
+        .map(|item| item.name)
+        .collect();
+    if let Some(evidence) = &attachment.evidence_sql {
+        used.extend(
+            parameter_matches(evidence)
+                .into_iter()
+                .map(|item| item.name),
+        );
+    }
+    let implicit: Vec<&str> = attachment
+        .implicit_arguments
+        .iter()
+        .map(|(name, _)| name.as_str())
+        .collect();
+    let (name, _) = attachment
+        .explicit_arguments
+        .iter()
+        .find(|(name, _)| !used.contains(&name.as_str()) && !implicit.contains(&name.as_str()))?;
+    let definition: String = attachment
+        .definition_path
+        .as_deref()
+        .map_or_else(|| "its built-in definition".to_owned(), str::to_owned);
+    Some(format!(
+        "{} audit '{}' passes argument '{name}', which the audit's SQL in {definition} never \
+         uses; remove '{name}' from the attachment",
+        attachment.owner_label, attachment.definition_name
+    ))
+}

crates/sqlbuild-attachments/src/audits/main/render_attached_audit.rs

@@ -1,6 +1,6 @@
 //! Attached generic audit rendering: argument merge, SQL, severity and run scope.
 
-use crate::audits::_helpers::rendering::{merged_arguments, policies, rendered};
+use crate::audits::_helpers::rendering::{merged_arguments, policies, rendered, unused_argument};
 use crate::audits::models::{ArgumentValue, AuditAttachment, AuditRendering, RenderedAudit};
 
 /// Render one attachment, or the error rendering raises.
@@ -14,6 +14,9 @@ pub fn render_attached_audit(attachment: &AuditAttachment) -> AuditRendering {
             .as_deref()
             .map(|evidence| rendered(attachment, evidence, &arguments))
             .transpose()?;
+        if let Some(message) = unused_argument(attachment) {
+            return Err(message);
+        }
         Ok(RenderedAudit {
             sql_body,
             evidence_sql,

crates/sqlbuild-attachments/src/audits/models.rs

@@ -28,6 +28,8 @@ pub struct AuditAttachment {
     /// Python's `str(owner_file)` and the generic audit name, which its error messages quote.
     pub owner_label: String,
     pub definition_name: String,
+    /// The project file defining the audit, or `None` for a built-in audit.
+    pub definition_path: Option<String>,
     pub sql_body: String,
     pub evidence_sql: Option<String>,
     /// Arguments from the attached resource, such as `model` and `column`; always text.

crates/sqlbuild-attachments/src/audits/tests/helpers.rs

@@ -40,6 +40,7 @@ pub(super) fn attachment(
     AuditAttachment {
         owner_label: "models/schema.yml".to_owned(),
         definition_name: "accepted_values".to_owned(),
+        definition_path: None,
         sql_body: "SELECT * FROM t WHERE @column NOT IN (@'values')".to_owned(),
         evidence_sql: Some("SELECT @column FROM t".to_owned()),
         implicit_arguments: vec![("column".to_owned(), "status".to_owned())],

crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs

@@ -80,6 +80,28 @@ fn given_attached_audits_when_rendering_then_python_rendering_is_returned() {
                  implicit column from attached context",
             ),
         },
+        AttachedAuditTestCase {
+            description: "an argument the audit SQL never reads names itself for removal",
+            attachment: AuditAttachment {
+                definition_path: Some("audits/generic/accepted_values.sql".to_owned()),
+                ..attachment(
+                    vec![
+                        values.clone(),
+                        (
+                            "where_condition".to_owned(),
+                            ArgumentValue::Text("status <> 'void'".to_owned()),
+                        ),
+                    ],
+                    None,
+                    None,
+                )
+            },
+            expected_rendering: failed(
+                "models/schema.yml audit 'accepted_values' passes argument 'where_condition', \
+                 which the audit's SQL in audits/generic/accepted_values.sql never uses; remove \
+                 'where_condition' from the attachment",
+            ),
+        },
         AttachedAuditTestCase {
             description: "a missing argument is Python's error before the policies",
             attachment: attachment(Vec::new(), Some("fatal"), None),

crates/sqlbuild-cli/src/dispatch/main/event_export_warning.rs

@@ -1,5 +1,7 @@
 //! The stderr warning for lifecycle events dropped or failed at command exit.
 
+use sqlbuild_core::event_export::constants::DEFAULT_EVENT_EXPORT_INVOCATION_TIMEOUT_SECONDS;
+
 use crate::dispatch::constants::SHUTDOWN_TIMEOUT_SETTING;
 use crate::dispatch::models::{ExportSummary, ExporterLoss};
 
@@ -32,7 +34,9 @@ pub fn event_export_warning(summary: &ExportSummary, command_succeeded: bool) ->
     };
     Some(format!(
         "{prefix}: {} of {} {unit} dropped, {} failed ({sink_label} {names}). \
-         Increase {SHUTDOWN_TIMEOUT_SETTING} or check sink health.",
+         Check sink health: a sink call slower than {DEFAULT_EVENT_EXPORT_INVOCATION_TIMEOUT_SECONDS}s \
+         drops that sink's events until it returns; events still queued at exit need a longer \
+         {SHUTDOWN_TIMEOUT_SETTING}.",
         summary.dropped, summary.accepted, summary.failed
     ))
 }

crates/sqlbuild-cli/src/dispatch/tests/test_event_export_warning.rs

@@ -18,7 +18,9 @@ fn given_final_export_summary_when_formatting_warning_then_reports_loss_only() {
             expected_warning: Some(
                 "Warning: command completed successfully, but lifecycle event export was \
                  incomplete: 140 of 248 events dropped, 0 failed (sink 'orders_events'). \
-                 Increase sinks.lifecycle.shutdown_timeout or check sink health.",
+                 Check sink health: a sink call slower than 1s drops that sink's events until it \
+                 returns; events still queued at exit need a longer \
+                 sinks.lifecycle.shutdown_timeout.",
             ),
         },
         EventExportWarningTestCase {
@@ -28,7 +30,9 @@ fn given_final_export_summary_when_formatting_warning_then_reports_loss_only() {
             expected_warning: Some(
                 "Warning: lifecycle event export was incomplete: 0 of 10 events dropped, \
                  1 failed (sink 'orders_events'). \
-                 Increase sinks.lifecycle.shutdown_timeout or check sink health.",
+                 Check sink health: a sink call slower than 1s drops that sink's events until it \
+                 returns; events still queued at exit need a longer \
+                 sinks.lifecycle.shutdown_timeout.",
             ),
         },
         EventExportWarningTestCase {
@@ -41,7 +45,9 @@ fn given_final_export_summary_when_formatting_warning_then_reports_loss_only() {
             expected_warning: Some(
                 "Warning: command completed successfully, but lifecycle event export was \
                  incomplete: 4 of 40 event deliveries dropped, 1 failed (sink 'orders_events'). \
-                 Increase sinks.lifecycle.shutdown_timeout or check sink health.",
+                 Check sink health: a sink call slower than 1s drops that sink's events until it \
+                 returns; events still queued at exit need a longer \
+                 sinks.lifecycle.shutdown_timeout.",
             ),
         },
     ];

crates/sqlbuild-model-config/src/header_metadata/_helpers/audits.rs

@@ -90,12 +90,12 @@ fn configured_audit<N: AuthoredNode>(
     let minimum_samples = optional_count(
         entry(options, "minimum_samples"),
         option_site,
-        (definition, "minimum_samples"),
+        (definition, "minimum_samples", options),
     )?;
     let evidence_limit = optional_count(
         entry(options, "evidence_limit"),
         option_site,
-        (definition, "evidence_limit"),
+        (definition, "evidence_limit", options),
     )?;
     let meta = optional_meta(entry(options, "meta"), option_site)?;
     let arguments: Vec<(N, N)> = options