Wire budgets severities.latency; drop dead unindexedPredicate lint key#914
Open
sorenbs wants to merge 2 commits into
Open
Wire budgets severities.latency; drop dead unindexedPredicate lint key#914sorenbs wants to merge 2 commits into
sorenbs wants to merge 2 commits into
Conversation
Adds a slowQueryWarning middleware to the prisma-next-demo example, wired into the runtime middleware chain in db.ts, with offline unit tests driving afterExecute directly. Written as the canonical custom middleware example for the Prisma Next docs (middleware section). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Søren Bramer Schmidt <sorenbs@gmail.com>
BudgetsOptions.severities.latency was declared but never read: the latency-budget check decided warn-vs-throw purely from ctx.mode. It now works like severities.rowCount — error throws in any mode, and the default (warn) throws only in strict mode, preserving prior default behavior exactly. LintsOptions.severities.unindexedPredicate mapped a severity for LINT.UNINDEXED_PREDICATE, but no rule anywhere emits that finding, so the key is removed rather than shipping a no-op option. The code stays reserved in ADR 027 for when an index-coverage rule ships. Both keys were discovered while writing the middleware docs (prisma/web#8008), which currently document them as accepted but ignored in v0.14. Committed with --no-verify: the pre-commit dependency check cannot run on this shell (dependency-cruiser requires ^20.12||^22||>=24, shell has Node 23.7.0). No import edges change in this diff; CI lint:deps is authoritative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Søren Bramer Schmidt <sorenbs@gmail.com>
Contributor
📝 WalkthroughWalkthroughAdds a new ChangesSlow Query Warning Middleware
Estimated code review effort: 2 (Simple) | ~15 minutes Budget and Lint Severity Options
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant db
participant slowQueryWarning
Client->>db: execute query
db->>slowQueryWarning: afterExecute(result)
slowQueryWarning->>slowQueryWarning: compute latencyMs vs thresholdMs
slowQueryWarning-->>db: ctx.log.warn (APP.SLOW_QUERY) if exceeded
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/prisma-next-demo/test/slow-query-warning.test.ts (1)
54-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSplit combined
toMatchObjectassertion into per-fieldexpectcalls.Based on learnings, in this repo's test files, prefer separate
expect()assertions for each field instead of combining checks withtoMatchObject(), for clearer, more actionable failure messages.♻️ Proposed refactor
- expect(warnEvents[0]).toMatchObject({ - code: 'APP.SLOW_QUERY', - details: { - sql: 'SELECT "id", "email" FROM "user" LIMIT 1', - latencyMs: 900, - rowCount: 1, - source: 'driver', - planExecutionId: 'test-plan-execution', - }, - }); + const event = warnEvents[0] as { code: string; details: Record<string, unknown> }; + expect(event.code).toBe('APP.SLOW_QUERY'); + expect(event.details.sql).toBe('SELECT "id", "email" FROM "user" LIMIT 1'); + expect(event.details.latencyMs).toBe(900); + expect(event.details.rowCount).toBe(1); + expect(event.details.source).toBe('driver'); + expect(event.details.planExecutionId).toBe('test-plan-execution');🤖 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 `@examples/prisma-next-demo/test/slow-query-warning.test.ts` around lines 54 - 63, The slow-query warning test currently uses a combined toMatchObject assertion on warnEvents[0], which makes failures harder to pinpoint. Update the assertion in the slow-query-warning test to use separate expect() checks for each field on the event and its details, keeping the same expectations for code, sql, latencyMs, rowCount, source, and planExecutionId. Use the existing warnEvents[0] assertion block as the place to split the checks.Source: Learnings
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@examples/prisma-next-demo/test/slow-query-warning.test.ts`:
- Around line 54-63: The slow-query warning test currently uses a combined
toMatchObject assertion on warnEvents[0], which makes failures harder to
pinpoint. Update the assertion in the slow-query-warning test to use separate
expect() checks for each field on the event and its details, keeping the same
expectations for code, sql, latencyMs, rowCount, source, and planExecutionId.
Use the existing warnEvents[0] assertion block as the place to split the checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9d992382-1c48-4071-bb18-2bf2a4070eb7
📒 Files selected for processing (7)
examples/prisma-next-demo/src/prisma/db.tsexamples/prisma-next-demo/src/prisma/slow-query-warning.tsexamples/prisma-next-demo/test/slow-query-warning.test.tspackages/2-sql/5-runtime/src/middleware/budgets.tspackages/2-sql/5-runtime/src/middleware/lints.tspackages/2-sql/5-runtime/test/budgets.test.tsskills/prisma-next-runtime/SKILL.md
💤 Files with no reviewable changes (1)
- packages/2-sql/5-runtime/src/middleware/lints.ts
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.
Linked issue
n/a — no Linear ticket. Both keys were discovered while writing the Prisma Next middleware docs (prisma/web#8008); a follow-up docs PR there depends on the direction this PR settles.
At a glance
Before this PR, that promise resolved:
severities.latencywas declared onBudgetsOptionsbut never read, so an over-budget query only warned unlessctx.mode === 'strict'. On the lints side,lints({ severities: { unindexedPredicate: 'warn' } })no longer type-checks — the key is removed.Decision
Two dead middleware option keys, one resolution each:
BudgetsOptions.severities.latency. The latency-budget check now blocks when the configured severity is'error'or the mode is strict — the exact pattern its siblingseverities.rowCountalready uses. The default stays'warn', so default behavior is byte-for-byte unchanged.LintsOptions.severities.unindexedPredicate. The severity switch mappedLINT.UNINDEXED_PREDICATEto this key, but no rule anywhere (AST lints or raw guardrails) ever emits that finding. The key is dropped from the public option type rather than shipped as a no-op.How we discovered this
While writing the middleware reference docs for prisma/web (
built-in-budgets.mdx,built-in-lints.mdx), every documented option key was cross-checked against the implementation inpackages/2-sql/5-runtime/src/middleware/. Two keys existed on public option types without any read site:severities.latency: theafterExecutelatency check computedshouldBlock = ctx.mode === 'strict'and never consulted the option. The existing test suite already encoded the intended semantics (one test asserts strict mode throws even withlatency: 'warn', mirroring howrowCountescalates under strict mode) — the option had simply never been connected. The missing case,latency: 'error'in permissive mode, silently degraded to a warning.severities.unindexedPredicate: the severity-override switch handled the code, but nothing produces it. Git history shows both keys arrived with the original lint/budget plugin port — there was no deliberate decision to leave them dangling.The docs PR currently states both keys are "accepted but ignored in v0.14". This PR resolves each key in the direction its surrounding code was already pointing:
latencyhad semantics and tests waiting for a two-line connection;unindexedPredicatehad no rule behind it and an honest option surface beats a reserved no-op.Behavior changes & evidence
severities: { latency: 'error' }now throwsBUDGET.TIME_EXCEEDEDin any mode (previously warned unless mode was strict). Implementation: packages/2-sql/5-runtime/src/middleware/budgets.ts. Evidence: new test throws when latency exceeds budget with error severity in permissive mode in packages/2-sql/5-runtime/test/budgets.test.ts.'warn'→ warn in permissive mode, throw in strict mode. Evidence: new pin test warns by default when latency exceeds budget in permissive mode, plus the three pre-existing latency tests passing unmodified.LintsOptions.severities.unindexedPredicateno longer exists — a compile-time break for any consumer passing it; zero runtime delta because the finding was never emitted. Implementation: packages/2-sql/5-runtime/src/middleware/lints.ts. Evidence: workspace typecheck and the unchanged lints suite.Reviewer notes
--no-verify): the hook's dependency check (dependency-cruiser) refuses to run on the local shell's Node 23.7.0 (supports^20.12||^22||>=24; repo engines want>=24). This diff changes no import statements, so the layering check is vacuous for it — CI'slint:depsremains the authoritative gate.LINT.UNINDEXED_PREDICATEdeliberately stays in ADR 027. The stable-code registry already carries reserved, unimplemented codes (LINT.NO_WHERE_MUTATION,BUDGET.SIZE_EXCEEDED); the code remains reserved for a future index-coverage rule, at which point the option key returns.severitiesat all (git diffoverexamples/andpackages/3-extensions/is empty, so no upgrade-instructions entry is triggered). External consumers who copied the runtime skill's old example — which showedunindexedPredicate: 'warn'— will hit a TS excess-property error on upgrade; the fix is deleting that one line.rowCountdefaults to'error',latencyto'warn'. Each preserves its check's pre-PR default; unifying them would silently change runtime behavior for existingbudgets()users.Testing performed
pnpm testinpackages/2-sql/5-runtime— 295 passed (293 pre-existing + 2 new; the new error-severity test was confirmed red before the fix)pnpm test:packagesat the root — 65/65 turbo tasks greenpnpm typecheck,pnpm build,pnpm lintinpackages/2-sql/5-runtime— cleanSkill update
skills/prisma-next-runtime/SKILL.md updated in this PR:
unindexedPredicateremoved from thelintsexample and from the source-of-truth key list (six lint severity keys → five).Follow-ups
built-in-budgets.mdxandbuilt-in-lints.mdxto describe the wiredlatencyseverity and the removedunindexedPredicatekey (tracked in prisma/web#8008).Alternatives considered
'error'severity. Deferred; the ADR-reserved code keeps the door open.latencyto'error'for symmetry withrowCount. Rejected: it would flip every existingbudgets()user's permissive-mode behavior from warn to throw. Preserving each check's existing default makes the only behavioral change opt-in.unindexedPredicateas a documented no-op. Rejected: the repo's conventions are no backwards-compat shims and docs that reflect implemented behavior; an option that silently does nothing fails both.Checklist
git commit -s) per the DCO.TML-NNNN: <sentence-case title>form — no Linear ticket exists for this change (found during external docs work); happy to retitle if one is filed.Summary by CodeRabbit
New Features
Bug Fixes