docs/code-improvement-tiers.md

Code Improvement Tiers — a TDD-first plan from a full-codebase review

This is the code-quality companion to improvement-proposals.md (which proposes changes to the game) and improvement-roadmap.md (which sequences them). Nothing here changes a rule, a number, or a mechanic. Every entry is about the code that implements them: a defect, a missing guard, a type gap, or a structure that made one of those possible.

Status

Tier 1 is delivered. All 42 items shipped across seven commits, each fix preceded by a test verified red against the real behaviour, and the mutation-proven gaps re-checked by re-introducing the exact defect. Suite went from 1,897 → 1,964 tests, still 0 failures; npm run typecheck stays clean; // @ts-check coverage went from 10 files to 20.

Three things the work itself turned up, beyond the findings as written:

• The NET round-trip tests rejected a first cut of the save hardening that stamped pacified: false and rally.nodeId: null unconditionally. They were right: a hardening pass must be the identity on a valid save, byte for byte. Both now adopt only fields the save actually carries.
• test/_helpers.js's header claims its fingerprints are "immune to Map-iteration / JSON-key order". True of entitySnapshot, whose tuples are joined in a fixed field order — but galaxySnapshot stringified diplomacy and market pressure raw, so identical values in a different insertion order read as a divergence. Another false comment, found the same way as the six in the table below.
• Adopting // @ts-check on persist.js surfaced a real asymmetry: deserializeGalaxy's object literal omitted lanes while createGalaxy's includes it, so a Galaxy was briefly missing a field its own typedef requires. Three further typedef drifts (Building.rally.nodeId, Building.queue[].alt, Building.rivalAscended) came out of the same pass.

D5 (strictNullChecks) was evaluated and deliberately deferred, as the item itself proposed: turning it on today yields 198 errors across the 20 checked files. That is a genuine project with its own fallout, not a Tier-1 slice — it belongs after the Tier-2 structural work, when fewer of those errors are in code that is about to be rewritten anyway.

The silhouette-collision half of C6 also moves to Tier 2 as the plan intended: the test is cheap, but it goes red until the Antimatter Gate and Torpedo Battery get distinct art, and that is an art decision, not a code one.

Tier 2 is delivered. All 20 items shipped across five commits. Suite 1,964 → 2,002 tests, 0 failures throughout; typecheck clean. Full-suite wall clock 2m17s → 1m34s.

Three outcomes worth recording, because they changed the plan rather than following it:

• The per-frame allocation item did not survive measurement. Memoizing shade/hexA on (hex, percent) looks like an obvious win — tiny fixed key space, called per building per frame. Benchmarked on a 120-building, 306-unit frame it is a loss: garbage 51.5 → 62.2 KB/frame, frame time 0.22 → 0.24 ms, because the cache key is itself a fresh string per call. Reverted, with the numbers written at the function so the next attempt starts from a baseline instead of the same intuition. Only the loop-invariant hoist survived.
• tickSelfPlay's ordering was corrected in the comment, not the code — the option the finding explicitly sanctioned. Making the claim true needs either a frozen pre-tick read or moving the "ai" call out of sim.js's hardcoded pipeline; both restructure the tick contract, so they sit in Tier 3. The measured bound (one frame, one direction) is now pinned by a characterisation test so it cannot widen unnoticed.
• Two fixes changed behaviour a test fixture depended on, and both were retargeted rather than weakened: consolidating radiusOf broke formation.js, whose callers pass unit-shaped literals with no kind field (the helper resolves by type first, with a comment on why); and the name-based bye tie-break moved which Swiss candidate draws the bye, so a fixture's winner stopped fighting.

The rebuild-signature co-location is done to the extent Tier 2 allows: the 141-line expression is now a named panelSignature() function rather than an inline block, which is the seam the Tier-3 panel registry moves into. Genuine per-panel co-location needs the decomposition itself.

Tier 3 is delivered. All seven refactors resolved across eight commits. Suite 2,002 → 2,051 tests, 0 failures throughout; typecheck clean.

Every design question in the table below now has an answer, and three of them changed what got built:

• The AI decision seam is "export in place", not a new aiDecisions.js leaf. The helpers read state and ctx and belong with their phase; a module existing only for testability would have to import back into the layer it came from. Nine predicates are now exported and directly drivable. The 13-site build idiom did collapse — into tryBuild/tryBuildAt — once it was clear the two steps that fail silently are canAct and spend: omit the first and the AI ignores its APM budget, omit the second and it founds buildings for free. Neither shows up on the default archetype, whose apm is null, which is why five sites had already drifted to relying on a canAct three lines up. The builder stays a parameter, because two sites have always founded with workers[0] rather than the nearest free worker, and collapsing that would move the determinism fingerprint — a behaviour change, not a refactor.
• The declarative save schema was evaluated and rejected, and replaced with the guard it was for. Every field's sanitizer in persist.js is deliberately different (popCap/matchTimeLimit must not go through num(), since Number(null) === 0 turns "no override" into "end the match at time 0"), and the comments explaining each choice are the most valuable thing in the file. A table would wrap that same bespoke logic in per-entry closures while the payload has to stay byte-identical: more risk, less clarity, no new safety. What the table was for — catching a field written by serPlanet and never read by rehydratePlanet — is now a payload-level round-trip comparison for the game save and the galaxy save alike, so the next such field fails immediately instead of resetting silently on load.
• hudSelection.js split by "derive vs render", not by panel. The registry the table proposes is still the destination, but the seam that mattered first was panelSignature — the panel's entire reactivity model, which decides whether it rebuilds at all. It and the derivations it shares with the renderers now live in hudPanelSignature.js, a genuine leaf outside the hud → hudSelection cycle: 2,242 → 1,915 lines, with the panel's output byte-identical, verified rather than assumed by the golden lock the table asked for (25 fixtures, committed one commit ahead of the first extraction).

Four things the work turned up that were not in the plan:

• Three guards written during Tier 3 were vacuous on their first cut, and each was caught by mutation rather than by review. The dead-import guard located the import block by walking lines until one stopped looking like an import — and since every file opens with a prose header comment, it broke out on the first sentence containing a bracket and reported a file with twenty dead imports as clean. The save round-trip guard's first mutation (a field defaulting identically on both passes) proved nothing, because both payloads agreed. The panel-fixture guard used a node count as a proxy for "the setup took" and flagged the Habitat, whose whole panel is legitimately two rows. The lesson is the same one Tier 1's mutation pass taught: a green guard is evidence of nothing until you have watched it go red.
• 25 dead imports in hudSelection.js alone, twenty of them predating this work — repairBtn, departBtn, saveBtn, pauseBtn, starmapBtn, resourcesEl, clockEl, scenarioStatusEl and more, left behind by code that moved to hud.js. A dead import advertises a dependency that does not exist, so the import block misleads anyone reading it, and the module-reachability guard counts an edge nothing walks. The new guard found three more elsewhere (aiWorkers's UPGRADES, formation's UNITS, input's dragCamera).
• The DOM test double had grown to nine hand-rolled copies, and the differences between them were not considered decisions — they were whichever subset of the DOM the file that copied it happened to need. overlays' copy tracked parents so .remove() really spliced and hudSelection's did not; hud's querySelector walked the tree and update's was hardcoded to null; observer's getBoundingClientRect returned a real 800×600 box and saveload's returned zeros; saveload's appendChild discarded its argument outright. A test written against a weak copy silently asserts less than the identical test written against a strong one, and nothing told you which you had. Consolidated into test/_dom.js with no assertion weakened — the two axes that genuinely change what the code under test does (the 2D context, the viewport size) stay configurable.
• The render extraction found its own targets by measurement rather than by the plan's list. Of the proposed primitives, the ones that earned their place were the three where copying had already caused or hidden a bug: centeredText (whose restore is the point — textAlign is canvas-wide state, and two sites shipped without it, invisible only because drawFrame's outer save/restore swallowed the damage once per frame), drawLabelChip (two sites had independently hand-rolled the same plate arithmetic), and hiddenByFog (four copies of a rule whose failure is silent in the direction that matters — it paints the enemy's army through the fog). A blanket fillStroked collapse was measured and skipped: the 26 candidate sites set their fill/stroke state in enough different orders that a helper would have moved the assignments, not removed them.

How this was produced

Eight reviewers worked the codebase in parallel, one per domain — engine core sim, AI + tools/ailab.js, economy/logistics, persistence/galaxy, UI/HUD/input, render, the test suite itself, and the cross-cutting seams (types, import graph, duplication, tooling). Each was given CONTRIBUTING.md's hard rules as constraints and told that a finding without a citation and a reproduction is not a finding.

The evidence bar matters for how you read what follows. Findings were not inferred from reading code — they were executed. The reviewers ran real games, mutated real source in scratch clones and re-ran the full suite, instrumented canvas contexts to count save/restore depth, ran tsc probes against engine/types.js, and built the import graph from actual import statements across 181 files. Where a number appears below it was measured. I spot-checked the highest-impact claims against the tree myself and found no overstatements.

Baseline at the time of review (commit c775143):

Test suite	1,897 tests, 0 failures, 4,723 assert.* calls across 101 *.test.js files (31,711 lines)
Wall clock	145 s here; measured separately at 150.4 s full vs. 20.5 s without test/ailab.test.js — that one file is 87% of the suite's wall clock for 3.4% of its tests
npm run typecheck	exits 0, zero output
Production code	58,701 lines; 76 shipped modules; largest is hudSelection.js at 2,201 lines / 131 KB
Type coverage	// @ts-check on 10 of 76 modules (13%) — all in engine/, 0 of 29 root modules

The four patterns worth internalising

Individual findings are listed by tier below. But the same four shapes turned up independently in domains that never talked to each other, and they are the more useful output of this review.

1. Comments assert invariants the code does not hold

This codebase explains why, which is a real strength — and it has quietly become a liability, because several load-bearing "why" comments are now factually false, and each one was believed by a later reader. Six were found independently:

• engine/bomb.js:139 says its radiusOf is "same as movement.js/formation.js's own" — the three bodies have diverged, and only bomb.js's handles buildings. movement.js's copy is in a // @ts-checked file whose JSDoc advertises @param {Unit|Building} while silently returning the magic constant 9 for any building.
• engine/separation.js:100 says a stale autoTarget "reads as still-combat-mode for one tick too long — harmless." It is never cleared on a move order at all, so it reads that way permanently.
• engine/industry.js:300 lists the callers that "never touch this cache" and names the HUD — while buildingConcern, 90 lines below in the same file, calls it, and renderBuildings.js:122 calls that every frame.
• engine/persist.js:828 says the lane load path re-runs "exactly like runLanes' own per-cycle validation does at runtime." It skips the de-duplication assignShipToLane enforces.
• engine/aiMilitary.js:294 says its corrective pull-back "keeps the WHOLE parked group inside the radius." The correction is radial and the farthest slot is an off-axis corner, so it never converges — measured residual grows from 0.1 px at 8 units to 1.7 px at 40.
• tools/selfplay.js:81 says both controllers "think on the IDENTICAL pre-tick snapshot." Measured over 400 think cycles, the "player" seat had already mutated orders on 13% of them and founded buildings on 1% before the "ai" seat read the state.

The lesson is not "write fewer comments." It is that a prose invariant in this codebase should be accompanied by the test that makes it executable — otherwise it decays into a confident-sounding lie that the next contributor builds on.

2. Guards check the easy direction

Nearly every guard in the repo verifies the direction that was easy to think of, and is structurally blind to its complement:

Guard	Checks	Blind to
save-hardening NET round-trip	serialize→load→serialize is stable	a field absent from both sides — which is exactly how galaxy.rivalAscended shipped unpersisted
render-roster	a type isn't drawn as the generic fallback	a type drawn identically to another real type — 3 real collisions today, one of them the Odyssey wonder
static-integrity	every relative import resolves to a file	a module that resolves fine but nothing imports — the orphan bug that shipped in 2a07a69
ailab bye tests	a bye never gains a candidate credit	a bye never costs rank — it does, structurally
ailab override tests	overrides don't leak out of a duel	two candidates colliding within one duel — silently merged
engine-purity	every file in engine/	anything in an engine/ subdirectory, and data.js (19 importers, imported by 9 engine modules)

3. The type system is opt-in and declawed

engine/types.js is a good idea implemented halfway, and the halfway state is worse than either end.

• 12 fields exist at runtime but are missing from 5 typedefs (proved with a tsc probe returning 12 errors and 0 control failures) — including State.playerAi, the central field of the flagship self-play feature, omitted by the very commit that added it.
• Adding // @ts-check to 12 of the most load-bearing engine files — sim.js, persist.js, combat.js among them — produces zero errors, because strict: false + noImplicitAny: false makes every un-annotated param any. engine/recycle.js already carries the pragma and has 0 @param tags across its 7 exports: nominally covered, checks nothing.
• The drift actively blocks the file-by-file expansion CONTRIBUTING.md prescribes: annotating one function in engine/supply.js immediately produces three errors in correct code, which trains contributors to distrust the checker.

A pragma-only coverage campaign would take the number to 100% and catch zero bugs. Fix the typedefs first.

4. Coverage is example-based and first-cycle

The suite has genuinely good hygiene — zero tests with no assertion, zero assertions on literals, one load-bearing try/catch, and excellent assertion messages. What it lacks is properties. It proves "a worker fills the battery once", never "and again after that"; it asserts a specific number at a specific site, never a conserved quantity. Two of this review's sharpest bugs live precisely there — a servers tally that only ever climbs, and a wonder that keeps eating the treasury after it is done — and both sit inside green suites.

The reviewer proved the gap by mutation testing: 15 realistic regressions injected into a scratch clone, 6 survived the full suite, including gutting drawBuildingBars (every health bar, build-progress bar and stall badge in the game) to return;, dropping the entire playerAi block from saves, and typo'ing <script src="main.js"> in index.html to a nonexistent file — which ships a blank white screen with CI green on both Node versions.

---

Tier 1 — Ship now

High value, low risk, no design decision required. Every item has a named red-first test and an S/M effort. Items marked ⚠ replay change simulation output and need their own commit with determinism*.test.js and balance.test.js re-verified.

1A. Live defects

Every one of these was reproduced by running the real code.

#	Defect	Where	Fix	E
A1	Escorts strand on a ghost leader forever. keepFollowingLeader decides its leader is gone by hp <= 0, but deployColonyShip and updateUnitRecycle remove units at full hp. Escorting a colony ship and deploying it — the canonical Odyssey play — permanently bricks the escorts, orbiting a unit no longer in state.units.	engine/movement.js:99; removals at engine/recycle.js:151, engine/colony.js:40	Ask the state, not the field: || !state.units.has(leader.id)	S
A2	Squad-release loop splices the array it iterates, so every splice skips the next element. Dropping two adjacent followers releases only the first; the second keeps squadLeader pointing at a leader whose own list no longer contains it.	engine/commands.js:161-165, :46-53	for (const old of [...leader.squadFollowers])	S
A3	A Torpedo Battery is locked out of auto-resupply forever. countLogistics resets servers for factories and combust plants but not for the third input-taking kind inputNeedsOf grew (def.ammo), so the tally only climbs — 7 after one run, permanently above MAX_SERVERS. This is verbatim the bug the file already documents as fixed for power stations at haul.js:161.	engine/haul.js:158-164	Reset from the single source of truth: if (inputNeedsOf(b)) b.servers = 0;	S
A4	A completed Antimatter Gate eats the Strategic tier forever. updateWonder clamps the charge but not the spend, and victory.js:80 deliberately never ends a galaxy game. Measured: 90 AI cores / 135 antimatter / 45 plasmatorp destroyed in 450 sim-seconds after the Gate finished. The Leviathan competes for the same three goods.	engine/wonder.js:43-51	Early-return at charge >= 1; clamp p to the remaining charge so the last tick pays only for what it banks	S
A5	A Rival Gate ascension is undone by save/load. galaxy.rivalAscended — the idempotency latch the code calls by that name — is never written to the payload (0 hits in persist.js, 0 in test/). On reload the permanent stance ceiling is lost and the ascended event re-fires on every Continue. Autosave runs every 12 s.	engine/persist.js:706-733	Additive: persist as an array, restore filtered against known exactly like claims at :778. No version bump.	S
A6	Freight Lane and Colony Policy panels are dead controls. Their handlers do mutate(); renderHUD(); but the 141-line rebuild signature contains no lane or policy term, so the panel never redraws. Reproduced by node identity: commodity chips, + New Lane, disband, and every Colony Policy control are inert — and clicking again silently toggles the state back.	hudSelection.js:1043-1092, :1129-1174; root cause :204-344	Add the two missing signature terms (serialized lane set, colony policy), gated on game.galaxy	S
A7	One throwing draw permanently kills the view. drawFrame has a bare save()/restore() pair; loop.js:44 swallows a throwing render so the loop survives — but the camera transform is never popped, so every later frame draws on top of it, including the backdrop clear. Measured save-depth stays at 1 forever. A live trigger exists: renderEffects.js:579 destructures building.rally unguarded, and cleanEntity never defaults rally.	render.js:113/:146; renderEffects.js:579	try { … } finally { ctx.restore(); } + a rally guard + restore textAlign at renderBuildings.js:139/:723, renderNodes.js:38	S
A8	An inert Helium Bomb renders as armed, and idle units snap to another unit's commanded facing. The module-level _disp scratch is filled with Object.assign, which never deletes stale keys, so one unit's optional fields leak into the next. Measured: the armed-core fill appears twice for one armed bomb. It is a safety cue.	renderUnits.js:43, :69	const d2 = { ...u, x: d.x, y: d.y } (S) — or drop the scratch entirely (M)	S
A9	runDuel merges both candidates' overrides into one shared table. Two variants of the same dial — the natural A/B workflow — silently degenerate into a mirror match against a Frankenstein row belonging to neither, and report a normal-looking winner. Every tuning decision runs through this.	tools/ailab.js:672-673, :476	Diff the two candidates' key sets first; throw a named error on any shared key	S
A10	The barracks hides units the engine would happily build. The HUD reimplements availability as "every cost commodity has a local node", a rule the engine does not have — and the engine's own tradeables is the same predicate one clause longer (present.has(c) || (res[c] || 0) > 0). Ship gas to a gas-free world and the Wraith button is absent, not greyed.	hudSelection.js:1405-1407 vs engine/market.js:219-222	Export commodityAvailable(state, owner, com) from market.js; both callers use it	S
A11	cachedPowerThrottle never invalidates on a tick-less state (undefined !== undefined is false forever) — the stub idiom used throughout industry.test.js and reachable from any non-tick() caller. Also buildingConcern's factory epsilon (1e-6) disagrees with updateProduction's (none), so the badge can read "bufferFull" while the factory produces.	engine/industry.js:302-317, :406, :422	Initialise _powerCacheTick = -1; align the epsilon; fix the two false comments	S
A12	Owner-parametric leak: plannedMix/wantsDeepIndustry read state.ai's difficulty and strategy regardless of who is asking. Measured on a two-controller Odyssey state, the "player" controller's entire deep-industry identity is decided by the other controller's difficulty. rivalGateEligible is already owner-parametric and already wrong.	engine/aiWorkers.js:157, :181	Thread owner through, defaulting "ai" so single-owner behaviour is byte-identical	S
A13	⚠ replay — autoTarget is never cleared on a move order. It is written in exactly one place and read by isCombatMode, so a unit pulled out of a fight with Move (the documented way to disengage) uses tight combat packing forever instead of the padded idle spacing SEPARATION_PAD_MULT exists to give it.	engine/combat.js:32-36, :53-58	Clear it in the move branch before returning. Own commit; re-check balance.test.js.	S

1B. Save-safety — untrusted input reaching live state

cleanEntity is genuinely well-hardened, with ~20 coerced per-field cases and excellent reasoning. That discipline stops at the entity boundary. Three whole sub-objects reachable from an Odyssey save are written into live state verbatim.

#	Hole	Where	Measured consequence
B1	Diplomacy block spread verbatim — { ...createDiplomacy(), ...P.diplomacy }, untrusted object wins. clamp is the identity on a non-number, so stance string-concatenates: "friendly" → a 4,801-char string in ~160 sim-seconds → trips sanitizeSave's own MAX_STRING_LEN on the game's own output. Autosave rotates both generations, so both save slots become permanently unloadable and the campaign is gone.	engine/persist.js:798	Data loss. Fix pattern already in the file (cleanPlayer, :329).
B2	Market pressure/glut overlaid unvalidated. Runtime clamps to ±0.6 / 0.85; the load path clamps nothing. Measured: unitPrice of 5,000,005; a negative sell price; 125 M credits from one sell; and a string pressure makes galaxy.credits irreversibly NaN (updateMarket is multiplicative, so it never self-heals).	engine/persist.js:795-797	Economy death or infinite credits. The bands already exist in market.js — export and reuse them.
B3	AI controller restored verbatim, 15 fields, no num(), no enum check — in a file where a unit's facing gets a five-line comment justifying its coercion. nextAttackAt: "later" makes state.time >= nextAttackAt false forever: the AI's attack timeout is permanently disabled, with no crash. state.playerAi has zero save coverage in any test file.	engine/persist.js:639-670	A quiet game reads as boring, not broken.
B4	updateService derefs unit.cargo unguarded while every sibling path guards it. persist.js:122 nulls a freighter's cargo while leaving aiLogistics: true, so a corrupt save loads "clean" and then throws on tick 1 inside the rAF loop, past load's try/catch — the player cannot get back in. This is verbatim the failure production.js:76 already guards against.	engine/haul.js:476-477	Unrecoverable. Mint the slot lazily at the consumer.
B5	Lane load path drops two runtime invariants. Duplicated shipIds survive (measured: 750 ore moved by one 250-hold hauler in one cycle — 3× throughput), and laneSeq is not lifted past the ids the save carries, so a new lane can collide and become permanently unreachable from the UI. Both fix patterns are already written twice in the same function (maxOwnEntityId, maxGId).	engine/persist.js:831-844, :789	Free throughput; an undeletable lane.
B6	rally is the one coordinate pair never clamped. order and anchor are both clamped for the stated reason that garbage "flows straight into stepToward" — rally flows to exactly the same place via production.js:108. Measured: a 1e9 rally spawns units at 2722, 3084 on a 1600×1000 map, outside every fog and grid bound.	engine/persist.js:98-269	Reuse the existing clampOrderCoords closure.

1C. Guards that don't guard

The most alarming section, because these are the mechanisms CONTRIBUTING.md cites as making its hard rules executable.

#	Guard	What's wrong	Fix
C1	determinism.test.js	Fingerprints with a local, weaker snapshot than the project's own _helpers.entitySnapshot — proved blind, field by field, to unit cargo qty and commodity, move-order target coordinates, building charge, tier, constructing, and state.time. _helpers.js:8 exists so "a dozen suites don't each re-roll them slightly differently" — and the most load-bearing suite re-rolled it.	Delete the local snapshot() and mulberry32; import from _helpers.js. Add a sensitivity test as the guard-on-the-guard. S
C2	determinism-roster.test.js	Sweeps all 11 worlds but only 400 ticks = 40 sim-seconds — measured 0 combat orders on every world, ≤8 units, ≤3 buildings. The deep run uses the weak fingerprint on one world; the broad run replays only the opening. Raising to 2,000 ticks costs +1.3 s and buys 6,034 combat-order ticks.	Raise the loop; assert the fixture actually reaches combat. S
C3	engine-purity + static-integrity	Both use non-recursive readdirSync. A file placed at engine/net/lockstep.js calling Math.random, Date.now, window.localStorage, document and importing a nonexistent module passed all four guards, 8 pass / 0 fail. The purity test's own rationale mentions netcode — the first thing that growth produces is engine/net/. Separately, data.js (19 importers, 9 of them engine modules) is scanned by neither guard.	One shared recursive walkJs(dir); extend the purity scan to root modules transitively imported by engine/. S
C4	index.html	Never validated. Changing src="main.js" to src="mian.js" survives the full suite — a totally dead game, green on two Node versions plus typecheck. main.js is also the one root module no test imports.	Resolve every src/href in index.html against disk. S
C5	Module reachability	Nothing checks that a shipped module is reachable from the single entry point. This bug already shipped (2a07a69: the Tech Chart button and T hotkey silently did nothing), and three modules still hang off bare side-effect imports in main.js. 7 modules attach top-level listeners and would lose all wiring silently.	Walk the graph from the entry point parsed out of index.html; allow-list engine/types.js. S
C6	render-roster.test.js	Proves "not the generic fallback", not "distinct from every other type". Traced pairwise: antimatter_gate is drawn as a slightly larger antimatterforge — the Odyssey wonder, a victory condition — and torpedobattery as a smaller torpedoworks. Also never varies entity state, so constructing, capital, electrified, tier pips, armed, laden and veterancy branches are all unreached; drawUnits is tested with exactly one unit type (Skiff, 1 of 18).	Radius-normalized silhouette uniqueness with a named SHARED_HULLS allowlist; roster × state-variant sweep. S (test)
C7	Render entry points	9 of 14 exported render functions have zero test call sites, drawFrame among them — the orchestrator owning draw order, selSet threading and the save/restore of A7. A wrong draw order silently reintroduces the "ship paints out a base's health bar" bug that render.js:135 exists to fix.	Draw-order assertion + doesNotThrow smokes with realistic state. M
C8	Vacuous render tests	drawBuildingBars and drawBuildGhost are covered only by assert.doesNotThrow. Replacing each body with return; survives the full 1,832-test suite — every health bar, build-progress bar, buffer gauge and stall badge can vanish, green. The same mutation on drawEffects/drawBuildings/drawUnits/drawTerrain/drawNodes was killed every time: the recording-Proxy pattern works, it just wasn't applied here.	Trace assertions using this file's existing recording fakeCtx. S
C9	Save round-trip	Dropping aiColonyTarget, aiLastThreatAt, or the entire playerAi block from persist.js survives the full suite. (aiWaveCount and a wonder's charge were both killed — the per-field pattern exists, it just stops short.)	One table-driven controller round-trip test. S
C10	UI import-safety	CONTRIBUTING.md:52 requires UI modules to import cleanly under Node. 8 of 11 throw — hud.js, hudSelection.js, overlays.js, boot.js, observerPanel.js, techChart.js, setup.js, main.js — and the pattern is inconsistent within files (boot.js:101 guards, boot.js:71 doesn't). All 13 DOM test files install globalThis.document before importing, papering over it permanently.	Child-process-per-module import check (a sibling's stub would mask it). M
C11	data-integrity.test.js	Guards exactly one stringly-typed cross-reference. Three more seams are unguarded and currently clean: commodity keys across all cost/upkeep/consumes/inputs tables and 18 recipes, BUILDINGS[*].produces against UNITS, and ODYSSEY_WORLDS against PLANETS + the archetype map. The repo has been bitten by this genre twice already.	Three roster-driven tests, each red-provable with one typo. S
C12	Suite integrity	npm test passes no path, relying on Node's implicit discovery globs (which changed across 18/20/22). Nothing asserts the ten named guard files still exist — delete one and CI is green with a smaller number nobody reads. _helpers.js is itself spawned as a test file.	"test": "node --test test/" + a meta-test parsing the guard names out of CONTRIBUTING.md. S
C13	Perf-in-determinism	The suite's only two wall-clock assertions live inside determinism.test.js (measured headroom 4.8×, not the "~6×" claimed, on a two-version CI matrix). If the perf alarm trips, the file that goes red is the determinism guard — training contributors to dismiss a red determinism file as flaky.	Move both to test/perf-guard.test.js, budgets unchanged. S
C14	Untested shipped behaviour	Commit ac44339's ore-surplus army-cap feature has zero tests — proved by mutation: replacing its body with return 0 leaves all 1,897 green. The commit records "manually probed" in place of tests. Blind because every standingArmyCap test runs in skirmish, where the code short-circuits.	Three endless-state tests, incl. the skirmish guard. S
C15	Hot-path contract	grid.js's shared _scratch buffer has a documented aliasing contract ("none makes a second queryNeighbors call while a prior result is still being iterated") that no test asserts. Two call sites hold it across a loop body. A violation would deterministically skip separation pairs — invisible to determinism testing, since both runs would be wrong identically.	Pin the reuse contract by array identity, plus the real guard. S
C16	Dead branch	loop.js:39's backlog-drop branch is unreachable at the shipped hz — 0.25 s clamp ÷ dtFixed 0.05 = exactly MAX_SUBSTEPS 5, an undocumented exact identity coupling three constants. Measured 0 times taken at hz=20, 72 at hz=60. Bump to 30 Hz and a dead line starts running, handing the renderer an interpolation alpha ≥ 1.	Cover it at hz: 60; name the coupling at the constant. S

1D. Make the type system real

Order matters here — D1 unblocks everything else.

1. D1 — Fix the 12 drifted fields in engine/types.js. State.popCap, State.playerAi, AiState.strategy, AiState.lastThreatAt, Galaxy.lanes, Galaxy.laneSeq, GalaxySettings.startId, GalaxySettings.popCap, and Diplomacy.{provokedAt, goodwill, request, lastFavorBucket}; plus Archetype.{wantsRefinery, turretCount, maxBarracks, garrison} and Order.{tx, ty, explore, patrol}. Also decide whether Diplomacy.lastAiUnits — declared but never constructed — should be declared or deleted. Guard it with a text-diff test between each typedef block and its factory literal, asserting on the sorted array so the failure names the field. S, zero runtime risk (types.js has no runtime code and zero importers).
2. D2 — Make the pragma mean something. Add a static-integrity test asserting every // @ts-check file annotates its exported functions with the shared typedefs. Red today on engine/recycle.js (7 exports, 0 annotations). Without this, expanding coverage raises a number and catches nothing.
3. D3 — Adopt, one file per commit, in this order: supply.js → aiCommon.js → colliders.js → victory.js → production.js → the AI leaves (aiStrategy, aiDifficulty, aiArchetypes — all 0 errors once D1 lands) → scout.js → persist.js. persist.js is the prize: every save field is a State/Unit/Building field read by name, so annotating it makes save-shape drift a check-time error. Leave galaxy.js and entities.js for last.
4. D4 — Add "tools/**/*.js" to jsconfig.json's include. tools/ is not in the program at all today, so ailab.js and selfplay.js cannot be checked even if they opted in.
5. D5 — Evaluate strictNullChecks separately. engine/haul.js has the pragma and Unit.cargo is correctly typed Cargo|null, yet B4's crash produces no error because strict: false turns off null checking. This is its own change with its own fallout — keep it off the critical path.

1E. Documentation drift

• E1 — docs/improvement-roadmap.md:77 still lists the Technologist archetype as pending Phase 2 work, and improvement-proposals.md:491 still describes Kybernet in the present tense as playing "a generic Economist". It shipped in 17e2aad. The roadmap's Phase 2 gates Phase 4, so a completed item sitting in the pending table can block downstream work. S
• E2 — README.md's Project layout omits 19 shipped modules, including hudSelection.js (the largest file in the repo) and main.js (the page's only entry point, and the file whose missing import caused 2a07a69). The README never mentions npm run typecheck, which CI runs as a required step. CONTRIBUTING.md:82 lists 6 // @ts-check files; the real count is 10 — understating existing coverage by 40%, on the list that is the on-ramp for D3. S

---

Tier 2 — Structural

Real value, but each needs care: several move replays and need re-baselining, and a few change a public surface.

Correctness with a wider blast radius

• Collection-point freighter stalls empire-wide. assignShuttle's "don't leave yet" guard runs through zoneFirst's unbounded global fallback, so a partly-loaded ship waits on a backlog anywhere in the empire. Reproduced with two bases 3,600 px apart. It is also the only zone-aware scan that doesn't forward unit.homeCC. Single-CC games are unaffected by construction. engine/haul.js:623-631. M
• Yield bonus overfills a hold, and save/load then deletes the excess (measured 251 → 250 on a 250 cap). Breaks the hold's capacity and "a save round-trips identically" — invisible to replay testing because both runs agree until someone saves. engine/gather.js:138-143. S
• garrisonSlots' single corrective pass never converges (residual 0.1 px at 8 units → 1.7 px at 40), and the branch has never executed under npm test — every dispersal test uses 1 or 5 units around a bare CC, which doesn't overshoot until ~60. engine/aiMilitary.js:301-314. S
• order.patrol carries two incompatible meanings — a boolean requeue flag in sim.js:170 and a numeric circuit index in scout.js:54. Latent today only because issueScout bypasses the order queue; index 0 is falsy, 1–3 requeue forever. S
• nearestGatherDrop/nearestCommandCenter are the only "nearest" scans with no id tie-break, while six siblings pin theirs explicitly. zoneFirst resolves zone membership by identity against the result, so two equidistant CCs make every zone boundary depend on Map insertion order. Correct today; a different kind of guarantee from its neighbours. S
• radiusOf triplicated and already divergent — bomb.js's handles buildings, the other two return 9, and movement.js's // @ts-checked JSDoc advertises Unit|Building. Move the correct body to engine/colliders.js. S, ⚠ replay-adjacent (hot path; verify all call sites pass units — they do).
• Galaxy meta sets unfiltered (pacified, reached, discovered, wonBy, settings) while claims, colonyPolicies and worlds are filtered three lines away. Free domination milestones from junk ids, and a tampered settings builds a NaN-sized map on the next jump — the galaxy twin of a hazard already closed on the skirmish path. Careful: reached must filter against milestone ids, not world ids. M

Tests that would have caught the above

• Conservation-of-resources property test. No test anywhere sums total holdings across treasury + building store/input + freighter freight + worker cargo. This one property catches A4 and the overfill bug together, and it documents the intentional exceptions (rig overflow spill, wonder feed, recycle fraction) which today are only prose. The helpers already exist and are exported. M
• "Every state field is persisted or deliberately transient." The NET round-trip tests cannot express "forgotten". A denylist test over Object.keys(state) / Object.keys(createGalaxy(...)) goes red today on rivalAscended — the general net that would have caught A5. Fold claims, discovered, wonBy and rivalAscended into _helpers.galaxySnapshot too. M
• Panel-completeness meta-test. Table-driven, one row per panel family, asserting the panel actually rebuilds when its own state changes. Red today naming exactly the unwatched panels — converts A6's class of omission from invisible to red. M
• commandAt precedence table. The 10-branch right-click ladder is game policy the engine does not re-check, and issueEscort/issueFerryFreighter/issueSetHomeBase/issueAssistBuild/issueRepair have zero occurrences in input.test.js. No test pins which branch wins when two match. M
• ailab.test.js permanently mutates the shipped STRATEGIES table (aggressive.garrisonMult 0.4 → 0.9) with no restore — so 21 later tests measure a strategy the game never ships, and the suite's own three leak-detector tests capture their baseline after the leak. snapshotTables/restoreTables are already exported and simply not imported. S

Speed and structure

• test/ailab.test.js is 87% of the suite's wall clock — ~130 s for 65 tests, all guarding a dev bench. Roughly 51 s is spent simulating real matches to assert pure combinatorics (pairing counts, bye rotation, sort order, ceil(log2(n))). The file already demonstrates the fast idiom once, at :834, and its own comment explains why it's better. Dropping the loop from ~150 s to ~20 s is the difference between "run the full suite every cycle" being followed and being routed around. M
• Swiss standings rank raw win totals across unequal match counts, so the ranking is literally "everyone who avoided a bye, then everyone who didn't" — and the round-1 bye is decided by CLI argument order (measured: always the last-listed candidate). This is the mirror image of the bug e9ad1d0 just fixed. M
• The side-swap runs its two directions on different map seeds (measured overlap: 0 of 2), so the one mechanism built to detect seat asymmetry is confounded with seed variance and cannot fire — while tickSelfPlay has a real, measured, always-same-direction seat edge. Sort the names into the seed hash; swapAsym already shows what the paired design looks like. S (re-baselines duel/Swiss fixtures)
• tickSelfPlay's ordering claim. Pick one deliberately: make it true, alternate by tick parity, or correct the comment and pin the bound with a characterisation test. Do not leave the comment and the code disagreeing. M
• Rebuild-signature co-location. 141 lines with 11 inline IIFEs, each re-deriving a lookup that rebuildSelectionPanel derives again further down the file. M (Partly done, 2026-08-08: rebuildSelectionPanel is now a dispatcher over twelve named renderX() families instead of one 871-line body, so the "render" third of the registry exists already. What is left is moving each family's signature term next to its renderer — now a per-family move rather than one big one.)
• Production rosters copied out of BUILDINGS[].produces in three hand-maintained shapes, with the Odyssey gate duplicated differently from the engine's own. Land the test before the refactor — it passes today. S
• Silhouette collisions (C6's art half — the Gate deserves a bespoke hull), per-frame allocation (measured 2.16 ms of pure JS and 0.9 MB/s of garbage at 306 units, before any rasterization; pop caps go to 300 per side), five byte-identical cross-module function bodies (the two that actually bit — clamp, negate — are shared now and re-forking is guarded by test/static-integrity.test.js), and the UI import cycle (now 7 modules, not 6: competition.js joined it deliberately. Still benign only because every back-edge is called at runtime rather than at module-evaluation time, and still failing as a blank white screen if that stops being true — but no longer "guarded nowhere": a Tarjan check in test/static-integrity.test.js freezes the cluster's exact membership, so a new cycle anywhere, or an eighth member here, fails the suite). Each S–M.

---

Tier 3 — Deep refactors, design call first

Do not start these until the design question in each is answered. All are worth doing eventually; none is urgent, and each is a poor first PR.

All seven are now delivered (see Status, above, for the three whose answers changed what got built). Outcomes, in the table's order:

1. hudSelection.js — split by derive vs render: hudPanelSignature.js, 2,242 → 1,915 lines, behind a 25-fixture golden lock landed one commit ahead of the extraction. The per-panel registry remains the destination; this is the seam it moves into.
2. haul.js — four explicit *_PHASES sets with a terminal unknown-phase guard. The collection-point asymmetry was ruled intentional and is now documented at the code rather than inferable from it.
3. AI decisions — nine predicates exported in place; the 13-site build idiom collapsed into tryBuild/tryBuildAt.
4. Save schema — the field table was rejected on inspection and replaced by a payload-level round-trip guard, which delivers the property the table was for.
5. Render boilerplate — centeredText, drawLabelChip, hiddenByFog. A blanket fillStroked collapse was measured and skipped.
6. Test harness — nine hand-rolled DOM doubles consolidated into test/_dom.js, one file at a time, with a suite-integrity guard against regrowth.
7. Formation ranking — per-cluster, not global. The first test written for it compared destinations to origin centroids and so measured nothing; rewritten to the real invariant (ranking permutes within a cluster, never across).

Refactor	The question to answer first	Safety net required before starting
hudSelection.js decomposition — 2,201 lines, 2 exports, one 845-line function with 17 entity-type branches. Proposed: a panel registry of { key, match, signature, render } over five modules, leaving a ~250-line orchestrator. Co-locating signature with render is what structurally prevents A6 recurring.	Registry shape, and whether the runtime hudSelection → hud → hudSelection cycle constrains it	Golden panelTree() serializer over ~20 fixtures, committed with the harness; every extraction commit must leave all goldens untouched
haul.js's four phase machines — 657 lines, 14 untyped phase strings, five near-identical "walk there, then act" legs, two provably dead branches, and a behavioural asymmetry hidden in the duplication (updateHaul banks at a collection point; updateService/updateFerry walk past one to the CC).	Is the collection-point asymmetry intentional? That is a behaviour decision, not a refactor	Pin both behaviours first. The unknown-phase guard — a terminal else unit.order = null — is a Tier-1 slice worth splitting out now
AI decision extraction — ai.js has one export, aiContext is private, so every phase test must stage a whole match. Twelve pure helpers are unreachable; this is the direct cause of C14 and the garrisonSlots gap. Plus a 13-site build idiom.	Where the seam lives (aiDecisions.js leaf vs. exports), and how to handle the two build sites that do extra work inside the if	Characterisation tests on standingArmyCap and withoutHomeGuard against hand-built plain objects
Declarative save schema — ~630 of persist.js's 865 lines are hand-rolled per-field code across four functions, with the 15 controller fields written out four times with two hand-typed prefixes. This is the root cause of A5, B1–B4 and B6 all being omissions rather than mistakes.	Whether the payload must stay byte-identical (it must — every .prev autosave in the wild)	The controller field-set equality test, and the two version-coupling guards, both Tier 1 and cheap: SAVE_VERSION === GALAXY_SAVE_VERSION while serPlanet is shared, since the galaxy embeds per-planet payloads with no per-planet v
Render boilerplate extraction — ~1/5 of renderBuildings.js's 809 lines is copy-pasted drawing shape (30 hairline-outline sites, four near-verbatim blocks). New roster art is written by copy-paste, which is how the silhouette collisions and the missing textAlign restores arrived.	How far to push the primitives	Land C6's state-variant coverage first, then pinned per-type traces
Shared test harness — _helpers.js is imported by 7 of 101 files; the rest hand-roll 6 divergent fingerprints, 7 fakeCtx variants (with three-way circular attribution in their comments) and 1,538 lines of DOM scaffolding. C1 is the direct cost of that divergence.	Nothing structural — but migrate one file per commit; the stubs differ in load-bearing ways	Per-file pass-count baseline; a shared fake being stricter is a feature, and any test it newly fails is a genuine finding
Formation range-ranking vs. clustering — rankSlotsByRange flattens the nested cluster layout formationSlots deliberately builds, so units cross-swap sub-formations and walk an extra ~150 px.	Per-cluster or global ranking?	A multi-cluster fixture asserting each follower lands nearer its own centroid

---

Suggested sequence

Five PRs, ordered so each makes the next safer. Roughly a week of focused work for Tier 1.

1. The guards, first. C1 + C2 together (they are two halves of one hole — deep-but-weak and broad-but-shallow), C3, C4, C12, C13. Nothing else in this document is trustworthy until the determinism guard can see cargo and the purity scan can see a subdirectory. Cost: ~1.3 s of suite time and a handful of small test edits.
2. D1 + D2. Fix the 12 drifted typedefs and add the annotation-density guard. Cheap, zero runtime risk, and it unblocks D3 — which is otherwise actively blocked, since annotating supply.js today produces errors in correct code.
3. Save-safety: B1 → B2 → B4 → B5 → B6, plus A5. All identity-for-valid-saves, so the NET round-trip tests stay green throughout. B1 first: it is the only one that destroys a campaign outright.
4. The live defects: A1, A2, A3, A4, A6, A7, A8, A9. Each is small and self-contained; A3, A4 and A7 fix bugs whose identical twin is already fixed and documented elsewhere in the same file, so the tests are close to copy-paste. Hold A13 back for its own commit with the balance harness re-checked.
5. C5, C6, C8, C9, C11, C14, C15, C16 + E1, E2. Pure coverage and documentation. This is where the mutation-testing findings get closed, and it is the batch that makes steps 1–4 stay fixed.

Then reassess. Tier 2 splits cleanly across parallel workstreams; Tier 3 should wait until its design questions have actual answers.

(Historical: this sequence was followed. Tier 1 shipped in seven commits, Tier 2 in five, Tier 3 in eight — each tier merged as its own PR, with the design questions answered in writing before the corresponding Tier-3 refactor started.)

---

Appendix — findings by domain

Full per-domain reports, with the reproductions and exact TDD plans behind each summary line above, were produced for: engine core sim (8), AI + ailab (10), economy/logistics (10), persistence/galaxy (10), UI/HUD/input (8), render (8), test suite (11), and cross-cutting (10) — 75 findings total, of which 69 survived de-duplication and ranking into the 42 Tier-1, 20 Tier-2 and 7 Tier-3 entries above.

Domain health, in one line each:

• Engine core sim — genuinely good. Determinism discipline is real (_gi indices, hashStr tie-breaks, two-pass splash), the hot path is allocation-conscious. Weak seam: entity liveness is asked by hp while two removal paths delete live units.
• Economy/logistics — well-built; buffers clamped, throttles order-independent, tie-breaks explicit. Weak seam: coverage is example-based and first-cycle, so "works once" hides "never again".
• Persistence/galaxy — excellent for entities, and that discipline stops exactly at the entity boundary.
• AI + ailab — unusually well-decomposed; ai.js threading one AiContext through phase modules is a good structure. Weak seam: the newest tier carries its correctness claims in prose.
• UI/HUD/input — hygiene is better than it looks (AbortController teardown, null-guarded queries, innerHTML only from literals). The problem is shape, not rot.
• Render — the drawing is well-reasoned (real culling, cached silhouettes, batched fog). The problem is the seam between "a draw threw" and "the frame recovers".
• Test suite — good by the metrics most suites fail. The holes are in the guards, not the ordinary tests.
• Cross-cutting — architecture is healthy: zero engine→UI leaks, one documented cycle, zero orphans. The gaps are in what's enforced, not what's true.