Take release eligibility from the scenario, not from the schedule - #108
Open
thc1006 wants to merge 3 commits into
Open
Take release eligibility from the scenario, not from the schedule#108thc1006 wants to merge 3 commits into
thc1006 wants to merge 3 commits into
Conversation
The checker accused the repository's own scenario 0 run. Scenario 0 starts every balloon released at reset and its release schedule never fires. It still builds one, and with the shipped seed balloon 0 is scheduled for step 400 while the committed baseline pops it at step 370. Comparing pops against the schedule therefore reported the official submission as popping a balloon before it was released. Eligibility now comes from the environment: a balloon is eligible from the first step when reset already has it released, and from its scheduled step otherwise. Read from the canonical reset rather than special cased on the scenario number, so it stays right for whatever scenario 2 does. The same mask replaces the submitted status in the reachability check. Reading the status there let a file mark a balloon released early, point at a rocket pass from before the real release as its closest approach, and flip to popped on the official step: the timing check saw a pop that was not early and the reachability check saw a balloon it thought was released, so both passed on a pop the environment would never have detected. Every existing test built its submission by hand from scenario 1 cut to one balloon, and none ran a shipped scenario end to end. That is what let this ship. There is a test now that runs scenario 0 with the example agent and puts the result through verify(), which is the cheapest possible statement that the checker is fit to judge a real submission. Verified by mutation: the bare schedule and the submitted release mask both fail. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Review pointed at the tests and it was right: none of them reached _release_eligibility on the production path. Scenario 0 starts every balloon released, the reduced scenario-1 fixture has one balloon and so a release step of zero, and the forged-release test supplies its own mask. All three produce an all-True mask either way. Measured: returning ones(...) from that function, and moving its comparison off by one, both left every test in the file green. So the scenario-1 bypass this branch exists to close was pinned by nothing. It takes the regenerated facts now rather than the scenario, which makes the rule a pure function with its own boundary tests, and lets verify() rebuild the canonical world once instead of once per caller. That rebuild runs the balloon Monte Carlo, a hundred flights for scenario 1, and it was being paid for twice per submission. The suite drops from 59 to 35 seconds, and a call-count test keeps the next check that needs the world from making it three. Two smaller ones from the same review. The new full-scenario test class was defined after the unittest.main() guard, so running the file directly skipped the only end-to-end check in it. And when eligibility was missing the release check reported "every pop is on or after the balloon's release step", which is an affirmative statement about a comparison that never ran; it says it was not evaluated instead. Verified by mutation: all-True eligibility, the off-by-one, an unevaluated check reporting success, and a second regeneration all fail. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Adversarial review of this branch found the fix revertible at its call site with a completely green suite. Replacing the mask with `ones_like` inside `verify()` left all 27 tests passing, and the same stub accepted a fabricated score end to end: forge the submitted status to released from row 0, park the rocket on a balloon's ground position, flip to popped on the scenario's release step, and the verdict comes back ACCEPTED with a score of 1. The cause is the fixture rather than the tests. Every check either drives a consumer directly and hands it a mask, or goes through `verify()` with a submission whose mask is all True: scenario 1 cut to one balloon gives a schedule of `arange(1) * 50`, which is `[0]`, and scenario 0 starts every balloon released. So the mask had no observable effect at the call site and nothing could tell whether it arrived. Two balloons is the smallest fixture that can. The schedule becomes `[0, 50]`, the run is 40 steps, so balloon 1 is on the ground for the whole submission and a claim against it is refusable without reference to where the rocket went. The fixture asserts that shape rather than assuming it, since a one-balloon schedule would make the two tests beside it hold with the rule deleted. The shape guard is pinned too. The mask is built from the scenario and the status matrix comes from the submission, so their widths are a competitor's to disagree with, and combining them anyway raises out of `verify()`, which `main()` does not catch. One malformed file would end the batch. Three mutations that were silent now each fail a named test: the mask stubbed all-True inside `verify()`, the consistency consumer unwired, and the shape guard removed. A control survives. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.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
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 to #97. The checker it added accuses the repository's own scenario 0 run.
Measured on develop
Scenario 0 releases every balloon at reset and its schedule has no effect, but the checker compared each pop against that schedule. An honest official submission is reported as popping a balloon before release.
The other direction, in scenario 1
Reachability used the submitted status as its release mask:
So a file could mark a balloon released before the scenario allows, point at a rocket pass from before the real release as its closest approach, and flip to popped on the official step. The timing check saw a pop that was not early; the reachability check saw a balloon it believed released. Both passed, on a pop the environment would never have detected because it would not have looked at that balloon during that interval.
The fix
One mask, taken from the environment: a balloon is eligible from the first step when reset already has it released, and from its scheduled step otherwise. Read from the canonical reset rather than special-cased on the scenario number, so it stays correct for whatever scenario 2 does. Both checks use it.
Why it shipped
Every existing test in this file builds its submission by hand, from scenario 1 cut to a single balloon. None ran a shipped scenario end to end. There is one now: scenario 0 with the example agent, through
verify(), asserting no findings. That is the cheapest possible statement that the checker is fit to judge a real submission, and it fails against the old rule.Verified by mutation, both caught:
Local CI green on a clean tree: ruff check, ruff format,
uv lock --check, 318 passed withBPC_RUN_SLOW_TESTS=1.Not in this PR
The same review raised status-matrix schema validation (an empty
balloon_statuscurrently passes a zero-score submission) and per-file isolation in the CLI batch loop. Both are real; they are a separate change from the release rule.