Stage 4 exit audit: three overstated verdicts, seven doc defects, one unbuildable criterion

[AdamClemens]

Closes Stage 4 with a per-criterion exit audit, run under prompts/common/AUDITOR.md's stance in a session that did not write the work under review.

The audit ran against a green make ci (605 tests, 99% coverage, 53 scenarios across 14 feature files) and a green real CI run on main (33163793986). Seven of the ten verdicts held. Three did not. That makes four stage audits in a row — Stages 1, 2, 3 and 4 — to find real defects behind a green build.

Stage 4 still closes at ten of ten, but not the ten it started with.

Criterion 4 — the Advection conservation scenario had no teeth

Its fixture gives every boundary cell zero velocity, so every boundary flux is velocity_normal * phi = 0 whatever face value the scheme picks, while interior faces cancel structurally inside accumulate_flux_to_cells. It passes for any flux array. Verified, not inferred: forcing every advective face flux to 0.0 left it green.

That is exactly the qualifier the criterion had reserved — "a bounded scheme can still fail to conserve if its flux accounting is wrong" — and the criterion's own first-named fixture, "a periodic or fully-closed domain", is the one that carries it.

Fixed by building the case nobody had: Conservation on a fully periodic domain (all four edges periodic, velocity (1.7, -0.9), no boundary condition configured at all). Every boundary face now carries a real nonzero flux, and global cancellation is a genuine property of the wrap accounting. Mutation-verified both ways: the clamped-wrap mutation drifts the total 52.0 → 54.87 and fails it, while leaving the old scenario passing.

The weak scenario is kept, with its limitation written into the feature file rather than deleted — it still checks that a zero-velocity boundary face contributes nothing additively.

Worth noting for reviewers: Diffusion's conservation scenario, written the same day in the same shape, does have teeth (mutating its boundary-gradient branch drifts the total by 37.4). The difference is not care — diffusion's boundary flux has no velocity factor in front of it to zero the whole term out.

Criterion 6 — the criterion was unbuildable as written

It asked for a shared step vocabulary in tests/golden/conftest.py, reused rather than re-derived. A conftest.py applies only to its own directory subtree, and all nine of Stage 4's binding modules live in tests/unit/. Proven rather than argued: a tests/unit/ scenario using a step defined in tests/golden/conftest.py fails with StepDefinitionNotFoundError.

So this was never criterion-versus-convention. The criterion named a venue that could not serve its consumers; tests/unit/CLAUDE.md's "each binding test supplies its own local steps" grew into that vacuum and hardened into a principle nobody re-examined.

The convention's own argument was also weaker than it read. It warned that sharing couples nine tasks' fixtures — but they were already coupled in substance and merely uncoupled in maintenance: origin=(0.5, -1.0), spacing=(0.2, 0.3) byte-identical across eight modules, _face_normal_velocity across four, _west_face across four.

Resolution: fix the venue, share the building blocks, not the step definitions. tests/unit/_numerics.py is the tests/golden/_demo.py counterpart — an existing in-repo pattern, not a new one — and all nine modules import from it.

	before	after
condition-double declarations	9	0
local _face_normal_velocity	4	0
local _west_face	4	0
modules carrying the mesh constants	8	0

Step definitions went 109 → 110 (the audit's own new scenario), and that is the point: the target was re-derivation, not step count.

A shared tests/unit/conftest.py of step definitions was considered and rejected in the same decision — a shared step must populate a shared context, forcing one _Context type across nine modules, which is the coupling the convention was right about. Each module keeps its own _Context and its own step bodies.

Criterion 10 — the sweep covered the wrong files

It checked the files Stage 4 touched, not the files Stage 4 invalidated. Seven defects, none in a file any Stage 4 task opened:

• engine.md's Flux entry named no module for accumulate_flux_to_cells — which this criterion's own text explicitly required.
• Four engine.md entries carried a decaying count of surviving _Null* classes ("the five that still do" … "the two"), of which there have been zero since TASK-029 — contradicting Criterion 7 two rows above in the same table.
• rendering.md called simulation/frame scheduling "Stage 4+ work" three commits after Stage 4 shipped it.
• bootstrap.py's module docstring said "No simulation functionality" twenty lines above its own import ... simulation_step.
• Three more (two in overview.md, one in adr/ADR-002) found by the new rule the first four produced, run before the verdict was written.

The bootstrap.py one is the sharpest: the identical stale self-description was found in __main__.py's CLI help text and fixed the same morning (73ff113). That fix's blast-radius sweep stopped at the file it had been pointed at.

The rule this produced

docs/practices.md gains "A stage's documentation sweep is a grep, not a diff review". The rule above it — "Let a checked artifact carry status, not a tense", written by Stage 3's own audit — converted engine.md's Implementation: lines to checked paths and stopped there; Stage 4 then reproduced the class in three files it did not cover. A count is not a tense, and a negation in a src/ docstring is not a documentation field.

So the new rule is three greps run before a stage closes: the stage's own number, every count of anything the stage changed, and the negations (does not exist, no … yet, nothing that, still). It found three more defects the moment it was written, which is a better argument for it than the four that prompted it.

Also recorded, not fixed

docs/CHANGELOG-DESIGN.md had no entry after 2026-08-22 — Stages 3 and 4 entirely, against Session Workflow step 4. This PR appends the audit's own entry and records the gap. It deliberately does not backfill six days of decision history from its outcomes; that would mean narrating sessions nobody present witnessed. Carried as an open item in docs/planning/backlog.md §13 with the decision it needs stated.

Verification

• make ci green: 606 tests, 99% coverage, 54 scenarios across 14 feature files, all twelve gates.
• make check-claims: only its two known false positives.
• Every mutation quoted above was run and reverted individually; the numbers are read off actual failure output, not predicted.
• The conftest-scoping claim was confirmed by a throwaway probe module, run and then deleted.
• Post-refactor re-check: the clamped-wrap mutation still fails all three periodic-dependent scenarios, so the Criterion 6 refactor did not weaken the Criterion 4 fix.

Reviewer notes

Two things kept deliberately rather than collapsed, in case they look like oversights:

• face_normal_velocity_toward's explicit neighbour parameter. A periodic face has no mesh-reported neighbour, and hiding that behind a default would hide the one difference the scenario exists to check.
• The CG module's zero-gradient-everywhere rationale, moved to its call site rather than lost with the local class it used to live on — that choice is what makes the fixture semi-definite, not a convenient default.

🤖 Generated with Claude Code