The #293 guard added in #298 catches a misdirected update half exactly once per server lifetime, and the shipped default --repetitions 3 spends that one shot inside a single invocation. The run then publishes a clean-looking result with update_unattributed: 0.
Mechanism
Three facts on master e99d2b5, each individually correct:
finalize_update_stats is infallible — the mixed phase completes all its writes before any verdict is reached. (This was deliberate, and right: it is what fixed the --allow-partial-corpus blocker.)gate_update_attribution is called at experiment.rs:1585, inside the repetition loop that opens at :1539.--repetitions defaults to 3 (cli.rs:182), and publication is best-of-N (:1655, " (best of reps)").
The writes that rep 1 detects as unattributed are the writes that create the rows whose absence it detected. Rep 2 therefore sees every write as a clean overwrite, reports update_unattributed: 0, and is published as the best rep.
Reproduction
400-row corpus, ids 300–399 renamed to 1000–1099 — still 400 indexed docs, so the reuse check reports verified and no waiver is involved.
### RUN 1 (--repetitions 1, corpus reset to 400):
exit=1 results=0 DBSIZE=430
### RUN 2 (identical command, nothing reset):
exit=0 results=2 DBSIZE=430
published: {'update_count': 120, 'update_unattributed': 0, 'update_rps': 1158.2, 'mean_recall': 0.74}
And within one invocation at the shipped default:
Experiment stage: Reuse check — server holds 400 of 400 expected rows
Search failed (rep 1/3): mixed workload: 30 of 120 dispatched updates were accepted by the server ...
→ QPS: 411.0, Recall: 0.7400, ... (best of reps)
--- result files: 2
{'update_count': 120, 'update_rps': 1233.0, 'update_attribution': 'corpus_row',
'update_unattributed': 0, ...}
corpus_reuse = {'status': 'verified', ..., 'waived_by_allow_partial_corpus': False}
Why this matters
It defeats the guard's own headline scenario. A misdirected update half — the #293 source-bug class — writes to wrong:<id> on rep 1, is detected, and those keys then exist. Rep 2 reads every write as a clean overwrite and publishes. The published result file carries no trace: update_unattributed: 0, update_attribution: corpus_row, corpus_reuse.status: verified, no waiver.
Partial mitigation: record_rejected_experiment still fires, so the run summary contains both a rejection and a green point for the same (engine, dataset) pair — 1 entry in rejected_experiments, 1 in search_results. A reader of the per-search JSON alone cannot distinguish this run from a clean one.
Not introduced by the measurement/policy split
At 454d4d1, before that split, finalize_update_stats returned Err per-rep with the same effect. The split did not create this and did not fix it. The --allow-partial-corpus blocker it did fix is genuinely fixed — verified separately:
⚠ WARNING: mixed workload: 30 of 120 dispatched updates ... (continuing because
--allow-partial-corpus is set; `update_unattributed` is recorded in the result file)
exit=0
published: {'update_count': 90, 'update_unattributed': 30, 'update_failures': 0, 'mean_recall': 0.74}
Why the review missed it
The gate was mutation-tested thoroughly and in isolation — never-abort, always-abort, inverted waiver, and a dropped denominator term were all caught, and four integration tests drive the failing branch end to end. Every one of those runs uses a single repetition. The dimension nobody varied is the repetition count, which is 3 by default. The guard is correct; its interaction with best-of-N publication is not.
Suggested direction
Hoist the decision above the rep loop: reject the point if any repetition reported unattributed writes, rather than rejecting a repetition. Alternatively carry max(unattributed) across reps into the published record, so a run that ever saw an unattributed write cannot publish a zero.
The second is weaker but strictly additive, and it preserves the property that made the split correct — measurement stays per-rep and infallible, and only the policy input becomes cross-rep.
Either way the fix needs a test at --repetitions 3 specifically. A test at 1 cannot see this.
Provenance
Found by an adversarial review of PR #298 that reported after the PR merged. Reproduced live against merged master e99d2b5 with a dedicated Redis on a private port; the two structural facts (repetitions default 3, gate inside the loop at :1585 vs loop at :1539) were independently confirmed by reading the merged tree.
The #293 guard added in #298 catches a misdirected update half exactly once per server lifetime, and the shipped default
--repetitions 3spends that one shot inside a single invocation. The run then publishes a clean-looking result withupdate_unattributed: 0.Mechanism
Three facts on master
e99d2b5, each individually correct:finalize_update_statsis infallible — the mixed phase completes all its writes before any verdict is reached. (This was deliberate, and right: it is what fixed the--allow-partial-corpusblocker.)gate_update_attributionis called atexperiment.rs:1585, inside the repetition loop that opens at:1539.--repetitionsdefaults to 3 (cli.rs:182), and publication is best-of-N (:1655," (best of reps)").The writes that rep 1 detects as unattributed are the writes that create the rows whose absence it detected. Rep 2 therefore sees every write as a clean overwrite, reports
update_unattributed: 0, and is published as the best rep.Reproduction
400-row corpus, ids 300–399 renamed to 1000–1099 — still 400 indexed docs, so the reuse check reports
verifiedand no waiver is involved.And within one invocation at the shipped default:
Why this matters
It defeats the guard's own headline scenario. A misdirected update half — the #293 source-bug class — writes to
wrong:<id>on rep 1, is detected, and those keys then exist. Rep 2 reads every write as a clean overwrite and publishes. The published result file carries no trace:update_unattributed: 0,update_attribution: corpus_row,corpus_reuse.status: verified, no waiver.Partial mitigation:
record_rejected_experimentstill fires, so the run summary contains both a rejection and a green point for the same (engine, dataset) pair — 1 entry inrejected_experiments, 1 insearch_results. A reader of the per-search JSON alone cannot distinguish this run from a clean one.Not introduced by the measurement/policy split
At
454d4d1, before that split,finalize_update_statsreturnedErrper-rep with the same effect. The split did not create this and did not fix it. The--allow-partial-corpusblocker it did fix is genuinely fixed — verified separately:Why the review missed it
The gate was mutation-tested thoroughly and in isolation — never-abort, always-abort, inverted waiver, and a dropped denominator term were all caught, and four integration tests drive the failing branch end to end. Every one of those runs uses a single repetition. The dimension nobody varied is the repetition count, which is 3 by default. The guard is correct; its interaction with best-of-N publication is not.
Suggested direction
Hoist the decision above the rep loop: reject the point if any repetition reported unattributed writes, rather than rejecting a repetition. Alternatively carry
max(unattributed)across reps into the published record, so a run that ever saw an unattributed write cannot publish a zero.The second is weaker but strictly additive, and it preserves the property that made the split correct — measurement stays per-rep and infallible, and only the policy input becomes cross-rep.
Either way the fix needs a test at
--repetitions 3specifically. A test at 1 cannot see this.Provenance
Found by an adversarial review of PR #298 that reported after the PR merged. Reproduced live against merged master
e99d2b5with a dedicated Redis on a private port; the two structural facts (repetitionsdefault 3, gate inside the loop at:1585vs loop at:1539) were independently confirmed by reading the merged tree.