Adversarial review: the harness had the hole it was written to close - #277
Merged
Merged
Conversation
Three mistakes have recurred in this repository's measurements, each more than once, and knowing the lesson has not prevented repeating it within hours -- the cross-window comparison was corrected in one PR and repeated in the next. `src/evaluation/ab.py` makes each one structurally hard. Arms alternate. There is no mode that runs them in sequence, because the second arm otherwise inherits a warm process and whatever the API is doing that minute. That has twice produced a result in the direction the author was hoping for, and once showed a real improvement as a regression. Every arm declares a `precondition`, checked after each sample, and the summary refuses to draw a conclusion when one fails. A prompt that asked for "exactly 1" alternate returned four; a ContextVar set around a call was overwritten by the node inside it. Both produced ordinary-looking numbers that were measuring the baseline twice. And the summary reports p50 with min and max, never a percentile the sample cannot support. A p90 was quoted from fifteen samples, where it is about the second-highest value. The configuration is applied before every sample rather than once per arm, because a setting applied once and mutated in between fails the same way as never applying it. Each test recreates a mistake actually made here, and both guards were verified by sabotage: removing the alternation fails the alternation test, ignoring preconditions fails the refusal test. Exercised on the real graph too, not only on stubs -- expansion on 6.22s against off 2.86s, precondition held. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`precondition` was only worth the discrimination in it, and nothing checked for any. `lambda: True` reads like a guard, passes every sample and proves nothing -- the same vacuous shape this repository has found three times elsewhere today, sitting in the file written to stop it. After each sample, with that arm's configuration still active, the other arms' preconditions are evaluated. If one of them also holds, the two cannot tell the arms apart and the comparison is refused with a distinct message. That is the rule the rest of the work arrived at -- an assertion that something is absent proves nothing until the same check has shown it can be present -- applied to the guard rather than only to the thing guarded. The new check immediately caught the test helper in this file, whose preconditions returned a constant. The helper now reads a shared marker the arms actually set, and a separate always-true arm exists solely to be caught. Two smaller ones. Re-using arms across runs silently merged two distributions into what looks like one noisy measurement, and is refused. And ordering reversed rather than rotated, so with three or more arms the middle one stayed in the middle and carried a systematic warming bias -- exactly the effect the alternation exists to remove, left in place for every case except two arms. All three verified by sabotage, and the first attempt at that is worth recording: removing the discrimination check appeared to fail nothing, because the edit had silently not applied. Re-done with the sabotage itself asserted, the test fails as it should. The precondition needed a precondition. 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
1 participant
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.
Follow-up review of #276. Builds on it, so merge #276 first.
The harness had the vacuity hole it exists to stop
preconditionis only worth the discrimination in it, and nothing checked for any.lambda: Truereads like a guard, passes every sample, and proves nothing — the same shape found three times elsewhere today, sitting in the file written to prevent it.After each sample, with that arm's configuration still active, the other arms' preconditions are evaluated. If one also holds, the two cannot tell the arms apart and the comparison is refused with its own message. That is the rule the rest of this work arrived at — an assertion that something is absent proves nothing until the same check has shown it can be present — applied to the guard rather than only to the thing guarded.
It immediately caught the test helper in this very file, whose preconditions returned a constant. The helper now reads a shared marker the arms actually set, and a separate always-true arm exists solely to be caught.
Two smaller ones
The sabotage needed its own precondition
All three verified by sabotage, and the first attempt is worth recording: removing the discrimination check appeared to fail nothing, because the edit had silently not applied. Re-done with the sabotage itself asserted, the test fails as it should.
That is the same class of error the harness is about, one level up. The check needed a check.
595 passed, 1 skipped; mypy over 151 files, ruff clean.
🤖 Generated with Claude Code