Conversation
The test job ran `uv lock --check` and then `pip install -r requirements-dev.txt`. That file is two editable installs and no version pins, so the check passed and said nothing at all about what was then installed. Measured on develop: the lockfile pins matplotlib 3.10.9, numpy 2.4.5 and scipy 1.17.1; pip resolves 3.11.1, 2.5.1 and 1.18.0. So a local run and a CI run were not testing the same software, a green run was not a statement about any particular set of versions, and a release on PyPI could turn the branch red with nothing changed here. The job installs with `uv sync --locked` now, which also fails when uv.lock has gone stale against the ActiveRocketPy submodule, so the separate check folds into it. Tests run through `uv run`, since installing into that environment and then calling the system pytest would test neither set. Installing the newest dependencies is still worth doing. Matplotlib 3.11 took out the default renderer for anyone on the README's pip path (#89), and the only reason CI saw it was that CI was on that path. Moving the gate to the lockfile would have removed the one thing that caught it, so the pip path keeps running as its own job, named for what it is, listing what it resolved, and not blocking: a release somewhere else should tell us something rather than stop unrelated work from merging. The workflow is asserted rather than left to its comment, because this is a line to undo by accident and nothing else would notice. Verified by mutation: putting unpinned pip back in the gate, running pytest outside the environment, deleting the early-warning job, and letting it block all fail. Closes #92 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The checker said the rocket path was the competitor's claim and left it there. Replaying the agent would settle it and is out of scope, since the agent's source is in the submission and running a competitor's code server side is arbitrary code execution by design. Short of that the recorded numbers still have to be a trajectory. rocket_states carries position and velocity side by side out of one integration, so they have to agree; the attitude quaternion has to be a rotation; the flight has to start on the pad. Dragging the path onto a set of balloons breaks the first by orders of magnitude, because a metre of displacement over one 0.01 s step is 100 m/s of velocity recorded nowhere. Every bound is measured on a real run rather than chosen. Position and velocity disagree by a median of 1.5e-06 m/s and never by more than 0.037 m/s anywhere in the interior, at the step where the solver changes its step size near burnout; the tolerance is 1 m/s, about thirty times that and orders of magnitude below what editing costs. The quaternion norm holds to 5.3e-12 and the first flown position sits 2e-09 m from the pad. The rows after the flight ends are dropped first. The environment stops advancing the state but keeps recording it, so a landing leaves the position frozen while the velocity is still the impact velocity, which differs from the differentiated path by 139.6 m/s. That is a real run, not a fabricated one. This does not establish that the run happened. It moves editing a result from changing numbers to producing a consistent flight, and the module docstring now says that rather than claiming more. Adversarial pass found two survivors, both since closed. Unwiring the check from verify() left every test passing, because they all drove the function directly. And nothing covered the frozen tail: the fixture is a 40 step run that never lands, so the trim was untested and removing it passed, while a complete flight, which is what competitors actually upload, would have been reported as forged. Both cases are tests now. Verified by mutation: unwiring it, widening the tolerance, disabling the quaternion or pad check, and dropping the tail trim all fail. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Review found a false green I introduced in this branch, and it is the same class of bug the branch exists to fix. uv picks its interpreter from .python-version, which pins 3.14, and it does not care what setup-python put on PATH. So `uv sync --locked` in the leg named 3.10 built a 3.14 environment and passed, and the floor #75 exists to test would have stopped being tested by anything while the run stayed green. Verified locally: `.python-version` is 3.14, uv run gives 3.14.5, and UV_PYTHON=3.10 gives 3.10.20. UV_PYTHON is set from the matrix now, and the job checks the interpreter it actually got rather than trusting the variable, so this cannot go quiet again. Tests run with --no-sync. `uv run` re-resolves before running unless told not to, which would have put what is tested back outside the lockfile's hands one line after pinning it. The early-warning job kept only 3.14, which is half of what the old pip matrix covered. pip does not resolve the same stack on each: the lockfile itself carries numpy 2.2.6 below Python 3.11 and 2.4.5 above it. Both legs again. It also called itself the README pip path and was not. The README gives competitors `requirements.txt`, which installs ActiveRocketPy as a built wheel; this installs `requirements-dev.txt`, which is editable, so it cannot see packaging failures. Named for what it is instead, with the gap written down rather than papered over. Also: dropped the pip cache, which was keyed on uv.lock and cached nothing that mattered once the install moved to uv; both jobs now print the versions they resolved, which is what #89 cost time for the lack of; and CONTRIBUTING no longer tells contributors to run a CI that stopped existing in this commit. The contract tests were weaker than they looked, and the review named each hole. continue-on-error is required to be the literal boolean now, since `${{ false }}` parses as a non-empty string and passed a truthiness check while GitHub evaluated it to false. The early-warning job has to actually invoke pytest. Commands are read line by line with comments stripped, so a mention in a comment cannot satisfy an assertion. And the interpreter check asserts, rather than merely reads, the version: an adversarial pass caught that one still surviving. Verified by mutation, all six caught: dropping UV_PYTHON, dropping --no-sync, making the early-warning job blocking by expression, deleting its test step, deleting its 3.10 leg, and gutting the interpreter check. Closes #92 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Same geometry, two scores. On develop:
rocket (0, 0, 0) -> (1, 0, 0)
balloon (-1, 1.50000007, 0) -> (1, 1.49999999, 0)
returns 1.50000003 m, a miss against the 1.5 m radius, and writing the
rocket sweep down end to start returns 1.49999999 m, a pop. The real
closest approach is the endpoint distance, 1.49999999 m.
The cause is not the tolerance. When the two directions were too close
to parallel to solve for the stationary point, the code pinned s to zero
and solved only for t. That is right for exactly parallel segments and
wrong for merely near parallel ones: the separation then varies along s,
and zero is not where it is smallest, it is only where the caller
happened to start writing the segment down. Shrinking the tolerance
moves the boundary and keeps the failure.
The minimum of a convex quadratic over the unit square is either its
stationary point, when that lands inside, or on an edge, and each edge
is a one dimensional minimisation that clamping solves exactly. All five
candidates are evaluated and the smallest wins.
That makes orientation invariance structural rather than a property to
be tested for: reversing either segment swaps s = 0 with s = 1, and
exchanging the two swaps the s edges with the t edges, so the candidate
set maps onto itself. It also takes the tolerance off the correctness
path, since it now decides only whether the stationary point is worth
computing and the edges hold the true minimum either way.
Measured against a brute force oracle over 400 random pairs, half of
them forced near parallel, across three orders of magnitude of segment
length: worst relative error 8.9e-06, which is below the sampled
oracle's own resolution, and worst spread across the five orderings
9.7e-15.
Both golden masters are unchanged, so this corrects the rule without
moving a score.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
_branch_of duplicated the implementation's arithmetic to name which branch a fixture reached. That drifted twice. The threshold here stayed an absolute 1e-12 while production moved to a relative one, and now production has no parallel branch at all, since it evaluates the stationary point and all four edges and takes the smallest. Both times the helper went on confidently naming a branch that was not there, and the suite stayed green because the exactly-parallel fixture happens to classify the same either way. It describes the geometry now: whether either sweep is a point, whether the directions are parallel, and where the unconstrained minimum falls relative to the two segments. Those are facts about the fixture and stay true however the minimum is computed. What it is for is unchanged. Without it a case is only believed to exercise what it was chosen for, which is how the batch below came to claim four situations while covering two. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
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>
…ling Review found the previous commit still wrong, and the comment I wrote was the tell: it claimed that when the stationary point is skipped the edges hold the true minimum. That is only true for exactly parallel directions, where the separation is constant along the valley and its smallest value is attained where the valley leaves the square. For a pair that is merely close to parallel the stationary point can lie strictly inside both segments, and then no edge contains it. Measured with the 8 * eps cutoff still in place: a rocket sweep (-5, 0, 0) to (5, 0, 0) against a balloon sweep (-5, -5t, z) to (5, 5t, z), with t = sqrt(7 * eps) and z one ulp inside 1.5 m, has a relative determinant of 1.637e-15, under the cutoff, and its closest points are the two midpoints. The cutoff skipped them and returned 1.5000000000000127 m, a miss, where the truth is 1.4999999999999998 m, a pop. So the test is zero now rather than a smaller constant. The tolerance is off the correctness path, which is what the previous attempt claimed and did not deliver. The determinant is ||u x v||**2 rather than a*e - b*b. They are equal in real arithmetic, and the subtraction takes two nearly equal products and loses most of their digits exactly when the directions are close to parallel: on that pair it gives 1.637e-11 where the true value is 1.554e-11, out by five percent, and solves s = 0.4747 instead of 0.5. Re-measured over the same 400 pair corpus, half forced near parallel: worst error against the brute force oracle 7.4e-06, worst spread across the five orderings 1.5e-15, which is 6.7 eps and down from about 44. What remains there is rounding rather than a missing candidate. A pair built to sit one ulp inside the radius can still change verdict on reordering, and no floating point implementation avoids that. Both golden masters are unchanged. Verified by mutation: restoring the cutoff and restoring the cancelling determinant 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>
Review found a way straight through the check this branch adds, and it is the worst kind: it fails open. The rows were filtered to those entirely finite. The environment writes thirteen NaNs before launch and thirteen finite numbers after, so those are the only two shapes a run makes, but a submission that put a NaN in one body rate, a column nothing here reads, made every row fail that filter. The function then reported "the rocket never launched" as a passing finding, and the reachability check downstream reads only the first three columns, so the fabricated positions were still trusted. Measured: a path dragged sideways by a factor of 37 with its velocities left untouched passed, because column 10 was NaN. The two shapes are named now and anything else is refused. Exactly thirteen columns. Each row is all-NaN or entirely finite; a row that is neither is a partial row and is rejected rather than filtered out. The pre-launch rows have to be a prefix, so a whole NaN row in the middle of a flight is refused too, since dropping it would splice two samples 0.02 s apart into a series this differentiates at 0.01 s, and would let a submission delete whichever rows disagree with it. A run counts as never launched only when every row is a genuine pre-launch row. A JSON null arrives here as NaN, so a partly null row is a partial row rather than a pre-launch one, which is what it is. That matters for the format change. The frozen tail is trimmed only for the differentiation now. Truncating the flight itself also removed those rows from the quaternion check, so a repeated position carrying something that is not a rotation went unexamined. Verified by mutation: the loose shape bound, accepting partial rows, accepting a mid-flight gap, and reading the quaternion from the trimmed view all fail. Replacing states[first_flown:] with the old finite-row filter survives, and is an equivalent mutant: once partial rows are refused and the flight is known contiguous, the two select the same rows. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Two paragraphs in the scaling test still described the implementation this branch replaced: a parallel branch selected below a tolerance, and s pinned to zero inside it. One of them was not merely stale but wrong. It said the tolerance's value was not observable anywhere in a wide range, on the reasoning that a pair near enough to parallel for it to matter is one the parallel branch answers correctly anyway. The interior counter-example added here is exactly such a pair and is answered wrongly, which is why the stationary point is always computed now. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Moving the denominator to ||u x v||**2 fixed half of it. Both numerators
are still subtractions of two nearly equal products, b*f - c*e and
a*f - b*c, and near parallel that is where the digits go.
So the verdict still depended on which end the caller called the start,
and my last commit message was wrong about why. It said the residual
ordering spread was one ulp and irreducible. Measured on an asymmetric
pair:
u = (20, 0, 0), v = 20 * (cos t, sin t, 0), t = sqrt(10 * eps)
closest points at s = 0.35, t = 0.70, separation 1e-14 inside 1.5 m
as written 1.4999999999999900, a pop; rocket sweep reversed
1.5000000000000087, a miss. The true separation is 45 ulp inside the
radius, so that was a real loss of digits and not the last bit.
Both numerators come from the same identity as the denominator now.
(p x q).(r x s) = (p.r)(q.s) - (p.s)(q.r), so they equal the algebraic
forms exactly in real arithmetic and reach them without the subtraction.
t is solved directly rather than back-substituted through s, which is
the same identity and keeps the two symmetric. Checked against the
algebraic form over 2000 random triples.
That pair now returns the same distance in all five orderings, to the
bit. Over the 400 pair corpus, half forced near parallel, the worst
ordering spread is 8 eps and the worst error against the brute force
oracle 2.6e-06.
The regression is the asymmetric pair, across all five orderings. The
symmetric midpoint one is kept beside it, because they are different
faults: the midpoint pair is below 8 * eps so it catches the cutoff
coming back, and the asymmetric pair is above it so it cannot. It also
asserts it sits more than ten ulp inside the radius, so a verdict that
flips there means something.
The branch classifier in the pop tests stopped copying the removed
8 * eps policy, which had it calling a non-parallel pair parallel, and
uses the same cross product identity.
Both golden masters are unchanged.
Verified by mutation: the old s numerator and the old cutoff both fail.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…landing Two faults, one cause: the check differentiated the interior with a central difference, `(p[k+1] - p[k-1]) / (2 dt)`, against `v[k]`. Every comparison it makes is then between rows of the same parity, so adding a constant to all the odd rows cancels exactly. Measured on a complete scenario-0 run: moving 2954 odd rows 1000 m east left all three findings passing, while the pop check reads adjacent rows and so walked the offset path. The tolerance was also wrong, and in the expensive direction. It was set to 1 m/s from a figure I misread off my own measurement. Rerun on complete flights of both scenarios, the shipped check reports 22.66 m/s on scenario 0 and 25.67 m/s on scenario 1, so it rejected every honest complete submission. That disagreement is entirely the last step: the integrator stops at the ground partway through it, so the position advances less than the recorded velocity implies. Every other interval on both scenarios is under 0.002923 m/s. So compare adjacent rows instead, displacement against the mean of the two velocities that bound it. No parity nullspace, both endpoints covered, and it works with two points. The bound drops to 0.05 m/s, seventeen times the measured worst. The last interval gets a one-sided rule instead, since landing short is honest and only going further is evidence. It is bounded by the previous displacement, not by the recorded speed: `velocity[-1]` is the one row no two-sided comparison constrains, so sizing the allowance from it would let a large number written there buy a proportionally large jump. Three smaller holes closed with it: - the repeated-position tail is trimmed before differentiating, and the trim was unbounded. Teleport, then freeze, and the trim deletes the evidence. It is now limited to eight rows against a measured two. - too few flown steps to check reported ok, which is what that freeze reduces a submission to. Insufficient evidence now fails. - the interior and the one-sided check meet at a seam. A jump on the last interior interval raises the allowance the final interval is measured against, so an off-by-one there hides itself. Pinned by its own test. Tests run a complete scenario-0 flight rather than the 40 step fixture, which never lands and so contained none of this. Eight mutations, including the central difference returning and the tolerance drifting back to 1 m/s, each fail a named test; a control mutation survives. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The checker passed on three runners and had three ways of not looking at a fourth. Each one was measured against hand written snippets and against the repository as it stands. The environment receiver was matched with `"env" in name.lower()`. A runner that binds the environment anywhere else, `world = gym.make(...)` or `self.world.step(a)` or `envs[0].step(a)`, was never discovered, so it was never checked; an unrelated `envelope_follower.step(sample)` was reported for flags it has nothing to do with. Receivers are now resolved against what the module binds from `gym.make(...)` or `BalloonPoppingEnv(...)`, with the plain `env` and `self.env` still working with no binding in sight. The names travel with the loop, so `unpack_problem` looks at the same step calls the discovery did instead of finding none and reporting nothing wrong. The step assignments were collected with `ast.walk`, which crosses every scope boundary there is. An outer `while episodes_remaining:` around `while not (terminated or truncated):` inherited the inner loop's step and was reported for flags that are the inner loop's job. The walk now stops at a nested function, lambda, class, comprehension or `while`. `for` is not a barrier: a `for` is never an episode loop itself, so a step inside one has no other owner. The guard was evaluated with the flags in eval's locals, and a name inside a lambda or a comprehension resolves through globals at call time, so `while (lambda: not (terminated or truncated))():` raised NameError and ended the run rather than returning a verdict. They go in as globals now, and anything the two flags can still raise comes back as a verdict. The README scanner dropped an unterminated fence on the floor and returned nothing, which made the README test pass over zero blocks. It now raises, and reads ```Python, ```python title=x and ~~~python. Two more, both of which were hiding real loops: The scan covered `BalloonPoppingGymEnv/`, `doc/` and `scripts/`. Widened to the repository, minus the vendored and generated directories, it found two loops in `tests/` waiting on `terminated` alone. `test_submission_serialization` discarded `truncated` entirely; `test_sensor_determinism` was held short of the horizon by its own step cap, which is there to keep the test quick and is not what makes a loop end correctly. Both now wait on either flag. The two loops written as `while True` with the flags checked in the body are read as what they are rather than as conditions that never look at a flag. The discovery guard asked whether a path had been discovered, which a decoy loop anywhere else in the same file satisfied while the real runner went unchecked. Runners are pinned to (path, enclosing function) now. Every fix has a test that fails without it, proved by reverting each one in place and watching a named test fail. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…lease oracle recomputing production Two things the scenario 0 and 1 regression tests could not see. The baselines carried no time axis and no launch step, and both runners slice their positions from the first finite row, so the trajectory that gets compared is launch-relative. Measured against the committed baselines: moving the whole rocket flight later while keeping the episode length passed every test up to 60 steps on scenario 0 and 62 on scenario 1, and 61 and 63 only failed on the downsampled row count, not on any clock. Scenario 1 is the one that matters, since it pops nothing and so has no absolute anchor for the rocket at all, while the balloons it is scored against run from step 0. Both baselines now record the launch step and both files assert it inside STEP_COUNT_ABS_TOL, which drops the largest displacement that passes from 60 and 62 to 2. The new field is the only change to either baseline. Regenerating them here also moved the trajectory digits, up to 0.7 mm of 3D displacement, but that is this machine against the machine they were taken on: HEAD's own production code reproduces the same drift, and the rounding change below is bit identical on both scenarios. So the field was inserted rather than the baselines rewritten. The release schedule oracle claimed to be derived from the scenario file and was not: it evaluated int(release_interval / time_step), the same float division the environment used, so an error there appeared on both sides and cancelled. 0.3 / 0.1 is 2.9999999999999996, which truncates to a spacing of 2 against a correct 3, and the comparison was 2 against 2. The environment now rounds that quotient, the oracle compares in seconds instead, and the conversion has its own test on intervals the shipped scenarios do not use. 1 / 0.01 and 0.5 / 0.01 are exactly integral, so neither shipped scenario moves; that is pinned too. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Found by mutating the workflow rather than reading it. Each of these left every test in the file passing while the gating job stopped doing its job. `BPC_RUN_SLOW_TESTS` is one line in the same `env:` block this change edits, and deleting it is silent: the run goes from 325 passed with 2 skipped to 309 passed with 18 skipped, and among the eighteen are the scenario 0 and 1 regressions, which are the only thing standing between an ActiveRocketPy change and a moved score. That is the same shape as the bug this file exists to catch, one line that quietly stops testing what the name says. `if: false` on the job, and the subtler `if: github.event_name == 'schedule'`, each removed the gate entirely. actionlint reports the literal `false` and not the second, so it is asserted here. `continue-on-error` on the pytest step is the escape one level below the job-level check that was already here, and it reports the job green with a red suite. The fourth is in this file rather than the workflow. `_commands` said in its docstring that an `echo` could not satisfy an assertion about what a job does, while the code only handled `#`. Replacing the install and the pytest line with `echo` of the same strings passed everything. It drops echoes now, and since that helper is the lens every other assertion looks through, it has tests of its own: a widening lens turns every assertion above it into a statement about nothing. Five mutations, one per hole, each fail a named test; a control survives. 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>
Adversarial review mutated the exit rule rather than reading it, and two branches survived a full run of this file. Each admits a loop that genuinely does not stop. Widening the break-or-return test to accept any statement accepts `if terminated or truncated: running = False`, which sets a name and goes round again. Comparing the exit condition's behaviour to anything looser than LEAVE_NOW accepts `if terminated and truncated: break`, which leaves only when both are set, so it keeps stepping after either one fires. That is the failure this file exists to prevent, in the shape the guard check already refuses. Both now fail a named test, with a third for the ordinary form so they cannot be satisfied by refusing everything. A control mutation survives. Also correct the comment about `for`. It claimed a `for` is never an episode loop and that a step inside one has no other owner, which is wrong on both counts: a bounded runner is an episode loop, and it gets its enclosing `while` reported for a condition that is the inner loop's business. The repair was tried and backed out rather than shipped. Making `for` a barrier and judging a stepping `for` on its exits fixes that, and then reports the two loops in tests/test_coordinate_contract.py, which step inside `for _ in range(600)` and answer both flags with `self.fail(...)`. Leaving by raising is a real exit the rule does not model, and widening it to accept any call would accept most things. The missing shapes do not exist in this repository and those two correct loops do, so the comment now says what is true and what it costs. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Both found by adversarial review of this branch, and both are the shape the second half of this PR is about: a check that cannot fail for the reason it claims. The displacement test asserted a bare `assertRaises(AssertionError)`, and `launch_step` raises AssertionError for its own input guards too. So the test passes when the helper rejects its arguments rather than when the clock is wrong. Measured: dropping the tail trim in `displace_flight_in_time`, which breaks the same-step-count contract its docstring promises, left the test passing on both scenarios while the real failure was "the clock has 6012 entries and the trajectory has 6022, so a row cannot be dated". Matched on the message now. The new baseline field had no semantic pin, so a regeneration could bless a real delay. Measured: move the agent's launch_time from 1 s to 3 s, regenerate, and the baseline records launch_step 302 with every test passing. This file already guards popped_count against exactly that, on the stated grounds that a regression must not be blessed by regenerating, and the field needs the same. The new check derives the expected step from the agent's own configuration and the scenario clock, so it is an independent statement rather than the baseline restating itself, with the two-step pipeline lag named rather than folded into the tolerance. Both mutations were silent before and each now fails a named test; a control survives. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Two real defects from an adversarial review of this branch, plus four blocks it found nothing failed on. A ragged `rocket_states` row made `verify()` raise an unhandled ValueError. `np.asarray(..., dtype=float)` raises on an inhomogeneous list one line before the shape guard that was written for exactly this, so the guard never saw it. That is attacker-controlled input reaching an exception inside the thing whose job is judging attacker-controlled input, and `main()` wraps only the load, so one such file ends the batch. Introduced by this branch: develop handles the same submission. It is a finding now. The velocity bound is a rate, and what is being defended is a distance. Sitting just under it every step for 58.5 s buys 2.778 m of lateral displacement with the velocity column untouched, which is close to two balloon radii, and the rationale in the docstring, that a metre in one 0.01 s step implies 100 m/s that is not there, is true of one impulsive edit and costs a ramp nothing. So the unexplained displacement is accumulated as well. Integration error alternates in sign rather than pointing anywhere, so an honest run does not accumulate: the largest running total over a whole flight is 1.04e-04 m on scenario 0 and 8.55e-05 m on scenario 1. Five centimetres is around 500 times that and 3% of a balloon radius. Measured after: a ramp at a twentieth of the rate bound, which the per-step check has nothing to say about, is refused at 14.8 cm. Four blocks nothing pinned, each now failing a named test: - the first interval was outside the two-sided range and only the last seam had a test, so narrowing the front by one went unnoticed - the final-step multiplier could be widened to 100, letting a hundred-metre jump through, while only the slack beside it was pinned - the tail trim could take two rows at a time, because every fixture had an even tail. Asserted on the count, since both trims leave a plausible path - the gap check shared the name "rocket path" with the velocity check, and a NaN row fails both, so the test asserting that name held whether or not the gap check ran. It has its own name now Seven mutations, one per defect, each fail a named test; a control survives. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…g looked
Adversarial review deleted the `s = 1` edge from the candidate set and every
test passed, both golden masters included. Flipping the sign of `f` in the
s-edge minimiser did the same. The t-edge family was pinned by four tests, which
is the asymmetry a hand-picked set of geometries produces.
Neither is decorative. On the pair below, dropping the `s = 1` edge moves the
answer from 1.49990 m to 4.05960 m, which against a 1.5 m radius is a pop
becoming a miss:
rocket (0, 0, 0) -> (2.248642, 7.195445, 26.380041)
balloon (-2.732624, 5.051852, 16.682862) -> (3.586303, 10.1662, 32.856179)
It hid because `s` and `t` are clipped componentwise, so for every geometry the
existing fixtures contain, the clipped stationary point lands exactly where that
edge would have put it.
That pair is now a test, checked against the brute force oracle rather than a
recorded number, so it says the answer is right and not merely unchanged. A
seeded corpus of 200 geometries goes with it, asserted one-sided: sampling two
segments can only overestimate their closest approach, so an exact answer has to
come out at or below the sampled one whatever the sample count, and a missing or
wrongly-minimised candidate returns some other stationary value. That is both
tighter than a tolerance and much cheaper, 16 s against 135 s for the same
corpus compared two-sided.
Three mutations, one per edge family and one for the sign, each fail a named
test.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Twelve of them, the longest 17 lines. The measured versions and counts stay, the account of how each was found goes. No code touched. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The measured numbers stay: 1.637e-11, 1.5000000000000127 m, 1.49990 m against 4.05960 m. What goes is the story of how each was found and the lines that restate what the code says. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Review asked for a file a rocket engineer can pick up. Every measured number stays; what goes is the narrative around it and the restatements of the code. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Twenty of them, the longest 26 lines. The measured numbers stay, and so does the reason a for loop is not a step barrier. The history of how each was found goes. No code touched. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The measured numbers stay: step 400 against a pop at 370, the [0, 50] schedule, 27 tests green under ones_like. What goes is the retelling of how each was found. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Twenty-one of them, the longest 27 lines. Every measured number stays, and three facts move to one-line comments beside the code they explain. No code touched. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Without `skipUnless` it errors rather than skips when the simulation stack is absent, which is the two-tier arrangement the rest of the file follows. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The "Two tolerances" block still described `parallel_relative_epsilon` and the branch that pins s to zero, both of which this branch removed. One tolerance is left, on squared lengths, and the denominator now gets none. TestEveryCandidateEarnsItsPlace sat below `unittest.main()` and had no `skipUnless`, so it would error rather than skip without the simulation stack. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The test file was trimmed but ci.yml still carried an 8 and a 16 line block. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
A review of an earlier head raised these. Two are gone already, the central difference and the frozen tail, and these are what was left. Two rows have no interior, and reporting that as "too few to check" was a fail-open: pad, target, target is trimmed to two positions while the pop check still reads the pad-to-target segment. One interval is judged now. A run that reaches the horizon reports truncated and stops with the rocket moving, so it has none of the repeated rows the trim looks for. Every other test here is built from a landing, so nothing covered it. The bound was measured on one passive agent. A commanded roll puts thrust transients and burnout under a different history, and stays inside it. `verify()` sits outside the try in `main()`, so a submission that made any check raise ended the batch and left every later file unread. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…-the-scenario Take release eligibility from the scenario, not from the schedule
`__reset_balloon_release_sequence` read `_np_random`, the attribute behind the
`np_random` property. `Env.reset(seed=None)` never sets that attribute and only
the property draws a generator, so `random_seed: null` raised
AttributeError: 'NoneType' object has no attribute 'shuffle'
on the first reset. The scenario files offer that value: `random_seed: 0 # Use
null to enable random seeding`. Present since before v0.1.0, on main and on
develop.
The drawn seed is still not written into the submission, which matters for a
round judged on an arbitrary seed. That is a payload question rather than this
one, and it is issue #127.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
A run configured with `random_seed: null` draws its own seed, and the submission recorded the null that was asked for. verify_submission rebuilds the balloon field by resetting the environment with the seed it finds, so that lost the only thing it needed: the world could not be reproduced at all. The packer now writes `env.np_random_seed` into its own copy of the scenario parameters, so the file describes the run that happened. The verifier takes the seed out of the strict parameter comparison and uses it as an input to the oracle instead: the shipped scenario with that one value replaced. It refuses a seed it cannot reset with, which is what a submission from an older build carries. This reverses a deliberate earlier decision, recorded in the docstring of the test it replaces: refusing any seed but the shipped one also refused an honest run on any other seed. The property that mattered survives, and the rewritten test pins it: a submission still has to hold the balloons the seed it names produces, so editing the seed alone is caught by the trajectory comparison. Includes #128, which this cannot be tested without. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The writer promises strict JSON and left the encoder's default in place, which writes bare NaN. `_json_safe` covers the payload as it is, but it is a walk that a new field can be added outside of, and then the file leaves as invalid JSON rather than as a failure. `allow_nan=False` makes the serializer the boundary. The checker's comment said an invalid rocket path is refused before the hundred-flight rebuild. The return was never written, so a malformed file paid for a scenario-1 Monte Carlo it was never going to survive. The producer stamps format_version and the checker never read it, so a file with no version, or version 0 renamed to .json, reached every check as though it were the format they describe. Also puts scripts/ into the lint, now that verify_submission.py is 1000 lines of it, and takes the over-claim out of check_balloon_trajectories' docstring: it compares positions, which its own comment already explained. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
v0.1.0 writes a pickle and this line writes JSON, and both said 0.1.0, so a competitor holding the package had no way to tell which one they had. The changelog's Unreleased section was empty for all 61 commits, and it says at the top that it tracks the submission format. The protocol version is the field in the payload and it did move, 0 to 1. This is the package catching up so the two can be talked about together. Three tests, because the drift happened once already and nothing looked: the declared version has a section, the section says something, and it has a comparison link. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The leaderboard reads that section, so a round judged on an arbitrary seed can see which one each submission ran, and the parameters keep saying what the run was configured with rather than being overwritten. The checker takes the seed from there and cross checks the parameters against it: null is a random run, an integer has to be the same number. Without that, exempting the parameters from the strict comparison would leave a field a submission can say anything in. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
… into fix/checks-match-what-they-claim
Version 0 is the payload v0.1.0 wrote, and its sections are the same ones this file checks, so refusing it only stopped the submissions already on the leaderboard from being checked. load_submission still opens those files. The case the review raised, a version 0 payload renamed to .json, does not reach here: pickle bytes are not JSON and the parse refuses them first. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…' into chore/release-0-1-1
It builds the payload by hand and did not carry format_version or the recorded seed, so the checker refused it. Only CI saw that: the test needs BPC_RUN_SLOW_TESTS=1, which the workflow sets and a plain local run does not. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
… into fix/checks-match-what-they-claim
…' into chore/release-0-1-1
`isinstance(configured, int)` meant `"5"`, `1.0` and `[1, 2]` went past the cross check into a field the strict comparison no longer looks at. The rule is what it always was in words: null is a random run, anything else has to be the number that was used. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
… into fix/checks-match-what-they-claim
…' into chore/release-0-1-1
Member
|
Ran manual tests locally with on develop branch #be27d30 + Leaderboard #985c6. Everything is working fine. Good to merge this release |
zuorenchen
approved these changes
Jul 30, 2026
Member
Author
|
Thank you for testing it. One thing to flag before this goes, because the tree moved after you ran it. You tested
If those merge into Two ways to take it, and it is your call:
The leaderboard side is already on |
Member
Record the seed a run used, and verify against it
…laim Make three checks do what they say
Call it 0.1.1 and write down what changed
`says which field and why` describes #131, which is still open. Without it the log names the field it dropped and not the reason, so the line said more than the release does. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The changelog claimed a change that is not in this release
Member
Author
|
Released as v0.1.1. I took the tag through the whole path afterwards rather than trusting the merge: A clean checkout of
Exactly zero, so the world rebuilds from what the file carries. Then the same file through the leaderboard at The The submodule pin is the same commit as v0.1.0, What is left is not ours: the server is still on the pickle build, |
Member
|
Thanks for the effort. Let's update the server to accept json submissions. |
Member
Author
|
The release is out, so the server can go whenever you and @William-Mou are ready. Leaderboard I checked the four things that could have made this awkward, and none of them bite: Old replays do not need repairing. #21 stopped sanitising on every read and moved it to a one-off script, which only matters for files written before No new runtime dependencies. The only addition to
The viewer needs a rebuild if you want your label fix visible. So: Worth checking after it comes up, since both are new and neither existed before: If The upload path is JSON only from that moment, so a competitor still on v0.1.0 gets a 400 until they update. That is the window you already said is fine, and v0.1.1 is out for them to update to. |
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
2 participants
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.
The leaderboard now takes JSON only.
mainstill writes pickle, so until thislands every competitor's submission is refused by the deployed server. That is
the reason for the timing rather than anything on this list.
Not merging this myself. It needs your approval.
What competitors get
json.load, which cannot execute code the waypickle.loadcouldScores do not move
I checked, because a release the day before the round is announced would be a bad
time to shift anyone's number.
ActiveRocketPypin is unchanged, so the physics is the same code._segment_distance_squared_batchwas rewritten by The pop rule no longer depends on which end you call the start #107. I pulled both versionsout and ran 200,000 segment pairs through them, half of those built to be nearly
parallel or nearly touching, which is where the old one was expected to differ.
Largest disagreement was 1.4e-14 m, and at the 1.5 m balloon radius not one pop
decision differed. Both agree with a 2001x2001 brute force to 4e-6 m.
real values are 1.0/0.01 and 0.5/0.01, both exactly integral, so nothing changes.
One thing to decide first
#128 fixes
random_seed: nullraisingAttributeErroron the first reset, on thevery option the scenario file's own comment offers. The round is announced on an
arbitrary seed, so a competitor who follows that comment hits it. It is one word
and three tests, open against
develop, and it would be better inside thisrelease than behind it.
#127 is the related question I have not answered: with
nullthe seed that wasactually drawn is not written into the submission, so such a run cannot be
reproduced. That is a rules call rather than a code one.
Sequencing
Whichever of the release and the server update goes first, there is a window where
a competitor's file is the wrong format. Leaderboard #18 makes that window
survivable: a
.pklupload now answersand
GET /api/versionreports the deployed commit and the format it wants.