TML-2693: include adapter + driver capabilities in createExecutionContext (parity with CLI emit)#602
Conversation
|
Warning Review limit reached
More reviews will be available in 31 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR centralizes capability-matrix merging via a new export ChangesCapability Matrix Merge Implementation and Adoption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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/extension-cipherstash
@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/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/emitter
@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: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/integration/test/sql-builder/setup.ts (1)
116-116: ⚡ Quick winUse the stack’s driver descriptor for capability folding to avoid drift.
Line 116 passes
postgresDriver, but the stack was created with a wrapped driver descriptor. Reusingstack.driverkeeps merged capabilities aligned with the exact descriptor used to instantiate runtime.Proposed change
- context = createExecutionContext({ contract: sqlContract, stack, driver: postgresDriver }); + context = createExecutionContext({ contract: sqlContract, stack, driver: stack.driver });🤖 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 `@test/integration/test/sql-builder/setup.ts` at line 116, The test is instantiating the execution context with the raw postgresDriver which can desynchronize capability folding from the wrapped driver used to create the stack; update the call to createExecutionContext to pass the stack's actual driver descriptor (use stack.driver) instead of postgresDriver so that merged capabilities remain consistent with the runtime that instantiated the stack (keep sqlContract and stack the same).
🤖 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.
Inline comments:
In `@packages/1-framework/1-core/framework-components/src/shared/capabilities.ts`:
- Around line 16-29: Replace the unsafe "as" casts in sortDeep: import blindCast
from "`@prisma-next/utils/casts`" (or named import blindCast) and use blindCast<T,
"sortDeep preserves runtime shape, but the recursive generic type relationship
can’t be expressed to TS">(...) to cast the mapped array result and the final
next object instead of using "as unknown as T" and "as T"; alternatively, change
sortDeep to operate on unknown internally and perform a single
blindCast<T,...>(result) at the function boundary so only one explicit cast
remains.
In `@packages/2-sql/5-runtime/test/sql-context.test.ts`:
- Line 928: The test currently compares serialized JSON strings —
expect(JSON.stringify(context.contract.capabilities)).toBe(JSON.stringify(expected))
— which is brittle; change it to a structural deep equality assertion such as
expect(context.contract.capabilities).toEqual(expected) (or toStrictEqual if you
want strict matching) so the test compares the actual objects
(context.contract.capabilities vs expected) directly for clearer diffs and more
robust comparisons.
In `@test/integration/test/sql-builder/setup.ts`:
- Line 24: The unsafe cast using "as Contract" on the result of new
SqlContractSerializer().deserializeContract(contract) should be replaced with
the repository's cast helper to make the unsafe boundary explicit; locate the
deserialization call that assigns to sqlContract and replace the bare cast with
the repo's cast/assert helper (e.g., castToContract or similar) so the value
returned by SqlContractSerializer.deserializeContract is validated/annotated via
the central cast utility rather than a raw TypeScript assertion.
---
Nitpick comments:
In `@test/integration/test/sql-builder/setup.ts`:
- Line 116: The test is instantiating the execution context with the raw
postgresDriver which can desynchronize capability folding from the wrapped
driver used to create the stack; update the call to createExecutionContext to
pass the stack's actual driver descriptor (use stack.driver) instead of
postgresDriver so that merged capabilities remain consistent with the runtime
that instantiated the stack (keep sqlContract and stack the same).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 762f10e4-4022-483a-878d-bd0f7d9bd5bd
📒 Files selected for processing (9)
packages/1-framework/1-core/framework-components/src/exports/components.tspackages/1-framework/1-core/framework-components/src/shared/capabilities.tspackages/1-framework/1-core/framework-components/test/capabilities.test.tspackages/1-framework/3-tooling/cli/src/control-api/contract-enrichment.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/execution-stack.test.tspackages/2-sql/5-runtime/test/parameterized-types.test.tspackages/2-sql/5-runtime/test/sql-context.test.tstest/integration/test/sql-builder/setup.ts
…untime + emit reuse (TML-2693) The CLI emit path (`enrichContract`) and the SQL runtime (`createExecutionContext`) both need to fold a stack of component descriptors capabilities into a single matrix. Until now the primitive lived inline in `contract-enrichment.ts`, so the runtime had no way to call it without dragging the CLI into its dependency graph. This change lifts the inline `mergeCapabilities` + `extractCapabilityMatrix` logic out of `contract-enrichment.ts` into a new shared helper `mergeCapabilityMatrices` in `@prisma-next/framework-components/components`, with its own unit tests. `enrichContract` now delegates to it; its public output is unchanged for any type-valid Contract (`Contract.capabilities` is non-optional `Record<string, Record<string, boolean>>`). The helper also treats a non-object `base` as the empty matrix rather than throwing, which keeps `enrichContract` byte-identical for the existing CLI tests that pass an unsafely-cast mock contract. D1 of TML-2693: lays the substrate. D2 wires `createExecutionContext` to call the same helper; D3 removes the test-side `enrichContract` workaround in `test/integration/test/sql-builder/setup.ts`. Signed-off-by: Will Madden <madden@prisma.io>
…up (TML-2693) Addresses CodeRabbit review on #602: - `capabilities.ts`: refactor `sortDeep` to operate on `unknown` internally and blindCast only once at the `mergeCapabilityMatrices` return boundary, replacing three bare `as` casts. Restores the lint:casts delta to 0. - `sql-context.test.ts`: swap the `JSON.stringify(...).toBe(JSON.stringify(...))` full-stack equality assertion for a direct `.toEqual` — both compare the same structurally sorted matrix, but the structural assertion gives clearer diffs. - `test/integration/test/sql-builder/setup.ts`: replace the bare `as Contract` cast on the deserializer boundary with `blindCast<Contract, "…">` so the unsafe boundary is named at the call site. No behaviour change. Signed-off-by: Will Madden <madden@prisma.io>
74454e5 to
4ccf24e
Compare
…t (TML-2693)
In-process consumers that build a contract via `defineContract({ family, target, extensionPacks }, …)` and hand it to `createExecutionContext` previously saw only the target + extension-pack capabilities baked into the IR; adapter and driver contributions landed only via the CLI emit path through `enrichContract`. Runtime capability gates (`sql.returning`, `postgres.distinctOn`, …) therefore rejected operations the adapter actually supported.
This change folds the same merge that `enrichContract` performs at emit time into the runtime by reusing the shared `mergeCapabilityMatrices` helper (extracted in the preceding refactor). After the existing requirements check, the runtime computes the merged matrix from target + adapter + (optional) driver + extension packs and rebinds the local contract so the entire downstream pipeline — codec / queryOperation / mutationDefault collection and the returned `ExecutionContext.contract` — observes the merged matrix.
`createExecutionContext` gains a new optional `driver?: RuntimeDriverDescriptor<…>` field on its options. The driver is consulted *only* as a capability source here; its lifecycle (connect/close) stays where it is, on `instantiateExecutionStack` + `runtime`. Existing callers that do not pass `driver` continue to work unchanged. Three pre-existing tests that asserted referential equality of `context.contract` to the input were relaxed to structural equality, since the returned contract is now always a fresh object carrying the merged matrix.
TML-2693 D2. D1 (commit 6ddac17) extracted the shared helper; D3 will drop the test-side `enrichContract` workaround in `test/integration/test/sql-builder/setup.ts` and feed the driver descriptor to `createExecutionContext` via the new option.
Signed-off-by: Will Madden <madden@prisma.io>
…p (TML-2693) With createExecutionContext now folding stack capabilities at runtime (990956b, building on the shared mergeCapabilityMatrices helper from 6ddac17), the in-memory contract path no longer needs the CLI emit-time enrichment as a test-side workaround. Pass the unenriched in-memory contract directly to SqlContractSerializer .deserializeContract and supply postgresDriver via the new options.driver field so the runtime fold sees the driver capabilities the same way enrichContract did. Removes the only test-side consumer of @prisma-next/cli/control-api as a runtime workaround. The remaining authoring/ usages of enrichContract are legitimate CLI-emit-parity tests and are out of scope here. Signed-off-by: Will Madden <madden@prisma.io>
…up (TML-2693) Addresses CodeRabbit review on #602: - `capabilities.ts`: refactor `sortDeep` to operate on `unknown` internally and blindCast only once at the `mergeCapabilityMatrices` return boundary, replacing three bare `as` casts. Restores the lint:casts delta to 0. - `sql-context.test.ts`: swap the `JSON.stringify(...).toBe(JSON.stringify(...))` full-stack equality assertion for a direct `.toEqual` — both compare the same structurally sorted matrix, but the structural assertion gives clearer diffs. - `test/integration/test/sql-builder/setup.ts`: replace the bare `as Contract` cast on the deserializer boundary with `blindCast<Contract, "…">` so the unsafe boundary is named at the call site. No behaviour change. Signed-off-by: Will Madden <madden@prisma.io>
Replace bare casts in mergeCapabilityMatrices, use structural equality in capability folding tests, and pass stack.driver for integration setup so capability folding matches the instantiated stack descriptor. Signed-off-by: Will Madden <madden@prisma.io>
4ccf24e to
bd5d3eb
Compare
|
Actionable comments posted: 0 |
…-2693) `stack.driver` is typed as `RuntimeDriverDescriptor<…> | undefined` because the field is optional on `SqlExecutionStack`, which trips `exactOptionalPropertyTypes` when passed to `createExecutionContext`. Bind the (cursor-disabled) driver descriptor to a local variable and feed the same value to both `createSqlExecutionStack` and `createExecutionContext` so both observe a narrow non-optional type without a non-null assertion. Signed-off-by: Will Madden <madden@prisma.io>
…up (TML-2693) Addresses CodeRabbit review on #602: - `capabilities.ts`: refactor `sortDeep` to operate on `unknown` internally and blindCast only once at the `mergeCapabilityMatrices` return boundary, replacing three bare `as` casts. Restores the lint:casts delta to 0. - `sql-context.test.ts`: swap the `JSON.stringify(...).toBe(JSON.stringify(...))` full-stack equality assertion for a direct `.toEqual` — both compare the same structurally sorted matrix, but the structural assertion gives clearer diffs. - `test/integration/test/sql-builder/setup.ts`: replace the bare `as Contract` cast on the deserializer boundary with `blindCast<Contract, "…">` so the unsafe boundary is named at the call site. No behaviour change. Signed-off-by: Will Madden <madden@prisma.io>
At a glance
In-process consumers building a contract via
defineContract(...)and handing it tocreateExecutionContextnow see the same capability matrix the CLI-emitted contract carries. The integration tests forsql-builderhad been working around the gap by pre-enriching test-side; the workaround is gone:The change
createExecutionContext(in@prisma-next/sql-runtime) now folds the stack's capability contributions into the contract before exposing it to the runtime — using the same merge primitiveenrichContractalready uses at CLI emit time. Both callers go through the shared helper, so the matrix the runtime sees is byte-identical to the matrix the CLI emits.createExecutionContext's options gain an optionaldriver?field so in-memory consumers can pass{ adapter, driver }and get the full match.Why the gap existed
PR #574 (TML-2653) removed
capabilitiesfromdefineContract. Capability authoring moved out of the contract and into the stack components — target, adapter, driver, extension packs — each contributing through their descriptor's optionalcapabilitiesfield. The CLI'senrichContractfolds those contributions together at emit time and writes the merged matrix intocontract.json.The runtime had no equivalent fold. An in-process consumer building a contract via
defineContract({ family, target, extensionPacks }, …)and handing it tocreateExecutionContextsaw only target + extension-pack capabilities. The adapter's and driver's contributions never reached the matrix runtime gates check against, so calls likesql.returningorpostgres.distinctOnwere rejected for operations the adapter actually supports.Structured for review
Three commits, dependency-ordered for a reviewer who wants to walk it:
6ddac1795— extractmergeCapabilityMatricesinto@prisma-next/framework-components/components. Pure refactor;enrichContract's public output is byte-identical (the existingcontract-enrichment.test.tsstayed green unchanged).990956bff— call the helper fromcreateExecutionContext. Adds the optionaldriver?option; rebinds the localcontractso all downstream construction and the returnedExecutionContext.contractcarry the merged matrix.74454e508— delete the test-side workaround intest/integration/test/sql-builder/setup.ts.Things to scrutinize
mergeCapabilityMatricestakesReadonlyArray<{ capabilities?: unknown }>rather thanTargetBoundComponentDescriptor[]. The runtime caller'sSqlExecutionStackdescriptors satisfy the structural shape directly;enrichContractkeeps its ownTargetBoundComponentDescriptor[]parameter and narrows internally.basehandling in the helper. A malformed runtimebase(only reachable via unsafe casts) returns the empty matrix rather than throwing onObject.entries(undefined). Pinned by an explicit "tolerates a malformed base" test so the tolerance isn't load-bearing on accident.toBe(contract)assertions relaxed totoEqual(contract)insql-context.test.ts,execution-stack.test.ts, andparameterized-types.test.ts.createExecutionContextnow always returns a fresh contract object carrying the merged matrix, even when no contributor adds anything — identity equality is structurally false. None of the three tests were about contract identity; they used.toBeincidentally. The identical assertion in Mongo'smongo-execution-stack.test.tsis deliberately untouched (Mongo runtime is out of scope).Out of scope
enrichContract's shape (theextensionPacksMetafold;sortDeepof the rest of the contract) — left as-is.extensionPacksMetaat runtime — emit-time-only metadata; runtime doesn't gate on it.profileHash/ capability-hash regeneration — emit-time concern; in-memory merge doesn't replay hashing.test/integration/test/authoring/that legitimately callenrichContractto assert emit shape.Alternatives considered
Wiring driver capabilities into the runtime. Three options were on the table:
SqlExecutionStackWithDrivervariant. Either pass the WithDriver shape intocreateExecutionContext, or widenSqlExecutionStackitself. Rejected because the driver is only a capability source here — its lifecycle is wired separately viainstantiateExecutionStack+runtime.connect. Reading it off the stack would leak a structural quirk into the function's input shape and make the capability-source intent implicit rather than documented.{ adapter, driver }would still see only adapter and target capabilities at runtime.driver?field tocreateExecutionContext's options. Strictly additive — every existing caller compiles unchanged. The driver is consulted only as a capability source; runtime driver lifecycle is unchanged. Matches AC3's wording literally.Helper home —
framework-components/componentsvscontract. The helper takes contributor-shaped input.framework-componentsis whereTargetBoundComponentDescriptorlives and is already a foundation layer that both CLI tooling and SQL runtime depend on;@prisma-next/contractis type-only.Parity test against
enrichContractdirectly vs against the shared helper. AssertingenrichContract(contract, …).capabilities === createExecutionContext(...).contract.capabilitiesinsql-runtime's package-local tests would import@prisma-next/cli/control-api— upward-layer forsql-runtime, and would either triplint:depsor require a layering exception. The chosen approach asserts both sides against the shared helper; byte-equality follows by transitivity (the helper extraction commit pinsenrichContract↔ helper; the runtime-fold commit pinscreateExecutionContext↔ helper).