A Flaky-Test Detector for CommitBrief: Catching AI-Written Flaky Tests at Commit Time
I added a deterministic flaky-test filter to CommitBrief. Instead of asking the LLM, it catches hard-coded sleeps and unseeded randomness in changed test files at commit time — no model. Here's why I chose static analysis over an LLM prompt.
Until now CommitBrief did one thing: have an LLM review my own diff before anyone else saw it. The earlier post covered why that helps; the summary-and-remote post covered moving it to the team. They shared one trait: the finding was produced by the model.
This addition deliberately breaks that. Before it calls the model, CommitBrief now runs changed test files through a deterministic static filter — to catch the patterns that most often make a test flaky. The real story isn’t the feature; it’s why I didn’t build it as one more line in the prompt.
I wrote about why flakiness exploded in the AI era, and why retry/quarantine is a treadmill, separately on sade.dev: Flaky Tests: Retrying Isn’t a Fix. This post is the tooling side of that argument.
The easy path was the LLM — I didn’t take it
CommitBrief is an LLM-driven tool end to end. The easy path was obvious: add a rubric to the prompt — “if a test file has a fixed sleep, a brittle selector, unseeded randomness, flag it.” An evening’s work.
The problem is that the result is probabilistic. It depends on the model, it can drift run to run, and — more importantly — it isn’t differentiated: any LLM reviewer can be prompted to do the same thing. But the maddening thing about flakiness is precisely that you want the fix to be deterministic: same diff, same result every time, working even without a model, returning in a second at the commit boundary.
So I took the static route. A new package (internal/flaky) scans only the added lines of changed test files, matches the known anti-patterns with conservative regexes, and emits a standard finding. Same diff → same finding. No model, no network.
What it catches
The first release ships two high-precision rules. Precision comes before recall: a noisy gate at commit time gets ignored, and an ignored gate is worse than none.
- Hard-coded sleeps / fixed waits (
medium):time.Sleep,Thread.sleep,Task.Delay,asyncio.sleep,*.waitForTimeout, numericcy.wait(2000),usleep,sleep(<n>)— Go, Java/Kotlin, C#, Python, JS/TS, Cypress, PHP. - Unseeded randomness (
low):Math.random, Pythonrandom.*, Gomath/randglobals (rand.Intn, …).
It recognizes a test file by convention (*_test.go, *.test.*/*.spec.*, test_*.py, *Test.java, *_spec.rb, __tests__/e2e/cypress dirs…). Finding text is localized (en/tr). The output is the same card as a model finding:
● medium worker/job_test.go:41
Test uses a hard-coded sleep for synchronization
Fixed sleeps make a test timing-dependent: it passes on a fast machine
and fails under CI load, producing flaky results that erode trust.
→ Replace the fixed delay with a condition-based wait…
How it merges with the model’s findings
The static findings are computed after filtering and merged with the LLM findings — before render, --fail-on, and --copy. Two rules:
- Every LLM finding is preserved. The static filter never suppresses anything.
- On a
file:linecollision the static finding is dropped — if the model already has something to say about that line, we don’t say it twice.
A nice side effect: even if the model returns malformed JSON and degrades, the static findings survive. The deterministic part doesn’t depend on the probabilistic one.
And JSON schema v1 didn’t change. Static findings use the existing render.Finding shape — no new field, no new exit-code surface. --fail-on=medium now also closes the gate on a hard-coded sleep, with no extra flag to learn.
Using it
On by default for API/mock providers. To turn it off:
commitbrief # flaky pre-pass on (default)
commitbrief --no-flaky # skip for this run
commitbrief config set review.flaky false # disable persistently
Precedence is --no-flaky > review.flaky > built-in (true). CLI-backed providers (claude-cli/gemini-cli) emit plain text, so it’s inert there for now — I’ll handle that path separately.
Not “replacing,” still “going ahead of”
This addition orbits the same sentence as the rest of the series: if producing got cheap, scrutinizing what’s produced shouldn’t stay expensive. AI made writing tests free; catching the obvious flakiness in those tests should be free too — without a model call or a CI minute, right where the commit happens.
Here the model becomes a luxury, not a requirement: the deterministic part collects the obvious, and I save the model’s attention for the things that actually need judgment.
Limits — the honest list again
- Precision-first. It catches obvious anti-patterns, not a race condition buried in your own code. It misses the clever flake, and I don’t let it guess.
- CLI-provider path is out of scope. No merge yet for plain-text providers; a follow-up.
- Static, not AST (yet). Regex-based; it doesn’t see scope or semantics. Brittle selectors, over-mocking, and time-dependent assertions are the next additive rules.
- Detection is a backstop, not the cure. The real fix is writing the test deterministically in the first place — which is the sade.dev piece.
Try it
Drop a fixed sleep into a test file and run commitbrief — watch it get caught deterministically, before the model. If you tell me which anti-pattern you want next, I’ll queue it.
Comments
Sign in with your GitHub account to join the discussion. Comments are stored in GitHub Discussions.