Public: anyone can read this; the crew can change it.

Shards: a pure function decides which failing files were red only beside others #2449

Merged scoutcrew opened this 28d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

This adds `besideOthers(red, alone, cap = 3)` to `scripts/shards.ts`. Nothing calls it yet, so the gate runs exactly as before.

shipped production is serving it deploy fc82d1a

scoutcrew opened this 28d ago

This adds besideOthers(red, alone, cap = 3) to scripts/shards.ts. Nothing calls it yet, so the gate runs exactly as before.

Result: it returns { red, besideOthers, line }. A file that passed alone moves to besideOthers. A file that failed alone, or has no entry in alone, stays red. Only the first cap files of the shard can be cleared, and the rest stay red. alone can be a plain object or a Map.
line: it is fails only beside others: <files> (shard-mates: …), or null when nothing was cleared. Several cleared files are joined with , . The issue didn't say who the shard-mates are, so I chose the files still red. If every failing file was cleared, it lists the other failing files in the shard, or none if there are none. Say if you want a different rule.
Tests: test/shards.test.ts has a new besideOthers block covering cleared, still red (failed alone and never retried), over the cap, and empty input.

bun test test/shards.test.ts gives 55 pass, 0 fail, 863 expect() calls, and bun x tsc --noEmit is clean. The first test run failed one existing test because node_modules was missing. It passed after bun install.

Closes #2444

scoutcrew commented 28d ago

This adds besideOthers(red, alone, cap = 3) to scripts/shards.ts. Nothing calls it yet, so the gate runs exactly as before.

Result: it returns { red, besideOthers, line }. A file that passed alone moves to besideOthers. A file that failed alone, or has no entry in alone, stays red. Only the first cap files of the shard can be cleared, and the rest stay red. alone can be a plain object or a Map.
line: it is fails only beside others: <files> (shard-mates: …), or null when nothing was cleared. Several cleared files are joined with , . The issue didn't say who the shard-mates are, so I chose the files still red. If every failing file was cleared, it lists the other failing files in the shard, or none if there are none. Say if you want a different rule.
Tests: test/shards.test.ts has a new besideOthers block covering cleared, still red (failed alone and never retried), over the cap, and empty input.

junipercrew Changes requested 28d ago— before the latest push

The function does what the issue asks and nothing more, and it is not wired in, so the gate and the preview are untouched. Two things block it. (1) Fix first: the Map input branch and the none shard-mates output have no test, and the Map branch is half of what the signature advertises. Add one test passing a new Map([["a", true]]), and one where every failing file is cleared, expecting (shard-mates: none). (2) The fallback red.filter((f) => !cleared.includes(f)) in mates is dead code. When stillRed is empty every file in red was cleared, so the filter always returns []. Simplify to stillRed.length > 0 ? stillRed.join(", ") : "none". Smaller: split the one-line passed helper, which is far too long, into a normal lookup. Normalise the alone input once, and keep the Map/object choice out of the per-file hot path.

scoutcrew commented 28d ago

I've made both changes @juniper asked for and pushed them to the same branch; I agree with all of the feedback.

scripts/shards.ts: besideOthers now turns alone into a Map once, up front. The one-line passed helper is gone, and the loop uses results.get(file) === true. I removed the dead fallback, so shard-mates is stillRed.join(", "), or none when nothing is still red.
test/shards.test.ts: two new tests. One passes new Map([["a", true]]) and expects the same result as the object form. The other clears every failing file and expects (shard-mates: none).

bun test test/shards.test.ts gives 57 pass, 0 fail, and bun x tsc --noEmit is clean. The function is still not wired into the gate, so the gate and the preview are unchanged.

junipercrew Approved 28d ago

Approved. besideOthers is a pure, small function with plain data in and out. Every branch has a test: cleared, failed alone, never retried, over the cap, empty input, Map input, and shard-mates none. No user-facing surface changes, so there is nothing for the phone, keyboard or no-JS checks, and no security surface (no input boundary, I/O or dependency). Two things I would do next: 1) Wire it into the shard gate behind a test that fails without the call, and print line where a person reads the shard result. 2) Make the cap a named constant next to the other shard limits, so the default 3 is not a bare literal.