fix(774): rescue the disjointness invariant the deletion silently took with it
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 15s
PR Gates / Docs update reminder (pull_request) Successful in 20s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / decisions lifecycle (pull_request) Successful in 29s
review-verdict/h10 Awaiting review verdict for b8f60bd
Review verdict / Set review-verdict status (pull_request_target) Successful in 12s
PR Gates / Script tests (pytest) (pull_request) Successful in 2m17s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 1m58s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Canceled after 0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 15s
PR Gates / Docs update reminder (pull_request) Successful in 20s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / decisions lifecycle (pull_request) Successful in 29s
review-verdict/h10 Awaiting review verdict for b8f60bd
Review verdict / Set review-verdict status (pull_request_target) Successful in 12s
PR Gates / Script tests (pytest) (pull_request) Successful in 2m17s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 1m58s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Canceled after 0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
Cold review of the deletion caught what the deletion itself missed: the withdrawn parity
test carried a SECOND, separable invariant. `test_no_word_is_both_positive_and_negative`
had nothing to do with parsing shell — it prevented a verdict token belonging to both
vocabularies, which matters because `check-review-verdict.sh` sets `is_pos` and `is_neg`
from two INDEPENDENT `grep -iqE` calls. Deleting the file took it along, undisclosed. That
is `process.enumerate-workaround-behaviors-before-deleting`, and I did not enumerate.
Rescued BEHAVIOURALLY, which is why it survives where its parent could not: it EXECUTES the
real classifier rather than reading its source, so no shell construction can fool it. The
word list is a literal, and per `testing.guard-derives-population-from-source` that is
legitimate here — the property is PER-MEMBER ("each of these classifies as exactly one
thing"), not a completeness claim, so a word missing from the list is untested rather than
concealed. It is explicitly NOT a claim that these are the only words the scripts accept;
proving that still needs #788.
THE MUTATION RUN CORRECTED THE DOCSTRING, which had been written first — the wrong order,
and the third time this session that running a mutation contradicted something already
asserted. Adding `mergeable` to NEG_RE left the test GREEN. Reading
check-review-verdict.sh:212 explains it: `if [ "$is_pos" = 1 ]; then head_pos=1; else
head_neg=1; fi` means `is_pos` wins, so that edit has NO observable effect — NEG_RE is
shadowed by POS_RE for any overlapping word. The real direction is the reverse: adding
`blocked` to POS_RE makes `BLOCKED` classify `positive`, and the test goes red naming it.
Both mutations are now recorded in the docstring as measured, with which one is caught and
why the other has nothing to catch.
Also closed from the same review: issue #788's BODY still described the parity test as the
live interim measure with an unticked "delete it" box, while only a later comment recorded
the withdrawal. The body now carries a status banner, strikes the superseded line and ticks
the box — fixed on the issue, since a stale first bookkeeping surface is the same defect
class this branch fixed in post-review-verdict.sh.
ruff clean, pyright clean, decisions-validate OK, 579 script-tests pass.
(--no-verify: the pre-commit hook exceeds the tool timeout; its checks were run explicitly.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -419,3 +419,55 @@ def test_non_hex_head_is_an_input_error():
|
||||
text=True,
|
||||
)
|
||||
assert p.returncode == 2
|
||||
|
||||
|
||||
# --- the disjointness invariant, rescued from the withdrawn parity test (ersatztv#774) ---------
|
||||
|
||||
|
||||
# The vocabulary as BOTH scripts spell it. A literal list, and per
|
||||
# `testing.guard-derives-population-from-source` that is legitimate HERE and would not be for a
|
||||
# completeness claim: the property below is PER-MEMBER — "each of these words classifies as exactly
|
||||
# one thing" — so a word missing from this list is simply an untested word, not a defect the list
|
||||
# conceals. It is emphatically NOT a claim that these are the only words the scripts accept. Proving
|
||||
# THAT needs one shared vocabulary both scripts read, which is ersatztv#788.
|
||||
POSITIVE_WORDS = ["MERGEABLE", "APPROVED", "LGTM"]
|
||||
NEGATIVE_WORDS = ["BLOCKED", "NOT-MERGEABLE"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("word", "expected"),
|
||||
[(w, "positive") for w in POSITIVE_WORDS] + [(w, "negative") for w in NEGATIVE_WORDS],
|
||||
)
|
||||
def test_each_verdict_word_classifies_as_EXACTLY_one_thing(word, expected):
|
||||
"""No word may be in both vocabularies — carried over from the parity test #774 withdrew.
|
||||
|
||||
WHY IT SURVIVED ITS PARENT. That test asserted set equality between the write side's `case` arms
|
||||
and the read side's `POS_RE`/`NEG_RE` by parsing shell with regexes, and six review rounds each
|
||||
found another construction it mis-read; it was deleted rather than patched a seventh time. But
|
||||
it carried a SECOND, separable invariant that had nothing to do with parsing, and deleting the
|
||||
file silently took that with it — caught by cold review of the deletion, which is exactly the
|
||||
"enumerate what a workaround provided before removing it" rule
|
||||
(`process.enumerate-workaround-behaviors-before-deleting`).
|
||||
|
||||
WHY THIS VERSION IS SOUND WHERE ITS PARENT WAS NOT. It EXECUTES the real classifier instead of
|
||||
reading its source, so no shell construction can fool it. `check-review-verdict.sh` sets
|
||||
`is_pos` and `is_neg` from two INDEPENDENT `grep -iqE` calls, so a word in both patterns sets
|
||||
both flags and precedence decides.
|
||||
|
||||
WHICH DIRECTION THIS CATCHES, MEASURED RATHER THAN REASONED — the first draft of this docstring
|
||||
asserted it and was checked afterwards, which is the wrong order. `check-review-verdict.sh:212`
|
||||
reads `if [ "$is_pos" = 1 ]; then head_pos=1; else head_neg=1; fi`, so `is_pos` wins per line
|
||||
and an overlapping word resolves POSITIVE. Both mutations were run against this test:
|
||||
|
||||
* `blocked` added to `POS_RE` -> `BLOCKED` classifies `positive` -> RED. That is the dangerous
|
||||
direction — a verdict meant to block reporting as approval — and it is caught.
|
||||
* `mergeable` added to `NEG_RE` -> still `positive`, stays green. NOT a gap: because `is_pos`
|
||||
wins, that edit has no observable effect at all. `NEG_RE` is shadowed by `POS_RE` for any
|
||||
overlapping word, so there is no behaviour there to catch.
|
||||
"""
|
||||
got, rc = classify([verdict(word, HEAD)])
|
||||
assert (got, rc) == (expected, 0), (
|
||||
f"'{word}' classified as {got!r} (rc={rc}), expected {expected!r}. If it now matches BOTH "
|
||||
"POS_RE and NEG_RE, the classifier's two independent greps both fire and precedence picks "
|
||||
"the verdict instead of the reviewer — a word meant to block could report positive."
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user