Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/migrate-staging.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,26 @@
# separate `production` branch promotion, and auto-applying irreversible DDL to
# prod on merge is a different risk decision. This runner has no down
# migrations.
#
# THE SECRET MUST BE THE SESSION-MODE POOLER URI, NOT THE DIRECT ONE.
# `db.<ref>.supabase.co` publishes only an AAAA record, and GitHub-hosted
# runners have no outbound IPv6 — a direct string fails with "Network is
# unreachable" / "server closed the connection unexpectedly" before it ever
# authenticates. (Same wall on a home network without a global IPv6 address;
# it is why staging migrations had to be applied by hand.)
#
# Use the pooler host on port 5432 — SESSION mode. Not 6543 (transaction mode),
# which drops the session-level behaviour psycopg and DDL rely on. Session mode
# behaves like a direct connection. Note the pooler also changes the username to
# `postgres.<ref>`.
#
# Take the host from the dashboard's Connect panel rather than assembling it:
# projects are assigned to NUMBERED pooler clusters (`aws-0-`, `aws-1-`, ...)
# and the number is NOT derivable from the region — staging and production are
# both us-west-2 yet sit on different clusters. A wrong prefix fails with
# "Tenant or user not found", which is at least distinguishable from a bad
# password. `backend/scripts/pooler_url.py` builds the URI from an env file so
# the password is never copied by hand.
name: Migrate (staging)

on:
Expand DownExpand Up@@ -56,7 +76,7 @@ jobs:
SUPABASE_DB_URL: ${{ secrets.STAGING_SUPABASE_DB_URL }}
run: |
if [ -z "${SUPABASE_DB_URL}" ]; then
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret (the DIRECT connection string, port 5432, not the pooler) to enable."
echo "::notice::STAGING_SUPABASE_DB_URL is not set — skipping. Add the secret to enable: the SESSION-mode pooler URI on port 5432, user postgres.<ref>. Take the host from the dashboard's Connect panel — projects are assigned to NUMBERED pooler clusters (aws-0-, aws-1-, ...) and the number is not derivable from the region, so do not assume aws-0. 'python scripts/pooler_url.py .env.staging <cluster-prefix> --raw' builds the URI from the env file. Not the direct db.<ref>.supabase.co string — that is IPv6-only and unreachable from GitHub runners — and not port 6543, which is transaction mode."
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
Expand Down
8 changes: 6 additions & 2 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,14 +45,18 @@ python -m pytest tests/ -q # backend test suite
Database (run from `backend/`; migrations are raw DDL, never dashboard SQL):

```
python -m db.migrate # apply pending migrations (needs SUPABASE_DB_URL = direct conn string)
python -m db.migrate # apply pending migrations (SUPABASE_DB_URL = SESSION-mode pooler URI, port 5432)
python -m db.migrate --baseline # record migrations as applied without running them
python -m db.seed_staging # idempotent fake demo dataset on the new schema
```

The `db/` scripts read `.env` by default; for staging/prod ops run them under
`dotenv -f .env.staging run -- python -m db.<script>` so they hit the right project.
Migrations are immutable once applied — add a new numbered file, never edit an old one.
Migrations are immutable once applied — add a new timestamp-prefixed file
(`date -u +%Y%m%d%H%M%S`), never edit an old one. `SUPABASE_DB_URL` must be the
SESSION-mode pooler URI (port 5432, user `postgres.<ref>`); the direct
`db.<ref>.supabase.co` host is IPv6-only and unreachable from most networks, and
port 6543 is transaction mode and breaks DDL. `scripts/pooler_url.py` builds it.

Docker (full stack from repo root):

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -233,7 +233,7 @@ npm run dev # → http://localhost:3000
| `GOOGLE_CLIENT_SECRET` | — | Google OAuth client secret |
| `SESSION_SECRET` | — | HMAC secret for session tokens (min 32 bytes) |
| `ALLOWED_EMAIL_DOMAINS` | — | Comma-separated sign-in email-domain allowlist (default `bu.edu`). Empty value disables the check (any domain may sign in). |
| `SUPABASE_DB_URL` | — | Supabase **direct** connection string (port 5432, not the pooler) — used only by the `db.migrate` migration runner, never at app runtime |
| `SUPABASE_DB_URL` | — | Supabase **session-mode pooler** URI (port 5432, user `postgres.<ref>`) — used only by the `db.migrate` migration runner, never at app runtime. Not the direct `db.<ref>` host (IPv6-only, unreachable from most networks); not port 6543 (transaction mode, breaks DDL) |
| `LOGFIRE_TOKEN` | — | If set, traces ship to logfire.pydantic.dev. Without it, Logfire stays local-only. The Sapling scrubber redacts prompt/output content before egress regardless. |
| `SAPLING_MODEL_CLASSIFIER` | — | Override classifier-agent model (default `gemini-2.5-flash-lite`) |
| `SAPLING_MODEL_SUMMARY` | — | Override summary-agent model (default `gemini-2.5-flash-lite`) |
Expand DownExpand Up@@ -312,7 +312,7 @@ SAPLING_EVAL_UPDATE_BASELINES=1 python tests/evals/run_all.py # refresh baselin

## Migrations

Schema lives as ordered SQL files in `backend/db/migrations/` (numeric prefix = apply order, `0001`–`0030`). A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects directly with `psycopg` over the Supabase **direct** connection string (`SUPABASE_DB_URL`, not the pooler); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.
Schema lives as ordered SQL files in `backend/db/migrations/`, applied in filename order. New migrations use a UTC timestamp prefix (`date -u +%Y%m%d%H%M%S`); the legacy `NNNN_` files are frozen and must never be renamed, since the ledger keys on basename and a rename re-runs the migration. See `backend/db/migrations/README.md`. A minimal runner (`backend/db/migrate.py`) applies pending files in order and records each in a tracking table, so it's idempotent — re-running only applies what's new. The runner connects with `psycopg` over the **session-mode pooler** URI (`SUPABASE_DB_URL`, port 5432, user `postgres.<ref>`); this is the one sanctioned exception to the `db/connection.py::table()`-only convention, since runtime PostgREST can't execute DDL.

```bash
cd backend
Expand Down
29 changes: 24 additions & 5 deletions backend/db/migrate.py
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
"""Minimal migration runner for Supabase Postgres (#197).

App runtime uses db/connection.py::table() (PostgREST), which cannot execute DDL.
Migrations are raw DDL, so this admin tool connects directly with psycopg over the
Supabase *direct* connection string (SUPABASE_DB_URL, NOT the pooler). This is the
one sanctioned exception to the table()-only convention.
Migrations are raw DDL, so this admin tool connects with psycopg over
SUPABASE_DB_URL. This is the one sanctioned exception to the table()-only
convention.

WHICH CONNECTION STRING: the SESSION-mode pooler, port 5432.

This file used to say "the direct connection string, NOT the pooler". That
warning was about TRANSACTION mode (port 6543), which drops the session-level
behaviour psycopg and DDL depend on — and it is still correct about 6543. But
it predates Supabase moving the direct host to an IPv6-only endpoint:
db.<ref>.supabase.co now publishes only an AAAA record, so it is unreachable
from GitHub-hosted runners and from any network without a global IPv6 address.
Session mode behaves like a direct connection and its host publishes an A
record, so it is the reachable substitute — not a compromise.

Two details that are easy to miss: the pooler changes the username to
`postgres.<ref>`, and projects sit on NUMBERED clusters (`aws-0-`, `aws-1-`,
...) whose number is not derivable from the region. Take the host from the
dashboard's Connect panel, or let scripts/pooler_url.py assemble the URI from
an env file.

Usage:
SUPABASE_DB_URL=postgresql://... python -m db.migrate # apply pending
Expand DownExpand Up@@ -120,8 +137,10 @@ def main() -> int:
db_url = os.environ.get("SUPABASE_DB_URL", "").strip()
if not db_url:
print(
"ERROR: SUPABASE_DB_URL is not set "
"(Supabase → Settings → Database → Connection string → Direct).",
"ERROR: SUPABASE_DB_URL is not set (Supabase → Connect → "
"Session pooler, port 5432, user postgres.<ref>). The direct "
"db.<ref>.supabase.co host is IPv6-only and unreachable from most "
"networks; port 6543 is transaction mode and breaks DDL.",
file=sys.stderr,
)
return 1
Expand Down
204 changes: 204 additions & 0 deletions backend/scripts/migration_drift_report.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
"""Read-only drift report between backend/db/migrations/ and a live database.

Answers the question you must answer before applying a backlog of migrations to
an environment that has been touched outside the repo (#317): **is the ledger
merely behind, or is it lying?**

SUPABASE_DB_URL=... python scripts/migration_drift_report.py

Writes nothing. Runs no DDL. Safe against production.

Four sections:

PENDING — on disk, not in schema_migrations.
ORPHANS — in schema_migrations, not on disk. Means the environment ran SQL
that never existed in this repo (dashboard editor, ad-hoc script).
A filename collision here is the dangerous shape: staging having
recorded `0032_retire_summer_2026.sql` while the repo's own
`0032_rooms_missing_columns.sql` is pending means two different
migrations share a number.
ALREADY — objects a PENDING migration would create that ALREADY EXIST.
Every row here is the ledger lying: the schema moved without
being recorded, so "pending" overstates what will actually run.
Migrations written with IF NOT EXISTS will no-op safely; ones
without it will fail the whole run.
BLOCKERS — live rows that already violate a UNIQUE index a PENDING migration
would create. IF NOT EXISTS cannot save this one: it is a DATA
conflict, invisible to a schema-only diff, and it is what turns a
clean-looking backlog into a half-applied run.

The object list is parsed from the migration SQL itself (CREATE TABLE / ADD
COLUMN / CREATE INDEX), so it stays correct as migrations are added — nothing
to keep in sync by hand.

Exit codes: 2 no SUPABASE_DB_URL, 1 drift found (no ledger, orphans, a
non-idempotent collision, or a data blocker), 0 clean. Nonzero means "do not
apply on top of this" — so the report can gate CI directly rather than being
re-implemented inline, which is how the workflow preflight and this script
drifted apart in the first place.
"""
from __future__ import annotations

import os
import pathlib
import re
import sys

import psycopg

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
ast-grep outline backend/db/connection.py --items all
rg -n -C2 'psycopg\.connect|def table\b|schema_migrations' \
backend/db/connection.py backend/db/migrate.py backend/scripts/migration_drift_report.py

Repository: SaplingLearn/Sapling

Length of output: 4274


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== migrations script =="
cat -n backend/scripts/migration_drift_report.py
echoecho"== migrate imports/context =="
sed -n '1,110p' backend/db/migrate.py | cat -n
echoecho"== all psycopg usage =="
rg -n 'psycopg|migration_drift_report|schema_migrations|information_schema|db/connection\.py::table\b|BackendTable|SupabaseTable' backend -g '*.py'

Repository: SaplingLearn/Sapling

Length of output: 16548


Add an approved read-only catalog access boundary.

backend/scripts/migration_drift_report.py opens a direct psycopg connection at line 60. The allowed backend/db/migrate.py exception is for raw DDL, not this read-only catalog-check script. Route this through a catalog helper/API that remains under db/connection.py::table() or document a narrow read-only exception before merge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 37, Replace the direct
psycopg connection in migration_drift_report.py with the approved read-only
catalog helper/API exposed through db/connection.py::table(), preserving the
script’s catalog-query behavior. If no suitable helper exists, document and
implement a narrowly scoped read-only exception rather than reusing the
migrate.py raw-DDL exception.

Source: Coding guidelines


MIGRATIONS = pathlib.Path(__file__).resolve().parent.parent / "db" / "migrations"

RE_TABLE = re.compile(r"create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_.]+)", re.I)
RE_COLUMN = re.compile(
r"alter\s+table\s+([a-z0-9_.]+)\s+add\s+column\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)",
re.I,
)
RE_INDEX = re.compile(r"create\s+(?:unique\s+)?index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)", re.I)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
rg -n -i --glob '*.sql' \
'create\s+(unique\s+)?index\s+concurrently\b' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 158


🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect the regex usage and migration files of interest without executing repo code.
wc -l backend/scripts/migration_drift_report.py
sed -n '1,120p' backend/scripts/migration_drift_report.py
printf'\nSQL migration files creating indexes:\n'
rg -n -i --glob '*.sql''create\s+(?:unique\s+)?index\b' backend/db/migrations | head -50

Repository: SaplingLearn/Sapling

Length of output: 10931


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY syntax grammar

💡 Result:

In PostgreSQL, the CONCURRENTLY option is used with the CREATE INDEX command to build an index without locking out concurrent writes (inserts, updates, or deletes) to the table [1][2]. The syntax grammar for creating an index concurrently is [1][2]: CREATE [ UNIQUE ] INDEX CONCURRENTLY [ [ IF NOT EXISTS ] name ] ON [ ONLY ] table_name [ USING method ] ( { column_name | ( expression) } [ COLLATE collation ] [ opclass [ ( opclass_parameter = value [,... ]) ] ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [,...]) [ INCLUDE ( column_name [,...]) ] [ NULLS [ NOT ] DISTINCT ] [ WITH ( storage_parameter [= value] [,... ]) ] [ TABLESPACE tablespace_name ] [ WHERE predicate ] Key considerations for this command include: 1. Operational Impact: While it avoids blocking writes, the command requires two scans of the table and must wait for existing transactions that could modify or use the index to terminate [1][2]. 2. Performance: It requires more total work and takes significantly longer to complete than a standard index build [1][2]. It also imposes extra CPU and I/O load on the database [1][2]. 3. Failure Recovery: If the build fails (e.g., due to a deadlock or a duplicate key violation), the index is left in an invalid state. The recommended recovery method is to drop the invalid index and retry the CREATE INDEX CONCURRENTLY command [2].

Citations:


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS supported since version grammar

💡 Result:

The syntax CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported in PostgreSQL version 10 and all subsequent versions [1]. The IF NOT EXISTS clause was introduced for the CREATE INDEX command in PostgreSQL 10 to allow index creation without throwing an error if an index with the same name already exists [1]. When using this syntax, the index name is mandatory [2][1]. The CONCURRENTLY option allows the index to be built without locking out concurrent writes to the table [2][3]. As of PostgreSQL 10 and later, the general grammar for this command is: CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] ON table_name [ USING method ] ( { column_name | ( expression) } [... ]) [... ] [2][1]

Citations:


Support CREATE INDEX CONCURRENTLY in RE_INDEX.

RE_INDEX ignores CONCURRENTLY before the index name. When a pending migration uses the valid CREATE INDEX CONCURRENTLY ... form, the report can skip its existing-index collision check and return clean while the migration still fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` at line 56, Update RE_INDEX to
optionally match the CONCURRENTLY keyword between the optional IF NOT EXISTS
clause and the index name, while preserving existing CREATE INDEX and CREATE
UNIQUE INDEX forms.


# A UNIQUE index is the one thing in a pending migration that IF NOT EXISTS
# cannot make safe: it still fails if the live data already violates it. That
# is a DATA problem, invisible to a schema-only diff, and it is what turns a
# clean-looking backlog into a half-applied run. Parsed so the check follows
# whatever migrations are actually pending.
RE_UNIQUE_INDEX = re.compile(
r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+"
r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?",
re.I | re.S,
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
# Inspect pending migration syntax that the current regular expression can skip.
rg -n -i --glob '*.sql' \
'create\s+unique\s+index|create\s+unique\s+index\s+concurrently|using\s+\w+|nulls\s+(not\s+)?distinct' \
backend/db/migrations

Repository: SaplingLearn/Sapling

Length of output: 2285


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== script outline =="
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"== relevant ranges =="
sed -n '1,240p' backend/scripts/migration_drift_report.py
echo"== migration samples =="forfin backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0022_unique_nulls.sql 2>/dev/null \
backend/db/migrations/0027_gradescope.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 7436


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import reRE_UNIQUE_INDEX = re.compile( r"create\s+unique\s+index\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s+" r"on\s+([a-z0-9_.]+)\s*\(([^)]*)\)(?:\s*where\s+([^;]+))?", re.I | re.S,)cases = { "plain": 'CREATE UNIQUE INDEX IF NOT EXISTS documents_request_id_user_unique ON documents USING btree (request_id, user_id)', "concurrently": 'CREATE UNIQUE INDEX idx_unique ON users USING btree (email) CONCURRENTLY', "quoted_index_table_columns": 'CREATE UNIQUE INDEX "My Index" ON "My Schema"."My Table" ("Column One", column_two)', "nulls_not_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email) NULLS NOT DISTINCT', "nulls_distinct": 'CREATE UNIQUE INDEX idx_u ON users (email NULLS DISTINCT)', "partial": 'CREATE UNIQUE INDEX idx_partial ON users (email) WHERE (active)', "with_index_params": 'CREATE UNIQUE INDEX idx_ivfflat ON courses USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)',}for label, sql in cases.items(): m = RE_UNIQUE_INDEX.search(sql) print(f"{label}: matched={bool(m)}") if m: print(" groups =", m.groups()) else: print(" sql =", sql)# Extract existing migrations that call this parser's supported formfrom pathlib import Pathfor path in sorted(Path("backend/db/migrations").glob("*.sql")): text = path.read_text(errors="ignore") for m in RE_UNIQUE_INDEX.finditer(text): print(f"EXISTS {path}:{m.start()}: idx={m.group(1)} table={m.group(2)} cols={m.group(3)[:80]} where={m.group(4)[:40] if m.group(4) else None}")PYforfin \
backend/db/migrations/0039_rag_vector_store.sql \
backend/db/migrations/0018_documents_request_id.sql \
backend/db/migrations/0020_academics_split.sql \
backend/db/migrations/0021_gradebook.sql \
backend/db/migrations/0023_graph_integrity.sql \
backend/db/migrations/0036_offering_null_section_unique.sql;do
[ -f"$f" ] ||continueecho"--- $f ---"
sed -n '1,90p'"$f"done

Repository: SaplingLearn/Sapling

Length of output: 18440


Cover the full CREATE UNIQUE INDEX grammar checked by the report.

RE_UNIQUE_INDEX only matches the CREATE UNIQUE INDEX ... ON table(columns) [WHERE ...] form. It skips USING index_type and CONCURRENTLY clauses, and it can miss quoted identifiers. Add parser coverage for these supported forms, or report them as unchecked rather than printing no data blockers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 53 - 56, Update
RE_UNIQUE_INDEX and the migration-report parsing flow to cover supported CREATE
UNIQUE INDEX variants, including CONCURRENTLY, USING index_type, and quoted
identifiers. For any syntax the parser still cannot recognize, classify it as
unchecked instead of silently producing no data blockers.

)


def bare(name: str) -> str:
return name.split(".")[-1]
Comment on lines +70 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"## locate file"
git ls-files | grep -F 'backend/scripts/migration_drift_report.py'||trueecho"## outline"
ast-grep outline backend/scripts/migration_drift_report.py ||trueecho"## relevant lines"
wc -l backend/scripts/migration_drift_report.py
sed -n '1,180p' backend/scripts/migration_drift_report.py | cat -n
echo"## references to bare/schema_migrations/public/auth"
rg -n "bare|schema_migrations|\.migrate|search_path|current_schema|public|auth|table_schema|schemaname|obj_schema" backend/scripts/migration_drift_report.py backend/db ||true

Repository: SaplingLearn/Sapling

Length of output: 19317


🏁 Script executed:

#!/bin/bashset -euo pipefail
python3 - <<'PY'import refrom pathlib import Pathp = Path('backend/scripts/migration_drift_report.py')if not p.exists(): print('missing backend/scripts/migration_drift_report.py') raise SystemExittext = p.read_text()print('## parsed sections')for start, name in [(48, 'bare'), (61, 'report_catalog_operations'), (98, 'probe_ledger')]: lines = text.splitlines() print(f'\n--- {name} at line {start+1} ---') limit = 160 print('\n'.join(f'{i+1}: {line}' for i in range(max(0, start-limit), min(len(lines), start+limit+limit+20))))print('\n## schema/index name extraction occurrences')for m in re.finditer(r'\b[bare]\s*\(', text): print('bare call around line', text.count('\n',0,m.start('bare')+1)+1)print('bare imports:', bool(re.search(r'from.*import.*bare|import.*utils|sys.path', text)))for m in re.finditer(r'\.split\("[.]\"', text[:200]): print('split found at', text.count('\n',0,m.start())+1, 'context=', text[max(0,m.start()-80):m.end()+80])print('\n## catalog filter fields')for m in re.finditer(r'(SELECT .*?FROM .*?(\n.*?){0,8}.*?WHERE)', text, re.S): if 'catalog' in text[:m.start()] or 'migration_drift' in text[:m.start()]: passprint('index usage around ',"n/a")PY

Repository: SaplingLearn/Sapling

Length of output: 354


Keep schema-qualified identifiers in the drift checks.

bare() drops schema qualification, then the catalog queries filter only by table_name, column_name, or indexname. PostgreSQL allows identically named tables or indexes in different schemas. A pending migration targeting public.foo can be reported as conflicting because auth.foo exists, and an unqualified search can read schema_migrations from an unintended search-path schema instead of the migration schema.

Use the parsed schema from qualified names. For unqualified names, apply the same schema rules used by backend/db/migrate.py and avoid relying only on the default search path. Apply this to the table, column, and index existence queries at lines 101, 109, and 118.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 49 - 50, Update
bare() and the catalog queries in the drift-check flow to preserve parsed schema
qualification instead of filtering only by object names. Apply the same
schema-resolution rules as backend/db/migrate.py for unqualified names, and
ensure the table, column, and index checks use the resolved schema explicitly
rather than relying on the search path.



def main() -> int:
url = os.getenv("SUPABASE_DB_URL")
if not url:
print("SUPABASE_DB_URL is not set", file=sys.stderr)
return 2

files = sorted(p.name for p in MIGRATIONS.glob("*.sql"))
conn = psycopg.connect(url)

has_ledger = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
("schema_migrations",),
).fetchone()[0]
if not has_ledger:
print("NO LEDGER — this database has never been migrated by db/migrate.py.")
print("Reconcile with `python -m db.migrate --baseline` against a schema you")
print("have verified is current, rather than applying.")
return 1

recorded = {
r[0] for r in conn.execute("SELECT filename FROM schema_migrations").fetchall()
}
pending = [f for f in files if f not in recorded]
orphans = sorted(recorded - set(files))

print(f"on disk {len(files)} | recorded {len(recorded)} | pending {len(pending)}\n")

print("PENDING")
for p in pending:
print(f" {p}")
if not pending:
print(" none")

print("\nORPHANS (recorded here, absent from the repo)")
for o in orphans:
clash = [f for f in files if f.split("_")[0] == o.split("_")[0]]
note = f" <-- NUMBER COLLIDES WITH {clash[0]}" if clash else ""
print(f" {o}{note}")
if not orphans:
print(" none")

# Which objects the pending migrations would create, that already exist.
print("\nALREADY EXISTS (pending migration, object already present)")
found_any = False
unsafe_already = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
hits: list[str] = []

for tbl in {bare(t) for t in RE_TABLE.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.tables WHERE table_name = %s",
(tbl,),
).fetchone()[0]
if n:
hits.append(f"table {tbl}")

for tbl, col in {(bare(t), c) for t, c in RE_COLUMN.findall(sql)}:
n = conn.execute(
"SELECT count(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
(tbl, col),
).fetchone()[0]
if n:
hits.append(f"column {tbl}.{col}")

for idx in set(RE_INDEX.findall(sql)):
n = conn.execute(
"SELECT count(*) FROM pg_indexes WHERE indexname = %s", (idx,)
).fetchone()[0]
if n:
hits.append(f"index {idx}")

if hits:
found_any = True
idempotent = "if not exists" in sql.lower()
if not idempotent:
unsafe_already = True
flag = "safe: uses IF NOT EXISTS" if idempotent else "!! NO IF NOT EXISTS — would fail"
print(f" {name} ({flag})")
Comment on lines +147 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report idempotence for each conflicting operation.

idempotent checks whether IF NOT EXISTS occurs anywhere in the migration. A migration can contain CREATE TABLE IF NOT EXISTS and a later non-idempotent ADD COLUMN. If the column already exists, this report labels the migration safe even though db/migrate.py will fail.

Track the idempotence modifier with each extracted table, column, or index. Render the safety result for each detected conflict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 125 - 129, Update the
conflict extraction and reporting flow around the migration parser and the
`idempotent`/`flag` logic so idempotence is evaluated for each extracted table,
column, or index operation rather than across the entire migration SQL. Preserve
the existing conflict detection, but associate each detected conflict with its
own `IF NOT EXISTS` status and render that status in the corresponding output
line.

for h in sorted(hits):
print(f" {h}")
if not found_any:
print(" none — the ledger is behind, not lying")

# Data blockers: a pending UNIQUE index that live rows already violate.
print("\nDATA BLOCKERS (pending UNIQUE index vs rows already present)")
blocked = False
for name in pending:
sql = (MIGRATIONS / name).read_text()
for idx, table, cols, pred in RE_UNIQUE_INDEX.findall(sql):
cols_sql = ", ".join(c.strip() for c in cols.split(","))
where = f" WHERE {pred.strip()}" if pred else ""
try:
dupes = conn.execute(
f"SELECT {cols_sql}, count(*) FROM {bare(table)}{where} "
f"GROUP BY {cols_sql} HAVING count(*) > 1 LIMIT 5"
).fetchall()
Comment on lines +168 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"Files matching migration_drift_report.py:"
fd -a 'migration_drift_report.py'.||true
file="$(fd 'migration_drift_report.py'.| head -n 1)"if [ -n"${file:-}" ];thenechoecho"Line count:"
wc -l "$file"echoecho"Outline:"
ast-grep outline "$file"||trueechoecho"Relevant section lines 120-180:"
sed -n '120,180p'"$file"| nl -ba -v120
fiechoecho"Search for duplicate/unique report symbols:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE""${file:-backend/scripts/migration_drift_report.py}"||trueechoecho"Search for SQL parser definitions around duplicate checks:"
rg -n "def .*duplicate|class .*Duplicate|parse|definition|constrain" backend -S ||true

Repository: SaplingLearn/Sapling

Length of output: 850


🏁 Script executed:

#!/bin/bashset -euo pipefail
file="backend/scripts/migration_drift_report.py"echo"Line count:"
wc -l "$file"echoecho"Relevant lines 1-175 with Python-line-number-style numbering:"
python3 - <<'PY'from pathlib import Pathp = Path("backend/scripts/migration_drift_report.py")for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}")PYechoecho"Search relevant terms in file:"
rg -n "dupes|count\\(\\*\\)|IS NOT NULL|NULLS NOT DISTINCT|UNIQUE|unique|where|cols_sql|KEYS|RE_KEY""$file" -S ||trueechoecho"Read-only behavioral probe for PostgreSQL UNIQUE duplicate semantics from parser docs snippet if available, plus local duplicate logic shape:"
python3 - <<'PY'from pathlib import Pathimport rep = Path("backend/scripts/migration_drift_report.py")src = p.read_text()print("has null_not_distinct_term:", "NULLS NOT DISTINCT" in src)print("has null check before duplicate group:", "IS NOT NULL" in src and "GROUP BY" in src)for name, regex in [ ("RE_KEY", r"(?<=RE_KEY = re\.compile\()('|\")(.*)\\1"), ("RE_UNIQUE_INDEX", r"(?<=RE_UNIQUE_INDEX = re\.compile\()('|\")(.*)\\1"),]: m = re.search(regex, src, re.S) print(f"{name}: {m.group(2) if m else None}")PY

Repository: SaplingLearn/Sapling

Length of output: 9653


🌐 Web query:

PostgreSQL unique index NULL treated as distinct nulls not distinct

💡 Result:

In PostgreSQL, the handling of NULL values in unique indexes and unique constraints is controlled by the NULLS DISTINCT or NULLS NOT DISTINCT clauses [1][2]. By default, PostgreSQL treats NULL values as distinct [1][3]. This means that a unique index or constraint will allow multiple rows to contain a NULL value in the indexed column(s), as they are not considered equal to each other [1][2]. To change this behavior so that NULL values are treated as equivalent (and thus a unique constraint will reject multiple NULLs), you can specify the NULLS NOT DISTINCT clause when creating the index or the constraint [1][4][2]. Example usage for an index: CREATE UNIQUE INDEX name ON table (column) NULLS NOT DISTINCT; Example usage for a table constraint: CREATE TABLE products ( product_no integer UNIQUE NULLS NOT DISTINCT,...); Alternatively, you can explicitly specify the default behavior by using NULLS DISTINCT [2][5]. As noted in the PostgreSQL documentation, because the SQL standard leaves null treatment in unique constraints as implementation-defined, it is important to be aware of this setting when writing portable applications [2][5].

Citations:


🌐 Web query:

PostgreSQL unique index WHERE NULLs not distinct where predicate

💡 Result:

In PostgreSQL, the NULLS NOT DISTINCT clause is used with unique indexes to define how NULL values are handled regarding uniqueness constraints [1][2]. By default, PostgreSQL treats NULL values as distinct, meaning a unique index (or unique constraint) allows multiple rows to contain a NULL value in the indexed column(s) [1][2]. When you specify NULLS NOT DISTINCT, the index treats NULL values as equal to one another [1][3]. Consequently, if a table has this type of unique index, it will reject any attempt to insert or update a row if another row already exists with a NULL value in that same column (or combination of columns) [1][4]. Syntax: CREATE UNIQUE INDEX index_name ON table_name (column_name) NULLS NOT DISTINCT; This clause can be applied to both manually created unique indexes and unique constraints [1][2]. It is important to note that this feature is applicable to B-tree indexes [1][4]. Additionally, you can combine this with a WHERE predicate to create a partial unique index that treats NULLs as non-distinct only for the subset of rows that satisfy the predicate [2][5].

Citations:


Exclude NULLs from unique-index duplicate checks unless the index declares NULLS NOT DISTINCT.

The report checks GROUP BY counts, so multiple rows with NULL in any key column are reported as duplicates even though a default PostgreSQL unique index allows them. Add IS NOT NULL filters for each key column unless the parsed CREATE UNIQUE INDEX includes NULLS NOT DISTINCT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 155 - 158, Add a
null-exclusion predicate to the duplicate query built in the migration drift
report: require every unique-index key column to be non-NULL by default, but
skip this filter when the parsed CREATE UNIQUE INDEX declares NULLS NOT
DISTINCT. Combine the generated predicates with the existing where clause before
the GROUP BY query in the duplicate-check flow.

except Exception as exc: # table may not exist yet — that's fine
conn.rollback()
print(f" {name}: {idx} — could not check ({type(exc).__name__})")
continue
if dupes:
blocked = True
print(f" {name}: {idx} WOULD FAIL — duplicate rows exist:")
for d in dupes:
print(f" {d}")
Comment on lines +176 to +180

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print raw duplicate key values.

d contains database values from arbitrary unique-index columns. This can place email addresses, tokens, or other sensitive values in CI logs. Print the index name and duplicate count by default. Allow raw values only through an explicit local diagnostic mode with redaction.

As per coding guidelines, “Protect sensitive columns with the encryption helpers … at read boundaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/scripts/migration_drift_report.py` around lines 163 - 167, Update the
duplicate-reporting block in the migration drift report to stop printing raw
values from each duplicate entry. By default, print only the index name and
duplicate count; if an explicit local diagnostic mode is enabled, allow values
through the established redaction/encryption helpers before printing. Keep
setting blocked and reporting the affected index unchanged.

Source: Coding guidelines

if not blocked:
print(" none")

# Exit nonzero on anything that makes "apply the backlog" unsafe, so this
# can gate CI as-is. PENDING alone is not drift — that is the normal state
# of an environment that is merely behind.
problems = []
if orphans:
problems.append(f"{len(orphans)} orphan(s) recorded but absent from the repo")
if unsafe_already:
problems.append("an object already exists for a migration without IF NOT EXISTS")
if blocked:
problems.append("live rows already violate a pending UNIQUE index")
if problems:
print("\nDRIFT — do not apply on top of this:")
for p in problems:
print(f" {p}")
return 1

return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading