Repository navigation
chore(scripts): read the models catalog from Neon instead of Supabase - #157
Merged
Merged
Conversation
The Supabase project is being deleted. gen:models now queries public.ai_models in Neon over DATABASE_URL with pg (numeric columns cast to float8 so the snapshot keeps JSON numbers).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![mogplex[bot]](https://avatars.githubusercontent.com/in/3107927?s=60&v=4){kind=small}
There was a problem hiding this comment.
Mogplex PR Review
Status: Attention needed
Approve-ready change: the swap from Supabase PostgREST to a direct Neon read in scripts/gen-models-table.ts is faithful — same filters, ordering, column set and JSON shape — and pg@8.23.1 carries no known advisories. Fold in three things before merge: the published docs and AGENTS.md still say the snapshot comes from Supabase, the DATABASE_URL contract (shape, sslmode, least-privilege role) is undocumented, and the new SQL has no coverage in tests/.
4 findings were added inline.
Warnings
- Published docs still say the snapshot comes from Supabase (
content/docs/web/models.mdx)content/docs/web/models.mdxtells readers: "The snapshot is regenerated from the Supabaseai_modelstable bypnpm gen:models." After this change the source is Neon, and the Supabase project is being deleted, so that statement is wrong on a page that ships to users.- Update the sentence (and any other Supabase wording on the page) in this PR or a follow-up.
AGENTS.mdcarries the same stale wording in two places: "Refreshsrc/data/ai-models.jsonfrom Supabase" and "src/data/ai-models.jsonis a snapshot of the Supabaseai_modelstable" — the repo's own agent guidance will keep sending contributors at the dead project.- Nothing else in
package.jsonorsrc/references the old Supabase keys, so the doc strings are the only remaining Supabase trace.
| const DATABASE_URL = requireEnv('DATABASE_URL'); | ||
|
|
||
| const OUTPUT = resolve(process.cwd(), 'src/data/ai-models.json'); | ||
|
|
There was a problem hiding this comment.
Warning: Document the required DATABASE_URL shape (sslmode) and prefer a read-only role
new Client({ connectionString: DATABASE_URL })carries no TLS configuration of its own.- node-postgres defaults to
ssl: falseunless the connection string carriessslmode=…(orPGSSLMODEis set), and Neon "rejects connections that do not use SSL/TLS" — so aDATABASE_URLcopied without the query parameter fails with a confusing server-side error instead of the script's own "Missing DATABASE_URL" message. - Document the expected value, e.g.
postgresql://<user>:<pass>@<neon-host>/<db>?sslmode=require, in the script header comment and/or the PR description. - The
requireEnvfailure message only says "Add it to .env.local first"; make it say where to get the URL and thatsslmodeis required. - The script only needs
selectonpublic.ai_models; prefer a dedicated role withGRANT SELECTon that table over handing every contributor a full production credential, since the connection string now carries a real password instead of the old public anon key. - No credential exposure found:
.env*.localis gitignored soDATABASE_URLstays out of the repo, andpnpm-lock.yamlis updated sopnpm install --frozen-lockfilein CI still resolves.
| capabilities, | ||
| pricing_input::float8 as pricing_input, | ||
| pricing_output::float8 as pricing_output, | ||
| pricing_cache_read::float8 as pricing_cache_read, |
There was a problem hiding this comment.
Suggestion: Note the row-count jump before the snapshot is refreshed
- The committed
src/data/ai-models.jsonis a ~20 KB file, which can hold at most ~80 model entries at the current formatting, while the Neon query returns 177 rows. - The catalog cardinality therefore differs substantially between the two sources, consistent with the old PostgREST path being filtered by anon-role RLS.
- The snapshot is intentionally not refreshed in this PR, so the next
pnpm gen:modelswill grow the rendered table oncontent/docs/web/models.mdxseveral times over. - Worth a follow-up PR that refreshes the snapshot, eyeballs the page, and confirms the extra rows are genuinely visible to users rather than rows RLS was intentionally hiding.
| rows = (await client.query<Row>(QUERY, [PAGE_SIZE])).rows; | ||
| } finally { | ||
| await client.end(); | ||
| } |
There was a problem hiding this comment.
Suggestion: Consider an explicit ssl option instead of relying on the URL
- Passing
sslin the config object andsslmodein the connection string interact: per the node-postgres SSL docs, thesslobject is replaced when the URL containssslcert/sslkey/sslrootcert/sslmode. - If you ever add an explicit
ssl: { rejectUnauthorized: true }(or Neon'sca.pemforverify-full), decide deliberately which source of truth wins. - Note the choice in a comment so a future reader does not assume the in-code option is authoritative.
| // the snapshot shape the PostgREST export produced. | ||
| const QUERY = ` | ||
| select | ||
| id, |
There was a problem hiding this comment.
Suggestion: Consider testing the query independently of the live database
- The SQL is now embedded in the script, so nothing in
tests/covers the filters (is_hidden = false and is_available = true), the ordering, or the column set. - A typo in a column name only surfaces when someone runs the script against production.
- The repo already runs
node --test tests/*.test.mjs; a small test asserting the query string contains the expected filters and ordering would help. - Alternatively, a dry-run mode that prints the planned output without writing the file keeps the same benefit without a test.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Mogplex Supabase project (
enxvgkxsrpbtqlaeotjz) is being deleted. Production has served from Neon since Aug 3.pnpm gen:modelsstill readai_modelsfrom Supabase over PostgREST, so it was producing a catalog frozen at the cutover and will stop working once the project is gone.What changes
scripts/gen-models-table.tsqueriespublic.ai_modelsin Neon overDATABASE_URL, usingpg. It keeps the same filters (is_hidden = false,is_available = true), ordering and output shape. Numeric columns are cast tofloat8so the JSON keeps number types.pgand@types/pgare added as dev dependencies..env.localneedsDATABASE_URL(read access to Mogplex Neon) instead of the twoNEXT_PUBLIC_SUPABASE_*keys.The snapshot itself (
src/data/ai-models.json, last refreshed Jul 2) is not refreshed in this PR. A test run against Neon wrote 177 models with the right types. That run was reverted to keep this PR to the data-source change.Verification
pnpm types:check: passpnpm lint: passpnpm test: 18 passpnpm exec tsx scripts/gen-models-table.tsagainst Neon prod (read-only): wrote 177 models; output revertedNeed help on this PR? Tag
@codesmith-botwith what you need.