flaky: two load-related test flakes in the suite that gates every deploy

[OsherElhadad]

Two intermittent failures, both surfacing only under concurrent load. One investigation, not two mysteries found separately six weeks apart.

Neither is caused by the code under test. Both surfaced while a dozen agents ran proxies on one box at load average 10–14 — a condition normal CI does not reproduce, which is why they have gone unnoticed.

1. dash/TestEventsDerivesItsScopeFromTheResolver

Fails on a full go test ./... competing with another concurrent run.

• 5/5 clean isolated, 30/30 clean under -count=30 -cpu=1, green on a solo package run.
• Reproduced on a commit whose only dash change was a comment, so it cannot reach /api/events.
• Cause: timing-sensitive SSE test — it asserts on whatever the recorder has flushed at the moment it looks. The dash package takes ~356s, so under contention the flush it needs has not landed.

2. TestDashboardAddsNoRequestLatencyWithContentCapture

Established by interleaved runs against origin/main, so it predates and is independent of any current branch.

• Cause: a latency assertion measured on a machine already saturated. It compares request latency with content capture on against a budget that assumes the box is not competing with a dozen proxies.

Suggested fix

Both want an explicit synchronisation point rather than an implicit one:

• For (1): poll for the expected event with a deadline, the way wait_for_rows already does for the request table, instead of asserting on the first read.
• For (2): the budget needs to be either load-relative or the measurement taken under a quiescence check. Raising the constant just moves the flake.

Sleeping longer fixes neither.

[OsherElhadad]

Addendum: the statistical argument, and it changes the fix for (2)

The reachability analysis for (2) TestDashboardAddsNoRequestLatencyWithContentCapture holds — codesafe is [format, dedup, failed_run, cmdfilter, extract, collapse, cachesplit] (config/config.go:189, and again in its full doc at :223), with no extract_llm, so the reachable delta is one closure allocation, one nil check, and a fourth DefaultPinPrefixes entry making isPinPrefix do up to four HasPrefix compares instead of three. Nanoseconds against a max(5ms, 10% of baseline) budget.

But the decisive point is statistical, not mechanical: 2-of-7 versus 0-of-7 is not evidence of a difference. Fisher exact one-sided p = 0.23. At a 15% true flake rate, P(a clean arm shows 0 of 7) = 0.32 — a clean run is the expected outcome even if both sides flake identically. The interleaved-runs experiment that established "it is not branch-caused" was never powered to answer that question; it reached the right conclusion for a weaker reason than it appeared to.

So (2) does not want a sync point — it wants a robust budget. Measure the baseline in the same run so both sides see the same machine load, or skip the assertion above a load threshold. A latency budget calibrated on a quiet box is a flake generator on a busy one and will keep firing regardless of what any branch does. Raising the constant only moves it.

The sync-point recommendation still stands for (1) dash/TestEventsDerivesItsScopeFromTheResolver, which is a genuine read-whatever-has-been-flushed race. Two tests, two different fixes, one shared cause.

General lesson worth recording

Check the noise floor before concluding a difference exists. This is the third time in this project that "n too small" was the real answer:

• these two flakes, where 2-of-7 vs 0-of-7 read as a branch effect;
• a retracted floor finding at n=2, where a 1.43 pp arm gap sat inside a 2.01 pp within-arm spread;
• a per-session replay figure that looked like two reviewers disagreeing and turned out to be two definitions of the same quantity.

Worth a line in whatever guidance covers benchmark and flake triage.