fix(workbench): retire beh.sandbox from the baseline, confirmed at n=10 - #453
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9 tasks
`assertAgainstBaseline` reads the UNION of the scenario's expectedFailures and the baseline file's, so removing the entry from the scenario alone left it live: every run printed "défaut connu semble corrigé sur wizard-enhance/beh.sandbox". A permanent notice is ignored as fast as a permanent green, which is what the bidirectional ratchet exists to prevent. Removed on two arguments, not one. Structural: `deep-agent/service.ts` builds its agent with `createAgent` and the OpenScreen tools alone, so the phantom ls/grep/glob surface this check probes for is gone. A call to one of them today would be a hallucination — an UNEXPECTED failure, which is the signal the scenario wants. The check itself stays for exactly that. Measured, as the README requires before any removal: n=10 on 2026-08-21 against deepseek-v4-flash (resolved; deepseek-chat requested), 10/10, Wilson [72%, 100%]. Verified after the edit that the notice stops firing. `dsl.focus.not-fabricated` scored 10/10 in the SAME run and stays, because it has only the observation behind it and no structural argument — the faulty code is still there. That is the difference between "the cause is gone" and "variance was kind ten times", and it is why the numbers alone were never going to settle this. The single-rep verification run makes the point again: `beh.multiplier` passed there and printed "seems fixed", having been 9/10 at n=10. The baseline's `behaviour`/`dsl` numbers are left as the 2026-07-31 archive they are, and now say so. Nothing READS them — the ratchet compares ids only — and re-recording from this run would freeze an anomalous pass in which `beh.counts` regressed. Hand-edited rather than `--update-baseline`, which would have harvested both intermittent entries on one green run. Also renames the synthetic `beh.no-false-negative` in `l0/stats-report.wb.ts`: that id no longer exists anywhere in the pack, and a fabricated fixture wearing a dead real name reads as a live one.
EtienneLescot
force-pushed
the
fix/retire-sandbox-baseline
branch
from
August 21, 2026 22:32
24464ad to
0bedc05
Compare
9 tasks
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.
Summary
assertAgainstBaselinereads the union of the scenario'sexpectedFailuresand the baseline file's, so removingbeh.sandboxfrom the scenario alone left it live. Every run printed:A permanent notice is ignored as fast as a permanent green, which is exactly what
lib/baseline.tssays the bidirectional ratchet exists to prevent.Removed on two arguments, not one
Structural.
deep-agent/service.tsbuilds its agent withcreateAgentand the OpenScreen tools alone — the phantomls/grep/globsurface this check probes for is gone. A call to one of them today would be a hallucination, i.e. an unexpected failure, which is the signal the scenario wants. The check itself stays for exactly that reason; only the expectation is retired.Measured, as
README.md§ "Le ratchet tourne dans les deux sens" requires before any removal:beh.sandboxdsl.focus.not-fabricatedbeh.multiplierbeh.countsn=10 on 2026-08-21, resolved model
deepseek-v4-flash(deepseek-chatrequested; theMODÈLE RÉSOLUnotice fired). Verified after the edit that the notice stops firing.Why
dsl.focus.not-fabricatedscored 10/10 and stays anywayIt has only the observation behind it — no structural argument, and the scenario documents the faulty code as still present. That is the difference between "the cause is gone" and "variance was kind ten times", and it is why the numbers alone were never going to settle either entry.
The single-rep verification run after the edit makes the point again from the other side:
beh.multiplierpassed there and printed "seems fixed", having just been measured 9/10 at n=10. One green run proves nothing, which is precisely what the README says.The baseline's numbers are an archive, and now say so
behaviour: 0.6667was computed on 2026-07-31 over a check set that no longer exists —beh.no-false-negativehas since become the judgedbeh.attributes-the-limitunder a new id. They are not re-recorded:BaselineVerdictderives no field from them andassertAgainstBaselinecompares ids only; everybehaviourread incli.tsandreport.tscomes from the run's own score, not the baseline's.beh.countsregressed.Hand-edited rather than
--update-baseline, which would have harvested both intermittent entries on a single green run — the exact failure the README forbids.Also
l0/stats-report.wb.tsusedbeh.no-false-negativeas a synthetic id in a fabricated report fixture. Nothing read it from the registry so nothing broke, but a fixture wearing a dead real name reads as a live one. Renamed tobeh.synthetique.Related issue
Refs #428 — one of the two pre-existing defects surfaced by #452 and deliberately left out of it, each being a claim about the product that needed its own measurement.
Stacked on #452
Branched from
feat/workbench-migrate-cursor-denial, notmain: that branch already editsbaselines/wizard-enhance.json(it removed thebeh.no-false-negativeline) andwizard-enhance.scn.ts. Merge #452 first.Type of change
Release impact
Desktop impact
Testing
npm run wb:live -- --scenario wizard-enhance --reps 10 --label sandbox-confirm— the measurement above.beh.sandboxnotice no longer fires while the two intermittent ones still do.workbench/l0— 44 failures, the pre-existing absent-fixture baseline, unchanged; 328 passing.workbench/l1— 21 failures, same baseline; 62 passing.npm run wb:typecheck,npx tsc --noEmit,npx tsc -p tsconfig.test.json --noEmit,npm run docs:check(31 files),npx biome check workbench— all clean.One thing this run surfaced and did not fix
beh.countsfailed 1 of 10 and is not inexpectedFailures, so the ratchet correctly flagged it as a regression. It is a genuine finding about model behaviour on this scenario, not about anything in this PR, and it needs its own measurement rather than a rider here. Worth an issue if it reproduces.