fix(pii): break weighted-score ties by confidence in the NER scanner - #31961
Conversation
get_highest_score_label ranks candidates by "score * appearances * 0.8" and picks
the winner with max(). On an exact tie max() returns whichever key was recorded
first, so the classification depended on the order the sample rows happened to be
scanned in rather than on the evidence.
A column of dashed US SSNs hits that tie. The customised registry's
us_driving_license patterns include ^\d{3}-\d{2}-\d{4}$ at 0.3, which matches every
row, while US_SSN only matches a subset (a sequential-digit sample is rejected and
another reads as US_ITIN):
US_DRIVER_LICENSE 0.3 * 5 * 0.8 = 1.200
US_SSN 0.5 * 3 * 0.8 = 1.200
The first row produces only US_DRIVER_LICENSE, so it is recorded first and a weak
0.3 guess outranks a 0.5 match, tagging national IDs as driving licences.
Adding the raw confidence as the secondary key makes the stronger match win and
removes the dependency on insertion order. Where both the weighted total and the
confidence are equal the outcome is unchanged, so the existing expectations hold.
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Khairajani
added
safe to test
The tie-break only engages when the weighted totals land on the same number. The
SSN-shaped licence pattern matches every dashed 3-2-4 value unconditionally while
US_SSN matches only the subset passing Presidio's validator, so the licence
appearance count is always >= the SSN one. Solving 0.3n > 0.5k puts the crossover at
60%: any column where fewer than 60% of the sampled values validate was labelled a
driving licence, which is ordinary for masked, placeholder or synthetic SSNs.
["000-12-3456", "666-45-6789", "111-00-2222", "333-44-0000", "543-21-0987"]
US_DRIVER_LICENSE 0.3 * 5 * 0.8 = 1.200 <- wins outright, no tie
US_SSN 0.5 * 1 * 0.8 = 0.400
Deleting the pattern is not the answer: Mississippi prints its nine-digit licence
numbers in Social Security positions, so the shape is a real licence format and
dropping it loses that detection entirely (a column of such values then matches
nothing). Scoring it very weak instead keeps the licence reading available when
nothing else claims the column, while a validated SSN match outranks it.
Measured against build_analyzer_engine() with presidio 2.2.358, before -> after:
dashed SSN, 3 of 5 validate US_SSN -> US_SSN
masked SSN, 1 of 5 validate US_DRIVER_LICENSE -> US_SSN
licence shape, none validate US_DRIVER_LICENSE -> US_DRIVER_LICENSE
ordinary licences (A1234567) US_DRIVER_LICENSE -> US_DRIVER_LICENSE
|
Confirmed and fixed in Reproduced case C against The 60% crossover is right: the licence pattern matches the shape unconditionally while US_SSN matches only what passes Presidio's validator, so its appearance count is always >= the SSN one. Deleting the pattern is not the fix. Mississippi prints its nine-digit licence numbers in Social Security positions ( Measured before -> after:
Both commits stay: the demotion fixes this pair, the tie-break fixes the general case where any two entities land on the same weighted total and the winner fell out of dict insertion order. Added a regression test covering the masked-SSN column. On co-firing: agreed that Presidio's dedup is same-entity-type only, so both results survive the span. I looked at de-duplicating across entity types by span, but it does not fix case C on its own — on the rows where the SSN validator rejects the value there is no competing result to lose to, so the licence match still accrues an appearance. The score is what has to change. |
The 0.3 x 5 illustration is now synthetic rather than something the SSN-shaped licence pattern still produces, so describe the general shape of the tie instead of attributing it to that pair.
|
Ran the suites for real now ( OSS unit tests — Control — same tests with only Downstream check — Collate's |
Code Review ✅ ApprovedUpdates the NER scanner to break weighted-score ties using raw confidence, preventing order-dependent classification mismatches between overlapping PII recognizers. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
2 participants
What
NERScanner.get_highest_score_labelnow breaks ties on the raw confidence, plus a regressiontest for the case below.
top_entity = max( entities_score, - key=lambda type_: entities_score[type_].score * entities_score[type_].appearances * 0.8, + key=lambda type_: ( + entities_score[type_].score * entities_score[type_].appearances * 0.8, + entities_score[type_].score, + ), )Why
Candidates are ranked by
score * appearances * 0.8and the winner is taken withmax(), whichon an exact tie returns whichever key was recorded first. The classification therefore depended
on the order the sample rows happened to be scanned in.
A column of dashed US SSNs lands exactly on that tie. The customised registry overrides
UsLicenseRecognizerwithpatterns.us_driving_license, which contains^\d{3}-\d{2}-\d{4}$atconfidence 0.3 — the shape of a dashed SSN — so it matches every row, while
US_SSNmatches onlya subset (
123-45-6789is rejected as sequential digits,987-65-4321reads asUS_ITIN):The first row yields only
US_DRIVER_LICENSE, so it is recorded first and wins the tie: a weak 0.3guess outranks a 0.5 match and national IDs get tagged as driving licences.
Reproduced against the real registry with presidio 2.2.358 on both macOS/arm64 and a Linux
container — same result in both, so this is not environmental. With the secondary key,
US_SSNwins.Scope
This surfaced when
NERScanner.__init__moved from a stockAnalyzerEnginetobuild_analyzer_engine()(#31890), which is what brings the customised driving-license patternsinto this scanner. That move looks intentional and is not reverted here — the defect is the
order-dependent tie-break, not the recognizer set. The same commit already made the sibling
ranking in
algorithms/utils.pydeterministic; this applies the equivalent treatment toget_highest_score_label.Where both the weighted total and the confidence are equal, the winner is unchanged, so the two
existing assertions in
test_get_highest_score_labelstill hold.Note on coverage
test_ner_scanner.py::test_scan_entitiesasserts onlytag_fqn == "PII.Sensitive"for thesesamples.
US_SSNandUS_DRIVER_LICENSEboth map toPII.Sensitive, so the flip is invisible toit — the added assertion checks the entity itself.
Greptile Summary
The PR makes NER entity selection deterministic when weighted totals tie and reduces the confidence of the SSN-shaped driving-license pattern to avoid displacing validated SSNs.
Confidence Score: 5/5
The PR appears safe to merge based on the available follow-up-review scope.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Analyze sampled column values] --> B[Aggregate confidence and appearances by entity] B --> C[Rank by confidence times appearances times 0.8] C --> D{Weighted totals tied?} D -- No --> E[Select highest weighted total] D -- Yes --> F[Select higher raw confidence] E --> G[Map entity to PII tag] F --> GReviews (3): Last reviewed commit: "test(pii): keep the tie-break comment ac..." | Re-trigger Greptile