Two-sided span counting: precision per prediction span, recall per annotation - #185
Open
omri374 wants to merge 5 commits into
Open
Two-sided span counting: precision per prediction span, recall per annotation#185omri374 wants to merge 5 commits into
omri374 wants to merge 5 commits into
Conversation
Replace per-annotation counting of predictions in SpanEvaluator with a
two-pass scheme. The recall pass keeps existing semantics: each
annotation independently checks whether same-type predictions cover it
at IoU >= threshold (pairwise for one span, combined for several) and
becomes TP or FN. The precision pass then counts every prediction span
exactly once: num_predicted equals the actual span count, and each span
is either credited (participated in a successful match) or an FP.
Previously a prediction overlapping several annotations was counted
once per annotation, and a group of same-type spans was counted once
per group, so num_predicted could drift above or below the real span
count depending on gold layout, one annotation could be counted as both
FN and TP, and a blob prediction could earn double credit at lenient
thresholds. Precision is now (num_predicted - false_positives) /
num_predicted; true_positives counts covered annotations and may differ
from the number of credited predictions.
Also:
- Single-span coverage uses exact pairwise Span.iou; the combined-IoU
path is reserved for genuine multi-span coverage.
- Each span appears in exactly one confusion-matrix cell: a wrong-type
detection at >= threshold is one (ann_type, pred_type) cell, with no
fallback (ann_type, "O") / ("O", pred_type) entries.
- Removed the superseded scenario-dispatch helpers and dead matching
machinery (~370 lines).
- Documented the counting contract in docs/span_evaluation.md and
docs/span_matching_strategies.md, added typed docstrings throughout,
and added contract tests for all six overlap scenarios.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ut adding new sections Keep the docs' original structure; only fix the statements the two-sided counting change made inaccurate (precision formula, prediction counting, multi-span FP accounting) and compress the new blob-overlap coverage to a short section. Drop the added counting-rules and confusion-matrix convention sections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve conflicts with the 0.3.2 release and the hierarchy projection work: - CHANGELOG: 0.3.2 shipped after this branch was written, so the two-sided counting entries move from the 0.3.2 section to Unreleased > Behavior Changes, next to the deepest-annotated-ancestor projection entry. - span_evaluator.calculate_score_on_df: keep the allow_generic_entities parameter docstring from main inside this branch's typed docstring. - test_low_iou_descendants_use_the_projected_type: two NAME spans that jointly fail to cover a PERSON annotation now count one false positive per span (num_predicted 2, FP 2), as documented for two-sided counting, while still being attributed to the projected PERSON label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMHDXc7d43rZLiRrunemYY
negruber1
reviewed
Sep 10, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Wrong-type predictions can still be duplicated in the confusion matrix when they overlap multiple annotations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces two-sided span counting so recall is annotation-based while precision reflects actual prediction spans.
Changes:
- Reworks matching, precision calculation, and confusion-matrix accounting.
- Adds regression tests for fragmented and over-wide predictions.
- Updates documentation and changelog guidance.
File summaries
| File | Description |
|---|---|
presidio_evaluator/evaluation/span_evaluator.py |
Implements two-sided matching and metrics. |
tests/evaluation/test_span_evaluator.py |
Tests counting and overlap scenarios. |
tests/entity_mapping/test_hierarchical_evaluation.py |
Updates projected-label expectations. |
docs/span_evaluation.md |
Documents metric semantics. |
docs/span_matching_strategies.md |
Documents overlap handling. |
CHANGELOG.md |
Records behavioral changes. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
With level="both" (the default, also used by calculate_hierarchical_scores)
the per-type pass and the global PII pass share one EvaluationResult. The
precision pass wrote the ("O", type) confusion cell and the FP ModelError in
both passes, so every false positive was recorded twice at the binary level
and per-type results gained a spurious ("O", "PII") cell plus a duplicate
"PII" FP record at branch and detailed level. The PII pass now updates the
pii_* counters only, matching how the recall pass already handled FNs.
Document the confusion matrix as annotation-centric: each annotation is in
exactly one row cell, the "O" row holds predictions represented by no
annotation cell, and column totals are not the prediction ledger.
Tests: level="both" records each error once; no "PII" label leaks into
per-type results; binary/branch/detailed share one ledger (tp + fn ==
num_annotated, one record per FP/FN, row totals == num_annotated, pii_*
counters identical across levels and equal to the binary PII counts).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SMHDXc7d43rZLiRrunemYY
omri374
marked this pull request as ready for review
September 10, 2026 08:49
negruber1
previously approved these changes
Sep 10, 2026
At thresholds of 0.5 or below, two prediction types can both reach the threshold on the same annotation, and the recall pass wrote a cell for each: a TP annotation also gained a wrong-entity cell, and an FN annotation covered by two wrong types gained two. Row totals then exceeded num_annotated. The strongest match now claims the row: a same-type match wins, otherwise the wrong type with the highest IoU (ties broken by name). Spans of the losing types are plain false positives in the "O" row and carry an FP record but no WrongEntity record. This is the tie rule from the _compare_multiple_overlaps fix in the confusion-matrix consistency PR, applied to the two-sided matcher; its three consistency tests are included unchanged and pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMHDXc7d43rZLiRrunemYY
negruber1
approved these changes
Sep 10, 2026
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
SpanEvaluatorcounted predictions through the annotations they overlapped: a prediction span overlapping two annotations enterednum_predictedtwice, and a group of same-type spans jointly covering one annotation entered it once.num_predictedtherefore drifted both above and below the model's actual output depending on gold layout, an annotation could be counted as both FN and TP, and a blob prediction could earn double TP credit at lenient thresholds (a sloppy span covering two golds scored P=1.0, R=1.0 by being counted twice on both sides).This PR replaces that with two-sided counting — each metric counted in its natural unit, each unit counted exactly once:
num_predicted= actual span count; every span is either credited (participated in ≥1 successful match) or an FP. Precision =(num_predicted − FP) / num_predicted.The two numerators may legitimately differ (one wide span covering two annotations = 2 recall hits, 1 credited prediction), which is why precision is no longer
TP / num_predicted.Counting contract (all covered by tests)
Invariants:
TP + FN == num_annotated;num_predicted== actual emitted span count.Also in this PR
Span.iou; the combined-IoU path (slightly inflated at boundaries) is reserved for genuine multi-span coverage. Borderline single-span matches can flip at exact threshold values (see the "Cyprus ( Greek )" example below).(type, type)when covered,(type, predicted type)when a different type covers it at IoU ≥ τ,(type, "O")otherwise, so row totals equalnum_annotatedat every threshold. When several types reach the threshold on one annotation (possible at τ ≤ 0.5), the annotation's own type claims the cell, else the wrong type with the highest IoU; spans of the other types are false positives in the"O"row. This is the tie rule from Fix confusion matrix consistency for overlapping entity predictions #198, applied to the new matcher; Fix confusion matrix consistency for overlapping entity predictions #198's threeTestConfusionMatrixConsistencytests are included unchanged, so Fix confusion matrix consistency for overlapping entity predictions #198 is superseded by this PR. The"O"row holds only prediction spans represented by no annotation cell, so a wrong-type detection is no longer written to both(type, predicted type)and("O", predicted type). Column totals are not the prediction ledger: one prediction covering two annotations appears in two cells while counting once innum_predicted. Documented in a new "Confusion Matrix and Error Records" section ofdocs/span_evaluation.md.ModelErrorrecords are written by the per-type pass only. Withlevel="both"(the default, also used bycalculate_hierarchical_scores) the global PII pass shares the same result object; it now updates thepii_*counters and nothing else. Previously every FP was recorded twice at the binary level, and per-type results gained a spurious("O", "PII")cell plus a duplicate"PII"FP record at branch and detailed level._compare_single_overlaps,_compare_multiple_overlaps) collapsed into one uniform rule; dead helpers (_find_best_match,_check_if_matched_already,_handle_unmatched_predictions,_update_wrong_entities,_add_to_processed_predictions) deleted.docs/span_evaluation.mdanddocs/span_matching_strategies.mdcorrected where they described the old counting (precision formula, prediction counting, multi-span FP accounting) plus a short section on one prediction overlapping multiple annotations; typed docstrings added throughoutspan_evaluator.py.Merge with
mainmainhas been merged into this branch. Conflicts were limited to the CHANGELOG section placement and thecalculate_score_on_dfdocstring, which now also documents theallow_generic_entitiesparameter added onmain. One test from the hierarchy projection work (#196),test_low_iou_descendants_use_the_projected_type, encoded the old per-group FP count; it now expects one FP per failed span (num_predicted == 2,false_positives == 2), still attributed to the projectedPERSONlabel.Behaviour across hierarchical levels
calculate_hierarchical_scoresruns the same evaluator on the binary, branch and detailed projections, each withlevel="both". The following hold at every level, verified bytest_hierarchical_levels_share_one_ledgerat τ ∈ {0.5, 0.75, 0.9} and on the fullsynth_dataset_v2run described below:tp + fn == num_annotatedfor every type;num_predictedequals the independently counted span total.ModelErrorrecords equals the FP and FN counters.num_annotatedat every threshold; only the level's own labels appear in cells and error records (no"PII"leakage into branch or detailed results).pii_*counters are identical at all three levels and equal the binary level'sPIIper-type counts, because both collapse labels to PII before span creation.("PII", "PII"),("PII", "O")and("O", "PII")equal TP, FN and FP exactly.Effect on a real evaluation run
Notebook 5's pipeline was run on
data/synth_dataset_v2.json(1500 sentences): predict,CanonicalMapperwithsuppress_prediction_only()and the notebook's issue resolutions, thencalculate_hierarchical_scores(beta=2). The model was the default PresidioAnalyzerEnginewith spaCyen_core_web_lg(the OpenMed Hugging Face model used in the notebook was not available in the environment). Predictions were generated once, and the identical mapped DataFrame was scored with the old evaluator (main) and this branch at τ ∈ {0.5, 0.75, 0.9}.Headline numbers at τ = 0.75
num_predictednum_predicted(detailed)num_predictedis now the true span count. An independent count of prediction spans from the span builder gives 1730 at the binary level and, per type at the detailed level, DATE_TIME 375, LOCATION 356, PERSON 808, PHONE_NUMBER 56. The new evaluator reports exactly these; the old one was off in both directions (PII −166, PERSON +13, DATE_TIME −3, PHONE_NUMBER −1, CONTACT −1).TP + FNwas 609 against 604 annotations, new is 604.Examples from the dataset
Spans below are after skip-word merging, which is why some predictions appear joined.
Fragments over one annotation: one FP per fragment, not per group (sentence 52, τ = 0.75)
Gold LOCATION covers the address and
Sutri(merged across the skip words between them). Predictions: LOCATIONCrown Street Kishiev Squares, LOCATIONSutri. Combined IoU is below threshold.num_predictedSame pattern on a phone number (sentence 392, τ = 0.75). Gold CONTACT
0490 39 07 81; predictions CONTACT0490, DATE_TIME39 07, CONTACT81. Old: PHONE_NUMBERnum_predicted2, FP 1. New:num_predicted3, FP 2. The sentence really contains three PHONE_NUMBER spans (the two fragments plus the fax number).One blob over two annotations: counted once (sentence 109, τ = 0.75)
Gold PERSON
Tomomi, gold ORGANIZATIONHawkins , Richardson and Santana. The model tagged each name as PERSON, and skip-word merging joins them into a single PERSON span covering the whole sentence. It overlaps both annotations and matches neither.num_predictedSame pattern with a correct type on one side (sentence 407, τ = 0.75)
Prediction LOCATION
Australia Akifumi(merged across, but) overlaps gold LOCATIONAustraliaand gold PERSONAkifumi. Old: LOCATIONnum_predicted2, FP 2. New:num_predicted1, FP 1. This over-count is why old PERSON and LOCATION precision were slightly too low.Annotation counted as both TP and FN (sentence 669, τ = 0.5)
Gold LOCATION
13813 3784 Archwood Avenue; predictions DATE_TIME13813, LOCATIONArchwood Avenue St.. The old code processed the DATE_TIME group first and recorded an FN, then processed the LOCATION group and recorded a TP for the same annotation.num_annotatedFour more sentences (302, 346, 836, 1481) show the same double count.
Borderline single-span match (sentences 292 and 1255, τ = 0.5)
Gold LOCATION
Cyprus ( Greek ); predictions LOCATIONCyprus, DEMOGRAPHICGreek. Exact pairwise IoU is 0.4615. The old combined-IoU path computed 0.50 for the same pair and passed the threshold. New: FN plus FP instead of TP.Verification
test_two_sided_counting_semantics(the 6-scenario contract table, incl. the P =(np−fp)/np≠tp/npregression case and both ledger invariants),test_single_prediction_overlapping_multiple_annotations_counted_once(the original double-counting repro),test_level_both_records_each_error_once(duplicate records withlevel="both"),test_global_pass_leaves_no_pii_traces_in_per_type_results,test_hierarchical_levels_share_one_ledger(binary/branch/detailed invariants at τ ∈ {0.5, 0.75, 0.9}),test_annotation_row_is_claimed_by_strongest_match_at_tie, and Fix confusion matrix consistency for overlapping entity predictions #198'sTestConfusionMatrixConsistency(3 tests, unchanged). 732 unit tests pass on the merged tree; 9 pre-existing expectations updated (8 intest_span_evaluator.py, 1 intest_hierarchical_evaluation.py), each a direct consequence of the rules above.num_predictednow equals the independently-counted actual span total everywhere (old code was wrong on 6 of 18 types, in both directions).synth_dataset_v2.jsonas described above, including the per-level ledger checks.Downstream impact
tp/predictedfromper_typecounts (orpii_true_positives / pii_predicted) must switch to(predicted − fp)/predicted—tp/predictedcan now exceed 1.synth_dataset_v2.jsonwith the default Presidio analyzer, expect PII precision to step down by about 0.06 and PERSON precision to step up by about 0.01, with recall unchanged.ModelError.most_common_fp_tokens) no longer double count, since the PII pass no longer emits its own FP records.end+1phantom character, shifted ranges for subsequent spans) — now isolated to the genuine multi-span path and a candidate for a follow-up fix.🤖 Generated with Claude Code