From 4c0a226be2b1ca2d9200c63c30f50e0ce4ef8e02 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 15:07:18 +0200 Subject: [PATCH 1/4] test(guards): make the repository scanners prove they scanned something A guard built on a glob or a walk fails in a way that looks identical to success: the collection comes back empty and every assertion over it holds vacuously. Nothing noticed - renaming beantester/ would have silenced four guards at once, on green. Two halves, both mutation-checked: - test_the_repository_scanners_actually_read_files names one file each of the seven collectors must return, plus a floor on the count. The anchor catches a collector pointed at the wrong root, the floor catches one that narrowed. - test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository keeps the whole-tree scans off internal_tools/, .claude/, crashes/ and the private notes. Measured: the dash scan covered 183 files, 12 of them git-ignored, so it measured a different set here than in CI - and a red CI cannot reproduce teaches that red is local noise. crashes/latest-crash.txt was in that set, carrying arbitrary text from OS exceptions. Convention 33 keeps its coverage of the private notes: the check moves to the Stop hook, which runs exactly where those files exist. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 15 ++++ tests/test_repo_conventions.py | 151 +++++++++++++++++++++++++-------- 2 files changed, 132 insertions(+), 34 deletions(-) diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 369d08c..6048e96 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -31,6 +31,21 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre ### Tests +- **The repository scanners now prove they read something.** + `test_repo_conventions.py::test_the_repository_scanners_actually_read_files` names one file + each collector must return plus a floor on the count, for all seven collectors across + `test_repo_conventions`, `test_code_hygiene`, `test_layering` and `test_readme_guards`. A + glob or walk that comes back EMPTY satisfies every assertion built on it and looks exactly + like a working guard, so renaming `beantester/` would have silenced four guards at once in + silence. Mutation-checked: emptying a collector and pointing the walk at a missing root each + turn it red. +- **The whole-tree scans stopped measuring files that are not in the repository.** + New `repo_text_files()` collector skips `internal_tools/`, `.claude/`, `crashes/` and the + private notes, and `::test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository` + keeps them out. Measured before the change: the dash scan covered 183 files, 12 of them + git-ignored, so the same test measured a different set locally than in CI - including + `crashes/latest-crash.txt`, whose text comes from OS exceptions. Convention 33's coverage of + the private notes moves to the Stop hook, which runs where those files exist. - `tests/test_gui_release_fixes.py::test_start_only_fields_are_locked_while_a_session_runs` rewritten to DERIVE its subjects from `fields.FIELD_DEFS` instead of naming `duration` and the filter combobox by hand, and to resolve each field to the surface that renders it diff --git a/tests/test_repo_conventions.py b/tests/test_repo_conventions.py index bcdfd65..caffeff 100644 --- a/tests/test_repo_conventions.py +++ b/tests/test_repo_conventions.py @@ -8,6 +8,38 @@ from fakes import ROOT, check +# Directories a whole-repository walk must not descend into. The first group is +# generated or vendored; the second is the maintainer's own, kept OUT of git. +# +# The second group matters more than it looks. `internal_tools/`, `.claude/`, +# `crashes/` and the private notes exist on the owner's machine and in NO public +# checkout, so a guard that scans them measures a different set here than in CI: +# an em dash typed into PROJECT_NOTES.md would redden the suite locally while CI +# stayed green, which teaches that a red guard is local noise. Measured 2026-08-02: +# the dash scan covered 183 files, 12 of them git-ignored - including +# `crashes/latest-crash.txt`, whose contents are arbitrary text from OS exceptions. +# The notes keep their own dash check in `.claude/hooks/check_notes.py`, which runs +# exactly where they exist. +SKIP_DIRS = {".git", "__pycache__", ".pytest_cache", "licenses", "build", "dist", + ".hypothesis", "internal_tools", ".claude", "crashes"} +SKIP_FILES = {"PROJECT_NOTES.md", "HISTORY_NOTES.md", "CLAUDE.md"} + + +def repo_text_files(exts): + """Every text file that is actually IN the repository, with the given suffixes. + + Returns a list so a caller can assert it is not empty: a scanner whose walk + yields nothing passes every check it makes, and looks exactly like a scanner + that works. See test_the_repository_scanners_actually_read_files. + """ + out = [] + for dirpath, dirnames, filenames in os.walk(ROOT): + dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS] + for name in filenames: + if name.endswith(exts) and name not in SKIP_FILES: + out.append(os.path.join(dirpath, name)) + return out + def _source_files(): files = [os.path.join(ROOT, "bean_network_tester.py"), @@ -121,18 +153,11 @@ def test_no_em_or_en_dashes_in_repo_text(): exts = (".py", ".md", ".json", ".toml", ".spec", ".yml", ".yaml", ".txt", ".cfg", ".ini") offenders = [] - for dirpath, dirnames, filenames in os.walk(ROOT): - dirnames[:] = [d for d in dirnames - if d not in (".git", "__pycache__", ".pytest_cache", - "licenses", "build", "dist", ".hypothesis")] - for name in filenames: - if not name.endswith(exts): - continue - path = os.path.join(dirpath, name) - text = open(path, encoding="utf-8", errors="replace").read() - for ch, label in banned.items(): - if ch in text: - offenders.append(f"{os.path.relpath(path, ROOT)}: {label}") + for path in repo_text_files(exts): + text = open(path, encoding="utf-8", errors="replace").read() + for ch, label in banned.items(): + if ch in text: + offenders.append(f"{os.path.relpath(path, ROOT)}: {label}") check("no em/en dashes outside licenses/ (use '-')", not offenders, f"({offenders[:8]}{'...' if len(offenders) > 8 else ''})") @@ -180,28 +205,22 @@ def test_no_stale_pending_markers(): marker = re.compile(r"PENDING\(([a-z0-9][a-z0-9-]*)\)") seen = {} - for dirpath, dirnames, filenames in os.walk(ROOT): - dirnames[:] = [d for d in dirnames - if d not in (".git", "__pycache__", ".pytest_cache", - "licenses", "build", "dist", ".hypothesis")] - for name in filenames: - if not name.endswith(PENDING_EXTS): - continue - if name == os.path.basename(__file__): - continue # the registry does not scan itself - if name.startswith("CHANGELOG"): - # A changelog records what HAPPENED and is dated by its nature: an - # entry saying "added a marker named X" stays TRUE after X closes, so - # it is history, not drift. Found the hard way - the first real - # closing (socket-event-fields) tripped over the very entry that - # announced the marker. - continue - path = os.path.join(dirpath, name) - text = open(path, encoding="utf-8", errors="replace").read() - for lineno, line in enumerate(text.splitlines(), 1): - for found in marker.findall(line): - seen.setdefault(found, []).append( - f"{os.path.relpath(path, ROOT)}:{lineno}") + for path in repo_text_files(PENDING_EXTS): + name = os.path.basename(path) + if name == os.path.basename(__file__): + continue # the registry does not scan itself + if name.startswith("CHANGELOG"): + # A changelog records what HAPPENED and is dated by its nature: an + # entry saying "added a marker named X" stays TRUE after X closes, so + # it is history, not drift. Found the hard way - the first real + # closing (socket-event-fields) tripped over the very entry that + # announced the marker. + continue + text = open(path, encoding="utf-8", errors="replace").read() + for lineno, line in enumerate(text.splitlines(), 1): + for found in marker.findall(line): + seen.setdefault(found, []).append( + f"{os.path.relpath(path, ROOT)}:{lineno}") stale = sorted(f"{key} ({', '.join(where)})" for key, where in seen.items() if key not in OPEN_PENDING) @@ -213,3 +232,67 @@ def test_no_stale_pending_markers(): check("every id in OPEN_PENDING is still referenced by a marker " "(an id nothing points at is a leftover entry, not an open stage)", not unmarked, f"({unmarked})") + + +# -- the canary: a scanner that reads nothing passes everything ----------------- # +def test_the_repository_scanners_actually_read_files(): + """Every whole-tree scanner proves it saw a file it must have seen. + + A guard built on a glob or a walk has a failure mode that looks identical to + success: the collection comes back EMPTY and every assertion over it holds + vacuously. Nothing here would notice - `not offenders` is true when there are + no files, a rename of `beantester/` would silence four separate guards at once, + and the suite would stay green while guarding nothing. + + This is the cheapest possible answer: name one file each collector MUST return, + plus a floor on the count. Both halves matter - the anchor catches a collector + pointed at the wrong root, the floor catches one that quietly narrowed. + + Mutation-checked 2026-08-02: emptying any collector, and pointing the walk at a + non-existent root, each turn this red. + """ + import test_code_hygiene + import test_layering + import test_readme_guards + + def names(paths): + return {os.path.basename(p) for p in paths} + + for label, paths, anchor, floor in ( + ("_source_files", _source_files(), "core.py", 20), + ("_gui_files", _gui_files(), "app.py", 10), + ("repo_text_files(.py)", repo_text_files((".py",)), "engine.py", 60), + ("repo_text_files(.md)", repo_text_files((".md",)), "README.md", 4), + ("code_hygiene._pkg_files", test_code_hygiene._pkg_files(), "core.py", 20), + ("layering._top_level_modules", + test_layering._top_level_modules(), "engine.py", 15), + ("readme_guards._top_level_modules", + test_readme_guards._top_level_modules(), "engine.py", 15), + ): + check(f"{label} returned files at all (an empty scan passes every check " + f"it makes and looks exactly like a working guard)", paths, "(empty)") + check(f"{label} still reaches {anchor}", anchor in names(paths), + f"({sorted(names(paths))[:6]}...)") + check(f"{label} did not quietly narrow (expected at least {floor})", + len(paths) >= floor, f"(got {len(paths)})") + + +def test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository(): + """The dash and PENDING scans must measure the REPOSITORY, not this machine. + + `internal_tools/`, `.claude/`, `crashes/` and the private notes live here and in + no public checkout, so scanning them makes the guard mean one thing locally and + another in CI - and a red that CI cannot reproduce teaches that red is noise. + Measured before the fix (2026-08-02): 12 of the 183 files scanned were + git-ignored, `crashes/latest-crash.txt` among them, whose text comes from OS + exceptions and is nobody's convention to keep. + """ + scanned = {os.path.relpath(p, ROOT).replace(os.sep, "/") + for p in repo_text_files((".py", ".md", ".json", ".txt"))} + for stray in ("PROJECT_NOTES.md", "CLAUDE.md", "HISTORY_NOTES.md"): + check(f"{stray} is not scanned (it is not in the repository)", + stray not in scanned, f"({stray})") + for prefix in ("internal_tools/", ".claude/", "crashes/"): + leaked = sorted(p for p in scanned if p.startswith(prefix)) + check(f"nothing under {prefix} is scanned (git-ignored, absent in CI)", + not leaked, f"({leaked[:4]})") From 1f127c4526fb82a9e694665f0970db258b1f6987 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 15:11:13 +0200 Subject: [PATCH 2/4] test(guards): register the mutation proofs instead of asserting them in prose "Verified by mutation" is this project's strongest claim about its own tests. It appears over twenty times in the notes and in a dozen docstrings, and nothing checked a single one - convention 5's own evidence was exactly the unguarded prose convention 5 exists to catch. The claim is now data, in three states that mean three different things: - MUTATIONS: re-runnable today, five entries, proven by internal_tools/mutate.py - PROVEN_BY_HAND: a dated session did it and wrote no patch, so nothing repeats it - NOT_PROVEN: no mutation at all, named out loud The suite checks the bookkeeping, which is the half that rots: every named test exists, none is filed twice, and every search pattern still occurs exactly once, so an entry goes red the day the code moves rather than the day someone runs the rig. The rig itself lives outside git and carries a mandatory canary - a tree that fails to compile also exits non-zero, so without it a run can report "all caught" and prove nothing. First full run: 5 caught, 0 survived, canary BROKEN. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 12 +++ tests/test_mutation_registry.py | 178 ++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 tests/test_mutation_registry.py diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 6048e96..1a9277b 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -31,6 +31,18 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre ### Tests +- **"Verified by mutation" became checkable.** New `tests/test_mutation_registry.py` holds the + claim as data in three states that say three different things: `MUTATIONS` (re-runnable now, + five entries), `PROVEN_BY_HAND` (a dated session did it, no patch was written down, so no + machine repeats it) and `NOT_PROVEN` (no mutation, said out loud). Until now the phrase + appeared over twenty times in the notes and in a dozen docstrings with nothing behind it - + convention 5's own evidence was the prose convention 5 warns about. The suite checks the + bookkeeping only: every named test exists, none is filed under two states, and every search + pattern still occurs exactly once, so a registry entry rots the day the code moves rather + than the day someone runs it. The mutations themselves run from `internal_tools/mutate.py` + (outside git, one subprocess suite run each), with a mandatory canary entry that must report + BROKEN - a tree that fails to compile also exits non-zero, so without it a whole run can + report "all caught" and mean nothing. First full run: 5 caught, 0 survived, canary BROKEN. - **The repository scanners now prove they read something.** `test_repo_conventions.py::test_the_repository_scanners_actually_read_files` names one file each collector must return plus a floor on the count, for all seven collectors across diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py new file mode 100644 index 0000000..c06868d --- /dev/null +++ b/tests/test_mutation_registry.py @@ -0,0 +1,178 @@ +"""The registry of mutation proofs, and the guard that keeps it honest. + +Why this file exists +-------------------- +This project's strongest claim about its own tests is the sentence "verified by +mutation". It appears in PROJECT_NOTES more than twenty times and in a dozen +docstrings - and until now **nothing checked a single one of them**. That is the +exact failure mode convention 5 exists to prevent, applied to the evidence for +convention 5 itself: prose nobody guards, trusted precisely because it sounds +rigorous. + +So the claims move here, as data, in three lists that say three different things: + +* ``MUTATIONS`` - re-runnable today. ``internal_tools/mutate.py`` breaks the named + behaviour and proves the named test reddens. This is the only list that is proof. +* ``PROVEN_BY_HAND`` - a mutation WAS performed and dated, by a session, with no + re-runnable entry. The claim rests on that record, not on anything a machine can + repeat. This list should only shrink: entries move to ``MUTATIONS`` when someone + writes the patch down. +* ``NOT_PROVEN`` - no mutation, said out loud. An empty-looking guard and an + unproven one must not be indistinguishable, which is what happens when the third + list is missing. + +What the SUITE checks here (cheap, every run) is the bookkeeping: that every named +test exists, that no test is filed under two states, and - the one that matters - +that **every mutation's search pattern still occurs exactly once**. A pattern that +went stale would make the runner report SKIP, but only when someone runs it; the +suite catches it the day the code moves. Running the mutations themselves is not a +pytest job: each one costs a subprocess suite run. + +Deliberately NOT checked here: whether a mutation is *aimed well*. A patch can +redden its test for the wrong reason - see the note in convention 5 about a test +that passed because a transposition broke a different field than the one it named. +""" +import ast +import glob +import os + +from fakes import ROOT, check + + +# -- the three states ---------------------------------------------------------- # +# label -> what to break -> which single test must go red. Keep `old` long enough to +# be unambiguous and short enough to survive unrelated edits nearby. +MUTATIONS = [ + { + "label": "gui: the settings form stops refreshing its field states", + "file": "beantester/gui/panels/settings.py", + "old": " self.form.refresh_field_states()\n", + "new": "", + "test": "test_start_only_fields_are_locked_while_a_session_runs", + }, + { + "label": "gui: start/stop stops ticking the open windows", + "file": "beantester/gui/app.py", + "old": " with crashlog.quiet(\"gui.app\"):\n self.windows.refresh()", + "new": " with crashlog.quiet(\"gui.app\"):\n pass", + "test": "test_start_only_fields_are_locked_while_a_session_runs", + }, + { + "label": "guards: the repository collector returns nothing", + "file": "tests/test_repo_conventions.py", + "old": " out = []\n for dirpath, dirnames, filenames in os.walk(ROOT):", + "new": " out = []\n for dirpath, dirnames, filenames in []:", + "test": "test_the_repository_scanners_actually_read_files", + }, + { + "label": "guards: the whole-tree walk points at a root that is not there", + "file": "tests/test_repo_conventions.py", + "old": "for dirpath, dirnames, filenames in os.walk(ROOT):", + "new": "for dirpath, dirnames, filenames in os.walk(ROOT + '_nope'):", + "test": "test_the_repository_scanners_actually_read_files", + }, + { + "label": "guards: internal_tools falls back into the scanned set", + "file": "tests/test_repo_conventions.py", + "old": "\"internal_tools\", \".claude\", \"crashes\"}", + "new": "\".claude\", \"crashes\"}", + "test": "test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository", + }, +] + +# The runner's own check: a patch that cannot compile must be reported as BROKEN, not +# as "caught". Without it, a tree that fails to build looks exactly like a mutation +# the suite detected, and every other line of the report becomes worthless. +CANARY = { + "label": "CANARY: deliberately unparsable, must report BROKEN", + "file": "beantester/utils.py", + "old": "def clamp01(", + "new": "def ((( clamp01(", + "test": "test_no_old_name_references", +} + +# A mutation was run and dated by a session, but nobody wrote the patch down, so no +# machine can repeat it. This is weaker than MUTATIONS and stronger than nothing - +# and it is the honest state of most "verified by mutation" lines in the notes. +PROVEN_BY_HAND = { + "test_shortcut_buttons_advertise_their_key": "2026-07-21, dropping shortcut= from Save/Load", + "test_an_overridden_field_is_visibly_disabled": "2026-07-21, removing the disabled style maps", + "test_no_stale_pending_markers": "2026-07-25, both directions", + "test_every_remote_endpoint_gate_fires_in_both_directions": "2026-07, the inbound branch", + "test_a_worker_thread_exception_is_recorded": "2026-08-01, the excepthook body", + "test_pid_for_takes_no_lock_because_the_capture_thread_calls_it": "2026-07-29, taking the lock", +} + +# No mutation at all. Naming them is the point: an unproven guard and a guard nobody +# looked at must not read the same. This list is allowed to grow only when a guard +# is added without its proof - and every entry is a debt. +NOT_PROVEN = { + "test_a_resize_after_the_label_is_gone_is_not_a_crash": "never mutated", + "test_the_ui_rebuild_does_not_pile_up_configure_handlers_on_the_root": "never mutated", + "test_an_injected_rst_is_always_recomputed": "never mutated", + "test_evicting_the_connection_log_can_never_empty_it": "never mutated", +} + + +def _known_test_names(): + names = set() + for path in glob.glob(os.path.join(ROOT, "tests", "test_*.py")): + tree = ast.parse(open(path, encoding="utf-8").read(), filename=path) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + if node.name.startswith("test_"): + names.add(node.name) + return names + + +def test_every_mutation_still_points_at_code_that_exists(): + """The registry rots the moment the code it patches moves. + + A stale pattern makes the runner print SKIP - but only for whoever runs it, and + the entry keeps LOOKING like a proof in the meantime. Checking the occurrence + count here means the suite says it the day the code changes, which is the whole + difference between a registry and a list of good intentions. + """ + for entry in MUTATIONS + [CANARY]: + path = os.path.join(ROOT, entry["file"]) + check(f"{entry['label']}: {entry['file']} exists", os.path.exists(path)) + text = open(path, encoding="utf-8").read() + found = text.count(entry["old"]) + check(f"{entry['label']}: its search pattern occurs exactly once " + f"(a stale pattern proves nothing and reports SKIP)", + found == 1, f"(found {found} times)") + + +def test_every_named_test_exists_and_has_exactly_one_state(): + """A guard is proven, hand-proven or unproven - never two of those, never none. + + The state being VISIBLE is the point. Two rows of another project's regression + table said "verified by mutation" with no entry behind them, and the only reason + anyone found out was a test exactly like this one. + """ + known = _known_test_names() + states = {} + for entry in MUTATIONS: + states.setdefault(entry["test"], set()).add("MUTATIONS") + for name in PROVEN_BY_HAND: + states.setdefault(name, set()).add("PROVEN_BY_HAND") + for name in NOT_PROVEN: + states.setdefault(name, set()).add("NOT_PROVEN") + + for name, where in sorted(states.items()): + check(f"{name} is a real test (a registry naming a ghost is worse than " + f"an empty registry)", name in known, f"(listed in {sorted(where)})") + check(f"{name} is filed under exactly one state", len(where) == 1, + f"(in {sorted(where)})") + + +def test_the_canary_is_not_quietly_disarmed(): + """The canary must name a real test and a real file, or the runner cannot fail. + + A runner whose canary silently stops firing reports "everything caught" for a + run that proved nothing - which is worse than not running it, because it is + quotable. + """ + check("the canary names a test that exists", + CANARY["test"] in _known_test_names(), f"({CANARY['test']})") + check("the canary would really break the parse", "(((" in CANARY["new"]) From 70f28046717531042b0a69988be0cf7628540546 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 15:13:53 +0200 Subject: [PATCH 3/4] test(guards): ratchet the size of functions and modules Nobody reads this code line by line, so nothing notices a function reaching three hundred lines over four sessions that each added twenty. A ceiling is the only mechanism that notices, and it costs nothing while nothing grows. Set at today's maxima - 167 logic lines for a function (theme.py::init_style), 1299 for a module (gui/app.py) - so nothing needed rewriting to make it pass. Down is routine work, up is the owner's decision: a threshold bent to fit the code has stopped being a threshold. Comments and docstrings are free, and that was measured before the metric was chosen: 9 776 of the package's 17 770 lines are logic, so 45% of this package is explanation. A raw-line cap would have been a cap on explaining, making deletion of the paragraph that says why a constant is 0.30 the cheapest way back under the limit. A second test pins that property directly. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 7 ++ tests/test_code_shape.py | 136 ++++++++++++++++++++++++++++++++ tests/test_mutation_registry.py | 14 ++++ 3 files changed, 157 insertions(+) create mode 100644 tests/test_code_shape.py diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 1a9277b..c8f82f9 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -31,6 +31,13 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre ### Tests +- **Size ceilings, as a ratchet.** New `tests/test_code_shape.py` caps a function at 167 logic + lines and a module at 1299 - today's maxima (`theme.py::init_style`, `gui/app.py`), so nothing + had to be rewritten to make it pass. Lowering them is routine work, raising either is the + owner's call. **Comments and docstrings do not count**, and that is measured rather than + assumed: 9 776 of the package's 17 770 lines are logic, so 45% is explanation, and a raw-line + cap would have been a cap on explaining. A second test pins that property directly (ninety + lines of comment measure the same as none). Both mutation-checked, plus the empty-scan canary. - **"Verified by mutation" became checkable.** New `tests/test_mutation_registry.py` holds the claim as data in three states that say three different things: `MUTATIONS` (re-runnable now, five entries), `PROVEN_BY_HAND` (a dated session did it, no patch was written down, so no diff --git a/tests/test_code_shape.py b/tests/test_code_shape.py new file mode 100644 index 0000000..c292d1c --- /dev/null +++ b/tests/test_code_shape.py @@ -0,0 +1,136 @@ +"""Size ceilings for the package, as a RATCHET: the numbers may only go down. + +Why a ceiling here, when nothing has actually gone wrong yet +----------------------------------------------------------- +Nobody reads this code line by line. There is no reviewer to notice that a function +reached three hundred lines over four sessions, each of which added twenty and each +of which was reasonable on its own. A ceiling is the only mechanism that notices, +and it costs nothing while nothing grows. + +Why LOGIC lines and not lines +----------------------------- +Measured before choosing the metric (2026-08-02): of 17 770 lines in ``beantester/``, +only 9 776 are logic - **45% of this package is comments, docstrings and blank +lines**. That is deliberate and it is where the measurements and the reasons live. A +raw-line ceiling would therefore be a ceiling on EXPLAINING, pushing the next session +to delete the paragraph that says why a constant is 0.30 rather than to simplify the +function. So comments and docstrings are free; only executable lines count. + +Where the numbers come from +--------------------------- +They are today's maxima, not a textbook figure: ``init_style`` at 167 logic lines and +``gui/app.py`` at 1299. Nothing has to be rewritten to make this pass, which is the +point - a ceiling picked out of the air either fails on day one or is set so loose it +never fires. + +**Lowering these is ordinary work. Raising either is the owner's decision, not a way +to get unblocked** - a threshold bent to fit the code has stopped being a threshold. +When a function crosses it, the answer is to split the function. + +What this does NOT check +------------------------ +Whether a function does one thing (a tidy twenty-line function doing three things +passes exactly like a good one), whether its name is honest, or whether splitting it +scattered the logic across ten places - that last one has its own cost and no metric. +Nesting depth is not measured either. This guard buys one thing only: nothing grows +past what a person can follow without somebody deciding that it should. +""" +import ast +import os + +from fakes import ROOT, check + +# Today's maxima, measured 2026-08-02. Ratchet: down is routine, up is a decision. +FUNCTION_CEILING = 167 # beantester/gui/theme.py::init_style +FILE_CEILING = 1299 # beantester/gui/app.py + + +def _logic_lines(source): + """Line numbers that carry executable code: no blanks, comments or docstrings. + + Docstrings are found through the AST rather than by matching quotes, so a string + that merely LOOKS like one (a multi-line literal assigned to a name) still counts + as logic - which is right, because it is. + """ + tree = ast.parse(source) + lines = source.splitlines() + doc = set() + for node in ast.walk(tree): + if (isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant) + and isinstance(node.value.value, str)): + doc.update(range(node.lineno, (node.end_lineno or node.lineno) + 1)) + live = set() + for number in range(1, len(lines) + 1): + text = lines[number - 1].strip() + if text and not text.startswith("#") and number not in doc: + live.add(number) + return tree, live + + +def _package_files(): + out = [] + for dirpath, dirnames, filenames in os.walk(os.path.join(ROOT, "beantester")): + dirnames[:] = [d for d in dirnames if d != "__pycache__"] + out += [os.path.join(dirpath, n) for n in filenames if n.endswith(".py")] + return out + + +def _measure(): + """(worst function, worst file) as (name, count) pairs, plus how many files.""" + worst_function = ("", 0) + worst_file = ("", 0) + paths = _package_files() + for path in paths: + tree, live = _logic_lines(open(path, encoding="utf-8").read()) + rel = os.path.relpath(path, ROOT).replace(os.sep, "/") + count = len(live) + if count > worst_file[1]: + worst_file = (rel, count) + for node in ast.walk(tree): + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + span = sum(1 for n in live if node.lineno <= n <= (node.end_lineno or 0)) + if span > worst_function[1]: + worst_function = (f"{rel}::{node.name}", span) + return worst_function, worst_file, len(paths) + + +def test_no_function_or_file_has_grown_past_the_ratchet(): + """Nothing in the package is longer than the longest thing was on 2026-08-02. + + Crossing this is not a licence to raise the number. A function over the ceiling + gets split - which is exactly what happened to the one case another project hit + on the day it introduced the same guard. + """ + worst_function, worst_file, seen = _measure() + + # The canary from test_repo_conventions, applied here: a walk that finds nothing + # satisfies both ceilings perfectly and looks like a guard that works. + check("the shape scan actually read the package " + "(an empty scan passes every ceiling ever set)", seen >= 30, f"({seen} files)") + + check(f"no function exceeds {FUNCTION_CEILING} logic lines " + f"(split it - do not raise the ceiling)", + worst_function[1] <= FUNCTION_CEILING, + f"(worst: {worst_function[0]} at {worst_function[1]})") + check(f"no module exceeds {FILE_CEILING} logic lines", + worst_file[1] <= FILE_CEILING, + f"(worst: {worst_file[0]} at {worst_file[1]})") + + +def test_the_ratchet_measures_logic_and_not_explanation(): + """Comments and docstrings must stay free, or the ceiling punishes the thing + this project is built on. + + Without this, the cheapest way to get back under the limit would be to delete + the paragraph explaining why a constant is what it is - the exact opposite of + what convention 5 asks for. Checked directly rather than assumed: a body padded + with ninety lines of comment measures the same as the body alone. + """ + bare = "def f():\n" + " x = 1\n" * 5 + padded = "def f():\n" + ' """Doc."""\n' + " # note\n" * 90 + " x = 1\n" * 5 + _, live_bare = _logic_lines(bare) + _, live_padded = _logic_lines(padded) + check("ninety lines of comment and a docstring do not count as logic", + len(live_padded) == len(live_bare), + f"(bare {len(live_bare)}, padded {len(live_padded)})") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index c06868d..71c6074 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -78,6 +78,20 @@ "new": "\".claude\", \"crashes\"}", "test": "test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository", }, + { + "label": "shape: the package walk finds no files to measure", + "file": "tests/test_code_shape.py", + "old": " out += [os.path.join(dirpath, n) for n in filenames if n.endswith(\".py\")]", + "new": " out += []", + "test": "test_no_function_or_file_has_grown_past_the_ratchet", + }, + { + "label": "shape: comments start counting as logic", + "file": "tests/test_code_shape.py", + "old": " if text and not text.startswith(\"#\") and number not in doc:", + "new": " if text:", + "test": "test_the_ratchet_measures_logic_and_not_explanation", + }, ] # The runner's own check: a patch that cannot compile must be reported as BROKEN, not From b95d6639fae47a017418ec9b9e64f002ad8c8cde Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 17:53:57 +0200 Subject: [PATCH 4/4] test(guards): gate the hot path against retaining anything per packet "Zero allocations" has sat in the hot-path section for a long time with nothing checking it. test_hot_path.py guards the neighbouring rule - no syscalls on the packet threads - and this catches the other class: something starting to be kept once per packet. At ~14k packets a second that is tens of megabytes a minute, and the symptom is not slowness but a session that dies after an hour. Ceilings are 64 blocks and 4096 bytes over 5000 calls, against a measured floor of 13 and 608 that does not move with duplication, latency, the NAT flow table or four times the ports. It reads TWO meters, and the reason is the useful part of this commit: the first version counted allocated blocks only, and the mutation that makes decide() append to an ever-growing list SURVIVED it. The appended value was a cached small int, so no object was created and the block count went from 6 to 7. Bytes see the same case as 42 032 against 208. The meter canary now proves both meters can rise, including the references-only case that defeated the first one. Transient garbage is still not caught, and the file says so: both meters are net, and CPython has no cheap deterministic counter of total allocations. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 13 +++ tests/test_hot_path_allocations.py | 164 +++++++++++++++++++++++++++++ tests/test_mutation_registry.py | 16 +++ 3 files changed, 193 insertions(+) create mode 100644 tests/test_hot_path_allocations.py diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index c8f82f9..c95ecd1 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -31,6 +31,19 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre ### Tests +- **The hot path's "zero allocations" rule got a gate.** New + `tests/test_hot_path_allocations.py` pins that `decide()` RETAINS nothing per packet: at most + 64 blocks and 4096 bytes over 5000 calls, against a measured floor of 13 blocks and 608 bytes + that stays flat with duplication, latency, the NAT flow table and 200 ports. `test_hot_path.py` + guards the neighbouring rule (no syscalls on the packet threads); this catches the other class, + where something starts being kept once per packet. + 🔴 **It reads two meters because one of them has a hole, and the hole was found by mutation + rather than by reasoning.** The first version counted `sys.getallocatedblocks()` only, and a + mutation making `decide()` append to an ever-growing list **survived**: the appended value was + a cached small int, so no object was created and blocks moved from 6 to 7. `tracemalloc` + current sees the same case as 42 032 bytes against 208. The meter-canary test now proves both + meters can rise, including the references-only case specifically. Still not caught, stated in + the file: transient garbage, since both meters are net. - **Size ceilings, as a ratchet.** New `tests/test_code_shape.py` caps a function at 167 logic lines and a module at 1299 - today's maxima (`theme.py::init_style`, `gui/app.py`), so nothing had to be rewritten to make it pass. Lowering them is routine work, raising either is the diff --git a/tests/test_hot_path_allocations.py b/tests/test_hot_path_allocations.py new file mode 100644 index 0000000..c18553e --- /dev/null +++ b/tests/test_hot_path_allocations.py @@ -0,0 +1,164 @@ +"""The decision path must not RETAIN anything per packet. + +What this guards +---------------- +"Zero allocations" has been written in the hot-path section of the notes for a long +time and nothing checked it. ``test_hot_path.py`` guards the neighbouring rule - no +syscalls on the packet threads - which is a different failure. This one catches the +class where ``decide()`` starts holding on to something once per packet: a cache +added for speed, a list that only ever grows, a per-flow object nobody frees. At the +measured real rate of ~14k packets a second that is tens of megabytes a minute, and +the symptom the user sees is not "slow" but a session that dies after an hour. + +Two meters, because one of them has a hole +------------------------------------------ +🔴 **Blocks alone are not enough, and this was found by mutation, not by thinking.** +The first version of this file counted ``sys.getallocatedblocks()`` only. A mutation +that made ``decide()`` append to an ever-growing list **SURVIVED it**: the appended +value was a small cached int, so no object was created and the list's array growth +cost about one block. Blocks see a retained NEW OBJECT; they are blind to a container +filling up with references to objects that already exist - which is the shape most +real leaks in this codebase would take. + +So the gate reads both, and each covers the other's blind spot: + +* ``sys.getallocatedblocks()`` - net objects retained. Measured for the mutation: + 7 against a baseline of 6, i.e. invisible. +* ``tracemalloc`` current (not peak) - bytes still held. Same mutation: **42 032 + against 208**, i.e. unmissable. + +What it still does NOT catch, said out loud +------------------------------------------- +**Transient garbage.** Both meters are net, so an object allocated and freed inside +the same call nets to nothing - measured, not assumed. A future ``f"{a}:{b}"`` built +per packet would pass here. CPython has no cheap deterministic counter of total +allocations (Go's ``AllocsPerRun`` has no equivalent), and pretending otherwise would +be the guard that promises more than it measures. Churn stays a job for measurement +(rule 5), not for this gate. + +Why the numbers are ceilings and not zero +----------------------------------------- +Measured across configurations (2026-08-02, three runs each, gc collected then +disabled, values identical every time): the pass-through path costs **13 blocks and +608 bytes per 5000 calls** and does not move - not with 200 ports instead of 50, not +with the NAT flow table armed, not with latency. Duplication takes it to 15 blocks +and 656 bytes. Those are interpreter bookkeeping, not per-packet retention, so the +ceilings sit far above them and far below a real regression: one retained reference +per packet is 42 kB, one retained object is +5000 blocks. +""" +import gc +import random +import sys +import tracemalloc + +from fakes import check + +from beantester.core import BeanCore + +CALLS = 5000 +BLOCK_CEILING = 64 # measured floor 13, flat across configurations +BYTE_CEILING = 4096 # measured floor 608; a one-reference-per-packet leak is 42k + + +def _decide_many(core, rng, count): + for i in range(count): + core.decide(100, True, 5000 + (i % 50), i * 0.001, rng, + remote_ip="1.2.3.4", remote_port=443, is_tcp=True) + + +def _cost_of(core, count=CALLS): + """(net blocks, net bytes) retained by ``count`` decisions, warmed and gc-quiet.""" + rng = random.Random(7) + _decide_many(core, rng, 200) # fill every lazy structure first + gc.collect() + gc.disable() + tracemalloc.start() + try: + blocks_before = sys.getallocatedblocks() + bytes_before = tracemalloc.get_traced_memory()[0] + _decide_many(core, rng, count) + return (sys.getallocatedblocks() - blocks_before, + tracemalloc.get_traced_memory()[0] - bytes_before) + finally: + tracemalloc.stop() + gc.enable() + + +def test_the_meter_can_actually_see_retention(): + """Prove the instrument before believing a zero from it. + + A gate whose measurement is stuck at zero passes forever and reads exactly like + a clean hot path. This is the same reason the mutation runner carries a canary: + an instrument that cannot report failure is not evidence. Retaining 5000 objects + must show up as thousands of blocks. + """ + if not hasattr(sys, "getallocatedblocks"): # non-CPython: say so, loudly + raise AssertionError("this interpreter has no getallocatedblocks; the hot " + "path allocation gate cannot run and must not be " + "reported as passing") + gc.collect() + gc.disable() + tracemalloc.start() + try: + blocks_before = sys.getallocatedblocks() + bytes_before = tracemalloc.get_traced_memory()[0] + kept = [object() for _ in range(5000)] + blocks = sys.getallocatedblocks() - blocks_before + # The case that defeated the block meter: a container filling with + # references to an object that ALREADY exists. No object is created, so + # blocks barely move - only the bytes do. + references = [] + bytes_before_refs = tracemalloc.get_traced_memory()[0] + blocks_before_refs = sys.getallocatedblocks() + for _ in range(5000): + references.append(100) # a cached small int: nothing new is made + ref_blocks = sys.getallocatedblocks() - blocks_before_refs + ref_bytes = tracemalloc.get_traced_memory()[0] - bytes_before_refs + retained = tracemalloc.get_traced_memory()[0] - bytes_before + finally: + tracemalloc.stop() + gc.enable() + + check("the block meter reports thousands when 5000 objects are retained " + "(a meter stuck at zero would pass the gate below forever)", + blocks >= 4000, f"(saw {blocks} for {len(kept)} objects)") + check("the byte meter reports thousands when 5000 objects are retained", + retained >= 4000, f"(saw {retained} bytes)") + check("the byte meter catches a container of REFERENCES, which the block meter " + "cannot see - this is the hole a surviving mutant exposed on 2026-08-02", + ref_bytes >= 4000 and ref_blocks < 100, + f"(refs: {ref_blocks} blocks, {ref_bytes} bytes)") + + +def test_the_decision_path_retains_nothing_per_packet(): + """A default engine judging 5000 packets holds on to nothing. + + Pass-through is the configuration the tool spends most of its life in and the + one "collect, do not damage" promises is free (see test_passthrough.py). + """ + blocks, retained = _cost_of(BeanCore()) + check(f"decide() retains at most {BLOCK_CEILING} blocks over {CALLS} packets " + f"(one retained object per packet would be {CALLS})", + blocks <= BLOCK_CEILING, f"(retained {blocks} blocks)") + check(f"decide() retains at most {BYTE_CEILING} bytes over {CALLS} packets " + f"(a container growing by one reference per packet is about 42 kB, and " + f"the block count above cannot see it)", + retained <= BYTE_CEILING, f"(retained {retained} bytes)") + + +def test_the_armed_gates_do_not_retain_per_packet_either(): + """Impairment on is still not a licence to keep a copy of every packet. + + Duplication is the one gate that legitimately hands a second packet onward, so + it is the honest worst case to point this at. + """ + for label, arm in (("duplication", lambda c: setattr(c, "dup", 1.0)), + ("latency", lambda c: setattr(c, "latency_s", 0.05)), + ("nat flow table", lambda c: setattr(c, "nat_timeout_s", 30))): + core = BeanCore() + arm(core) + blocks, retained = _cost_of(core) + check(f"with {label} armed, decide() retains at most {BLOCK_CEILING} blocks", + blocks <= BLOCK_CEILING, f"(retained {blocks} blocks)") + check(f"with {label} armed, decide() retains at most {BYTE_CEILING} bytes", + retained <= BYTE_CEILING, f"(retained {retained} bytes)") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 71c6074..d9babad 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -92,6 +92,22 @@ "new": " if text:", "test": "test_the_ratchet_measures_logic_and_not_explanation", }, + { + "label": "hot path: decide() starts keeping one object per packet", + "file": "beantester/core.py", + "old": " with self._lock:\n # 1) process targeting", + "new": (" with self._lock:\n" + " self.__dict__.setdefault(\"_leak\", []).append(size)\n" + " # 1) process targeting"), + "test": "test_the_decision_path_retains_nothing_per_packet", + }, + { + "label": "hot path: the allocation meter stops seeing retention", + "file": "tests/test_hot_path_allocations.py", + "old": " kept = [object() for _ in range(5000)]", + "new": " kept = [object() for _ in range(0)]", + "test": "test_the_meter_can_actually_see_retention", + }, ] # The runner's own check: a patch that cannot compile must be reported as BROKEN, not