Replace the singular player with a participant roster - #19
Open
sbddesign wants to merge 10 commits into
Open
Conversation
`player` is gone from `game.ts`. `Game.start` takes a `MatchSetup` with one hull per seat, `Game.step` takes one `Controls` per seat, and a ship is flown by whatever intent was supplied for it — the milestone-1 substitution applied to everybody rather than to one privileged hull. Single-player is a match of one. The dividing line everything else follows from: a seat decides outcomes, `local` only decides what is drawn. Camera, HUD, alarms and gun pitch read `local`; nothing that reads it may reach a hull, a score or a result, or two machines watching one match would disagree about what happened in it. Writing the check for that found two leaks, both invisible with one seat. The win bonus was added to the drawn seat's running score, so the seat being watched would end the match with a different number from the seat beside it — `sealResult` now reads state and writes none. And elimination ended the run when the drawn seat died rather than when the arena emptied, which made the moment a match resolves depend on who was looking. Death is now a per-seat state instead of a mode the whole game enters. The old `stepDeathSequence` returned early from the tick and ran a second copy of the arena loop inside the cutscene; that works for exactly one dying participant and cannot be made to work for two, so there is one arena loop and a wreck is just a hull that happens to be dead. The alarms that the early return used to silence by accident are now gated on the drawn seat being alive on purpose. Respawn landed as a policy, not the default, which is the one place this departs from the plan. `PLANS/NEON_ORBIT_PHASE_B.md` had milestone 3 replacing run-end with respawn outright — but single-player *is* a match with one seat, and a shipped game where dying neither ends the run nor reaches the debrief has no lose condition at all. So `MatchSetup.respawn` defaults to off and PvP turns it on. One code path; the flag only decides what a finished cutscene hands over to. Faction resolution goes seat-to-faction by construction and faction-to-seat by lookup. `humanFaction` throws on anything that is not a real roster index, which makes its caller's error handling load-bearing, and this is the caller: damage arrives carrying factions that belong to nobody. `seatOf` returns nothing on a miss and never `humanFaction(seats.indexOf(x))`, the line that mints FACTION_AI from an indexOf miss and quietly puts a human on the NPC side. Two attribution holes are preserved rather than fixed, because fixing either means deciding a match rule. An unattributable kill — sear, a mine — pays seat 0, which is bit-for-bit today's behaviour with one seat and arbitrary with more. And a bolt landing on another participant does damage and pays nothing, since hits and bounties are credited by the enemy ship's own callbacks. Both are milestone 8's, both are named in the code, and the second has a check pinning it so closing it has to be deliberate. Evidence. 294 simcheck + 41 balance, exit 0, and the recorded baseline still matches on all three airframes — the load-bearing measurement here, since it is the net this milestone was warned about needing. 68 new assertions across two seats, per-seat scoring, intent routing, respawn-vs-elimination and presentation independence. Bundle 674.89 kB / 179.21 kB gzipped against 671.10 / 177.96 on origin/main, both built in the same tree. Mutation-tested: 20 deliberate breakages, 19 caught by a named assertion. The crossed checks are what earn their keep — flying every seat on `intents[0]` passes "both hulls moved" and "both hulls fired" without complaint, so the discriminator is swapping the two intent streams and demanding the outcomes swap with them. Pointing the squadron at the drawn seat survived until `position` was part of the fingerprint, because two runs agree on hull, score and speed while flying completely different fights. The one survivor is recorded rather than papered over: restoring the win-bonus mutation leaves the whole suite green, and that is provable rather than assumed — `sealResult(true)` is reached from one call site and `finish` clears the arena before returning, so nothing can observe the write until milestone 8 makes a win stop ending the match. The note lives at `sealResult`. One trap accounted for three of the four bugs found in the new checks themselves, and each time the tell was the assertion count rather than an error. `finish` calls `clearArena`, so the tick that resolves a match is the tick `snapshot` starts returning null: one check read a scoreline afterwards and took the suite down with a TypeError at ok=247 of 280, summary never printed; one recorded a resolution tick the null check had already broken out of; and one turned a mutant that had been caught by a named assertion into a mutant caught only by a crash — `exit=1 ok=232 FAIL=0`, which reads exactly like a pass if you only grep for FAIL. All three now sample per tick and keep the last live sample, and the comparisons that could have run on two empty strings have a length floor under them.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Review of `7d134c9` returned six findings, four P1, against 294 passing checks. All six reproduced. Fixed, and each now has an assertion that fails on the bug it names. **An eliminated seat re-entered its own death sequence.** `wreck: Wreck | null` used `null` for two different states — "flying" and "cutscene over, staying dead" — so the tick's rule "dead hull with no wreck starts its cutscene" matched an eliminated seat again on the very next tick. Reproduced at `deaths=3` and climbing every 2.4 seconds, re-sealing the local result each time. Now a three-shape `SeatPhase`: flying, wrecked, eliminated. Unrepresentable rather than guarded against. **A finishing match erased another seat's wreck.** `finish` fired as soon as nobody was left *flying*, which a seat mid-explosion satisfies, and `clearArena` took the second wreck with it 85 ticks into its 144. The match now waits until every seat is eliminated. A win is gated the same way, because the squadron can empty on the very tick somebody dies — before the tick loops were merged that was unreachable rather than handled. **The squadron was a function of who was watching.** `otherShips(localSpec.id)` made the enemy hulls depend on the drawn seat: same roster, same seed, same intents, a different fight per machine. Keyed on seat 0 instead — arbitrary in the same way `bountyGoesTo`'s fallback is arbitrary, and roster-stable, which is the part that matters. Milestone 9 owns what actually fills an arena. **Pause asked the drawn seat.** `paused` stops the whole simulation, so the same match in the same state could be frozen from one machine and not another. Asked of the roster now. **Two calls mutated before validating.** `step` advanced the environment before checking its argument count, so a refused tick still moved the world clock and a caller that retried advanced it twice. `start` tore the arena down before finding out whether the setup was buildable, leaving `active` true with no seats — a blank zombie in place of the match the caller already had. Both validate first. **`local: NaN` threw instead of clamping**, under a comment that said "clamped, not fatal". `Math.min`/`Math.max` propagate `NaN`; same trap and same fix as `clamp` in `ship.ts`, and deliberately not `Number.isFinite` — `Infinity` is a clampable request for the last seat. On method, because the code was the easy half. **A universal property checked at one sample is a coin toss, and this is the third time in this repo.** The presentation-independence check existed, compared the right things, and ran on one seed — the single masking case out of seven. PR #17 recorded this shape twice already. Contiguous ranges, not samples, and that now applies to seeds as well as indices: the check runs eight. **An outcome fingerprint is blind to trajectory.** Two runs agree on hull, score, kills and speed while flying completely different fights, because a hull nobody has hit reads the same wherever it is. Position had to be in the fingerprint before the squadron mutation could be seen at all. **A check that walks the right state can still look at the wrong thing.** The eliminated-seat restart happened inside a check that exercised exactly that state. It asserted the survivor was still flying and that no result had been reported, and never looked at the corpse. **Evidence a reviewer cannot replay is weak evidence** — the same criticism as milestone 2, and fair. Twenty mutation runs were reported from a terminal and deleted. Now `npm run check:mutants` (`scripts/mutate.mjs`), checked in, with a CI job that runs it plus `npm run check` and the build; there was no CI at all before. The harness needed hardening before it could carry that weight, and two of its holes were the trap it exists to catch. Pointed at a name matching no entry, it printed "Every mutation was caught by a named assertion" and exited 0 having run nothing — a green verdict over an empty set, inside the guard against green verdicts over empty sets. And every verdict in it is "did the suite report a failure", which means nothing unless the suite reports none to begin with: against an already-red baseline it would have claimed 29 catches while proving zero. Both now refuse to pass, and both refusals were verified by construction rather than by reading the code — a filter matching nothing, and a deliberately broken assertion committed and run. It also has four verdicts rather than two, since a mutant that names its assertion and *then* aborts has hidden every check after it; that caught one case in my own new tests and one harness entry that had silently stopped applying. **The shipped frame loop keeps calling into a finished match, and nothing tested it.** `main.ts` skips the simulation only on the hangar screen — on pause and on debrief it still steps and renders every frame with its one-slot intent array, against a roster `clearArena` has already emptied. Had the new length check been unconditional, the game would have thrown on the first frame of every debrief: a crash on the most-travelled screen in the game, behind a fully green suite. Now asserted, with the mutation to prove the assertion bites. Coverage added on top of the six: a seat respawning while the survivor keeps flying; a respawn point replaying from its seed and differing on another; two scorers with separate streaks, hits and accuracy; the whole win-over-wreck state, constructed rather than waited for. Evidence at this head, clean tree: **357 simcheck + 41 balance, 0 FAIL, exit 0**, typecheck and build clean, and the recorded baseline unchanged on all three airframes. `npm run check:mutants`: control run green, then **29 mutations, 29 caught by a named assertion, 0 survived, 0 caught only by a crash, 0 unapplied.** Still outstanding and not closed here: no browser pass on the changed camera/HUD/audio/wreck presentation — the loop test above is the nearest a headless run gets, and it is not the same thing. And the win-bonus purity has no mutation entry, because restoring `seat.score += bonus` is provably unobservable: `sealResult(true)` is reached from one call site and `finish` clears the arena before returning. It becomes observable at milestone 8, and the harness says so rather than omitting it quietly.
…uite ran
Two P1s from the re-review of `700912a`. Both reproduced first.
**The pause overlay could sit in front of a running match.** `src/main.ts` gated
the pause *screen* on `game.dying` and then called `pause()` — two answers to one
question. `dying` is the drawn seat's explosion; `pause()` refuses while *any*
seat is exploding. With a remote participant wrecked and the local hull still
flying they disagreed: the panel went up, the pause was refused, and the frame
loop kept stepping combat behind it. Measured on the same state, before and after:
old gate (dying): panel shown = true simulation frozen = false
new gate (pause()): panel shown = false simulation frozen = false
local seat travelled 140.5 units in the following second
`Game.pause()` now returns whether it actually paused, and `main.ts` acts on the
answer rather than predicting it. Single-seat behaviour is unchanged: with one
seat, "the drawn seat is exploding" and "some seat is exploding" are the same
sentence, which is exactly why this was invisible until there were two.
The property is asserted where it can be — `main.ts` is not reachable from a
headless run, so the check pins what made it wrong: **`dying` is not the pause
condition**, the return value is, and the two are demonstrably different in a
two-seat state. A caller that reaches for `dying` again fails here.
**The mutation control passed a partial suite.** It proved the suite was green and
non-empty, which is not the same as proving it *ran*. A copy of this head with one
nine-assertion invocation deleted reported `exit=0 ok=350 FAIL=0`, sailed through
the control, had all its mutants reported caught, and exited 0. Green-and-short is
the dangerous shape precisely because every verdict in the harness is "did the
suite report a failure", and a suite missing a whole test reports none.
The control now requires the expected assertion count and the suite's own summary
line. `EXPECTED_ASSERTIONS` is pinned and deliberately brittle, for the same
reason the recorded baseline is, and its failure message says what to do.
**And the guard is exercised on every run rather than by hand once.** Before it
does anything else, the harness deletes one real test invocation, confirms the
control refuses the result, and puts it back:
self-test (one test invocation removed): exit=0 ok=360 FAIL=0
-> it ran 360 assertions where 367 were expected — something stopped running
(green, short, and correctly refused — the guard bites)
It also refuses to run at all if that anchor is missing, or if removing it happens
to make the suite *fail* — because then the self-test would be proving something
about a red suite rather than about a green-but-short one. A guard nobody has
watched reject anything is documentation.
**One pre-existing bug fixed alongside, and it is separable.** `environment.step()`
ran before the not-live return, so a paused game kept advancing its pod respawn
clocks — a player who paused for a minute came back to a re-armed arena. Byte for
byte the same on the base commit, so not this PR's regression and not charged to
it by the review. Fixed here because it is the line the roster work moved, and
leaving a known-wrong neighbour untouched is how it gets inherited again. Say the
word and it comes out.
Evidence at this head, clean tree: **367 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: self-test refuses a short suite, control green at exactly
367, then **31 mutations, 31 caught by a named assertion, 0 survived, 0 caught
only by a crash, 0 unapplied** — including two new ones for the fixes above, which
name `pause refuses anyway, and says so` and `advances not at all while it is
paused`.
Still open and still not mine to close: the browser pass on the changed
camera/HUD/audio/wreck presentation.
One P1 from the re-review of `cad2275`, and it is the sharpest one in this thread:
the production code was correct and the test protecting it was a claim rather than
a check. The pause repair lived in `src/main.ts`, inside `boot()`, which needs a
canvas and an overlay — so nothing headless could call it. Restoring the exact old
caller (show the panel, call `pause()`, ignore the answer) left **367 simulation +
41 balance green and all 31 mutants caught**. The regression was reintroducible
with every mandatory job passing.
Worse than the gap: my own comment on the check said it "cannot reach
`src/main.ts`" and then described the caller bug it was standing in for. A test
that certifies a callee and *claims* to protect its caller is worse than no test,
because the claim is what stops anybody writing the real one. That is the same
failure as milestone 2's comment certifying a decoupling that had not happened,
and this time I wrote the certification myself while fixing the thing it was
about.
`src/ui/pause-flow.ts` now owns the decision, `main.ts` calls it, and the suite
runs it. The shape of the return is the part worth keeping: `enter()` hands back
**the screen you are now on**, not a boolean saying whether it worked. A boolean is
a thing a caller can ignore — which was precisely the bug — and there is no way to
ignore a value you have to assign. `main.ts` is now `screen = pauseFlow.enter()`.
Tested at both boundaries. Against a recording host, both answers: an accepted
pause reaches the pause screen and raises the panel; a refused one stays in flight
and does **nothing at all** — asserted on the recorded call log, not just on the
return value, because "right screen, panel up anyway" is the same bug wearing a
correct answer. And against a real `Game` in the state the shipped UI cannot
construct today — a remote seat mid-explosion with the local hull flying — the
transition is refused, no panel goes up, and the simulation is not frozen. Then
the mirror, so refusal is not the only outcome the flow can produce: once every
explosion has finished, the same transition succeeds.
Three caller-boundary mutations, all caught by named assertions:
the pause transition ignores a refused pause FAIL=4
- a refused pause stays in flight — paused
- and does nothing at all — pause->false,showPanel
- the transition is refused by the real game — paused
- no panel went up over it — pause->false,showPanel
the pause transition shows the panel before asking FAIL=4
leaving restarts the sim before hiding the panel FAIL=1
Evidence at this head, clean tree: **379 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: self-test refuses a short suite, control green at exactly
379, then **34 mutations, 34 caught by a named assertion, 0 survived, 0 caught
only by a crash, 0 unapplied.**
Still open, unchanged and still not closable from here: the browser pass on camera,
HUD, audio and wreck feel, and real `pointerlockchange` ordering.
One P1 from the re-review of `54e4aff`, and it lands on a claim I made rather than
only on code. I wrote that a caller "cannot ignore a value it has to assign".
**That is false.** `pauseFlow.enter()` as a bare statement is valid TypeScript that
discards the result, after which the panel goes up, `screen` stays `'flight'`, and
Resume refuses because it is not on the pause screen — the player is sealed behind
the overlay. Reproduced: `npx tsc` clean, `npm run check` all green.
That is the third round on one transition, and the shape repeated each time. Every
fix moved the tested boundary one layer inward and left the shipped caller outside
it:
1. `main.ts` kept its own copy of the pause condition (`game.dying`) and disagreed
with `Game.pause()`. Overlay over a running match — 140 units of travel in the
following second.
2. `Game.pause()` returned its answer and `main.ts` honoured it, but nothing
headless can call `boot()`, so restoring the old caller left 367 checks and 31
mutants green.
3. The decision moved into a DOM-free flow that returned the new screen and left
`main.ts` to assign it. See above.
The generalisation, now in `PLANS/NEON_ORBIT_PHASE_B.md`: **a seam that hands a
decision back to untested code has not moved the decision.** Returning an answer is
not the same as taking responsibility for it, and every "the caller cannot get this
wrong" argument I made was an argument about taste rather than about reachability.
`src/ui/screens.ts` now owns the screen variable, in a holder `main.ts` shares by
reference — a holder rather than a setter callback, because a callback is one more
line of untested adapter that can be wired to nothing. `enter` and `exit` return
`void`, write `state.screen` themselves, and own their own preconditions, so the
`screen !== 'flight'` test that used to sit at the call site cannot drift out of
step with them. `main.ts` has no pause decision left: Escape and P are
`pauseFlow.toggle()`, pointer-lock loss is `pauseFlow.enter()`, Resume is
`pauseFlow.exit()`, and what remains is five one-line adapters with no branches.
The check that closes it asserts recoverability rather than return values, driven
through a model of the shipped key handler: press Escape, press it again, and be
back in flight — then ten presses, because a toggle that works once is still a
trap. Both screen writes are mutated, and the round-trip assertion is what names
them:
entering the pause screen never writes the screen FAIL=6
- one press of Escape pauses — flight
- Escape keeps working, press after press — flight,flight,flight,...
leaving the pause screen never writes the screen FAIL=4
- a second press gets the player back out again
— stuck on paused — the panel is up and nothing will take it down
Plus both preconditions, the toggle direction, the panel/ask ordering, and the
original refusal case: eight mutations on this one module, and the live half of the
test drives a real `Game` through the same flow in the state the shipped UI cannot
construct — a remote seat mid-explosion with the local hull flying.
One more found by the test itself while writing it: the live rig wired `pause` to
the real game and left `resume` as a log entry, then asserted the real game had
resumed. It failed, correctly. A double whose halves reach different places is a
check on nothing in particular.
Evidence at this head, clean tree: **389 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: self-test refuses a short suite, control green at exactly
389, then **39 mutations, 39 caught by a named assertion, 0 survived, 0 caught only
by a crash, 0 unapplied.**
Still open and still not closable from here: the browser pass on camera, HUD, audio
and wreck feel, and real `pointerlockchange` ordering.
Two findings from the fifth review round on `4488bad`.
**The dev hook was returning the browser's `Screen` object.** Moving the screen
state out of `main.ts` left `return screen` in `window.__neon.screen`, which
compiles against the DOM global — so the documented hook reported neither `hangar`,
`flight`, `paused` nor `debrief`. My own regex rewrote the assignments and the
comparisons and never touched the bare read. Fixed; the four documented values are
now asserted reachable and reported by name, which is the check whose absence let a
type-correct nonsense read through.
**And the fourth round on the pause transition.** `createPauseFlow({ ...state }, host)`
is valid TypeScript that hands over a *copy*: the app launches, the flow still sees
`hangar`, Escape does nothing, and the whole 389 + 41 suite plus 39 mutations stay
green. Reproduced.
That is four rounds on one transition, and the mechanism was different every time —
a boolean, a return value, an object reference — while the shape never changed:
1. main.ts held its own pause condition (game.dying) overlay over a live match
2. pause() returned its answer for main.ts to honour boot() unreachable
3. the flow returned the screen for main.ts to assign a bare call discards it
4. the flow wrote a holder main.ts passed in a spread copy detaches it
Each fix moved the tested boundary one layer inward and left the *last step* of the
decision in code no test could reach. So this one removes the step rather than
relocating it: `createScreens` owns the variable outright. Nothing to hand it,
nothing to assign back, no identity to get wrong. `main.ts` reads `screens.screen`
and calls `moveTo` / `enterPause` / `exitPause` / `togglePause`.
`moveTo` cannot reach `'paused'` — excluded in the type and refused at runtime for
the bundled JavaScript. Only `enterPause` may raise the overlay, which is what makes
the trap those four rounds kept producing *unrepresentable* from outside rather than
merely absent today.
Eleven mutations on this module now, one per round plus the surface the rewrite
added. The round-4 equivalent — there being no holder left to copy — is a getter
answering from a snapshot taken at construction, and it fails on the property that
regression actually broke:
the screen is reported from a stale copy FAIL=8
- the app can launch — hangar
- and pausing sees the screen the app moved to
— hangar — the transitions and the reads disagree
the app can move itself onto the pause screen FAIL=2
moving to another screen does not change the screen FAIL=4
**What is still not covered, said plainly rather than claimed closed.** `main.ts`
still chooses which transition to call, and its five host adapters are one-line
lambdas; nothing headless executes either, because nothing headless can call
`boot()`. The residue is now "did the app call the right method" rather than "did
the app correctly finish a decision the module started" — a smaller and duller
class, but not an empty one. Closing it means executing `boot()`, which needs a
browser and a real dependency rather than another refactor. Recorded in
`PLANS/NEON_ORBIT_PHASE_B.md` instead of asserted away.
Evidence at this head, clean tree: **396 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: self-test refuses a short suite, control green at exactly
396, then **42 mutations, 42 caught by a named assertion, 0 survived, 0 caught only
by a crash, 0 unapplied.**
The one P2 from the sixth review round on `dce4d56`, and it is the same mistake as
the pause rounds in the smallest possible form: the check written to protect the dev
hook toured a local `createScreens` rig and never touched `window.__neon`. Restoring
the exact stale read — `return screen`, which compiles against the DOM global — left
TypeScript, 396 simulation checks, 41 balance checks, 42 mutations and the build
entirely green. Reproduced before fixing.
A check that certifies the *source* and claims the *reader* is the pattern this whole
review has been about, and I wrote another one while closing the last.
`src/core/dev-hook.ts` builds and installs the hook, so a headless run can execute
both. The design point beyond the ask: `screens` and `game` are passed **whole**
rather than as `() => screens.screen` lambdas, so the field that regressed has no
adapter left to aim at the wrong thing. The only function handed over is `start`,
because it is a command rather than a reading.
Tested through an installed hook on a stand-in global — the property descriptor, the
getters, and the objects behind them:
- all four documented screens, by name, as the app moves through them
- and asserted to be *strings*, because the bug returned a browser `Screen` object
- the run view arrives with the match and keeps up with it
- the input view is live, and writing to what the console is shown does not reach the
device — `__neon` is a window, not a cheat
- the launch command reaches the app
- and the hook can be replaced, which is what `configurable` is for: without it the
second install of a dev session throws and the console keeps reporting a game that
no longer exists
Four mutations on it, each named by the assertion it breaks:
the dev hook reports the screen it was built with FAIL=1
- it reports all four documented screens — hangar,hangar,hangar,hangar
the dev hook reports the run it was built with FAIL=2
the dev hook hands over the live input struct FAIL=1
- writing to it does not reach the device — device pitch=-1
the dev hook cannot be reinstalled FAIL=1
Evidence at this head, clean tree: **405 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: self-test refuses a short suite, control green at exactly
405, then **46 mutations, 46 caught by a named assertion, 0 survived, 0 caught only
by a crash, 0 unapplied.**
Unchanged and still named rather than claimed closed: `boot()` and its host adapters
have no browser execution path, so "did the app call the right method" is not covered
by anything. A browser smoke harness would close that class for every later milestone
and is worth its own piece of work rather than this PR's.
Two P1s from the seventh review round on `c9ae0ff`. The first is in the shipped
game — the first production defect found since `700912a`, and it corrects something
I said last round about the findings all being in scaffolding.
**A teammate's win was overwriting the drawn seat's sealed loss.** In a two-seat
match without respawn the drawn seat can be eliminated — sealing its loss — and a
surviving participant can then clear the squadron. `finish(sealResult(true))`
reported `won: true` for a participant who had been dead for seconds, and computed
the win bonuses off a hull sitting at zero. Their run ended at their death; a
teammate finishing the job afterwards is not their victory. Now
`finish(pendingResult ?? sealResult(true))`, and the SECTOR CLEAR callout is
suppressed for a player being handed a loss.
Nothing changes for the single-seat game: `pendingResult` is only set by the local
seat's death, and the only way to reach that branch with one seat is to still be
alive.
**Why it went untested is the more useful half.** The existing win-over-wreck rig
walks exactly this state and *deliberately draws the survivor* — its comment says
so, to keep the reported result a clean win rather than a loss sealed earlier.
Choosing the convenient viewpoint is how the other one goes unplayed. The mirror
now runs the same match, same seed, same mine, from both seats, and asserts they
report different results. The score assertion is an equality against the scoreline
at death rather than a threshold: a bound was the first attempt and was a bad proxy,
because this seat legitimately earns 1813 points from the documented
unattributable-kill fallback. A companion check confirms its score keeps climbing
after death, so the seal is demonstrably doing work.
**And the mutation harness was certifying incomplete runs.** The assertion-count
guard applied only to the green control, never to each mutant. The full run at the
previous head contained its own counterexample: the "respawn fires on the frame of
death" mutant reported 365 ok + 35 FAIL = 400 of 405, reached its summary, and was
classified `CAUGHT`. Five assertions never ran and the job exited 0.
Every mutant now has to run the expected total. That exposed the real problem
underneath, which was in the suite rather than the harness: **its assertion count
was a function of behaviour.** Three blocks skipped their checks when a precondition
failed — the wreck-render section behind `if (reachedDying)`, my own two-seat pause
block, and the dev-hook test's early return. Allowlisting the shortfall was the easy
answer; making the count constant is the right one, and there was a version that is
both complete and non-vacuous: measure to `null`, then let every check fail by name
with "no cutscene was reached". Six named failures instead of one, and 414 either
way. All 47 mutants now run exactly 414 of 414.
The harness has a second self-test for it, because the first only ever exercised the
control rule. It combines a real mutation with a removed test invocation to produce a
run that is *failing and short at once* — the exact shape that slipped through — and
refuses to proceed unless the verdict rule rejects it:
self-test (one test invocation removed): ok=407 FAIL=0 -> refused (green, short)
self-test (failing *and* short): ok=401 FAIL=6 (407/414 ran) -> refused
control (no mutation): ok=414 FAIL=0 -> accepted
Evidence at this head, clean tree: **414 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: both self-tests bite, control green at exactly 414, then
**47 mutations, 47 caught by a named assertion, every one running 414 of 414, 0
survived, 0 caught only by a crash, 0 unapplied.**
Unchanged: `boot()` and its host adapters still have no browser execution path.
Two P1s from the eighth review round on `71169ee`, both in the mutation harness,
and both reproduced before fixing.
**The verdict self-test never invoked the verdict.** It computed its own `complete`
boolean while real mutants used a separate `finished` expression in the loop. So
deleting the assertion-count clause from the *production* rule left the self-test
still printing "the verdict bites", all 47 mutants reported caught, and the job
green — a regression guard passing while the thing it guards was gone. **A rule with
two implementations is a rule with one untested one**, which is the same shape as
every earlier round in this review: a check that certifies its own copy of the
decision.
There is now one `classify(result)` and the loop and all three self-tests call it.
Each self-test states the verdict it expects and refuses if `classify` disagrees.
**And `CAUGHT` ignored the exit code.** With `fail > 0` the classifier consulted only
completeness, so a suite printing failures while exiting 0 certified every mutant as
caught. Reproduced by changing the simulation footer to `process.exit(0)`: 47 caught,
job green, and `npm run check:sim` would have told CI it passed. That is not a caught
mutation, it is a broken gate — the mutation is beside the point, because nothing the
run says can be trusted. It has its own verdict now, and only `CAUGHT` is a pass.
The three self-tests are ordered by dependency rather than by narrative, which the
first attempt got wrong: with the exit contract broken, the completeness fixture also
exits zero, so running it first reported a broken gate as a completeness problem.
The exit contract comes first because if the suite cannot tell its caller it failed,
no verdict about anything else means anything.
self-test 1 — loud, exits zero: exit=0 FAIL=6 (414/414) -> BROKEN-GATE
self-test 2 — green but short: exit=0 FAIL=0 -> ran 407 of 414
self-test 3 — loud but short: exit=1 FAIL=6 (407/414) -> CAUGHT-THEN-ABORTED
(all three refused by the functions the loop itself uses)
Each fixture also has to *be* what it claims: a self-test whose patch silently missed,
or whose fixture failed to fail, refuses the whole run rather than passing vacuously.
That fires usefully — with the exit contract already altered, the zero-exit fixture
cannot be constructed at all, and the harness says so instead of proceeding.
Three probes replayed against the result: stripping the count clause from `classify`
is refused by name, a zero-exit suite is refused by name, and deleting the
`BROKEN-GATE` branch is refused by name.
Evidence at this head, clean tree: **414 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: all three self-tests refused through the production
deciders, control green at exactly 414, then **47 mutations, 47 caught cleanly, every
one running 414 of 414, 0 not-cleanly-caught, 0 survived, 0 unapplied.**
Unchanged: `boot()` and its host adapters still have no browser execution path.
Two findings from the ninth review round on `36e925d`, both in the mutation harness.
**A signal-killed suite could be certified as caught.** `spawnSync().status` is
`null` when the child is killed, and `null !== 0`, so avoiding "clean exit" was not
the same as handling "never exited": every non-zero status was treated alike, and a
run that had printed a complete failure summary before being killed read as `CAUGHT`.
A signal is not a verdict — the suite did not decide anything, something else stopped
it. `KILLED` now, non-numeric status rejected by the classifier and by the control
guard, and `r.signal` retained because with `status === null` it is the only thing
that says what happened.
**And the self-tests all stopped at `classify`.** The loop was free to ignore its
answer: returning `'CAUGHT'` for any failing run kept all four self-tests green, had
47 mutants reported caught, and exited 0. So the verdict-to-exit path is now one
`finalVerdict`, and the fixtures run **as child processes** — `node
scripts/mutate.mjs --fixture=N` — with the parent asserting the real command's real
exit status. Loop, `classify`, counters, exit: nothing left between the verdict and
the number CI reads.
The first version of those fixtures did not close it, and the gap is the finding in
miniature. A survivor, an unapplied patch and a clean catch all passed under a loop
that hard-coded `'CAUGHT'` for failures — because **none of them was a failing run
that should have been judged something else.** The missing fixture is loud and short
at once: a real mutation plus a removed test invocation, which must be
`CAUGHT-THEN-ABORTED` and must fail the job. It is what catches both the loop
ignoring `classify` and `finalVerdict` ignoring `notClean`; both probed, both refused
by name.
Five self-tests now, ordered by dependency:
1 — loud, exits zero exit=0 FAIL=6 (414/414) -> BROKEN-GATE
2 — green but short exit=0 FAIL=0 -> ran 407 of 414
3 — loud but short exit=1 FAIL=6 (407/414) -> CAUGHT-THEN-ABORTED
4 — every exit status 7 exact spawnSync shapes through classify()
5 — the job's own exit 4 fixtures as child runs, 3 must fail it, 1 must pass
Self-test 4 answers the null-status finding with exact inputs rather than a
stand-in — a `spawnSync` result is a handful of fields, so the killed, zero-exit,
short, crashed and clean shapes can all be constructed and put through the production
classifier.
Evidence at this head, clean tree: **414 simcheck + 41 balance, 0 FAIL, exit 0**,
typecheck and build clean, recorded baseline unchanged on all three airframes.
`npm run check:mutants`: five self-tests refused through the production deciders,
control green at exactly 414, then **47 mutations, 47 caught cleanly, every one
running 414 of 414, 0 not-cleanly-caught, 0 survived, 0 unapplied.**
Unchanged: `boot()` and its host adapters still have no browser execution path.
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
1 participant
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.
Milestone 3 of
PLANS/NEON_ORBIT_PHASE_B.md.playeris gone fromgame.ts:Game.starttakes aMatchSetupwith one hull per seat,Game.steptakes oneControlsper seat, and a ship is flown by whatever intent was supplied for it. Single-player is a match of one.226 + 41 → 294 + 41, exit 0. The recorded baseline still matches on all three airframes, which is the measurement that matters here — it is the net BOLTy asked for at milestone 2, doing its job under the milestone it was built for.
The dividing line
A seat decides outcomes;
localonly decides what is drawn. Camera, HUD, alarms and gun pitch readlocal; nothing that reads it may reach a hull, a score or a result, or two machines watching one match would disagree about what happened in it.Writing the check for that found two leaks, both invisible with one seat:
sealResultnow reads state and writes none.Neither is subtle code. The lesson is that "presentation only" stays true by being tested, not by being labelled.
Death is per-seat, and respawn is a flag
The old
stepDeathSequencereturned early from the tick and ran a second copy of the arena loop inside the cutscene. That works for exactly one dying participant and cannot be made to work for two, so there is one arena loop now and a wreck is just a hull that happens to be dead. The alarms that the early return used to silence by accident are now gated on the drawn seat being alive on purpose.Respawn defaults to off, which is the one place this departs from the plan. The plan had milestone 3 replacing run-end with respawn outright — but single-player is a match with one seat, and a shipped game where dying neither ends the run nor reaches the debrief has no lose condition at all. One code path; the flag only decides what a finished cutscene hands over to. Asserted in both directions from the same seed, the same intents and the same fatal mine, so the flag is demonstrably the only difference.
The guard at the caller
humanFactionthrows, which makes its caller's error handling load-bearing — and the roster is the caller. Damage arrives carrying factions that belong to nobody.seatOfresolves faction → seat by lookup and returns nothing on a miss; neverhumanFaction(seats.indexOf(x)), the line that mintsFACTION_AIfrom anindexOfmiss and quietly puts a human on the NPC side.Two holes preserved on purpose
Both are milestone 8's, both named in the code:
Mutation results
20 deliberate breakages, 19 caught by a named assertion, none caught only by a crash.
The crossed checks earn their keep. Flying every seat on
intents[0]passes "both hulls moved" and "both hulls fired" without complaint, so the discriminator is swapping the two intent streams and demanding the outcomes swap with them. Pointing the squadron at the drawn seat survived untilpositionwas part of the fingerprint — two runs agree on hull, score and speed while flying completely different fights.The one survivor is recorded rather than papered over. Restoring
seat.score += bonusleaves all 294 green, and that is provable rather than assumed:sealResult(true)is reached from exactly one call site, andfinishcallsclearArenabefore returning, so nothing can observe the write. It becomes observable at milestone 8, where a win stops ending the match. The note lives atsealResultinstead of in a check that would assert nothing.Three bugs in my own new checks, one trap
finishcallsclearArena, so the tick that resolves a match is the ticksnapshotstarts returning null. That accounted for three of the four bugs I found in the new checks, and every time the tell was the assertion count rather than an error:TypeError, ok=247 of 280, summary never printedexit=1 ok=232 FAIL=0— which reads exactly like a pass if you grep for FAILAll three now sample per tick and keep the last live sample, and the comparisons that could have run on two empty strings have a length floor under them.
Verification
At
7d134c9, clean tree:npm run check— 294 simcheck + 41 balance, 0 FAIL, exit 0npm run build— clean; 674.89 kB / 179.21 kB gzipped, against 671.10 / 177.96 onorigin/mainbuilt in the same treeWhat a human still has to look at
Nothing in the shipped game should play differently — one seat, elimination, seat 0 at
PLAYER_SPAWN. The regression pass is the same six items as #17 (own-guns pitch, enemy friendly fire, score attribution, mine bounty, station scrape, sun burn), plus: death still ends the run and still reaches the debrief after the full cutscene. That last one is the whole reason respawn is a flag.🤖 Generated with Claude Code