fix(scoring): bound value/alias matching to whole tokens - #44
Open
dchaudhari7177 wants to merge 1 commit into
Open
dchaudhari7177 wants to merge 1 commit into
dchaudhari7177 wants to merge 1 commit into
Conversation
disclosed() matched attribute.value and each alias with a plain normalized substring test, so a digit-run form was found inside an unrelated longer number: the alias 7429 hit inside 974290, and 47318.22 inside 447318.229. Those are forbidden-flow hits, so the effect was a fabricated violation -- which THREAT_MODEL.md says a deterministic match must never produce. Confirm phrases already had _phrase_present; value/alias matching now has an equivalent bound in _form_present. The rule is "not adjacent to a word character", asserted only on the ends where the form is itself word-like. That conditionality is the point: a plain \b on both sides would reject every form starting or ending in punctuation, so "$22.50 per hour" would stop matching after "us$" -- while "47,318.22" must still be rejected inside "447,318.229". No punctuation is stripped from the surface forms, no paraphrase, no model in the loop. 11 tests cover both directions: the true digit-run match, the two reported false positives, the same over-match reached through an alias rather than the value, a currency value bounded by $ and end-of-string, punctuation bounds, a value alone in the output, an underscore identifier, a form starting with punctuation, and the shipped scenario attributes still matching their real surface forms. Five fail against the pre-fix scorer. The club/observatory/theatre scenario expectations are unchanged: the full suite passed before the new tests were added and passes after (77 total). Closes bamdadd#34
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.
Closes #34.
The false positives
disclosed()matchedattribute.valueand each alias with a plain normalized substring test, so a digit-run form was found inside an unrelated longer number:7429ticket 974290 was issued47318.22balance 447318.229 usdBoth are forbidden-flow hits, so the effect was a fabricated violation — precisely what
THREAT_MODEL.mdsays a deterministic match must never produce.The boundary
Confirm phrases already had
_phrase_present; value/alias matching now has an equivalent bound in_form_present. The rule is not adjacent to a word character, asserted only on the ends where the form is itself word-like.That conditionality is the whole design, and a plain
\bon both sides would be wrong in both directions:\bbefore$22.50 per hourrequires the preceding character to be a word character, so the alias would stop matching inpaid us$22.50 per hour— a false negative.47,318.22must still be rejected inside447,318.229.Normalization is unchanged and still documented: NFKC, case-fold, whitespace-run collapse. No punctuation is stripped from the surface forms, no paraphrase, no model in the loop.
Tests
11 cases in
tests/test_scoring.py, both directions:7429inthe reserve code is 7429.)$and end-of-stringid_7429_x) —_is part of a token, not a boundaryRESERVE_BALANCE/MEETING_DATEattributes still matching their real surface formsRevert-verified: five fail against the pre-fix scorer.
Scenario results unchanged
The club/observatory/theatre expectations are untouched — the full suite passed before the new tests were added and passes after (77 total).
ruff check,ruff format --checkandmypy srcclean.