Finish the test consolidation #162 described but left behind

[thalida]

Follow-on to #162, which described a backlog of "still on the list, all
mechanical" work in its description and then merged without filing it. This is
that work, plus a correction to what it actually amounted to.

Numbers

	before	after
runtime cases, frontend	1,800	1,797	−3
runtime cases, backend	435	435	0
static it / def	1,931	1,905	−26
not.toThrow sites	53	16	−70%
LOC			−539

43 files changed, 485 insertions, 1024 deletions. Backend coverage 90.19%,
gate is 80%. Both goldens have a zero-byte diff against the merge-base.

The LOC number is the real one. Collapsing ten it()s onto an it.each with
ten rows removes ten declarations and a hundred lines while running exactly the
same assertions, so a reader's burden falls much further than the runtime count
does. Where the runtime count should fall is deletion, and only three
deletions here were justified.

Most of the backlog was already done

The audit document #162 left behind lists nine finding classes. Working through
them one at a time, most of what it said remained had in fact been fixed inside
that same PR — the document was written mid-flight and never reconciled against
the merge:

• test_cache.py's load-missing/corrupt/version triple across four cache kinds
  is already test_damaged_cache_reads_miss, a 4x3 parametrize.
• _parse_svg_length is already parametrized.
• getStreetWidth is already an it.each.
• _commitFixtures.ts, repoLabelPositioning.test.ts and
  treeRendererCommitLookup.test.ts no longer exist.

Every claim in this PR was checked against the merged tree before acting on it.
That is also how the two rejections below were found.

Two of the audit's prescriptions were wrong

statsFixtures.ts does not need literal RepoStats. The audit called its
authorHue a mirror of the backend's _author_hue that had to track it, and
prescribed replacing the derivation with literal fixtures at each call site.
But nothing on this side reads a hue it did not get from the stats object it was
handed: authorColor.test.ts says outright that the name-to-hue hash is
backend-owned, and the one hue assertion in firefliesPlacement looks the value
up from the same stats object. There is no circular assertion to break, and the
prescription would have expanded 25 call sites to fix it. The mirror stays, with
a comment that says what it actually is and why the values are frozen —
decorationGolden digests the orb colours they produce, which is how the
attempt to change them was caught.

The ring cases in fireflies.test.ts do not duplicate orbitRings.test.ts.
orbitRings drives the pool directly; these drive it through the assembly, and
cover the wiring between the two that neither side covers alone.

What was fixed

Assertions that agreed with the code no matter what either did.
spatialGrid recomputed Math.ceil(100 / MIN_CELL_SIZE) on the expected side
of three assertions. islandGeometry recomputed the sqrt(2) x 1/cos(pi/12)
corner correction, then asserted its own arithmetic exceeded hypot(100, 100) —
a comparison between two literals with the subject nowhere in it.

Assertions the type system already makes. 28 typeof/Array.isArray sites
and 9 assertIsInstance. Every backend one deleted was already subsumed by the
next line: isinstance(commits, list) in front of len(commits) > 0 and a
sortedness check. Two stay — busyness avg/busy can really drift int to float
through a division and the JSON contract cares, and test_config's
isinstance(settings(), Settings) is the no-crash assertion, not a type claim.

not.toThrow hiding its own diagnostics. The sixteen layout invariant
checks in algorithm.test.ts wrapped helpers that throw with the offending pair
of rects in the message; the wrapper caught it and reported "expected function
not to throw" instead.

Five copies of one EventSource stub, three of which carried a comment saying
they mirrored one of the others.

Three copies of the WebGL renderer stub. city/index,
city/initialFraming and city/interaction/inputHandlers each carried the same
~35-line FakeWebGLRenderer plus the same postFx stub, differing in one place:
index's forceContextLoss calls a spy, which is the assertion behind "dispose
releases the context, not just its resources". The shared factory takes that
callback, so the one real difference is visible instead of buried in three
near-copies. They come in via await import(...) inside the mock factory, since
vi.mock hoists above the import block. Dropping the class also dropped the
only use of import * as THREE in all three files.

Seven TestCase methods for build_authors_list, each re-importing the
function to assert one input-output pair, now one parametrize with named rows
— [email-only] rather than a row index.

A local mkDir shadowing the imported one with different behaviour.
algorithm.test.ts imports mkFile/mkDir from cityFixtures and then
redefines both. The local mkFile was byte-identical. The local mkDir was
not: it re-prefixes paths recursively and accumulates descendants_count down
the tree where the shared one does one level. Two functions, one name, one file,
and nothing announcing the difference. The local one is now mkDeepDir.

useManifestSourceExcludes.test.ts, named for the behaviour it was filed
for rather than the module it tests, folded into useManifestSource's suite —
same two entry points, same stub.

Two copies of _final_manifest, byte-identical, the second's docstring
saying outright that it mirrored the first. It moves to conftest.py, which
already declares itself the home for exactly that. test_scan_dirty no longer
imports scan_tree or Manifest, since the helper was its only use of either.

Four building-dimension assertions that computed their expected value from
the same config the subject reads — dim.h against dim.floors * FLOOR_HEIGHT
is the implementation compared to itself. Each replacement number was derived by
hand from the documented formula and then confirmed: square 24.8 →
round(2.48) = 2 floors, portrait clamps to aspect 2.5 → round(6.2) = 6, panorama
clamps to 0.4 → round(0.992) = 1, non-media 100 lines → 6.

That last one is the payoff. The comment claimed the sqrt interpolation "lands
at 8 floors"; it lands at 6. The assertion was floors > 1, true either way, so
the file had been carrying a wrong number with nothing to catch it.

Coverage went up, not down

Three assertions were strengthened rather than shortened, and the proof is that
the strengthened one bites where the original could not. Changing lerpRange's
Math.round to Math.floor now fails getLightness > interpolates linearly in between — 47.5 becomes 47 instead of 48. The assertion it replaced was
toBeGreaterThan(cfg.min) and toBeLessThan(cfg.max), which 47 satisfies. That
regression was previously invisible.

The same held for the sky teardown test. "dispose() stops the effect — later
SCENE mutations do not throw" was true whether the effect stopped or not. It now
captures uSkyColor, disposes, writes a new SKY_COLOR, and asserts the uniform
did not move. Commenting out stopEffect() in components/sky/index.ts makes it
red; the old version stayed green.

repoLabel and island took the same treatment for the same reason —
_applyTransform writes group.position and the island effect writes
group.visible, both unguarded — and both were verified the same way, by
commenting out their stopEffect() and watching the right test go red.
repoLabel's old assertion mutated OPACITY, which only reaches the materials
behind their guards, so it could not have caught that under any wording.

gem and footprint could not be fixed that way, and it is worth saying why.
gem's dispose() nulls edges/body and its effect body is entirely
edges?.material guards; footprint's nulls material and its effect sits
behind if (material). A leaked subscription is absorbed and looks identical
from the test. There is nothing to observe, so those two are renamed to pin what
they actually check — the guards — rather than claim a teardown check they
cannot make.

The rest of the sweep took not.toThrow from 53 sites to 16. Most were wrappers
around assertions already sitting on the next line (trees ended in
expect(refreshSpy).not.toHaveBeenCalled(), which is the spy proof the audit
asked for; streets ended in a sidewalk colour check), and the wrapper only
swallowed the stack on failure. Five construction-time cases — build before the
picker exists, poke the settings signal, assert nothing threw — now assert the
inertness instead: nothing built, group still empty.

Also strengthened: the unknown-extension hue, which asserted the hash was
reproducible and in range without saying what it was and now pins 259; and the
tree placement client's Worker fallback, which asserted Array.isArray on a
typed TreePlacement[] and now pins the empty result.

Comments

A pass over the test comments, since they had drifted the same way the
assertions had. Gone: which issue introduced a system, that a MIN_BUFFER floor
used to pad tiny bboxes and is gone, that consumers previously built
THREE.Colors inline, that the camera now lives on perFrame.rig. Where the
history was carrying the actual reason, the reason is restated in the present
tense; where it was not, it is deleted. Also gone: the ASCII banner rules and
their "Tests" headings across five component files, footprint's restatement of
its own API signature, and its "mirrors sky/island/repoLabel test style" aside.

Not done

The two files that dominate the remaining LOC are structural, not mechanical,
and do not belong in a test-only PR:

• scrubController.test.ts — 53 tests, 2,131 lines, first assertion on line
  535. That is createScrubController takes the whole world: 534 lines of scaffolding before the first assertion #161, and it is a source problem: createScrubController takes
  the whole world, so every test must build one.
• commitPane.test.tsx — 40 tests, 748 lines.

Also left, and all small:

• The other pure-function TestCase classes in test_scan.py
  (LineCountCapTests, TreeSignatureTests, ScanTreeStreamingTests). Each
  builds a different file or tree per case, so a table needs a factory per row
  and the win is thinner than build_authors_list's was.
• The TREES/BUILDING_DIMENSIONS reset block repeated in three files, and the
  one-line flush() in two hook tests.
• streetOpacity.test.ts and the small buildings/ files, which are named for
  behaviours rather than modules but are not obviously wrong where they sit.

scrubController's withCommitDateRanges — a port of the backend's
compute_commit_date_ranges — is deliberately not on that list. It should
become literal ranges, but doing it against the current shape of that file is
wasted work; it belongs with #161.

Closes nothing. #161 stays open.

🤖 Generated with Claude Code