Skip to content

Commit b986780

Browse files
committed
docs(ai): state the still-failing case's reachability durably
Two comments justified the `Fail → Fail` Warn arm by saying `--refresh-baseline` writes the current report verbatim with no `any_fail` check. That was true when written and is about to stop being true: phase-rs#7029 adds exactly that check. Both comments would then assert, in the tree, a property the tree no longer has — and because they are comments, nothing would fail. They would simply be wrong. The conclusion they support is unaffected, so only the justification changes. `Fail → Fail` stays reachable no matter what guards the write path, because nothing revalidates a committed baseline when it is loaded: a baseline blessed before the guard, or hand-edited, still carries the failure. That reason is true before and after phase-rs#7029, which is the property a durable comment needs. Found by sweeping this branch's own claims against a branch in flight, rather than against the current tree — an instrument that came from review catching the same shape one layer down, where a doc comment in phase-rs#7029 asserted the paired arm "never consults `status`", true at that head and false here. Four surfaces carried the claim: both comments, the commit message of the first commit on this branch, and the PR body. The commit message is deliberately left alone — this repo squash-merges, so branch messages do not survive to the tree, and a force-push to reword a doomed message would churn review for no durable effect. The PR body is corrected in place, and the source comments here. Assisted-by: ClaudeCode:claude-opus-5
1 parent 52fbc5a commit b986780

1 file changed

Lines changed: 14 additions & 6 deletions

File tree

crates/phase-ai/src/duel_suite/compare.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,14 @@ fn classify_row(
410410
// wrong as one that hid a regression (the same reason the W/L tier warns on
411411
// L→W and the draw tier warns on draw→decisive).
412412
// * STILL failing is reported every run rather than passing quietly. Review
413-
// showed this is reachable, not theoretical: `--refresh-baseline` writes the
414-
// current report verbatim with no `any_fail` check, so one refresh from a
415-
// failing run would otherwise make that matchup exit 0 forever.
413+
// showed this is reachable, not theoretical: nothing revalidates a committed
414+
// baseline when it is loaded, so a baseline that already sanctions a failure
415+
// keeps sanctioning it, and that matchup exits 0 forever. (The wording is
416+
// deliberately about the baseline rather than about `--refresh-baseline`
417+
// writing without a verdict check: that WAS the mechanism, and #7029 adds the
418+
// missing check, so naming it here would make this comment false the day that
419+
// lands. Reachability does not depend on it — a baseline blessed before the
420+
// guard, or hand-edited, is unaffected by any guard on the write path.)
416421
//
417422
// It is a Warn and not a Fail deliberately: the exit code answers "did this change
418423
// make things worse", and the baseline — however it got that way — already
@@ -1445,9 +1450,12 @@ mod tests {
14451450
/// A matchup that was ALREADY failing in the baseline and is still failing is reported every
14461451
/// run, not passed over in silence.
14471452
///
1448-
/// Reachable, not theoretical: `--refresh-baseline` writes the current report verbatim with no
1449-
/// `any_fail` check, so a single refresh from a failing run would otherwise make that matchup
1450-
/// exit 0 forever. Warn rather than Fail is deliberate — see the arm's comment.
1453+
/// Reachable, not theoretical, and durably so: nothing revalidates a committed baseline on
1454+
/// load, so a baseline that already sanctions a failure keeps sanctioning it regardless of
1455+
/// what guards the write path. (#7029 adds the missing verdict check to `--refresh-baseline`;
1456+
/// this row stays reachable through baselines blessed before it, or hand-edited.)
1457+
///
1458+
/// Warn rather than Fail is deliberate — see the arm's comment.
14511459
#[test]
14521460
fn a_matchup_still_failing_is_reported_every_run() {
14531461
let games: &[(u64, Option<u8>, u32)] = &[(1, Some(0), 10), (2, Some(1), 10)];

0 commit comments

Comments
 (0)