From eb60bcc41cf90bc17a2d7320a208a3bac8891a07 Mon Sep 17 00:00:00 2001 From: Xetera Date: Tue, 23 Jun 2026 17:16:42 +0300 Subject: [PATCH 1/2] chore: type schema in postToSiteApi --- src/reporters/site-api.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/reporters/site-api.ts b/src/reporters/site-api.ts index 1a6891c..5122f87 100644 --- a/src/reporters/site-api.ts +++ b/src/reporters/site-api.ts @@ -1,5 +1,5 @@ import * as github from "@actions/github"; -import type { ComputedStats, IndexRecommendation, Nudge, SQLCommenterTag, StatisticsMode, TableReference } from "@query-doctor/core"; +import type { ComputedStats, FullSchema, IndexRecommendation, Nudge, SQLCommenterTag, StatisticsMode, TableReference } from "@query-doctor/core"; import { DEFAULT_CONFIG, type AnalyzerConfig } from "../config.ts"; import type { OptimizedQuery } from "../sql/recent-query.ts"; @@ -342,7 +342,7 @@ export async function postToSiteApi( queries: CiQueryPayload[], statisticsMode?: StatisticsMode, computedStats?: ComputedStats, - schema?: unknown, + schema?: FullSchema, ): Promise { const payload: CiRunPayload = { repo: process.env.GITHUB_REPOSITORY ?? "", @@ -366,7 +366,7 @@ export async function postToSiteApi( if (!token) { console.warn( "TOKEN is not set — POST /ci/runs will be rejected as Unauthorized. " + - "Set TOKEN to your Query Doctor project token.", + "Set TOKEN to your Query Doctor project token.", ); } From 55b5b00198683cb31c4107ed631e7daee91a388c Mon Sep 17 00:00:00 2001 From: Xetera Date: Tue, 23 Jun 2026 20:21:41 +0300 Subject: [PATCH 2/2] feat: post pr comment based on schema changes --- src/reporters/github/github.test.ts | 72 +++++++++ src/reporters/github/github.ts | 27 ++++ src/reporters/github/schema-change.test.ts | 117 ++++++++++++++ src/reporters/github/schema-change.ts | 169 +++++++++++++++++++++ src/reporters/github/success.md.j2 | 13 ++ src/reporters/site-api.ts | 17 +++ 6 files changed, 415 insertions(+) create mode 100644 src/reporters/github/schema-change.test.ts create mode 100644 src/reporters/github/schema-change.ts diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index 22f49c6..b977811 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -636,3 +636,75 @@ describe("unset-baseline callout (Site #3297 / #3312)", () => { expect(output).not.toContain("No comparison branch configured"); }); }); + +describe("schema change section", () => { + const addedTableOp = { + op: "add" as const, + path: "/tables/0", + value: { type: "table", oid: 1, schemaName: "public", tableName: "orders", columns: [] }, + }; + const droppedIndexOp = { op: "remove" as const, path: "/indexes/3" }; + + test("buildViewModel surfaces a non-rendering view when metadata has no schemaChange", () => { + const ctx = makeContext({ comparison: makeComparison(), runMetadata: makeMetadata() }); + const vm = buildViewModel(ctx); + expect(vm.schemaChange.hasChanges).toBe(false); + }); + + test("buildViewModel ignores schemaChange when changed is false", () => { + const ctx = makeContext({ + comparison: makeComparison(), + runMetadata: makeMetadata({ schemaChange: { changed: false, operations: [addedTableOp] } }), + }); + const vm = buildViewModel(ctx); + expect(vm.schemaChange.hasChanges).toBe(false); + }); + + test("buildViewModel treats null schemaChange (degraded read) as no change", () => { + const ctx = makeContext({ + comparison: makeComparison(), + runMetadata: makeMetadata({ schemaChange: null }), + }); + const vm = buildViewModel(ctx); + expect(vm.schemaChange.hasChanges).toBe(false); + }); + + test("template renders schema changes vs the comparison branch", () => { + const ctx = makeContext({ + comparison: makeComparison(), + comparisonBranch: "main", + runMetadata: makeMetadata({ + schemaChange: { changed: true, operations: [addedTableOp, droppedIndexOp] }, + }), + }); + const output = renderTemplate(ctx); + + expect(output).toContain("2 schema changes vs main"); + expect(output).toContain("**Added**"); + expect(output).toContain("table public.orders"); + expect(output).toContain("**Removed**"); + expect(output).toContain("index (removed)"); + }); + + test("template renders no schema section when unchanged", () => { + const ctx = makeContext({ + comparison: makeComparison(), + runMetadata: makeMetadata({ schemaChange: { changed: false, operations: [] } }), + }); + const output = renderTemplate(ctx); + expect(output).not.toContain("schema change"); + }); + + test("singular wording for a single schema change", () => { + const ctx = makeContext({ + comparison: makeComparison(), + comparisonBranch: "main", + runMetadata: makeMetadata({ + schemaChange: { changed: true, operations: [addedTableOp] }, + }), + }); + const output = renderTemplate(ctx); + expect(output).toContain("1 schema change vs main"); + expect(output).not.toContain("1 schema changes"); + }); +}); diff --git a/src/reporters/github/github.ts b/src/reporters/github/github.ts index ce410c4..71df1eb 100644 --- a/src/reporters/github/github.ts +++ b/src/reporters/github/github.ts @@ -18,6 +18,12 @@ import { } from "../reporter.ts"; import type { CiQueryPayload, ImprovedQuery, RegressedQuery } from "../site-api.ts"; +import { + buildSchemaChangeView, + schemaChangeHeading, + schemaChangeLabel, + type SchemaChangeView, +} from "./schema-change.ts"; n.configure({ autoescape: false, trimBlocks: true, lstripBlocks: true }); @@ -123,9 +129,24 @@ function buildQueryLinks(ctx: ReportContext): Record { return links; } +/** + * Schema delta between this PR and the comparison baseline, sourced from the run + * metadata. Absent when the API predates the field or couldn't resolve the + * baseline schema (`null`), and empty when the schema is unchanged — all collapse + * to a non-rendering view so the template stays a single `if hasChanges` guard. + */ +function buildSchemaChange(ctx: ReportContext): SchemaChangeView { + const change = ctx.runMetadata?.schemaChange; + if (!change || !change.changed) { + return { hasChanges: false, total: 0, groups: [] }; + } + return buildSchemaChangeView(change.operations); +} + export function buildViewModel(ctx: ReportContext) { const hasComparison = !!ctx.comparison; const queryLinks = buildQueryLinks(ctx); + const schemaChange = buildSchemaChange(ctx); if (!hasComparison) { return { @@ -138,6 +159,9 @@ export function buildViewModel(ctx: ReportContext) { newQueryCount: 0, hasComparison: false, queryLinks, + schemaChange, + schemaChangeHeading, + schemaChangeLabel, }; } @@ -177,6 +201,9 @@ export function buildViewModel(ctx: ReportContext) { newQueryCount: ctx.comparison!.newQueries.length, hasComparison: true, queryLinks, + schemaChange, + schemaChangeHeading, + schemaChangeLabel, }; } diff --git a/src/reporters/github/schema-change.test.ts b/src/reporters/github/schema-change.test.ts new file mode 100644 index 0000000..2d516a6 --- /dev/null +++ b/src/reporters/github/schema-change.test.ts @@ -0,0 +1,117 @@ +import { test, expect, describe } from "vitest"; +import type { Op } from "jsondiffpatch/formatters/jsonpatch"; +import { + buildSchemaChangeView, + schemaChangeLabel, + type SchemaChangeKind, +} from "./schema-change.ts"; + +function entriesFor(view: ReturnType, kind: SchemaChangeKind) { + return view.groups.find((g) => g.kind === kind)?.entries ?? []; +} + +describe("buildSchemaChangeView", () => { + test("empty operations produce no changes", () => { + const view = buildSchemaChangeView([]); + expect(view.hasChanges).toBe(false); + expect(view.total).toBe(0); + expect(view.groups).toHaveLength(0); + }); + + test("added table is grouped under 'added' with a qualified name", () => { + const ops: Op[] = [ + { + op: "add", + path: "/tables/0", + value: { type: "table", oid: 1, schemaName: "public", tableName: "users", columns: [] }, + }, + ]; + const view = buildSchemaChangeView(ops); + expect(view.hasChanges).toBe(true); + const added = entriesFor(view, "added"); + expect(added).toEqual([{ kind: "added", object: "table", name: "public.users" }]); + }); + + test("added index names itself as table.index", () => { + const ops: Op[] = [ + { + op: "add", + path: "/indexes/0", + value: { + type: "index", + oid: 42, + schemaName: "public", + tableName: "users", + indexName: "users_email_idx", + }, + }, + ]; + const added = entriesFor(buildSchemaChangeView(ops), "added"); + expect(added[0]).toEqual({ kind: "added", object: "index", name: "users.users_email_idx" }); + }); + + test("removed object is grouped under 'removed' (no value to name it)", () => { + const ops: Op[] = [{ op: "remove", path: "/constraints/2" }]; + const view = buildSchemaChangeView(ops); + const removed = entriesFor(view, "removed"); + expect(removed).toEqual([{ kind: "removed", object: "constraint", name: "(removed)" }]); + }); + + test("property-level replace is a 'changed' entry carrying the sub-path", () => { + const ops: Op[] = [{ op: "replace", path: "/indexes/0/isUnique", value: true }]; + const changed = entriesFor(buildSchemaChangeView(ops), "changed"); + expect(changed).toEqual([ + { kind: "changed", object: "index", name: "", detail: "isUnique" }, + ]); + }); + + test("extension uses extensionName and is unqualified", () => { + const ops: Op[] = [ + { + op: "add", + path: "/extensions/0", + value: { extensionName: "pg_trgm", version: "1.0", schemaName: "public" }, + }, + ]; + const added = entriesFor(buildSchemaChangeView(ops), "added"); + expect(added[0]).toEqual({ kind: "added", object: "extension", name: "pg_trgm" }); + }); + + test("move ops and unknown collections are ignored", () => { + const ops: Op[] = [ + { op: "move", from: "/tables/0", path: "/tables/1" }, + { op: "add", path: "/unknownCollection/0", value: { name: "x" } }, + ]; + const view = buildSchemaChangeView(ops); + expect(view.hasChanges).toBe(false); + }); + + test("mixed ops total and order by added → removed → changed", () => { + const ops: Op[] = [ + { op: "replace", path: "/tables/0/tablespace", value: "fast_ssd" }, + { op: "remove", path: "/indexes/3" }, + { + op: "add", + path: "/tables/1", + value: { type: "table", oid: 5, schemaName: "public", tableName: "orders", columns: [] }, + }, + ]; + const view = buildSchemaChangeView(ops); + expect(view.total).toBe(3); + expect(view.groups.map((g) => g.kind)).toEqual(["added", "removed", "changed"]); + }); +}); + +describe("schemaChangeLabel", () => { + test("named entry", () => { + expect( + schemaChangeLabel({ kind: "added", object: "table", name: "public.users" }), + ).toBe("table public.users"); + }); + + test("changed entry with detail and no name", () => { + expect( + schemaChangeLabel({ kind: "changed", object: "index", name: "", detail: "isUnique" }), + ).toBe("index · isUnique"); + }); +}); diff --git a/src/reporters/github/schema-change.ts b/src/reporters/github/schema-change.ts new file mode 100644 index 0000000..f5dd2e9 --- /dev/null +++ b/src/reporters/github/schema-change.ts @@ -0,0 +1,169 @@ +import type { Op } from "jsondiffpatch/formatters/jsonpatch"; + +export type SchemaChangeKind = "added" | "removed" | "changed"; + +export interface SchemaChangeEntry { + kind: SchemaChangeKind; + /** Human label for the object class, e.g. "table", "index", "column". */ + object: string; + /** Best-effort display name, e.g. "public.users" or "users.users_email_idx". */ + name: string; + /** For "changed" entries, the dotted sub-path that changed, e.g. "isUnique". */ + detail?: string; +} + +export interface SchemaChangeGroup { + kind: SchemaChangeKind; + entries: SchemaChangeEntry[]; +} + +export interface SchemaChangeView { + hasChanges: boolean; + total: number; + groups: SchemaChangeGroup[]; +} + +const COLLECTION_LABELS: Record = { + tables: "table", + indexes: "index", + constraints: "constraint", + functions: "function", + extensions: "extension", + views: "view", + types: "type", + triggers: "trigger", +}; + +const KIND_ORDER: SchemaChangeKind[] = ["added", "removed", "changed"]; + +interface ParsedPath { + collection: string; + /** Remaining segments after the collection + element index. */ + rest: string[]; +} + +function parsePath(path: string): ParsedPath | null { + const segments = path.split("/").filter((s) => s.length > 0); + if (segments.length < 2) return null; + const [collection, , ...rest] = segments; + if (!(collection in COLLECTION_LABELS)) return null; + return { collection, rest }; +} + +/** + * Best-effort display name for a schema object pulled from an add/replace op's + * `value`. Each FullSchema collection carries different identifying fields, so + * fall back through the plausible name keys and qualify with the schema where + * one exists. Returns null when nothing nameable is present (e.g. a `remove` op, + * which carries no value). + */ +function objectName(collection: string, value: unknown): string | null { + if (typeof value !== "object" || value === null) return null; + const v = value as Record; + const str = (key: string): string | undefined => + typeof v[key] === "string" ? (v[key] as string) : undefined; + + const schema = str("schemaName"); + const qualify = (name: string | undefined): string | null => { + if (!name) return null; + return schema ? `${schema}.${name}` : name; + }; + + switch (collection) { + case "tables": + return qualify(str("tableName")); + case "indexes": { + const table = str("tableName"); + const index = str("indexName"); + if (index && table) return `${table}.${index}`; + return qualify(index); + } + case "constraints": { + const table = str("tableName"); + const constraint = str("constraintName"); + if (constraint && table) return `${table}.${constraint}`; + return qualify(constraint); + } + case "functions": + return qualify(str("objectName")); + case "extensions": + return str("extensionName") ?? null; + case "views": + return qualify(str("viewName")); + case "types": + return qualify(str("typeName")); + case "triggers": { + const table = str("tableName"); + const trigger = str("triggerName"); + if (trigger && table) return `${table}.${trigger}`; + return qualify(trigger); + } + default: + return null; + } +} + +function entryFromOp(op: Op): SchemaChangeEntry | null { + if (op.op === "move") return null; + const parsed = parsePath(op.path); + if (!parsed) return null; + const object = COLLECTION_LABELS[parsed.collection]; + + // A `replace`/`add` op deeper than the element root (e.g. /indexes/0/isUnique) + // is a property change on an existing object, not a whole-object add. A `remove` + // carries no value, so we can only name a whole-object removal, not a property one. + const isElementRoot = parsed.rest.length === 0; + + if (op.op === "add" && isElementRoot) { + const name = objectName(parsed.collection, op.value); + return { kind: "added", object, name: name ?? "(unknown)" }; + } + if (op.op === "remove" && isElementRoot) { + return { kind: "removed", object, name: "(removed)" }; + } + // Property-level add/replace, or a nested remove — all "changed" on the object. + const detail = parsed.rest.length > 0 ? parsed.rest.join(".") : undefined; + return { kind: "changed", object, name: "", detail }; +} + +export function buildSchemaChangeView(operations: Op[]): SchemaChangeView { + const byKind: Record = { + added: [], + removed: [], + changed: [], + }; + + for (const op of operations) { + const entry = entryFromOp(op); + if (!entry) continue; + byKind[entry.kind].push(entry); + } + + const groups: SchemaChangeGroup[] = KIND_ORDER + .map((kind) => ({ kind, entries: byKind[kind] })) + .filter((g) => g.entries.length > 0); + + const total = groups.reduce((sum, g) => sum + g.entries.length, 0); + + return { + hasChanges: total > 0, + total, + groups, + }; +} + +const KIND_HEADINGS: Record = { + added: "Added", + removed: "Removed", + changed: "Changed", +}; + +export function schemaChangeHeading(kind: SchemaChangeKind): string { + return KIND_HEADINGS[kind]; +} + +/** One-line label for an entry, e.g. "table public.users" or "index users.idx · isUnique". */ +export function schemaChangeLabel(entry: SchemaChangeEntry): string { + const base = entry.name ? `${entry.object} ${entry.name}` : entry.object; + return entry.detail ? `${base} · ${entry.detail}` : base; +} diff --git a/src/reporters/github/success.md.j2 b/src/reporters/github/success.md.j2 index ccd5cb6..57fe1f9 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -78,6 +78,19 @@ {% endif %} +{% if schemaChange.hasChanges %} +
+{{ schemaChange.total }} schema change{{ "s" if schemaChange.total != 1 else "" }} vs {{ comparisonBranch }} + +{% for group in schemaChange.groups %} +**{{ schemaChangeHeading(group.kind) }}** + +{% for entry in group.entries %} +- {{ schemaChangeLabel(entry) }} +{% endfor %} +{% endfor %} +
+{% endif %} {% if statisticsMode.kind == "fromAssumption" %} Using assumed statistics ({{ statisticsMode.reltuples }} rows/table). For better results, sync production stats. {% endif %} diff --git a/src/reporters/site-api.ts b/src/reporters/site-api.ts index 5122f87..4c3713b 100644 --- a/src/reporters/site-api.ts +++ b/src/reporters/site-api.ts @@ -2,6 +2,7 @@ import * as github from "@actions/github"; import type { ComputedStats, FullSchema, IndexRecommendation, Nudge, SQLCommenterTag, StatisticsMode, TableReference } from "@query-doctor/core"; import { DEFAULT_CONFIG, type AnalyzerConfig } from "../config.ts"; import type { OptimizedQuery } from "../sql/recent-query.ts"; +import type { Op } from "jsondiffpatch/formatters/jsonpatch"; interface CiRunPayload { repo: string; @@ -120,6 +121,22 @@ export interface CiRunMetadata { unset: boolean; mcpCall: string; } | null; + /** + * Schema delta of this run's pushed schema against the latest schema stored + * for the resolved comparison baseline — the same baseline the roll-up uses, + * so the schema diff and the query signals agree on what "the target branch" + * is. `operations` is a jsondiffpatch JSON Patch (RFC 6902) over the + * {@link FullSchema} shape, keyed by table/index/constraint OID server-side. + * + * Optional/nullable to mirror {@link baseline}: absent on a Site API that + * predates this field (deploy skew — render nothing), `null` when the API + * couldn't resolve the baseline schema (a degraded read — distinct from a + * clean run with no schema change, which is `changed: false`). + */ + schemaChange?: { + changed: boolean; + operations: Op[]; + } | null; } /**