Skip to content

fix(schema): re-sync schema.sql with the chats-overview migration; make the drift gate required - #330

Draft
amal66 wants to merge 1 commit into
Open-Legal-Products:mainfrom
amal66:olp-pr/schema-sql-chats-rpc-sync
Draft

fix(schema): re-sync schema.sql with the chats-overview migration; make the drift gate required#330
amal66 wants to merge 1 commit into
Open-Legal-Products:mainfrom
amal66:olp-pr/schema-sql-chats-rpc-sync

Conversation

@amal66

@amal66 amal66 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

TL;DR

The Schema drift gate ("Fresh install vs upgraded deployment") has been red on main since 57d754c. This PR makes backend/schema.sql carry the exact text of the get_chats_overview definition that migration 20260812_01_collection_pagination_queries.sql ships — a formatting-only change, no semantic difference — which restores the fresh-install and upgrade paths to identical fingerprints. It also proposes making this check required on main (instructions below — needs repo admin).

Root cause

57d754c ("fix: harden collection pagination and directory loading") rewrote get_chats_overview in the migration with the ownership predicate split across four lines:

     or (
       p.id is not null
       and p.user_id = p_user_id
     )

while schema.sql kept the same predicate on one line. Postgres stores SQL-language function bodies as opaque strings, so the drift gate — correctly — compares them textually: same semantics, different bytes, fingerprints diverge. Every push to main and every PR rebased onto it has carried the red check since (e.g. #251's rebase inherited it).

The fix

Exactly what the gate's own failure message prescribes: a migration was merged without being folded into schema.sql, so update schema.sql — never the already-shipped migration. After this change the entire get_chats_overview definition is byte-identical between backend/schema.sql and the migration (verified by extracting and diffing both definitions). One file, formatting only; the deployed function this produces is unchanged.

Verification

The Schema drift workflow runs on this PR — its result on this branch is the test, since the failing comparison (baseline + all migrations vs. this schema.sql) is exactly what the job executes.

Making this check required (needs admin — @willchen96)

This incident is the argument: the gate caught the drift immediately, but nothing stopped the merge, so main stayed red and every open PR inherited a failure it didn't cause. The Protect main ruleset currently requires a PR with one approval but no status checks at all. To make this one blocking:

  1. Merge this PR first (flipping the switch while the check still fails would leave the fix itself hard to land, and other PRs stay red until they pick up the fix from main).
  2. Settings → Rules → Rulesets → "Protect main" → Add a new rule: "Require status checks to pass" → add check Fresh install vs upgraded deployment (source: GitHub Actions). Optionally enable "Require branches to be up to date before merging" so the comparison always runs against current migrations.
  3. While there, consider also requiring the other gates that already run on every PR (Backend build and tests, Frontend build and tests, dependency-audit (root/backend/frontend), gitleaks (full history)) — today none of them block a merge either.

I'd have flipped it directly, but ruleset edits need admin on the repo and this account only has push.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XHBa7CRB3MBRyy3YJd7h8z

….sql

The schema-drift gate compares a fresh install (schema.sql) against an
upgraded deployment (baseline + replayed migrations) by fingerprinting
both databases. Function bodies are opaque strings to Postgres, so they
compare textually — and 57d754c shipped migration
20260812_01_collection_pagination_queries.sql rewriting
get_chats_overview with the ownership predicate split across four lines
while schema.sql kept the one-line form. Same semantics, different
bytes: the gate has been red on main and on every PR rebased onto it
since.

schema.sql now carries the migration's exact text — the whole function
definition is byte-identical between the two files — which is the
gate's own prescription: when a migration merges without being folded
into schema.sql, schema.sql is what gets updated, never the
already-shipped migration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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