feat(gate): capture overrides the test-presence diff heuristic (#3502)#175
Merged
Conversation
…502) The test-presence gate is a pure diff heuristic: it flags a data-access change when no related data-layer test changed alongside it. The "related" match is by directory or filename stem, so a repo whose test doesn't mirror the source path false-positives even when a real-DB test covers the change — e.g. Nutcracker #749, where `tests/pg/postgres.test.ts` exercises `src/db/postgres.ts` against Postgres but doesn't share its directory or stem. Capture is ground truth for the blind spot this gate exists to catch: a query that runs against Postgres in no test produces no captured query. So when the run captured new query surface, a real-DB test demonstrably ran the change — observed execution now overrides the diff guess and the flag is dropped. Suppression is run-level for now; per-query→file attribution (which would let a partially-tested PR still flag its one uncovered file) is the next rung, #3503. It errs on the safe under-fire side the gate already favours. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Query Doctor Analysis
3 queries analyzed
0 regressed · 0 improved · 0 new · 0 removed
2 pre-existing issues
SELECT "guests"."id", "guests"."session_id", "guests"."username", "guests"."avatar_path", "guests"."color", "guests"."side", "guests"."audio_recording_path", "guests"."audio_recording_public", "gue...
indexassets(event_id, inserted_at desc)
cost 31,003,449 → 1,498 (100% reduction)SELECT * FROM guest_ip_addresses WHERE ip_address = '127.0.0.1';
indexguest_ip_addresses(ip_address)
cost 154,402 → 8 (100% reduction)
Using assumed statistics (10000000 rows/table). For better results, sync production stats.
More detail → get_ci_run({ runId: "019f5dae-9758-7db3-a665-631f897d95c1" }) · view run · docs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The test-presence gate (
evaluateTestPresence) is a pure diff heuristic — it flags a data-access change when no related data-layer test changed alongside it, matching "related" by directory or filename stem. That misfires when a repo's test doesn't mirror the source path, even though a real-DB test covers the change.This teaches the gate to prefer capture over the guess: when the run captured new query surface, a real-DB test demonstrably executed the change against Postgres, so the flag is dropped.
Why
Capture is ground truth for the exact blind spot this gate exists to catch: a query that runs against Postgres in no test produces no captured query. If QD captured new queries, the diff heuristic's "no related test" conclusion is contradicted by observed execution.
Concrete case — Nutcracker #749
findByUsertosrc/db/postgres.tsand a real-DB test intests/pg/postgres.test.ts.tests/**pg**/postgres.test.tsdoesn't share a directory or stem withsrc/**db**/postgres.ts, soisRelatedmissed it.LIMIT 50shape — straight from the test'sfindByUser(u, 50), not the route's 100/500) and listed them in the very same comment that claimed nothing exercises them.With this change,
newQueryHashes.length > 0→ the verdict isnull→ the warning never posts.How
evaluateTestPresence(files, config, capture?)gains an optionalcapturewithnewQueryHashes. Non-empty → returnnull.main.tsfeedscomparison.newQuerieshashes into the gate (already computed before the gate runs).Limitation (next rung, #3503)
Suppression is run-level: a PR that tests file A but ships an untested query in file B is fully cleared for now. Per-query→file attribution is #3503. This errs on the under-fire side the gate already favours (warn-only).
Tests