Skip to content

Commit 511b699

Browse files
veksenclaude
andcommitted
fix: count only analyzed queries in queryStats.total
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) <noreply@anthropic.com>
1 parent b1cc380 commit 511b699

4 files changed

Lines changed: 138 additions & 3 deletions

File tree

src/reporters/github/github.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
11
import { test, expect, describe } from "vitest";
2+
import { readFileSync } from "node:fs";
3+
import { fileURLToPath } from "node:url";
4+
import { dirname, join } from "node:path";
5+
import n from "nunjucks";
26
import { formatCost, queryPreview, buildViewModel } from "./github.ts";
3-
import type { ReportContext } from "../reporter.ts";
7+
import { isQueryLong, renderExplain, type ReportContext } from "../reporter.ts";
48
import type { RunComparison } from "../site-api.ts";
59

10+
const __filename = fileURLToPath(import.meta.url);
11+
const __dirname = dirname(__filename);
12+
const successTemplate = readFileSync(join(__dirname, "success.md.j2"), "utf-8");
13+
14+
n.configure({ autoescape: false, trimBlocks: true, lstripBlocks: true });
15+
16+
function renderTemplate(ctx: ReportContext) {
17+
const viewModel = buildViewModel(ctx);
18+
return n.renderString(successTemplate, {
19+
...ctx,
20+
...viewModel,
21+
isQueryLong,
22+
renderExplain,
23+
formatCost,
24+
});
25+
}
26+
627
describe("formatCost", () => {
728
test("formats small numbers without commas", () => {
829
expect(formatCost(9)).toBe("9");
@@ -300,3 +321,22 @@ describe("buildViewModel", () => {
300321
});
301322

302323
});
324+
325+
describe("template rendering", () => {
326+
test("renders queryStats.total as the query count", () => {
327+
const ctx = makeContext({
328+
queryStats: { total: 5, matched: 3, optimized: 1, errored: 0 },
329+
comparison: makeComparison(),
330+
});
331+
const output = renderTemplate(ctx);
332+
expect(output).toContain("5 queries analyzed");
333+
});
334+
335+
test("renders queryStats.total in no-comparison mode", () => {
336+
const ctx = makeContext({
337+
queryStats: { total: 3, matched: 1, optimized: 0, errored: 0 },
338+
});
339+
const output = renderTemplate(ctx);
340+
expect(output).toContain("3 queries analyzed");
341+
});
342+
});

src/reporters/reporter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export type ReportMetadata = {
6262
declare const s: unique symbol;
6363

6464
export interface ReportStatistics {
65-
/** Total number of queries seen in the log */
65+
/** Number of unique, non-filtered queries analyzed */
6666
total: number;
6767
/** Number of queries that matched the query pattern */
6868
matched: number;

src/reporters/site-api.test.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { test, expect, describe } from "vitest";
22
import {
3+
buildQueries,
34
compareRuns,
45
type CiQueryPayload,
56
type PreviousRun,
67
} from "./site-api.ts";
8+
import type { QueryProcessResult } from "../runner.ts";
79

810
function makeQuery(hash: string, cost: number = 100): CiQueryPayload {
911
return {
@@ -31,6 +33,99 @@ function makePreviousRun(queries: CiQueryPayload[]): PreviousRun {
3133
};
3234
}
3335

36+
describe("buildQueries", () => {
37+
test("filters out invalid results", () => {
38+
const results: QueryProcessResult[] = [
39+
{
40+
kind: "no_improvement",
41+
fingerprint: "hash-a",
42+
rawQuery: "SELECT 1",
43+
formattedQuery: "SELECT 1",
44+
cost: 10,
45+
existingIndexes: [],
46+
nudges: [],
47+
tags: [],
48+
referencedTables: [],
49+
},
50+
{ kind: "invalid" },
51+
{
52+
kind: "no_improvement",
53+
fingerprint: "hash-b",
54+
rawQuery: "SELECT 2",
55+
formattedQuery: "SELECT 2",
56+
cost: 20,
57+
existingIndexes: [],
58+
nudges: [],
59+
tags: [],
60+
referencedTables: [],
61+
},
62+
];
63+
64+
const queries = buildQueries(results);
65+
expect(queries).toHaveLength(2);
66+
expect(queries.map((q) => q.hash)).toEqual(["hash-a", "hash-b"]);
67+
});
68+
69+
test("filters out ignored query hashes", () => {
70+
const results: QueryProcessResult[] = [
71+
{
72+
kind: "no_improvement",
73+
fingerprint: "hash-a",
74+
rawQuery: "SELECT 1",
75+
formattedQuery: "SELECT 1",
76+
cost: 10,
77+
existingIndexes: [],
78+
nudges: [],
79+
tags: [],
80+
referencedTables: [],
81+
},
82+
{
83+
kind: "no_improvement",
84+
fingerprint: "hash-b",
85+
rawQuery: "SELECT 2",
86+
formattedQuery: "SELECT 2",
87+
cost: 20,
88+
existingIndexes: [],
89+
nudges: [],
90+
tags: [],
91+
referencedTables: [],
92+
},
93+
];
94+
95+
const queries = buildQueries(results, {
96+
ignoredQueryHashes: ["hash-a"],
97+
acknowledgedQueryHashes: [],
98+
regressionThreshold: 10,
99+
minimumCost: 0,
100+
});
101+
expect(queries).toHaveLength(1);
102+
expect(queries[0].hash).toBe("hash-b");
103+
});
104+
105+
test("count reflects deduplicated output, not raw input length", () => {
106+
const results: QueryProcessResult[] = [
107+
{
108+
kind: "no_improvement",
109+
fingerprint: "hash-a",
110+
rawQuery: "SELECT 1",
111+
formattedQuery: "SELECT 1",
112+
cost: 10,
113+
existingIndexes: [],
114+
nudges: [],
115+
tags: [],
116+
referencedTables: [],
117+
},
118+
{ kind: "invalid" },
119+
{ kind: "invalid" },
120+
{ kind: "invalid" },
121+
];
122+
123+
const queries = buildQueries(results);
124+
// 4 results in, but only 1 valid query out
125+
expect(queries).toHaveLength(1);
126+
});
127+
});
128+
34129
describe("compareRuns", () => {
35130
describe("new query detection via previousRun", () => {
36131
test("when previousRun has no queries, all current queries are new", () => {

src/runner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ export class Runner {
216216
}
217217

218218
async processQuery(log: ExplainedLog): Promise<QueryProcessResult> {
219-
this.queryStats.total++;
220219
const { query } = log;
221220
const queryFingerprint = await fingerprint(query);
222221
if (this.ignoredQueryHashes.has(queryFingerprint)) {
@@ -257,6 +256,7 @@ export class Runner {
257256
}
258257
return { kind: "invalid" };
259258
}
259+
this.queryStats.total++;
260260
const indexCandidates = analyzer.deriveIndexes(
261261
this.stats.ownMetadata,
262262
indexesToCheck,

0 commit comments

Comments
 (0)