From 511b699a038d583d5e69bb4502772a356558485a Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Fri, 27 Mar 2026 19:41:10 +0400 Subject: [PATCH] fix: count only analyzed queries in queryStats.total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move queryStats.total++ past all early-return filters (ignored, introspection, duplicate, catalog) so it reflects the actual number of unique queries analyzed — not the raw log entry count. This fixes the mismatch between the PR comment ("X queries analyzed") and the app's query list without needing a post-hoc patch in main.ts. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/reporters/github/github.test.ts | 42 ++++++++++++- src/reporters/reporter.ts | 2 +- src/reporters/site-api.test.ts | 95 +++++++++++++++++++++++++++++ src/runner.ts | 2 +- 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index c676fd3..07fa0b5 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -1,8 +1,29 @@ import { test, expect, describe } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; +import n from "nunjucks"; import { formatCost, queryPreview, buildViewModel } from "./github.ts"; -import type { ReportContext } from "../reporter.ts"; +import { isQueryLong, renderExplain, type ReportContext } from "../reporter.ts"; import type { RunComparison } from "../site-api.ts"; +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); +const successTemplate = readFileSync(join(__dirname, "success.md.j2"), "utf-8"); + +n.configure({ autoescape: false, trimBlocks: true, lstripBlocks: true }); + +function renderTemplate(ctx: ReportContext) { + const viewModel = buildViewModel(ctx); + return n.renderString(successTemplate, { + ...ctx, + ...viewModel, + isQueryLong, + renderExplain, + formatCost, + }); +} + describe("formatCost", () => { test("formats small numbers without commas", () => { expect(formatCost(9)).toBe("9"); @@ -300,3 +321,22 @@ describe("buildViewModel", () => { }); }); + +describe("template rendering", () => { + test("renders queryStats.total as the query count", () => { + const ctx = makeContext({ + queryStats: { total: 5, matched: 3, optimized: 1, errored: 0 }, + comparison: makeComparison(), + }); + const output = renderTemplate(ctx); + expect(output).toContain("5 queries analyzed"); + }); + + test("renders queryStats.total in no-comparison mode", () => { + const ctx = makeContext({ + queryStats: { total: 3, matched: 1, optimized: 0, errored: 0 }, + }); + const output = renderTemplate(ctx); + expect(output).toContain("3 queries analyzed"); + }); +}); diff --git a/src/reporters/reporter.ts b/src/reporters/reporter.ts index 165296f..8e55315 100644 --- a/src/reporters/reporter.ts +++ b/src/reporters/reporter.ts @@ -62,7 +62,7 @@ export type ReportMetadata = { declare const s: unique symbol; export interface ReportStatistics { - /** Total number of queries seen in the log */ + /** Number of unique, non-filtered queries analyzed */ total: number; /** Number of queries that matched the query pattern */ matched: number; diff --git a/src/reporters/site-api.test.ts b/src/reporters/site-api.test.ts index 13acfa1..d91ee0a 100644 --- a/src/reporters/site-api.test.ts +++ b/src/reporters/site-api.test.ts @@ -1,9 +1,11 @@ import { test, expect, describe } from "vitest"; import { + buildQueries, compareRuns, type CiQueryPayload, type PreviousRun, } from "./site-api.ts"; +import type { QueryProcessResult } from "../runner.ts"; function makeQuery(hash: string, cost: number = 100): CiQueryPayload { return { @@ -31,6 +33,99 @@ function makePreviousRun(queries: CiQueryPayload[]): PreviousRun { }; } +describe("buildQueries", () => { + test("filters out invalid results", () => { + const results: QueryProcessResult[] = [ + { + kind: "no_improvement", + fingerprint: "hash-a", + rawQuery: "SELECT 1", + formattedQuery: "SELECT 1", + cost: 10, + existingIndexes: [], + nudges: [], + tags: [], + referencedTables: [], + }, + { kind: "invalid" }, + { + kind: "no_improvement", + fingerprint: "hash-b", + rawQuery: "SELECT 2", + formattedQuery: "SELECT 2", + cost: 20, + existingIndexes: [], + nudges: [], + tags: [], + referencedTables: [], + }, + ]; + + const queries = buildQueries(results); + expect(queries).toHaveLength(2); + expect(queries.map((q) => q.hash)).toEqual(["hash-a", "hash-b"]); + }); + + test("filters out ignored query hashes", () => { + const results: QueryProcessResult[] = [ + { + kind: "no_improvement", + fingerprint: "hash-a", + rawQuery: "SELECT 1", + formattedQuery: "SELECT 1", + cost: 10, + existingIndexes: [], + nudges: [], + tags: [], + referencedTables: [], + }, + { + kind: "no_improvement", + fingerprint: "hash-b", + rawQuery: "SELECT 2", + formattedQuery: "SELECT 2", + cost: 20, + existingIndexes: [], + nudges: [], + tags: [], + referencedTables: [], + }, + ]; + + const queries = buildQueries(results, { + ignoredQueryHashes: ["hash-a"], + acknowledgedQueryHashes: [], + regressionThreshold: 10, + minimumCost: 0, + }); + expect(queries).toHaveLength(1); + expect(queries[0].hash).toBe("hash-b"); + }); + + test("count reflects deduplicated output, not raw input length", () => { + const results: QueryProcessResult[] = [ + { + kind: "no_improvement", + fingerprint: "hash-a", + rawQuery: "SELECT 1", + formattedQuery: "SELECT 1", + cost: 10, + existingIndexes: [], + nudges: [], + tags: [], + referencedTables: [], + }, + { kind: "invalid" }, + { kind: "invalid" }, + { kind: "invalid" }, + ]; + + const queries = buildQueries(results); + // 4 results in, but only 1 valid query out + expect(queries).toHaveLength(1); + }); +}); + describe("compareRuns", () => { describe("new query detection via previousRun", () => { test("when previousRun has no queries, all current queries are new", () => { diff --git a/src/runner.ts b/src/runner.ts index 111c31c..102ac2f 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -216,7 +216,6 @@ export class Runner { } async processQuery(log: ExplainedLog): Promise { - this.queryStats.total++; const { query } = log; const queryFingerprint = await fingerprint(query); if (this.ignoredQueryHashes.has(queryFingerprint)) { @@ -257,6 +256,7 @@ export class Runner { } return { kind: "invalid" }; } + this.queryStats.total++; const indexCandidates = analyzer.deriveIndexes( this.stats.ownMetadata, indexesToCheck,