diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 8da41fc..39d9a4c 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -7513,3 +7513,52 @@ real logged frame/position/paused values before being corrected. - **PyFlow 0.4.0**, cut when Stage 9 closed. A minor bump on two independent grounds: shipped physics changed, and two configuration fields were removed. + +### Findings (second pass, same day) + +- **Four of the nine validators in `make ci` returned success when they + examined nothing.** Found by asking, of the repository generally, what + else might not be implemented as believed, and answering it + mechanically: every validator run with its discovery function replaced + by one returning an empty set. `check_docs`, `check_duplicate_blocks`, + `check_references` and `check_scenarios` printed their ordinary success + message and exited 0. The other five already failed correctly, and + nothing distinguished the two groups. +- **`check_references.py` had carried a twelve-line comment explaining + this exact failure mode since 2026-08-30** -- written as instance 2 of + `docs/practices.md`'s "A rule that matches nothing reports nothing" -- + and its own `main()` had no guard against it. +- **`check_scenarios.py` guarded one half of itself and not the other, + and that one was not an omission.** Its bindings side failed loudly + when no module bound a feature file; its features side printed "No + feature files found; nothing to check" and returned 0, in the same + function -- deliberately, with a test stating the reason: the gate was + built before Stage 4 wrote its first `.feature`, so an empty + `tests/features/` was then the repository's ordinary state. The reason + expired on 2026-08-27 with TASK-023 and went unrevisited for 17 days, + under the gate `adr/ADR-007-executable-acceptance-criteria.md` rests + on entirely. An exemption states its own expiry condition and then + nobody is assigned to watch it. +- **Three other classes were probed and came back clean**, which is worth + recording so the next pass does not redo them: no configuration field + is unread by engine code (the generalisation of TASK-055's sweep to all + 14 schema dataclasses); the six ADR-003 registries agree exactly with + the schema `Literal`s that select them (`periodic` and + `boussinesq_buoyancy` are apparent mismatches and both are deliberate, + the latter pinned by `tests/integration/ + test_boussinesq_buoyancy_registration.py` in three directions); and all + 163 `@then` step definitions assert something. + +### Decisions + +- **The fix is a sweep, not four guards.** Four guards would be four + fixes and a hope. `tests/unit/test_validator_guards.py` runs every + `check_*.py` with nothing to find and requires a non-zero exit *and* a + message saying so, with `test_every_validator_is_covered` failing if a + validator is added without an entry. A gate that cannot return non-zero + (`check_claims`, advisory by design) is excluded by name with its + reason, never by absence. +- **The general form is recorded as the rule rather than the four + instances**: when a check reports success, ask what it would have + reported had it examined nothing. If those outputs are the same, it is + not yet a check. diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index df6a951..71832ec 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -381,7 +381,7 @@ This paragraph previously said `make install` and `make test` were still expected to fail, pending `uv.lock` and a test suite (B2/C1) -- stale since 2026-08-16 and corrected 2026-08-19. Both now succeed: `uv.lock` is committed (B2) and `make test` runs the suite with coverage -(C1a/C1b): **1240 tests as of 2026-09-13**, up from 1209 on 2026-09-11 +(C1a/C1b): **1259 tests as of 2026-09-13**, up from 1209 on 2026-09-11 (TASK-052, Stage 9: 9 in `tests/unit/test_boundary_velocity.py` and 3 in `tests/golden/test_sealed_box.py` for the wall-permeability fix and its own golden demo, plus 5 the fixtures those changed gained along the way; @@ -394,7 +394,11 @@ the other four did not hold the `velocity.*` filter); then TASK-055: sweep, and the guard that the sweep reaches anything) and a net -1 in `tests/unit/test_configuration.py`, which lost the mutual-exclusivity rejection test along with the two fields it was about and gained four -against the re-homed net-flux rule. +against the re-homed net-flux rule; then 19 in +`tests/unit/test_validator_guards.py`, the sweep proving every gate in +`make ci` fails when it examines nothing, which four of the nine did +not (`docs/practices.md`, "A rule that matches nothing reports +nothing", fourth instance). Before that, 1209 on 2026-09-11 (the Stage 8 exit audit, closing the gap between three Completion Criteria and what actually checked them: 4 in diff --git a/docs/planning/status.md b/docs/planning/status.md index 6e1f1f6..361dd20 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -47,7 +47,7 @@ pie showData ## Live repository facts - **49** `CLAUDE.md` files -- **1240** tests collected +- **1259** tests collected - **156** Gherkin scenarios (`tests/features/*.feature`) ## Stages diff --git a/docs/practices.md b/docs/practices.md index e9149d6..7a6ffee 100644 --- a/docs/practices.md +++ b/docs/practices.md @@ -1717,6 +1717,59 @@ consults it.** `ALLOWED_MISSING`, `PLANNED`, and any list like them should be assumed inert until something has been seen to fail because of an entry in them. +### The fourth instance was the gates themselves, 2026-09-13 + +All three cases above are a check whose *pattern* matched nothing. The +fourth is a check whose *input* was empty, and it went unnoticed longer +because every instance above trained attention on the pattern. + +Asked what else the repository might not be implementing as believed, +a sweep ran every validator with its discovery function replaced by one +returning nothing. **Four of the nine validators in `make ci` returned +success**: `check_docs`, `check_duplicate_blocks`, `check_references` +and `check_scenarios`. The other five (`check_dates`, +`check_documents`, `check_graph`, `check_manifest`, `check_stages`) +already failed correctly. Nothing distinguished the two groups -- it was +not a judgement anybody had made, just whichever author happened to +think of it. + +Two details make this the sharpest instance rather than merely another +one: + +- **`check_references.py` carries a twelve-line comment explaining this + exact failure mode**, written on 2026-08-30 as instance 2 above, and + its `main()` had no guard against it. A principle stated in a comment + and not implemented two hundred lines below is the defect the comment + is about. +- **`check_scenarios.py` printed "No feature files found; nothing to + check" and returned 0** -- while its *bindings* side, in the same + function, failed loudly on the same condition. **That exemption was + correct when written and had a test saying why**: the gate predates + Stage 4's first `.feature` file, so an empty `tests/features/` was + once the repository's ordinary state. The reason expired on + 2026-08-27 when TASK-023 landed that file; nothing revisited it, and + it sat for 17 days under the one gate + `adr/ADR-007-executable-acceptance-criteria.md` depends on entirely. + This is "A checkable trigger still needs somebody to check it" + (below) applied to an *exemption* rather than a promise -- a + distinction worth carrying, because an exemption states its own + expiry condition and then nobody is assigned to watch it. + +**The fix is a sweep, not four guards.** Four guards would be four fixes +and a hope; `tests/unit/test_validator_guards.py` runs every +`check_*.py` with nothing to find and requires a non-zero exit *and* a +message saying so, with `test_every_validator_is_covered` failing if a +new validator is added without an entry. A gate that cannot return +non-zero at all (`check_claims`, advisory by design) is excluded by name +with its reason, never by being absent -- an unexplained omission is how +this gap existed. + +**The general form, which is what to carry forward:** *when a check +reports success, ask what it would have reported had it examined +nothing.* If those two outputs are the same, the check is not yet a +check. That question is cheap, it is mechanical, and applying it to nine +validators took one afternoon and found four. + ### Render it and look at it before calling a rendering stage done **This applies to documents, not only to pixels** (added 2026-09-04). diff --git a/docs/repository-inventory.md b/docs/repository-inventory.md index 1fe9b48..c79a2c9 100644 --- a/docs/repository-inventory.md +++ b/docs/repository-inventory.md @@ -16,7 +16,7 @@ reading job and lives in the manifest. Test counts and coverage are not here either -- those come from running the suite, not from listing files. -**370 tracked files** across 49 directories; +**371 tracked files** across 49 directories; 2 are empty. ## (root) @@ -483,6 +483,7 @@ listing files. - `test_structured_cartesian_mesh.py` - `test_temperature_field.py` - `test_uniform_vertex_coordinate_system.py` +- `test_validator_guards.py` - `test_vector_field.py` - `test_velocity_field_support.py` diff --git a/tests/integration/test_frame_failure.py b/tests/integration/test_frame_failure.py index ca469c6..23c0f19 100644 --- a/tests/integration/test_frame_failure.py +++ b/tests/integration/test_frame_failure.py @@ -62,6 +62,18 @@ fluid: viscosity: 0.01 + +# The one glfw case below opens a **real window** on any machine with a +# display, and what a watcher sees is a lid-driven cavity tearing itself +# apart -- arrows exploding off screen -- because that is what this +# fixture is for. Reported as a suspected engine failure by the +# maintainer on 2026-09-13, which is a fair reading of an unlabelled +# window doing that. Stage 7's whole premise is that the render window +# explains itself without the config file beside it; this one did not, +# so it says so now. `rendering.title` is drawn *in* the scene +# (`hud.build_title_text`), not only in the OS title bar. +rendering: + title: "PyFlow self-test -- this run is SUPPOSED to diverge" """ _FRAMES = 25 diff --git a/tests/unit/test_check_scenarios.py b/tests/unit/test_check_scenarios.py index 9be9562..0ad1f8b 100644 --- a/tests/unit/test_check_scenarios.py +++ b/tests/unit/test_check_scenarios.py @@ -130,12 +130,30 @@ def test_scenario_outlines_count_as_scenarios(tree: tuple[Path, Path]) -> None: assert _load(features, tests).main() == 1 -def test_an_empty_features_directory_is_not_a_failure(tree: tuple[Path, Path]) -> None: - """Before Stage 4 writes its first physics feature, a branch that - touches no scenarios must not fail this gate. +def test_an_empty_features_directory_is_a_failure(tree: tuple[Path, Path]) -> None: + """**This test asserted the opposite until 2026-09-13, and it was + right when it was written.** Its reason was stated and real: "Before + Stage 4 writes its first physics feature, a branch that touches no + scenarios must not fail this gate." The gate was built before the + content it guards existed, so an empty `tests/features/` was the + ordinary state of the repository. + + That reason expired on 2026-08-27, when TASK-023 landed the first + `.feature` file, and nothing revisited it -- the same shape as + `docs/practices.md`'s "A checkable trigger still needs somebody to + check it". With 32 feature files and 156 scenarios in the tree, an + empty features directory no longer means "not yet"; it means the + discovery path broke, and the gate + `adr/ADR-007-executable-acceptance-criteria.md` rests on entirely + reported success over it. + + The bindings half of the same function has always failed loudly on + its own empty case (the test directly above this one). The asymmetry + was not a judgement anybody made about the two halves -- it was one + half's exemption outliving its reason. """ features, tests = tree - assert _load(features, tests).main() == 0 + assert _load(features, tests).main() == 1 def test_the_real_feature_tree_has_no_unrun_scenarios( diff --git a/tests/unit/test_validator_guards.py b/tests/unit/test_validator_guards.py new file mode 100644 index 0000000..9167a1f --- /dev/null +++ b/tests/unit/test_validator_guards.py @@ -0,0 +1,130 @@ +"""Every gate in `make ci` must fail when it examines nothing. + +**"A rule that matches nothing reports nothing" is this repository's own +phrase**, written into `check_dates.py`, `check_documents.py`, +`check_stages.py` and -- as a twelve-line comment explaining the failure +mode after it had already cost the project once -- `check_references.py`. +Four validators stated the principle. Four implemented it. They were not +the same four. + +A gate that returns success while discovering zero files is +indistinguishable, in CI output, from a gate that discovered everything +and found no problems. That is worse than having no gate: the pipeline +reports a check that did not happen. + +This sweep is the enforcement, because the alternative is remembering. +Each validator is imported, the function it discovers work through is +replaced with one returning nothing, and `main()` must return non-zero. +`test_every_validator_is_covered` keeps the map honest -- a new +`check_*.py` with no entry fails here rather than being silently +exempt, which is the mistake this whole file exists to stop being +possible (`tests/unit/test_boundary_field_reachability.py` has the same +shape, for the same reason). +""" + +from __future__ import annotations + +import importlib +import io +import sys +from collections.abc import Callable +from contextlib import redirect_stdout +from pathlib import Path +from typing import Any + +import pytest + +_VALIDATOR_DIR = Path(__file__).resolve().parents[2] / "tools" / "validators" + +if str(_VALIDATOR_DIR) not in sys.path: + sys.path.insert(0, str(_VALIDATOR_DIR)) + + +# Validator module -> (the attribute it discovers work through, a +# replacement returning "nothing found"). Each discovery point is named +# explicitly because there is no way to infer it: a validator's own +# glob, `git ls-files` call, or document parse is the thing being +# emptied, and those have no common shape. +_DISCOVERY: dict[str, tuple[str, Callable[..., Any]]] = { + "check_dates": ("tracked_files", lambda *a, **k: []), + "check_docs": ("iter_markdown_files", lambda *a, **k: []), + "check_documents": ("tracked_markdown", lambda *a, **k: []), + "check_duplicate_blocks": ("_tracked_markdown_files", lambda *a, **k: []), + "check_graph": ("_data_files", lambda *a, **k: []), + "check_manifest": ("_tracked_files", lambda *a, **k: []), + "check_references": ("tracked_paths", lambda *a, **k: (set(), set())), + "check_scenarios": ("feature_files", lambda *a, **k: []), + "check_stages": ("parse_stages", lambda *a, **k: []), +} + +# `check_claims.py` is deliberately advisory and exits 0 even with +# findings (root `CLAUDE.md`), so "returns non-zero" is not a property it +# has. It is excluded by name, with that reason, rather than by being +# absent from the map -- an unexplained omission is how the gap above +# happened. +_ADVISORY = {"check_claims"} + + +def _run_with_nothing_found(module_name: str) -> tuple[int, str]: + attribute, blank = _DISCOVERY[module_name] + module = importlib.import_module(module_name) + assert hasattr(module, attribute), ( + f"{module_name}.{attribute} no longer exists -- this sweep is patching a " + "name that is gone, so it proves nothing about the real discovery path" + ) + original = getattr(module, attribute) + setattr(module, attribute, blank) + try: + buffer = io.StringIO() + with redirect_stdout(buffer): + code = int(module.main()) + return code, buffer.getvalue() + finally: + setattr(module, attribute, original) + + +@pytest.mark.parametrize("module_name", sorted(_DISCOVERY)) +def test_a_validator_that_finds_nothing_fails(module_name: str) -> None: + code, output = _run_with_nothing_found(module_name) + + assert code != 0, ( + f"{module_name} returned success while examining nothing at all. In CI that is " + f"indistinguishable from a clean run. Its output was: {output.strip()!r}" + ) + + +@pytest.mark.parametrize("module_name", sorted(_DISCOVERY)) +def test_a_validator_that_finds_nothing_says_so(module_name: str) -> None: + """Failing is necessary but not sufficient -- a validator that fails + with a message about some unrelated problem sends whoever reads CI + looking in the wrong place. The message has to name the real cause. + """ + _, output = _run_with_nothing_found(module_name) + + assert output.strip(), f"{module_name} failed silently when it found nothing" + assert any(phrase in output.lower() for phrase in ("nothing", "no ", "not find", "empty")), ( + f"{module_name} failed, but its message does not say it found nothing: {output.strip()!r}" + ) + + +def test_every_validator_is_covered() -> None: + """The guard on the sweep itself. + + Without this, adding `tools/validators/check_something_new.py` with + no entry in `_DISCOVERY` leaves it unswept and every test above still + green -- a sweep that covers less than it appears to, which is the + exact shape of the defect being fixed. + """ + on_disk = {path.stem for path in _VALIDATOR_DIR.glob("check_*.py")} + + assert on_disk, f"no validators found under {_VALIDATOR_DIR} -- has the layout changed?" + + unswept = on_disk - set(_DISCOVERY) - _ADVISORY + assert not unswept, ( + f"{sorted(unswept)} are gates nothing here sweeps. Add each to _DISCOVERY with " + "the function it discovers work through, or to _ADVISORY with the reason it " + "cannot return non-zero." + ) + + stale = set(_DISCOVERY) - on_disk + assert not stale, f"{sorted(stale)} are in _DISCOVERY but no longer exist on disk" diff --git a/tools/validators/CLAUDE.md b/tools/validators/CLAUDE.md index 5d4efca..3651dd9 100644 --- a/tools/validators/CLAUDE.md +++ b/tools/validators/CLAUDE.md @@ -392,3 +392,83 @@ Deliberately as narrow as that rule: only this one claim, phrased in one fixed form, is extracted and checked -- not a general "every number in this document is checked" rule, which would need a reader the same way `check_claims.py`'s advisory scope already explains. + + +--- + +## Every gate here must fail when it examines nothing + +**Standing rule for this directory, 2026-09-13**, enforced by +`tests/unit/test_validator_guards.py` rather than by remembering. + +A validator that returns success while discovering zero files produces +CI output identical to one that discovered everything and found no +problems. `docs/practices.md`'s "A rule that matches nothing reports +nothing" had recorded three instances of this shape by 2026-08-30 -- +all of them a check whose *pattern* matched nothing. The fourth was a +check whose *input* was empty, and it lived here. + +**Four of the nine gates in `make ci` returned success when handed an +empty discovery set**: `check_docs.py`, `check_duplicate_blocks.py`, +`check_references.py` and `check_scenarios.py`. Five already failed +correctly (`check_dates.py`, `check_documents.py`, `check_graph.py`, +`check_manifest.py`, `check_stages.py`). Three of the four were simple +omission. The fourth, `check_scenarios.py`, was a deliberate exemption +whose stated reason had expired -- see below, and note the difference: +the omissions cost nothing to fix, while that one needed its original +justification read before it could honestly be overridden. + +Two of the four are worth naming individually, because each is its own +lesson: + +- **`check_references.py` states this principle in a comment and did not + implement it.** The twelve lines above `EXTS` exist because the + `.feature` extension was missing from that tuple for two days and + every feature path in every document went unchecked behind a green + gate. That comment ends "a rule that matches nothing reports nothing, + which reads exactly like a pass" -- and `main()`, two hundred lines + below, printed "Every path named in prose resolves" over an empty + file list. +- **`check_scenarios.py` guarded one half of itself and not the other, + because one half's exemption outlived its reason.** Its *bindings* + side has always failed loudly when no test module binds a feature + file. Its *features* side printed "No feature files found; nothing to + check" and returned 0 -- and that was **correct when written**, with + the reason stated in + `tests/unit/test_check_scenarios.py`'s own test: the gate was built + before Stage 4 wrote its first `.feature`, so an empty + `tests/features/` was the ordinary state of the repository. The reason + expired on 2026-08-27 when TASK-023 landed the first feature file, and + nothing revisited it -- `docs/practices.md`'s "A checkable trigger + still needs somebody to check it", applied to an exemption rather than + a promise. With 32 feature files in the tree, an empty directory no + longer means "not yet"; it means discovery broke, under the one gate + `adr/ADR-007-executable-acceptance-criteria.md` depends on entirely, + since pytest itself says nothing about a `.feature` file no module + runs. + +**When adding a validator here, the guard is part of the validator, not +an extra.** `test_validator_guards.py` will fail on a new `check_*.py` +that has no entry in its `_DISCOVERY` map, so the choice is to name the +function it discovers work through or to declare -- by name, with a +reason -- that it cannot return non-zero at all. Only `check_claims.py` +is in the second category, because it is advisory by design and exits 0 +even with findings. + +**The message matters as much as the exit code.** The sweep asserts both: +a gate that fails with a message about some unrelated problem sends +whoever reads CI looking in the wrong place. The wording to copy is +`"... -- has the layout changed?"` followed by `"A rule that matches +nothing reports nothing."` -- `check_documents.py`'s, the only one using +that exact two-line form before 2026-09-13, now shared by the four +fixed that day. `check_dates.py` and `check_stages.py` say the same +thing in one line of their own, and `check_manifest.py` reaches the same +outcome through a different rule entirely +(`collective-rule-matches-something` fires when a declared rule matches +no tracked file, so an empty repository fails it). Three phrasings for +one idea is more than ideal and less than worth churning every file +over; the sweep checks the property, not the sentence. + +**The general question this came from, worth asking of any check:** when +it reports success, what would it have reported had it examined nothing? +If those are the same output, it is not yet a check. diff --git a/tools/validators/check_docs.py b/tools/validators/check_docs.py index d2b46d8..687da07 100644 --- a/tools/validators/check_docs.py +++ b/tools/validators/check_docs.py @@ -154,8 +154,14 @@ def check_file(md_file: Path) -> list[tuple[int, str, str]]: def main() -> int: + markdown_files = iter_markdown_files() + if not markdown_files: + print(f"No Markdown files found under {REPO_ROOT} -- has the layout changed?") + print("A rule that matches nothing reports nothing.") + return 1 + total_broken = 0 - for md_file in iter_markdown_files(): + for md_file in markdown_files: for lineno, target, reason in check_file(md_file): rel = md_file.relative_to(REPO_ROOT) print(f"{rel}:{lineno}: broken link '{target}' ({reason})") diff --git a/tools/validators/check_duplicate_blocks.py b/tools/validators/check_duplicate_blocks.py index 0037de3..0f26976 100644 --- a/tools/validators/check_duplicate_blocks.py +++ b/tools/validators/check_duplicate_blocks.py @@ -112,6 +112,11 @@ def check_duplicate_blocks(root: Path = REPO_ROOT) -> list[str]: def main() -> int: + if not _tracked_markdown_files(REPO_ROOT): + print(f"No tracked Markdown files found under {REPO_ROOT} -- has the layout changed?") + print("A rule that matches nothing reports nothing.") + return 1 + findings = check_duplicate_blocks() if findings: for finding in findings: diff --git a/tools/validators/check_references.py b/tools/validators/check_references.py index 986cb18..04fb1bf 100644 --- a/tools/validators/check_references.py +++ b/tools/validators/check_references.py @@ -247,6 +247,20 @@ def check_file(md: Path, rel: str, files: set[str], dirs: set[str]) -> list[tupl def main() -> int: files, dirs = tracked_paths() + + # **The comment above `EXTS` has explained this exact failure mode + # since 2026-08-30 -- "a rule that matches nothing reports nothing, + # which reads exactly like a pass" -- and this function had no guard + # against it until 2026-09-13.** Stating a principle in a comment + # and not implementing it two hundred lines below is the same defect + # the comment is about, which is why it is worth recording here + # rather than quietly adding the check. + prose = [f for f in files if f.endswith(".md") and f not in EXCLUDED_FILES] + if not prose: + print(f"No prose files found among {len(files)} tracked path(s) -- has the layout changed?") + print("A rule that matches nothing reports nothing.") + return 1 + total = 0 for rel in sorted(f for f in files if f.endswith(".md")): if rel in EXCLUDED_FILES: diff --git a/tools/validators/check_scenarios.py b/tools/validators/check_scenarios.py index cc4a493..c181e4f 100644 --- a/tools/validators/check_scenarios.py +++ b/tools/validators/check_scenarios.py @@ -84,8 +84,18 @@ def collect_bindings() -> tuple[dict[str, list[Path]], dict[str, set[str]]]: def main() -> int: features = feature_files() if not features: - print("No feature files found; nothing to check.") - return 0 + # **This returned 0 until 2026-09-13**, on the reading that no + # feature files means no unbound scenarios. That is true and + # beside the point: this gate is the only thing standing between + # `adr/ADR-007-executable-acceptance-criteria.md` and a + # repository whose acceptance criteria silently never run, so a + # run of it that examined nothing is the one outcome that must + # never look like a pass. The bindings side of this same + # function has always failed loudly; the asymmetry was + # accidental. + print(f"No feature files found under {_display(FEATURES_DIR)} -- has the layout changed?") + print("A rule that matches nothing reports nothing.") + return 1 whole, individual = collect_bindings() problems: list[str] = []