fix(cdc): null-key ordering checks, zero stale_after, epoch event times, replay_risk docs - #387
Merged
Merged
Conversation
…es, replay_risk docs - Null CDC keys (#325): the per-key running watermark used groupby(key).cummax(), which drops null keys, so rows with a null key were never classified as out_of_order or late. Group with dropna=False so null-key rows form one group of their own (works on pandas 1.5 and 2.x), and report them as a new `missing_key` warning (row labels as samples). The trust penalty formula is unchanged. - Zero stale_after (#326): the freshness penalty divided the age by stale_after, raising ZeroDivisionError for "0s". With stale_after == 0 the penalty is now the limit of age / stale_after: 1.0 for any positive age and 0.0 otherwise, matching the `stale` defect (age > stale_after). A negative stale_after now raises ValueError. - Integer epoch event times (#327): numeric event-time columns went through pd.to_datetime without a unit and were read as nanoseconds, so Debezium ts_ms values landed in 1970. Parse them with the shared parse_timestamps helper, which infers the unit from magnitude as clean_timeseries does, and add `event_time_unit` ("s", "ms", "us", "ns") to set it explicitly. String and datetime columns are unchanged. - replay_risk (#328): behaviour kept. The replay_threshold docstring now says at least one duplicate_key row is required, so a late-only batch never raises replay_risk. Closes #325 Closes #326 Closes #327 Closes #328
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
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
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
Fixes four
fd.cdc_profilebugs:dropna=True, so rows with a null key were never classified asout_of_orderorlate. Grouping now usesdropna=False, so null-key rows form one group of their own. A newmissing_keywarning reports how many rows have a null key. The trust penalty formula is unchanged.stale_after=0no longer raisesZeroDivisionError(cdc_profile raises ZeroDivisionError when stale_after is zero #326). The freshness penalty isage / stale_after. At zero it now takes the limit of that formula: 1.0 for any positive age and 0.0 otherwise, which matches thestaledefect. A negativestale_afterraisesValueError.ts_mswere read as nanoseconds, giving 1970 dates. They now use the sharedparse_timestampshelper (added in fix(timeseries): epoch and mixed-offset timestamps, MAD zero-scale, stable anomaly columns, cdc tz #360), with the same unit guess asclean_timeseries. A newevent_time_unitkeyword ("s","ms","us","ns") sets the unit explicitly.replay_riskdocs match behaviour (cdc_profile replay_risk never fires for late-only batches, contrary to its docstring #328). Per the maintainer decision, the code is unchanged. Thereplay_thresholddocstring now says at least one duplicate-key row is required, so late-only batches never raisereplay_risk.Behaviour changes: numeric event-time columns get a guessed epoch unit instead of always nanoseconds (small integers such as row numbers are read as seconds); key columns with nulls add a
missing_keywarning (no effect onpassedor penalties); negativestale_afterraisesValueError.Tests
New
tests/test_cdc_profile_fixes.pycovering each issue's repro plus:stale_afterevent_time_unitreplay_riskbehaviourVerification
ruff check .: passesmypy src/freshdata: no issuesCloses #325
Closes #326
Closes #327
Closes #328