Investigate: NULL/NaT dates may silently corrupt date-based metrics (RFM, cohort, churn, timeline) #536

What this is

While hardening CLVStats NULL handling I confirmed (with a repro) that a NULL/NaT transaction_date silently corrupts its output, and fixed it there (it now filters undated rows at ingest). A three-agent read-through of the rest of openretailscience/ then flagged the same code pattern in several other metrics. Those flags are from static analysis, not reproduced — this ticket is to verify each candidate, confirm or dismiss it, and decide the right handling. Treat the claims below as hypotheses with line numbers, not confirmed bugs.

Root pattern to check for

A NULL/NaT in a date column is handled inconsistently:

• min() / max() / first / last over a date column skip NULLs.
• ibis/SQL GROUP BY on a date (or a truncate/cast("date") of one) keeps NULL as its own group; .count() = COUNT(*) counts it.
• .delta() / date subtraction / period math yield NULL → NaN.
• pandas groupby(..., dropna=True) (the default) silently drops NaT groups.

core.validation.ensure_tznaive_datetime checks the column type and rejects tz-aware, but does not reject NULL values — so nothing catches this class today. CLVStats (branch feature/experimental-pareto-nbd-adapter) is the reference fix: data.filter(transaction_date.notnull()) at ingest, documented.

Candidates to investigate

For each: write a repro (a customer/row with a NaT date), confirm the described failure, then decide handling (filter the row, drop the customer, raise, or "not actually a problem").

1. segmentation/rfm.py:214 — RFMSegmentation._compute_rfm. Hypothesis: an all-NaT customer gets recency_days = NULL (via max().delta(...)) but is not dropped (frequency/monetary still count), so the NULL-recency row enters the NTILE / percent_rank scoring and shifts recency bucket boundaries for every other customer. Verify whether the row survives scoring and whether bucketing actually shifts.

2. analysis/cohort.py:47/192/202 — CohortAnalysis._calculate_cohorts / _periods_between. Hypothesis: a NULL date → period_shopped = NaT survives the second group_by; _periods_between → NaN → .astype("int64") (line 86) raises IntCastingNaNError on modern pandas (whole cohort table errors out). This is the highest-severity candidate if real (crash, not silent) — verify first with a real repro.

3. analysis/customer.py:80 — _distinct_customer_days (shared helper). Hypothesis: truncate("D") of NULL + .distinct() keeps a phantom (customer_id, NaT) "purchase day". This helper feeds Mkdocs pipeline #4 and Testing pipeline Trigger #5, so confirm this one first.

4. analysis/customer.py:371 — TransactionChurn._calculate. Hypothesis: the phantom NaT day is in the row_number()/lead() window before the churn filter drops it; depending on the backend's NULL sort order, a churned customer is counted as retained (churn under-counted) or transaction-number buckets shift. Backend-dependent — worth checking on duckdb + at least one of mssql/oracle.

5. analysis/customer.py:242 — DaysBetweenPurchases._calculate. Likely least severe — it filters gap_days.notnull(), so no NaN result; the residual question is only whether a real gap adjacent to the phantom day is dropped from the mean. May be acceptable / not worth fixing.

6. plots/broken_timeline.py:113 — plot. Different mode: pandas groupby(dropna=True) silently drops NaT-period rows → an understated timeline (silent data loss, not corruption). Verify and decide whether a plot warrants a guard.

Reported clean by the audit (spot-check, don't re-audit deeply)

segmentation/{segstats,nlr,threshold,hml}, core/validation, analysis/{revenue_tree,cross_shop,product_association,composite_rank,gain_loss,customer_decision_hierarchy,haversine}, analysis/customer.PurchasesPerCustomer, plots/{index,price}, utils/{date,label}, experimental/metrics/distribution/{pct_of_stores,acv} — these group on non-temporal keys, use COUNT(col) (NULL-safe), or exclude NULL dates via a .between()/equality predicate.

If confirmed, decide a consistent policy

Two shapes to weigh:

• A shared helper (extend ensure_tznaive_datetime, or add ensure_no_null_dates) in core/validation.py, called wherever a metric ingests a date column; or
• Each date-consuming metric filters NULL dates at ingest, like CLVStats now does.

Note the correct semantics may differ per metric (cohort crash vs. RFM mis-binning vs. timeline drop) — dropping the row, dropping the customer, and raising are all defensible depending on the metric.

Acceptance criteria

• Each candidate above has a repro test and is marked confirmed / dismissed.
• Confirmed ones either fixed (with the chosen policy) or split into their own follow-up issues.
• A decision recorded on whether to add a shared NULL-date guard vs. per-metric filtering.

Source: audit run while hardening CLVStats; covered all of openretailscience/.