Share the duplicated tool helpers and cut four empty claims - #80
Merged
Conversation
Eleven byte-identical text readers and writers across seven modules, two copies of a hand-rolled XML escape and two copies of column_number all collapse into one implementation. read_text, write_text and column_number now live in tools/sanitise_workbook.py, which every caller already imports through its sys.path entry, and the escape is xml.sax.saxutils.escape, which is the same three replacements in the same order. The parser fixture that exec'd the removed xml_escape drops it with the function. test_regression_tests_remain_proportionate_to_production_tools asserted a test-to-tool line ratio of at least 0.50. It proves nothing about behaviour and fails on a legitimate trim, so it and its only helper, physical_code_lines, are gone. The ruff-format pre-commit hook has no CI counterpart, and python -m ruff format --check . reports 45 files it would reformat, so the hook goes rather than the repository being reformatted; CONTRIBUTING.md no longer promises it. Prose: docs/australian-modelling.md names the front-loaded expense profile instead of gesturing at it, docs/workbook-use.md drops a claim that fits any spreadsheet, two CHANGELOG.md kicker sentences and its two em dashes go, and llms.txt opens with what the library actually ships. Verified: ruff check, mypy, verify_workbook, verify_sources, verify_signatures, verify_previous_names, verify_index, verify_afe all pass, and unittest discover runs 216 tests OK (218 before the two deleted tests). Breaking the shared reader to return "" fails 22 postbuild tests; restored. No workbook, src/, AFE store or functions.csv change.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewPowered by Qodo |
ryanduguid
added a commit
that referenced
this pull request
Sep 9, 2026
ryanduguid
added a commit
that referenced
this pull request
Sep 9, 2026
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 this changes
Tools only;
ozzit.xlsx,src/,functions.csvand the AFE store are untouched.test_regression_tests_remain_proportionate_to_production_tools(a test-to-tool line ratio of 0.50 that proves nothing about behaviour and fails on any legitimate trim) andphysical_code_lines, which had no other caller.read_textandwrite_textpair intools/sanitise_workbook.py, which the postbuild modules already reach; every copy was checked identical by AST hash before merging.xml_escapebodies becomexml.sax.saxutils.escape(fuzzed against the old body on 3,469 inputs; identical for&,<,>and quotes). The copy inside the frozen v3.0.0 provenance rebuild stays.column_numberwas defined twice with identical bodies; one lives insanitise_workbook.py, which both callers already import (keeping either original would have pulled a mutating tool into a read-only gate or the reverse).ruff-formatpre-commit hook goes:ruff format --checkwould reformat 45 files today and no CI gate checks formatting, so the hook rewrote files nothing verified. CONTRIBUTING says the hooks run the pinned ruff check.docs/australian-modelling.mdnames the profile (front-loaded, the author's own wording from the lease schedule's help) instead of "the profile AASB 16 is known for" and drops the kicker;docs/workbook-use.mddrops the sentence that fits any spreadsheet and keeps the specific volatility facts; CHANGELOG loses two bold kickers and two em dashes;llms.txtopens on the workbook's own facts (138 functions, no VBA or add-ins) instead of the shared template sentence.Why
Verification
Mutation check: stubbing the shared
read_textto return an empty string fails 22 postbuild tests plus one error; restored, 42 postbuild tests OK.Checklist
.envcontents, appear in the diff, the commit messages or this description.