fix(privacy): classify from every value, inline rules first, rule keys first, non-string labels, quarantine nullable columns - #401
Merged
Conversation
Root cause: classify_columns built its value sample from the first 200 non-null cells of each column (_SAMPLE_ROWS). Entity, value-regex, Luhn and context classification all read only that sample, so a column whose first 200 values held nothing sensitive stayed unclassified and no action reached later cells. The report gave no sign that a sample was used. Fix: classification now reads every distinct non-null value of a column, de-duplicated in first-seen order (falling back to str() de-duplication for unhashable cells). Entity detection runs detect_in_text over newline-joined chunks of at most 64,000 characters and takes the union, stopping once every entity type the config can report has been found. The value-regex classifier still reads each value; the Luhn and context checks read each chunk. Only the values fed to classification change; the detectors and patterns are untouched. The report metadata gains classification_values_scanned with the distinct value count per column. Closes #246
Root cause: classify_columns ranked every matching rule by classifier specificity alone (column-name > entity > regex > context), comparing inline and pack rules on the same scale; rule order only broke ties. An inline rule matching by entity type, value regex or context therefore lost to any pack rule matching the column name, which contradicts the PrivacyPolicy docstring and is the usual way to override a pack. Fix: rank each match by (is_inline, specificity), where an inline rule is one listed in policy.rules. Any in-scope inline match beats every pack rule; specificity still decides within each group, and ties still go to the earlier rule. Out-of-scope inline rules are filtered out before ranking, as before, and pack-only policies classify exactly as they did. The PrivacyPolicy docstring and the specificity comment now describe this. Closes #284
Root cause: _resolve_key checked rule.key_env, then policy.key_env, then rule.key, then policy.key. A rule with its own literal key was switched to the organisation-wide key as soon as the policy-level environment variable was set, so tokens changed between environments and joins against previously tokenized data broke. Fix: resolve rule settings first and policy defaults second: rule.key_env (when set to a non-empty value), rule.key, policy.key_env, policy.key. A rule that sets no key resolves exactly as before (policy.key_env, then policy.key), and tokenize with no key anywhere still raises. PrivacyRule and PrivacyPolicy docstrings state the order. Closes #285
Root cause: classify_columns keys its results by str(label), and apply_privacy_policy then indexed the frame with that string. Any frame with integer (or other non-str) column labels that had a classified column raised KeyError, although fd.clean accepts such frames (headerless CSVs, DataFrame(ndarray)). Duplicate labels, or distinct labels with the same string form such as 1 and "1", could not be addressed by a single report key either. Fix: apply_privacy_policy maps each report key back to its original label and uses that label to read, write and drop the column; report keys stay strings. classify_columns and apply_privacy_policy raise ValueError for duplicated labels and for labels that collide once stringified. Only the label lookup changes; masking, hashing and tokenize defaults are untouched. Refs #232
… a TypeError Root cause: the quarantine action in apply_privacy_policy substitutes its string placeholder with Series.where. Nullable Int64 and boolean arrays (and categoricals) cannot hold a string, so pandas raised TypeError and the whole policy run failed, while object, string, numpy numeric and datetime columns were quarantined normally. Fix: when Series.where rejects the placeholder for the column's dtype, cast the column to object and substitute there, as an object column behaves. Missing cells stay missing and every dtype that already worked is unchanged. Only the quarantine substitution changes. The Int64 and boolean quarantine cases that tests/test_privacy_missing_and_labels.py had excluded are re-enabled.
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)
6 tasks
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
One commit per fix. Builds on #399 (merged).
#246: classify policy columns from every distinct value
classify_columnsbuilt its sample from the first 200 non-null cells of each column, so PII after row 200 was never classified and no action reached it.detect_in_textover newline-joined chunks of at most 64,000 characters and takes the union. It stops once every reportable entity type has been found.detect_in_textare unchanged.classification_values_scanned(distinct values read per column).#284: inline rules take priority over pack rules
Rules were ranked by classifier specificity across inline and pack rules alike, so an inline rule matching by entity, regex or context lost to a pack column-name rule.
(is_inline, specificity): any in-scope inline match beats every pack rule.PrivacyPolicydocstring and the specificity comment describe this.#285: a rule's own key before policy defaults
_resolve_keycheckedpolicy.key_envbeforerule.key, so setting an organisation-wide variable silently changed the tokens of a rule with its own key.rule.key_env>rule.key>policy.key_env>policy.key.PrivacyRuleandPrivacyPolicydocument the order.#232 (part 5): non-string column labels
apply_privacy_policyindexed the frame withstr(label), so integer labels raisedKeyError.classify_columnsandapply_privacy_policyraiseValueErroron duplicate labels, or on labels that collide once stringified (1and"1").Quarantine on nullable integer, boolean and categorical columns
The
quarantineaction substituted its string placeholder withSeries.where, which raisesTypeErroron nullableInt64/booleanand categorical columns, failing the whole policy run.tests/test_privacy_missing_and_labels.py(fix(privacy): missing values stay missing, categorical k-anonymity, duplicate labels, fpe audit metadata, NER status #399) are re-enabled.Behaviour changes
keybeatspolicy.key_env.ValueError.Int64/boolean/categorycolumns come back as object dtype.Tests
New
tests/test_privacy_policy_classification.py:detect_in_textcall counter: calls equal ceil(chars / limit), repeated values cost one call, and detection stops early after one chunkrule.key_envbeatsrule.keyValueErroris pinned['<EMAIL>', '<EMAIL>']1/"1"collision and duplicate-label errors for both functionsInt64,boolean,string,categoryand object columns withpd.NA.Verification
ruff check .: passedmypy src/freshdata: no issuespytest -m "not online and not large"on main 78b7790 + this branch:pytest tests/truthbench: 250 passedCloses #246
Closes #284
Closes #285
Refs #232