Conversation
TML-2025 Follow-up: pack-provided mutation default functions registry (remove PSL hardcoding)
ContextMilestone 5 ( That solves parity, but the default-function vocabulary and per-column applicability are still effectively hard-coded (even if moved out of the interpreter). Canonical spec + plan
GoalDesign and implement a framework-component contribution surface so targets/adapters/extension packs can provide:
Then wire PSL provider and TS authoring surfaces to consume this vocabulary as the single source of truth. Acceptance criteria
References
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAn extensible mutation-defaults system was introduced: default-function handlers and generator descriptors are composed from contributors and passed into the PSL interpreter. Generator IDs moved from enumerated literals to regex-constrained flat strings validated at runtime and resolved from composed registries; runtime now enforces generator ownership and uniqueness. Changes
Sequence Diagram(s)sequenceDiagram
participant PSL as PSL Document
participant Interp as PSL Interpreter
participant Scalars as Scalar Descriptors<br/>(composed)
participant Defaults as Default-Function Registry<br/>(composed)
participant Generators as Generator Registry<br/>(composed)
participant IR as Contract IR
PSL->>Interp: provide document + target + scalar descriptors + controlMutationDefaults
Interp->>Scalars: resolve field scalar -> ColumnDescriptor
Interp->>Defaults: lower default function calls (if any)
Defaults-->>Interp: returns storage default OR execution generator spec
alt execution generator produced
Interp->>Generators: lookup generator by id
Generators-->>Interp: GeneratorDescriptor (or missing)
Interp->>Generators: check applicableCodecIds vs field.codecId
Generators-->>Interp: optionally resolveGeneratedColumnDescriptor
end
Interp->>IR: emit resolved fields, storage defaults, execution defaults
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
@prisma-next/runtime-executor
@prisma-next/sql-runtime
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/contract-authoring
@prisma-next/contract-ts
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/cli
@prisma-next/emitter
@prisma-next/eslint-plugin
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@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/family-sql
@prisma-next/sql-kysely-lane
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-lane
@prisma-next/target-postgres
@prisma-next/adapter-postgres
@prisma-next/driver-postgres
@prisma-next/core-control-plane
@prisma-next/core-execution-plane
@prisma-next/config
@prisma-next/contract
@prisma-next/operations
@prisma-next/plan
@prisma-next/utils
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
docs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.md (1)
49-49: Clarify "For this slice".The phrase "For this slice" is ambiguous. Consider specifying whether this means "in this initial implementation", "for this architectural aspect", or "in this PR scope". Clearer wording would help readers understand the scope and any planned evolution.
Suggested alternatives
-For this slice, applicability is keyed by `codecId` only. +In this initial implementation, applicability is keyed by `codecId` only.Or, if referring to architectural scope:
-For this slice, applicability is keyed by `codecId` only. +For mutation default applicability, the key is `codecId` only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/adrs/ADR 169 - Declared applicability for mutation default generators.md at line 49, The phrase "For this slice" is ambiguous; update the sentence containing "For this slice, applicability is keyed by `codecId` only." to explicitly state the scope—e.g., "In this ADR, applicability is keyed by `codecId` only." or "In this initial implementation, applicability is keyed by `codecId` only."—so readers know whether the statement refers to the ADR, the initial implementation, or the PR scope; pick the most accurate scope and apply that wording consistently throughout the document.packages/2-sql/2-authoring/contract-ts/src/contract.ts (1)
80-82: Consider extracting the generator ID pattern to avoid duplication.This regex pattern is duplicated in
packages/2-sql/1-core/contract/src/validators.ts(lines 25-27). Both use the same pattern for validating flat generator IDs.♻️ Suggested approach
Extract the pattern to a shared constant in
@prisma-next/sql-contractthat both validators can import:// In `@prisma-next/sql-contract/validators` or a shared constants file export const GENERATOR_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]*$/;This ensures the validation logic stays in sync across both packages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts` around lines 80 - 82, Duplicate regex for generator IDs is used in GeneratorIdSchema and the validators in packages/2-sql/1-core/contract/src/validators.ts; extract it to a shared exported constant (e.g., GENERATOR_ID_PATTERN) in the `@prisma-next/sql-contract` package and import it from both places. Replace the inline regex in GeneratorIdSchema and the other validator with a reference to GENERATOR_ID_PATTERN, keeping the existing narrow(...) logic (use the pattern.test(value) call) so behavior is unchanged. Update exports so the constant is available to both packages and adjust import statements in files that used the inline regex.packages/2-sql/3-tooling/family/src/core/assembly.ts (1)
261-290: Avoid multi-passcontrolMutationDefaults()evaluation in assembly.The current flow evaluates descriptor contributions in separate passes (generator map helper + function loop). If a descriptor computes contributions dynamically, this can drift across passes. Prefer one pass that collects both generators and functions from the same snapshot.
♻️ Proposed refactor (single-pass collection)
export function assembleControlMutationDefaultContributions( descriptors: ReadonlyArray<SqlControlDescriptorWithContributions>, ): AssembledControlMutationDefaultContributions { const defaultFunctionRegistry = new Map<string, ControlMutationDefaultFunctionEntry>(); const functionOwners = new Map<string, string>(); - const generatorMap = createControlMutationDefaultGeneratorDescriptorMap(descriptors); + const generatorMap = new Map<string, ControlMutationDefaultGeneratorDescriptor>(); + const generatorOwners = new Map<string, string>(); for (const descriptor of descriptors) { const contributions = descriptor.controlMutationDefaults?.(); if (!contributions) { continue; } + + for (const generatorDescriptor of contributions.generatorDescriptors) { + const owner = generatorOwners.get(generatorDescriptor.id); + if (owner) { + throw new Error( + `Duplicate mutation default generator id "${generatorDescriptor.id}". Descriptor "${descriptor.id}" conflicts with "${owner}".`, + ); + } + generatorMap.set(generatorDescriptor.id, generatorDescriptor); + generatorOwners.set(generatorDescriptor.id, descriptor.id); + } for (const [functionName, handler] of contributions.defaultFunctionRegistry) { const owner = functionOwners.get(functionName); if (owner) { throw new Error( `Duplicate mutation default function "${functionName}". Descriptor "${descriptor.id}" conflicts with "${owner}".`, ); } defaultFunctionRegistry.set(functionName, handler); functionOwners.set(functionName, descriptor.id); } } return { defaultFunctionRegistry, generatorDescriptors: Array.from(generatorMap.values()), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/3-tooling/family/src/core/assembly.ts` around lines 261 - 290, The assembly currently calls createControlMutationDefaultGeneratorDescriptorMap(descriptors) and then iterates descriptors again calling descriptor.controlMutationDefaults() a second time, which can yield different results for dynamic contributions; change assembleControlMutationDefaultContributions to perform a single-pass: for each descriptor call descriptor.controlMutationDefaults() once, add its generator descriptors into a local generatorMap (mirroring the shape createControlMutationDefaultGeneratorDescriptorMap produces) and simultaneously merge its defaultFunctionRegistry entries into defaultFunctionRegistry while checking duplicates via functionOwners; finally return defaultFunctionRegistry and Array.from(generatorMap.values())—remove the separate createControlMutationDefaultGeneratorDescriptorMap call and ensure you reference descriptor.id when reporting conflicts.packages/1-framework/2-authoring/ids/src/index.ts (1)
115-118: UseifDefined()instead of inline conditional spread forparams.This segment should follow the repo’s standardized conditional spread helper.
♻️ Proposed refactor
const resolvedDescriptor = resolveBuiltinGeneratedColumnDescriptor({ id, - ...(params ? { params } : {}), + ...ifDefined('params', params), });As per coding guidelines: "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/2-authoring/ids/src/index.ts` around lines 115 - 118, Replace the inline conditional spread used when building the args for resolveBuiltinGeneratedColumnDescriptor: instead of spreading ...(params ? { params } : {}), use the repository helper ifDefined from `@prisma-next/utils/defined` and spread ...ifDefined({ params }) so the call becomes resolveBuiltinGeneratedColumnDescriptor({ id, ...ifDefined({ params }) }); ensure you import ifDefined if it's not already present and adjust the import list accordingly; keep references to resolvedDescriptor, resolveBuiltinGeneratedColumnDescriptor, id, and params.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/2-authoring/ids/src/index.ts`:
- Line 143: The file is re-exporting builtinGeneratorIds from src/index.ts which
violates the rule against cross-file re-exports outside an exports/ surface;
locate the original definition of builtinGeneratorIds (the module that defines
that symbol) and either (a) add the export there or (b) create/declare it under
this package's exports/ surface and export it from that file, then remove the
cross-file re-export line "export { builtinGeneratorIds };" from src/index.ts so
the package API only exposes symbols via the designated exports/ surface.
In `@packages/1-framework/2-authoring/ids/src/runtime.ts`:
- Around line 5-8: Replace the cast lookup that uses (spec.id as
BuiltinGeneratorId) with a type guard using Object.hasOwn to safely narrow the
key before accessing idGenerators: check Object.hasOwn(idGenerators, spec.id)
(or equivalent predicate) and only then read idGenerators[spec.id] into
generator; if the guard fails, throw the same Error for unknown built-in ID.
This removes the blind cast while keeping the same behavior around idGenerators,
BuiltinGeneratorId, and spec.id.
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Line 800: The current double-cast on the builder variable bypasses type
safety; replace "as unknown as DynamicContractBuilder" with the concrete
SqlContractBuilder type (or remove the cast entirely) so builder is
declared/initialized as the SqlContractBuilder returned by
defineContract().target(input.target); update the builder declaration to use
SqlContractBuilder (or let TypeScript infer it) so you can call storageType(),
table(), model(), and build() without blind casts and preserve type safety.
In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts`:
- Around line 32-35: Add a short inline comment explaining the intentional
double-cast on the InterpretPslDocumentToSqlContractIRInput argument to
interpretPslDocumentToSqlContractIRInternal: state that the double-cast (as
unknown as InterpretPslDocumentToSqlContractIRInput) is used only in this test
to bypass stricter typing for test setup/mocking and is permitted per the
“test-only escapes” guideline; place the comment directly above or beside the
double-cast to make intent explicit.
In `@packages/2-sql/4-lanes/sql-lane/test/utils/guards.test.ts`:
- Line 96: The test case named "collects ColumnRefs from nested OperationExpr"
uses a raw numeric timeout ({ timeout: 1_000 }) which violates the project's
guideline; replace the numeric literal with the shared test timeout constant
from `@prisma-next/test-utils` (e.g., use timeouts.unit or another appropriate
timeouts.* constant) in the it(...) options so the test reads the timeout from
the shared helper rather than a raw number.
In `@packages/2-sql/5-runtime/README.md`:
- Around line 23-24: The Architecture section in README.md omits the new
mutation default generator concept, causing inconsistency with the earlier lines
that add "Mutation Default Generator Composition" and "No Implicit Generator
Baseline"; update the Architecture list (referencing SqlStaticContributions) to
include mutation default generators and explain that SqlStaticContributions now
aggregates composed default generators from target/adapter/extension
contributors and that the runtime resolves generator ids only from those
composed contributors (no builtin fallback). Keep the wording concise, follow
the project's doc guideline to describe the current state (not history), and
ensure the new entry references "mutation default generators" and "generator
ids" so readers can map it to the earlier points.
In `@packages/3-targets/3-targets/postgres/package.json`:
- Around line 29-30: Remove the invalid "uniku" dependency from package.json
(the entry "uniku": "^0.0.12") so installs no longer fail; if a different
package was intended, replace "uniku" with the correct npm package name and
update any import/use sites accordingly (search for references to "uniku" in the
postgres target). Ensure package.json only contains valid dependencies needed by
the postgres target, then run npm/yarn install to verify.
In
`@packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts`:
- Line 52: The test "stores raw default expressions from database" should not
use the hardcoded timeout 1_000; import and use the shared test timeout helper
instead (e.g., the exported timeouts constant from the test helpers) and replace
the inline 1_000 in the it(...) timeout option with the appropriate shared value
(for example timeouts.short or timeouts.default), ensuring you add the import of
the timeouts helper at the top and update the it(...) invocation that contains
the literal 1_000.
---
Nitpick comments:
In `@docs/architecture` docs/adrs/ADR 169 - Declared applicability for mutation
default generators.md:
- Line 49: The phrase "For this slice" is ambiguous; update the sentence
containing "For this slice, applicability is keyed by `codecId` only." to
explicitly state the scope—e.g., "In this ADR, applicability is keyed by
`codecId` only." or "In this initial implementation, applicability is keyed by
`codecId` only."—so readers know whether the statement refers to the ADR, the
initial implementation, or the PR scope; pick the most accurate scope and apply
that wording consistently throughout the document.
In `@packages/1-framework/2-authoring/ids/src/index.ts`:
- Around line 115-118: Replace the inline conditional spread used when building
the args for resolveBuiltinGeneratedColumnDescriptor: instead of spreading
...(params ? { params } : {}), use the repository helper ifDefined from
`@prisma-next/utils/defined` and spread ...ifDefined({ params }) so the call
becomes resolveBuiltinGeneratedColumnDescriptor({ id, ...ifDefined({ params })
}); ensure you import ifDefined if it's not already present and adjust the
import list accordingly; keep references to resolvedDescriptor,
resolveBuiltinGeneratedColumnDescriptor, id, and params.
In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts`:
- Around line 80-82: Duplicate regex for generator IDs is used in
GeneratorIdSchema and the validators in
packages/2-sql/1-core/contract/src/validators.ts; extract it to a shared
exported constant (e.g., GENERATOR_ID_PATTERN) in the `@prisma-next/sql-contract`
package and import it from both places. Replace the inline regex in
GeneratorIdSchema and the other validator with a reference to
GENERATOR_ID_PATTERN, keeping the existing narrow(...) logic (use the
pattern.test(value) call) so behavior is unchanged. Update exports so the
constant is available to both packages and adjust import statements in files
that used the inline regex.
In `@packages/2-sql/3-tooling/family/src/core/assembly.ts`:
- Around line 261-290: The assembly currently calls
createControlMutationDefaultGeneratorDescriptorMap(descriptors) and then
iterates descriptors again calling descriptor.controlMutationDefaults() a second
time, which can yield different results for dynamic contributions; change
assembleControlMutationDefaultContributions to perform a single-pass: for each
descriptor call descriptor.controlMutationDefaults() once, add its generator
descriptors into a local generatorMap (mirroring the shape
createControlMutationDefaultGeneratorDescriptorMap produces) and simultaneously
merge its defaultFunctionRegistry entries into defaultFunctionRegistry while
checking duplicates via functionOwners; finally return defaultFunctionRegistry
and Array.from(generatorMap.values())—remove the separate
createControlMutationDefaultGeneratorDescriptorMap call and ensure you reference
descriptor.id when reporting conflicts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Lite
Run ID: a5303313-8592-47a0-b73b-32b7eeaf71b5
⛔ Files ignored due to path filters (17)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/psl-contract-authoring/plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/Follow-up - Pack-provided mutation default functions registry-plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/plan.mdis excluded by!projects/**projects/psl-contract-authoring/references/authoring-surface-gap-inventory.mdis excluded by!projects/**projects/psl-contract-authoring/spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Follow-up - Pack-provided mutation default functions registry.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 1 - Pluggable contract sources.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 2 - PSL parser.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 3 - Fixture-driven parity harness.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 4 - Parameterized attributes and pgvector parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 5 - ID variants and default function parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 6 - Close-out.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 7 - PSL sql template literal syntax.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/code-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/system-design-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/walkthrough.mdis excluded by!projects/**
📒 Files selected for processing (46)
.cursor/rules/use-timeouts-helper-in-tests.mdcdocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mdpackages/1-framework/1-core/shared/contract/src/types.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.jsonpackages/2-sql/2-authoring/contract-ts/src/contract.tspackages/2-sql/3-tooling/family/README.mdpackages/2-sql/3-tooling/family/src/core/assembly.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/3-tooling/family/src/exports/control.tspackages/2-sql/3-tooling/family/test/mutation-default-assembly.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.tstest/integration/test/authoring/parity/default-pack-slugid/contract.tstest/integration/test/authoring/parity/default-pack-slugid/expected.contract.jsontest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/default-pack-slugid/schema.prismatest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
1d1f33c to
f275c8a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/2-sql/5-runtime/README.md (1)
23-24: Use one term for generator concept across the README.Line 23 says “execution default generators,” but the rest of this README uses “mutation default generators.” Consider standardizing this bullet to the latter for consistency and easier scanning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/5-runtime/README.md` around lines 23 - 24, Update the README phrase "execution default generators" to use the same term as the rest of the document: "mutation default generators". Locate the bullet containing "execution default generators" (the first bullet under the feature list) and replace that phrase so the bullet reads "Mutation Default Generator Composition: Assemble mutation default generators from composed target/adapter/extension contributors" to keep terminology consistent across the README.packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts (1)
231-231: Useexpect.not.objectContaining()for absence checks.Please align this assertion with the repo matcher convention for “field must be absent”.
♻️ Suggested matcher update
- expect(columns['no_default']).not.toHaveProperty('default'); + expect(columns['no_default']).toEqual( + expect.not.objectContaining({ default: expect.anything() }), + );As per coding guidelines, "Use
expect.not.objectContaining()for absence checks".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts` at line 231, Replace the assertion that uses .not.toHaveProperty for the "no_default" column with the repo convention using expect.not.objectContaining: change the assertion on columns['no_default'] to use expect(...).toEqual(expect.not.objectContaining({ default: expect.anything() })) so the test asserts absence of the default field via expect.not.objectContaining rather than .not.toHaveProperty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/2-sql/5-runtime/README.md`:
- Around line 23-24: Update the README phrase "execution default generators" to
use the same term as the rest of the document: "mutation default generators".
Locate the bullet containing "execution default generators" (the first bullet
under the feature list) and replace that phrase so the bullet reads "Mutation
Default Generator Composition: Assemble mutation default generators from
composed target/adapter/extension contributors" to keep terminology consistent
across the README.
In
`@packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts`:
- Line 231: Replace the assertion that uses .not.toHaveProperty for the
"no_default" column with the repo convention using expect.not.objectContaining:
change the assertion on columns['no_default'] to use
expect(...).toEqual(expect.not.objectContaining({ default: expect.anything() }))
so the test asserts absence of the default field via expect.not.objectContaining
rather than .not.toHaveProperty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c4481c6c-7bb9-4f6a-9d79-b6ed47ca4f8d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-sql/4-lanes/sql-lane/test/utils/guards.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/2-sql/2-authoring/contract-psl/test/provider.test.ts (1)
87-112:⚠️ Potential issue | 🔴 CriticalMissing required
optionsparameter in calls toprismaContract.The function signature requires
options: PrismaContractOptionsas a non-optional parameter. Lines 87 and 173 callprismaContract('./schema.prisma')without passing any options. Both calls needbaseOptionspassed as the second argument, consistent with other tests in the file (e.g., line 131, 211, 250, 289, 348, 388, 413).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/provider.test.ts` around lines 87 - 112, The tests call prismaContract('./schema.prisma') without the required options argument; update those calls to pass the shared baseOptions (of type PrismaContractOptions) as the second parameter so they match other tests (e.g., change prismaContract('./schema.prisma') to prismaContract('./schema.prisma', baseOptions)); find usages by searching for prismaContract and ensure both occurrences in this test file receive baseOptions.
🧹 Nitpick comments (9)
packages/3-targets/6-adapters/postgres/test/adapter.test.ts (1)
1039-1042: Usetimeouts.defaultinstead oftimeouts.databaseOperation.This test performs pure in-memory AST lowering with no actual database operations. The
databaseOperationtimeout is documented for "database operations (queries, setup, teardown)" including table creation and data insertion, none of which occur here. Usetimeouts.defaultfor general transformation tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/test/adapter.test.ts` around lines 1039 - 1042, The test titled 'lowers SELECT with operation expression in include ORDER BY' is using timeouts.databaseOperation but performs only in-memory AST lowering (no DB work); change the timeout argument in the it(...) call from timeouts.databaseOperation to timeouts.default to reflect that this is a general transformation test rather than a database operation test.packages/2-sql/1-core/contract/test/validate.test.ts (2)
65-107: Split these new cases into a dedicated test file.This suite is already large; adding more scenarios here makes it harder to navigate and maintain. Move the execution-default generator-id cases into a focused file (for example,
validate.execution-defaults.test.ts).As per coding guidelines,
**/*.test.tsfiles should stay under 500 lines and be split by logical boundaries with descriptive names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/test/validate.test.ts` around lines 65 - 107, Split the two tests that cover execution default generator ids out of validate.test.ts into a new focused test file (e.g., validate.execution-defaults.test.ts): copy the two it(...) blocks referencing validateContract<SqlContract<SqlStorage>>(...) and the execution.mutations.defaults scenarios into the new file, keep or add any necessary shared fixtures (baseContract) imports or recreations, update imports to reference validateContract, SqlContract and SqlStorage, and remove the copied blocks from the original validate.test.ts so both files stay under the 500-line guideline and tests run unchanged.
104-106: Assert the specific generator-id failure reason.Line 104 currently accepts any structural validation error. Consider also asserting
/a flat generator id/so the test proves this exact rule.Suggested assertion tightening
- expect(() => validateContract<SqlContract<SqlStorage>>(contract)).toThrow( - /Contract structural validation failed/, - ); + expect(() => validateContract<SqlContract<SqlStorage>>(contract)).toThrow( + /a flat generator id/, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/test/validate.test.ts` around lines 104 - 106, The test currently asserts any structural validation error from validateContract<SqlContract<SqlStorage>>(contract); tighten it to assert the specific generator-id failure by matching the error message for "a flat generator id" as well (e.g., change the toThrow regex to include /a flat generator id/ or add a separate expect that the thrown error message matches /a flat generator id/), referencing the validateContract call and the SqlContract/SqlStorage types so the test ensures this exact rule is enforced.packages/2-sql/2-authoring/contract-ts/src/contract.ts (1)
80-87: Centralize generator-id schema to avoid validation drift.
GeneratorIdSchemahere duplicates the same regex rule inpackages/2-sql/1-core/contract/src/validators.ts(Line 25-27). Reuse a shared exported schema/helper so both validation paths stay aligned.Refactor direction
-import { ColumnDefaultSchema, ForeignKeySchema } from '@prisma-next/sql-contract/validators'; +import { + ColumnDefaultSchema, + ForeignKeySchema, + generatorIdSchema, +} from '@prisma-next/sql-contract/validators'; -const GeneratorIdSchema = type('string').narrow((value, ctx) => { - return /^[A-Za-z0-9][A-Za-z0-9_-]*$/.test(value) ? true : ctx.mustBe('a flat generator id'); -}); - const ExecutionMutationDefaultValueSchema = type({ kind: "'generator'", - id: GeneratorIdSchema, + id: generatorIdSchema, 'params?': 'Record<string, unknown>', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts` around lines 80 - 87, Replace the local duplicated GeneratorIdSchema with the shared exported generator-id schema from the core validators module: remove the local const GeneratorIdSchema and import the shared exported symbol (the generator id schema exported from the core validators file) and use that in the ExecutionMutationDefaultValueSchema id field; ensure the import name matches the exported identifier and update any references to GeneratorIdSchema in this file to use the imported symbol so both packages share the same validation logic.packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts (2)
133-160: Test for duplicate generator IDs is under the wrong describe block.This test calls
createControlMutationDefaultGeneratorDescriptorMapbut is nested underdescribe('assembleControlMutationDefaultContributions'). Consider moving it to a separate describe block or renaming the parent describe to reflect it covers both functions.Suggested structure
+describe('createControlMutationDefaultGeneratorDescriptorMap', () => { it('throws for duplicate generator ids', () => { // ...existing test... }); +});Or rename the parent describe block to cover all assembly functions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts` around lines 133 - 160, The test asserting duplicate generator ids is misplaced under the describe('assembleControlMutationDefaultContributions') block; move that it(...) into a new or existing describe that pertains to createControlMutationDefaultGeneratorDescriptorMap (or rename the parent describe to cover both assembly helpers) so tests are grouped by the function they exercise; specifically relocate the test referencing createControlMutationDefaultGeneratorDescriptorMap (and the duplicated generator fixtures) into a describe block named for createControlMutationDefaultGeneratorDescriptorMap (or update the parent describe title) to keep organization consistent.
162-208: Scalar type descriptor tests are also under the wrong describe block.Lines 162-208 test
assemblePslInterpretationContributionsbut are nested underdescribe('assembleControlMutationDefaultContributions').Suggested structure
+describe('assemblePslInterpretationContributions', () => { it('collects scalar type descriptor contributions', () => { // ...existing test... }); + it('throws for duplicate scalar type descriptors', () => { // ...existing test... }); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts` around lines 162 - 208, The two "it" cases that exercise assemblePslInterpretationContributions are currently nested inside the describe('assembleControlMutationDefaultContributions') block; move those tests so they live under the appropriate describe for assemblePslInterpretationContributions (or a new describe with that name) so the specs reference assemblePslInterpretationContributions directly; ensure the test names remain the same and keep the test bodies invoking createDescriptor and assemblePslInterpretationContributions unchanged.packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts (2)
221-228: Normalize usage signatures before formatting diagnostics.
" "currently passes the filter. Trimming first avoids noisy/blank signature entries in error messages.Small cleanup
- const usageSignatures = entry.usageSignatures?.filter((signature) => signature.length > 0); + const usageSignatures = entry.usageSignatures + ?.map((signature) => signature.trim()) + .filter((signature) => signature.length > 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts` around lines 221 - 228, The code building signatures from registry entries allows whitespace-only usage signatures through the filter; update the logic that computes usageSignatures in default-function-registry.ts (the Array.from(registry.entries()) .flatMap block) to trim each signature before filtering — e.g. map each entry.usageSignatures to signature.trim() and then filter by trimmed.length>0, and then fall back to [`${functionName}()`] when the resulting array is empty; ensure you reference the existing variables (registry, entry, usageSignatures, signatures) so the change is localized.
17-22: RequirecolumnCodecIdin lowering context for stronger applicability guarantees.Keeping this optional allows call sites to omit codec context and accidentally bypass intended applicability checks.
Proposed tightening
export interface DefaultFunctionLoweringContext { readonly sourceId: string; readonly modelName: string; readonly fieldName: string; - readonly columnCodecId?: string; + readonly columnCodecId: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts` around lines 17 - 22, The DefaultFunctionLoweringContext currently makes columnCodecId optional which allows callers to omit codec info; change the interface so columnCodecId is required (remove the ? on columnCodecId) in DefaultFunctionLoweringContext and then update all call sites, implementations, and tests that construct or receive a DefaultFunctionLoweringContext (search for uses of DefaultFunctionLoweringContext, functions like anyLowering/ lower* implementations, and places that pass sourceId/modelName/fieldName) to supply a concrete columnCodecId value; ensure any helper builders or factories that create this context are updated to accept/propagate columnCodecId and adjust types/signatures accordingly so the compiler enforces the presence of the codec id.packages/2-sql/5-runtime/src/sql-context.ts (1)
381-400: Add an explicit fallback incomputeExecutionDefaultValue.Making unsupported kinds fail explicitly avoids silent/ambiguous behavior when contract data is invalid or evolves.
Suggested hardening
function computeExecutionDefaultValue( spec: ExecutionMutationDefaultValue, generatorRegistry: ReadonlyMap<string, RuntimeMutationDefaultGenerator>, ): unknown { switch (spec.kind) { case 'generator': { const generator = generatorRegistry.get(spec.id); if (!generator) { throw runtimeError( 'RUNTIME.MUTATION_DEFAULT_GENERATOR_MISSING', `Contract references mutation default generator '${spec.id}' but no runtime component provides it.`, { id: spec.id, }, ); } return generator.generate(spec.params); } + default: { + throw runtimeError( + 'RUNTIME.MUTATION_DEFAULT_KIND_UNSUPPORTED', + 'Unsupported execution mutation default kind.', + { spec }, + ); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/5-runtime/src/sql-context.ts` around lines 381 - 400, computeExecutionDefaultValue currently handles only spec.kind === 'generator' and can return undefined for unknown kinds; add an explicit fallback branch for unsupported spec.kind values that throws a runtimeError (similar style to the existing error) so invalid or evolved contract data fail loudly. Locate the computeExecutionDefaultValue function and the switch over spec.kind and add a default/case for other kinds that throws a clear runtimeError referencing spec.kind and spec (or spec.id when available) so callers never get an ambiguous undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/2-sql/2-authoring/contract-psl/test/provider.test.ts`:
- Around line 87-112: The tests call prismaContract('./schema.prisma') without
the required options argument; update those calls to pass the shared baseOptions
(of type PrismaContractOptions) as the second parameter so they match other
tests (e.g., change prismaContract('./schema.prisma') to
prismaContract('./schema.prisma', baseOptions)); find usages by searching for
prismaContract and ensure both occurrences in this test file receive
baseOptions.
---
Nitpick comments:
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 65-107: Split the two tests that cover execution default generator
ids out of validate.test.ts into a new focused test file (e.g.,
validate.execution-defaults.test.ts): copy the two it(...) blocks referencing
validateContract<SqlContract<SqlStorage>>(...) and the
execution.mutations.defaults scenarios into the new file, keep or add any
necessary shared fixtures (baseContract) imports or recreations, update imports
to reference validateContract, SqlContract and SqlStorage, and remove the copied
blocks from the original validate.test.ts so both files stay under the 500-line
guideline and tests run unchanged.
- Around line 104-106: The test currently asserts any structural validation
error from validateContract<SqlContract<SqlStorage>>(contract); tighten it to
assert the specific generator-id failure by matching the error message for "a
flat generator id" as well (e.g., change the toThrow regex to include /a flat
generator id/ or add a separate expect that the thrown error message matches /a
flat generator id/), referencing the validateContract call and the
SqlContract/SqlStorage types so the test ensures this exact rule is enforced.
In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts`:
- Around line 221-228: The code building signatures from registry entries allows
whitespace-only usage signatures through the filter; update the logic that
computes usageSignatures in default-function-registry.ts (the
Array.from(registry.entries()) .flatMap block) to trim each signature before
filtering — e.g. map each entry.usageSignatures to signature.trim() and then
filter by trimmed.length>0, and then fall back to [`${functionName}()`] when the
resulting array is empty; ensure you reference the existing variables (registry,
entry, usageSignatures, signatures) so the change is localized.
- Around line 17-22: The DefaultFunctionLoweringContext currently makes
columnCodecId optional which allows callers to omit codec info; change the
interface so columnCodecId is required (remove the ? on columnCodecId) in
DefaultFunctionLoweringContext and then update all call sites, implementations,
and tests that construct or receive a DefaultFunctionLoweringContext (search for
uses of DefaultFunctionLoweringContext, functions like anyLowering/ lower*
implementations, and places that pass sourceId/modelName/fieldName) to supply a
concrete columnCodecId value; ensure any helper builders or factories that
create this context are updated to accept/propagate columnCodecId and adjust
types/signatures accordingly so the compiler enforces the presence of the codec
id.
In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts`:
- Around line 80-87: Replace the local duplicated GeneratorIdSchema with the
shared exported generator-id schema from the core validators module: remove the
local const GeneratorIdSchema and import the shared exported symbol (the
generator id schema exported from the core validators file) and use that in the
ExecutionMutationDefaultValueSchema id field; ensure the import name matches the
exported identifier and update any references to GeneratorIdSchema in this file
to use the imported symbol so both packages share the same validation logic.
In `@packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts`:
- Around line 133-160: The test asserting duplicate generator ids is misplaced
under the describe('assembleControlMutationDefaultContributions') block; move
that it(...) into a new or existing describe that pertains to
createControlMutationDefaultGeneratorDescriptorMap (or rename the parent
describe to cover both assembly helpers) so tests are grouped by the function
they exercise; specifically relocate the test referencing
createControlMutationDefaultGeneratorDescriptorMap (and the duplicated generator
fixtures) into a describe block named for
createControlMutationDefaultGeneratorDescriptorMap (or update the parent
describe title) to keep organization consistent.
- Around line 162-208: The two "it" cases that exercise
assemblePslInterpretationContributions are currently nested inside the
describe('assembleControlMutationDefaultContributions') block; move those tests
so they live under the appropriate describe for
assemblePslInterpretationContributions (or a new describe with that name) so the
specs reference assemblePslInterpretationContributions directly; ensure the test
names remain the same and keep the test bodies invoking createDescriptor and
assemblePslInterpretationContributions unchanged.
In `@packages/2-sql/5-runtime/src/sql-context.ts`:
- Around line 381-400: computeExecutionDefaultValue currently handles only
spec.kind === 'generator' and can return undefined for unknown kinds; add an
explicit fallback branch for unsupported spec.kind values that throws a
runtimeError (similar style to the existing error) so invalid or evolved
contract data fail loudly. Locate the computeExecutionDefaultValue function and
the switch over spec.kind and add a default/case for other kinds that throws a
clear runtimeError referencing spec.kind and spec (or spec.id when available) so
callers never get an ambiguous undefined.
In `@packages/3-targets/6-adapters/postgres/test/adapter.test.ts`:
- Around line 1039-1042: The test titled 'lowers SELECT with operation
expression in include ORDER BY' is using timeouts.databaseOperation but performs
only in-memory AST lowering (no DB work); change the timeout argument in the
it(...) call from timeouts.databaseOperation to timeouts.default to reflect that
this is a general transformation test rather than a database operation test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8ee8d5be-1920-4d95-8841-71bd7d094858
⛔ Files ignored due to path filters (17)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/psl-contract-authoring/plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/Follow-up - Pack-provided mutation default functions registry-plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/plan.mdis excluded by!projects/**projects/psl-contract-authoring/references/authoring-surface-gap-inventory.mdis excluded by!projects/**projects/psl-contract-authoring/spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Follow-up - Pack-provided mutation default functions registry.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 1 - Pluggable contract sources.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 2 - PSL parser.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 3 - Fixture-driven parity harness.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 4 - Parameterized attributes and pgvector parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 5 - ID variants and default function parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 6 - Close-out.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 7 - PSL sql template literal syntax.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/code-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/system-design-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/walkthrough.mdis excluded by!projects/**
📒 Files selected for processing (45)
.cursor/rules/use-timeouts-helper-in-tests.mdcdocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mdpackages/1-framework/1-core/shared/contract/src/types.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.jsonpackages/2-sql/2-authoring/contract-ts/src/contract.tspackages/2-sql/3-tooling/family/README.mdpackages/2-sql/3-tooling/family/src/core/assembly.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/3-tooling/family/src/exports/control.tspackages/2-sql/3-tooling/family/test/mutation-default-assembly.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.tstest/integration/test/authoring/parity/default-pack-slugid/contract.tstest/integration/test/authoring/parity/default-pack-slugid/expected.contract.jsontest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/default-pack-slugid/schema.prismatest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/3-targets/6-adapters/postgres/src/exports/control.ts
- .cursor/rules/use-timeouts-helper-in-tests.mdc
- packages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.json
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- test/integration/test/authoring/parity/default-pack-slugid/packs.ts
- packages/2-sql/5-runtime/test/utils.ts
- test/integration/test/authoring/parity/default-pack-slugid/contract.ts
- packages/2-sql/5-runtime/src/exports/index.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts
- packages/2-sql/2-authoring/contract-psl/src/exports/index.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.ts
- packages/1-framework/1-core/shared/contract/src/types.ts
- docs/architecture docs/ADR-INDEX.md
- test/integration/test/authoring/parity/default-pack-slugid/schema.prisma
- packages/2-sql/2-authoring/contract-psl/README.md
- test/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
f275c8a to
70d3b9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/2-sql/2-authoring/contract-psl/test/fixtures.ts (1)
24-35: Switch the optionalparamsspread toifDefined().This helper currently uses inline conditional spreading for
params, which the repo bans in package/test code. Please rewrite this withifDefined()so the fixtures stay aligned with the shared object-construction convention. As per coding guidelines, "UseifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts` around lines 24 - 35, Replace the inline conditional spread in executionGenerator with the shared helper: import ifDefined from '@prisma-next/utils/defined' and change the generated object spread from ...(params ? { params } : {}) to ...ifDefined({ params }, params) so that executionGenerator uses ifDefined for the optional params property.packages/2-sql/1-core/contract/test/validate.test.ts (1)
65-107: Move these cases into a dedicated execution-defaults test file.This file is already well past the repository's 500-line limit, and adding more scenarios here makes it harder to navigate and maintain. Please move the new generator-id coverage into a focused
validate.execution-defaults*.test.tssplit instead of extending this catch-all file further. As per coding guidelines, "Keep test files under 500 lines ... split test files when they exceed 500 lines, contain multiple distinct concerns that can be logically separated, or have multiple top-leveldescribeblocks that can be split by functionality".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/test/validate.test.ts` around lines 65 - 107, Move the two test cases that exercise execution default generator ids out of the large validate.test.ts into a new focused test file (e.g. validate.execution-defaults.test.ts); copy the two it(...) blocks that reference validateContract, SqlContract, and SqlStorage into that new file, keep the same imports/setup used in the original (baseContract, validateContract, types), and remove them from the original file so validate.test.ts stays under the 500-line guideline and concerns remain separated.packages/2-sql/2-authoring/contract-ts/src/contract.ts (1)
78-85: Reuse the shared generator-id validator here.This regex now exists in both
packages/2-sql/2-authoring/contract-ts/src/contract.tsandpackages/2-sql/1-core/contract/src/validators.ts, so authoring-side and core validation can drift independently on the next rule change. Please export the TypeScript validator from the shared contract package and consume it here instead of keeping a second copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts` around lines 78 - 85, Replace the duplicated regex validator by importing the shared generator-id validator from the common contract validators and use it in place of the local GeneratorIdSchema; specifically, remove the local GeneratorIdSchema declaration, add an import for the shared generator-id validator (the exported symbol from the shared validators package) at the top of this file, and update ExecutionMutationDefaultValueSchema to reference that imported validator instead of the inline regex; ensure the import name matches the exported symbol from the shared package and run types/tsc to confirm no type breaks.packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts (1)
1-863: Consider splitting this test file.At 863 lines, this file exceeds the 500-line guideline. The tests naturally group into distinct concerns that could be split:
- Basic PSL-to-IR mapping (models, types, mappings)
- Relation handling
- Default function lowering
- Extension/namespace handling
- Diagnostics/error cases
This would improve navigability and align with the test file size guideline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts` around lines 1 - 863, The test file is too long (863 lines) and should be split into smaller, focused test files grouped by concern; create multiple files (e.g., basic-mapping.test.ts, relations.test.ts, defaults.test.ts, extensions.test.ts, diagnostics.test.ts) and move the corresponding it blocks into them by locating tests under the top-level describe('interpretPslDocumentToSqlContractIR') and using helpers like interpretPslDocumentToSqlContractIR (the wrapper), interpretPslDocumentToSqlContractIRInternal, createBuiltinLikeControlMutationDefaults, postgresScalarTypeDescriptors, and postgresTarget to maintain shared setup; extract any shared fixtures/imports into a small test-utils or fixtures module and update imports in each new test file so assertions and helper usage remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-sql/3-tooling/family/src/core/assembly.ts`:
- Around line 234-320: Materialize each descriptor's controlMutationDefaults()
once and reuse it: in assembleControlMutationDefaultContributions, first loop
descriptors and call descriptor.controlMutationDefaults?.(), storing results
(e.g., an array of {descriptor, contributions} or a Map from descriptor.id to
contributions) and skip nulls; then use that cached collection both to build the
generator descriptors (rather than calling
createControlMutationDefaultGeneratorDescriptorMap(descriptors) which triggers a
second evaluation) and to register defaultFunctionRegistry entries. Either
change createControlMutationDefaultGeneratorDescriptorMap to accept the cached
contributions collection or call its logic inline using the cached
contributions; reference controlMutationDefaults,
assembleControlMutationDefaultContributions, and
createControlMutationDefaultGeneratorDescriptorMap. Also add a regression test
that asserts controlMutationDefaults() is invoked exactly once per descriptor.
---
Nitpick comments:
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 65-107: Move the two test cases that exercise execution default
generator ids out of the large validate.test.ts into a new focused test file
(e.g. validate.execution-defaults.test.ts); copy the two it(...) blocks that
reference validateContract, SqlContract, and SqlStorage into that new file, keep
the same imports/setup used in the original (baseContract, validateContract,
types), and remove them from the original file so validate.test.ts stays under
the 500-line guideline and concerns remain separated.
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 24-35: Replace the inline conditional spread in executionGenerator
with the shared helper: import ifDefined from '@prisma-next/utils/defined' and
change the generated object spread from ...(params ? { params } : {}) to
...ifDefined({ params }, params) so that executionGenerator uses ifDefined for
the optional params property.
In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts`:
- Around line 1-863: The test file is too long (863 lines) and should be split
into smaller, focused test files grouped by concern; create multiple files
(e.g., basic-mapping.test.ts, relations.test.ts, defaults.test.ts,
extensions.test.ts, diagnostics.test.ts) and move the corresponding it blocks
into them by locating tests under the top-level
describe('interpretPslDocumentToSqlContractIR') and using helpers like
interpretPslDocumentToSqlContractIR (the wrapper),
interpretPslDocumentToSqlContractIRInternal,
createBuiltinLikeControlMutationDefaults, postgresScalarTypeDescriptors, and
postgresTarget to maintain shared setup; extract any shared fixtures/imports
into a small test-utils or fixtures module and update imports in each new test
file so assertions and helper usage remain unchanged.
In `@packages/2-sql/2-authoring/contract-ts/src/contract.ts`:
- Around line 78-85: Replace the duplicated regex validator by importing the
shared generator-id validator from the common contract validators and use it in
place of the local GeneratorIdSchema; specifically, remove the local
GeneratorIdSchema declaration, add an import for the shared generator-id
validator (the exported symbol from the shared validators package) at the top of
this file, and update ExecutionMutationDefaultValueSchema to reference that
imported validator instead of the inline regex; ensure the import name matches
the exported symbol from the shared package and run types/tsc to confirm no type
breaks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 68bde77d-7f42-4281-b1c7-583dda879f33
⛔ Files ignored due to path filters (17)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/psl-contract-authoring/plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/Follow-up - Pack-provided mutation default functions registry-plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/plan.mdis excluded by!projects/**projects/psl-contract-authoring/references/authoring-surface-gap-inventory.mdis excluded by!projects/**projects/psl-contract-authoring/spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Follow-up - Pack-provided mutation default functions registry.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 1 - Pluggable contract sources.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 2 - PSL parser.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 3 - Fixture-driven parity harness.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 4 - Parameterized attributes and pgvector parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 5 - ID variants and default function parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 6 - Close-out.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 7 - PSL sql template literal syntax.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/code-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/system-design-review.mdis excluded by!projects/**projects/psl-contract-authoring/specs/reviews/walkthrough.mdis excluded by!projects/**
📒 Files selected for processing (47)
.cursor/rules/use-timeouts-helper-in-tests.mdcdocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mdpackages/1-framework/1-core/shared/contract/src/types.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.jsonpackages/2-sql/2-authoring/contract-ts/src/contract.tspackages/2-sql/3-tooling/family/README.mdpackages/2-sql/3-tooling/family/src/core/assembly.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/3-tooling/family/src/exports/control.tspackages/2-sql/3-tooling/family/test/mutation-default-assembly.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.tstest/integration/test/authoring/parity/default-pack-slugid/contract.tstest/integration/test/authoring/parity/default-pack-slugid/expected.contract.jsontest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/default-pack-slugid/schema.prismatest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- docs/architecture docs/ADR-INDEX.md
- .cursor/rules/use-timeouts-helper-in-tests.mdc
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- packages/1-framework/1-core/shared/contract/src/types.ts
- test/integration/test/authoring/parity/default-pack-slugid/contract.ts
- packages/2-sql/3-tooling/family/README.md
- packages/2-sql/3-tooling/family/src/core/migrations/types.ts
- packages/2-sql/5-runtime/test/utils.ts
- packages/3-targets/6-adapters/postgres/test/adapter.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/control.ts
- packages/2-sql/5-runtime/src/exports/index.ts
- test/integration/test/authoring/parity/default-pack-slugid/expected.contract.json
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- test/integration/test/authoring/parity/default-pack-slugid/schema.prisma
- packages/2-sql/4-lanes/sql-lane/test/utils/guards.test.ts
- packages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/1-framework/2-authoring/ids/src/index.ts (1)
7-7:⚠️ Potential issue | 🟡 MinorMove
builtinGeneratorIdsto the package's real export surface.Importing the symbol first and then re-exporting it from
src/index.tsis still a cross-file re-export. Please expose it from the designatedexports/surface instead of forwarding it here.As per coding guidelines: "Do not reexport things from one file in another, except in the exports/ folders."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/2-authoring/ids/src/index.ts` at line 7, The file is re-exporting builtinGeneratorIds from src/index.ts which violates the rule against cross-file re-exports; instead export builtinGeneratorIds from the package's official export surface under exports/ (create or add to the appropriate exports file in exports/ and export the symbol there, e.g. export { builtinGeneratorIds } from './path/to/original/module'), remove the re-export/import forwarding from src/index.ts, and update any internal import sites if needed to import from the package's public export path rather than src/index.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/2-authoring/ids/src/index.ts`:
- Around line 22-34: The resolver resolveNanoidColumnDescriptor must not
silently coerce invalid sizes to 21; validate params?.['size'] and if it exists
but is not an integer between 2 and 255, throw a clear error (e.g., "nanoid size
must be an integer between 2 and 255") so generated.params cannot diverge from
the descriptor. If params.size is undefined, keep the default length 21;
otherwise use the validated value for typeParams.length and ensure
generated.params matches that validated size. Target
resolveNanoidColumnDescriptor, the generated.params handling, and
typeParams.length when implementing this change.
---
Duplicate comments:
In `@packages/1-framework/2-authoring/ids/src/index.ts`:
- Line 7: The file is re-exporting builtinGeneratorIds from src/index.ts which
violates the rule against cross-file re-exports; instead export
builtinGeneratorIds from the package's official export surface under exports/
(create or add to the appropriate exports file in exports/ and export the symbol
there, e.g. export { builtinGeneratorIds } from './path/to/original/module'),
remove the re-export/import forwarding from src/index.ts, and update any
internal import sites if needed to import from the package's public export path
rather than src/index.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 781d0de8-adb4-44cc-921b-a0e5acae8061
📒 Files selected for processing (5)
packages/1-framework/2-authoring/ids/src/generator-ids.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/test/ids.test.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
8d3e462 to
ad6722f
Compare
SevInf
reviewed
Mar 10, 2026
Contributor
Code Review — PR #214: feat(sql): compose mutation defaults via registriesBranch: Findings Summary
🔴 P1 — Critical1.
🟡 P2 — Important2. Assembly-layer errors are unstructured
3. Generator
4. Test fixture duplicates ~230 lines of production code
5. Redundant runtime guards on required interface fields
6. Removed user-friendly suggestions for mistyped generator names
🔵 P3 — Nice-to-Have7. Type duplication between family and PSL layers (~65 lines) — 8. CI workflow uses direct 9. No introspection API for composed registry state — Neither 10. 11. 12. Git history cleanup recommended — 33 commits is high. 3 review-artifact commits net to zero, fix chains should be squashed, ADR 170 and test timeout housekeeping could be separate PRs. Suggest squashing to ~10-15 focused commits before merge. PerformanceNo regressions. All registry lookups are O(1) SecurityNo critical vulnerabilities. ArchitectureFully compliant with domain/layer/plane rules. Zero import violations. Clean separation of control-time and runtime seams. Correctly follows existing composition patterns. The Agent-Native6/7 capabilities agent-accessible. All composition is programmatic. Contract artifacts are machine-readable. Main gap: no registry introspection API. Recommended Next Steps
Review performed by 8 parallel agents: architecture-strategist, security-sentinel, performance-oracle, pattern-recognition-specialist, code-simplicity-reviewer, kieran-typescript-reviewer, agent-native-reviewer, git-history-analyzer |
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
ad6722f to
c5dbc99
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
packages/3-targets/6-adapters/postgres/src/exports/runtime.ts (1)
48-56: Consider extracting shared generator factory.This function contains no Postgres-specific logic—it maps all
builtinGeneratorIdsidentically. If other adapters (e.g., MySQL) need the same generators, consider extracting this to a shared utility (e.g., in@prisma-next/ids/runtime) to avoid duplication across adapter packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/src/exports/runtime.ts` around lines 48 - 56, The createPostgresMutationDefaultGenerators function duplicates a generic mapping of builtinGeneratorIds to generator objects (using generateId) with no Postgres-specific logic; extract this shared factory into a reusable utility (e.g., a function named createBuiltinMutationDefaultGenerators) in a common runtime module (such as an ids/runtime shared package) and replace createPostgresMutationDefaultGenerators to call that shared function, keeping references to builtinGeneratorIds and generateId so other adapters (MySQL, etc.) can import and reuse the same generator factory.packages/2-sql/5-runtime/src/sql-context.ts (1)
386-398: Consider adding runtime schema validation for generator params.The
spec.paramsare passed directly togenerator.generate()without runtime validation. While authoring-time validation exists in PSL lowering, a malformed contract (or future changes) could pass invalid params to generators at runtime.If generators define a
paramsSchema, validatingspec.paramshere would provide defense-in-depth and clearer error messages when params are invalid.♻️ Sketch of optional validation
case 'generator': { const generator = generatorRegistry.get(spec.id); if (!generator) { throw runtimeError( 'RUNTIME.MUTATION_DEFAULT_GENERATOR_MISSING', `Contract references mutation default generator '${spec.id}' but no runtime component provides it.`, { id: spec.id }, ); } + // If generator declares a paramsSchema, validate params + if (generator.paramsSchema && spec.params) { + const result = generator.paramsSchema(spec.params); + if (result instanceof arktype.errors) { + throw runtimeError( + 'RUNTIME.MUTATION_DEFAULT_PARAMS_INVALID', + `Invalid params for mutation default generator '${spec.id}'.`, + { id: spec.id, params: spec.params }, + ); + } + } // nosemgrep: javascript.express.security.express-wkhtml-injection.express-wkhtmltoimage-injection return generator.generate(spec.params); }This would require extending
RuntimeMutationDefaultGeneratorto optionally includeparamsSchema. Consider deferring if authoring-time validation is deemed sufficient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/5-runtime/src/sql-context.ts` around lines 386 - 398, The code calls generator.generate(spec.params) from the generatorRegistry without validating spec.params; add runtime validation by extending RuntimeMutationDefaultGenerator to optionally expose a paramsSchema and, before calling generator.generate in the 'generator' case, validate spec.params against that schema and throw a clear runtimeError (e.g., RUNTIME.MUTATION_DEFAULT_GENERATOR_INVALID_PARAMS) including the generator id and validation errors if validation fails; keep the existing path to call generator.generate(spec.params) only when params pass validation.packages/2-sql/1-core/contract/test/validate.test.ts (1)
65-107: Move these execution-default cases into a split test file.
validate.test.tsis already well past the 500-line cap, and adding another concern here makes it harder to navigate. Please move these new cases into a focused file such asvalidate.execution-defaults.test.ts.As per coding guidelines "Keep test files under 500 lines to maintain readability and navigability" and "Use descriptive file names with the pattern
{base}.{category}.test.ts."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/test/validate.test.ts` around lines 65 - 107, Extract the two "it" cases that reference execution defaults from validate.test.ts into a new focused test file named validate.execution-defaults.test.ts: copy the tests that construct contract using baseContract and call validateContract<SqlContract<SqlStorage>> (the cases titled "accepts custom execution default generator ids" and "rejects non-flat execution default generator ids") into the new file, import the same helpers (baseContract, validateContract, SqlContract, SqlStorage) at the top, remove the original tests from validate.test.ts, keep the tests' code and assertions unchanged, and run the test suite to ensure imports and types are correct.packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts (1)
35-49: UseifDefined()for these conditional spreads.The inline
...(cond ? { ... } : {})pattern appears in both object builders here. This repo standardizes onifDefined()for conditional spreads, so it would be better to align these constructors with the rest of the package.As per coding guidelines,
{packages,examples,test}/**/*.{ts,tsx}: UseifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns.Also applies to: 316-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts` around lines 35 - 49, Replace the inline conditional spread in executionGenerator with the repo-standard ifDefined() helper: change the spread ...(params ? { params } : {}) to ...ifDefined(params, p => ({ params: p })) after importing ifDefined from '@prisma-next/utils/defined'; also apply the same replacement to the similar conditional spread used elsewhere in this file (the other object-builder around the 316-325 region) so both constructors use ifDefined() for conditional spreads.packages/2-sql/2-authoring/contract-psl/test/fixtures.ts (1)
24-33: UseifDefined()for the optionalparamsfield.Inline conditional spreads diverge from the helper this repo uses for optional object properties.
♻️ Proposed cleanup
+import { ifDefined } from '@prisma-next/utils/defined'; + function executionGenerator(id: string, params?: Record<string, unknown>) { return { ok: true as const, value: { kind: 'execution' as const, generated: { kind: 'generator' as const, id, - ...(params ? { params } : {}), + ...ifDefined('params', params), }, }, }; }As per coding guidelines, "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts` around lines 24 - 33, Replace the inline conditional spread for optional params in executionGenerator with the repository helper: instead of using ...(params ? { params } : {}), call the ifDefined helper from '@prisma-next/utils/defined' to conditionally add the params property; update the generation object in executionGenerator to use ifDefined('params', params) so the optional property follows the project's standard pattern.packages/2-sql/2-authoring/contract-psl/src/interpreter.ts (1)
1115-1116: Consider clearer empty registry typing.
new Map<string, never>()is functionally correct but semantically awkward. A more explicit empty map type could improve readability.Suggested alternative
const defaultFunctionRegistry = - input.controlMutationDefaults?.defaultFunctionRegistry ?? new Map<string, never>(); + input.controlMutationDefaults?.defaultFunctionRegistry ?? + (new Map() as ControlMutationDefaultRegistry);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts` around lines 1115 - 1116, The defaultFunctionRegistry fallback uses new Map<string, never>() which is semantically confusing; replace it with a clearer function-registry type such as new Map<string, (...args: unknown[]) => unknown>() (or new Map<string, Function>() / Map<string, unknown> depending on expected usage) so the intent is explicit. Update the assignment for defaultFunctionRegistry (the expression using input.controlMutationDefaults?.defaultFunctionRegistry) to use the clearer Map type and adjust any surrounding type annotations if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-code-security.yml:
- Line 36: The semgrep invocation places the positional target "." after flags
and leaves $BASELINE_ARG unquoted; update the semgrep command (the line invoking
"semgrep scan --severity ERROR --error --sarif --config auto --exclude-rule ...
--output=semgrep-results.sarif $BASELINE_ARG") so the positional argument "."
comes immediately after "semgrep scan" and before any flags, and quote
"$BASELINE_ARG" to prevent word-splitting/empty-value issues (e.g., place "."
first and change $BASELINE_ARG to "$BASELINE_ARG").
- Around line 31-36: Avoid interpolating ${{ github.event.pull_request.base.sha
}} directly in the run shell; instead expose it via an env var and reference
that safe variable in the script. Set an environment variable (e.g.,
BASELINE_COMMIT) from ${{ github.event.pull_request.base.sha }} and in the run
block test it with [ -n "$BASELINE_COMMIT" ] to populate BASELINE_ARG, then pass
$BASELINE_ARG to the semgrep invocation (the variables to edit are BASELINE_ARG
and the semgrep command lines shown in the diff).
In `@examples/prisma-next-demo/prisma-next.config.ts`:
- Around line 22-31: The prismaContract call includes a redundant
composedExtensionPacks option which duplicates the top-level extensionPacks and
causes providers to merge without deduping (e.g., appending pgvector twice);
remove the composedExtensionPacks: extensionPacks.map((pack) => pack.id) entry
from the prismaContract(...) options so the contract relies on the
loader-provided composedExtensionPacks in the sourceContext and avoid duplicate
extension pack entries.
In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts`:
- Around line 59-65: The ControlMutationDefaultEntry.lower method currently
returns unknown which forces an unsafe cast to LoweredDefaultResult downstream;
change the signature of ControlMutationDefaultEntry.lower to return
LoweredDefaultResult (the discriminated union) instead of unknown, update all
implementations of lower to construct and return the correct discriminated
objects, and remove the unsafe cast at the callsite that treats the result as
LoweredDefaultResult (the code using the cast should accept the typed return
directly). Ensure usages of ParsedDefaultFunctionCall and
DefaultFunctionLoweringContext within lower still match the contract and that
any callers (e.g., the place performing the former "as LoweredDefaultResult"
cast) rely on the new typed return.
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 91-321: The fixture duplicates the full builtin lowering registry
and generator descriptors in createBuiltinLikeControlMutationDefaults(); replace
the inlined defaultFunctionRegistry and generatorDescriptors with the real
production contribution (or, if tests only need limited behavior, replace them
with minimal stub maps that expose only the functions/descriptors used by these
tests). Locate createBuiltinLikeControlMutationDefaults and swap the hardcoded
Map and array for a reference to the assembled production contribution (or
construct tiny stub entries for functions like 'uuid','cuid','nanoid', and
generators 'uuidv4','nanoid', etc.) so tests no longer mirror the production
implementation.
In `@packages/2-sql/3-tooling/family/src/core/assembly.ts`:
- Around line 248-268: Replace the bare Error throws in the duplicate checks
with the repo’s structured error envelope (do not throw raw Error): when
detecting duplicates for generatorDescriptor.id and for functionName (in the
blocks using generatorOwners, generatorMap, defaultFunctionRegistry,
functionOwners and descriptor.id) construct and throw a structured/typed error
(e.g., DuplicateContributionError or createErrorEnvelope) that includes a stable
code (like "DUPLICATE_GENERATOR" / "DUPLICATE_FUNCTION"), the conflicting keys
(generatorDescriptor.id or functionName) and both descriptor ids (descriptor.id
and owner) in the error details; make the same change for the scalar-descriptor
duplicate branch noted at lines 290-298 so callers can programmatically inspect
code and details instead of parsing a message.
In `@packages/2-sql/3-tooling/family/src/core/migrations/types.ts`:
- Around line 58-61: The handler return type currently uses unknown; define a
local discriminated union (e.g. ControlMutationDefaultFunctionResult) that
models the valid lowering outcomes and use that instead of unknown for
ControlMutationDefaultFunctionHandler; update the type declaration to return
ControlMutationDefaultFunctionResult and ensure the union's discriminant and
payload fields match the lowering result shape expected by the interpreter and
consumers (use the existing symbols ControlMutationDefaultFunctionCall and
ControlMutationDefaultLoweringContext to locate the handler signature and
replace the unknown return type).
In
`@packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts`:
- Around line 87-132: The lowerNow and lowerAutoincrement functions currently
ignore input.context and therefore allow invalid combinations like String
`@default`(now()) or String `@default`(autoincrement()); update both lowerers
(lowerNow and lowerAutoincrement) to validate the target field's
codec/native-type via input.context (e.g., check context.field or
context.applicableCodecIds) and return a failure LoweredDefaultResult with an
applicability diagnostic when the codec/type is incompatible, or alternatively
remove these from direct lowering and instead register them via
createPostgresMutationDefaultGeneratorDescriptors with appropriate
applicableCodecIds so the registry enforces compatibility.
- Around line 2-14: This file in src/core imports control-plane types and
lowering handlers (ControlMutationDefaultFunctionEntry,
ControlMutationDefaultGeneratorDescriptor, PslScalarTypeDescriptor,
builtinGeneratorRegistryMetadata, resolveBuiltinGeneratedColumnDescriptor,
DefaultFunctionLoweringContext, DefaultFunctionLoweringHandler) which makes core
depend on control APIs—move all lowering/descriptor factory logic that
references those symbols out of src/core into a control-plane module (e.g.,
create src/exports/control.ts or a new src/control/* file), reimplement the
factories and DefaultFunctionLoweringHandler implementations there, update any
package exports to expose them from the new control module, and leave only
genuinely shared types/helpers in src/core that do not import
`@prisma-next/family-sql/control` or `@prisma-next/sql-contract-psl`; also update
any import sites to import the factories/handlers from the new control module
rather than src/core.
- Around line 79-85: parseStringLiteral currently trims outer quotes but doesn't
decode Postgres-style escapes, so dbgenerated() defaults that contain escaped
quotes/newlines are emitted with backslashes; replace the ad-hoc reparse with
the parser-provided decoded string or implement proper PSL unescaping: update
parseStringLiteral to accept and return the parser's parsed string value (use
the token/value produced by the SQL parser instead of raw source) or if only raw
text is available, unescape per PostgreSQL string-literal rules (handling E''
escapes, backslash escapes, doubled quotes, and standard_conforming_strings) and
return the fully decoded string; apply the same fix where parseStringLiteral is
used (including the other occurrence referenced around parseStringLiteral lines
252-273).
---
Nitpick comments:
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 65-107: Extract the two "it" cases that reference execution
defaults from validate.test.ts into a new focused test file named
validate.execution-defaults.test.ts: copy the tests that construct contract
using baseContract and call validateContract<SqlContract<SqlStorage>> (the cases
titled "accepts custom execution default generator ids" and "rejects non-flat
execution default generator ids") into the new file, import the same helpers
(baseContract, validateContract, SqlContract, SqlStorage) at the top, remove the
original tests from validate.test.ts, keep the tests' code and assertions
unchanged, and run the test suite to ensure imports and types are correct.
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 1115-1116: The defaultFunctionRegistry fallback uses new
Map<string, never>() which is semantically confusing; replace it with a clearer
function-registry type such as new Map<string, (...args: unknown[]) =>
unknown>() (or new Map<string, Function>() / Map<string, unknown> depending on
expected usage) so the intent is explicit. Update the assignment for
defaultFunctionRegistry (the expression using
input.controlMutationDefaults?.defaultFunctionRegistry) to use the clearer Map
type and adjust any surrounding type annotations if necessary.
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 24-33: Replace the inline conditional spread for optional params
in executionGenerator with the repository helper: instead of using ...(params ?
{ params } : {}), call the ifDefined helper from '@prisma-next/utils/defined' to
conditionally add the params property; update the generation object in
executionGenerator to use ifDefined('params', params) so the optional property
follows the project's standard pattern.
In `@packages/2-sql/5-runtime/src/sql-context.ts`:
- Around line 386-398: The code calls generator.generate(spec.params) from the
generatorRegistry without validating spec.params; add runtime validation by
extending RuntimeMutationDefaultGenerator to optionally expose a paramsSchema
and, before calling generator.generate in the 'generator' case, validate
spec.params against that schema and throw a clear runtimeError (e.g.,
RUNTIME.MUTATION_DEFAULT_GENERATOR_INVALID_PARAMS) including the generator id
and validation errors if validation fails; keep the existing path to call
generator.generate(spec.params) only when params pass validation.
In
`@packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts`:
- Around line 35-49: Replace the inline conditional spread in executionGenerator
with the repo-standard ifDefined() helper: change the spread ...(params ? {
params } : {}) to ...ifDefined(params, p => ({ params: p })) after importing
ifDefined from '@prisma-next/utils/defined'; also apply the same replacement to
the similar conditional spread used elsewhere in this file (the other
object-builder around the 316-325 region) so both constructors use ifDefined()
for conditional spreads.
In `@packages/3-targets/6-adapters/postgres/src/exports/runtime.ts`:
- Around line 48-56: The createPostgresMutationDefaultGenerators function
duplicates a generic mapping of builtinGeneratorIds to generator objects (using
generateId) with no Postgres-specific logic; extract this shared factory into a
reusable utility (e.g., a function named createBuiltinMutationDefaultGenerators)
in a common runtime module (such as an ids/runtime shared package) and replace
createPostgresMutationDefaultGenerators to call that shared function, keeping
references to builtinGeneratorIds and generateId so other adapters (MySQL, etc.)
can import and reuse the same generator factory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6adc8522-9c55-4891-a92c-00d089b52d73
📒 Files selected for processing (46)
.cursor/rules/use-timeouts-helper-in-tests.mdc.github/workflows/pr-code-security.ymldocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mdexamples/prisma-next-demo/prisma-next.config.tspackages/1-framework/1-core/shared/contract/src/types.tspackages/1-framework/2-authoring/ids/src/generator-ids.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/1-framework/2-authoring/ids/test/ids.test.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.jsonpackages/2-sql/2-authoring/contract-ts/src/contract.tspackages/2-sql/3-tooling/family/README.mdpackages/2-sql/3-tooling/family/src/core/assembly.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/3-tooling/family/src/exports/control.tspackages/2-sql/3-tooling/family/test/mutation-default-assembly.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/control.ts
- packages/1-framework/1-core/shared/contract/src/types.ts
- packages/2-sql/2-authoring/contract-ts/src/contract.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts
- packages/3-targets/6-adapters/postgres/test/adapter.test.ts
- packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts
- packages/1-framework/2-authoring/ids/test/ids.test.ts
- packages/1-framework/2-authoring/ids/src/generators.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/2-sql/1-core/contract/src/validators.ts
- packages/2-sql/5-runtime/test/utils.ts
- packages/2-sql/5-runtime/test/mutation-default-generators.test.ts
- packages/1-framework/2-authoring/ids/src/runtime.ts
- packages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.ts
- packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts
- packages/2-sql/4-lanes/sql-lane/test/utils/guards.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.ts
- packages/1-framework/2-authoring/ids/src/generator-ids.ts
- packages/2-sql/3-tooling/family/src/exports/control.ts
SevInf
reviewed
Mar 12, 2026
SevInf
reviewed
Mar 12, 2026
SevInf
reviewed
Mar 12, 2026
Contributor
|
More fundamental design question to this: suppose I am working on extension that adds I see 2 possible answers:
|
Contributor
Author
|
@SevInf my idea was that either package can provide the linkage from generator to column type:
|
wmadden
added a commit
that referenced
this pull request
Mar 20, 2026
test(sql-runtime): add test for user-provided values overriding generated defaults Verifies that applyMutationDefaults skips the generator when the user explicitly provides a value for a column that has a generated default. PR #214 review thread (SevInf) EOF )
wmadden
added a commit
that referenced
this pull request
Mar 20, 2026
test(sql-runtime): clarify test title for duplicate generator error case Rename from "includes both owners when duplicate generator ids are composed" to "throws error naming both owners when duplicate generator ids are composed" so the title reflects this is an error-path test. PR #214 review thread (SevInf) EOF )
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
docs/architecture docs/subsystems/7. Migration System.md (2)
343-343: Rephrase capability wording as requirements, not declarations.“capability declarations” reads like contract-defined capability ownership. Prefer “capability requirements” to align with adapter-reported/negotiated semantics.
As per coding guidelines: “Frame contract capabilities as requirements, not definitions” and “Avoid wording that implies the contract owns or defines capabilities.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/subsystems/7. Migration System.md at line 343, Update the wording to frame capabilities as requirements rather than declarations: change the sentence to say that when a contract change only affects capability requirements (no schema changes), the runner verifies the environment satisfies the new requirements and updates the `profile_hash` in the marker without applying any DDL, while the `core_hash` stays the same; ensure you replace "capability declarations" with "capability requirements" wherever this sentence appears so it aligns with adapter-reported/negotiated semantics and the project's phrasing guidelines.
363-370: Unify error-code format with the documented convention.Line 363 defines a stable
category/codeformat, but Lines 369-370 useMIGRATION.*. Please either normalize those codes to the documented shape or adjust the convention text to match actual emitted formats.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/subsystems/7. Migration System.md around lines 363 - 370, The documented convention states errors must use a stable category/code shape but the table mixes formats; update the two migration-style codes to follow category/code (e.g., replace "MIGRATION.AMBIGUOUS_LEAF" and "MIGRATION.NO_RESOLVABLE_LEAF" with "migration/ambiguous-leaf" and "migration/no-resolvable-leaf") or alternatively change the convention line to include the dot-separated form—make the change consistently in the table and any references to the symbols "MIGRATION.AMBIGUOUS_LEAF" and "MIGRATION.NO_RESOLVABLE_LEAF" so they match the chosen convention.packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts (1)
37-42: Type duplication betweenDefaultFunctionRegistryEntryandControlMutationDefaultEntry.These two interfaces have identical shapes — both define
lower: DefaultFunctionLoweringHandler(or equivalent signature) andusageSignatures?: readonly string[]. This was noted as a P3 finding in the PR review comments. Consider either:
- Using
DefaultFunctionRegistryEntrydirectly forControlMutationDefaultRegistry, or- Having
ControlMutationDefaultEntryextendDefaultFunctionRegistryEntry♻️ Suggested consolidation
-export interface ControlMutationDefaultEntry { - readonly lower: (input: { - readonly call: ParsedDefaultFunctionCall; - readonly context: DefaultFunctionLoweringContext; - }) => LoweredDefaultResult; - readonly usageSignatures?: readonly string[]; -} - -export type ControlMutationDefaultRegistry = ReadonlyMap<string, ControlMutationDefaultEntry>; +export type ControlMutationDefaultRegistry = DefaultFunctionRegistry;Also applies to: 59-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts` around lines 37 - 42, The interfaces DefaultFunctionRegistryEntry and ControlMutationDefaultEntry are identical; remove the duplication by making ControlMutationDefaultEntry reuse DefaultFunctionRegistryEntry (either replace ControlMutationDefaultEntry with DefaultFunctionRegistryEntry in ControlMutationDefaultRegistry or declare ControlMutationDefaultEntry extends DefaultFunctionRegistryEntry), then update the ControlMutationDefaultRegistry type to ReadonlyMap<string, DefaultFunctionRegistryEntry> (or the extended name) and adjust any references to ControlMutationDefaultEntry/ControlMutationDefaultRegistry accordingly; ensure DefaultFunctionLoweringHandler and usageSignatures remain unchanged..github/workflows/record-cli.yml (1)
74-84: Consider pinning the PR action to a commit hash for consistency.Other actions in this workflow use commit hashes (e.g.,
actions/checkout@8e8c483...), butpeter-evans/create-pull-requestuses a version tag@v7. For supply-chain security consistency, consider pinning to a specific commit hash.Example with commit hash
- name: Create pull request - uses: peter-evans/create-pull-request@v7 + uses: peter-evans/create-pull-request@5e914681df9dc83aa4e4905692ca88bde2c9da4d # v7.0.5You can find the latest commit hash for v7 at https://github.com/peter-evans/create-pull-request/releases
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/record-cli.yml around lines 74 - 84, Replace the floating tag for the create-pull-request action with a pinned commit hash to match other actions' supply-chain pinning; locate the line using "uses: peter-evans/create-pull-request@v7" and update it to "uses: peter-evans/create-pull-request@<commit-hash>" (fetch the appropriate v7 commit SHA from the action's GitHub releases page), commit the workflow change, and ensure the rest of the block (commit-message, title, body, branch, delete-branch) remains unchanged.packages/1-framework/3-tooling/cli/scripts/record.ts (1)
54-54: Consider usingpatheinstead ofnode:path.Per repository conventions,
patheis preferred overnode:pathfor path operations in TypeScript files. This provides consistent behavior across platforms and works in non-Node runtimes.Based on learnings: "In the Prisma-next repository, prefer importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files)."
Suggested change
-import { dirname, join, relative, resolve } from 'node:path'; +import { dirname, join, relative, resolve } from 'pathe';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/scripts/record.ts` at line 54, Replace the import of path utilities from 'node:path' with equivalent named imports from 'pathe' (remove "import { dirname, join, relative, resolve } from 'node:path'") and update any uses of dirname, join, relative, and resolve in this file to rely on those 'pathe' exports (ensure tree-shaken named imports are used). This keeps behavior consistent with repository conventions and non-Node runtimes while preserving the existing symbol names (dirname, join, relative, resolve) used elsewhere in record.ts.packages/1-framework/3-tooling/cli/src/commands/db-verify.ts (1)
303-305: Remove theResult<never, CliStructuredError>cast in this branch.After
CliStructuredError.is(result.failure), you already have the exact failure value. Rebuilding the error result withnotOk(result.failure)keeps this branch type-safe and avoids an unchecked cast.As per coding guidelines "Avoid blind casts like `as unknown as X` in production TypeScript code. Use type predicates (`value is X`) or type guards instead to let the compiler narrow types safely."♻️ Suggested change
- if (CliStructuredError.is(result.failure)) { - const exitCode = handleResult(result as Result<never, CliStructuredError>, flags, ui); + if (CliStructuredError.is(result.failure)) { + const exitCode = handleResult(notOk(result.failure), flags, ui); process.exit(exitCode); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts` around lines 303 - 305, The branch uses an unsafe cast when calling handleResult; instead of casting to Result<never, CliStructuredError>, construct a proper error Result with notOk(result.failure) so the compiler can narrow types via the CliStructuredError.is() predicate; update the call to handleResult to pass notOk(result.failure) (keeping flags and ui) rather than using the Result<never, CliStructuredError> cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/subsystems/7. Migration System.md:
- Around line 241-250: Add a language tag to the fenced code block that shows
the migration directory tree so markdown linters won't raise MD040; locate the
triple-backtick fence immediately before the "migrations/" block in the
Migration System document and change it to include a language tag (for example,
```text) and ensure the closing triple-backticks remain unchanged.
In `@packages/1-framework/3-tooling/cli/CLI_RECORDING.md`:
- Around line 44-58: Update the unlabeled fenced code blocks in CLI_RECORDING.md
so markdownlint stops flagging them: change the triple-backtick fences around
the diagram (the block starting at the top shown in the diff and the second
block around lines 107-123) to include a language tag such as text or plaintext
(e.g., ```text) rather than being unlabeled; ensure both fenced blocks are
consistently updated to the chosen label so the document remains lint-clean.
In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts`:
- Around line 212-216: The schema structural check is being run without the
resolved dbConnection, causing mismatches; update the call to
client.schemaVerify to forward the selected dbConnection (the same one resolved
into dbConnection and passed to client.verify) so that schemaVerify({
contractIR: contractJson, strict: false, onProgress }) becomes schemaVerify({
contractIR: contractJson, strict: false, onProgress, dbConnection }) (or the
appropriate parameter name expected by client.schemaVerify) so both
client.verify and client.schemaVerify use the same connection context.
---
Nitpick comments:
In @.github/workflows/record-cli.yml:
- Around line 74-84: Replace the floating tag for the create-pull-request action
with a pinned commit hash to match other actions' supply-chain pinning; locate
the line using "uses: peter-evans/create-pull-request@v7" and update it to
"uses: peter-evans/create-pull-request@<commit-hash>" (fetch the appropriate v7
commit SHA from the action's GitHub releases page), commit the workflow change,
and ensure the rest of the block (commit-message, title, body, branch,
delete-branch) remains unchanged.
In `@docs/architecture` docs/subsystems/7. Migration System.md:
- Line 343: Update the wording to frame capabilities as requirements rather than
declarations: change the sentence to say that when a contract change only
affects capability requirements (no schema changes), the runner verifies the
environment satisfies the new requirements and updates the `profile_hash` in the
marker without applying any DDL, while the `core_hash` stays the same; ensure
you replace "capability declarations" with "capability requirements" wherever
this sentence appears so it aligns with adapter-reported/negotiated semantics
and the project's phrasing guidelines.
- Around line 363-370: The documented convention states errors must use a stable
category/code shape but the table mixes formats; update the two migration-style
codes to follow category/code (e.g., replace "MIGRATION.AMBIGUOUS_LEAF" and
"MIGRATION.NO_RESOLVABLE_LEAF" with "migration/ambiguous-leaf" and
"migration/no-resolvable-leaf") or alternatively change the convention line to
include the dot-separated form—make the change consistently in the table and any
references to the symbols "MIGRATION.AMBIGUOUS_LEAF" and
"MIGRATION.NO_RESOLVABLE_LEAF" so they match the chosen convention.
In `@packages/1-framework/3-tooling/cli/scripts/record.ts`:
- Line 54: Replace the import of path utilities from 'node:path' with equivalent
named imports from 'pathe' (remove "import { dirname, join, relative, resolve }
from 'node:path'") and update any uses of dirname, join, relative, and resolve
in this file to rely on those 'pathe' exports (ensure tree-shaken named imports
are used). This keeps behavior consistent with repository conventions and
non-Node runtimes while preserving the existing symbol names (dirname, join,
relative, resolve) used elsewhere in record.ts.
In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts`:
- Around line 303-305: The branch uses an unsafe cast when calling handleResult;
instead of casting to Result<never, CliStructuredError>, construct a proper
error Result with notOk(result.failure) so the compiler can narrow types via the
CliStructuredError.is() predicate; update the call to handleResult to pass
notOk(result.failure) (keeping flags and ui) rather than using the Result<never,
CliStructuredError> cast.
In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts`:
- Around line 37-42: The interfaces DefaultFunctionRegistryEntry and
ControlMutationDefaultEntry are identical; remove the duplication by making
ControlMutationDefaultEntry reuse DefaultFunctionRegistryEntry (either replace
ControlMutationDefaultEntry with DefaultFunctionRegistryEntry in
ControlMutationDefaultRegistry or declare ControlMutationDefaultEntry extends
DefaultFunctionRegistryEntry), then update the ControlMutationDefaultRegistry
type to ReadonlyMap<string, DefaultFunctionRegistryEntry> (or the extended name)
and adjust any references to
ControlMutationDefaultEntry/ControlMutationDefaultRegistry accordingly; ensure
DefaultFunctionLoweringHandler and usageSignatures remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 3d7aa9c9-7f60-4da9-ad72-774cca936723
⛔ Files ignored due to path filters (39)
packages/1-framework/3-tooling/cli/recordings/svgs/db-init/apply.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/db-init/dry-run.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/db-init/help.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/db-update/additive-dry-run.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/db-update/help.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/db-update/no-changes.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/direct-update/01-contract-emit-v2.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/direct-update/02-db-update-dry-run.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/direct-update/03-db-update-apply.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/direct-update/04-db-update-noop.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/direct-update/05-db-verify.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-missing-marker/01-db-verify-fail.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-missing-marker/02-db-schema-verify-fail.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-missing-marker/03-db-introspect-empty.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-missing-marker/04-db-init-recovery.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-missing-marker/05-db-verify-pass.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-phantom/01-db-verify-false-positive.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-phantom/02-db-schema-verify-fail.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-phantom/03-db-introspect-diverged.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-phantom/04-db-update-recovery.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-phantom/05-db-schema-verify-pass.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-stale-marker/01-db-verify-fail.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-stale-marker/02-db-schema-verify-fail.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-stale-marker/03-db-update-recovery.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/drift-stale-marker/04-db-verify-pass.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/01-contract-emit.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/02-db-init-dry-run.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/03-db-init.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/04-db-init-idempotent.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/05-db-verify.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/06-db-schema-verify.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/07-db-schema-verify-strict.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/08-db-introspect.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/09-db-verify-json.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/recordings/svgs/greenfield-setup/10-db-schema-verify-json.svgis excluded by!**/*.svgpackages/1-framework/3-tooling/cli/test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/psl-contract-authoring/plan.mdis excluded by!projects/**projects/psl-contract-authoring/spec.mdis excluded by!projects/**
📒 Files selected for processing (92)
.gitattributes.github/workflows/record-cli.ymldocker-compose.yamldocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mddocs/architecture docs/adrs/ADR 171 - Parameterized native types in contracts.mddocs/architecture docs/subsystems/7. Migration System.mdpackage.jsonpackages/1-framework/1-core/migration/control-plane/src/errors.tspackages/1-framework/1-core/migration/control-plane/src/exports/errors.tspackages/1-framework/3-tooling/cli/CLI_RECORDING.mdpackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/package.jsonpackages/1-framework/3-tooling/cli/recordings/.gitignorepackages/1-framework/3-tooling/cli/recordings/ascii/db-init/apply.asciipackages/1-framework/3-tooling/cli/recordings/ascii/db-init/dry-run.asciipackages/1-framework/3-tooling/cli/recordings/ascii/db-init/help.asciipackages/1-framework/3-tooling/cli/recordings/ascii/db-update/additive-dry-run.asciipackages/1-framework/3-tooling/cli/recordings/ascii/db-update/help.asciipackages/1-framework/3-tooling/cli/recordings/ascii/db-update/no-changes.asciipackages/1-framework/3-tooling/cli/recordings/ascii/direct-update/01-contract-emit-v2.asciipackages/1-framework/3-tooling/cli/recordings/ascii/direct-update/02-db-update-dry-run.asciipackages/1-framework/3-tooling/cli/recordings/ascii/direct-update/03-db-update-apply.asciipackages/1-framework/3-tooling/cli/recordings/ascii/direct-update/04-db-update-noop.asciipackages/1-framework/3-tooling/cli/recordings/ascii/direct-update/05-db-verify.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/01-db-verify-fail.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/02-db-schema-verify-fail.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/03-db-introspect-empty.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/04-db-init-recovery.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/05-db-verify-pass.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/01-db-verify-false-positive.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/02-db-schema-verify-fail.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/03-db-introspect-diverged.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/04-db-update-recovery.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/05-db-schema-verify-pass.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/01-db-verify-fail.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/02-db-schema-verify-fail.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/03-db-update-recovery.asciipackages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/04-db-verify-pass.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/01-contract-emit.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/02-db-init-dry-run.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/03-db-init.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/04-db-init-idempotent.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/05-db-verify.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/06-db-schema-verify.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/07-db-schema-verify-strict.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/08-db-introspect.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/09-db-verify-json.asciipackages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/10-db-schema-verify-json.asciipackages/1-framework/3-tooling/cli/recordings/config.tspackages/1-framework/3-tooling/cli/recordings/fixtures/contract-additive.tspackages/1-framework/3-tooling/cli/recordings/fixtures/contract-base.tspackages/1-framework/3-tooling/cli/recordings/fixtures/prisma-next.config.tspackages/1-framework/3-tooling/cli/scripts/record.tspackages/1-framework/3-tooling/cli/src/commands/db-verify.tspackages/1-framework/3-tooling/cli/src/utils/cli-errors.tspackages/1-framework/3-tooling/cli/src/utils/formatters/verify.tspackages/1-framework/3-tooling/cli/src/utils/global-flags.tspackages/1-framework/3-tooling/cli/test/setup.tspackages/1-framework/3-tooling/cli/test/utils/test-helpers.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tstest/integration/.gitignoretest/integration/package.jsontest/integration/test/cli-journeys/README.mdtest/integration/test/cli-journeys/brownfield-adoption.e2e.test.tstest/integration/test/cli-journeys/config-errors.e2e.test.tstest/integration/test/cli-journeys/connection-and-contract-errors.e2e.test.tstest/integration/test/cli-journeys/db-update-workflows.e2e.test.tstest/integration/test/cli-journeys/drift-marker.e2e.test.tstest/integration/test/cli-journeys/drift-migration-dag.e2e.test.tstest/integration/test/cli-journeys/drift-schema.e2e.test.tstest/integration/test/cli-journeys/greenfield-setup.e2e.test.tstest/integration/test/cli-journeys/help-and-flags.e2e.test.tstest/integration/test/cli-journeys/multi-step-migration.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.tstest/integration/test/cli.db-verify.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-add-table.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-additive.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-base.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-destructive.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-v3.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-verify/contract.tstest/integration/test/utils/journey-test-helpers.tstest/integration/vitest.journeys.config.tsturbo.json
✅ Files skipped from review due to trivial changes (48)
- packages/1-framework/3-tooling/cli/package.json
- docs/architecture docs/adrs/ADR 171 - Parameterized native types in contracts.md
- .gitattributes
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/03-db-update-recovery.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/04-db-init-idempotent.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/01-db-verify-fail.ascii
- packages/1-framework/1-core/migration/control-plane/src/exports/errors.ts
- packages/1-framework/3-tooling/cli/recordings/ascii/direct-update/04-db-update-noop.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/direct-update/01-contract-emit-v2.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/db-init/apply.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/02-db-schema-verify-fail.ascii
- docker-compose.yaml
- packages/1-framework/3-tooling/cli/recordings/ascii/db-update/no-changes.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/05-db-verify.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/06-db-schema-verify.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/02-db-init-dry-run.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/db-update/additive-dry-run.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/db-init/dry-run.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/direct-update/05-db-verify.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/03-db-introspect-empty.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/03-db-init.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/db-update/help.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/04-db-verify-pass.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/db-init/help.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/direct-update/03-db-update-apply.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/08-db-introspect.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/05-db-schema-verify-pass.ascii
- package.json
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/01-db-verify-fail.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/direct-update/02-db-update-dry-run.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/02-db-schema-verify-fail.ascii
- docs/architecture docs/ADR-INDEX.md
- packages/1-framework/3-tooling/cli/recordings/fixtures/contract-base.ts
- packages/1-framework/3-tooling/cli/recordings/fixtures/prisma-next.config.ts
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/03-db-introspect-diverged.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/09-db-verify-json.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/01-db-verify-false-positive.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/04-db-init-recovery.ascii
- packages/1-framework/3-tooling/cli/src/utils/cli-errors.ts
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/07-db-schema-verify-strict.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-stale-marker/02-db-schema-verify-fail.ascii
- packages/1-framework/3-tooling/cli/test/setup.ts
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/10-db-schema-verify-json.ascii
- packages/1-framework/3-tooling/cli/recordings/config.ts
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-missing-marker/05-db-verify-pass.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/drift-phantom/04-db-update-recovery.ascii
- packages/1-framework/3-tooling/cli/recordings/ascii/greenfield-setup/01-contract-emit.ascii
- packages/1-framework/3-tooling/cli/recordings/.gitignore
wmadden
added a commit
that referenced
this pull request
Mar 20, 2026
…ated defaults Verifies that applyMutationDefaults skips the generator when the user explicitly provides a value for a column that has a generated default. PR #214 review thread (SevInf)
wmadden
added a commit
that referenced
this pull request
Mar 20, 2026
Rename from 'includes both owners when duplicate generator ids are composed' to 'throws error naming both owners when duplicate generator ids are composed' so the title reflects this is an error-path test. PR #214 review thread (SevInf)
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
60e904e to
f8f9355
Compare
…gistry Route @default(functionCall) lowering through an injected registry so function semantics stay out of the parser and interpreter core. This adds TS-parity mappings for uuid/ulid/nanoid/dbgenerated, preserves storage defaults/literals, and emits stable diagnostics for unknown functions versus invalid arguments.
…yping Narrow generator ids to the contract union and update tests to use index-signature-safe assertions. This keeps Milestone 5 coverage intact while satisfying repo-wide strict typecheck settings.
Address review feedback by replacing blind ID generator casts with a type guard, removing the direct re-export pattern in ids index, narrowing the interpreter builder cast to a single cast, and documenting the intentional test-only escape.
Resolve review feedback by switching inline numeric timeout values to shared @prisma-next/test-utils timeout constants in the affected test suites.
Update SQL runtime architecture docs to include mutation default generators and remove an invalid postgres target dependency, with lockfile updates from pnpm install.
…tests
After rebase, the interpreter requires target and scalarTypeDescriptors
in all calls. Tests that only passed { document } now fail with
PSL_TARGET_CONTEXT_REQUIRED. Add baseInput with postgres fixtures and
pass baseOptions to provider helper calls missing the options argument.
Extract builtinGeneratorIds and BuiltinGeneratorId into generator-ids.ts so that index.ts (the main entry point) no longer transitively imports uniku. This prevents uniku side-effect imports from leaking into downstream dist bundles (e.g. target-postgres/control.mjs) where uniku is not a declared dependency.
Add error-path test for generateId with unknown generator id in ids package. Add comprehensive unit tests for control-mutation-defaults.ts in adapter-postgres covering all default function lowering handlers, generator descriptors, and PSL scalar type descriptors.
- Remove unused builtinGeneratorIds import from generators.ts - Import BuiltinGeneratorId from generator-ids.ts in runtime.ts (moved during previous refactor) - Fix test stubs to match ControlMutationDefaultFunctionCall and ControlMutationDefaultLoweringContext type shapes (required raw, span, modelName, fieldName properties)
Record the thin-core/fat-interfaces boundary and add a follow-on spec to move ID generator implementations and privileged vocabularies out of low layers into composed components.
Semgrep flags generator.generate(...) with an Express/wkhtml SSRF rule. Suppress the rule at the callsite.
Semgrep auto ruleset misclassifies generator.generate(...) as an Express wkhtml/phantom SSRF sink. Exclude the specific rule from PR code scanning to avoid noisy false positives.
Inline the generator map logic into assembleControlMutationDefaultContributions so each descriptor's controlMutationDefaults() is called exactly once, avoiding snapshot drift between generator and function collection passes. Add regression test asserting single evaluation per descriptor.
resolveNanoidColumnDescriptor now throws when size is present but not an integer between 2 and 255, preventing generated.params from diverging from the declared column descriptor.
Adds ADR 170 to define pack-provided type constructors and field presets. Records dot namespacing, hard-error duplicates, and that presets may imply constraints. Updates the ids follow-up spec to use composed column helpers aligned with this model.
Rephrases ADR 170 to be more accessible and intent-first while preserving the same decisions.
Updates ADR 170 to lead with concrete TS and PSL examples and clarifies that common family or target helpers may be unnamespaced while extension helpers stay namespaced.
…r rebase Remove obsolete codecTypes/operationTypes from test contract mappings, matching upstream changes to SqlMappings type.
… changes After rebasing onto main, the provider source callback now receives a ContractSourceContext parameter (providing composedExtensionPacks from the CLI layer), target and scalarTypeDescriptors are required in PrismaContractOptions, and pgvector nativeType no longer includes params (just 'vector' with typeParams.length separate). - Widen ControlMutationDefaults to accept control-layer handler types (unknown return) so assembly output is directly assignable - Update demo config to use assemblePslInterpretationContributions - Fix integration test to pass required prismaContract options - Align pgvector test expectations with nativeType expansion hooks
…ies across ADRs ADR 169: rewrite section 3 to separate surface-agnostic mutation default descriptors from surface-specific argument resolution, with explicit naming guidance against PSL-coupled terminology in shared types. ADR 170: cross-reference ADR 169 (defaults) and new ADR 171 (typeParams). Renumber parameterized native types ADR from 170 to 171 to resolve the number collision with type constructors and field presets.
Add ADR 170 implementation as a follow-up milestone in the PSL contract authoring project plan and spec. Type constructors (e.g. Uuid(7)) and field presets (e.g. CreatedAt) bundle column type + mutation default + constraints into a single pack-provided declaration. Linear: TML-2093
…lt function handlers ControlMutationDefaultFunctionHandler previously returned `unknown`, losing type information at the assembly boundary and forcing an unsafe `as LoweredDefaultResult` cast in the PSL interpreter. Define ControlMutationDefaultFunctionResult as the precise discriminated union in family-sql, update ControlMutationDefaultEntry.lower in contract-psl to match, and remove the cast.
…ated defaults Verifies that applyMutationDefaults skips the generator when the user explicitly provides a value for a column that has a generated default. PR #214 review thread (SevInf)
Rename from 'includes both owners when duplicate generator ids are composed' to 'throws error naming both owners when duplicate generator ids are composed' so the title reflects this is an error-path test. PR #214 review thread (SevInf)
wmadden
force-pushed
the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
from
f8f9355 to
cfb9fb4
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/2-sql/1-core/contract/test/validate.test.ts (1)
65-107: Split this spec before adding more scenarios.This file is already well above the 500-line limit; please move these new execution-default cases into a focused split file (for example
validate.execution-defaults.test.ts) to keep tests navigable.As per coding guidelines, "Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/test/validate.test.ts` around lines 65 - 107, Move the two execution-default specs ("accepts custom execution default generator ids" and "rejects non-flat execution default generator ids") out of this large validate.test.ts into a new focused test file for execution-default scenarios (e.g., validate.execution-defaults.test.ts); retain the same test bodies and imports (keep references to validateContract, SqlContract, SqlStorage, and baseContract) and update any test-suite describe blocks as needed so the tests run standalone and the original file is kept under 500 lines.packages/2-sql/1-core/contract/src/validators.ts (1)
24-26: Centralize generator-id validation to prevent cross-layer drift.This regex/schema now exists in multiple places; consider exporting a shared
GeneratorIdSchema(or at least a shared regex constant) so contract-ts and core validation can’t diverge over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/src/validators.ts` around lines 24 - 26, The generator-id validation is duplicated across layers; extract and export a single shared schema/constant (e.g., GeneratorIdSchema or GENERATOR_ID_REGEX) and replace local occurrences of generatorIdSchema with an import of that shared symbol so contract-ts and core validation remain consistent; update the current generatorIdSchema definition to re-export the centralized validation (use the same type('string').narrow(...) or a reusable regex constant) and adjust imports in any modules that currently define their own regex.packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts (1)
230-258: Add a closest-match hint toPSL_UNKNOWN_DEFAULT_FUNCTION.The current fallback always dumps the full supported-signature list, which is noisy for simple typos. Since the registry already carries
usageSignatures, this is a good place to prepend a nearest-match hint without changing the new composition model.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts` around lines 230 - 258, Update lowerDefaultFunctionWithRegistry to compute the nearest registry key to input.call.name (using a simple string-distance / fuzzy-match over registry.keys(), and prefer matching against usageSignatures entries when available) and prepend a short "Did you mean 'X'?" hint to the diagnostic message before the full Supported functions list; keep formatSupportedFunctionList unchanged but you may reuse its output for the full list while constructing the hint, and reference symbols: lowerDefaultFunctionWithRegistry, formatSupportedFunctionList, ControlMutationDefaultRegistry, entry.usageSignatures, and input.call.name.packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts (1)
24-131: Move these new cases into the existing split interpreter suites.The added context/default/diagnostic coverage pushes this file further into an all-in-one test suite even though the package already has more focused siblings for those concerns. Moving these blocks there will keep failures easier to localize and stop
interpreter.test.tsfrom growing past the repo’s test-file-size boundary again.As per coding guidelines, “Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files.”
Also applies to: 597-862
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts` around lines 24 - 131, Move the three new "it" test cases into the more focused interpreter suites that already handle context, defaults and diagnostics instead of leaving them in the monolithic interpreter.test.ts: specifically relocate the tests titled "returns diagnostics when target context is missing", "uses composed scalar type descriptors without hardcoded fallback", and "does not derive generated column type without descriptor resolver" into the existing split suites that handle diagnostics, scalar descriptor composition, and default/generator behavior; update those target files to import and use the same helpers referenced here (parsePslDocument, interpretPslDocumentToSqlContractIRInternal, postgresTarget, postgresScalarTypeDescriptors, builtinControlMutationDefaults) and ensure any local test-specific fixtures (the custom scalarTypeDescriptors Map and controlMutationDefaults block) are moved with their tests; finally remove these three blocks from this file so interpreter.test.ts stays under the test-file-size guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/2-authoring/ids/src/index.ts`:
- Around line 89-106: Return deep-cloned or frozen copies of registry data
instead of sharing references: when building builtinGeneratorRegistryMetadata,
copy or Object.freeze the applicableCodecIds array and any object wrappers so
callers can't mutate builtinGeneratorMetadataById state; in
resolveBuiltinGeneratedColumnDescriptor, if
metadata.resolveGeneratedColumnDescriptor exists return a cloned/frozen result
(and if falling back to metadata.generatedColumnDescriptor return a
cloned/frozen copy) so callers cannot mutate shared typeParams,
applicableCodecIds, or generatedColumnDescriptor objects from the
builtinGeneratorMetadataById registry.
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 89-107: The test currently asserts any "Contract structural
validation failed" but should assert the specific flat-id generator failure;
update the assertion in the test that calls validateContract (the case building
a contract with execution.mutations.defaults containing
onCreate.kind='generator' id='pack/slugid') to expect the precise validation
message or pattern for non-flat generator ids (e.g. match the validator's error
text for "flat id" or the path under execution.mutations.defaults/onCreate)
instead of the generic structural failure.
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 35-40: The input interface declares target and
scalarTypeDescriptors as required but the implementation (function using
InterpretPslDocumentToSqlContractIRInput) defensively checks for their absence
and returns PSL_TARGET_CONTEXT_REQUIRED and PSL_SCALAR_TYPE_CONTEXT_REQUIRED;
update the contract to match runtime behavior by making both target and
scalarTypeDescriptors optional on InterpretPslDocumentToSqlContractIRInput
(e.g., mark as ?), keep the existing runtime checks in the interpreter (where
PSL_TARGET_CONTEXT_REQUIRED and PSL_SCALAR_TYPE_CONTEXT_REQUIRED are returned)
and update any callers/tests to either supply those fields or intentionally omit
them without needing as unknown casts; do not remove the runtime guards unless
you intentionally want the caller to be required to provide them.
In `@packages/2-sql/3-tooling/family/test/mutation-default-assembly.test.ts`:
- Around line 127-129: The test expectations for
assembleControlMutationDefaultContributions currently only assert the duplicated
function name and so miss checking that the error message includes the
conflicting owner/descriptors; update the assertions (for the occurrences around
the current expect and also at the other two locations referenced) to assert
that the thrown error message includes the two descriptor/owner identifiers
(e.g. both descriptor ids or owner names) in addition to the duplicate function
id by changing the matcher to check for those owner strings (use a regex or
snapshot that includes both owner identifiers) so regressions that drop owner
context will fail.
In `@packages/2-sql/5-runtime/src/sql-context.ts`:
- Around line 58-60: The RuntimeMutationDefaultGenerator currently exposes
generate(params?) but doesn't validate spec.params before calling it; add a
params schema/validator field to the RuntimeMutationDefaultGenerator interface
(e.g., paramsSchema or validateParams using Arktype) and, in places where
generator.generate(...) is invoked (the current call site around
RuntimeMutationDefaultGenerator usage and the other similar path at the section
referenced ~381-399), run the schema validator (reuse the pattern from
validateTypeParams()) and throw a stable runtime error when validation fails;
only call generator.generate with validated/parsed params to ensure
malformed/stale contract data is rejected early and consistently.
In `@packages/2-sql/5-runtime/test/mutation-default-generators.test.ts`:
- Around line 218-255: Add a small params-forwarding test that exercises the
generateId({ id, params }) branch of createTestMutationDefaultGenerators: extend
the existing test suite to include a contract default using a built-in generator
(e.g., id: 'nanoid') with explicit params (for example size/length) and call
context.applyMutationDefaults({ op: 'create', table: 'user', values: {} });
assert the returned/generated id respects the passed params (e.g., length or
pattern) so the test fails if params are not forwarded to generateId; reference
generateId and createTestMutationDefaultGenerators and use
context.applyMutationDefaults to locate where to add the case.
---
Nitpick comments:
In `@packages/2-sql/1-core/contract/src/validators.ts`:
- Around line 24-26: The generator-id validation is duplicated across layers;
extract and export a single shared schema/constant (e.g., GeneratorIdSchema or
GENERATOR_ID_REGEX) and replace local occurrences of generatorIdSchema with an
import of that shared symbol so contract-ts and core validation remain
consistent; update the current generatorIdSchema definition to re-export the
centralized validation (use the same type('string').narrow(...) or a reusable
regex constant) and adjust imports in any modules that currently define their
own regex.
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 65-107: Move the two execution-default specs ("accepts custom
execution default generator ids" and "rejects non-flat execution default
generator ids") out of this large validate.test.ts into a new focused test file
for execution-default scenarios (e.g., validate.execution-defaults.test.ts);
retain the same test bodies and imports (keep references to validateContract,
SqlContract, SqlStorage, and baseContract) and update any test-suite describe
blocks as needed so the tests run standalone and the original file is kept under
500 lines.
In `@packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts`:
- Around line 230-258: Update lowerDefaultFunctionWithRegistry to compute the
nearest registry key to input.call.name (using a simple string-distance /
fuzzy-match over registry.keys(), and prefer matching against usageSignatures
entries when available) and prepend a short "Did you mean 'X'?" hint to the
diagnostic message before the full Supported functions list; keep
formatSupportedFunctionList unchanged but you may reuse its output for the full
list while constructing the hint, and reference symbols:
lowerDefaultFunctionWithRegistry, formatSupportedFunctionList,
ControlMutationDefaultRegistry, entry.usageSignatures, and input.call.name.
In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts`:
- Around line 24-131: Move the three new "it" test cases into the more focused
interpreter suites that already handle context, defaults and diagnostics instead
of leaving them in the monolithic interpreter.test.ts: specifically relocate the
tests titled "returns diagnostics when target context is missing", "uses
composed scalar type descriptors without hardcoded fallback", and "does not
derive generated column type without descriptor resolver" into the existing
split suites that handle diagnostics, scalar descriptor composition, and
default/generator behavior; update those target files to import and use the same
helpers referenced here (parsePslDocument,
interpretPslDocumentToSqlContractIRInternal, postgresTarget,
postgresScalarTypeDescriptors, builtinControlMutationDefaults) and ensure any
local test-specific fixtures (the custom scalarTypeDescriptors Map and
controlMutationDefaults block) are moved with their tests; finally remove these
three blocks from this file so interpreter.test.ts stays under the
test-file-size guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e53156d3-a62f-4257-9a53-f544621de649
⛔ Files ignored due to path filters (16)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/psl-contract-authoring/plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/Follow-up - Pack-provided mutation default functions registry-plan.mdis excluded by!projects/**projects/psl-contract-authoring/plans/plan.mdis excluded by!projects/**projects/psl-contract-authoring/references/authoring-surface-gap-inventory.mdis excluded by!projects/**projects/psl-contract-authoring/references/category-error-built-in-id-generators.mdis excluded by!projects/**projects/psl-contract-authoring/spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Follow-up - Pack-provided mutation default functions registry.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 1 - Pluggable contract sources.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 2 - PSL parser.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 3 - Fixture-driven parity harness.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 4 - Parameterized attributes and pgvector parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 5 - ID variants and default function parity.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 6 - Close-out.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/Milestone 7 - PSL sql template literal syntax.spec.mdis excluded by!projects/**projects/psl-contract-authoring/specs/follow-up-move-id-generators-to-composition.spec.mdis excluded by!projects/**
📒 Files selected for processing (55)
.cursor/rules/use-timeouts-helper-in-tests.mdc.github/workflows/pr-code-security.ymldocs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mddocs/architecture docs/adrs/ADR 171 - Parameterized native types in contracts.mdexamples/prisma-next-demo/prisma-next.config.tspackages/1-framework/1-core/shared/contract/src/types.tspackages/1-framework/2-authoring/ids/src/generator-ids.tspackages/1-framework/2-authoring/ids/src/generators.tspackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/2-authoring/ids/src/runtime.tspackages/1-framework/2-authoring/ids/test/ids.test.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.jsonpackages/2-sql/2-authoring/contract-ts/src/contract.tspackages/2-sql/3-tooling/family/README.mdpackages/2-sql/3-tooling/family/src/core/assembly.tspackages/2-sql/3-tooling/family/src/core/migrations/types.tspackages/2-sql/3-tooling/family/src/exports/control.tspackages/2-sql/3-tooling/family/test/mutation-default-assembly.test.tspackages/2-sql/4-lanes/sql-lane/test/utils/guards.test.tspackages/2-sql/5-runtime/README.mdpackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tstest/integration/test/authoring/parity/default-pack-slugid/contract.tstest/integration/test/authoring/parity/default-pack-slugid/expected.contract.jsontest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/default-pack-slugid/schema.prismatest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
✅ Files skipped from review due to trivial changes (18)
- .cursor/rules/use-timeouts-helper-in-tests.mdc
- packages/1-framework/1-core/shared/contract/src/types.ts
- docs/architecture docs/adrs/ADR 171 - Parameterized native types in contracts.md
- packages/1-framework/2-authoring/ids/src/generators.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/1-framework/2-authoring/ids/src/generator-ids.ts
- packages/1-framework/2-authoring/ids/test/ids.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts
- test/integration/test/authoring/parity/default-pack-slugid/schema.prisma
- docs/architecture docs/ADR-INDEX.md
- packages/2-sql/2-authoring/contract-psl/src/exports/index.ts
- packages/2-sql/3-tooling/family/src/core/assembly.ts
- packages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.ts
- packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- packages/2-sql/3-tooling/family/src/core/migrations/types.ts
- packages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts
- test/integration/test/authoring/parity/default-pack-slugid/expected.contract.json
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.ts
- packages/2-sql/4-lanes/sql-lane/test/utils/guards.test.ts
- packages/1-framework/2-authoring/ids/src/runtime.ts
- packages/3-targets/6-adapters/postgres/src/exports/runtime.ts
- examples/prisma-next-demo/prisma-next.config.ts
- packages/2-sql/5-runtime/src/exports/index.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/control.ts
- test/integration/test/authoring/templates/prisma-next.config.parity-psl.ts
- packages/3-targets/6-adapters/postgres/test/adapter.test.ts
- docs/architecture docs/adrs/ADR 169 - Declared applicability for mutation default generators.md
- test/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
- packages/2-sql/2-authoring/contract-ts/src/contract.ts
- test/integration/test/authoring/parity/default-pack-slugid/contract.ts
- packages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.json
- packages/2-sql/3-tooling/family/src/exports/control.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/2-sql/2-authoring/contract-psl/test/fixtures.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.ts
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
… and extensionPacks The rebase onto main brought in changes that now emit capabilities and extensionPacks into contracts. Update the golden file to match.
wmadden
added a commit
that referenced
this pull request
Mar 23, 2026
…ated defaults Verifies that applyMutationDefaults skips the generator when the user explicitly provides a value for a column that has a generated default. PR #214 review thread (SevInf)
wmadden
added a commit
that referenced
this pull request
Mar 23, 2026
Rename from 'includes both owners when duplicate generator ids are composed' to 'throws error naming both owners when duplicate generator ids are composed' so the title reflects this is an error-path test. PR #214 review thread (SevInf)
wmadden
deleted the
tml-2025-follow-up-pack-provided-mutation-default-functions-registry-remove-psl
branch
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
3 participants
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.
closes TML-2025
Key snippet(s)
Runtime execution: resolve default handler by contract id
Intent
Make mutation default behavior component-composable end-to-end across two distinct seams: control-time PSL lowering composes a default-function vocabulary plus generator applicability descriptors, and runtime mutation-default application composes generator implementations that resolve emitted
idreferences (with stable missing-id errors).Change map
The story
ControlMutationDefaultscontribution bundles a default-function lowering registry and generator descriptors that declare applicablecodecIds, plus assembly rules that fail on duplicates.@default(...)calls via the registry, then validates declared applicability by comparing the column’scodecIdto the generator descriptor’sapplicableCodecIds.mutationDefaultGenerators()from target/adapter/packs, andapplyMutationDefaultsresolves each emitted generatoridfrom that runtime registry with a stable missing-id error.Behavior changes & evidence
PSL default-function support is registry-driven: PSL
@default(uuid())is only accepted when composed components contribute theuuidhandler; otherwise it fails as unknown.Applicability is enforced at emit-time using declared codec ids:
@default(slugid())lowers to an execution default only when the generator descriptor declares applicability to the column’scodecId; otherwise it emits a targeted diagnostic.Runtime mutation defaults resolve emitted generator ids through composed runtime components: runtime no longer relies on hardwired generator wiring; it looks up
execution.mutations.defaults[*].onCreate.idin the assembled runtime registry, and throws a stable error when missing.Compatibility / migration / risk
target+scalarTypeDescriptorsinputs, and accepts composedcontrolMutationDefaultsas an optional input; there is no default target or provider-local assembly.RUNTIME.MUTATION_DEFAULT_GENERATOR_MISSING).Non-goals / intentionally out of scope
codecId-only).Summary by CodeRabbit
New Features
Documentation
Breaking Changes