Skip to content

feat(reporters): render the API's named schema changes - #217

Merged
veksen merged 1 commit into
mainfrom
feat-named-schema-changes
Aug 5, 2026
Merged

feat(reporters): render the API's named schema changes#217
veksen merged 1 commit into
mainfrom
feat-named-schema-changes

Conversation

@veksen

@veksen veksen commented Aug 5, 2026

Copy link
Copy Markdown
Member

Goal

Pairs with Query-Doctor/Site#3855, which is the other half. A PR that adds one column should have the CI comment name that column.

This is the rendering half. Site computes the named list; this reads it. Merge Site first, or this changes nothing.

What

Before, a change inside an object printed as a patch path. On Site#3794, which adds ci_runs.statistics_payload_id, the comment said:

Changed table · columns.17

After, the same run reads:

Added column public.ci_runs.statistics_payload_id

The positional churn goes with it. Site's summary drops a column's attnum and every oid, so the · columns.3.order and · oid lines stop appearing.

How

buildSchemaChange in github.ts prefers metadata.schemaChange.changes and falls back to buildSchemaChangeView(operations). entryFromOp is untouched, so an API that predates the field renders as it does today.

buildNamedSchemaChangeView in schema-change.ts only groups what the API sends. It infers nothing, because the inference is what produced the path in the first place: /tables/12 indexes a baseline schema this process never holds.

properties renders as the entry's detail, so a column that changed type reads Changed column public.users.email · columnType, isNullable.

An empty changes with changed: true renders nothing. Site derives changed from the patch, which still counts an oid that moved because the two snapshots came from different databases. That case leaves the gate row with nothing under it, which is worse than it sounds: the gate says the schema changed and lists no evidence. Site#3855 asks whether the gate should follow changes instead. This PR does not decide it.

Tests

github.test.ts renders a run whose metadata carries both the patch and the named list, and asserts the comment contains the column's name and not columns.17. It fails on main with the exact string this fixes.

schema-change.test.ts covers grouping by kind, properties as detail, and the empty list. The existing patch-path tests are unchanged and still pass, which is the fallback.

Full suite 425 passed across 43 files. npm run typecheck clean.

The comment printed a JSON Patch path when a change sat inside an object:
a column added to `ci_runs` read "Changed table · columns.17". The path
indexes a baseline schema this process never holds, so the reporter could
not resolve it into a name, and it dropped the element index entirely.

Site#3855 adds `metadata.schemaChange.changes`, the same delta with every
object named. Prefer it when the API sends it. The same run now reads
"Added column public.ci_runs.statistics_payload_id". An API that predates
the field still sends only the patch, and `entryFromOp` still handles it.

Site's summary also drops the positional churn the patch carries: a
column's attnum, which two databases at the same schema number
differently, and every oid. Those lines disappear from the comment.

Co-Authored-By: Claude <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query Doctor — 6 successful checks

 Cost regression — No query went up more than 5%
 Untested data access — No changed data-access file without a test
 New query — No new queries
 New query with index recommendation — No new query ships an index recommendation
 Schema drift — No schema changes
 High-value nudge — No index or rewrite past the threshold


More details via MCP → get_ci_run({ runId: "019fcf73-9854-79f3-acb7-272374a0f5b7" }) · view run · docs
3 queries read against main on assumed statistics of 10,000,000 rows per table. Sync production stats for costs measured against your real data.

@veksen
veksen merged commit 84e2ca8 into main Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant