From 24782e6b95351362a11ac26937595a67211fc6a7 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Mon, 13 Jul 2026 18:51:21 -0400 Subject: [PATCH] feat(gate): let capture override the test-presence diff heuristic (#3502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/gate/test-presence.test.ts | 23 +++++++++++++++++++++++ src/gate/test-presence.ts | 25 ++++++++++++++++++++++++- src/main.ts | 15 ++++++++++----- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/gate/test-presence.test.ts b/src/gate/test-presence.test.ts index 43e8430..8578a76 100644 --- a/src/gate/test-presence.test.ts +++ b/src/gate/test-presence.test.ts @@ -214,4 +214,27 @@ describe("evaluateTestPresence", () => { expect(verdict?.reason).toContain("flagged conservatively"); expect(verdict?.nextStep).toContain("test"); }); + + // A repo whose data-layer test doesn't sit beside the source or share its stem + // (e.g. `tests/pg/postgres.test.ts` for `src/db/postgres.ts`) trips the diff + // heuristic — but the run captured the new queries, proving a test ran them. + test("passes when capture reports new queries the run executed", () => { + const files = [ + changed("src/db/postgres.ts", "return db.select().from(projects).where(eq(projects.userId, id));"), + ]; + expect(evaluateTestPresence(files)).not.toBeNull(); + const verdict = evaluateTestPresence(files, undefined, { + newQueryHashes: ["f13683ee48e6a487", "53350021ed44ae14"], + }); + expect(verdict).toBeNull(); + }); + + test("still flags when capture reports no new queries", () => { + const verdict = evaluateTestPresence( + [changed("src/db/postgres.ts", "return db.select().from(projects);")], + undefined, + { newQueryHashes: [] }, + ); + expect(verdict?.dataAccessFiles).toEqual(["src/db/postgres.ts"]); + }); }); diff --git a/src/gate/test-presence.ts b/src/gate/test-presence.ts index 4330189..f695a8a 100644 --- a/src/gate/test-presence.ts +++ b/src/gate/test-presence.ts @@ -260,14 +260,26 @@ const TRIAGE_HINT = "If this change intentionally needs no test, note why on the PR so the " + "exception is auditable rather than silent."; +/** + * Capture evidence from the run, used to override the diff heuristic with what + * actually executed. `newQueryHashes` are the fingerprints this run captured + * that the baseline had not — query surface this PR introduced that a real-DB + * test ran against Postgres. Empty when there is no baseline to diff against. + */ +export interface TestPresenceCapture { + newQueryHashes: readonly string[]; +} + /** * Evaluate the gate. Returns a verdict listing the changed data-access files * that have no related data-layer test, or `null` when the PR passes — no query - * change, or every query change has a related test alongside it. + * change, every query change has a related test alongside it, or capture proves + * the change ran against Postgres. */ export function evaluateTestPresence( files: ChangedFile[], config: TestPresenceConfig = DEFAULT_TEST_PRESENCE_CONFIG, + capture?: TestPresenceCapture, ): TestPresenceVerdict | null { const { dataAccessChanged, dataLayerTestChanged } = classifyChangedFiles( files, @@ -277,6 +289,17 @@ export function evaluateTestPresence( (path) => !dataLayerTestChanged.some((test) => isRelated(path, test)), ); if (untested.length === 0) return null; + + // Capture is ground truth for the blind spot this gate exists to catch: a + // query change that runs against Postgres in no test produces no captured + // query. If this run captured new query surface, a real-DB test *did* exercise + // the change — observed execution overrides the diff heuristic's "no related + // test" guess, so don't flag. Per-query→file attribution (which would let a + // partially-tested PR still flag its one uncovered file) is the next rung, + // #3503; until then this suppresses at the run level, on the safe under-fire + // side the gate already favours. + if (capture && capture.newQueryHashes.length > 0) return null; + return { condition: "untested-data-access", verdictClass: "uncertain-conservative-flag", diff --git a/src/main.ts b/src/main.ts index 8274e9f..092acc8 100644 --- a/src/main.ts +++ b/src/main.ts @@ -211,16 +211,21 @@ async function runInCI( ); } - // Crude test-presence gate (#3496): flag data-access changes that ship with - // no data-layer test. A pure diff heuristic — independent of capture and of - // the baseline comparison, so it runs even on a brand-new PR with no - // baseline. Best-effort: a GitHub API hiccup must never sink the whole run. + // Test-presence gate (#3496): flag data-access changes that ship with no + // data-layer test. A diff heuristic, but capture overrides it (#3502): when + // the run captured new query surface, the change demonstrably ran against + // Postgres, so the "no related test" guess is dropped. Runs even on a + // brand-new PR with no baseline (then capture is empty and it's diff-only). + // Best-effort: a GitHub API hiccup must never sink the whole run. if (env.GITHUB_TOKEN) { try { const changedFiles = await fetchPrChangedFiles(env.GITHUB_TOKEN); if (changedFiles) { + const capture = reportContext.comparison + ? { newQueryHashes: reportContext.comparison.newQueries.map((q) => q.hash) } + : undefined; reportContext.testPresenceVerdict = - evaluateTestPresence(changedFiles) ?? undefined; + evaluateTestPresence(changedFiles, undefined, capture) ?? undefined; } } catch (err) { log.warn(`Test-presence gate skipped: ${err}`, "main");