fix: rates.py for_period ambiguity guard - #122
Merged
Merged
Conversation
Adversarial review of the rates_test.py extraction-coverage work found that for_period() had no uniqueness guard, unlike every other lookup in this module: load_flat()/_extract_one() and test/rates_test.py's own extract() all refuse (None, or a loud failure) when a pattern matches more than once, but for_period() returned the FIRST matching row via re.finditer() and never checked for a second one agreeing or not. A duplicated or decoy period row above the real one - the exact failure extract() exists to catch in the reference file itself - would silently hand for_period() callers (audit.py's B1/B4/B5/F5 checks) whichever value came first, not necessarily the right one. Proved it: a decoy row with a conflicting figure ahead of the real МРЗ row made for_period() return the decoy's 999.99 instead of 620.20, silently, before this fix; it now returns None, forcing the caller to refuse the check rather than audit against a wrong number - the whole reason this module exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvbzEbQwxwxxHAtXfPMrno
svedbg
force-pushed
the
fix/rates-for-period-ambiguity
branch
from
September 6, 2026 13:16
14cc4a9 to
33eda5c
Compare
Merged
svedbg
added a commit
that referenced
this pull request
Sep 6, 2026
Version bump only. scripts/rates.py ships with the plugin and its for_period() behaviour changed (PR #122, already on main): it used to return the first matching row for a period silently; it now refuses (None) when two rows disagree on the same period, rather than risk auditing against whichever row a table restructure happened to put first. Anyone who installed 2.19.1 via /plugin has no way to know whether their copy carries this fix without a version bump. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvbzEbQwxwxxHAtXfPMrno
svedbg
added a commit
that referenced
this pull request
Sep 6, 2026
Version bump only. scripts/rates.py ships with the plugin and its for_period() behaviour changed (PR #122, already on main): it used to return the first matching row for a period silently; it now refuses (None) when two rows disagree on the same period, rather than risk auditing against whichever row a table restructure happened to put first. Anyone who installed 2.19.1 via /plugin has no way to know whether their copy carries this fix without a version bump. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvbzEbQwxwxxHAtXfPMrno
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
1 participant
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
Found by adversarial review of the rates-extraction-coverage work (PR #119):
scripts/rates.py'sfor_period()had no uniqueness guard, unlike every other lookup in the module -load_flat()/_extract_one()andtest/rates_test.py's ownextract()all refuse when a pattern matches more than once, butfor_period()returned the FIRST matching row viare.finditer()with no check for a second one agreeing or not.A duplicated or decoy period row above the real one - the exact failure
extract()exists to catch in the reference file itself - would silently handfor_period()callers (audit.py's B1/B4/B5/F5 checks) whichever value came first. Proved it: a decoy row with a conflicting figure ahead of the real МРЗ row madefor_period()return the decoy's 999.99 instead of 620.20, silently. It now returnsNone, forcing the caller to refuse rather than audit against a wrong number.Touches shipped code (
skills/trz-expert/scripts/rates.py), not justtest/- kept as its own PR for that reason.Test plan
Noneinstead of the wrong value)🤖 Generated with Claude Code
https://claude.ai/code/session_01XvbzEbQwxwxxHAtXfPMrno