diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index e983008..8fca354 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -781,11 +781,11 @@ describe("schema change section", () => { }); const output = renderTemplate(ctx); - expect(output).toContain("Added"); - expect(output).toContain("**Added**"); - expect(output).toContain("table public.orders"); - expect(output).toContain("**Removed**"); - expect(output).toContain("index (removed)"); + // Each line states its own kind, so the block needs no heading above it. + expect(output).toContain("Added table public.orders"); + expect(output).toContain("Removed index"); + expect(output).not.toContain("**Added**"); + expect(output).not.toContain("**Removed**"); }); test("closes the schema list so the gates after it are not nested inside it", () => { @@ -800,15 +800,22 @@ describe("schema change section", () => { }), }); const lines = renderTemplate(ctx).split("\n"); + const gateRow = lines.findIndex((l) => l.includes("**Schema drift**")); + const entries = lines.filter((l) => l.startsWith("")); - // A line straight after a list item is lazy continuation: markdown folds it - // into that `
  • `, so every gate row below renders inside the list. - const lastEntry = lines.findLastIndex((l) => l.startsWith("- ")); + // Every expansion sits tight under its gate row, the way the regression and + // recommendation blocks do. A blank line here would drop the block further + // than any other caption in the comment. + expect(lines[gateRow + 1]).toBe(entries[0]); + + // But the block must close, or the roster's leading is computed against a + // 12px line box and the next gate row is pulled up tighter than the rest. + const lastEntry = lines.lastIndexOf(entries.at(-1)!); expect(lines[lastEntry + 1]).toBe(""); - // And a line straight after the gate row is a soft break inside the gate's - // own paragraph, so the group heading loses the space above it. - expect(lines[lines.indexOf("**Added**") - 1]).toBe(""); + // Caption size, not body size — otherwise the detail reads as loud as the + // gate rows it sits beneath. + expect(entries.every((l) => l.endsWith(""))).toBe(true); }); test("template renders no schema section when unchanged", () => { @@ -830,8 +837,7 @@ describe("schema change section", () => { }), }); const output = renderTemplate(ctx); - expect(output).toContain("**Added**"); - expect(output).toContain("table public.orders"); + expect(output).toContain("Added table public.orders"); }); }); diff --git a/src/reporters/github/github.ts b/src/reporters/github/github.ts index a0e300b..6badf4e 100644 --- a/src/reporters/github/github.ts +++ b/src/reporters/github/github.ts @@ -21,7 +21,6 @@ import { import type { CiQueryPayload, ImprovedQuery, RegressedQuery } from "../site-api.ts"; import { buildSchemaChangeView, - schemaChangeHeading, schemaChangeLabel, type SchemaChangeView, } from "./schema-change.ts"; @@ -242,7 +241,6 @@ export function buildViewModel(ctx: ReportContext) { hasComparison: false, queryLinks, schemaChange, - schemaChangeHeading, schemaChangeLabel, modeledTablesNotice, gateSummary, @@ -310,7 +308,6 @@ export function buildViewModel(ctx: ReportContext) { hasComparison: true, queryLinks, schemaChange, - schemaChangeHeading, schemaChangeLabel, modeledTablesNotice, gateSummary, diff --git a/src/reporters/github/schema-change.test.ts b/src/reporters/github/schema-change.test.ts index 2d516a6..6b1bf33 100644 --- a/src/reporters/github/schema-change.test.ts +++ b/src/reporters/github/schema-change.test.ts @@ -54,7 +54,7 @@ describe("buildSchemaChangeView", () => { 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)" }]); + expect(removed).toEqual([{ kind: "removed", object: "constraint", name: "" }]); }); test("property-level replace is a 'changed' entry carrying the sub-path", () => { @@ -106,12 +106,12 @@ describe("schemaChangeLabel", () => { test("named entry", () => { expect( schemaChangeLabel({ kind: "added", object: "table", name: "public.users" }), - ).toBe("table public.users"); + ).toBe("Added table public.users"); }); test("changed entry with detail and no name", () => { expect( schemaChangeLabel({ kind: "changed", object: "index", name: "", detail: "isUnique" }), - ).toBe("index · isUnique"); + ).toBe("Changed index · isUnique"); }); }); diff --git a/src/reporters/github/schema-change.ts b/src/reporters/github/schema-change.ts index f5dd2e9..79ddbf2 100644 --- a/src/reporters/github/schema-change.ts +++ b/src/reporters/github/schema-change.ts @@ -119,7 +119,10 @@ function entryFromOp(op: Op): SchemaChangeEntry | null { return { kind: "added", object, name: name ?? "(unknown)" }; } if (op.op === "remove" && isElementRoot) { - return { kind: "removed", object, name: "(removed)" }; + // A `remove` op carries no value, so there is no name to print. The line + // reads "removed index" — the kind prefix already says what happened, which + // the old "(removed)" placeholder was standing in for. + return { kind: "removed", object, name: "" }; } // Property-level add/replace, or a nested remove — all "changed" on the object. const detail = parsed.rest.length > 0 ? parsed.rest.join(".") : undefined; @@ -152,18 +155,20 @@ export function buildSchemaChangeView(operations: Op[]): SchemaChangeView { }; } -const KIND_HEADINGS: Record = { +const KIND_LABELS: 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". */ +/** + * One-line label for an entry, e.g. "Added table public.users" or + * "Changed index users.idx · isUnique". Each line states its own kind, so the + * entries need no group heading above them — which is what lets the block sit + * tight under its gate row the way every other expansion does. + */ export function schemaChangeLabel(entry: SchemaChangeEntry): string { - const base = entry.name ? `${entry.object} ${entry.name}` : entry.object; + const object = entry.name ? `${entry.object} ${entry.name}` : entry.object; + const base = `${KIND_LABELS[entry.kind]} ${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 49b0fec..16177f3 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -29,15 +29,12 @@ {% endfor %} {% endif %} {% if g.condition == "schema-drift" and g.fired and schemaChange.hasChanges %} - {% for group in schemaChange.groups %} -**{{ schemaChangeHeading(group.kind) }}** - {% for entry in group.entries %} -- {{ schemaChangeLabel(entry) }} +{{ schemaChangeLabel(entry) }} {% endfor %} - {% endfor %} + {% endif %} {% endfor %}