From 8b8e26a5a69725a37ffe3b3976db1f717a95139e Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Tue, 4 Aug 2026 19:35:56 -0300 Subject: [PATCH] fix(reporters): stop prescribing a workflow change on a first run One template branch served both a project whose workflow lacks a push trigger and a correctly configured project on its first run, so the second was told to fix something that was already right. An unset comparison branch also rendered as an empty name, producing "push: branches: []". Closes Query-Doctor/Site#3841 --- src/main.ts | 8 ++----- src/reporters/baseline-notice.test.ts | 28 +++++++++++++++++++++++ src/reporters/baseline-notice.ts | 24 ++++++++++++++++++++ src/reporters/github/github.test.ts | 32 +++++++++++++++++++++++++-- src/reporters/github/success.md.j2 | 6 ++++- 5 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 src/reporters/baseline-notice.test.ts create mode 100644 src/reporters/baseline-notice.ts diff --git a/src/main.ts b/src/main.ts index 79d9086..d66629b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -28,6 +28,7 @@ import { ApiClient } from "./remote/api-client.ts"; import { Remote } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; import { PgbadgerSource } from "./sql/pgbadger.ts"; +import { baselineNotFoundMessage } from "./reporters/baseline-notice.ts"; import type { RecentQuerySource } from "./sql/recent-query.ts"; import type { FullSchema, RepoPolicyConfig } from "@query-doctor/core"; @@ -189,12 +190,7 @@ async function runInCI( if (result.kind === "found") { previousRun = result.run; } else if (result.kind === "not-found") { - log.info( - "main", - `No baseline found on branch "${comparisonBranch}". Comparison will be skipped. ` + - `To establish a baseline, run the analyzer on pushes to "${comparisonBranch}" ` + - `(add "push: branches: [${comparisonBranch}]" to your workflow trigger).`, - ); + log.info("main", baselineNotFoundMessage(comparisonBranch)); } else { // Transient fetch failure after retries — flag it so the comment says // "temporarily unavailable, re-run" rather than claiming there is no diff --git a/src/reporters/baseline-notice.test.ts b/src/reporters/baseline-notice.test.ts new file mode 100644 index 0000000..ae58fa2 --- /dev/null +++ b/src/reporters/baseline-notice.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, test } from "vitest"; +import { baselineNotFoundMessage } from "./baseline-notice.ts"; + +describe("baselineNotFoundMessage", () => { + test("names the branch and the trigger that records a baseline", () => { + const message = baselineNotFoundMessage("main"); + + expect(message).toContain("main"); + expect(message).toContain("push: branches: [main]"); + }); + + // The branch resolves from the configured comparison branch, then the pull + // request base, then the current branch. On a push run for a project with + // none set, all three can be absent, which is how this reached a real log. + test.each([ + ["empty", ""], + ["undefined", undefined], + ["null", null], + ])("never names a branch it does not have: %s", (_case, branch) => { + const message = baselineNotFoundMessage(branch); + + // What the real log read: `pushes to ""` and `push: branches: []`, which + // matches no branch and reads as an instruction the user cannot follow. + expect(message).not.toContain('""'); + expect(message).not.toContain("branches: []"); + expect(message).toContain("No comparison branch is set"); + }); +}); diff --git a/src/reporters/baseline-notice.ts b/src/reporters/baseline-notice.ts new file mode 100644 index 0000000..f2ad3ef --- /dev/null +++ b/src/reporters/baseline-notice.ts @@ -0,0 +1,24 @@ +/** + * What to log when no baseline exists for the comparison branch. + * + * The branch is resolved from the project's configured comparison branch, then + * the pull request base, then the current branch, so it can come out empty on a + * push run for a project that has not set one. Interpolating that empty string + * produced `pushes to ""` and `push: branches: []`, which matches no branch at + * all and reads as a broken instruction. + */ +export function baselineNotFoundMessage( + comparisonBranch: string | undefined | null, +): string { + if (!comparisonBranch) { + return ( + "No comparison branch is set, so nothing was compared. " + + "Set one in the project's CI settings, then push to it once so a baseline is recorded." + ); + } + return ( + `No baseline found on branch "${comparisonBranch}". Comparison will be skipped. ` + + `A baseline is recorded when the analyzer runs on a push to "${comparisonBranch}"; ` + + `if the workflow has no push trigger for it, add "push: branches: [${comparisonBranch}]".` + ); +} diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index 8fca354..06fca97 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -648,10 +648,38 @@ describe("baseline absent vs. temporarily unavailable (Site#3287)", () => { const output = renderTemplate(ctx); expect(output).toContain("No baseline on `staging`"); - expect(output).toContain("add a `push` trigger"); + // Still points at the push trigger, now as a condition rather than an + // instruction, because a correctly configured first run lands here too. + expect(output).toContain("`push` trigger"); expect(output).not.toContain("temporarily unavailable"); }); + // "" is what a first push run on a project with no configured branch actually + // produces: main.ts resolves the configured branch, then the PR base, then the + // current branch, and on that path all three can be absent. + test.each([ + ["absent", {}], + ["empty", { comparisonBranch: "" }], + ])("never names a branch it does not have: %s", (_case, overrides) => { + const output = renderTemplate(makeContext(overrides)); + + // An empty inline code span is the shape of the bug — `{{ comparisonBranch }}` + // interpolated to nothing, leaving "No baseline on ``" above instructions + // for a branch that was never named. + expect(output).not.toMatch(/``/); + // "is set", not just "No comparison branch": the unset-baseline warning + // lower in the template opens "No comparison branch configured", so the + // looser string would pass on the wrong block. + expect(output).toContain("No comparison branch is set"); + }); + + test("names the branch and keeps the push-trigger guidance when it has one", () => { + const output = renderTemplate(makeContext({ comparisonBranch: "staging" })); + + expect(output).toContain("No baseline on `staging`"); + expect(output).toContain("`push` trigger"); + }); + test("transient fetch failure renders a re-run message, not the no-baseline copy", () => { const ctx = makeContext({ comparisonBranch: "staging", @@ -663,7 +691,7 @@ describe("baseline absent vs. temporarily unavailable (Site#3287)", () => { expect(output).toContain("re-run the check"); // Must not tell the user to add a trigger that is already in place. expect(output).not.toContain("No baseline on `staging`"); - expect(output).not.toContain("add a `push` trigger"); + expect(output).not.toContain("`push` trigger"); }); }); diff --git a/src/reporters/github/success.md.j2 b/src/reporters/github/success.md.j2 index 16177f3..86507cb 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -50,7 +50,11 @@ Comparison temporarily unavailable. {% else %} No baseline found to compare against. -> **No baseline on `{{ comparisonBranch }}`** — the analyzer cannot detect regressions without a previous run. To establish a baseline, add a `push` trigger for your comparison branch so the analyzer runs on merges to `{{ comparisonBranch }}`. See the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger) for setup instructions. +{% if comparisonBranch %} +> **No baseline on `{{ comparisonBranch }}`.** The analyzer records a baseline when it runs on a push to `{{ comparisonBranch }}`, and compares later runs against it. If this run is the first, the next one has something to compare against. If the workflow has no `push` trigger for `{{ comparisonBranch }}`, add one: see the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger). +{% else %} +> **No comparison branch is set**, so nothing was compared. Set one in the project's CI settings, then push to it once so the analyzer records a baseline. See the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger). +{% endif %} {% endif %} {% if hasComparison and runMetadata and runMetadata.baseline and runMetadata.baseline.unset %}