fix: K5/K6 walk every column, count K6 once per row, and get a real test - #101
Merged
Conversation
A same-day external review of 2.14.0 found three defects in tools/k_checker.py
before anyone relied on it:
1. check() only walked analyse()'s known-concept columns. Against
test/generate_wide.py's own injected fixtures that found 0 of 28 K5 defects,
because a real file's benefit and deduction columns (a card allowance, a
voluntary-insurance premium) are not all in preflight.py's closed CONCEPTS
vocabulary - and the docstring's own argument for why K5/K6 are safe to
automate ("ask nothing about any other column") gave no reason to limit the
walk to known ones. Fixed by reading every header on the sheet directly.
2. K6 counted a cause and its consequence as two findings: an unrounded class
supplement flows into gross by construction, so the same defect surfaced once
in its own column and once in БРУТО (20 injected rows produced 40 findings).
otchet.md forbids counting a chain twice and test/structural_test.py already
enforces one K6 finding per row for the model's own checks; this tool now
does the same, in header order, stopping at the first money-like column.
Money-like excludes day counts, percentages and hours by concept or by the
same words a real layout uses for them (a generic exclusion needed once K5's
fix made every header, not only known ones, eligible for K6 too).
3. test/k_checker_test.py tested a hand-built fixture with every defect landing
in a known-concept column, so it could not have caught either bug. It now
also runs against test/generate_wide.py across seeds and compares counts to
the manifest exactly - no less, no more, the way structural_test.py already
does for the model's checks - and is wired into the pre-commit hook and both
CI jobs so a future regression here is caught the same way. Two new
hand-built shapes pin the exact regressions: K5 in an unrecognised column,
K6 chained into a second column for the same row.
Verified against test/generate_wide.py at 300 seeds: K5 129/129, K6 130/130,
zero false positives. Confirmed the new test has teeth by reverting to the
pre-fix check() and watching 23 seeds go red before restoring it.
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
A same-day external review of 2.14.0 found three real defects in
tools/k_checker.pybefore anyone relied on it in production:preflight.py's known-concept columns — 0 of 28 found againsttest/generate_wide.py's own fixtures. The check only walkedanalyse()'s known-concept dict; a real benefit/deduction column (e.g. "Карта (за сметка на работодателя)") isn't in that closed vocabulary. Fixed by reading every header on the sheet directly — matches the docstring's own argument that K5/K6 need no column-meaning knowledge.structural_test.py's existing "one per row" convention andotchet.md's ban on counting a chain twice.test/generate_wide.pyacross seeds and compares counts to the manifest exactly, wired into the pre-commit hook (which now also fires ontools/changes, not justtest/) and both CI jobs (--seeds 300invalidate,--seeds 3000indeep-suites).Test plan
python test/k_checker_test.py --seeds 300— K5 129/129, K6 130/130, zero false positivescheck(), watched 23 seeds go red, restored itpython test/run_tests.py,python test/skill_test.py— unaffected, still greenpre-commithook fires correctly on this PR'stools/change and passes🤖 Generated with Claude Code
https://claude.ai/code/session_01XvbzEbQwxwxxHAtXfPMrno