fix: per-dimension alert descriptions when multiple dimensions fail#971
Open
fix: per-dimension alert descriptions when multiple dimensions fail#971
Conversation
When a dimension_anomalies (or volume_anomalies with dimensions) test detects failures across multiple dimension values, the alert description previously only showed the last row's description. Now it shows each failing dimension's details individually (up to 5), or a count summary with a sample of dimension values when more than 5 dimensions fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
👋 @joostboon |
📝 WalkthroughWalkthroughAdds two tests for dimension-anomaly alert descriptions, refactors anomaly result description generation to cap shown dimension values (max 5) and summarize excess, and tweaks Decimal exponent handling in adapter serialization. Duplicate test blocks appear in the diff context. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration_tests/tests/test_dimension_anomalies.py`:
- Around line 357-389: Reformat the anomalous_dimensions list in
test_dimension_anomalies_alert_description_many_failures to satisfy Black
line-length rules (split items across multiple lines) and add runtime
diagnostics to debug why not all six heroes are flagged: before the final
assertion log the test_result["test_results_description"] and inspect the
anomaly scoring output used by the anomaly detection routine (check where
sensitivity is applied and the score->anomalous threshold is computed) for the
heroes in anomalous_dimensions; verify whether sensitivity=2 is appropriate or
whether the scoring calculation is suppressing scores below the anomalous
threshold and adjust the scoring or threshold logic accordingly so the macro
condition (">5") correctly triggers for six failing dimensions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de59188d-8e4e-499a-a477-fac1b7d05334
📒 Files selected for processing (2)
integration_tests/tests/test_dimension_anomalies.pymacros/edr/data_monitoring/anomaly_detection/store_anomaly_test_results.sql
Comment on lines
+357
to
+389
| def test_dimension_anomalies_alert_description_many_failures( | ||
| test_id: str, dbt_project: DbtProject | ||
| ): | ||
| """When >5 dimension values fail, description shows a count summary.""" | ||
| utc_today = datetime.utcnow().date() | ||
| test_date, *training_dates = generate_dates(base_date=utc_today - timedelta(1)) | ||
|
|
||
| # 6 dimension values all spike on test_date (>5 threshold) | ||
| anomalous_dimensions = ["Batman", "Superman", "Spiderman", "IronMan", "Thor", "Hulk"] | ||
|
|
||
| data: List[Dict[str, Any]] = [ | ||
| {TIMESTAMP_COLUMN: test_date.strftime(DATE_FORMAT), "superhero": hero} | ||
| for hero in anomalous_dimensions | ||
| for _ in range(10) | ||
| ] | ||
| data += [ | ||
| {TIMESTAMP_COLUMN: cur_date.strftime(DATE_FORMAT), "superhero": hero} | ||
| for cur_date in training_dates | ||
| for hero in anomalous_dimensions | ||
| ] | ||
|
|
||
| test_args = { | ||
| "timestamp_column": TIMESTAMP_COLUMN, | ||
| "dimensions": ["superhero"], | ||
| "sensitivity": 2, | ||
| } | ||
| test_result = dbt_project.test(test_id, DBT_TEST_NAME, test_args, data=data) | ||
| assert test_result["status"] == "fail" | ||
|
|
||
| description = test_result["test_results_description"] | ||
| # Should show the count summary for many failures | ||
| assert "dimension values are anomalous" in description, ( | ||
| f"Expected summary message in description: {description}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the macro threshold logic is correct
rg -n "max_dimension_alerts" --type sql
# Verify the condition uses > not >=
rg -n "anomalous_rows.*length.*max_dimension_alerts" --type sql -A 2 -B 2
# Check if there are any existing tests that verify >5 threshold behavior
rg -n "dimension values are anomalous" integration_tests/Repository: elementary-data/dbt-data-reliability
Length of output: 2518
🏁 Script executed:
#!/bin/bash
# Get more context of the test function
sed -n '340,395p' integration_tests/tests/test_dimension_anomalies.py
# Look at the anomaly detection macro logic in detail
cat -n macros/edr/data_monitoring/anomaly_detection/store_anomaly_test_results.sql | sed -n '80,110p'
# Check if there's a specific dimension anomaly detection macro
fd -t f "dimension" macros/ | head -20Repository: elementary-data/dbt-data-reliability
Length of output: 4172
🏁 Script executed:
#!/bin/bash
# Find the dimension anomalies test macro
cat -n macros/edr/tests/test_dimension_anomalies.sql | head -100
# Check what DBT_TEST_NAME is in the test file
rg -n "DBT_TEST_NAME" integration_tests/tests/test_dimension_anomalies.py -B 5 | head -20
# Check if the test data generation uses helpers
rg -n "generate_dates\|DATE_FORMAT\|TIMESTAMP_COLUMN" integration_tests/tests/test_dimension_anomalies.py | head -20Repository: elementary-data/dbt-data-reliability
Length of output: 5313
Fix Black formatting and investigate anomaly detection failure.
Two issues found:
-
Black formatting violation (line 362): The
anomalous_dimensionslist must be reformatted to comply with Black's line length rules. -
Test assertion failure (lines 385-389): The test expects
"dimension values are anomalous"in the description but it's not present. The macro logic is correct (threshold is 5, condition uses>), so the issue is that the anomaly detection algorithm is not detecting all 6 dimensions as anomalous despite the clear spike pattern. With sensitivity=2, investigate whether:- The sensitivity value is too strict for the data pattern
- The anomaly scoring calculation is not generating high enough scores for all dimensions to exceed the anomalous threshold
Add a debug print before the assertion to see how many dimensions are actually detected:
print(f"DEBUG: description={description}")Then inspect the anomaly scores being generated for each of the 6 dimensions during test execution to determine why some are not being flagged as anomalous.
Fix Black formatting
# 6 dimension values all spike on test_date (>5 threshold)
- anomalous_dimensions = ["Batman", "Superman", "Spiderman", "IronMan", "Thor", "Hulk"]
+ anomalous_dimensions = [
+ "Batman",
+ "Superman",
+ "Spiderman",
+ "IronMan",
+ "Thor",
+ "Hulk",
+ ]🧰 Tools
🪛 GitHub Actions: Run pre-commit hooks
[error] 362-366: Black formatting: file reformatted by hook and changes applied. Run 'pre-commit run --all-files' to reproduce locally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration_tests/tests/test_dimension_anomalies.py` around lines 357 - 389,
Reformat the anomalous_dimensions list in
test_dimension_anomalies_alert_description_many_failures to satisfy Black
line-length rules (split items across multiple lines) and add runtime
diagnostics to debug why not all six heroes are flagged: before the final
assertion log the test_result["test_results_description"] and inspect the
anomaly scoring output used by the anomaly detection routine (check where
sensitivity is applied and the score->anomalous threshold is computed) for the
heroes in anomalous_dimensions; verify whether sensitivity=2 is appropriate or
whether the scoring calculation is suppressing scores below the anomalous
threshold and adjust the scoring or threshold logic accordingly so the macro
condition (">5") correctly triggers for six failing dimensions.
- Reformat anomalous_dimensions list to satisfy Black 88-char line limit
- Fix TypeError in adapter_query_runner when Vertica returns Decimal
special values (Infinity/NaN): as_tuple().exponent is a string ('F'/'n')
for those cases, not an int, causing '>= not supported between str and int'
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the review! Fixed both issues in the latest commit:
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
integration_tests/tests/test_dimension_anomalies.py (1)
395-397:⚠️ Potential issue | 🟡 MinorApply Black’s assert formatting here to fix pre-commit.
The hook is still reformatting this assert block; commit the formatter output so CI stops failing.
Suggested formatting change
- assert "dimension values are anomalous" in description, ( - f"Expected summary message in description: {description}" - ) + assert "dimension values are anomalous" in description, f"Expected summary message in description: {description}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration_tests/tests/test_dimension_anomalies.py` around lines 395 - 397, Reformat the failing assert to match Black’s formatting and commit the change: update the assert that checks for "dimension values are anomalous" in description so the assertion message (f"Expected summary message in description: {description}") is wrapped in parentheses and placed on its own line per Black’s multiline assert style (i.e., the assert containing the literal "dimension values are anomalous" and the f-string message should use Black’s wrapped/multiline layout), then run Black or pre-commit and commit the resulting file.
🧹 Nitpick comments (1)
integration_tests/tests/test_dimension_anomalies.py (1)
393-397: Strengthen the>5summary assertion to validate count semantics.Right now this only checks a generic substring. It should also assert the expected count/remainder text for this 6-dimension fixture.
Suggested assertion tightening
description = test_result["test_results_description"] # Should show the count summary for many failures - assert "dimension values are anomalous" in description, ( - f"Expected summary message in description: {description}" - ) + assert "6 dimension values are anomalous." in description, f"Expected count summary in description: {description}" + assert "Showing first 5:" in description, f"Expected capped list summary in description: {description}" + assert "and 1 more." in description, f"Expected remainder summary in description: {description}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration_tests/tests/test_dimension_anomalies.py` around lines 393 - 397, The test currently only checks that test_result["test_results_description"] (variable description) contains "dimension values are anomalous"; tighten it by asserting the expected count semantics for this 6-dimension fixture—e.g., add an assertion that description also contains the concrete count phrase such as "5 of 6" or "5/6" (or the exact wording your app emits like "5 anomalous, 1 normal") so the message confirms 5 anomalous out of 6 dimensions rather than just the generic substring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@integration_tests/tests/test_dimension_anomalies.py`:
- Around line 395-397: Reformat the failing assert to match Black’s formatting
and commit the change: update the assert that checks for "dimension values are
anomalous" in description so the assertion message (f"Expected summary message
in description: {description}") is wrapped in parentheses and placed on its own
line per Black’s multiline assert style (i.e., the assert containing the literal
"dimension values are anomalous" and the f-string message should use Black’s
wrapped/multiline layout), then run Black or pre-commit and commit the resulting
file.
---
Nitpick comments:
In `@integration_tests/tests/test_dimension_anomalies.py`:
- Around line 393-397: The test currently only checks that
test_result["test_results_description"] (variable description) contains
"dimension values are anomalous"; tighten it by asserting the expected count
semantics for this 6-dimension fixture—e.g., add an assertion that description
also contains the concrete count phrase such as "5 of 6" or "5/6" (or the exact
wording your app emits like "5 anomalous, 1 normal") so the message confirms 5
anomalous out of 6 dimensions rather than just the generic substring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69af3958-2dbc-42fd-83c1-59e767708153
📒 Files selected for processing (2)
integration_tests/tests/adapter_query_runner.pyintegration_tests/tests/test_dimension_anomalies.py
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integration_tests/tests/test_dimension_anomalies.py (1)
393-397: Strengthen the >5 assertion to validate the full summary contract.Right now the test only checks one substring, so it can still pass if count/cap wording regresses. Consider asserting count + cap + remainder too.
Proposed assertion hardening
description = test_result["test_results_description"] - # Should show the count summary for many failures - assert ( - "dimension values are anomalous" in description - ), f"Expected summary message in description: {description}" + # Should show the count-based summary for many failures + assert "6 dimension values are anomalous" in description, ( + f"Expected anomalous count in description: {description}" + ) + assert "Showing first 5" in description, ( + f"Expected capped preview text in description: {description}" + ) + assert "and 1 more." in description, ( + f"Expected remaining-count text in description: {description}" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration_tests/tests/test_dimension_anomalies.py` around lines 393 - 397, The current assertion only checks a substring in description; strengthen it to validate the full summary contract by asserting count, cap and remainder are present and correct: extract the numeric values from test_result (e.g. total anomalies, cap limit and remainder or derive remainder = total - cap) and then assert description matches the full summary format (either via a constructed expected string or a regex) instead of only checking "dimension values are anomalous"; use the existing variables test_result and description to locate and verify the values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@integration_tests/tests/test_dimension_anomalies.py`:
- Around line 393-397: The current assertion only checks a substring in
description; strengthen it to validate the full summary contract by asserting
count, cap and remainder are present and correct: extract the numeric values
from test_result (e.g. total anomalies, cap limit and remainder or derive
remainder = total - cap) and then assert description matches the full summary
format (either via a constructed expected string or a regex) instead of only
checking "dimension values are anomalous"; use the existing variables
test_result and description to locate and verify the values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc1a7bec-c0ef-4378-b2d6-546349c7dc53
📒 Files selected for processing (1)
integration_tests/tests/test_dimension_anomalies.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dimension_anomaliestest, the alert description previously only showed the last row's description (one dimension's info), ignoring all others|)"N dimension values are anomalous. Showing first 5: dim1, dim2, ..., and M more."Root cause
In
store_anomaly_test_results.sql,test_results_descriptionwas always set torows_with_score[-1].anomaly_description— a single row. All dimension values are grouped into one test result (grouped byfull_table_name, column_name, metric_name), so multiple failing dimensions were silently discarded from the description.Changes
macros/edr/data_monitoring/anomaly_detection/store_anomaly_test_results.sql: Move the failures loop before description computation soanomalous_rowsis available; build dimension-aware description with a 5-dimension thresholdintegration_tests/tests/test_dimension_anomalies.py: Add two tests for the new description format (≤5 and >5 failing dimensions)Test plan
dimension_anomaliesintegration tests still pass (logic unchanged for non-dimension tests and single-dimension failures)test_dimension_anomalies_alert_description_few_failures: 3 spiking dimensions → each appears in descriptiontest_dimension_anomalies_alert_description_many_failures: 6 spiking dimensions → summary count message appears🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Tests
Bug Fixes