Skip to content

fix(freshcore): count duplicate rows natively for detection-only dedup - #405

Merged
kevincostner17 merged 2 commits into
mainfrom
fix/freshcore-native-duplicate-count
Sep 15, 2026
Merged

kevincostner17 merged 2 commits into
mainfrom
fix/freshcore-native-duplicate-count

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Summary

With drop_duplicates=False (the default), the FreshCore native module never counted duplicate rows. So engine="freshcore" recorded no detection and no duplicate_threshold warning, and duplicate_ratio_action="error" raised only through a pandas fallback. #375 already fixed the adapter side; this PR does the native part.

  • crates/freshcore: the duplicates kernel now has one duplicated_mask, shared by a new count_duplicates and by drop_duplicates. As in pandas, a frame with no rows or no columns has no duplicates.
  • execute_plan: counts duplicates at the same stage as the pandas dedup step (after string cleaning, empty-row removal and casts; before imputation and outliers), returns duplicates_detected, and records a detect_duplicates stage timing.
  • Adapter: it already reports duplicates_detected the way pandas does. It keeps its pandas fallback under duplicate_ratio_action="error" for native modules built before this change, which don't report the count.
  • CI: a new freshcore-native job runs cargo test, builds the extension with maturin, and runs the FreshCore tests against it. It uses a pinned Rust toolchain (1.98.1), SHA-pinned actions and a 10-minute timeout. No other job installs the extension, so the native parity tests are skipped everywhere else.
  • Docs: docs/freshcore.md (new "Building and testing" section), docs/fallback-matrix.md, docs/backends.md, CHANGELOG.

Behaviour change: engine="freshcore" with detection-only dedup now reports detected duplicates, warns above the threshold and raises DuplicateRatioError natively under "error", all matching pandas.

Tests

  • Rust: unit tests for count_duplicates, for duplicated_mask under first/last, that the count equals the rows dropped, and for frames without rows or columns.
  • tests/test_execution/test_freshcore_native_parity.py (skipped without freshdata_freshcore). Native runs use fallback_policy="error", so any fallback fails the test.
  • test_missing_native_module_falls_back_to_pandas now simulates the missing module, so it passes where the extension is built.

Verification

  • cargo test --locked: 12 passed. cargo clippy: no warnings in the changed code.
  • With the native module built via maturin develop: the parity file passed (28 tests), and pytest tests/test_execution -k freshcore passed (98 tests, 1 skipped for pyspark).
  • Full fast lane without the extension: Python 3.12 5573 passed; Python 3.9 5569 passed.
  • ruff check . and mypy src/freshdata: clean.

Closes #323

With drop_duplicates=False the native module never counted duplicates, so
engine="freshcore" recorded no detection, no duplicate_threshold warning,
and duplicate_ratio_action="error" never raised natively.

The native module now counts full-row duplicates at the pandas dedup stage
(after string cleaning, empty-row removal and casts; before imputation and
outliers), returns it as duplicates_detected and records a detect_duplicates
stage timing. The duplicates kernel shares one duplicated_mask between
counting and dropping, and, like pandas, finds no duplicates in a frame
without columns.

The adapter keeps its pandas fallback under duplicate_ratio_action="error"
for native modules built before this change, which do not report the count.

Closes #323
Adds a freshcore-native job: installs a pinned Rust toolchain with rustup,
runs the crate's cargo tests, builds freshdata_freshcore with maturin develop
and runs the FreshCore tests against it. No other job installs the extension,
so the native parity tests skip everywhere else.
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0a27c9c7-0e1a-4138-96ff-7d6b24f96ed3


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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

@kevincostner17
kevincostner17 merged commit 849d57f into main Sep 15, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FreshCore engine skips duplicate detection; duplicate_ratio_action='error' never raises

1 participant