fix: remove row(s) equal-to/not-equal-to conditions were inverted

[iabaako]

Pull Request Summary 🚀

What does this PR do? 📝

Fixes two logic bugs found while auditing every Prep step for inverted/incorrect condition handling:

1. RemoveRowsOperation._filter_by_equality — the "remove row(s)" action with condition value is equal to was inverted: it kept only the matching rows instead of removing them, and value is not equal to had the same problem in reverse (it kept only the non-matching rows). Both conditions now do what they say: equal to removes the matches and keeps the rest; not equal to removes the non-matches and keeps only the matches.
2. AddNewColumnOperation's count aggregation — pl.concat_list(cols).list.len() counted every source column regardless of nulls, so count always just returned the number of selected columns instead of the number of non-null values in them (as documented: "count of non-null values from selected columns"). Now uses .list.drop_nulls().list.len().

As a direct consequence, this also fixes #246: that issue reported that the Stata replication script generator (replication/prep_script_generator.py) and the real in-app Polars engine disagreed on equal_to/not_equal_to row-removal direction, and assumed the replication script needed to change to match Polars. It was actually the reverse — the Stata generator was already correct, and Polars was the buggy side. Fixing _filter_by_equality here brings them back into agreement; verified directly that both sides now produce the same remaining rows for equal_to and not_equal_to conditions on the same data.

Closes #246

Why is this change needed? 🤔

Both were found by manually verifying every Prep operation's behavior against its labeled intent, prompted by a user noticing "remove row(s)" seemed to behave backwards for equality conditions. The bug was long-standing and had been silently encoded as "expected" in the existing test suite (test comments literally said # equal_to keeps matching rows (removes non-matching)), so it was never caught by CI. Cross-checked against the replication script generator (replication/prep_script_generator.py), which independently implements the same condition and was already correct (keep if col != value for "equal to") — confirming the direction of this fix and that exported Stata scripts were never affected, only the live in-app engine.

How was this implemented? 🛠️

• _filter_by_equality: build the "is in value list" filter once, then apply it (not negated) for not_equal_to and negated for equal_to — the opposite of what the code did before.
• count aggregation: added .list.drop_nulls() before .list.len() so nulls aren't counted.
• Fixed the 3 existing tests that asserted the inverted equality behavior, and the 1 existing count test that asserted the always-equals-column-count behavior, to assert the correct outcome. Added new tests: equality with an explicit remaining-values assertion (not just row counts, so this can't silently regress again), and a count test with an all-missing row (expects 0).

How to test or reproduce? 🧪

1. uv run python -m pytest — full suite passes.
2. just lint-py / just fmt-python / pre-commit — clean.
3. Manual: add a "remove row(s)" prep step with condition "value is equal to X" — rows matching X are now removed, not kept.
4. Manual: add an "add new column" step with method "count" over columns that have missing values — the new column now reflects the actual non-null count per row instead of always equaling the number of selected columns.
5. Confirmed for Stata replication scripts may invert equal_to/not_equal_to row-removal conditions #246: the real Polars removal and the generated Stata keep if command now agree on the same remaining rows for equal_to/not_equal_to examples.

Screenshots (if applicable) 📷

N/A — bug fix, no new visual components.

Checklist ✅

• I have run and tested my changes locally
• I have limit this PR to less than 1000 lines of code change (if not, explain why)
• I have updated/added tests to cover my changes (if applicable)
• I have updated/added requirements to cover my changes (if applicable) — N/A, no new dependencies
• I have run linting and formatting on any code changes (if applicable)
• I have updated the documentation (README, etc.) accordingly — N/A, no user-facing docs changes needed
• I have reviewed and resolved any merge conflict

Note on stacking

This PR is stacked on fix/261-datetime-parse-crash (#262), itself stacked on fix/253-standardize-reapply-error-handling (#260) — both still open drafts. Base is set to fix/261-datetime-parse-crash so review only shows this PR's incremental diff. Retarget to main once the earlier PRs in the stack merge.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud