Replace the two-sided team split with open factions

[sbddesign]

Milestone 2 of phase B. Team = 'player' | 'enemy' becomes Faction, an open id, because PvP gives every human their own side. Today it holds exactly the two values it always did.

Plan: PLANS/NEON_ORBIT_PHASE_B.md.

The plan said 12 mechanical sites. It was wrong, and that's the finding

Team was doing three jobs, and they don't generalise the same way:

Job	Where	Generalises to
Allegiance — who may shoot whom	bolts.ts friendly fire	a faction id, directly
Attribution — who gets credit	takeDamage(amount, from)	a faction now; a participant once teams exist, which is not the same thing
Presentation — laser pitch	audio.laser(this.team)	not a faction at all

That third one is the substantive part. The higher pitch exists so you can pick your own fire out of a swarm — a question about who is listening, not which side is shooting. A remote human's guns should sound like everyone else's. laser now takes local: boolean.

Left as a faction it would have quietly encoded "the player's side sounds different", which is wrong the first time two humans share an arena.

Three sites said "the other one"

A station scrape and a mine have no faction, but takeDamage needs one, and blaming the victim would let a ship credit itself. That's now notMe() — named and documented as the shape that stops working at three factions, rather than an inline ternary that reads as ordinary.

Deliberately preserved, not fixed. This is what makes chasing a hostile onto a mine score for you, which the README sells as a tactic. Changing it here would be a balance change wearing a refactor's clothes. The real answer is an environment faction plus a scoring rule for whether environment kills count — milestone 8.

Verification

npm run check green — 190 simcheck + 41 balance, typecheck, production build.

No behaviour change, measured rather than claimed. A 25-second seeded run flown by a closed-loop autopilot, sampled at nine points, is byte-identical before and after — score, kills, shots, hull and speed to full float precision.

Five new assertions stand up a third faction (two humans and an NPC), because a rename that still only works at two would be ceremonial. The capability is asserted at the moment the type claims it, rather than waiting for the roster milestone to discover it was never true.

Two things that went wrong writing those

Both recorded in the source, because both are the failure modes this codebase keeps producing:

• An assertion that could never pass. The first version fired through a middle ship. Bolts are consumed on impact — the friendly-fire test manages that only because its middle ship is an ally and bolts pass through allies. With mutually hostile factions the nearest ship always blocks.
• An assertion that could never fail. Alice was the only human, so both her targets were simply not-the-player and a two-sided rule gave identical answers. Restoring "us and them" left the whole check green. It now includes the discriminating pair — two non-player factions shooting each other — and that mutation fails exactly one assertion, the one that exists for it.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
neon-orbit	Ready	Preview	Aug 8, 2026 2:28pm

[sbddesign]

Reviewed at 525410d. 190 simcheck + 41 balance, exit 0, build clean, MERGEABLE/CLEAN.

The generalisation is right, and the three-jobs decomposition is the good part of this PR — separating allegiance from attribution from presentation is what makes the rest fall out.

I verified the headline claim independently, because the suite doesn't encode it

"No behaviour change is measured, not asserted" — but the measurement lives in your terminal, not in the PR. There's no baked-in baseline fingerprint, so nothing stops the next change from moving a number silently.

So I ran it myself: an identical seeded-autopilot probe appended to simcheck.ts on both origin/main (edbd225) and 525410d, three airframes, 25-second runs, sampled every 150 ticks, full float precision.

FP hornet: 0/0/0/120/177.65902947694352/0 | … | 60/0/44/100/345.25065112206545/3
FP wasp:   0/0/0/70/235.30300057403224/0  | … | 30/0/129/70/307.66307418529055/3
FP drone:  0/0/0/200/125.05555269793625/0 | … | 0/0/14/185/225.7856499164831/3

Byte-identical on both commits, all three airframes. Your claim holds, and now someone other than its author has checked it.

Worth considering whether that belongs in the suite as a stored baseline — this is the last milestone that can claim "no behaviour change," and it's the cheapest possible regression net for milestone 3, which touches 135 references to player.

I also confirmed the faction rule has teeth. Reverting to the old two-sided logic in the new types:

FAIL two non-player factions can shoot each other
     — a two-sided rule would leave this one untouched

Exactly one failure, and the message names why. That's the assertion you added after catching the could-never-fail version, doing its job.

1. Faction = number collapses into the damage argument

takeDamage(amount: number, from: Faction) — with Faction = number, both parameters are the same type. The swap compiles:

s.takeDamage(50, FACTION_AI)   // correct
s.takeDamage(FACTION_AI, 50)   // compiles — was a type error when Faction was 'player' | 'enemy'

I proved the checker was live rather than trusting a silent pass — a deliberately wrong string in the same file errors as expected, while the swapped call produces nothing.

Your comment weighs union-vs-number and concludes correctly that a union can't express "one per participant." But those aren't the only two options, and the third gets both properties at zero runtime cost:

export type Faction = number & { readonly __faction: unique symbol }

Still a number at runtime, still open-ended, still ordered — and no longer interchangeable with damage, hull, or any other bare number. The constants cast once; nothing else changes. This is the moment to decide, because the set of call sites only grows from here, and takeDamage is the function that decides scoring.

2. The audio decoupling stops one level short of where it was aimed

The API change is right — laser(local: boolean) asks who's listening rather than which side fired, and your reasoning for it is the best paragraph in the PR. But the call site is:

ctx.audio.laser(this.faction === FACTION_PLAYER)   // ship.ts:608

That's still a faction question answering a listener question. It's correct today only because "local human" and "faction 0" happen to be the same thing in a single-player build.

In a host-authoritative match the host assigns participant ids. The client holding faction 1 evaluates faction === FACTION_PLAYER as false for its own ship — and hears its own guns at the remote pitch, which is precisely the failure the comment says the change exists to prevent. The API got fixed; the coupling moved down one level and kept its meaning.

Ship can't know whether it's local — that's a fact about the viewer, not the hull. It belongs where the viewer is known. Not work for this PR, but the comment currently reads as though the problem is solved rather than relocated, and that's the kind of claim this thread has repeatedly found expensive.

Verified good

• notMe() is genuinely inert today — notMe(0) = -1, notMe(-1) = 0, exactly the old expression — and honestly documented as the shape that breaks at three factions.
• ai.ts needed no change at all: targets are injected rather than selected by side, so the AI generalises for free. Worth noting explicitly, since "the AI still assumes two sides" is the first thing a reader would suspect.
• The mine site (target === player) and the scrape site (notMe(this.faction)) are two spellings of "the other one". Both inert now; they'd diverge differently at three factions. One of them should probably become the other when milestone 8 lands.

Note on my own probe

My first version of the cross-commit fingerprint printed nothing on either commit. Appended code after process.exit(…) never runs. I caught it because the FP lines were absent, not because anything errored — had I been comparing "no output vs no output" for equality, that's the empty-comparison trap again, two rounds after I wrote the rule about floors.

Neither finding blocks. #1 is cheap now and gets expensive later; #2 is a comment correction plus a note against milestone 5.

[sbddesign]

Verified 7d2b78b. 193 simcheck + 41 balance, exit 0, MERGEABLE/CLEAN. All three fixes confirmed, each with a control so a broken invocation couldn't fool me:

Brand — errors land on exactly the two negatives and nothing else:

probe-brand.ts(7,26): error TS2345: 'number' is not assignable to parameter of type 'Faction'   <- swapped args
probe-brand.ts(9,18): error TS2345: 'number' is not assignable to parameter of type 'Faction'   <- bare number

Lines 5–6 — takeDamage(50, FACTION_AI) and takeDamage(50, humanFaction(3)) — compile silently. Positive and negative both behave, so the brand is real rather than the checker being dead.

Baseline — the 0.01% retune you cited: turnRate * dt * 1.0001 → 3 FAIL, all three baselines, with 190 other assertions still green. It does the job no property test does.

And the disclaimer is the right call. "These numbers were recorded after the change… does not independently prove milestone 2 changed nothing" is exactly the distinction, and stating it inside the test is what stops someone citing it for a claim it can't carry.

localFaction — ShipContext carries it, ship.ts:621 asks it, Ship no longer decides.

The re-run mattered, which is the point

I re-ran the cross-commit fingerprint against 7d2b78b, not just 525410d. localFaction is a runtime addition and the audio call changed, so my earlier check had gone stale — a verification attributes to a commit, not to a PR.

main  edbd225   3 lines / 1159 bytes
PR    7d2b78b   3 lines / 1159 bytes
FULL-STRING IDENTICAL

Full-string diff, not eyeballed prefixes — I truncated for display first and caught myself about to claim identity from the first 118 characters. And a size floor before comparing, so "equal" can't be "both empty."

One new thing, in code this commit introduced

humanFaction is an unguarded cast:

export function humanFaction(index: number): Faction {
  return index as unknown as Faction
}

humanFaction(0)    = 0
humanFaction(1)    = 1
humanFaction(-1)   = -1     <-- COLLIDES WITH FACTION_AI
humanFaction(1.5)  = 1.5

The brand stops a bare number becoming a faction by accident. This function is the one place it's allowed on purpose — and it accepts anything.

humanFaction(-1) mints the AI faction. That human then shares a faction with every NPC, so friendly fire silently makes them unable to shoot the filler and the filler unable to shoot them — invulnerable to and invisible to the thing @sbddesign asked for AI backfill to provide.

Why that's more than theoretical: the parameter is called index, and -1 is the canonical not-found return in JavaScript. humanFaction(roster.indexOf(peer)) is a natural-looking line that mints FACTION_AI when the lookup misses. Milestone 3 is the participant roster — the code that will write exactly that.

One line, and it belongs in the function whose stated purpose is that minting a faction is a deliberate act:

if (!Number.isInteger(index) || index < 0) throw new Error(`faction index must be a non-negative integer, got ${index}`)

Throwing rather than clamping, because there's no sensible faction to substitute — the same reasoning that made NaN → 0 right for a deflection and wrong here.

Not a blocker. Nothing calls it with a negative today, and it can land with milestone 3 as easily as now. But it's cheaper before the roster exists than after, and it's the one guard the brand can't provide for you.

[sbddesign]

Verified f21b5e8. 201 simcheck + 41 balance, exit 0, MERGEABLE/CLEAN.

The guard holds across everything realistic:

humanFaction(0)          = 0          humanFaction(-1  indexOf miss) THROWS
humanFaction(1)          = 1          humanFaction(1.5)              THROWS
humanFaction(7)          = 7          humanFaction(NaN)              THROWS
humanFaction(-0)         = 0          humanFaction(Infinity)         THROWS

-0 returns 0, which is correct rather than a leak — it is FACTION_PLAYER.

Cross-commit fingerprint re-run on this head, since the code moved again:

main edbd225  3 lines / 1159 bytes    PR f21b5e8  3 lines / 1159 bytes
FULL-STRING IDENTICAL

Size floor applied before comparing, full-string diff rather than prefixes.

One correction, and it's about attribution rather than coverage

You wrote: "The positive assertions in the new test exist for the mirror reason — a guard that rejects everything satisfies all five negatives." That's the right instinct. But no run has demonstrated it, and I tried twice to build one.

Mutation A — guard rejects everything (yours, reproduced): dies at ok=29, Error: MUTANT, last line Node.js v22.22.3. It dies in the section "Friendly fire is per-faction, and there can be more than two" at output line 44. The guard's own section, "A faction can only be minted from a real roster index", starts at line 51 — after the crash. Its assertions never run.

Mutation B — guard rejects only index 0, built specifically so the earlier faction test would survive and the positives would be reachable: exit=1, ok=39, FAIL=0, same crash. Still dies first, just later.

So I can't construct a mutation that reaches those positives. Any breakage of humanFaction for a valid index takes the suite down before them, because earlier tests mint factions and an uncaught throw ends the process.

This is not a coverage hole. The mutant is caught, loudly, and nobody merges on exit=1. It's that the crash is doing the detecting and the positives are getting the credit — and the diagnostic you actually receive is "Error at line 49, 39 of 201 assertions ran" rather than "minting a valid index works — FAILED".

I'd correct the sentence and leave the code alone. Making the positives reachable means the harness catching throws so the suite continues, which is a real change with its own costs, for a failure mode that already stops the build. The useful thing to write down is the fact itself: those positives are unfalsifiable in this harness, and two attempts to falsify them are the evidence. That's exactly the "name what was measured" rule pointed at the newest code.

Worth one line in the phase-B notes too, since milestone 3 will call humanFaction from the roster: a guard that throws makes its caller's error handling load-bearing. A lookup miss goes from silently minting the wrong faction to killing the frame loop. That's the right trade — loud beats silent — but the roster needs to handle the miss rather than let it propagate.

Done on #17

Nothing structural left. The code is correct, the fingerprint matches main, and what remains is a sentence. Four findings on this PR, two of them in code added in response to earlier rounds — which is the argument for re-reviewing the fix rather than the original, and it's held every time.

@sbddesign f21b5e8 is mergeable.

[sbddesign]

Verified 55259b0. 201 + 41, exit 0, MERGEABLE/CLEAN. The mints helper does what you said — both mutations now report instead of one aborting.

And you were right to correct me. My two measurements were sound; the sentence I wrapped around them — "any breakage of humanFaction for a valid index takes the suite down first" — was a generalisation from n=2, and you falsified it in one attempt. Measured half, unmeasured half, mine this time. The distinction you drew ("I could not construct" vs "it cannot be constructed") is the right one and I should have written the weaker claim.

The asymmetry you equalised was the catching, not the asserting

const zero = mints(0)
check('minting from 0 works', zero.ok && zero.value === FACTION_PLAYER)   // ok AND value
check('minting a later participant works', mints(3).ok)                   // ok ONLY

mints(3) never looks at what came back. So a humanFaction that returns the wrong faction — without throwing — sails through. Two mutations, both realistic, both green at 201 ok, exit 0:

Math.min(index, 2)          "cap the roster at 3 players"     201 ok  exit 0
index === 5 ? -1 : index    participant 5 IS the AI faction   201 ok  exit 0

The first silently collides participants 3, 4, 5 into one faction — friendly fire then makes them unable to shoot each other, so in a five-player lobby the late joiners form a mutually-invulnerable bloc.

The second is the original bug back, one index over: a human minting FACTION_AI, unable to shoot the NPC filler and unable to be shot by it. That is precisely what this guard was added to prevent, and the suite is silent.

The check named for that property tests one index

check(
  'no valid human index can reach the AI faction',
  zero.ok && zero.value !== FACTION_AI,
)

The name quantifies over every valid human index. The condition examines zero — index 0, the same value already asserted on the line above. It is the only check in the section labelled "the property the guard exists to protect", and it protects it at exactly one point.

This is the first finding of this entire thread recurring in the newest code: a check whose name promises more than its condition asserts. The PR #10 vacuous guard, six rounds later, on the assertion explicitly designated as the important one.

Small fix, matching what's already there for zero:

for (const i of [0, 1, 2, 3, 7]) {
  const m = mints(i)
  check(`index ${i} mints itself`, m.ok && m.value === i)
  check(`index ${i} cannot reach the AI faction`, m.value !== (FACTION_AI as unknown as number))
}

Both my mutations die on that, and it costs one loop.

Not a blocker, and this is my last on #17

Nothing mints an index above 0 today, so neither mutation describes live behaviour — it's the milestone-3 roster that will mint 1, 2, 3… and that's the code these assertions exist to protect.

The identity humanFaction(i) === i is what makes roster indices and factions interchangeable, and it's currently asserted at i = 0 only. Worth closing before the roster leans on it, but it can land with milestone 3 as easily as here.

@sbddesign 55259b0 is mergeable either way — the code is correct, the tests are narrower than they read.

[sbddesign]

Verified eb744f0. 226 + 41, exit 0, MERGEABLE/CLEAN. Change is scripts/simcheck.ts only, so the shipped simulation hasn't moved since 55259b0 and the cross-commit fingerprint still attributes without re-running.

Both my mutations now die, and I added a third that my own suggestion would also have missed:

Math.min(index, 2)          exit 1  FAIL=7   "minting index 3 yields exactly 3 — got 2"
index === 5 ? -1 : index    exit 1  FAIL=2   "index 5 cannot reach the AI faction — got -1"
index === 6 ? 4 : index     exit 1  FAIL=2   "every index mints a distinct faction — 0,1,2,3,4,5,4,7,8"

That third one is the distinctness check earning its place: it prints the sequence with the collision visible, so the failure names the defect rather than just flagging it. Neither 4 nor 6 was in the list I proposed.

My fix had the defect it was fixing

[0, 1, 2, 3, 7] was a hand-picked sample standing in for a universal property — the same shape as the single-index check I was objecting to, just with more points. And it omitted 5, which was the index in my own mutation, in the same message. The finding and the remedy came from one review and missed each other.

You found it by implementing my suggestion literally and re-running my test against it. That's the right method and it's worth naming: the fix a reviewer proposes is also unreviewed code, and it arrives with the reviewer's authority attached — which makes it likelier to be applied as written and less likely to be probed. Your "every fix is new code" is the general form; a reviewer's suggested fix is its least-examined instance.

Contiguous 0..8 is right for the reason you gave: a sample leaves gaps by construction, and the gap sits wherever the next mistake lands.

Done

Nothing outstanding. Six findings, four of them in code written to address the previous round — including this last one, which was in code written to address me.

@sbddesign eb744f0 is mergeable.