The review standard that produced a first-time-working live run across 68 commits and 219 tests, extended by the Phase 2 live-preview build (33 commits, 418 tests, 23 rulings). This is reusable on any agent-driven build — the routing is project-specific, the lessons are not.
The standard, in one line
Verify by execution, not by reading. Passing tests are never evidence. Mutation testing is required wherever a fix claims to close a hole.
Added at the end of the build: trace a symptom to its single throw site and measure there, and treat a confident report as a claim to verify, not as evidence.
Added after the whole-branch review (2026-08-19): guard every mutation run against false SURVIVED and false CAUGHT, and check every instrument against an independent one — execution alone does not make a number true.
Added after Phase 2 (2026-08-25): a number is only as good as the question it was taken under, and the question drifts silently whenever the rig makes one parameter convenient — so write the parameter the rig chose for you into the caption, beside the number.
Added 2026-08-26: ask which error the real medium declines to produce before trusting a harness about a failure mode — a green suite is not evidence about anything that depends on a send failing (aposemati-loopback-blind-spot).
Why: every finding that mattered came from running something
Finding
How it was found
0.77 s machine-in-the-middle relaying a real photo
Photos silently misfiled when two equal-sized assets arrived out of order
An implementer reasoning about real orderings, then writing the discriminating test
A test named for a failure often does not catch it
This recurred across the whole build. Multiple reviews found that a test named for a failure did not actually catch that failure. Concrete instances:
racingRetransmitsOfOneAssetStoreItExactlyOncepasses under the broken TOCTOU implementation, because identical payloads make an overwrite indistinguishable from a dedupe. It is a dedupe test wearing a race test’s name.
aRecognisedDenialIsNotAlsoBlamedOnRange is vacuous — its input contains no unreachable marker, so the guard under test is never reached. It passes with the guard deleted.
bytesWithNoAnnouncementAreDroppedRatherThanMisfiled failed under none of seven mutations — a characterisation test, correctly reported as such.
A round trip through a synthesized Codable can only catch encode/decode asymmetry, because encoder and decoder are regenerated together from the same declaration. Key renames, swapped init assignments and — decisively — collapsing requestID into a computed alias of id all passed 111 tests with zero warnings. The fix is golden-JSON assertions against literal wire bytes.
The corollary
Mutate the code and re-run the named test. If the test still passes, the test is decoration. If a claimed fix cannot be shown to fail on the pre-fix base, it is not a regression guard.
A worked example of the discipline: a threshold review ran a 9-mutation matrix and found that only a value strictly inside (24.999992, 25.0) survived — an 8e-6 window. That is what “pinned” means.
Trace a symptom to its single throw site, then measure there
Two fix rounds in this project were spent on a bug a review had reported as fixed
Same bug, two different failure modes of the review process:
A confident, internally coherent report that asserted the opposite of its own error table. A fix round raised a different bound than the one that was failing, argued it well, and the field bug survived the round.
A mutation harness that produced a false negative (see below), sending the next round in the wrong direction.
The technique that finally caught it is mechanical, and cheap enough to be the default:
Take the exact string the user saw — here, the phone did not finish the handshake in time.
Find every place that string can be produced. In this case it came only from handshakeTimedOut.
Find every site that throws it. One:awaitReady(_:timeout:).
Measure at that site. It was still failing at 10.65 s against a 10 s bound. Raised to 30 s and instrumented in ed436a0.
Why this beats reading the diff
The diff of the wrong-bound fix looked correct, because it was a correct change to a bound — just not the bound in the failure path. Reading the diff would have passed it. Only the string → throw site → measurement chain distinguishes “a bound was raised” from “the bound that failed was raised”.
Corollary for dispatchers: a report is a claim. Confidence and internal coherence are not correlated with correctness, and an agent that is wrong about a premise will usually be wrong about it consistently throughout its report.
Related shape from the same build: fixing one thing created another twice — making refusal stick created an endless reject loop (e1cd333), and deferring reads to close a memory hole quietly became dropping data (86fb3f1). Both were caught by re-running the property the fix was not about.
Beware a false negative on mutation evidence
A wrong mutation result is worse than none
An early mutation run reported SURVIVES for both endianness and header size. That was a harness bug — swift test invoked with --scratch-path nested inside the copied tree. Re-run verbosely, the mutation was caught decisively (5 of 6 tests fail).
Record harness failures explicitly. A false “the test is weak” verdict sends a round of work in the wrong direction.
This turned out to be systemic, not a one-off — see the next section.
Related technique: Swift Testing 1501 supports await #expect(processExitsWith: .success) { ... }. It turns a mutation that can only be detected by a trap into a named failure with a source line, instead of a bare signal-5 with zero tests reported. Cost is one child process per run (~6 ms).
Mutation testing has two opposite failure modes
Neither was guarded against in any of this build's 14 task reviews
The whole-branch review found the branch’s entire mutation record is untrustworthy — in both directions:
False SURVIVED — the mutant was reverted, or never compiled in at all. Two mechanisms, both observed:
one reviewer’s directory-wide git checkout -- Sources Testssilently reverted another reviewer’s in-flight mutations. It happened during the review itself.
SwiftPM has a same-second mtime race: edit and rebuild inside the same second and it reuses the previous binary. You test the unmutated build and conclude the test is weak.
False CAUGHT — 21% of pristine (unmutated) runs fail under load on this repo, so an unrelated mutant looks caught by a failure it did not cause.
Both manufacture or destroy findings. Treat historical “mutant caught” claims in any ledger as unreliable unless the run was guarded.
The guard, adopted independently by two reviewers:
A canary test that passes only if the mutant is live. If the canary does not flip, the run is void — you were testing the wrong binary.
Hash the source and the built binary before and after each run. This catches both the revert and the mtime race, mechanically.
Concurrent agents must not share a git checkout
A directory-wide git checkout -- by one agent silently destroys another’s working state, and neither notices. Give each agent its own git clone --local. It is cheap, and it is the difference between a review and a race.
Verify by execution is necessary but NOT sufficient — check the instrument against an independent one
Both claims in the whole-branch review that turned out wrong were measurement artifacts, not misreadings of the code
Running something is not the same as measuring it. Every wrong number here came from an instrument that was itself wrong, and re-reading the code would never have caught any of them.
a 64-stranger flood figure that counted the attackers’ own memory as the victim’s. Fixed by splitting the harness into two processes.
a LoopbackTransport figure of 8.2 MB for 320 MB offered — caused by a harness reusing one Data buffer that copy-on-write shared across all 40 frames. A second attempt using Data(count:) was also invalid: untouched zero pages are never resident, so it showed 2.1 MB. Only distinct, fully touched buffers measure anything. The true figure is 320.8 MB.
The detection rule
Both were caught by another reviewer’s number disagreeing — never by anyone re-reading the code. A 20× disagreement between two measurements means someone has the wrong instrument; the useful response is to find out who, not to average them or to pick the more plausible one.
Corollary for memory measurement specifically: resident_size vs phys_footprint differ, and allocated-but-untouched memory is invisible to both. A harness that does not touch its buffers measures nothing at all.
The review’s own correction rate is the argument for adversarial review
Five findings were corrected by challenge rather than by their original author, and every correction held: the utun4 misclassification is not live; the POSIX 57 one-liner is worse than the bug; pairing does work on hardware; the C2 hybrid is ~115 lines rather than one; and LoopbackTransport does not have the copy behaviour claimed for it.
One reviewer diagnosed its own error precisely and it is worth repeating verbatim as a failure mode: it searched the handover for the failure, found it, and never looked for the success.
Five measurements in one phase answered a different question than the one asked
Added 2026-08-25 from Phase 2 — the phase's most transferable lesson
All five were carefully done. All five would have propagated into the phase’s conclusions. All five were caught by review rather than by a failing test — which is the other half of the lesson: a green suite is not evidence that a measurement measured the right thing.
A number is only as good as the question it was taken under, and the question drifts silently whenever the rig makes one parameter convenient. Write the parameter the rig chose for you into the caption, beside the number.
#
The measurement
What it actually answered
1
Preview interference reported as 0.24 ms, computed as p90(arrival intervals) − p90(send intervals)
Nothing. Interval spacing cancels any constant offset by construction — a still adding a uniform 50 ms to every frame leaves spacing unchanged and still reads ~0. It is also a difference of order statistics from two different distributions, which is not the p90 of anything. Measured properly (per frame, joined on sequence) the same run read send() p90 0.75 → 8.30 ms and end-to-end p90 0.84 → 9.51 ms.
2
A JPEG frame-size table used to size the whole loss projection
Taken only on the favourable side of a 2.1× downscale. On the other side — the dimension the phone probably delivers — the same ladder settles at 482,530 B with 8% headroom instead of 283,729 B with 46%. Nothing averages away high-frequency content when the source already fits.
3
Datagram loss p, as lost / (delivered × n + lost)
A denominator that omits every fragment that arrived in a frame that was later discarded — 19,915 of 39,168. A 2.03× overestimate, exactly 1 / (survival + p), largest precisely where the decision is made. It would have forced a false H.264 verdict and, by inflating survival above (1−p)^n, a false burstiness diagnosis from the same error. One arithmetic slip, two false findings.
4
A burstiness rule pointing the wrong way
Bursty loss concentrates drops in fewer frames, so MORE frames survive than (1 − p)^n predicts — the model is a lower bound and the table is pessimistic on a bursty link. The original wording said the opposite, and would have pushed a link that is better than modelled toward an expensive rewrite.
5
A memory-copy probe using distinct, fully-written buffers
Cannot distinguish copying from aliasing on the receiving side — once the sender releases its buffer, an aliasing channel retains exactly what a copying one does (27.5 MB vs 27.2 MB). The probe that discriminates repeatedly offers ONE reused buffer (31× separation) — the very method the earlier rule had ruled out.
That rule is right for measuring the SENDER’s side and wrong for measuring the RECEIVER’s. State which side you are measuring before you pick the probe.
And regardless of side: each variant must run in its OWN PROCESS. malloc reuses the previous test’s freed arena, so a second in-process measurement reads ~0.8 MB whatever the code does. Confirmed by the discriminating run: 116.08 MB retained for 100 MB offered across 1,000 distinct addresses, against 0.098 MB and one address for a deliberately aliasing control.
The general rule this phase settled on
Never subtract percentiles. Join on sequence, then take percentiles. More broadly: whenever a headline number is a difference, check whether the quantity being differenced can even contain the effect you are looking for.
Two tasks each correct in scope, jointly leaving a hole neither owned
The recurring defect shape of Phase 2, and it is invisible to per-task review by construction
A dead preview flow was permanently stuck: on the sending end nothing ever set isShutDown (transmit throws connectionFailed without calling shutDown(); the error path in the receive loop only runs after frames(), which the phone never calls), and the eviction check keys on isShutDown — so the dead channel stayed cached. The Mac retried every 2 s forever and got the same dead object back.
Task 7’s ledger deferred the sender half as “the receiving half is Task 8’s”. Task 8’s retry loop was the receiving half. Each deferral was correct in its own scope. One-line fix; found only by a whole-phase review reading the shipped state.
The same shape produced the phase’s other cross-task defect: a frameByteBudget default of 128 KB that rejected the very frames the phase existed to carry, because Task 3’s default and Task 6’s output were each reasonable alone.
The dispatcher's rule that follows
When a task defers half of a pair, name the task that owns the other half and check that it agrees. “Someone else’s” is not an owner. This is the concrete argument for a mandatory whole-phase review over and above per-task reviews.
A test double that collapses two roles hides defects that survive full review
Four mutation tests passed against a watchdog that could never fire on real hardware
LoopbackPreviewChannel makes sender and receiver the same object. The reset and stall watchdog were wired on the sending end, where the state they act on does not exist — reset() and stats are purely receive-side. Every loopback test passed. Measured on two real QUIC ends:
after 40 frames: sender delivered=0 outOfReach=0 | receiver delivered=40
sender reset(): receiver watermark unchanged
rebased peer sends 1..10: receiver delivered=40 FROZEN, receiver outOfReach=9
sender watchdog over 25 s of a fully frozen receiver: fired 0
The watchdog’s decision logic was correct in isolation and all four mutations were caught — including keying on silence alone. The logic was fine; the wiring was into a role that does not receive. delivered <= last AND outOfReach > last can never both hold when both are pinned at 0.
The rule
Any evidence about a two-role protocol must come from two distinct objects. The deciding check for the replacement was explicitly that establishedLoopbackLink() returns two distinct NetworkTransports and that the loopback double appears nowhere in the test file. State that as an acceptance criterion, not as a preference.
This is the interchangeable-implementations hazard from the Phase 1 lessons biting again in a new costume — and note that a plan defect (“the session layer must call reset() and run a watchdog”) caused it by never saying which side.
Apps/ is not a Package.swift target — add both xcodebuild invocations to the gate
The Mac app was silently broken for five commits
Through two full-suite runs and a warning-free swift build. swift test cannot reach anything under Apps/, and nothing else in Apps/ was unbuilt, so nothing signalled.
CODE_SIGNING_ALLOWED=NO keeps it out of the keychain entirely, and the .xcodeproj stays gitignored so the tree is untouched. It is the only thing that can catch this class, and it caught nothing else — which is exactly why it has to be mechanical rather than remembered.
Corollary, and it recurred: code that lives outside the test target’s reach gets reasoned about, never executed.previewEpoch, the epoch guard and the retry loop all live in Apps/. Anything further added there belongs behind a testable seam in the package.
Bounding a review explicitly produces better results than an open one
"Six-cycle only, no fuzz, no million-frame sweeps — and say what you skipped"
Verification on one task twice cost more than the implementation. Bounding the re-review that way turned a multi-hour verification into minutes without losing a single discriminating check — the six-cycle oscillation attack still ran with the climb-back fully armed and still refused every over-budget frame.
The “say what you skipped” clause is what makes it safe: a bounded review with its omissions stated is auditable, where an open review that quietly ran out of time is not.
Rulings that are worth copying, from 23 in one phase
Promote a Minor into a fix round when the report is false, not just when the code is. A report claiming “a nonsensical value gets an empty array, never a trap” was false for Int.min/Int.max (exit 133, SIGTRAP). “A report in this project states what ran; a false claim in one is worth more to fix than the trap is.” The implementer then found a third trap site the finding never named — the “guard looks right, still traps” case.
Record a skipped re-review as a deviation rather than skipping it silently. A one-clause report edit was verified by the dispatcher without a further round, and the ledger says so.
Accept and document a contradiction in your own brief rather than resolving it quietly in code. Two of this phase’s rulings do exactly that, with the measured cost of the accepted side attached.
Fix the constant, not the design. A never-decaying longestPatience of 1024 pinned the quality ladder indefinitely after six ordinary busy episodes; lowering it to 32 was the whole fix, and the convergence result survived intact. The remaining 64 s pin is documented rather than hidden.
When four mechanisms fail the same way, the mechanism is not the problem. See aposemati-peer-restart-inference-boundary — the fifth answer was to delete the mechanism.
rtk cannot be trusted for anything whose exact content matters
Session-wide finding: rtk’s diff rendering reported ”✅ Files are identical” for two files that differed on seven lines, and it truncated .gitignore when catted, hiding line 10. Use rtk proxy diff, rtk proxy git diff, rtk proxy cat for any load-bearing comparison. A regression surface confirmed with the summarising view is not confirmed.
Commit as you go
The clearest single-variable result of the whole build
Two agents were interrupted by the same machine-sleep event.
The one that had been committing incrementally lost nothing — its work survived as untracked files, it re-verified state itself, and it committed and reported.
The one that had not lost an entire fix round, and a dispatched review was lost outright (the agent vanished from the list and never reported).
“Commit as you go” was added to every dispatch after the second sleep-kill. Task 12 subsequently shipped across four incremental commits and survived.
Other process rules that earned their place
Assert the test count, never the exit code.swift test --filter <NoSuchThing> exits 0 and reports “0 tests passed”.
Scope review diffs precisely. A controller error handed a reviewer a range that excluded the task’s own security fix; the reviewer caught it and reviewed both states separately. Where two agents’ commits are cleanly separated by path, review a path-scoped diff rather than a commit range.
Report provenance honestly. The standard set here: “everything quoted above is output I captured from runs in this session… Nothing here is reconstructed from memory.”
Record controller errors in the ledger. Two are recorded verbatim in this build’s 419-line ledger, including the one that led to shipping a defeatable SAS. A ledger that only records successes is a marketing document.
Escalate rather than rule when a decision touches something the user has lost trust in. The keychain-vs-in-memory identity decision was escalated for exactly this reason, and produced a better answer than the plan had.
Defer minors deliberately, and count them. 27 deferred minors are enumerated in the ledger with their reasoning, waiting for the final whole-branch review — not forgotten, not silently fixed.
✅ A per-task review cannot see across tasks — and the whole-branch review proved it. Every review in this build was scoped to a single task’s diff, which makes cross-task inconsistencies structurally invisible. That is why a final whole-branch review over all 68 commits at once is a separate, mandatory step rather than a formality, and why it belongs in a fresh session with the handover doc and the ledger as its only inputs. It paid: it solved the POSIX 57 nobody could explain, found a 250 ms tolerance nobody had chosen, refuted an exploit claim, and struck seven deferred minors as disproved — none of which any single-task diff could have surfaced.
Interchangeable implementations are a property to re-verify, not a fact.LoopbackTransport and NetworkTransport were checked for interchangeability four times; the one round where they diverged (86fb3f1) was caught only because someone re-ran the comparison after an unrelated fix. Update 2026-08-19: the property is being retired, not restored — the fix that was supposed to preserve it only holds to 256 KB, and honouring it fully is mutually exclusive with the memory bound. The honest move was to write the divergence down as documented API, not to keep re-asserting a promise the code cannot keep.
Record what you could not explain. An intermittent POSIX 57 during pairing was reproducible under a flood harness on first run, cause unknown, noted independently by two reviewers — written down rather than dismissed as flake. Nine months of “flake” would have hidden it; instead the whole-branch review solved it in one pass (⭐ POSIX 57 is solved — the branch’s one unexplained defect). Writing down what you cannot explain is what makes it findable later.
A green suite that is not deterministically green is not a signal. 3 of 14 pristine runs on this repo failed under load — 21%. CI in that state cannot distinguish a regression from noise, and every mutation verdict taken under it is suspect.
A constant pinned only by a test that reads it is not pinned. Nine tuned constants here — several of them outputs of field measurement — are asserted against by tests that fetch the same constant. The suite structurally cannot disagree with any of them.
Count your own deferred minors. The handover said 27; the ledger held 29; the last four commits had no rulings at all. Seven of the 29 were then disproved outright by measurement — a deferred-minors list decays into fiction unless something eventually executes against it.
2026-08-26 — a session’s worth of process findings
⭐ The harness can only reproduce the quiet half of this defect class
This is why 546 tests, eight task reviews and a whole-branch review all missed a defect a
plugged-in phone found in ten minutes.
Over awdl0, a QUIC datagram send into a dead peer succeeds — the sender learns nothing. Over
loopback the same send fails immediately with
POSIXErrorCode(rawValue: 57): Socket is not connected. The harness takes the error path the
field never takes, so a test written for the defect passes against unmodified code: loopback
hands the sender the very error the radio withholds.
Form of the defect
Reproducible on loopback?
Quiet — the peer leaves while nothing is being sent
✅ yes
Live sender — 30 fps of successful sends into a dead peer
❌ not at all
The rule to carry forward
Anything that relies on send failure to detect peer death is untestable in-harness. It must be
reasoned about, or found on hardware. Do not read a green suite as evidence about it, and do not
let a passing test written for it count as coverage.
This is stronger than A test double that collapses two roles hides defects that survive full review
above. That lesson says a badly shaped double hides defects. This one says a faithful
two-process QUIC harness on loopback still cannot see this class — the double is not the problem,
the medium is. The generalisation: before trusting a harness about a failure mode, ask which
error the real medium declines to produce.
The mutation was run by hand, because this project has a precedent for a disproved fix
Commit 371b32a is the precedent: a fix later disproved because its test passed identically
with and without the change. Together with the two opposite mutation failure modes above, that
makes a reported mutation result a claim, not evidence.
So the fix for the viewfinder freeze was mutated by hand — revert only the source change, keep the
tests:
send-only flow after its peer went away: shut down false
Expectation failed: (noticed → nil) == true
send-only flow reopening: peer offered a replacement, it handed back the dead one
Expectation failed: (reopened) !== (accepted)
Both fail after ~15 s of timeout; with the fix restored both pass in 0.073 s. The second line
also independently confirmed a second defect — the transport cache was handing back a dead
channel — which no amount of reading the diff would have established.
Naming a mechanism before measuring it
Four mechanisms were asserted confidently while debugging a laggy preview. Three were wrong,
each dying on contact with a measurement:
claimed
reality
low light lowering the capture rate
bright light moved it 1–2 fps
zero-shutter-lag forcing full-sensor readout
disabling it changed nothing
a frame stream hanging on a dead flow
disproved by mutation test — the receive loop already ends it
.photo picking a 17 fps format
correct — found by printing the format table
The one that was right was found by printing something, not by reasoning. The pattern is old on
this project: every finding it is proud of came from someone reading a number.
Postscript, later the same day: the fourth row was right after all — but for a narrower reason
“A frame stream hanging on a dead flow” was correctly disproved as stated. The real mechanism sat
one layer over: the channel had no teardown path at all on the sending end, because a QUIC
datagram send into a dead peer succeeds over the radio. A disproved mechanism is not a disproved
symptom — see aposemati-loopback-blind-spot. And it too was found by printing something: four
lines of Mac-side log showing a clean 32-second retry cycle.
A plan’s confident prose stops the next reader checking
An implementation plan contained six defects that only appeared when someone tried to build from
it. Three shipped with prose defending the wrong thing — which is worse than saying nothing,
because a justification reads as evidence that someone already thought it through.
The two worst: a cancel() that would have dropped the QUIC link
(Measured 2026-08-26 — cancelling a send tears down the link), and
a drag gesture that compounds because translation is cumulative. Both were caught in review,
neither by the author.
Reading the next task's code before dispatching it caught six defects
Cheap insurance: undercounted call sites, a test double that was private in the wrong file and
non-copyable besides, a missing ceiling clamp, an unstated ordering constraint, and the two above.
Tools that lie
Third and fourth instances on this project:
rtk’s summarising returned zero matches on a grep where the raw tool found the two lines
that were the entire answer. Also once reported two differing files as identical. Use
rtk proxy <cmd>.
A timing test that flakes makes every green run untrustworthy
sendCostIsMeasuredAtBothTheLoopbackAndTheRadioSizedDatagram asserts a hard 50 ms p90. With an
unrelated Rust build saturating the machine — three rustc processes at 646/180/80% CPU, load
average 38.84 — it read 176 ms; alone minutes later, 30 ms. It cost three re-runs in one
session to separate “the machine is busy” from “something broke”. Tag it out of the default suite.
The same test has a SECOND, scarier signature — added 2026-08-26
Under load it also fails with handshakeTimedOut after 34 seconds — a link that never
established at all, which reads like a transport regression, not a timing flake. Nothing about
that failure says “the machine was busy”.
Run alone on the same machine it passed in 6.1 s, and a second full-suite run passed all
548. Both signatures are recorded here so the next person does not chase the wrong thing —
seeing handshakeTimedOut from this test is not, by itself, evidence that the transport broke.
This is the same discipline as record what you could not explain above, applied to a test that
is explained: a flaky test with two distinct failure modes needs both written down, because
documenting only the benign one teaches the next reader to dismiss the alarming one.
Tests that pass without the thing they test
Two caught by mutation this session: a clamp that changed the output in ~112,000 swept cases while
all 13 tests passed with it deleted, and an “empty rectangle” test whose numbers never reached
the floor it was named after. Break the code, watch the test go red, restore it. A test written
for an untested branch that nobody watched fail is just another assertion.