Deepen tests (32->164) & demos (5->20), harden errors, fix 3 detection/UX bugs#3
Open
cognis-digital wants to merge 1 commit into
Open
Deepen tests (32->164) & demos (5->20), harden errors, fix 3 detection/UX bugs#3cognis-digital wants to merge 1 commit into
cognis-digital wants to merge 1 commit into
Conversation
Tests: 32 -> 164. New focused suites for k-anonymity math, l-diversity math (including combined sensitive tuples), the 18 Safe Harbor rules (value + header detection), malformed/edge-case datasets, CLI exit codes and argument validation, SARIF 2.1.0 edge cases, and analyze_rows pass/fail composition. python -m pytest -q is green. Demos: 5 -> 20 narrated, offline, exit-0 scenarios covering payer/claims, genomics technical ids, CI exit codes, JSON pipeline consumption, free-text leaks, cell suppression, non-comma delimiters, device telemetry, the k/l trade-off, operator errors, the full Safe Harbor catalog, before/after remediation, clean-vs-dirty SARIF, small-cohort singletons, and multi- attribute QIs. run_all.py and docs/DEMOS.md updated; new synthetic fixtures added under demos/11-14. Bug fixes (public API unchanged): 1. SSN detection missed SSNs embedded in free text (the anchored ^...$ regex only matched whole cells). Added a bounded search so an SSN inside a clinical note is caught - the exact free-text leak the tool claims to find. The 3-2-4 dashed shape is distinct from phone numbers, so the search is false-positive safe. 2. Any bare 3-digit number >89 (e.g. a code, count, dosage, or ZIP3 prefix like 200/606) was mis-flagged as 'age > 89' (S3). Constrained the age heuristic to a plausible human age (90..125; oldest verified human is 122) to remove the false positives. 3. Passing --delimiter '\t' (a literal two-char token, a common shell mistake) crashed with an uncaught TypeError and exit 1 and no message. The CLI now resolves the \t / tab tokens to a real tab, and analyze_csv validates the delimiter is a single character with a clear ValueError. Error handling: - analyze_csv now raises a clear ValueError on an empty file (no header row) and rejects a non-single-character delimiter. - The CLI validates -k/-l are positive, that -l has --sensitive, and that -k/-l are given quasi-identifiers (previously gating on -k with no --qi silently PASSED - a dangerous false negative for a CI gate). It also catches IsADirectoryError/OSError and prints a readable error.
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
This PR roughly 4x's the depth of the repo with real work: it grows the test
suite from 32 -> 164 tests, the demo suite from 5 -> 20 narrated scenarios,
hardens error handling, and fixes three genuine detection/UX bugs -- all with the
public API unchanged.
Bug fixes
anchored
^\s*\d{3}-\d{2}-\d{4}\s*$regex, so it only matched when the SSNwas the entire cell. An SSN buried in a clinical note (
"... SSN 123-45-6789 to redact") escaped -- the exact free-text leak the tool claims to catch.Added a bounded search variant. The 3-2-4 dashed shape is distinct from phone
numbers, so the search is false-positive safe.
like
200(a count, dosage, code, or a ZIP3 prefix such as606) was flaggedas a patient age. Constrained the age heuristic to a plausible human age
(
90..125; the oldest verified human age is 122), eliminating the falsepositives while still catching real ages >= 90.
--delimiter '\t'crashed with an uncaughtTypeError. Passing theliteral two-character token
\t(a very common shell mistake) produced atraceback and a bare exit 1. The CLI now resolves
\t/tabto a real tab,and
analyze_csvvalidates the delimiter is a single character with a clearValueError.Error handling
analyze_csvraises a clearValueErroron an empty file (no header row) andrejects non-single-character delimiters.
-k/-lare positive, that-lis paired with--sensitive, and -- importantly -- that-k/-lare given--quasi-identifiers. Previously, gating on-kwith no--qisilentlyPASSED, a dangerous false negative for a tool used as a CI gate. It now also
catches
IsADirectoryError/OSErrorand prints a readable message.Tests (32 -> 164)
New focused suites:
test_k_anonymity.py,test_l_diversity.py(incl. combinedsensitive tuples),
test_safe_harbor.py(all 18 categories, value + headerdetection, the new regression cases),
test_malformed.py(empty/header-only/ragged/BOM/unicode/duplicate-header datasets and error paths),
test_cli.py(every documented exit code + argument validation + output formats),
test_sarif_edge.py(rule catalog, DEID-K/L results, path normalization), andtest_analyze.py(pass/fail composition,smallest_classes).python -m pytest -qis green.Demos (5 -> 20)
Fifteen new runnable, offline, exit-0 scenarios (see
docs/DEMOS.md), plus newsynthetic fixtures under
demos/11-14.demos/run_all.pyruns all twenty end toend under
PYTHONUTF8=1.