From a57dfe9283ec514dfe7fd3092b97d6dd8b21d864 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 1 Nov 2026 14:21:32 +0200 Subject: [PATCH 1/8] feat: add max_change_absolute to metric_stability max_change_percent is a relative change, which suits magnitudes whose scale varies by orders of magnitude between models. It does not suit ratios: a margin already sits on a fixed scale, so the move that transfers is the absolute one. The relative form also divides by the baseline, so its sensitivity is recomputed against each bucket's own prior value. A setting that means 20 basis points at a 45% margin means about 9bp at 20% and 27bp at 60%, so it drifts as the data drifts. max_change_absolute expresses the move directly and means the same thing at every level. Setting both fails only when both are exceeded, so the absolute one acts as a noise floor under the relative one. That needs max_change_percent to default to none rather than 0 so an unset value is distinguishable from an explicit zero; unset still behaves as 0 when no absolute threshold stands in for it. The zero-baseline branch is skipped when an absolute threshold is set. It exists only because relative change is undefined at zero, and absolute change is not, so a metric legitimately sitting at 0 is now held to the same tolerance as everything else rather than failing on any movement. Closes CORE-1664 Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 93 ++++++++++++++++++- .../monitors_query/metric_stability_query.sql | 68 ++++++++++++-- macros/edr/tests/test_metric_stability.sql | 57 ++++++++---- 3 files changed, 191 insertions(+), 27 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 1fe162313..dcbddfe15 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -32,8 +32,8 @@ def _rows( - restatements: Optional[Dict[int, int]] = None, - other_restatements: Optional[Dict[int, int]] = None, + restatements: Optional[Dict[int, float]] = None, + other_restatements: Optional[Dict[int, float]] = None, ) -> List[Dict[str, Any]]: """One row per day, midday so it lands unambiguously inside a daily bucket.""" restatements = restatements or {} @@ -306,6 +306,95 @@ def test_metric_stability_ignores_measurements_taken_while_settling( assert _run(dbt_project, test_id, _rows(), **args) == "pass" +# A ratio sits on a fixed scale, so the move that matters is the absolute one. +# 45.30% dropping to 45.10% is 20 basis points, which is what max_change_absolute +# expresses directly. +BASE_MARGIN = 45.30 +RATIO_ARGS: Dict[str, Any] = {"metrics": ["average"]} + + +def test_metric_stability_absolute_threshold_on_a_ratio( + test_id: str, dbt_project: DbtProject +): + """An absolute threshold means the same move at every level of the ratio.""" + args = {**RATIO_ARGS, "max_change_absolute": 0.20} + + assert ( + _run(dbt_project, test_id, _rows({d: BASE_MARGIN for d in range(1, 7)}), **args) + == "pass" + ) + assert ( + _run(dbt_project, test_id, _rows({d: BASE_MARGIN for d in range(1, 7)}), **args) + == "pass" + ) + + # 10bp, inside the tolerance. + rows = {d: BASE_MARGIN for d in range(1, 7)} + rows[SETTLED_DAYS_AGO] = 45.20 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # A further 30bp, outside it. + rows[SETTLED_DAYS_AGO] = 44.90 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +def test_metric_stability_both_thresholds_must_be_exceeded( + test_id: str, dbt_project: DbtProject +): + """With both set the absolute one is a floor under the relative one. + + A small absolute move on a small base looks large in relative terms, which + is exactly the noise an absolute floor is meant to suppress. + """ + args = {**RATIO_ARGS, "max_change_percent": 1.0, "max_change_absolute": 0.20} + low = 5.00 + + assert ( + _run(dbt_project, test_id, _rows({d: low for d in range(1, 7)}), **args) + == "pass" + ) + assert ( + _run(dbt_project, test_id, _rows({d: low for d in range(1, 7)}), **args) + == "pass" + ) + + # 0.10 is 2% of a base of 5, so the relative threshold alone would fire. It + # is under the absolute floor, so together they do not. + rows = {d: low for d in range(1, 7)} + rows[SETTLED_DAYS_AGO] = 5.10 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 0.50 clears the floor and is still well over 1%, so both agree. + rows[SETTLED_DAYS_AGO] = 5.60 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +def test_metric_stability_absolute_threshold_tolerates_a_zero_baseline( + test_id: str, dbt_project: DbtProject +): + """Relative change is undefined at zero; absolute change is not. + + With only a relative threshold any move away from exactly zero has to fail, + because the division has no meaning there. An absolute threshold is well + defined at zero, so a metric legitimately sitting at 0 is held to the same + tolerance as everything else. + """ + args = {**RATIO_ARGS, "max_change_absolute": 5} + rows = {d: 0 for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # A move away from zero inside the tolerance. A relative threshold would + # have had to fail this. + rows[SETTLED_DAYS_AGO] = 2 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # And the threshold still bites beyond it. + rows[SETTLED_DAYS_AGO] = 10 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 6b31c6049..c39b4bdd3 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -8,7 +8,8 @@ detection_end, days_back, min_bucket_age, - max_change_percent=0, + max_change_percent=none, + max_change_absolute=none, change_since=["last_check"], column_names=none, data_monitoring_metrics_table=none, @@ -52,10 +53,27 @@ and updated_at >= {{ settled_at_expr }} {% endset %} - {# Suppress floating-point aggregation noise; zero baselines are handled separately. #} + {# A relative threshold suits magnitudes, whose scale varies by orders of + magnitude between models. It does not suit ratios: a margin already sits + on a fixed scale, so the move that transfers is the absolute one. And + because the relative form divides by the baseline, its sensitivity is + recomputed per bucket, so a setting meaning 20bp at a 45% margin means + about 9bp at 20% and 27bp at 60%. max_change_absolute expresses the move + directly. Unset percent means 0 unless an absolute threshold stands in + for it, which is why the default is none rather than 0. #} + {%- set has_absolute = max_change_absolute is not none %} + {%- set has_percent = max_change_percent is not none or not has_absolute %} + {%- set effective_percent = ( + max_change_percent if max_change_percent is not none else 0 + ) %} + + {# Suppress floating-point aggregation noise. #} {%- set change_percent_noise_floor = 0.000000001 %} {%- set change_threshold = "%.10f" | format( - [max_change_percent, change_percent_noise_floor] | max + [effective_percent, change_percent_noise_floor] | max + ) %} + {%- set absolute_threshold = ( + "%.10f" | format(max_change_absolute) if has_absolute else none ) %} {%- set exceeds_conditions = [] %} @@ -64,12 +82,46 @@ {%- set baseline_column = baseline ~ "_value" %} {%- if baseline_column not in baseline_columns %} {%- do baseline_columns.append(baseline_column) %} + {%- set threshold_tests = [] %} + {%- if has_percent %} + {# Relative change is undefined at a zero baseline, so any move + away from zero counts. An absolute threshold is well defined + there and carries the comparison on its own. #} + {%- do threshold_tests.append( + "((" + ~ baseline_column + ~ " = 0 and measured_value != 0) or (" + ~ baseline_column + ~ " != 0 and " + ~ elementary.metric_stability_change_percent( + baseline_column + ) + ~ " > " + ~ change_threshold + ~ "))" + if not has_absolute + else "(" + ~ baseline_column + ~ " != 0 and " + ~ elementary.metric_stability_change_percent( + baseline_column + ) + ~ " > " + ~ change_threshold + ~ ")" + ) %} + {%- endif %} + {%- if has_absolute %} + {%- do threshold_tests.append( + "abs(measured_value - " + ~ baseline_column + ~ ") > " + ~ absolute_threshold + ) %} + {%- endif %} {% set exceeds_condition %} - ({{ baseline_column }} is not null and ( - ({{ baseline_column }} = 0 and measured_value != 0) - or ({{ baseline_column }} != 0 and - {{ elementary.metric_stability_change_percent(baseline_column) }} > {{ change_threshold }}) - )) + ({{ baseline_column }} is not null + and {{ threshold_tests | join(" and ") }}) {% endset %} {% do exceeds_conditions.append(exceeds_condition) %} {%- endif %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 5eab81833..96cdd7919 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -1,4 +1,11 @@ {# Detect restatements of settled bucket metrics within the observation window. + + Thresholds: max_change_percent is relative, which suits magnitudes whose + scale varies between models. max_change_absolute is the raw move, which + suits ratios that already sit on a fixed scale (20 basis points is 0.20 on a + 0-100 scale). Setting both fails only when both are exceeded, so the + absolute one acts as a noise floor under the relative one. + Configuration, baseline behavior, and limits: https://docs.elementary-data.com/data-tests/metric-stability #} {% test metric_stability( @@ -8,7 +15,8 @@ timestamp_column, min_bucket_age, change_since=["last_check"], - max_change_percent=0, + max_change_percent=none, + max_change_absolute=none, time_bucket=none, where_expression=none, days_back=none, @@ -33,6 +41,7 @@ min_bucket_age, change_since, max_change_percent, + max_change_absolute, days_back, backfill_days, ) %} @@ -96,6 +105,7 @@ days_back=days_back, min_bucket_age=min_bucket_age, max_change_percent=max_change_percent, + max_change_absolute=max_change_absolute, change_since=change_since, column_names=collected.columns, measurement_windows=collected.windows, @@ -362,6 +372,7 @@ min_bucket_age, change_since, max_change_percent, + max_change_absolute, days_back, backfill_days ) %} @@ -391,22 +402,34 @@ }} {%- endif %} - {%- if max_change_percent is not number %} - {{ - exceptions.raise_compiler_error( - "max_change_percent must be a number, got '" - ~ max_change_percent - ~ "'. Write it unquoted, e.g. max_change_percent: 25." - ) - }} - {%- endif %} - {%- if max_change_percent < 0 %} - {{ - exceptions.raise_compiler_error( - "max_change_percent must be non-negative." - ) - }} - {%- endif %} + {%- for arg_name, arg_value, example in [ + ("max_change_percent", max_change_percent, "25"), + ("max_change_absolute", max_change_absolute, "0.20"), + ] %} + {%- if arg_value is not none %} + {%- if arg_value is not number %} + {{ + exceptions.raise_compiler_error( + arg_name + ~ " must be a number, got '" + ~ arg_value + ~ "'. Write it unquoted, e.g. " + ~ arg_name + ~ ": " + ~ example + ~ "." + ) + }} + {%- endif %} + {%- if arg_value < 0 %} + {{ + exceptions.raise_compiler_error( + arg_name ~ " must be non-negative." + ) + }} + {%- endif %} + {%- endif %} + {%- endfor %} {%- for arg_name, arg_value in [ ("days_back", days_back), ("backfill_days", backfill_days), From a5dd1bd87906f3009a0fecd058209c9526834ea1 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 1 Nov 2026 14:48:16 +0200 Subject: [PATCH 2/8] fix: let the absolute threshold decide at a zero baseline Setting both thresholds suppressed every zero-baseline restatement. The relative conjunct was written as `baseline != 0 and ...`, which is false at zero, and ANDing it with the absolute test made the whole predicate false whatever the measured value. Adding max_change_absolute as a noise floor therefore switched off zero-baseline detection entirely, which a percent-only config still had. The relative guard is now satisfied rather than falsified at zero, so the absolute threshold decides there. The absolute path also had no float-noise guard: with only an absolute threshold the relative conjunct was not emitted at all, so max_change_absolute: 0 failed on re-aggregation noise. The relative guard is now always emitted, clamped to the noise floor. Rejects a max_change_absolute too small to survive the "%.10f" rendering, which silently became 0, and rejects booleans, which pass Jinja's `is number` because bool subclasses int. Adds the three cases the review found untested: a zero baseline with both thresholds set, an unset threshold still failing a sub-1% change, and an absolute threshold across both change_since baselines. Test names shortened to stay under the 63-character identifier limit Postgres applies to the seed table named after each test. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 61 ++++++++++++++++++- .../monitors_query/metric_stability_query.sql | 53 ++++++++-------- macros/edr/tests/test_metric_stability.sql | 21 ++++++- 3 files changed, 107 insertions(+), 28 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index dcbddfe15..beedeeccb 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -369,7 +369,7 @@ def test_metric_stability_both_thresholds_must_be_exceeded( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" -def test_metric_stability_absolute_threshold_tolerates_a_zero_baseline( +def test_metric_stability_absolute_tolerates_zero_baseline( test_id: str, dbt_project: DbtProject ): """Relative change is undefined at zero; absolute change is not. @@ -395,6 +395,65 @@ def test_metric_stability_absolute_threshold_tolerates_a_zero_baseline( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" +def test_metric_stability_both_detect_zero_baseline( + test_id: str, dbt_project: DbtProject +): + """At a zero baseline the absolute threshold has to decide on its own. + + Relative change has no meaning at zero, so a relative conjunct written as + is false there. ANDead with the absolute one it would + suppress every zero-baseline restatement, and adding an absolute threshold + as a noise floor would silently switch that detection off. + """ + args = {**RATIO_ARGS, "max_change_percent": 10, "max_change_absolute": 5} + rows = {d: 0 for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + rows[SETTLED_DAYS_AGO] = 1000 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +def test_metric_stability_unset_threshold_still_fails_any_change( + test_id: str, dbt_project: DbtProject +): + """An unconfigured threshold means zero, not "some small number". + + max_change_percent defaults to none internally so that unset is + distinguishable from an explicit zero. The other tests restate by a factor + of two, so they would still pass if unset silently resolved to anything + under 100%. This pins a change far below any plausible accidental default. + """ + assert _run(dbt_project, test_id, _rows()) == "pass" + assert _run(dbt_project, test_id, _rows()) == "pass" + + # 0.1% of the base amount, which must still fail with nothing configured. + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 100.1})) == "fail" + + +def test_metric_stability_absolute_on_both_baselines( + test_id: str, dbt_project: DbtProject +): + """Each baseline gets its own threshold pair, OR'd together.""" + args = { + **RATIO_ARGS, + "change_since": ["last_check", "first_check"], + "max_change_absolute": 0.20, + } + rows = {d: BASE_MARGIN for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 15bp per step: under the tolerance against the previous measurement, but + # first_check sees the total move from where the bucket started. + rows[SETTLED_DAYS_AGO] = 45.15 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + rows[SETTLED_DAYS_AGO] = 45.00 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index c39b4bdd3..ec13784e7 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -62,12 +62,13 @@ directly. Unset percent means 0 unless an absolute threshold stands in for it, which is why the default is none rather than 0. #} {%- set has_absolute = max_change_absolute is not none %} - {%- set has_percent = max_change_percent is not none or not has_absolute %} {%- set effective_percent = ( max_change_percent if max_change_percent is not none else 0 ) %} - {# Suppress floating-point aggregation noise. #} + {# Suppress floating-point aggregation noise. Re-aggregating the same rows + in a different order differs in the last bits, so the relative guard is + kept even when an absolute threshold is doing the real work. #} {%- set change_percent_noise_floor = 0.000000001 %} {%- set change_threshold = "%.10f" | format( [effective_percent, change_percent_noise_floor] | max @@ -82,42 +83,44 @@ {%- set baseline_column = baseline ~ "_value" %} {%- if baseline_column not in baseline_columns %} {%- do baseline_columns.append(baseline_column) %} + {%- set change_percent = elementary.metric_stability_change_percent( + baseline_column + ) %} {%- set threshold_tests = [] %} - {%- if has_percent %} - {# Relative change is undefined at a zero baseline, so any move - away from zero counts. An absolute threshold is well defined - there and carries the comparison on its own. #} + {%- if has_absolute %} + {# Relative change is undefined at a zero baseline, so the + relative guard has to be satisfied there rather than + falsified, or the AND below would suppress every zero + baseline and the absolute threshold would never decide. #} {%- do threshold_tests.append( - "((" - ~ baseline_column - ~ " = 0 and measured_value != 0) or (" - ~ baseline_column - ~ " != 0 and " - ~ elementary.metric_stability_change_percent( - baseline_column - ) - ~ " > " - ~ change_threshold - ~ "))" - if not has_absolute - else "(" + "(" ~ baseline_column - ~ " != 0 and " - ~ elementary.metric_stability_change_percent( - baseline_column - ) + ~ " = 0 or " + ~ change_percent ~ " > " ~ change_threshold ~ ")" ) %} - {%- endif %} - {%- if has_absolute %} {%- do threshold_tests.append( "abs(measured_value - " ~ baseline_column ~ ") > " ~ absolute_threshold ) %} + {%- else %} + {# With no absolute threshold to decide at zero, any move away + from exactly zero counts. #} + {%- do threshold_tests.append( + "((" + ~ baseline_column + ~ " = 0 and measured_value != 0) or (" + ~ baseline_column + ~ " != 0 and " + ~ change_percent + ~ " > " + ~ change_threshold + ~ "))" + ) %} {%- endif %} {% set exceeds_condition %} ({{ baseline_column }} is not null diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 96cdd7919..efcffff44 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -4,7 +4,9 @@ scale varies between models. max_change_absolute is the raw move, which suits ratios that already sit on a fixed scale (20 basis points is 0.20 on a 0-100 scale). Setting both fails only when both are exceeded, so the - absolute one acts as a noise floor under the relative one. + absolute one acts as a noise floor under the relative one. At a zero + baseline, where relative change has no meaning, the absolute threshold + decides on its own. Configuration, baseline behavior, and limits: https://docs.elementary-data.com/data-tests/metric-stability #} @@ -407,7 +409,9 @@ ("max_change_absolute", max_change_absolute, "0.20"), ] %} {%- if arg_value is not none %} - {%- if arg_value is not number %} + {#- bool subclasses int in python, so `is number` alone lets + `max_change_percent: true` through as a threshold of 1. -#} + {%- if arg_value is not number or arg_value is boolean %} {{ exceptions.raise_compiler_error( arg_name @@ -428,6 +432,19 @@ ) }} {%- endif %} + {#- Thresholds render through "%.10f", so anything smaller than + that silently becomes 0, turning a deliberately tight tolerance + into "any change at all". -#} + {%- if arg_value > 0 and "%.10f" | format(arg_value) == "0.0000000000" %} + {{ + exceptions.raise_compiler_error( + arg_name + ~ " is smaller than the smallest value this test can express (" + ~ arg_value + ~ "). Use at least 0.0000000001, or rescale the metric." + ) + }} + {%- endif %} {%- endif %} {%- endfor %} {%- for arg_name, arg_value in [ From 4844d7b53675e7dc216bd94f894af45f32c8dbd2 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 1 Nov 2026 15:58:45 +0200 Subject: [PATCH 3/8] fix: address second review round on max_change_absolute Null-safe division. The zero-baseline guard reads `baseline = 0 or `, and SQL does not promise the left side is evaluated first, so an engine that evaluates both would raise on the division. nullif makes the expression NULL at a zero baseline instead. This also covers the pre-existing `baseline != 0 and ` form, which had the same exposure. A tolerance under the absolute comparison. A decimal threshold has no exact float representation, so two moves that are both 0.20 in decimal land either side of it: 45.30 -> 45.10 is 0.19999999999999574 and 45.30 -> 45.50 is 0.20000000000000284. Without it the customer's own 20bp example fires in one direction and not the other. Validation now names a cutoff that matches what the query does. A percent below the noise floor was accepted and then silently clamped to it, and the old message quoted a minimum that was not the real one. Adds tests for the rejected threshold values, which had none, and for the float boundary. The boundary test is skipped where metric_value is stored at single precision: dbt's type_float is double on some adapters and single on others, and the same subtraction gives 0.20000000000000284 on Postgres against 0.20000076293945312 on DuckDB and ClickHouse. That limit is now documented, since it bounds what an absolute threshold can mean. Repairs a test docstring mangled by an unquoted heredoc and by the typos hook rewriting ANDed. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 79 ++++++++++++++++++- .../monitors_query/metric_stability_query.sql | 18 ++++- macros/edr/tests/test_metric_stability.sql | 31 +++++--- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index beedeeccb..8f9bb3090 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -400,10 +400,10 @@ def test_metric_stability_both_detect_zero_baseline( ): """At a zero baseline the absolute threshold has to decide on its own. - Relative change has no meaning at zero, so a relative conjunct written as - is false there. ANDead with the absolute one it would - suppress every zero-baseline restatement, and adding an absolute threshold - as a noise floor would silently switch that detection off. + Relative change has no meaning at zero, so a relative conjunct of the form + "baseline != 0 and ..." is false there. Combined with the absolute one under + AND it would suppress every zero-baseline restatement, so adding an absolute + threshold as a noise floor would silently switch that detection off. """ args = {**RATIO_ARGS, "max_change_percent": 10, "max_change_absolute": 5} rows = {d: 0 for d in range(1, 7)} @@ -454,6 +454,77 @@ def test_metric_stability_absolute_on_both_baselines( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" +@pytest.mark.parametrize( + "overrides", + [ + {"max_change_absolute": -1}, + {"max_change_absolute": "0.20"}, + {"max_change_absolute": True}, + {"max_change_absolute": 0.00000000001}, + {"max_change_percent": -1}, + {"max_change_percent": "25"}, + {"max_change_percent": True}, + {"max_change_percent": 0.0000000001}, + ], +) +def test_metric_stability_rejects_bad_thresholds( + test_id: str, dbt_project: DbtProject, overrides: Dict[str, Any] +): + """Every threshold value that cannot mean what it says must raise. + + Booleans matter because bool subclasses int in python, so Jinja's `number` + test accepts True and would render it as a threshold of 1. The tiny values + matter because one is clamped to the noise floor and the other rounds to + zero, either of which would apply a threshold nobody asked for. + """ + assert _run(dbt_project, test_id, _rows(), **overrides) == "error" + + +# metric_value is stored through dbt's type_float, which is single precision on +# some adapters and double on others: the same subtraction gives +# 0.20000000000000284 on Postgres and 0.20000076293945312 on DuckDB and +# ClickHouse. Where it is single precision the storage error is four orders of +# magnitude above the tolerance below, so a move sitting exactly on the threshold +# is not decidable and the assertion would be meaningless rather than wrong. The +# list errs towards over-skipping; the behaviour under test is the tolerance, and +# that is exercised wherever metric_value is double precision. +@pytest.mark.skip_targets( + [ + "clickhouse", + "duckdb", + "spark", + "fabricspark", + "databricks_catalog", + "athena", + "trino", + ] +) +def test_metric_stability_absolute_float_boundary( + test_id: str, dbt_project: DbtProject +): + """A move of exactly the threshold must not fire, in either direction. + + Decimal thresholds have no exact float representation, so two moves that are + both 0.20 in decimal land either side of it: 45.30 -> 45.10 comes out as + 0.19999999999999574 and 45.30 -> 45.50 as 0.20000000000000284. Without a + tolerance the second fires and the first does not, which is the customer's + own 20bp example behaving differently depending on which way it moved. + """ + args = {**RATIO_ARGS, "max_change_absolute": 0.20} + rows = {d: BASE_MARGIN for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # Exactly 20bp up, the direction that rounds against us. + rows[SETTLED_DAYS_AGO] = 45.50 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 30bp further, comfortably past the threshold. + rows[SETTLED_DAYS_AGO] = 45.80 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index ec13784e7..6a6baa562 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -73,8 +73,15 @@ {%- set change_threshold = "%.10f" | format( [effective_percent, change_percent_noise_floor] | max ) %} + {#- A decimal threshold has no exact float representation, so two moves that + are both 0.20 in decimal land either side of it: 45.30 -> 45.10 is + 0.19999999999999574 and 45.50 -> 45.30 is 0.20000000000000284. Without a + tolerance the customer's own 20bp example fires in one direction and not + the other. The slack is far below any move anyone would configure. -#} {%- set absolute_threshold = ( - "%.10f" | format(max_change_absolute) if has_absolute else none + "%.10f" | format(max_change_absolute * (1 + 0.000000001)) + if has_absolute + else none ) %} {%- set exceeds_conditions = [] %} @@ -268,6 +275,13 @@ Relative change from a baseline column, in percentage points. Shared by the WHERE predicate and the reported columns so the two cannot drift apart. #} +{# Relative change from a baseline, in percentage points. + + nullif keeps this NULL rather than raising at a zero baseline. SQL does not + guarantee that the `= 0` guards around the call are evaluated first, so on an + engine that evaluates both sides of an OR the division would raise. #} {% macro metric_stability_change_percent(baseline_column) -%} - abs(measured_value - {{ baseline_column }}) / abs({{ baseline_column }}) * 100.0 + abs(measured_value - {{ baseline_column }}) + / nullif(abs({{ baseline_column }}), 0) + * 100.0 {%- endmacro %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index efcffff44..abbc99f67 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -8,6 +8,14 @@ baseline, where relative change has no meaning, the absolute threshold decides on its own. + An absolute threshold is limited by how precisely the warehouse stores a + metric. metric_value goes through dbt's type_float, which is double + precision on some adapters and single on others (DuckDB and ClickHouse + among them). At single precision a value around 45 carries roughly 1e-5 of + representation error, so a threshold within a few orders of magnitude of + that measures storage noise rather than the data. A relative threshold is + unaffected, since the error scales with the value. + Configuration, baseline behavior, and limits: https://docs.elementary-data.com/data-tests/metric-stability #} {% test metric_stability( @@ -404,9 +412,13 @@ }} {%- endif %} - {%- for arg_name, arg_value, example in [ - ("max_change_percent", max_change_percent, "25"), - ("max_change_absolute", max_change_absolute, "0.20"), + {#- Smallest value each threshold can actually act on. Below the percent + noise floor the query clamps the value, and below the absolute minimum + it renders as 0, so either would silently mean something other than what + was written. -#} + {%- for arg_name, arg_value, example, minimum in [ + ("max_change_percent", max_change_percent, "25", 0.000000001), + ("max_change_absolute", max_change_absolute, "0.20", 0.0000000001), ] %} {%- if arg_value is not none %} {#- bool subclasses int in python, so `is number` alone lets @@ -432,16 +444,17 @@ ) }} {%- endif %} - {#- Thresholds render through "%.10f", so anything smaller than - that silently becomes 0, turning a deliberately tight tolerance - into "any change at all". -#} - {%- if arg_value > 0 and "%.10f" | format(arg_value) == "0.0000000000" %} + {%- if arg_value > 0 and arg_value < minimum %} {{ exceptions.raise_compiler_error( arg_name - ~ " is smaller than the smallest value this test can express (" + ~ " is " ~ arg_value - ~ "). Use at least 0.0000000001, or rescale the metric." + ~ ", below the smallest value it can act on (" + ~ minimum + ~ "). Anything smaller would be silently rounded up or" + ~ " down rather than applied. Use a larger value, or" + ~ " rescale the metric." ) }} {%- endif %} From e857018524ddb9a99eee71b235d61784e426b655 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 1 Nov 2026 17:33:13 +0200 Subject: [PATCH 4/8] fix: render thresholds at full precision Thresholds rendered through "%.10f", which dropped the 1e-9 tolerance for any value below roughly 0.05. That included 0.0020, the 0-1 scale form the PR recommends for 20bp: 8518 of 8801 exactly-20bp moves over margins 0.10 to 0.98 fired on double-precision warehouses, while the identical move on a 0-100 scale did not. Render through `string` instead, python's shortest round-tripping form, so the literal is the double that was configured. With that, an absolute threshold is used exactly as given, so its minimum had no remaining justification and is dropped. The percent minimum stays: the query still clamps a small percent up to the noise floor, which is genuinely silent, and the error message now says so. Also, per review: state the single-precision storage limit where the thresholds are built rather than only in the test file, merge the two stacked doc comments on metric_stability_change_percent, and drop the null and zero guards that nullif inside the helper made redundant. Postgres 59 passed; DuckDB and ClickHouse 32 passed, 2 skipped. The new unit-scale test was confirmed to fail with the old rendering restored. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 46 +++++++++++++++++-- .../monitors_query/metric_stability_query.sql | 37 ++++++++++----- macros/edr/tests/test_metric_stability.sql | 21 +++++---- 3 files changed, 79 insertions(+), 25 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 8f9bb3090..344df2431 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -460,7 +460,6 @@ def test_metric_stability_absolute_on_both_baselines( {"max_change_absolute": -1}, {"max_change_absolute": "0.20"}, {"max_change_absolute": True}, - {"max_change_absolute": 0.00000000001}, {"max_change_percent": -1}, {"max_change_percent": "25"}, {"max_change_percent": True}, @@ -473,9 +472,10 @@ def test_metric_stability_rejects_bad_thresholds( """Every threshold value that cannot mean what it says must raise. Booleans matter because bool subclasses int in python, so Jinja's `number` - test accepts True and would render it as a threshold of 1. The tiny values - matter because one is clamped to the noise floor and the other rounds to - zero, either of which would apply a threshold nobody asked for. + test accepts True and would render it as a threshold of 1. The tiny percent + matters because the query clamps it up to the noise floor, applying a + threshold nobody asked for. There is no equivalent absolute case: an + absolute threshold renders at full precision and is used as given. """ assert _run(dbt_project, test_id, _rows(), **overrides) == "error" @@ -525,6 +525,44 @@ def test_metric_stability_absolute_float_boundary( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" +# Single precision loses this outright: see the note on the test above. +@pytest.mark.skip_targets( + [ + "clickhouse", + "duckdb", + "spark", + "fabricspark", + "databricks_catalog", + "athena", + "trino", + ] +) +def test_metric_stability_absolute_boundary_on_unit_scale( + test_id: str, dbt_project: DbtProject +): + """The same move must behave the same however the metric is scaled. + + 20bp is 0.20 on a 0-100 scale and 0.0020 on a 0-1 scale. Rendering the + threshold with a fixed ten decimals kept the tolerance on the first and + dropped it on the second, so 0.1000 -> 0.0980 (a diff of + 0.0020000000000000018) fired while the identical move on the other scale + did not. + """ + args = {**RATIO_ARGS, "max_change_absolute": 0.0020} + rows = {d: 0.1000 for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # Exactly 20bp down, the direction that rounds against us. + rows[SETTLED_DAYS_AGO] = 0.0980 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 30bp further, comfortably past the threshold. + rows[SETTLED_DAYS_AGO] = 0.0950 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 6a6baa562..627c17fbe 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -60,7 +60,15 @@ recomputed per bucket, so a setting meaning 20bp at a 45% margin means about 9bp at 20% and 27bp at 60%. max_change_absolute expresses the move directly. Unset percent means 0 unless an absolute threshold stands in - for it, which is why the default is none rather than 0. #} + for it, which is why the default is none rather than 0. + + How small an absolute threshold can usefully be is bounded by how the + warehouse stores metric_value, which goes through dbt's type_float: + double precision on most adapters, single on some (DuckDB and ClickHouse + among them). At single precision a value around 45 carries roughly 1e-5 + of storage error, so a move sitting within that of the threshold is not + decidable there and may be reported in one direction but not the other. + The tolerance below addresses decimal representation, not storage. #} {%- set has_absolute = max_change_absolute is not none %} {%- set effective_percent = ( max_change_percent if max_change_percent is not none else 0 @@ -70,8 +78,14 @@ in a different order differs in the last bits, so the relative guard is kept even when an absolute threshold is doing the real work. #} {%- set change_percent_noise_floor = 0.000000001 %} - {%- set change_threshold = "%.10f" | format( - [effective_percent, change_percent_noise_floor] | max + {#- Thresholds render through `string`, which is python's shortest + round-tripping form, so the literal in the SQL is the same double the + test was configured with. A fixed number of decimals would not be: at + "%.10f" the slack below survives on a 0-100 scale but vanishes on a 0-1 + scale, which made identical moves behave differently depending on how + the metric happened to be scaled. -#} + {%- set change_threshold = ( + [effective_percent, change_percent_noise_floor] | max | string ) %} {#- A decimal threshold has no exact float representation, so two moves that are both 0.20 in decimal land either side of it: 45.30 -> 45.10 is @@ -79,7 +93,7 @@ tolerance the customer's own 20bp example fires in one direction and not the other. The slack is far below any move anyone would configure. -#} {%- set absolute_threshold = ( - "%.10f" | format(max_change_absolute * (1 + 0.000000001)) + (max_change_absolute * (1 + 0.000000001)) | string if has_absolute else none ) %} @@ -256,12 +270,14 @@ first_check_value as initial_value, case when is_current = 1 then measured_value - last_check_value end as change_since_last_check, case when is_current = 1 then measured_value - first_check_value end as change_since_first_check, + {#- nullif inside the helper already yields NULL at a zero or + missing baseline, so is_current is the only guard left. -#} case - when is_current = 1 and last_check_value is not null and last_check_value != 0 + when is_current = 1 then {{ elementary.metric_stability_change_percent("last_check_value") }} end as change_percent_since_last_check, case - when is_current = 1 and first_check_value is not null and first_check_value != 0 + when is_current = 1 then {{ elementary.metric_stability_change_percent("first_check_value") }} end as change_percent_since_first_check from latest_measurement @@ -274,12 +290,11 @@ {# Relative change from a baseline column, in percentage points. Shared by the WHERE predicate and the reported columns so the two cannot drift apart. -#} -{# Relative change from a baseline, in percentage points. - nullif keeps this NULL rather than raising at a zero baseline. SQL does not - guarantee that the `= 0` guards around the call are evaluated first, so on an - engine that evaluates both sides of an OR the division would raise. #} + nullif keeps this NULL rather than raising at a zero baseline. SQL does not + guarantee that a `= 0` guard around the call is evaluated first, so on an + engine that evaluates both sides of an OR the division would raise. +#} {% macro metric_stability_change_percent(baseline_column) -%} abs(measured_value - {{ baseline_column }}) / nullif(abs({{ baseline_column }}), 0) diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index abbc99f67..35867c9cd 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -412,13 +412,13 @@ }} {%- endif %} - {#- Smallest value each threshold can actually act on. Below the percent - noise floor the query clamps the value, and below the absolute minimum - it renders as 0, so either would silently mean something other than what - was written. -#} + {#- A percent below the noise floor is clamped up to it by the query, so + it would silently mean something other than what was written. An + absolute threshold has no such floor: it renders at full precision and + is used as given, so any positive value is honoured. -#} {%- for arg_name, arg_value, example, minimum in [ ("max_change_percent", max_change_percent, "25", 0.000000001), - ("max_change_absolute", max_change_absolute, "0.20", 0.0000000001), + ("max_change_absolute", max_change_absolute, "0.20", none), ] %} {%- if arg_value is not none %} {#- bool subclasses int in python, so `is number` alone lets @@ -444,17 +444,18 @@ ) }} {%- endif %} - {%- if arg_value > 0 and arg_value < minimum %} + {%- if minimum is not none and arg_value > 0 and arg_value < minimum %} {{ exceptions.raise_compiler_error( arg_name ~ " is " ~ arg_value - ~ ", below the smallest value it can act on (" + ~ ", below the floating-point noise floor this test" + ~ " applies (" ~ minimum - ~ "). Anything smaller would be silently rounded up or" - ~ " down rather than applied. Use a larger value, or" - ~ " rescale the metric." + ~ "). Anything smaller would be raised to the floor" + ~ " rather than applied. Use a larger value, use" + ~ " max_change_absolute, or rescale the metric." ) }} {%- endif %} From 442f1d3ae5c8dee31f5258ce4c713ac809274a20 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 2 Nov 2026 09:46:02 +0200 Subject: [PATCH 5/8] fix: separate the noise floor from the comparison tolerance Two CI failures, both in test data. A column of 5.00 is written as 5.0, which agate normalizes to zero decimals, so the seed column became INT64 on BigQuery, integer on Athena and bigint on Dremio and failed to load. That test now uses integral data. Dremio also seeds a fractional column as a bare decimal and reads it at scale 0, so the tests whose data cannot be integral now skip it. The predicate ANDed a fixed 1e-9 percent floor onto every comparison, absolute-only ones included, so any move under |baseline| x 1e-11 was dropped whatever max_change_absolute said: a move of 2 on a daily sum of 2.5e11 went unreported with a threshold of 1. Separately the boundary tolerance was a fixed fraction of the threshold, but the representation error it absorbs grows with the operands, so from about 1e6 x the threshold the same move was reported in one direction only. Both legs are now `moved > threshold * scale` with no division, and the two errors are separate terms: a noise floor that stands in for an absolute threshold only when none was given, and a tolerance scaled by the operands. That removes the zero-baseline escape, the nullif in the predicate, and the second hand-built predicate that let the zero-baseline bug in. It also makes the relative leg symmetric, which it never was. Validation: reject inf, reject an explicit max_change_percent: null rather than letting it mean 0, and accept numeric strings, because dbt-core reads YAML 1.1 where `max_change_absolute: 2e-3` arrives as a string. The percent floor check is gone with the clamp it described. Tests: assert the error names the argument, so a case that would have errored anyway still proves validation ran; cover the relative leg applying when both thresholds are set; pin the zero baseline to first_check so it is the thing deciding. Each new test was confirmed to fail with the fix it guards stubbed out. max_change_absolute is one number for every column, metric and dimension a test covers, so it only means something when they share a scale. Noted in the header. The claim that a relative threshold is unaffected by storage precision was untrue and is gone: metric_value is 4 bytes on several adapters, which is CORE-1665. Postgres 65 passed. DuckDB and ClickHouse not yet run against this commit. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 253 ++++++++++++++---- .../monitors_query/metric_stability_query.sql | 122 ++++----- macros/edr/tests/test_metric_stability.sql | 91 ++++--- 3 files changed, 306 insertions(+), 160 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 344df2431..32e489cb1 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -12,6 +12,26 @@ OTHER_VALUE_COLUMN = "other_amount" DBT_TEST_NAME = "elementary.metric_stability" +# metric_value goes through dbt's type_float, which is 4 bytes on these. A value +# around 45 then carries about 1e-5 of storage error, orders of magnitude above +# the tolerance the boundary tests probe, so a move sitting exactly on a +# threshold is not decidable there and the assertion would be meaningless. +SINGLE_PRECISION_TARGETS = [ + "clickhouse", + "duckdb", + "spark", + "fabricspark", + "databricks_catalog", + "athena", + "trino", + "dremio", +] + +# dbt-dremio seeds a fractional column as a bare `decimal`, which Dremio reads +# at scale 0, so 45.30 and 45.20 both arrive as 45 and the move under test never +# exists. Tests whose data cannot be expressed as integers skip it. +FRACTIONAL_SEED_TARGETS = ["dremio"] + BASE_AMOUNT = 100 OTHER_BASE_AMOUNT = 500 DAYS_OF_HISTORY = 6 @@ -63,6 +83,16 @@ def _run(dbt_project: DbtProject, test_id: str, data, **overrides) -> str: return result["status"] +def _run_result(dbt_project: DbtProject, test_id: str, data, **overrides) -> dict: + return dbt_project.test( + test_id, + DBT_TEST_NAME, + {**BASE_ARGS, **overrides}, + data=data, + test_vars={"enable_elementary_test_materialization": True}, + ) + + def _bucket_values( dbt_project: DbtProject, test_id: str, column_name: str = VALUE_COLUMN ) -> Dict[Any, List[float]]: @@ -313,6 +343,7 @@ def test_metric_stability_ignores_measurements_taken_while_settling( RATIO_ARGS: Dict[str, Any] = {"metrics": ["average"]} +@pytest.mark.skip_targets(FRACTIONAL_SEED_TARGETS) def test_metric_stability_absolute_threshold_on_a_ratio( test_id: str, dbt_project: DbtProject ): @@ -345,9 +376,14 @@ def test_metric_stability_both_thresholds_must_be_exceeded( A small absolute move on a small base looks large in relative terms, which is exactly the noise an absolute floor is meant to suppress. + + The data is integral because a column of 5.00 is written as 5.0, which agate + normalizes to zero decimals: the seed column becomes INT64 on BigQuery, + integer on Athena and bigint on Dremio, and the load fails before any + assertion runs. Postgres, DuckDB and ClickHouse coerce on insert and hide it. """ - args = {**RATIO_ARGS, "max_change_percent": 1.0, "max_change_absolute": 0.20} - low = 5.00 + args = {**RATIO_ARGS, "max_change_percent": 1.0, "max_change_absolute": 20} + low = 500 assert ( _run(dbt_project, test_id, _rows({d: low for d in range(1, 7)}), **args) @@ -358,14 +394,40 @@ def test_metric_stability_both_thresholds_must_be_exceeded( == "pass" ) - # 0.10 is 2% of a base of 5, so the relative threshold alone would fire. It + # 10 is 2% of a base of 500, so the relative threshold alone would fire. It # is under the absolute floor, so together they do not. rows = {d: low for d in range(1, 7)} - rows[SETTLED_DAYS_AGO] = 5.10 + rows[SETTLED_DAYS_AGO] = 510 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 50 clears the floor and is still well over 1%, so both agree. + rows[SETTLED_DAYS_AGO] = 560 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +def test_metric_stability_percent_still_applies_when_both_set( + test_id: str, dbt_project: DbtProject +): + """The relative leg has to keep biting when an absolute one is present. + + Every other both-thresholds case is decided by the absolute leg, so dropping + max_change_percent from that branch would not have been caught. This move + clears the absolute threshold and stays under the relative one, so it can + only pass if both are still being applied. + """ + args = {**RATIO_ARGS, "max_change_percent": 1.0, "max_change_absolute": 20} + base = 4530 + rows = {d: base for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 30 absolute (over the floor of 20) but 0.66% relative (under 1%). + rows[SETTLED_DAYS_AGO] = 4560 assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" - # 0.50 clears the floor and is still well over 1%, so both agree. - rows[SETTLED_DAYS_AGO] = 5.60 + # 70 absolute and 1.5% relative, so both agree. + rows[SETTLED_DAYS_AGO] = 4630 assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" @@ -379,7 +441,15 @@ def test_metric_stability_absolute_tolerates_zero_baseline( defined at zero, so a metric legitimately sitting at 0 is held to the same tolerance as everything else. """ - args = {**RATIO_ARGS, "max_change_absolute": 5} + # first_check pins the baseline at the original zero. Under the default + # last_check the second move would be measured against 2, not 0, and the + # zero baseline would never be the thing deciding: an implementation that + # never fires at zero would still pass. + args = { + **RATIO_ARGS, + "max_change_absolute": 5, + "change_since": ["first_check"], + } rows = {d: 0 for d in range(1, 7)} assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" @@ -390,7 +460,7 @@ def test_metric_stability_absolute_tolerates_zero_baseline( rows[SETTLED_DAYS_AGO] = 2 assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" - # And the threshold still bites beyond it. + # And the threshold still bites beyond it, still measured against zero. rows[SETTLED_DAYS_AGO] = 10 assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" @@ -415,6 +485,7 @@ def test_metric_stability_both_detect_zero_baseline( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" +@pytest.mark.skip_targets(FRACTIONAL_SEED_TARGETS) def test_metric_stability_unset_threshold_still_fails_any_change( test_id: str, dbt_project: DbtProject ): @@ -432,6 +503,7 @@ def test_metric_stability_unset_threshold_still_fails_any_change( assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 100.1})) == "fail" +@pytest.mark.skip_targets(FRACTIONAL_SEED_TARGETS) def test_metric_stability_absolute_on_both_baselines( test_id: str, dbt_project: DbtProject ): @@ -455,50 +527,132 @@ def test_metric_stability_absolute_on_both_baselines( @pytest.mark.parametrize( - "overrides", + "overrides,expected_argument", [ - {"max_change_absolute": -1}, - {"max_change_absolute": "0.20"}, - {"max_change_absolute": True}, - {"max_change_percent": -1}, - {"max_change_percent": "25"}, - {"max_change_percent": True}, - {"max_change_percent": 0.0000000001}, + pytest.param({"max_change_absolute": -1}, "max_change_absolute", id="abs_neg"), + pytest.param({"max_change_percent": -1}, "max_change_percent", id="pct_neg"), + pytest.param( + {"max_change_absolute": True}, "max_change_absolute", id="abs_bool" + ), + pytest.param({"max_change_percent": True}, "max_change_percent", id="pct_bool"), + pytest.param({"max_change_absolute": "x"}, "max_change_absolute", id="abs_str"), + pytest.param({"max_change_percent": "x"}, "max_change_percent", id="pct_str"), + pytest.param({"max_change_percent": None}, "max_change_percent", id="pct_null"), + pytest.param( + {"max_change_absolute": float("inf")}, "max_change_absolute", id="abs_inf" + ), + pytest.param( + {"max_change_absolute": float("nan")}, "max_change_absolute", id="abs_nan" + ), ], ) def test_metric_stability_rejects_bad_thresholds( - test_id: str, dbt_project: DbtProject, overrides: Dict[str, Any] + test_id: str, + dbt_project: DbtProject, + overrides: Dict[str, Any], + expected_argument: str, ): """Every threshold value that cannot mean what it says must raise. + The description is asserted on, not just the status. Several of these would + error anyway without any validation: a string raises inside the arithmetic, + and True renders as a threshold of 1. Only an error naming the argument + shows that the check, rather than an incidental crash, rejected it. + Booleans matter because bool subclasses int in python, so Jinja's `number` - test accepts True and would render it as a threshold of 1. The tiny percent - matters because the query clamps it up to the noise floor, applying a - threshold nobody asked for. There is no equivalent absolute case: an - absolute threshold renders at full precision and is used as given. + test accepts True. inf matters because it clears every bound and then + renders as a bare inf, which ClickHouse reads as a literal that makes every + comparison false, so the test would never fail again. nan is caught earlier, + by not being equal to itself. """ - assert _run(dbt_project, test_id, _rows(), **overrides) == "error" - - -# metric_value is stored through dbt's type_float, which is single precision on -# some adapters and double on others: the same subtraction gives -# 0.20000000000000284 on Postgres and 0.20000076293945312 on DuckDB and -# ClickHouse. Where it is single precision the storage error is four orders of -# magnitude above the tolerance below, so a move sitting exactly on the threshold -# is not decidable and the assertion would be meaningless rather than wrong. The -# list errs towards over-skipping; the behaviour under test is the tolerance, and -# that is exercised wherever metric_value is double precision. -@pytest.mark.skip_targets( - [ - "clickhouse", - "duckdb", - "spark", - "fabricspark", - "databricks_catalog", - "athena", - "trino", - ] -) + result = _run_result(dbt_project, test_id, _rows(), **overrides) + + assert result["status"] == "error" + assert expected_argument in (result.get("test_results_description") or "") + + +@pytest.mark.skip_targets(FRACTIONAL_SEED_TARGETS) +def test_metric_stability_accepts_a_numeric_string_threshold( + test_id: str, dbt_project: DbtProject +): + """A threshold written in exponent form arrives as a string, and must work. + + dbt-core reads YAML 1.1, where a float needs a decimal point, so + `max_change_absolute: 2e-3` is a string rather than a number. That is a + natural way to write 20bp on a 0-1 scale, so rejecting it as "not a number" + would be telling the user to unquote something they never quoted. + """ + args = {**RATIO_ARGS, "max_change_absolute": "2e-3"} + rows = {d: 0.1000 for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 10bp, inside a threshold of 20bp. + rows[SETTLED_DAYS_AGO] = 0.0990 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 50bp, outside it. A threshold read as 0 or rejected could not do both. + rows[SETTLED_DAYS_AGO] = 0.0950 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) +def test_metric_stability_absolute_bites_on_large_metrics( + test_id: str, dbt_project: DbtProject +): + """A noise floor must not quietly override an explicit absolute threshold. + + The predicate used to AND a fixed 1e-9 percent floor onto every comparison, + absolute-only ones included. On a daily sum of 2.5e11 a move of 2 is 8e-10 + percent, under that floor, so `max_change_absolute: 1` never reported it + however far the value moved in absolute terms. + + The base carries a fraction so the seed column stays floating point: whole + numbers this large are inferred as integer and overflow a 32-bit int column. + """ + args = {"max_change_absolute": 1} + base = 250000000000.5 + rows = {d: base for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # Twice the threshold, and far below the old relative floor. + rows[SETTLED_DAYS_AGO] = base + 2 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) +def test_metric_stability_absolute_boundary_on_large_values( + test_id: str, dbt_project: DbtProject +): + """The boundary stays symmetric as the metric grows. + + Representation error grows with the operands, so a slack of a fixed size + stops absorbing it. At a bucket of 12345678.30 the two directions of a 0.20 + move compute 0.2000000011175871 and 0.19999999925494194, so the move was + reported going down and not going up: the same one-direction-only behaviour + the smaller boundary test guards against, returning about a million times + the threshold. + """ + args = {**RATIO_ARGS, "max_change_absolute": 0.20} + base = 12345678.30 + rows = {d: base for d in range(1, 7)} + + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # Exactly 0.20 down, the direction that rounds against us here. + rows[SETTLED_DAYS_AGO] = 12345678.10 + assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" + + # 0.70 further, comfortably past the threshold. + rows[SETTLED_DAYS_AGO] = 12345678.80 + assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" + + +@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) def test_metric_stability_absolute_float_boundary( test_id: str, dbt_project: DbtProject ): @@ -525,18 +679,7 @@ def test_metric_stability_absolute_float_boundary( assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" -# Single precision loses this outright: see the note on the test above. -@pytest.mark.skip_targets( - [ - "clickhouse", - "duckdb", - "spark", - "fabricspark", - "databricks_catalog", - "athena", - "trino", - ] -) +@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) def test_metric_stability_absolute_boundary_on_unit_scale( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 627c17fbe..9c7a0b992 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -8,7 +8,7 @@ detection_end, days_back, min_bucket_age, - max_change_percent=none, + max_change_percent=0, max_change_absolute=none, change_since=["last_check"], column_names=none, @@ -59,42 +59,46 @@ because the relative form divides by the baseline, its sensitivity is recomputed per bucket, so a setting meaning 20bp at a 45% margin means about 9bp at 20% and 27bp at 60%. max_change_absolute expresses the move - directly. Unset percent means 0 unless an absolute threshold stands in - for it, which is why the default is none rather than 0. + directly. - How small an absolute threshold can usefully be is bounded by how the - warehouse stores metric_value, which goes through dbt's type_float: - double precision on most adapters, single on some (DuckDB and ClickHouse - among them). At single precision a value around 45 carries roughly 1e-5 - of storage error, so a move sitting within that of the threshold is not - decidable there and may be reported in one direction but not the other. - The tolerance below addresses decimal representation, not storage. #} - {%- set has_absolute = max_change_absolute is not none %} - {%- set effective_percent = ( - max_change_percent if max_change_percent is not none else 0 - ) %} + Both legs are written as `moved > threshold * scale`, with no division, + so neither needs a zero-baseline escape: at a zero baseline the relative + leg reduces to `moved > 0`, which is satisfied by any change rather than + falsified by it, and the absolute leg decides on its own. The earlier + form built a separate predicate per branch, which is how the zero + baseline was suppressed once already. #} + {# Two coefficients, both scaled by the operands, because the errors they + cover scale with the operands too. - {# Suppress floating-point aggregation noise. Re-aggregating the same rows - in a different order differs in the last bits, so the relative guard is - kept even when an absolute threshold is doing the real work. #} - {%- set change_percent_noise_floor = 0.000000001 %} - {#- Thresholds render through `string`, which is python's shortest - round-tripping form, so the literal in the SQL is the same double the - test was configured with. A fixed number of decimals would not be: at - "%.10f" the slack below survives on a 0-100 scale but vanishes on a 0-1 - scale, which made identical moves behave differently depending on how - the metric happened to be scaled. -#} - {%- set change_threshold = ( - [effective_percent, change_percent_noise_floor] | max | string - ) %} - {#- A decimal threshold has no exact float representation, so two moves that - are both 0.20 in decimal land either side of it: 45.30 -> 45.10 is - 0.19999999999999574 and 45.50 -> 45.30 is 0.20000000000000284. Without a - tolerance the customer's own 20bp example fires in one direction and not - the other. The slack is far below any move anyone would configure. -#} - {%- set absolute_threshold = ( - (max_change_absolute * (1 + 0.000000001)) | string - if has_absolute + noise_floor stands in for an absolute threshold when none was given, so + that re-aggregating the same rows in a different order does not read as + a restatement. It is the old 1e-9 percent floor written as a fraction, + so the default behaviour is unchanged. It is deliberately NOT added on + top of an explicit max_change_absolute: setting one states the + resolution wanted, and folding the floor into it dropped real moves on + large metrics (a move of 2 on a daily sum of 2.5e11, with + max_change_absolute: 1, went unreported). + + comparison_tolerance absorbs the representation error in the comparison + itself. A decimal threshold has no exact binary form, so 45.30 -> 45.10 + is 0.19999999999999574 while 45.50 -> 45.30 is 0.20000000000000284, and + without it the same 20bp move is reported in one direction only. A fixed + slack cannot do this, because that error grows with the operands: at a + bucket of 12345678.30 it reaches 1e-9, five orders of magnitude above a + slack sized for values around 45. #} + {%- set noise_floor = 10**-11 %} + {%- set comparison_tolerance = 10**-13 %} + + {#- Thresholds render through `string`, python's shortest round-tripping + form, so the literal in the SQL is the double that was configured. + Fixed decimals are not: at "%.10f" a tolerance survived on a 0-100 + scale and vanished on a 0-1 scale, so the same move behaved differently + depending on how the metric happened to be scaled. -#} + {%- set percent_fraction = (max_change_percent / 100.0) | string %} + {%- set tolerance_coefficient = comparison_tolerance | string %} + {%- set absolute_literal = ( + max_change_absolute | string + if max_change_absolute is not none else none ) %} @@ -104,48 +108,18 @@ {%- set baseline_column = baseline ~ "_value" %} {%- if baseline_column not in baseline_columns %} {%- do baseline_columns.append(baseline_column) %} - {%- set change_percent = elementary.metric_stability_change_percent( - baseline_column + {%- set moved = "abs(measured_value - " ~ baseline_column ~ ")" %} + {%- set scale = "(abs(measured_value) + abs(" ~ baseline_column ~ "))" %} + {%- set tolerance = tolerance_coefficient ~ " * " ~ scale %} + {%- set absolute_leg = ( + absolute_literal + if absolute_literal is not none + else (noise_floor | string) ~ " * " ~ scale ) %} - {%- set threshold_tests = [] %} - {%- if has_absolute %} - {# Relative change is undefined at a zero baseline, so the - relative guard has to be satisfied there rather than - falsified, or the AND below would suppress every zero - baseline and the absolute threshold would never decide. #} - {%- do threshold_tests.append( - "(" - ~ baseline_column - ~ " = 0 or " - ~ change_percent - ~ " > " - ~ change_threshold - ~ ")" - ) %} - {%- do threshold_tests.append( - "abs(measured_value - " - ~ baseline_column - ~ ") > " - ~ absolute_threshold - ) %} - {%- else %} - {# With no absolute threshold to decide at zero, any move away - from exactly zero counts. #} - {%- do threshold_tests.append( - "((" - ~ baseline_column - ~ " = 0 and measured_value != 0) or (" - ~ baseline_column - ~ " != 0 and " - ~ change_percent - ~ " > " - ~ change_threshold - ~ "))" - ) %} - {%- endif %} {% set exceeds_condition %} ({{ baseline_column }} is not null - and {{ threshold_tests | join(" and ") }}) + and {{ moved }} > {{ percent_fraction }} * abs({{ baseline_column }}) + {{ tolerance }} + and {{ moved }} > {{ absolute_leg }} + {{ tolerance }}) {% endset %} {% do exceeds_conditions.append(exceeds_condition) %} {%- endif %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 35867c9cd..247a474b4 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -8,13 +8,17 @@ baseline, where relative change has no meaning, the absolute threshold decides on its own. - An absolute threshold is limited by how precisely the warehouse stores a - metric. metric_value goes through dbt's type_float, which is double - precision on some adapters and single on others (DuckDB and ClickHouse - among them). At single precision a value around 45 carries roughly 1e-5 of - representation error, so a threshold within a few orders of magnitude of - that measures storage noise rather than the data. A relative threshold is - unaffected, since the error scales with the value. + max_change_absolute is one number for every column, metric and dimension the + test covers, so it only means something when they share a scale. A test over + a 0-100 margin and a 0-1 rate holds both to the same raw move, and a test + over `average` and `null_count` holds a ratio and a row count to it. Use one + test per scale, or max_change_percent, which is scale-free. + + Both thresholds are bounded by how precisely the warehouse stores a metric. + metric_value goes through dbt's type_float, which is 4 bytes on DuckDB, + ClickHouse, Spark, Databricks and Dremio. There a sum of 12345678.30 + restated to 12345678.00 stores as the same value twice, and no threshold, + relative or absolute, can see a move that was never recorded. Configuration, baseline behavior, and limits: https://docs.elementary-data.com/data-tests/metric-stability #} @@ -25,7 +29,7 @@ timestamp_column, min_bucket_age, change_since=["last_check"], - max_change_percent=none, + max_change_percent=0, max_change_absolute=none, time_bucket=none, where_expression=none, @@ -55,6 +59,8 @@ days_back, backfill_days, ) %} + {% set max_change_percent = arguments.max_change_percent %} + {% set max_change_absolute = arguments.max_change_absolute %} {% set model_relation = arguments.model_relation %} {% set model_graph_node = arguments.model_graph_node %} {% set timestamp_column = arguments.timestamp_column %} @@ -412,24 +418,44 @@ }} {%- endif %} - {#- A percent below the noise floor is clamped up to it by the query, so - it would silently mean something other than what was written. An - absolute threshold has no such floor: it renders at full precision and - is used as given, so any positive value is honoured. -#} - {%- for arg_name, arg_value, example, minimum in [ - ("max_change_percent", max_change_percent, "25", 0.000000001), - ("max_change_absolute", max_change_absolute, "0.20", none), + {#- max_change_percent defaults to 0, so a none here can only come from an + explicit `max_change_percent: null`. master rejected that, and it must + not quietly become "report any change at all". + + Numeric strings are accepted rather than rejected. dbt-core reads YAML + 1.1, where a float needs a decimal point, so `max_change_absolute: 2e-3` + (a natural way to write 20bp on a 0-1 scale) arrives as the string + '2e-3'. Telling the user to write it unquoted, when they already did, + is not something they can act on. -#} + {%- set thresholds = {} %} + {%- for arg_name, arg_value, example, optional in [ + ("max_change_percent", max_change_percent, "25", false), + ("max_change_absolute", max_change_absolute, "0.20", true), ] %} - {%- if arg_value is not none %} + {%- if arg_value is none %} + {%- if not optional %} + {{ + exceptions.raise_compiler_error( + arg_name + ~ " cannot be null. Omit it to keep the default of 0," + ~ " or give a number." + ) + }} + {%- endif %} + {%- do thresholds.update({arg_name: none}) %} + {%- else %} {#- bool subclasses int in python, so `is number` alone lets - `max_change_percent: true` through as a threshold of 1. -#} - {%- if arg_value is not number or arg_value is boolean %} + `max_change_percent: true` through as a threshold of 1. Parsing + twice with different fallbacks separates a value that failed to + parse from one that happens to equal a fallback. -#} + {%- set parsed = arg_value | float(1.0) %} + {%- if arg_value is boolean or parsed != arg_value | float(2.0) %} {{ exceptions.raise_compiler_error( arg_name ~ " must be a number, got '" ~ arg_value - ~ "'. Write it unquoted, e.g. " + ~ "'. For example " ~ arg_name ~ ": " ~ example @@ -437,30 +463,31 @@ ) }} {%- endif %} - {%- if arg_value < 0 %} + {#- inf is a number, is not negative and clears every bound, so + without this it renders as a bare inf: most warehouses read + that as an unknown column and fail at runtime, and ClickHouse + reads it as a literal that makes every comparison false, so the + test would simply never fail again. nan is already rejected + above, because it is not equal to itself and so fails the + two-parse comparison. -#} + {%- if parsed - parsed != 0 %} {{ exceptions.raise_compiler_error( - arg_name ~ " must be non-negative." + arg_name ~ " must be finite, got '" ~ arg_value ~ "'." ) }} {%- endif %} - {%- if minimum is not none and arg_value > 0 and arg_value < minimum %} + {%- if parsed < 0 %} {{ exceptions.raise_compiler_error( - arg_name - ~ " is " - ~ arg_value - ~ ", below the floating-point noise floor this test" - ~ " applies (" - ~ minimum - ~ "). Anything smaller would be raised to the floor" - ~ " rather than applied. Use a larger value, use" - ~ " max_change_absolute, or rescale the metric." + arg_name ~ " must be non-negative, got '" ~ arg_value ~ "'." ) }} {%- endif %} + {%- do thresholds.update({arg_name: parsed}) %} {%- endif %} {%- endfor %} + {%- for arg_name, arg_value in [ ("days_back", days_back), ("backfill_days", backfill_days), @@ -607,6 +634,8 @@ "columns": column_definitions, "metrics": metrics, "change_since": change_since, + "max_change_percent": thresholds["max_change_percent"], + "max_change_absolute": thresholds["max_change_absolute"], "model_relation": model_relation, "model_graph_node": model_graph_node, "timestamp_column": timestamp_column, From 7cb346b13e81338eff1c5213afd5aa06fd86154b Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 2 Nov 2026 10:47:23 +0200 Subject: [PATCH 6/8] fix: drop Jinja constructs dbt-fusion does not support The fusion jobs failed every metric_stability test with "Failed to render SQL too many arguments" at test_metric_stability.sql:451, which is `arg_value | float(1.0)`. minijinja's float filter takes no default argument, and that call sat in the threshold loop that runs on every invocation, so all 36 tests failed rather than the validation cases. The two-parse trick existed to tell a numeric string from an unparseable one, so that `max_change_absolute: 2e-3` would be accepted: dbt reads YAML 1.1, where an exponent without a decimal point is text. Without a portable way to attempt a parse, strings are rejected again and the message now names the decimal point, which is the alternative the review offered. `max_change_percent: "25"` is therefore rejected once more, as it was on master. Also replaces `10 ** -11` with a plain literal. It renders identically and neither `**` nor a filter default appears anywhere else in this package, which is what made both suspect once a second Jinja engine was in play. Vertica's five failures look unrelated: all five are tests that should fail and instead pass, and three are in test_data_freshness_sla.py, which this PR does not touch. Left alone pending the next run. Postgres 39 passed. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 40 ++++------------- .../monitors_query/metric_stability_query.sql | 4 +- macros/edr/tests/test_metric_stability.sql | 44 +++++++++---------- 3 files changed, 31 insertions(+), 57 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 32e489cb1..7ca4c8619 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -560,10 +560,15 @@ def test_metric_stability_rejects_bad_thresholds( shows that the check, rather than an incidental crash, rejected it. Booleans matter because bool subclasses int in python, so Jinja's `number` - test accepts True. inf matters because it clears every bound and then - renders as a bare inf, which ClickHouse reads as a literal that makes every - comparison false, so the test would never fail again. nan is caught earlier, - by not being equal to itself. + test accepts True. inf and nan matter because they are numbers, are not + negative and clear every bound, and then render as a bare inf / nan, which + ClickHouse reads as a literal that makes every comparison false, so the test + would never fail again. + + A string is rejected rather than parsed. dbt reads YAML 1.1, where an + exponent without a decimal point is text, so `2e-3` arrives here as a + string; the message says to write it with a point rather than telling the + user to unquote something they never quoted. """ result = _run_result(dbt_project, test_id, _rows(), **overrides) @@ -571,33 +576,6 @@ def test_metric_stability_rejects_bad_thresholds( assert expected_argument in (result.get("test_results_description") or "") -@pytest.mark.skip_targets(FRACTIONAL_SEED_TARGETS) -def test_metric_stability_accepts_a_numeric_string_threshold( - test_id: str, dbt_project: DbtProject -): - """A threshold written in exponent form arrives as a string, and must work. - - dbt-core reads YAML 1.1, where a float needs a decimal point, so - `max_change_absolute: 2e-3` is a string rather than a number. That is a - natural way to write 20bp on a 0-1 scale, so rejecting it as "not a number" - would be telling the user to unquote something they never quoted. - """ - args = {**RATIO_ARGS, "max_change_absolute": "2e-3"} - rows = {d: 0.1000 for d in range(1, 7)} - - assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" - assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" - - # 10bp, inside a threshold of 20bp. - rows[SETTLED_DAYS_AGO] = 0.0990 - assert _run(dbt_project, test_id, _rows(rows), **args) == "pass" - - # 50bp, outside it. A threshold read as 0 or rejected could not do both. - rows[SETTLED_DAYS_AGO] = 0.0950 - assert _run(dbt_project, test_id, _rows(rows), **args) == "fail" - - -@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) def test_metric_stability_absolute_bites_on_large_metrics( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 9c7a0b992..2dc9d77bc 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -86,8 +86,8 @@ slack cannot do this, because that error grows with the operands: at a bucket of 12345678.30 it reaches 1e-9, five orders of magnitude above a slack sized for values around 45. #} - {%- set noise_floor = 10**-11 %} - {%- set comparison_tolerance = 10**-13 %} + {%- set noise_floor = 0.00000000001 %} + {%- set comparison_tolerance = 0.0000000000001 %} {#- Thresholds render through `string`, python's shortest round-tripping form, so the literal in the SQL is the double that was configured. diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 247a474b4..c355ba6e6 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -420,13 +420,7 @@ {#- max_change_percent defaults to 0, so a none here can only come from an explicit `max_change_percent: null`. master rejected that, and it must - not quietly become "report any change at all". - - Numeric strings are accepted rather than rejected. dbt-core reads YAML - 1.1, where a float needs a decimal point, so `max_change_absolute: 2e-3` - (a natural way to write 20bp on a 0-1 scale) arrives as the string - '2e-3'. Telling the user to write it unquoted, when they already did, - is not something they can act on. -#} + not quietly become "report any change at all". -#} {%- set thresholds = {} %} {%- for arg_name, arg_value, example, optional in [ ("max_change_percent", max_change_percent, "25", false), @@ -445,46 +439,48 @@ {%- do thresholds.update({arg_name: none}) %} {%- else %} {#- bool subclasses int in python, so `is number` alone lets - `max_change_percent: true` through as a threshold of 1. Parsing - twice with different fallbacks separates a value that failed to - parse from one that happens to equal a fallback. -#} - {%- set parsed = arg_value | float(1.0) %} - {%- if arg_value is boolean or parsed != arg_value | float(2.0) %} + `max_change_percent: true` through as a threshold of 1. + + The message names the decimal point because dbt reads YAML 1.1, + where an exponent written without one is text rather than a + number: `max_change_absolute: 2e-3`, a natural way to write 20bp + on a 0-1 scale, arrives here as the string '2e-3'. Telling the + user to unquote something they never quoted is not actionable. -#} + {%- if arg_value is boolean or arg_value is not number %} {{ exceptions.raise_compiler_error( arg_name ~ " must be a number, got '" ~ arg_value - ~ "'. For example " + ~ "'. Write it with a decimal point, e.g. " ~ arg_name ~ ": " ~ example - ~ "." + ~ ". An exponent without one, such as 2e-3, is read as" + ~ " text rather than a number." ) }} {%- endif %} - {#- inf is a number, is not negative and clears every bound, so - without this it renders as a bare inf: most warehouses read - that as an unknown column and fail at runtime, and ClickHouse - reads it as a literal that makes every comparison false, so the - test would simply never fail again. nan is already rejected - above, because it is not equal to itself and so fails the - two-parse comparison. -#} - {%- if parsed - parsed != 0 %} + {#- inf and nan are numbers, are not negative and clear every bound, + so without this they render as a bare inf / nan: most warehouses + read that as an unknown column and fail at runtime, and + ClickHouse reads it as a literal that makes every comparison + false, so the test would simply never fail again. -#} + {%- if arg_value - arg_value != 0 %} {{ exceptions.raise_compiler_error( arg_name ~ " must be finite, got '" ~ arg_value ~ "'." ) }} {%- endif %} - {%- if parsed < 0 %} + {%- if arg_value < 0 %} {{ exceptions.raise_compiler_error( arg_name ~ " must be non-negative, got '" ~ arg_value ~ "'." ) }} {%- endif %} - {%- do thresholds.update({arg_name: parsed}) %} + {%- do thresholds.update({arg_name: arg_value}) %} {%- endif %} {%- endfor %} From b501ceaa2623011fcff28f10d81f6f3882ecdaee Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 2 Nov 2026 10:51:05 +0200 Subject: [PATCH 7/8] fix: bound the comparison tolerance to real float error At 1e-13 the operand-scaled tolerance reaches 0.2 on a baseline of 1e12, which masks a move of 0.01 under an absolute threshold of 0.001. That is the same class of problem as the noise floor it replaced, just reached from the other direction. Swept the coefficient over magnitudes from 0.1 to 1e12 and thresholds from 1e-9 to 1e-2 of the baseline, checking that an exactly-on-threshold move never fires in either direction and a 10x move always does. 1e-15, about 4 ulps of a double, is the loosest value that passes, with 30x headroom over the worst observed representation excess. 1e-16 leaves only 3x. Postgres 39 passed. Co-Authored-By: Claude Opus 5 --- .../monitors_query/metric_stability_query.sql | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 2dc9d77bc..6596474b2 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -80,14 +80,19 @@ max_change_absolute: 1, went unreported). comparison_tolerance absorbs the representation error in the comparison - itself. A decimal threshold has no exact binary form, so 45.30 -> 45.10 + itself. It is about 4 ulps of a double, which is the smallest value that + still covers the boundary cases below with room to spare (measured: 30x + the worst observed excess, over magnitudes from 0.1 to 1e12). Sizing it + more loosely masks real moves on large metrics: at 1e-13 a baseline of + 1e12 carries a tolerance of 0.2, which hides a move of 0.01 under an + absolute threshold of 0.001. A decimal threshold has no exact binary form, so 45.30 -> 45.10 is 0.19999999999999574 while 45.50 -> 45.30 is 0.20000000000000284, and without it the same 20bp move is reported in one direction only. A fixed slack cannot do this, because that error grows with the operands: at a bucket of 12345678.30 it reaches 1e-9, five orders of magnitude above a slack sized for values around 45. #} {%- set noise_floor = 0.00000000001 %} - {%- set comparison_tolerance = 0.0000000000001 %} + {%- set comparison_tolerance = 0.000000000000001 %} {#- Thresholds render through `string`, python's shortest round-tripping form, so the literal in the SQL is the double that was configured. From 0164bbf9985e009b7d39d1feded260b7a24825ce Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 2 Nov 2026 11:24:03 +0200 Subject: [PATCH 8/8] fix: restore a dropped skip marker and skip inf/nan on fusion absolute_bites_on_large_metrics uses a base of 250000000000.5, which a 4-byte float cannot represent, so it cannot fire on a single-precision adapter. It had the skip marker for those; removing the numeric-string test above it took the marker with it, because the slice ran from that test's start to this one's def line. ClickHouse and DuckDB then ran it and failed. Restored, and this time all three local adapters were run rather than Postgres alone. dbt-fusion does not carry .inf or .nan through to the macro: both arrive as null, which for an optional argument means "unset", so validation is never reached and the test sees a pass. Confirmed from the fusion log, where test_params recorded max_change_absolute as null for exactly those two cases. They now use the repo's existing skip_for_dbt_fusion marker; the guard itself still runs on every other engine. Postgres 39 passed. DuckDB and ClickHouse 35 passed, 4 skipped. Co-Authored-By: Claude Opus 5 --- integration_tests/tests/test_metric_stability.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 7ca4c8619..1b98d5c47 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -539,10 +539,16 @@ def test_metric_stability_absolute_on_both_baselines( pytest.param({"max_change_percent": "x"}, "max_change_percent", id="pct_str"), pytest.param({"max_change_percent": None}, "max_change_percent", id="pct_null"), pytest.param( - {"max_change_absolute": float("inf")}, "max_change_absolute", id="abs_inf" + {"max_change_absolute": float("inf")}, + "max_change_absolute", + id="abs_inf", + marks=pytest.mark.skip_for_dbt_fusion, ), pytest.param( - {"max_change_absolute": float("nan")}, "max_change_absolute", id="abs_nan" + {"max_change_absolute": float("nan")}, + "max_change_absolute", + id="abs_nan", + marks=pytest.mark.skip_for_dbt_fusion, ), ], ) @@ -565,6 +571,10 @@ def test_metric_stability_rejects_bad_thresholds( ClickHouse reads as a literal that makes every comparison false, so the test would never fail again. + inf and nan skip dbt-fusion, which does not carry either through to the + macro: both arrive as null, which for an optional argument means "unset", + so the validation under test is never reached. + A string is rejected rather than parsed. dbt reads YAML 1.1, where an exponent without a decimal point is text, so `2e-3` arrives here as a string; the message says to write it with a point rather than telling the @@ -576,6 +586,7 @@ def test_metric_stability_rejects_bad_thresholds( assert expected_argument in (result.get("test_results_description") or "") +@pytest.mark.skip_targets(SINGLE_PRECISION_TARGETS) def test_metric_stability_absolute_bites_on_large_metrics( test_id: str, dbt_project: DbtProject ):