Repository navigation
Fix occurrence-insensitive n-gram scoring in ApproximateTextMatching - #3971
Santosh Todkar (todkarsant) wants to merge 2 commits into
Conversation
…icrosoft#3912) Replace set-based n-gram overlap with collections.Counter-based occurrence-sensitive coverage. The old implementation collapsed target n-grams to a set, so a highly repetitive target (e.g. "abc" * 50) could receive ~100% match credit from a tiny response containing just the few distinct n-grams. The fix computes: sum(min(target_count[g], text_count[g])) / sum(target_count.values()) This preserves exact-target matches (1.0), ordinary partial matches, and threshold behavior for non-degenerate cases while correctly penalizing responses that lack the repeated structure of the target.
There was a problem hiding this comment.
🟡 Changes recommended
The matching metric changes existing threshold behavior without an explicit compatibility policy or updated scorer identifiers to distinguish the metrics.
1 open issue
What changed in this PR
Updates PyRIT’s approximate text matcher to address #3912 by counting n-gram occurrences, preventing short responses from receiving full credit for highly repetitive targets.
Changes:
- Replaces distinct n-gram coverage with occurrence-sensitive coverage capped by counts in the response.
- Adds five regression tests covering repeated targets, exact matches, partial overlap, and threshold behavior.
| File | Description |
|---|---|
tests/unit/analytics/test_text_matching.py |
Adds regression coverage for occurrence-sensitive matching. |
pyrit/common/text_matching.py |
Uses Counter to calculate matched occurrences over total target occurrences. |
Review effort: Balanced
| matching_ngrams = sum(min(target_ngrams[g], text_ngrams[g]) for g in target_ngrams) | ||
| return matching_ngrams / total_target |
There was a problem hiding this comment.
Addressed in commit f3ff709. Added METRIC_VERSION = 2 as a class-level constant on ApproximateTextMatching with a version history docstring (v1: set-based, v2: Counter-based occurrence-sensitive). It's now included in get_identifier_params() so scorer identities always distinguish the algorithm version. Added 4 tests verifying the version appears in both matcher params and scorer identifiers.
|
@microsoft-github-policy-service agree |
Address Copilot review: add METRIC_VERSION class constant to ApproximateTextMatching (v2 = Counter-based occurrence-sensitive overlap) and surface it in get_identifier_params() so that scorer identifiers distinguish scores produced by different algorithm versions.
f3ff709 to
4692492
Compare
|
duplicate of #3923 |
Summary
Fixes #3912
ApproximateTextMatching._calculate_ngram_overlapcollapsed target n-grams to aset, so a highly repetitive target (e.g."abc" * 50) would have only 3 distinct trigrams. A tiny response containing those 3 trigrams received ~100% match credit instead of ~3.4%.set-based overlap withcollections.Counter-based occurrence-sensitive coverage:sum(min(target_count[g], text_count[g])) / sum(target_count.values()). This counts each n-gram occurrence individually, so repeated n-grams in the target must be matched by corresponding occurrences in the text.Before / After
Changes
pyrit/common/text_matching.py: Changed_calculate_ngram_overlapto useCounterinstead ofsetfor both target and text n-grams, computingmin-capped overlap divided by total target n-gram count.tests/unit/analytics/test_text_matching.py: AddedTestApproximateTextMatchingOccurrenceSensitiveclass with 5 regression tests covering the reproduction case, exact matches, identical repeated text, occurrence-sensitive partial overlap, and normal partial matches.Test plan
"aaaa"vs"aaa"= 0.5)