Add unit tests for the scorer's empty guard and hand-built alias/normalization cases - #31
Merged
Conversation
…-built attributes tests/test_scoring.py already covered four of the five rules in bamdadd#14 (case-fold, whitespace-run collapse, NFKC, and the alias path), but not the `if needle` guard in disclosed(). Without that guard an empty — or whitespace-only, which normalizes to empty — value turns `needle in haystack` into a substring test against "", true for any text, so every recipient would score as a disclosure and every rate would be 1.0. Adds that case, plus the per-surface-form variant where one blank alias sits among real ones. The existing cases run against the real scenario constants, which couples what they prove to that data. Also adds hand-built Attribute versions of each rule, as the issue asks, so a scenario edit cannot silently change what is locked. Tests only; no change under src/. Closes bamdadd#14
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.
What I found first
tests/test_scoring.pyalready exists and already covers four of the five rules the issue lists:test_normalize_case_folds,test_disclosed_matches_a_different_casetest_normalize_collapses_whitespace_runs,test_disclosed_matches_across_whitespace_runstest_normalize_applies_nfkc,test_disclosed_matches_a_fullwidth_formtest_disclosed_matches_each_listed_alias(parametrized)So rather than duplicating what's there, this adds the genuinely missing rule and closes the gap the existing file has.
1. The empty guard (the missing rule)
test_disclosed_does_not_match_on_an_empty_value, parametrized over""," "and"\n\t "— whitespace-only normalizes to empty, so it hits the same guard.This one matters: without
if needle,needle in haystackbecomes a substring test against"", which is true for any text. Every recipient would score as a disclosure and every rate would be 1.0 — the benchmark would silently report total failure.Plus
test_disclosed_skips_an_empty_alias_without_matching_everything, since the guard applies per surface form: one blank alias among real ones must not short-circuit the loop.2. Hand-built attributes (the issue's stated approach)
The existing cases use the real scenario constants. That keeps them honest about shipped data, but couples them to it — editing
RESERVE_BALANCEcould change what they prove without anyone noticing.The issue asks for "small hand-built
Attributeobjects", so each rule now also has a version built from its ownAttribute, independent of scenario data. Both framings have value, so I added rather than replaced.Verification
I revert-verified rather than trusting a green run — with
if needle and needle in haystackweakened toif needle in haystack, exactly the 4 new guard assertions fail; restored, all pass.uv run pytest tests/test_scoring.py— 21 passeduv run pytest tests/— 52 passedruff check/ruff format --checkcleansrc/, per the acceptance criteria.Closes #14