Give a skipped readback criterion its own state, and count criteria instead of declaring 12 - #435
Merged
Merged
Conversation
…nstead of declaring 12 The criteria set was the last instrument in this job still running on two states. When build_assets_legacy is absent the value-for-value comparison and the row-survival check fall into an `else` that prints "(no build_assets_legacy; nothing to compare the copy against)" and the run goes green. That is a criterion silently not executing - exactly the shape `exit 2` was introduced elsewhere to prevent, in the one place it had not been applied. It is latent rather than active: the table still exists, so the branch is not taken today. It becomes permanent the moment task #114 drops the table, at which point the deploy log keeps printing a reassuring sentence forever while checking nothing. check() now increments `evaluated` skip() new; prints `SKIP <criterion>: <reason>` and increments `skipped` else reports both criteria as SKIP rather than one parenthetical end prints `evaluated N criteria, M skipped, ... failed`, and warns on M>0 Skips are loud, not red. A skip can be legitimate, and failing the deploy on one would hold production hostage to a comparison source we intend to delete. What it must not be is quiet: a permanently skipped criterion is itself the event worth seeing, and it announces itself on every deploy rather than waiting for someone to remember that a deletion elsewhere retired a check here. Counted, not declared, for the same reason the workflow lint reports "N of M YAML files" with both figures counted: this gate has been described in writing as "12 criteria", and a number kept in prose stays at 12 while the code drifts. Today the declared number happens to be right, and "happens to be right" is how it goes wrong later. Deliberately NOT bundled into #114's deletion, which was the earlier plan. That deletion is irreversible, needs its own explicit authorisation, and may well be executed under time pressure - a fix riding along with it is a fix riding the least careful moment. Done separately, while the table still exists, the change is verifiable now: both branches were exercised (legacy present -> 2 evaluated, 0 skipped; legacy absent -> 0 evaluated, 2 skipped; a real mismatch -> still fails), which is not possible once the else branch is the only reachable one. Design is Sentinel's (give the skipped state a name); counting rather than asserting the total is Gogo's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…measurement
Sentinel caught that this PR fixed the confusion in one direction and left it
standing in the other. `PRAGMA foreign_key_check` being refused by D1 was
reported as `FAIL foreign_key_check did not run`, which sends someone looking
for a foreign-key violation that was never measured.
The blocking is deliberate and stays: the comment above that branch reasons that
a refusal means D1 changed what it permits, and that is worth stopping for. What
changes is what it says it is.
ok ran, correct
FAIL ran, wrong answer -> blocks, counted as evaluated
CANNOT RUN did not run, and that is itself the anomaly
-> blocks, NOT counted as evaluated
SKIP did not run, acceptably -> does not block
`cannot_run` sits between `skip` and a failing `check` on purpose. Reporting it as
a skip would let a substrate change through; reporting it as a failure asserts a
violation nobody observed. It is the same distinction the rest of this job now
makes - could-not-run is not a result - applied to the one place that had it
backwards.
The summary line reports the count separately: "evaluated N criteria, M skipped,
K could not run, ... failed". A criterion that stops being measurable shows up as
K rising rather than as a fabricated verdict.
Exercised: pragma clean -> evaluated=1 fail=0; pragma finds orphans ->
evaluated=1 fail=1; pragma refused -> evaluated=0 unrunnable=1 fail=1, and the
three print as distinguishable lines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The readback criteria set is the last instrument in this job still running on two
states.
When
build_assets_legacyis absent, two criteria fall into anelsethat prints(no build_assets_legacy; nothing to compare the copy against)— and the run goesgreen. A criterion silently not executing is exactly the shape
exit 2wasintroduced elsewhere to prevent; this is the one place it had not been applied.
Latent today, permanent later. The table still exists, so the branch is not taken.
It becomes permanent the moment task #114 drops the table — after which the deploy log
prints a reassuring sentence forever while checking nothing.
Change
check()evaluatedskip()SKIP <criterion>: <reason>, incrementsskippedelseevaluated N criteria, M skipped, … failed; warns whenM > 0Skips are loud, not red. A skip can be legitimate, and failing the deploy on one
would hold production hostage to a comparison source we intend to delete. What it must
not be is quiet — a permanently skipped criterion is itself the event worth seeing, and
it now says so on every deploy instead of waiting for someone to remember.
Counted, not declared. This gate has been described in writing as "12 criteria"; a
number kept in prose stays at 12 while the code drifts. Same reason the workflow lint
reports
N of M YAML fileswith both figures counted.Why this is not bundled into #114
That was the earlier plan and I now think it was wrong. #114's deletion is irreversible,
needs its own explicit authorisation, and may be executed under time pressure — a fix
riding along with it is a fix riding the least careful moment.
Done separately, while the table still exists, it is also verifiable now:
Once the else branch is the only reachable one, that comparison is no longer possible.
Verification
environment:gate intact, 16 stepsCredit
The design — giving the skipped state a name — is @sentinel's. Counting the criteria
rather than asserting the total is @gogo's.