[AdamoElProfesor]

Fixes elementary-data/elementary#2370

Problem

Weekly buckets run Monday to Monday, but on Postgres datediff(..., 'week') starts weeks on Sunday. In get_start_bucket_in_data, every Sunday row was assigned to the next bucket:

• weekly metrics were shifted by one day (each week got the previous Sunday's rows);
• the last Sunday before max_bucket_end pointed to a bucket that doesn't exist, so the left join produced a metric row with a NULL bucket_start / bucket_end, and the sort(attribute='bucket_end') added in 0.26.0 failed with '<' not supported between instances of 'NoneType' and 'str'.

Fix

For weekly buckets, get_start_bucket_in_data now counts days and groups them by 7 (week × N becomes day × 7N). Since min_bucket_start is truncated to the start of the week, this lands every row in the right bucket, whatever the adapter considers the first day of the week. Other periods are unchanged.

Test

Added test_column_anomalies_weekly_buckets_with_sunday_rows: 12 weeks of data with nulls only on Sundays and a different count each week. It checks that the test passes, that no metric has a NULL bucket, and that each weekly null_count matches its Monday-to-Sunday week. It failed on master with the error from the issue and passes with the fix. The existing anomaly suites (column, dimension, volume, all columns, backfill, ranges, freshness) still pass on Postgres.

Note: get_metric_buckets_min_and_max also uses datediff(..., 'week') to compute max_bucket_end. I left it out of this PR to keep it focused, happy to look at it separately.

Summary by CodeRabbit

• Bug Fixes
  • Fixed weekly null-count anomaly monitoring for data recorded on Sundays. Weekly buckets now start on Monday and report the expected null counts for each week.

[github-actions]

👋 @AdamoElProfesor
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in the elementary repository.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ba946394-fed3-4d19-9f0f-3bf4b40db1d0

📥 Commits

Reviewing files that changed from the base of the PR and between a5f9f7a and 8a7558e.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ea433568-f922-44b6-9916-450e760c543d

📥 Commits

Reviewing files that changed from the base of the PR and between 5d22e8d and c1958ef.

📒 Files selected for processing (2)

• integration_tests/tests/test_column_anomalies.py
• macros/edr/data_monitoring/monitors_query/get_start_bucket_in_data.sql

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The weekly bucket calculation now converts the period to days and scales the count by seven. An integration test checks weekly null_count anomaly results for Sunday-only rows, including bucket start dates and stored counts.

Changes

Weekly column anomaly buckets

Layer / File(s)	Summary
Weekly bucket calculation and regression test macros/edr/data_monitoring/monitors_query/get_start_bucket_in_data.sql, integration_tests/tests/test_column_anomalies.py	The macro converts weekly periods to day periods and multiplies the bucket count by seven. The integration test checks that Sunday-only rows produce Monday bucket starts and expected stored null counts.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c1958

This change makes weekly column anomaly buckets count in days, so Sunday rows land in the correct Monday-to-Monday week. It adds a regression test for this case. No concrete merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: calculating weekly bucket offsets in days so Sunday rows are assigned to the correct week.
Linked Issues check	✅ Passed	Issue [#2370] requires weekly column_anomalies runs on Postgres to avoid NULL buckets and the resulting Python comparison error for Sunday rows. get_start_bucket_in_data now converts weekly period…
Out of Scope Changes check	✅ Passed	The changed macro directly implements the weekly bucket correction. The added integration test verifies the linked failure mode. No unrelated change is shown. The separate `get_metric_buckets_min_and_…

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[EladBarkay]

Thanks @AdamoElProfesor, clean fix and a well-designed test! 🙏

Fix: verified on Postgres 16. date_trunc('week') starts on Monday while dbt-postgres's datediff('week') counts from Sunday, so counting days from min_bucket_start is the right approach. It also fixes the incremental path (incremental models and sources), where min_bucket_start is the backfill start and can be any weekday: with a Monday run followed by a Thursday run, master produces a NULL bucket and a false anomaly, and this PR fixes it.

Test: varying the Sunday null counts is a nice touch, since a one-day shift can't hide. Two things before we merge:

1. It assumes weeks start on Monday. On BigQuery (timestamp_trunc(..., week)) and SQL Server (DATEFIRST) weeks start on Sunday, so the isoweekday() == 1 check and the expected_null_counts lookup would fail there. Instead, you could compute each bucket's expected count from the data, as the Sunday nulls in [bucket_start, bucket_start + 7 days).
2. It doesn't check that every week is present. assert null_counts passes as long as one week exists, so a change that dropped weeks, including the latest one (the week that crashed), would still pass. Asserting that all 12 weeks are there would cover it.

Non-blocking follow-ups (happy to take these on our side): a two-run incremental test (Monday, then Thursday), a weekly volume_anomalies test, and the same fix for periods_until_max in get_buckets_configuration.sql:88.

Dismissing this approval until the test is fixed: the new test fails on SQL Server (CI run: 2026-07-26 is not a Monday). This is point #1 from my review: SQL Server starts weeks on Sunday, and BigQuery does too. Point #2 (checking that all 12 weeks are present) can be a follow-up. The macro fix itself looks good.

[EladBarkay]

Dismissing this approval until the test is fixed: the new test fails on SQL Server (CI run: 2026-07-26 is not a Monday). This is point #1 from my review: SQL Server starts weeks on Sunday, and BigQuery does too. Please compute each bucket's expected count from the data, as the Sunday nulls in [bucket_start, bucket_start + 7 days), instead of assuming Monday. Point #2 (checking that all 12 weeks are present) can be a follow-up. The macro fix itself looks good.

[EladBarkay]

Thanks again @AdamoElProfesor! Since this one matters, we picked it up in #1083 so we could finish it. Your commit is kept there, so you keep the credit. We couldn't push to your fork's branch, so we opened a new PR.

What #1083 adds on top of your fix:

• The test no longer assumes weeks start on Monday. It reads the week start from the buckets, checks that every full week is there, and computes each week's expected count from the data. That makes it pass on SQL Server, BigQuery and Fabric.
• The same days-instead-of-weeks fix for periods_until_max in get_buckets_configuration.sql. On Postgres, a run on a Sunday used to add an unfinished bucket.

We'll close this PR once #1083 is merged.