From 5f73bca073641e1fc3eb6989aaeba14489890a55 Mon Sep 17 00:00:00 2001 From: bferanmi806-sketch Date: Mon, 21 Sep 2026 19:57:24 +0100 Subject: [PATCH 1/3] feat(detector): compare distinct ratio for historically unique columns (#94) --- src/scherlok/detector/cardinality.py | 60 ++++++++++++++++++++++++++++ src/scherlok/service.py | 8 +++- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/scherlok/detector/cardinality.py b/src/scherlok/detector/cardinality.py index 4139e75..daf5e90 100644 --- a/src/scherlok/detector/cardinality.py +++ b/src/scherlok/detector/cardinality.py @@ -9,18 +9,34 @@ CARDINALITY_WARNING_PCT = 50 # 50% change CARDINALITY_CRITICAL_PCT = 200 # 3x change (e.g. status went from 5 to 500) +# Ratio threshold for historically near-unique columns. +# When stored distinct_count / row_count >= UNIQUE_RATIO the column is +# considered near-unique and we compare distinct ratios instead of absolute +# distinct counts, so a volume drop does not fire cardinality noise. +UNIQUE_RATIO = 0.95 + def detect_cardinality_anomalies( table: str, column: str, current_dist: dict, stored_dist: dict, + current_vol: dict | None = None, + stored_vol: dict | None = None, *, history: Sequence[dict] | None = None, ) -> list[dict]: """Compare current distinct count against stored profile for a column. Returns anomalies when cardinality changes significantly. + + When the stored profile shows the column was near-unique + (distinct_count / row_count >= UNIQUE_RATIO) the distinct ratio + (distinct / rows) is compared instead of the absolute count. A + unique column that stays unique after a volume change is then silent, + while a unique column that suddenly has duplicates still fires. + Falls back to absolute comparison when volume profiles are missing + or row counts are zero. """ anomalies: list[dict] = [] @@ -63,6 +79,50 @@ def detect_cardinality_anomalies( if stored_card == 0: return anomalies + # Near-unique path: use distinct ratio when stored was near-unique + # and both volume profiles provide valid row counts. + if current_vol is not None and stored_vol is not None: + stored_rows = stored_vol.get("row_count") + current_rows = current_vol.get("row_count") + if ( + isinstance(stored_rows, int) + and isinstance(current_rows, int) + and stored_rows > 0 + and current_rows > 0 + and isinstance(stored_card, int) + and isinstance(current_card, int) + ): + stored_ratio = stored_card / stored_rows + if stored_ratio >= UNIQUE_RATIO: + current_ratio = current_card / current_rows + if stored_ratio == 0: + return anomalies + change_pct = abs(current_ratio - stored_ratio) / stored_ratio * 100 + direction = "decreased" if current_ratio < stored_ratio else "increased" + if change_pct >= CARDINALITY_CRITICAL_PCT: + anomalies.append({ + "table": table, + "type": "cardinality_change", + "message": ( + f"Column '{column}' distinct ratio {direction}: " + f"{stored_ratio:.3f} -> {current_ratio:.3f} " + f"({change_pct:.0f}% change)" + ), + "severity": Severity.CRITICAL, + }) + elif change_pct >= CARDINALITY_WARNING_PCT: + anomalies.append({ + "table": table, + "type": "cardinality_change", + "message": ( + f"Column '{column}' distinct ratio {direction}: " + f"{stored_ratio:.3f} -> {current_ratio:.3f} " + f"({change_pct:.0f}% change)" + ), + "severity": Severity.WARNING, + }) + return anomalies + change_pct = abs(current_card - stored_card) / stored_card * 100 direction = "increased" if current_card > stored_card else "decreased" diff --git a/src/scherlok/service.py b/src/scherlok/service.py index b9adffa..a49b1fa 100644 --- a/src/scherlok/service.py +++ b/src/scherlok/service.py @@ -84,7 +84,13 @@ def profile_and_detect( ) anomalies.extend( detect_cardinality_anomalies( - table, col_name, current_dist, stored_dist, history=distribution_history + table, + col_name, + current_dist, + stored_dist, + current_vol, + stored_vol, + history=distribution_history, ) ) store.save_profile(table, f"distribution:{col_name}", current_dist) From 391b4ebf6917fb9640bf1580ec4077d500aa058b Mon Sep 17 00:00:00 2001 From: Odunayo Balogun Date: Tue, 22 Sep 2026 15:14:48 +0100 Subject: [PATCH 2/3] fix: cover near-unique cardinality severity --- src/scherlok/detector/cardinality.py | 12 ++-- tests/test_detector.py | 89 ++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/scherlok/detector/cardinality.py b/src/scherlok/detector/cardinality.py index daf5e90..a87bae6 100644 --- a/src/scherlok/detector/cardinality.py +++ b/src/scherlok/detector/cardinality.py @@ -14,6 +14,10 @@ # considered near-unique and we compare distinct ratios instead of absolute # distinct counts, so a volume drop does not fire cardinality noise. UNIQUE_RATIO = 0.95 +# Ratio changes are bounded to 0-100%, so the absolute cardinality thresholds +# above cannot classify a near-unique column as critical. +UNIQUE_RATIO_WARNING_PCT = 10 +UNIQUE_RATIO_CRITICAL_PCT = 50 def detect_cardinality_anomalies( @@ -95,11 +99,11 @@ def detect_cardinality_anomalies( stored_ratio = stored_card / stored_rows if stored_ratio >= UNIQUE_RATIO: current_ratio = current_card / current_rows - if stored_ratio == 0: - return anomalies change_pct = abs(current_ratio - stored_ratio) / stored_ratio * 100 direction = "decreased" if current_ratio < stored_ratio else "increased" - if change_pct >= CARDINALITY_CRITICAL_PCT: + # Keep the exact 50% regression case in the warning band; + # a loss beyond that boundary is critical. + if change_pct > UNIQUE_RATIO_CRITICAL_PCT: anomalies.append({ "table": table, "type": "cardinality_change", @@ -110,7 +114,7 @@ def detect_cardinality_anomalies( ), "severity": Severity.CRITICAL, }) - elif change_pct >= CARDINALITY_WARNING_PCT: + elif change_pct >= UNIQUE_RATIO_WARNING_PCT: anomalies.append({ "table": table, "type": "cardinality_change", diff --git a/tests/test_detector.py b/tests/test_detector.py index 87afbe4..03271a9 100644 --- a/tests/test_detector.py +++ b/tests/test_detector.py @@ -300,3 +300,92 @@ def test_no_anomaly_zero_stored(self): stored = {"distinct_count": 0} anomalies = detect_cardinality_anomalies("t", "col", current, stored) assert len(anomalies) == 0 + + +class TestCardinalityNearUniqueColumns: + """Use distinct ratio for columns that were historically near-unique.""" + + STORED_DIST = {"distinct_count": 1000} + STORED_VOL = {"row_count": 1000} + + def test_volume_drop_on_unique_column_is_silent(self): + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 400}, + self.STORED_DIST, + {"row_count": 400}, + self.STORED_VOL, + ) + assert anomalies == [] + + def test_duplicates_on_unique_column_still_fire(self): + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 500}, + self.STORED_DIST, + {"row_count": 1000}, + self.STORED_VOL, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.WARNING + assert "decreased" in anomalies[0]["message"] + assert "distinct ratio" in anomalies[0]["message"] + + def test_low_cardinality_column_keeps_absolute_comparison(self): + anomalies = detect_cardinality_anomalies( + "users", + "plan", + {"distinct_count": 605}, + {"distinct_count": 5}, + {"row_count": 400}, + {"row_count": 1000}, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.CRITICAL + assert "distinct values" in anomalies[0]["message"] + + def test_falls_back_to_absolute_without_volume_profiles(self): + anomalies = detect_cardinality_anomalies( + "orders", "id", {"distinct_count": 400}, self.STORED_DIST + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.WARNING + + def test_falls_back_to_absolute_on_zero_row_counts(self): + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 400}, + self.STORED_DIST, + {"row_count": 0}, + {"row_count": 0}, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.WARNING + + def test_column_just_below_unique_threshold_uses_absolute(self): + anomalies = detect_cardinality_anomalies( + "orders", + "customer_id", + {"distinct_count": 360}, + {"distinct_count": 900}, + {"row_count": 400}, + {"row_count": 1000}, + ) + assert len(anomalies) == 1 + assert "distinct values" in anomalies[0]["message"] + + def test_large_loss_of_uniqueness_is_critical(self): + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 100}, + self.STORED_DIST, + {"row_count": 1000}, + self.STORED_VOL, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.CRITICAL + assert "distinct ratio" in anomalies[0]["message"] From f8b25bec1e00843a2b1a854618901e720d9ba8af Mon Sep 17 00:00:00 2001 From: Balogun Feranmi Date: Tue, 22 Sep 2026 22:29:23 +0100 Subject: [PATCH 3/3] fix(detector): NULL-aware uniqueness ratios, inclusive thresholds, float boundaries (#94) Use non-null rows (rows - null_count) as the distinct-ratio denominator so nulling values in a unique column does not read as a uniqueness loss. Make the ratio critical band inclusive (>= 50) to match the absolute path, and add a 1e-9 tolerance so exact 10%/50% boundaries do not fall just under on binary floats. Update the 50% regression case to CRITICAL and add NULL, boundary and partial-loss tests. --- src/scherlok/detector/cardinality.py | 90 +++++++++++++++++----------- tests/test_detector.py | 62 ++++++++++++++++++- 2 files changed, 116 insertions(+), 36 deletions(-) diff --git a/src/scherlok/detector/cardinality.py b/src/scherlok/detector/cardinality.py index a87bae6..3c31bb4 100644 --- a/src/scherlok/detector/cardinality.py +++ b/src/scherlok/detector/cardinality.py @@ -18,6 +18,10 @@ # above cannot classify a near-unique column as critical. UNIQUE_RATIO_WARNING_PCT = 10 UNIQUE_RATIO_CRITICAL_PCT = 50 +# Tolerance for binary floating-point boundaries (e.g. 1 - 0.9 is +# 0.09999999999999998, so a clean 10% loss would fall just under the warning +# band without this). +_RATIO_EPS = 1e-9 def detect_cardinality_anomalies( @@ -35,12 +39,14 @@ def detect_cardinality_anomalies( Returns anomalies when cardinality changes significantly. When the stored profile shows the column was near-unique - (distinct_count / row_count >= UNIQUE_RATIO) the distinct ratio - (distinct / rows) is compared instead of the absolute count. A + (distinct_count / non-null rows >= UNIQUE_RATIO) the distinct ratio + (distinct / non-null rows) is compared instead of the absolute count. A unique column that stays unique after a volume change is then silent, while a unique column that suddenly has duplicates still fires. - Falls back to absolute comparison when volume profiles are missing - or row counts are zero. + The denominator excludes NULLs (rows - null_count from each + distribution profile) so nulling values in a unique column does not + read as a uniqueness loss. Falls back to absolute comparison when + volume profiles are missing or non-null row counts are zero. """ anomalies: list[dict] = [] @@ -84,7 +90,9 @@ def detect_cardinality_anomalies( return anomalies # Near-unique path: use distinct ratio when stored was near-unique - # and both volume profiles provide valid row counts. + # and both volume profiles provide valid row counts. The denominator is + # non-null rows (rows - null_count) so NULLing values in a unique column + # does not read as a uniqueness loss. if current_vol is not None and stored_vol is not None: stored_rows = stored_vol.get("row_count") current_rows = current_vol.get("row_count") @@ -96,36 +104,48 @@ def detect_cardinality_anomalies( and isinstance(stored_card, int) and isinstance(current_card, int) ): - stored_ratio = stored_card / stored_rows - if stored_ratio >= UNIQUE_RATIO: - current_ratio = current_card / current_rows - change_pct = abs(current_ratio - stored_ratio) / stored_ratio * 100 - direction = "decreased" if current_ratio < stored_ratio else "increased" - # Keep the exact 50% regression case in the warning band; - # a loss beyond that boundary is critical. - if change_pct > UNIQUE_RATIO_CRITICAL_PCT: - anomalies.append({ - "table": table, - "type": "cardinality_change", - "message": ( - f"Column '{column}' distinct ratio {direction}: " - f"{stored_ratio:.3f} -> {current_ratio:.3f} " - f"({change_pct:.0f}% change)" - ), - "severity": Severity.CRITICAL, - }) - elif change_pct >= UNIQUE_RATIO_WARNING_PCT: - anomalies.append({ - "table": table, - "type": "cardinality_change", - "message": ( - f"Column '{column}' distinct ratio {direction}: " - f"{stored_ratio:.3f} -> {current_ratio:.3f} " - f"({change_pct:.0f}% change)" - ), - "severity": Severity.WARNING, - }) - return anomalies + stored_nulls = stored_dist.get("null_count", 0) or 0 + current_nulls = current_dist.get("null_count", 0) or 0 + if not isinstance(stored_nulls, int): + stored_nulls = 0 + if not isinstance(current_nulls, int): + current_nulls = 0 + stored_denom = stored_rows - stored_nulls + current_denom = current_rows - current_nulls + if stored_denom <= 0 or current_denom <= 0: + pass + else: + stored_ratio = stored_card / stored_denom + if stored_ratio >= UNIQUE_RATIO: + current_ratio = current_card / current_denom + change_pct = abs(current_ratio - stored_ratio) / stored_ratio * 100 + direction = "decreased" if current_ratio < stored_ratio else "increased" + # Inclusive on both bands to match the absolute path below; + # _RATIO_EPS keeps exact decimal boundaries (10%, 50%) + # from falling just under on binary floats. + if change_pct + _RATIO_EPS >= UNIQUE_RATIO_CRITICAL_PCT: + anomalies.append({ + "table": table, + "type": "cardinality_change", + "message": ( + f"Column '{column}' distinct ratio {direction}: " + f"{stored_ratio:.3f} -> {current_ratio:.3f} " + f"({change_pct:.0f}% change)" + ), + "severity": Severity.CRITICAL, + }) + elif change_pct + _RATIO_EPS >= UNIQUE_RATIO_WARNING_PCT: + anomalies.append({ + "table": table, + "type": "cardinality_change", + "message": ( + f"Column '{column}' distinct ratio {direction}: " + f"{stored_ratio:.3f} -> {current_ratio:.3f} " + f"({change_pct:.0f}% change)" + ), + "severity": Severity.WARNING, + }) + return anomalies change_pct = abs(current_card - stored_card) / stored_card * 100 direction = "increased" if current_card > stored_card else "decreased" diff --git a/tests/test_detector.py b/tests/test_detector.py index 03271a9..127c015 100644 --- a/tests/test_detector.py +++ b/tests/test_detector.py @@ -320,6 +320,8 @@ def test_volume_drop_on_unique_column_is_silent(self): assert anomalies == [] def test_duplicates_on_unique_column_still_fire(self): + # An exact 50% uniqueness loss is CRITICAL (>= on both bands, + # matching the absolute path). anomalies = detect_cardinality_anomalies( "orders", "id", @@ -329,10 +331,68 @@ def test_duplicates_on_unique_column_still_fire(self): self.STORED_VOL, ) assert len(anomalies) == 1 - assert anomalies[0]["severity"] == Severity.WARNING + assert anomalies[0]["severity"] == Severity.CRITICAL assert "decreased" in anomalies[0]["message"] assert "distinct ratio" in anomalies[0]["message"] + def test_exact_warning_boundary_fires_despite_float_repr(self): + # 1 - 0.9 is 0.09999999999999998 in binary floats; a clean 10% + # loss must still warn. + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 900}, + self.STORED_DIST, + {"row_count": 1000}, + self.STORED_VOL, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.WARNING + assert "distinct ratio" in anomalies[0]["message"] + + def test_partial_loss_below_critical_stays_warning(self): + # 1000 -> 800 is a 20% uniqueness loss: clearly warning, and clear + # of the 10% float edge. + anomalies = detect_cardinality_anomalies( + "orders", + "id", + {"distinct_count": 800}, + self.STORED_DIST, + {"row_count": 1000}, + self.STORED_VOL, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.WARNING + assert "distinct ratio" in anomalies[0]["message"] + + def test_nulling_unique_values_is_silent(self): + # 20% of a unique e-mail column nulled: 1,600 remaining values are + # still distinct, so the non-null ratio is unchanged and nothing fires. + anomalies = detect_cardinality_anomalies( + "users", + "email", + {"distinct_count": 1600, "null_count": 400}, + {"distinct_count": 2000, "null_count": 0}, + {"row_count": 2000}, + {"row_count": 2000}, + ) + assert anomalies == [] + + def test_null_aware_ratio_still_fires_on_real_duplicates(self): + # Same 20% NULLs, but only 800 distinct among the 1,600 non-null: + # uniqueness halved, so it must fire. + anomalies = detect_cardinality_anomalies( + "users", + "email", + {"distinct_count": 800, "null_count": 400}, + {"distinct_count": 2000, "null_count": 0}, + {"row_count": 2000}, + {"row_count": 2000}, + ) + assert len(anomalies) == 1 + assert anomalies[0]["severity"] == Severity.CRITICAL + assert "distinct ratio" in anomalies[0]["message"] + def test_low_cardinality_column_keeps_absolute_comparison(self): anomalies = detect_cardinality_anomalies( "users",