diff --git a/src/main.ts b/src/main.ts index 5744f26a..daebfef4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -82,22 +82,19 @@ async function runInCI( const queries = buildQueries(allResults, config); - // POST to Site API first so we get the run ID for the PR comment link - let runId: string | null = null; + // POST to Site API first so we get the run ID (for baseline exclusion) and + // the unified CI-signal metadata for the PR comment. + let runResult = null; if (siteApiEndpoint) { - runId = await postToSiteApi(siteApiEndpoint, queries, reportContext.statisticsMode, reportContext.computedStats); + runResult = await postToSiteApi(siteApiEndpoint, queries, reportContext.statisticsMode, reportContext.computedStats); } + const runId: string | null = runResult?.id ?? null; - // Build the run URL and query base URL for the PR comment - if (siteApiEndpoint && runId) { - // SITE_API_ENDPOINT is e.g. https://api.querydoctor.com - // The app lives at https://app.querydoctor.com — derive from the API URL - const appUrl = - process.env.SITE_APP_URL ?? - siteApiEndpoint.replace(/\/api\/?$/, "").replace("api.", "app."); - const baseUrl = appUrl.replace(/\/$/, ""); - reportContext.runUrl = `${baseUrl}/ixr/ci/${runId}`; - reportContext.queryBaseUrl = baseUrl; + // Run link and per-query links come straight from the API response. Both + // degrade gracefully: `url` is null and `queries` is empty for an unlinked repo. + if (runResult) { + reportContext.runUrl = runResult.url ?? undefined; + reportContext.runMetadata = runResult.metadata ?? undefined; } // Fetch previous run for comparison @@ -145,12 +142,14 @@ async function runInCI( // Block PR if regressions exceed thresholds if (reportContext.comparison && reportContext.comparison.regressed.length > 0) { + const queryLinks = new Map( + (reportContext.runMetadata?.queries ?? []).map((q) => [q.hash, q.link]), + ); const messages = reportContext.comparison.regressed.map((q) => { const preview = queryPreview(q.formattedQuery); const cost = `cost ${formatCost(q.previousCost)} → ${formatCost(q.currentCost)} (+${q.regressionPercentage.toFixed(1)}%)`; - const link = reportContext.runUrl - ? `\n ${reportContext.runUrl}/${q.hash}` - : ""; + const queryLink = queryLinks.get(q.hash); + const link = queryLink ? `\n ${queryLink}` : ""; return ` - ${preview}: ${cost}${link}`; }); core.setFailed( diff --git a/src/reporters/github/__snapshots__/github.test.ts.snap b/src/reporters/github/__snapshots__/github.test.ts.snap new file mode 100644 index 00000000..9dc6860f --- /dev/null +++ b/src/reporters/github/__snapshots__/github.test.ts.snap @@ -0,0 +1,47 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`CI-signal metadata parity (analyzer#141) > linked repo: renders rollup line, per-query links, footer, run link, and docs link 1`] = ` +"### Query Doctor Analysis + +28 queries analyzed +2 regressed · 1 improved · 3 new · 0 removed + +#### This PR improves queries + +- [SELECT 2](https://app.querydoctor.com/alice/proj/ci/9f3a1c20/improved-1)
cost 500 → 100 (80% reduction) + +#### This PR has regressions on queries + +- [SELECT 1](https://app.querydoctor.com/alice/proj/ci/9f3a1c20/regressed-1)
cost 120 → 170 (+42%) + + + + +Using assumed statistics (10000 rows/table). For better results, sync production stats. + +More detail → get_ci_run({ runId: "9f3a1c20" }) · view run · docs +" +`; + +exports[`CI-signal metadata parity (analyzer#141) > unlinked repo: rollup + footer + docs only, no run link, no per-query links 1`] = ` +"### Query Doctor Analysis + +28 queries analyzed +2 regressed · 1 improved · 3 new · 0 removed + +#### This PR improves queries + +- SELECT 2
cost 500 → 100 (80% reduction) + +#### This PR has regressions on queries + +- SELECT 1
cost 120 → 170 (+42%) + + + + +Using assumed statistics (10000 rows/table). For better results, sync production stats. + +More detail → get_ci_run({ runId: "9f3a1c20" }) · docs +" +`; diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index dbf41afb..cb0923ad 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -5,7 +5,7 @@ import { dirname, join } from "node:path"; import n from "nunjucks"; import { formatCost, queryPreview, buildViewModel } from "./github.ts"; import { isQueryLong, renderExplain, type ReportContext } from "../reporter.ts"; -import type { RunComparison } from "../site-api.ts"; +import type { CiRunMetadata, RunComparison } from "../site-api.ts"; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -340,3 +340,120 @@ describe("template rendering", () => { expect(output).toContain("3 queries analyzed"); }); }); + +function makeMetadata(overrides: Partial = {}): CiRunMetadata { + return { + rollup: { regressed: 2, improved: 1, new: 3, removed: 0 }, + rollupText: "2 regressed · 1 improved · 3 new · 0 removed", + footer: 'More detail → get_ci_run({ runId: "9f3a1c20" })', + docsUrl: "https://docs.querydoctor.com", + signalKeys: { + new: "signal.new", + regressed: "signal.regressed", + improved: "signal.improved", + index: "signal.index", + }, + queries: [ + { hash: "regressed-1", link: "https://app.querydoctor.com/alice/proj/ci/9f3a1c20/regressed-1" }, + { hash: "improved-1", link: "https://app.querydoctor.com/alice/proj/ci/9f3a1c20/improved-1" }, + ], + ...overrides, + }; +} + +describe("CI-signal metadata parity (analyzer#141)", () => { + const regressedComparison = makeComparison({ + regressed: [ + { + hash: "regressed-1", + query: "SELECT 1", + formattedQuery: "SELECT 1", + previousCost: 120, + currentCost: 170, + regressionPercentage: 42, + }, + ], + improved: [ + { + hash: "improved-1", + query: "SELECT 2", + formattedQuery: "SELECT 2", + previousCost: 500, + currentCost: 100, + improvementPercentage: 80, + previousIndexes: [], + currentIndexes: [], + }, + ], + }); + + test("linked repo: renders rollup line, per-query links, footer, run link, and docs link", () => { + const ctx = makeContext({ + comparison: regressedComparison, + runUrl: "https://app.querydoctor.com/alice/proj/ci/9f3a1c20", + runMetadata: makeMetadata(), + }); + const output = renderTemplate(ctx); + + // Roll-up line rendered verbatim (single source of truth — no re-derived grammar). + expect(output).toContain("2 regressed · 1 improved · 3 new · 0 removed"); + // Footer rendered verbatim. + expect(output).toContain('More detail → get_ci_run({ runId: "9f3a1c20" })'); + // Per-query rows link via metadata.queries, not a re-derived /ixr/ route. + expect(output).toContain("https://app.querydoctor.com/alice/proj/ci/9f3a1c20/regressed-1"); + expect(output).toContain("https://app.querydoctor.com/alice/proj/ci/9f3a1c20/improved-1"); + expect(output).not.toContain("/ixr/"); + // Run link and small docs link in the meta row. + expect(output).toContain('view run'); + expect(output).toContain('docs'); + // Per-signal icons aren't rendered yet (assets pending Site follow-up). + expect(output).not.toContain(" { + const ctx = makeContext({ + comparison: regressedComparison, + runUrl: undefined, + runMetadata: makeMetadata({ queries: [] }), + }); + const output = renderTemplate(ctx); + + // Shared elements still present. + expect(output).toContain("2 regressed · 1 improved · 3 new · 0 removed"); + expect(output).toContain('More detail → get_ci_run({ runId: "9f3a1c20" })'); + expect(output).toContain('docs'); + // No run link, no per-query links when the repo isn't linked. + expect(output).not.toContain("view run"); + expect(output).not.toContain("https://app.querydoctor.com/alice/proj/ci"); + // Query previews still render, just without anchors. + expect(output).toContain("SELECT 1"); + + expect(output).toMatchSnapshot(); + }); + + test("no metadata (degraded API response): no rollup or footer row", () => { + const ctx = makeContext({ + comparison: regressedComparison, + runMetadata: undefined, + }); + const output = renderTemplate(ctx); + + expect(output).not.toContain("regressed · "); + expect(output).not.toContain("get_ci_run"); + expect(output).not.toContain("docs"); + }); + + test("null docsUrl: docs link omitted, footer still rendered", () => { + const ctx = makeContext({ + comparison: regressedComparison, + runUrl: "https://app.querydoctor.com/alice/proj/ci/9f3a1c20", + runMetadata: makeMetadata({ docsUrl: null }), + }); + const output = renderTemplate(ctx); + + expect(output).toContain('More detail → get_ci_run({ runId: "9f3a1c20" })'); + expect(output).not.toContain(">docs"); + }); +}); diff --git a/src/reporters/github/github.ts b/src/reporters/github/github.ts index 0f5f9138..6504dc0d 100644 --- a/src/reporters/github/github.ts +++ b/src/reporters/github/github.ts @@ -21,6 +21,11 @@ import type { ImprovedQuery, RegressedQuery } from "../site-api.ts"; n.configure({ autoescape: false, trimBlocks: true, lstripBlocks: true }); +// NOTE: The Site API exposes presentation-agnostic `signalKeys` (available in +// the template via `runMetadata.signalKeys`), but we don't render per-signal +// icons yet — the image assets don't exist. Rendering is deferred until the +// Site follow-up that hosts them. See Query-Doctor/Site (analyzer#141 follow-up). + interface DisplayRecommendation extends ReportIndexRecommendation { queryPreview: string; } @@ -85,8 +90,18 @@ function addImprovementPreviews( })); } +/** Per-query detail links keyed by query hash, sourced from the run metadata. */ +function buildQueryLinks(ctx: ReportContext): Record { + const links: Record = {}; + for (const q of ctx.runMetadata?.queries ?? []) { + links[q.hash] = q.link; + } + return links; +} + export function buildViewModel(ctx: ReportContext) { const hasComparison = !!ctx.comparison; + const queryLinks = buildQueryLinks(ctx); if (!hasComparison) { return { @@ -97,6 +112,7 @@ export function buildViewModel(ctx: ReportContext) { preExistingRecommendations: [] as DisplayRecommendation[], newQueryCount: 0, hasComparison: false, + queryLinks, }; } @@ -123,6 +139,7 @@ export function buildViewModel(ctx: ReportContext) { preExistingRecommendations, newQueryCount: ctx.comparison!.newQueries.length, hasComparison: true, + queryLinks, }; } diff --git a/src/reporters/github/success.md.j2 b/src/reporters/github/success.md.j2 index 0a30f602..310c2b7d 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -1,7 +1,4 @@ ### Query Doctor Analysis -{% if runUrl %} -[View full run details]({{ runUrl }}) -{% endif %} {% if hasComparison %} {{ queryStats.analyzed | default('?') }} queries analyzed @@ -11,12 +8,17 @@ > **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. {% endif %} +{% if runMetadata %} + +{{ runMetadata.rollupText }} +{% endif %} {% if displayImproved.length > 0 %} #### This PR improves queries {% for q in displayImproved %} -- {% if runUrl %}[{{ q.queryPreview }}]({{ runUrl }}/{{ q.hash }}){% else %}{{ q.queryPreview }}{% endif %}
cost {{ formatCost(q.previousCost) }} → {{ formatCost(q.currentCost) }} ({{ q.improvementPercentage | round(0) }}% reduction){% if q.indexesChanged %}{% if q.previousIndexes.length > 0 %}
was using: {% for idx in q.previousIndexes %}{{ idx }}{% if not loop.last %}, {% endif %}{% endfor %}{% endif %}{% if q.currentIndexes.length > 0 %}
now using: {% for idx in q.currentIndexes %}{{ idx }}{% if not loop.last %}, {% endif %}{% endfor %}{% endif %}{% endif %}{{""}} +{% set link = queryLinks[q.hash] %} +- {% if link %}[{{ q.queryPreview }}]({{ link }}){% else %}{{ q.queryPreview }}{% endif %}
cost {{ formatCost(q.previousCost) }} → {{ formatCost(q.currentCost) }} ({{ q.improvementPercentage | round(0) }}% reduction){% if q.indexesChanged %}{% if q.previousIndexes.length > 0 %}
was using: {% for idx in q.previousIndexes %}{{ idx }}{% if not loop.last %}, {% endif %}{% endfor %}{% endif %}{% if q.currentIndexes.length > 0 %}
now using: {% for idx in q.currentIndexes %}{{ idx }}{% if not loop.last %}, {% endif %}{% endfor %}{% endif %}{% endif %}{{""}} {% endfor %} {% endif %} @@ -24,7 +26,8 @@ #### This PR has regressions on queries {% for q in displayRegressed %} -- {% if runUrl %}[{{ q.queryPreview }}]({{ runUrl }}/{{ q.hash }}){% else %}{{ q.queryPreview }}{% endif %}
cost {{ formatCost(q.previousCost) }} → {{ formatCost(q.currentCost) }} (+{{ q.regressionPercentage | round(0) }}%) +{% set link = queryLinks[q.hash] %} +- {% if link %}[{{ q.queryPreview }}]({{ link }}){% else %}{{ q.queryPreview }}{% endif %}
cost {{ formatCost(q.previousCost) }} → {{ formatCost(q.currentCost) }} (+{{ q.regressionPercentage | round(0) }}%) {% endfor %} {% endif %} @@ -42,7 +45,8 @@ #### This PR introduces queries with recommendations {% for r in displayRecommendations %} -- {% if runUrl %}[{{ r.queryPreview }}]({{ runUrl }}/{{ r.fingerprint }}){% else %}{{ r.queryPreview }}{% endif %}
recommended index {{ r.proposedIndexes | join(", ") }}
cost {{ formatCost(r.baseCost) }} → {{ formatCost(r.optimizedCost) }} ({{ (((r.baseCost - r.optimizedCost) / r.baseCost) * 100) | round(0) }}% reduction) +{% set link = queryLinks[r.fingerprint] %} +- {% if link %}[{{ r.queryPreview }}]({{ link }}){% else %}{{ r.queryPreview }}{% endif %}
recommended index {{ r.proposedIndexes | join(", ") }}
cost {{ formatCost(r.baseCost) }} → {{ formatCost(r.optimizedCost) }} ({{ (((r.baseCost - r.optimizedCost) / r.baseCost) * 100) | round(0) }}% reduction) {% endfor %} {% endif %} @@ -51,7 +55,8 @@ {{ preExistingRecommendations.length }} pre-existing issue{{ "s" if preExistingRecommendations.length != 1 else "" }} {% for r in preExistingRecommendations %} -- {% if runUrl %}[{{ r.queryPreview }}]({{ runUrl }}/{{ r.fingerprint }}){% else %}{{ r.queryPreview }}{% endif %}
index {{ r.proposedIndexes | join(", ") }}
cost {{ formatCost(r.baseCost) }} → {{ formatCost(r.optimizedCost) }} ({{ (((r.baseCost - r.optimizedCost) / r.baseCost) * 100) | round(0) }}% reduction) +{% set link = queryLinks[r.fingerprint] %} +- {% if link %}[{{ r.queryPreview }}]({{ link }}){% else %}{{ r.queryPreview }}{% endif %}
index {{ r.proposedIndexes | join(", ") }}
cost {{ formatCost(r.baseCost) }} → {{ formatCost(r.optimizedCost) }} ({{ (((r.baseCost - r.optimizedCost) / r.baseCost) * 100) | round(0) }}% reduction) {% endfor %} {% endif %} @@ -59,3 +64,7 @@ {% if statisticsMode.kind == "fromAssumption" %} Using assumed statistics ({{ statisticsMode.reltuples }} rows/table). For better results, sync production stats. {% endif %} +{% if runMetadata %} + +{{ runMetadata.footer }}{% if runUrl %} · view run{% endif %}{% if runMetadata.docsUrl %} · docs{% endif %} +{% endif %} diff --git a/src/reporters/reporter.ts b/src/reporters/reporter.ts index d95e73b1..b05121aa 100644 --- a/src/reporters/reporter.ts +++ b/src/reporters/reporter.ts @@ -1,5 +1,5 @@ import type { ComputedStats, IndexIdentifier, StatisticsMode } from "@query-doctor/core"; -import type { RunComparison } from "./site-api.ts"; +import type { CiRunMetadata, RunComparison } from "./site-api.ts"; export interface Reporter { provider(): string; @@ -83,8 +83,10 @@ export interface ReportContext { error?: Error; comparison?: RunComparison; comparisonBranch?: string; + /** The run page link (`metadata.url` from `POST /ci/runs`). Absent when the repo isn't linked. */ runUrl?: string; - queryBaseUrl?: string; + /** Unified CI-signal metadata: roll-up line, footer, per-query links, docs link, icon keys. */ + runMetadata?: CiRunMetadata; } export interface IndexStatistic { diff --git a/src/reporters/site-api.ts b/src/reporters/site-api.ts index 0e10445d..de92de93 100644 --- a/src/reporters/site-api.ts +++ b/src/reporters/site-api.ts @@ -69,6 +69,36 @@ export interface PreviousRun { queries: CiQueryPayload[]; } +/** + * Unified CI-signal metadata returned by `POST /ci/runs` (Site #3067). + * The analyzer renders these fields verbatim so the PR comment speaks the same + * language as the Slack/webhook alert. See analyzer#141. + */ +export interface CiRunMetadata { + /** Structured roll-up counts. The rendered roll-up line must equal `rollupText`. */ + rollup: { regressed: number; improved: number; new: number; removed: number }; + /** The roll-up line — render verbatim (equals the alert's `formatRollup(...)`). */ + rollupText: string; + /** The small "more detail" footer — render verbatim (equals `formatRunFooter(runId)`). */ + footer: string; + /** A small `docs` link. May be null. */ + docsUrl: string | null; + /** Presentation-agnostic icon keys for the four signal types. Map each to an image asset. */ + signalKeys: { new: string; regressed: string; improved: string; index: string }; + /** Per-query run-scoped detail links, keyed by query hash. Empty when the repo isn't linked. */ + queries: Array<{ hash: string; link: string }>; +} + +/** + * The `POST /ci/runs` response. `id` is always present; `url` is null and + * `metadata` is absent/degraded when the repo can't be resolved to a project. + */ +export interface CiRunResult { + id: string; + url: string | null; + metadata: CiRunMetadata | null; +} + export interface RunComparison { previousRunId: string; previousBranch: string; @@ -247,7 +277,7 @@ export async function postToSiteApi( queries: CiQueryPayload[], statisticsMode?: StatisticsMode, computedStats?: ComputedStats, -): Promise { +): Promise { const payload: CiRunPayload = { repo: process.env.GITHUB_REPOSITORY ?? "", branch: process.env.GITHUB_HEAD_REF || process.env.GITHUB_REF_NAME || "", @@ -273,9 +303,17 @@ export async function postToSiteApi( console.warn(`Site API responded with ${response.status}: ${text}`); return null; } - const body = (await response.json()) as { id: string }; + const body = (await response.json()) as { + id: string; + url?: string | null; + metadata?: CiRunMetadata | null; + }; console.log(`Site API ingestion successful: ${JSON.stringify(body)}`); - return body.id; + return { + id: body.id, + url: body.url ?? null, + metadata: body.metadata ?? null, + }; } catch (err) { console.warn(`Failed to POST to Site API: ${err}`); return null;