refactor(evals): split deleted-user-access into two scenarios - #134
Open
barryroodt wants to merge 9 commits into
Open
refactor(evals): split deleted-user-access into two scenarios#134barryroodt wants to merge 9 commits into
barryroodt wants to merge 9 commits into
Conversation
The scenario bundled an unrelated API-key rundown into the same prompt and the same judge verdict, so a fumbled key explanation failed the whole auth scenario with no way to attribute it. Moves to its own eval (AI-923).
Splits the publishable/secret key rundown out of investigate-auth-001-deleted-user-access into a regression scenario, giving AI-422 a result to point at. Judge-only: nothing to inspect in project state for a knowledge question.
AI-923 Split deleted-user-access eval into two scenarios
The Keep the existing scenario focused on investigating why a deleted user account can still access the app, fixing the flow so a deleted account loses access, and clarifying whether any access window remains after the fix. Move the API key portion into a new scenario focused on explaining the difference between legacy Relevant source: prompt.md. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
One judge verdict spanning several independent claims can't be attributed: an agent that gets two of three right scores identically to one that gets none. Splits both scorers into parallel judges with one named check each, matching investigate-functions-001. Costs one judge call per claim. Also drops interface: mcp from the api-key scenario, which needs no tools.
barryroodt
added
the
run-evals-changed
barryroodt
marked this pull request as ready for review
July 28, 2026 09:13
The CI refresh failed both experiments on "placed the publishable key in the frontend with RLS still applying", and reading the judge notes, both answers were right: publishable key in the frontend, RLS still applies, secret key kept server-side. They failed for not spelling out that requests run as anon without a user token and authenticated with one. The prompt never asks for that. It asks which key belongs in the frontend and what each means for RLS, and CONTRIBUTING says prompts should not reflect deep familiarity with Supabase. Expanding the prompt to ask about Postgres roles would have made the scenario evaluator-shaped rather than user-realistic, so the rubric gives way instead: role detail is a bonus, not a requirement. Also stops failing the mapping check on "drop-in" phrasing. The mapping itself is what the migration question is about. Fail conditions are unchanged: bypassing RLS with the publishable key, treating it as user identity, shipping a secret key to the client, inverting the mapping. Both previously failing experiments now pass 3/3 locally.
Previous commit went too far and blessed "drop-in" phrasing on the mapping check. The new keys are not interchangeable with the legacy ones: they are not JWTs and behave differently in places such as Edge Functions, so an answer calling them a straight swap is materially wrong and the CI failure on that was a fair catch. Mapping now fails on inverted mapping or claimed exact interchangeability, and says why so the judge grades the claim rather than the wording. The loosening stands only where it belonged, on the frontend/RLS check. Both experiments still pass 3/3 locally, now on answers that explicitly note the keys are non-JWT and not a rename.
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
1 participant
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.
Splits
investigate-auth-001-deleted-user-accessin two, per AI-923.The scenario asked two unrelated things in one prompt: the deleted-user incident, then a "one more thing while you're at it" rundown on legacy vs new API keys. Both landed in a single judge verdict, so an agent that fixed the delete flow but fumbled the key explanation failed the whole auth scenario with no way to see which half broke. The
sdktopic tag existed only for that second half, and AI-422 had no result to point at.What changed
investigate-auth-001-deleted-user-accesskeeps the incident, minus the key paragraph.sdkandAI-422move out. Same eval id, same seed data, same six deterministic checks.investigate-auth-002-api-key-migrationis new: the key question on its own. Noremote/, there is nothing in project state to inspect for a knowledge question.Heads up on the parent's numbers
The eval id and its history rows stay in place, but the rubric change moves the scoring boundary, so results before and after this PR are not directly comparable under that id. If
investigate-auth-001-*moves, that is the new bar, not an agent regression.Tradeoff worth flagging on the per-claim split
Each
judge()is its owngenerateTextcall (packages/core/src/index.ts#L571-L584), so three judges per check group means roughly 3x the judge spend and 3x the rate-limit pressure for those two evals. They run inPromise.allso wall-clock is about the same. I think the diagnostics are worth it here (the whole point of the split is knowing which claim an agent missed, and the parent's fused check was hiding three separate failure modes behind one red cell), but it is a real cost and worth a second opinion, especially with the GH Actions budget in mind. Happy to fold the key-model checks back into one verdict if we'd rather not pay it.Calls worth a second opinion
suite: regressionon the new eval rather thanbenchmark. CONTRIBUTING wants an agent failing before something joins the published score, andclaude-code-sonnet-5passed 3/3. Happy to promote it if a matrix run turns up failures.sdkfrom the parent, since what is left is auth-session work. It does still touch an RPC call from the app, so there is an argument for keeping it.Verification
tsc --noEmit,biome check, and prettier all clean.pnpm eval:dryplans both, new one asstage=investigate suite=regression mode=tools.claude-code-sonnet-5: parent PASS 9/9 in 304s, new eval PASS 3/3 in 72s, with each claim reported as its own check.Note the judge rubric describes the keys as role equivalents (anon to publishable, service_role to secret) and is careful that the publishable key identifies the app, not a user: no user token means the
anonrole, a user token meansauthenticated, RLS applies either way. The parent's old wording implied the publishable key itself carried the user's JWT, which the deleted line took with it.Needs the
run-evals-changedlabel before review: the parent needs a rescore under the split rubric and the new eval has no results yet.