From d1ddbf0f3c5ebd0bcfb05c76484390d6ced285bc Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Wed, 22 Jul 2026 17:05:20 -0300 Subject: [PATCH] feat(gate): route cost-regression blocking through the policy engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cost-regression already blocks a PR, but through the analyzer's old inline path, so a repo couldn't set it to warn/off the way it can for untested-data-access (#183) and schema-drift (#188). Route the regression arm through resolveVerdict/policyFor, matching those two conditions. regression-beyond-threshold is a finding in the shared taxonomy and defaults to fail, so a repo that sets nothing blocks exactly as before. warn caps it at a surfaced, non-blocking neutral; off suppresses it. The new-query gate is left on its inline setFailed: its default policy is warn, so routing it through resolveVerdict would silently stop it blocking — a behavior change that needs a separate human sign-off. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/gate/regression.test.ts | 24 +++++++++++++++++++++++ src/gate/regression.ts | 39 +++++++++++++++++++++++++++++++++++++ src/main.ts | 30 ++++++++++++++++++---------- 3 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 src/gate/regression.test.ts create mode 100644 src/gate/regression.ts diff --git a/src/gate/regression.test.ts b/src/gate/regression.test.ts new file mode 100644 index 0000000..d923c74 --- /dev/null +++ b/src/gate/regression.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { gateRegression } from "./regression.ts"; + +describe("gateRegression", () => { + it("blocks untriaged regressions by default", () => { + expect(gateRegression(2)).toEqual({ conclusion: "failure" }); + }); + + it("softens to a non-blocking warning under a warn policy", () => { + expect( + gateRegression(2, { "regression-beyond-threshold": "warn" })?.conclusion, + ).toBe("neutral"); + }); + + it("drops the gate under an off policy", () => { + expect( + gateRegression(2, { "regression-beyond-threshold": "off" }), + ).toBeNull(); + }); + + it("passes when there are no regressions", () => { + expect(gateRegression(0)).toBeNull(); + }); +}); diff --git a/src/gate/regression.ts b/src/gate/regression.ts new file mode 100644 index 0000000..a75e7b5 --- /dev/null +++ b/src/gate/regression.ts @@ -0,0 +1,39 @@ +// The cost-regression gate. `compareRuns` already produces the untriaged, +// beyond-threshold regression set (`comparison.regressed`, filtered by +// `acknowledgedQueryHashes` and `regressionThreshold`); this gate only decides +// the check outcome from its size and the repo policy. It blocks by default — +// unchanged from the old inline path — but now routes through the shared policy +// engine (#3500), so a repo can soften it to `warn` or `off` the way it can for +// untested-data-access and schema-drift. Detection is not this gate's job. + +import type { RepoPolicyConfig } from "@query-doctor/core"; +import { resolveVerdict } from "./policy.ts"; + +export interface RegressionGateResult { + /** `failure` blocks the check; `neutral` is a surfaced, non-blocking warning. */ + conclusion: "failure" | "neutral"; +} + +/** + * Decide the regression gate from the untriaged regression count and the repo + * policy. Returns the check outcome when there are regressions and the policy + * surfaces them, or `null` when there are none or the policy is `off`. + * + * `regression-beyond-threshold` is a `finding` in the shared taxonomy (concludes + * `failure`) and defaults to `fail`, so a repo that sets nothing blocks exactly + * as before. This is a second, repo-wide override layer on top of per-query + * triage: `off` suppresses the condition regardless of triage, `warn` surfaces + * every untriaged regression without blocking, `fail` blocks them as today. + */ +export function gateRegression( + regressedCount: number, + config: RepoPolicyConfig = {}, +): RegressionGateResult | null { + if (regressedCount <= 0) return null; + const { conclusion, surfaced } = resolveVerdict( + { condition: "regression-beyond-threshold", verdictClass: "finding" }, + config, + ); + if (!surfaced || conclusion === "success") return null; + return { conclusion }; +} diff --git a/src/main.ts b/src/main.ts index 696e2ce..521d649 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,6 +17,7 @@ import { import { formatCost, queryPreview } from "./reporters/github/github.ts"; import { fetchPrChangedFiles } from "./gate/changed-files.ts"; import { evaluateTestPresence } from "./gate/test-presence.ts"; +import { gateRegression } from "./gate/regression.ts"; import { resolveVerdict } from "./gate/policy.ts"; import { DEFAULT_CONFIG } from "./config.ts"; import { ApiClient } from "./remote/api-client.ts"; @@ -288,20 +289,33 @@ async function runInCI( return queryLink ? `\n ${queryLink}` : ""; }; - const blockingMessages: string[] = []; + const policyConfig: RepoPolicyConfig = + ("conditionPolicies" in config ? config.conditionPolicies : undefined) ?? + {}; const { regressed, newQueries } = reportContext.comparison; - if (regressed.length > 0) { + + // Regression arm routed through the shared policy engine (#3500), so a repo + // can soften it to warn/off like untested-data-access and schema-drift. + // `regressed` is already the untriaged, beyond-threshold set; the policy is + // a second, repo-wide layer on top of per-query triage. Default `fail`, so + // a repo that sets nothing blocks exactly as the old inline path did. + const regressionGate = gateRegression(regressed.length, policyConfig); + if (regressionGate) { const messages = regressed.map((q) => { const preview = queryPreview(q.formattedQuery); const cost = `cost ${formatCost(q.previousCost)} → ${formatCost(q.currentCost)} (+${q.regressionPercentage.toFixed(1)}%)`; return ` - ${preview}: ${cost}${linkFor(q.hash)}`; }); - blockingMessages.push( - `${regressed.length} untriaged regression(s) beyond threshold:\n${messages.join("\n")}`, - ); + const message = `${regressed.length} untriaged regression(s) beyond threshold:\n${messages.join("\n")}`; + if (regressionGate.conclusion === "failure") core.setFailed(message); + else core.warning(message); } + // New-query arm stays on its inline setFailed. Its default policy is + // `warn`, so routing it through resolveVerdict would silently stop it + // blocking — a behavior change that needs a human sign-off, tracked + // separately from this regression-only task. const gateNewQueries = gateEligibleNewQueries( newQueries, config.regressionThreshold, @@ -318,14 +332,10 @@ async function runInCI( const detail = `cost ${formatCost(opt.cost)}, index recommendation cuts it ${opt.costReductionPercentage.toFixed(1)}%`; return ` - ${preview}: ${detail}${linkFor(q.hash)}`; }); - blockingMessages.push( + core.setFailed( `${gateNewQueries.length} new quer${gateNewQueries.length === 1 ? "y" : "ies"} ship${gateNewQueries.length === 1 ? "s" : ""} with a high-impact index recommendation (acknowledge on the dashboard to allow):\n${messages.join("\n")}`, ); } - - if (blockingMessages.length > 0) { - core.setFailed(blockingMessages.join("\n\n")); - } } } finally { remote.off("schemaSynced", onSchemaSynced);