From 32b85e2a4edf77f22d35609418daa1e30b704351 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Mon, 14 Sep 2026 23:55:59 -0300 Subject: [PATCH] fix(reporters): count a warning in the comment's heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The heading was assembled in the template as "{failing} failing and {successful} successful check(s)", which has no place for a warning: a run with 1 FAIL, 1 WARN and 1 PASS announced two of its three rows. It reads "1 failing, 1 warning and 1 successful check", built in the view model where the join and the plural are readable. An outcome nothing concluded is left out rather than counted as a zero. Site's platform renders this same comment for repositories on its ingest path, and both post onto one pull request, so the two headings match again (Site #4201). Not identically: Site appends "· N not run", which this summary has no field for, and the heading's doc comment says so rather than claiming parity. --- src/reporters/github/github.test.ts | 13 ++++++++++ src/reporters/github/github.ts | 38 ++++++++++++++++++++++++++++- src/reporters/github/success.md.j2 | 2 +- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index 9ad67f5..1bd52a7 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -1110,6 +1110,19 @@ describe("heading wording", () => { expect(output).not.toContain("0 failing"); }); + // A run carrying a warning announced fewer checks than it printed (Site #4201). + test("counts a warning beside the rest", () => { + const ctx = makeContext({ + gates: [ + { condition: "new-query-index", label: "New query with index recommendation", fired: true, conclusion: "failure", found: "2 new queries ship a high-impact index recommendation" }, + { condition: "new-query", label: "New query", fired: true, conclusion: "neutral", found: "2 new queries, none with a prior baseline" }, + { condition: "schema-drift", label: "Schema drift", fired: false, conclusion: "success", found: "No schema changes" }, + ], + }); + + expect(renderTemplate(ctx)).toContain("Query Doctor — 1 failing, 1 warning and 1 successful check"); + }); + test("keeps both halves when something failed", () => { const ctx = makeContext({ gates: [ diff --git a/src/reporters/github/github.ts b/src/reporters/github/github.ts index 5f9ea4d..6ea82fa 100644 --- a/src/reporters/github/github.ts +++ b/src/reporters/github/github.ts @@ -8,7 +8,7 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); const success = readFileSync(join(__dirname, "success.md.j2"), "utf-8"); import n from "nunjucks"; -import { summarizeGates } from "../../gate/evaluate.ts"; +import { summarizeGates, type GateSummary } from "../../gate/evaluate.ts"; import { deriveIndexStatistics, isQueryLong, @@ -214,6 +214,39 @@ export function callSite( return funcName ? { name: funcName, file } : { name: file!, file: undefined }; } +/** + * The sentence above the roster: what the conditions concluded. + * + * Every outcome is named. Assembled in the template, it read "N failing and M + * successful checks", which had no room for a warning: a run with 1 FAIL, 1 WARN + * and 1 PASS announced two of its three rows (Site #4201). An outcome nothing + * concluded is left out rather than counted as a zero. + * + * DUPLICATED from `heading` in Site's `apps/api/src/ci/capture/capture-comment.ts`, + * which renders this same comment for repositories on the platform's ingest path. + * Both post onto one pull request, so a change to one is a change to both. + * + * Site's version also appends "· N not run". This one cannot: the + * `summarizeGates` beside it has no `unevaluated` count. Collapsing the two + * summaries is what would let the sentence be shared rather than copied. + */ +function formatGateHeading(summary: GateSummary): string { + const counted = [ + { count: summary.failing, word: "failing" }, + { count: summary.neutral, word: "warning" }, + { count: summary.successful, word: "successful" }, + ].filter((outcome) => outcome.count > 0); + + const last = counted.at(-1); + if (!last) return "no checks ran"; + + // Everything before the last outcome is a comma list. The last one carries + // the "and" and the noun, so the noun agrees with the count beside it. + const lead = counted.slice(0, -1).map(({ count, word }) => `${count} ${word}`); + const tail = `${last.count} ${last.word} check${last.count === 1 ? "" : "s"}`; + return lead.length > 0 ? `${lead.join(", ")} and ${tail}` : tail; +} + export function buildViewModel(ctx: ReportContext) { const hasComparison = !!ctx.comparison; const queryLinks = buildQueryLinks(ctx); @@ -222,6 +255,7 @@ export function buildViewModel(ctx: ReportContext) { // The roster the comment leads with. Derived from the same array the check // annotations read, so the heading cannot contradict the check (ADR-0009). const gateSummary = ctx.gates ? summarizeGates(ctx.gates) : undefined; + const gateHeading = gateSummary ? formatGateHeading(gateSummary) : undefined; // Failing first: the reader's question is what is blocking them, and the // roster otherwise reads in catalogue order regardless of what happened. const RANK = { failure: 0, neutral: 1, success: 2 } as const; @@ -251,6 +285,7 @@ export function buildViewModel(ctx: ReportContext) { schemaChangeLabel, modeledTablesNotice, gateSummary, + gateHeading, gateIconBase, brandMark, gates, @@ -318,6 +353,7 @@ export function buildViewModel(ctx: ReportContext) { schemaChangeLabel, modeledTablesNotice, gateSummary, + gateHeading, gateIconBase, brandMark, gates, diff --git a/src/reporters/github/success.md.j2 b/src/reporters/github/success.md.j2 index 89e502a..b15439e 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -1,5 +1,5 @@ {% if gateSummary %} -### Query Doctor — {% if gateSummary.failing > 0 %}{{ gateSummary.failing }} failing and {% endif %}{{ gateSummary.successful }} successful check{{ "" if gateSummary.successful == 1 else "s" }} +### Query Doctor — {{ gateHeading }} {% for g in gates %}  **{{ g.label }}** — {{ g.found }}