feat: supabase-js SDK evals (auth flow, relational report, key migration) - #129
feat: supabase-js SDK evals (auth flow, relational report, key migration)#129mandarini wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
f1b1b25 to
216ca87
Compare
mandarini
added
run-evals-changed
mandarini
added
the
run-evals-changed
mandarini
removed
the
run-evals-changed
mandarini
added
run-evals-changed
barryroodt
left a comment
barryroodt
left a comment
There was a problem hiding this comment.
These four are in good shape, and pulling the expected numbers out of the seeded migrations instead of hardcoding them is the right call. The auth driver holding one session across signup and both signin attempts is nice too.
One thing to sort before merge: implementation uses @supabase/supabase-js is the only failed check in all 13 benchmark failures, and the grep behind it doesn't prove SDK use. The pattern requires a quote before the package name but nothing after it (EVAL.ts:198), so @supabase/supabase-js-not-real matches, and an unused import passes just as well as a real one. So those three evals currently score whether a string appears somewhere under app/, which makes the codex/claude split hard to read.
My call is to fix the detection rather than name the SDK in the prompt, since unprompted reach is the thing you're measuring and that only means something if the gate is real. Resolving the import from the scored entry, or instrumenting module load, would both do it.
Rest is inline. The resolve-sdk-001 one is worth a look, a hardcoded stub currently passes it 4/4.
| const NAME = 'implementation uses @supabase/supabase-js'; | ||
| const scan = await ctx.exec( | ||
| `grep -rlE --exclude-dir=node_modules --include='*.mjs' --include='*.js' --include='*.cjs' --include='*.ts' ` + | ||
| `"['\\"](npm:)?@supabase/supabase-js" ${APP_DIR} || true` |
There was a problem hiding this comment.
"['\"](npm:)?@supabase/supabase-js" requires an opening quote but nothing after the package name, so it also matches @supabase/supabase-js-not-real. The comment above says we match the quoted specifier rather than a bare mention, and grep -rl over all of app/ means a commented-out line or a dead helper satisfies it. Since this check is what gates every failure here, could we resolve the import from the scored entry instead?
Same pattern at build-dataapi-001/EVAL.ts:174 and build-dataapi-002/EVAL.ts:162.
| const expectedDrafts = Number(draftRows[0]?.n ?? -1); | ||
|
|
||
| // 1. The public script still lists published posts (client-key path). | ||
| const posts = await ctx.exec(`cd ${APP_DIR} && npm run -s posts`, { |
There was a problem hiding this comment.
The four checks can all pass without any key migration happening. publishableKey and secretKey get read at lines 27-28, then only used for the guard, never injected into npm run posts / npm run stats and never compared against what the app ends up using.
So a script that prints the seeded titles and draft count, with no Supabase call at all, passes: check 1 and 2 match on output, check 3 sees no legacy marker, check 4 sees no secret reference. Worth asserting the new keys are actually in use, maybe by injecting them and failing when the app ignores them.
| written.stderr.trim() || written.stdout.trim() | ||
| ); | ||
| } | ||
| const run = await ctx.exec( |
There was a problem hiding this comment.
This runs the driver without installing app deps first, while the other three scorers do a guarded install before their run. A solution that adds @supabase/supabase-js to package.json but leaves no node_modules behind dies here on ERR_MODULE_NOT_FOUND, so the score depends on whether the agent happened to install rather than on what it submitted. Same guarded install before this line?
| // Be generous about a missing install step; the eval is about the report, | ||
| // not npm. A no-op when the agent already installed dependencies. | ||
| await ctx.exec( | ||
| `cd ${APP_DIR} && [ -d node_modules ] || npm install --no-audit --no-fund --silent || true`, |
There was a problem hiding this comment.
The || true swallows install failures, and the result isn't inspected. A registry hiccup or a bad lockfile then surfaces later as ERR_MODULE_NOT_FOUND from the script, which reads as an agent mistake in the results. Could we keep the install and push an explicit failed setup check with its stderr when it breaks?
Same at build-dataapi-002/EVAL.ts:58 and resolve-sdk-001/EVAL.ts:39.
| motivation: >- | ||
| build-dataapi-001-relational-report found every codex variant (0/4) skips | ||
| @supabase/supabase-js for a bare backend Data API script, hand-rolling raw | ||
| HTTP instead, while every claude-code variant (4/4) reached for it |
There was a problem hiding this comment.
The 4/4 claim for claude-code is already stale at this head, build-dataapi-001 shows claude-code-sonnet-5-no-skills failing, so it's 3/4. Since the split is the stated reason for adding this scenario, the number matters.
Also worth a user-facing citation here alongside the internal comparison, CONTRIBUTING.md asks for evidence from the user journey and right now this cites only the sibling eval. Same number in README.md:5.
| .env | ||
| .env*.local | ||
| # eval seed data may include a .env with well-known local demo keys | ||
| !evals/*/local/**/.env |
There was a problem hiding this comment.
!evals/*/local/**/.env un-ignores .env for every eval, including ones that don't exist yet, so a future seed can commit a real credential without tripping the guard. deploy-functions-001 shows a forced add works for an exact fixture. Scope it to evals/resolve-sdk-001-legacy-key-migration/local/app/.env?
| @@ -0,0 +1 @@ | |||
| v2.110.0 No newline at end of file | |||
There was a problem hiding this comment.
These look like generated CLI state that got picked up by the add. No other eval tracks .temp/ or .branches/, and this one records v2.110.0 while the prompt pins 2.109.1. Since local/ is copied into the agent workspace they'd land as seed data, so probably worth deleting all four (.temp/cli-latest and .branches/_current_branch here and under build-dataapi-002) and ignoring both dirs.
| `cd ${APP_DIR} && SUPABASE_URL="${apiUrl}" SUPABASE_SECRET_KEY="${secretKey}" node ${REPORT}`, | ||
| { timeoutMs: 60_000 } | ||
| ); | ||
| const actual = parseReport(run.stdout); |
There was a problem hiding this comment.
Minor: this parses stdout without looking at run.ok or the exit code, so a script that prints the right JSON and then exits 1 still passes. Same at build-dataapi-002/EVAL.ts:66.
2 participants
Adds four evals for supabase-js: dedicated coverage for the SDK's two most-used surfaces (auth and the PostgREST query builder), a regression canary for the API key migration, and a second Data API scenario probing an SDK-adoption gap the first one turned up.
build-auth-001-email-password-flow(benchmark): signup/sign-in/profile flow against stubbed app code. Gates on agents reaching for@supabase/supabase-jsunprompted (thebuild-functions-005pattern); the profile display name only survives if sent as signup user metadata.build-dataapi-001-relational-report(benchmark): backend reporting script over a seeded relational schema (embedded relations, aggregation). Gates on using the SDK query builder via the Data API rather than psql or a raw Postgres driver.build-dataapi-002-restock-alert-report(benchmark): same shape asbuild-dataapi-001— bare prompt, emptypackage.json, no SDK named — but a different schema/domain (inventory restock alerts vs. a sales report). Added after refreshed results onbuild-dataapi-001showed every claude-code variant (4/4) reaching for@supabase/supabase-jsunprompted while every codex variant (0/4) hand-rolled raw HTTP instead, despite otherwise-correct, RLS-safe implementations. One scenario isn't enough to tell a real model tendency from an artifact of that specific prompt, so this companion checks whether the split generalizes.resolve-sdk-001-legacy-key-migration(regression): seeds a working app on the legacy demo JWTs; agents must migrate it tosb_publishable_/sb_secret_keys without breaking behavior or leaking the secret key into client code.All four were sanity-checked locally against a real stack on the pinned CLI (2.109.1): migrations apply, reference solutions pass every scorer check, and the checks discriminate (missing signup metadata, publishable-key impersonation, leftover legacy keys, and SDK vs. raw-HTTP implementations all fail as intended). A scoped
.gitignoreexception ships the key-migration eval's seeded.env, which holds only the well-known local demo keys.Fix along the way:
build-dataapi-001-relational-reportandresolve-sdk-001-legacy-key-migrationinitially failed 100% of agents identically — a harness bug, not an agent gap. Both had omittedgotruefrom theirservices:frontmatter, so the local stack's auth service never started andsupabase statuscouldn't emitPUBLISHABLE_KEY/SECRET_KEY. Fixed by addinggotruetoservices:(matchingresolve-dataapi-002-secure-default-grants's pattern); results refreshed and now produce real signal.build-dataapi-002-restock-alert-reportstill needs a live run (no API keys in this pass) to confirm whether the adoption split holds.