feat: a coverage grid, so "is there an end to this" has a number (3.6.0) - #12
Conversation
All 68 tests asked whether a check fires. None asked whether it stays
quiet. That is the hole auth-2 fell through: wrong about half the time
it spoke, for three releases, green suite the whole way.
- Coverage grid: every check needs a firing fixture and a quiet fixture;
skippable checks need an n/a fixture. 83 cells, 83 filled. Computed by
running the audit over the fixtures, so it cannot drift. CI fails
below 100 percent. scripts/coverage_grid.py prints it.
- Two false negatives it found in its first hour, both missed by three
rounds of human review:
* @sentry/nextjs did not count as error tracking. The matcher knew
"sentry" and "@sentry" but not the scope prefix, so a Next.js app
with Sentry wired up correctly was told at CRITICAL it had none.
@vercel/otel and every scoped package likewise. @types/* still
correctly excluded.
* A plain migrations/ folder was never read for destructive SQL. Only
Prisma, Drizzle, Alembic and Rails were known.
- Five invariants, each with a test, so a class of defect cannot return
one instance at a time. Plus: every failing check carries a
recommendation, and no finding text uses an em or en dash.
- The stop rule, in docs/decisions/009.
114 tests.
|
Warning Review limit reached
Next review available in: 31 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Three review rounds in a row each found new defects. The question was asked plainly: is there an end to this? There was no way to answer, because nobody could say how much had been checked.
Now there is a number: 83 of 83.
The hole
All 68 tests asked one question: does this check fire?
None asked: does it stay quiet when it should?
That is the exact hole
auth-2fell through. Wrong about half the time it spoke, for three releases, with a green suite the whole way.The grid
Every check must have a fixture that makes it fire and a fixture that makes it stay quiet. Checks a profile can skip must also have a fixture that skips them.
The grid is computed by running the audit over the fixtures, not maintained by hand, so it cannot drift from the code. Add a check with no fixture and the build fails naming that check:
CI fails below 100 percent.
Borrowed from KICS, which refuses a query shipped without both a vulnerable and a safe fixture, and Checkov, which requires both a PASSED and a FAILED case.
Two false negatives it found in its first hour
Both missed by three rounds of human review. Both surfaced mechanically, by the grid demanding a passing fixture.
1.
@sentry/nextjsdid not count as error tracking.The matcher knew
sentryand@sentrybut not the scope prefix. A Next.js app with Sentry correctly wired up was told at CRITICAL that it had no error tracking at all. That is the most common error-tracker and framework pairing this tool audits.@vercel/otel,@sentry/nodeand every other scoped package were equally invisible.@types/pinostill correctly does not count as a logger. The existing suite caught that regression when I first fixed this, which is the net working.2. A plain
migrations/folder was never read for destructive SQL.The irreversible-migration check knew Prisma, Drizzle, Alembic and Rails. A
DROP TABLEin the folder that dbmate, golang-migrate, node-pg-migrate, sqlx and Supabase all use went unreported.Five invariants, each with a test
Each closes a whole class of defect, so it cannot come back one instance at a time.
InvariantsRefusesToGradeNothing(3.5.2)validate-toolkit.sh(3.5.2)CoverageGridTwo more properties now hold across every fixture: a failing check always carries a recommendation, and no finding text uses an em or en dash.
The stop rule
The engine is done when the grid is full, the invariants have tests, and CI is green. After that a review hunts for a new class of defect, not a new instance of an old one, and a review that finds nothing counts as a pass. Only a bug report from a real user reopens the engine.
Full reasoning in
docs/decisions/009, including the honest downside: fixtures are their own maintenance surface, and a fixture that misrepresents a real repo will make a check look covered when it is not. The count says a behavior is exercised. It does not say the behavior is right.Test plan
coverage_grid.py --fail-under 100exits 0, and is now a CI step./scripts/validate-toolkit.shgreen--fail-on criticalon this repo, exit 0plugin.jsonandmarketplace.json(x2), CHANGELOG entry, README states the number