Open
Conversation
📝 WalkthroughWalkthroughAdds a Mongo "data transform" migration operation and end-to-end support: new types, a dataTransform factory, serialization/deserialization, runner execution with pre/post checks and idempotency, driver/adapter wiring changes, tests, and example migrations demonstrating backfills. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Migration Client
participant Runner as MongoMigrationRunner
participant Adapter as MongoAdapter
participant Driver as MongoDriver
participant DB as MongoDB
Client->>Runner: execute(dataTransformOp)
Runner->>Runner: evaluate postchecks (idempotency)
alt all postchecks satisfied
Runner-->>Client: skipped (idempotent)
else postchecks not satisfied
Runner->>Runner: evaluate prechecks
alt any precheck failed
Runner-->>Client: PRECHECK_FAILED
else prechecks pass
loop each run plan
Runner->>Adapter: lower(plan)
Adapter-->>Runner: wireCommand
Runner->>Driver: execute(wireCommand)
Driver->>DB: run command
DB-->>Driver: rows/result
Driver-->>Runner: consumed/ok
end
Runner->>Runner: evaluate postchecks
alt postchecks satisfied
Runner-->>Client: success
else
Runner-->>Client: POSTCHECK_FAILED
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-pipeline-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
7ca0000 to
f5a79ad
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.ts (1)
51-53: Consider documenting the lifecycle contract.The empty
close()implementation with comment "Connection lifecycle managed externally" is correct, but consider whether theMongoDriverinterface should document this expectation to prevent misuse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.ts` around lines 51 - 53, The empty close() in dml-executor.ts relies on an external lifecycle contract that isn't documented; update the MongoDriver interface (the close() method) to include a clear doc comment/JSDoc stating that connection lifecycle is managed externally and implementations may be a no-op, and add a short JSDoc comment above the async close() in DmlExecutor (or the class containing close()) mirroring that contract so callers and implementors understand it's intentionally a no-op.packages/3-mongo-target/1-mongo-target/src/core/migration-factories.ts (1)
45-56: Cast toMongoQueryPlanassumes caller provides valid input.After excluding
symbolandBuildable, the remaining case is cast directly toMongoQueryPlanwithout validation. This relies on callers (thedataTransformfunction) only passing valid types, which is enforced by TypeScript at compile time but not at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/src/core/migration-factories.ts` around lines 45 - 56, resolveQueryResult currently casts any non-symbol/non-Buildable value to MongoQueryPlan without runtime validation; add a runtime type-guard for MongoQueryPlan (e.g., check required properties/methods that a plan must have) and throw a clear error if the check fails. Update resolveQueryResult to: after the isBuildable branch, verify the incoming result has the expected MongoQueryPlan shape (or implement an isMongoQueryPlan helper), then return it; if the check fails, throw an Error describing the invalid runtime type and include the offending value for debugging.packages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.ts (1)
262-299: Consider adding tests for additional pipeline stage kinds.Based on the
MongoPipelineStagetype definition (context snippet 4), additional stages likeskip,unwind,group,replaceRoot,count,sortByCount,sample,redact, andoutare supported but not tested here. While the current coverage is good for the most common stages, you may want to add tests for these in a follow-up to ensure complete deserialization coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.ts` around lines 262 - 299, Add unit tests covering the additional MongoPipelineStage variants so deserializePipelineStage is validated for all supported kinds: create test cases that instantiate stages like MongoSkipStage, MongoUnwindStage, MongoGroupStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage, MongoSampleStage, MongoRedactStage, and MongoOutStage (using their unique constructors/properties), JSON.stringify/JSON.parse each, call deserializePipelineStage(json) and assert result.kind and the returned stage's key properties (e.g., skip count, unwind field, group spec, replaceRoot spec, count field, sortByCount spec, sample size, redact expression, out target) to ensure correct round-trip deserialization.packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts (1)
271-288: Consider adding early termination for check evaluation.The current implementation collects all documents from the driver before evaluating filters (line 279-282). For checks that only need to confirm existence (
expect: 'exists'), this could be optimized to terminate early when the first matching document is found, avoiding unnecessary memory allocation for large result sets.💡 Optional optimization for early termination
private async evaluateDataTransformChecks( checks: readonly MongoDataTransformCheck[], adapter: MongoAdapter, driver: MongoDriver, filterEvaluator: FilterEvaluator, ): Promise<boolean> { for (const check of checks) { const wireCommand = adapter.lower(check.source); - const documents: Record<string, unknown>[] = []; - for await (const row of driver.execute<Record<string, unknown>>(wireCommand)) { - documents.push(row); + let matchFound = false; + for await (const row of driver.execute<Record<string, unknown>>(wireCommand)) { + if (filterEvaluator.evaluate(check.filter, row)) { + matchFound = true; + break; // Early termination when match found + } } - const matchFound = documents.some((doc) => filterEvaluator.evaluate(check.filter, doc)); const passed = check.expect === 'exists' ? matchFound : !matchFound; if (!passed) return false; } return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts` around lines 271 - 288, The loop in evaluateDataTransformChecks currently materializes all rows into documents before filtering; change it to stream-evaluate rows from driver.execute(wireCommand) directly using filterEvaluator.evaluate(check.filter, row) and avoid pushing into documents. For expect === 'exists' stop and mark the check passed as soon as a matching row is found; for expect === 'not exists' stop and return false as soon as a match is found, otherwise exhaust the iterator to confirm no matches. Update evaluateDataTransformChecks (and usage of MongoAdapter.lower, driver.execute, filterEvaluator.evaluate, and check.expect) to implement this early-termination behavior and remove the intermediate documents array.
🤖 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/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts`:
- Around line 303-361: deserializePipelineStage currently handles only 7 kinds
and will throw for the other MongoPipelineStage variants; extend its switch to
cover the missing kinds: add cases for 'skip', 'unwind', 'group', 'replaceRoot',
'count', 'sortByCount', 'sample', 'redact', and 'out', constructing the
corresponding stage objects (e.g., MongoSkipStage, MongoUnwindStage,
MongoGroupStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage,
MongoSampleStage, MongoRedactStage, MongoOutStage) and mapping record fields to
constructor args; for nested/complex fields reuse existing helpers like
deserializeFilterExpr, deserializePipelineStage, deserializeExpression (or the
appropriate deserializer in this file) to convert sub-documents/arrays (e.g.,
group specs, unwind path/arrayHandling, replaceRoot/newRoot, sample/size,
sortByCount/expression, redact/expression, out/collection spec) so the full
MongoPipelineStage union is supported and no unknown-kind error occurs.
In `@packages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts`:
- Around line 476-484: The tests call an undefined helper makeDriver() when
invoking runner.execute (e.g., driver: makeDriver()), causing a ReferenceError;
fix by either adding a simple makeDriver() factory that returns the expected
driver shape used by makeRunner()/runner.execute, or remove the driver:
makeDriver() argument from the failing test cases and let runner.execute rely on
its default/implicit driver like earlier tests do; update all occurrences where
driver: makeDriver() is used (search for makeDriver in this test file) so the
tests either supply a valid driver object or omit the parameter consistently.
---
Nitpick comments:
In `@packages/3-mongo-target/1-mongo-target/src/core/migration-factories.ts`:
- Around line 45-56: resolveQueryResult currently casts any
non-symbol/non-Buildable value to MongoQueryPlan without runtime validation; add
a runtime type-guard for MongoQueryPlan (e.g., check required properties/methods
that a plan must have) and throw a clear error if the check fails. Update
resolveQueryResult to: after the isBuildable branch, verify the incoming result
has the expected MongoQueryPlan shape (or implement an isMongoQueryPlan helper),
then return it; if the check fails, throw an Error describing the invalid
runtime type and include the offending value for debugging.
In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts`:
- Around line 271-288: The loop in evaluateDataTransformChecks currently
materializes all rows into documents before filtering; change it to
stream-evaluate rows from driver.execute(wireCommand) directly using
filterEvaluator.evaluate(check.filter, row) and avoid pushing into documents.
For expect === 'exists' stop and mark the check passed as soon as a matching row
is found; for expect === 'not exists' stop and return false as soon as a match
is found, otherwise exhaust the iterator to confirm no matches. Update
evaluateDataTransformChecks (and usage of MongoAdapter.lower, driver.execute,
filterEvaluator.evaluate, and check.expect) to implement this early-termination
behavior and remove the intermediate documents array.
In
`@packages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.ts`:
- Around line 262-299: Add unit tests covering the additional MongoPipelineStage
variants so deserializePipelineStage is validated for all supported kinds:
create test cases that instantiate stages like MongoSkipStage, MongoUnwindStage,
MongoGroupStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage,
MongoSampleStage, MongoRedactStage, and MongoOutStage (using their unique
constructors/properties), JSON.stringify/JSON.parse each, call
deserializePipelineStage(json) and assert result.kind and the returned stage's
key properties (e.g., skip count, unwind field, group spec, replaceRoot spec,
count field, sortByCount spec, sample size, redact expression, out target) to
ensure correct round-trip deserialization.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.ts`:
- Around line 51-53: The empty close() in dml-executor.ts relies on an external
lifecycle contract that isn't documented; update the MongoDriver interface (the
close() method) to include a clear doc comment/JSDoc stating that connection
lifecycle is managed externally and implementations may be a no-op, and add a
short JSDoc comment above the async close() in DmlExecutor (or the class
containing close()) mirroring that contract so callers and implementors
understand it's intentionally a no-op.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4567446e-2d41-4f68-a850-99c1c1215b77
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/mongo-migration-authoring/plans/data-migrations-plan.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/data-transform-check-unification.mdis excluded by!projects/**
📒 Files selected for processing (17)
packages/1-framework/3-tooling/migration/src/migration-ts.tspackages/2-mongo-family/4-query/query-ast/src/exports/control.tspackages/2-mongo-family/4-query/query-ast/src/migration-operation-types.tspackages/2-mongo-family/9-family/src/core/mongo-migration.tspackages/3-mongo-target/1-mongo-target/package.jsonpackages/3-mongo-target/1-mongo-target/src/core/migration-factories.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-runner.tspackages/3-mongo-target/1-mongo-target/src/exports/migration.tspackages/3-mongo-target/1-mongo-target/test/data-transform.test.tspackages/3-mongo-target/1-mongo-target/test/migration-e2e.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.tspackages/3-mongo-target/2-mongo-adapter/src/core/runner-deps.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/retail-store/migrations/20260416_backfill-product-status/migration.ts (1)
11-21: Consider removing the long explanatory block comment.This block can be trimmed since the implementation is already clear from the code.
As per coding guidelines, "Do not add comments if avoidable; prefer code which expresses its intent."♻️ Optional cleanup
-/** - * Backfills a `status` field on all products that don't already have one. - * - * - **check.source**: an aggregate query that finds products missing `status`, - * limited to 1 result (we only need to know if any exist). - * - **check.expect** (default `'exists'`): the transform runs when the source - * query finds at least one match. On retry, the postcheck (auto-inverted to - * `'notExists'`) skips re-execution if all products already have `status`. - * - **run**: an updateMany that sets `status: 'active'` on every product - * where the field is missing. - */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/retail-store/migrations/20260416_backfill-product-status/migration.ts` around lines 11 - 21, Remove or trim the long explanatory block comment at the top of the migration to avoid redundant documentation: delete the multi-line description that explains check.source, check.expect and run (or replace it with a single-line summary like "Backfills missing product.status to 'active'"), leaving the actual migration code (the check and run/updateMany logic that sets status) intact; reference symbols to find the code: look for the block immediately above the migration's check.source, check.expect and run/updateMany that mentions the status field and remove or shorten it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@examples/retail-store/migrations/20260416_backfill-product-status/migration.ts`:
- Around line 11-21: Remove or trim the long explanatory block comment at the
top of the migration to avoid redundant documentation: delete the multi-line
description that explains check.source, check.expect and run (or replace it with
a single-line summary like "Backfills missing product.status to 'active'"),
leaving the actual migration code (the check and run/updateMany logic that sets
status) intact; reference symbols to find the code: look for the block
immediately above the migration's check.source, check.expect and run/updateMany
that mentions the status field and remove or shorten it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 3f7d06a3-0a00-406a-b7fa-017ad4e6ccde
📒 Files selected for processing (3)
examples/retail-store/migrations/20260416_backfill-product-status/migration.jsonexamples/retail-store/migrations/20260416_backfill-product-status/migration.tsexamples/retail-store/migrations/20260416_backfill-product-status/ops.json
✅ Files skipped from review due to trivial changes (1)
- examples/retail-store/migrations/20260416_backfill-product-status/migration.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/integration/test/mongo/migration-authoring-e2e.test.ts (1)
58-76: 🛠️ Refactor suggestion | 🟠 Major
runOpsaccepts data ops, but policy currently blocksoperationClass: 'data'.Line 58 widens input to
AnyMongoMigrationOperation[], butALL_POLICYexcludes'data'. This makes the helper accept inputs it may reject at runtime.Proposed fix
const ALL_POLICY = { - allowedOperationClasses: ['additive', 'widening', 'destructive'] as const, + allowedOperationClasses: ['additive', 'widening', 'destructive', 'data'] as const, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/mongo/migration-authoring-e2e.test.ts` around lines 58 - 76, runOps currently types its ops as AnyMongoMigrationOperation[] but calls MongoMigrationRunner.execute with ALL_POLICY which forbids operationClass:'data', so either narrow the runOps parameter to exclude data operations or change the policy passed to the runner to one that permits data ops. Concretely, update the runOps signature (the function named runOps) to a type excluding { operationClass: 'data' } if the helper should only accept schema/index ops, or replace ALL_POLICY in the call to runner.execute with a policy constant that allows data operations (or compose a new policy that includes data) so the declared input type matches runtime validation.
🧹 Nitpick comments (2)
packages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.ts (2)
48-145: Group operation tests into nested contexts for clearer BDD flow.Consider nesting by command family (e.g.,
describe('insert commands'),describe('update commands'),describe('delete commands')) so preconditions and behavior are easier to scan and maintain.As per coding guidelines: "Group test assertions by the conditions under which they're true using nested
describe()blocks" and "Write tests in BDD-style with readable, grouped context."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.ts` around lines 48 - 145, Tests are flat and should be reorganized into BDD-style nested describe blocks by command family (e.g., describe('insert commands'), describe('update commands'), describe('delete commands'), describe('findOneAnd*'), describe('aggregate')), moving the related it(...) cases (those that instantiate MigrationMongoDriver and use InsertOneWireCommand, InsertManyWireCommand, UpdateOneWireCommand, UpdateManyWireCommand, DeleteOneWireCommand, DeleteManyWireCommand, FindOneAndUpdateWireCommand, FindOneAndDeleteWireCommand, AggregateWireCommand) into their respective nested describes; consolidate shared setup (creating the 'items' collection, inserting seed docs, and creating new MigrationMongoDriver(db)) into beforeEach hooks inside each nested describe so preconditions are clear and duplicated code is reduced, and keep the existing assertions inside the it blocks unchanged.
54-55: Prefer object matchers over repeatedtoHavePropertyassertions.For result payload checks,
toMatchObject(ortoEqualwhere exact) aligns better with repo test conventions and keeps assertions more uniform.Example refactor pattern
- expect(results[0]).toHaveProperty('insertedCount', 2); + expect(results[0]).toMatchObject({ insertedCount: 2 }); - expect(results[0]).toHaveProperty('modifiedCount', 1); + expect(results[0]).toMatchObject({ modifiedCount: 1 }); - expect(results[0]).toHaveProperty('name', 'charlie'); + expect(results[0]).toMatchObject({ name: 'charlie' });As per coding guidelines: "Prefer object comparison using
toEqualortoMatchObjectfor assertions on objects, rather than property-by-property checks withtoHavePropertyor individual property assertions."Also applies to: 66-67, 75-76, 84-85, 93-94, 102-103, 116-118, 126-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.ts` around lines 54 - 55, Replace repeated property assertions on the test result arrays with object matchers: instead of asserting expect(results).toHaveLength(1); expect(results[0]).toHaveProperty('insertedId'); (and the other similar pairs), assert the array length and use toMatchObject/toEqual on results[0] to check the expected shape (e.g., expect(results).toHaveLength(1); expect(results[0]).toMatchObject({ insertedId: expect.anything() }) or a more exact object when appropriate). Update the pairs at the other occurrences referenced (the blocks around the current checks) so each test uses a single object matcher for result objects rather than multiple toHaveProperty calls, keeping the variable name results and the same expectations for insertedId or other properties.
🤖 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/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts`:
- Around line 99-110: The data-transform path incorrectly increments
operationsExecuted even when executeDataTransform short-circuits for
idempotent/skipped ops; update executeDataTransform (in mongo-runner.ts) to
return a shape like { executed: boolean; failure?: MigrationRunnerResult }
(instead of possibly undefined), ensure it returns executed: false for early
postcheck-satisfied exits and for precheck failures (including failure payload),
and then change the caller block (where operation.operationClass === 'data') to
inspect the returned .executed flag: only increment operationsExecuted when
executed === true, and propagate/return any returned .failure immediately as
before.
---
Outside diff comments:
In `@test/integration/test/mongo/migration-authoring-e2e.test.ts`:
- Around line 58-76: runOps currently types its ops as
AnyMongoMigrationOperation[] but calls MongoMigrationRunner.execute with
ALL_POLICY which forbids operationClass:'data', so either narrow the runOps
parameter to exclude data operations or change the policy passed to the runner
to one that permits data ops. Concretely, update the runOps signature (the
function named runOps) to a type excluding { operationClass: 'data' } if the
helper should only accept schema/index ops, or replace ALL_POLICY in the call to
runner.execute with a policy constant that allows data operations (or compose a
new policy that includes data) so the declared input type matches runtime
validation.
---
Nitpick comments:
In `@packages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.ts`:
- Around line 48-145: Tests are flat and should be reorganized into BDD-style
nested describe blocks by command family (e.g., describe('insert commands'),
describe('update commands'), describe('delete commands'),
describe('findOneAnd*'), describe('aggregate')), moving the related it(...)
cases (those that instantiate MigrationMongoDriver and use InsertOneWireCommand,
InsertManyWireCommand, UpdateOneWireCommand, UpdateManyWireCommand,
DeleteOneWireCommand, DeleteManyWireCommand, FindOneAndUpdateWireCommand,
FindOneAndDeleteWireCommand, AggregateWireCommand) into their respective nested
describes; consolidate shared setup (creating the 'items' collection, inserting
seed docs, and creating new MigrationMongoDriver(db)) into beforeEach hooks
inside each nested describe so preconditions are clear and duplicated code is
reduced, and keep the existing assertions inside the it blocks unchanged.
- Around line 54-55: Replace repeated property assertions on the test result
arrays with object matchers: instead of asserting
expect(results).toHaveLength(1);
expect(results[0]).toHaveProperty('insertedId'); (and the other similar pairs),
assert the array length and use toMatchObject/toEqual on results[0] to check the
expected shape (e.g., expect(results).toHaveLength(1);
expect(results[0]).toMatchObject({ insertedId: expect.anything() }) or a more
exact object when appropriate). Update the pairs at the other occurrences
referenced (the blocks around the current checks) so each test uses a single
object matcher for result objects rather than multiple toHaveProperty calls,
keeping the variable name results and the same expectations for insertedId or
other properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9398642b-813b-4a25-ab39-4589ca426c3b
📒 Files selected for processing (6)
examples/retail-store/migrations/20260416_backfill-product-status/migration.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-runner.tspackages/3-mongo-target/1-mongo-target/test/data-transform.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.tspackages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.tstest/integration/test/mongo/migration-authoring-e2e.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/retail-store/migrations/20260416_backfill-product-status/migration.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts (1)
165-167:⚠️ Potential issue | 🟠 MajorNarrow
destinationContractbefore readingprofileHash.Line 166 uses a blind cast and Line 167 immediately dereferences it. Since operations execute before this point (lines 99–163), if a caller passes an invalid value for
destinationContract, the code throws after operations have already executed, leaving partial changes unrecoverable.Suggested fix
+function hasProfileHash(value: unknown): value is { readonly profileHash: string } { + return ( + typeof value === 'object' && + value !== null && + Object.hasOwn(value, 'profileHash') && + typeof Reflect.get(value, 'profileHash') === 'string' + ); +} + const destination = options.plan.destination; - const contract = options.destinationContract as { profileHash?: string }; - const profileHash = contract.profileHash ?? destination.storageHash; + const profileHash = hasProfileHash(options.destinationContract) + ? options.destinationContract.profileHash + : destination.storageHash;Per coding guidelines: "No blind casts in production code:
as unknown as Xis forbidden outside tests. Prefer type predicates or fix the type surface."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts` around lines 165 - 167, The code blindly casts options.destinationContract and immediately reads profileHash; add a runtime type-narrowing/validation step for options.destinationContract before any operations run (before using options.plan.destination or performing side effects) — implement a type guard like isDestinationContract(obj) that checks for the expected shape (e.g., object with optional profileHash string), call it early and either narrow the variable or throw a clear error, then compute const profileHash = contract.profileHash ?? destination.storageHash only after validation; update references to options.destinationContract/contract and ensure no operations execute prior to this check so partial changes cannot occur.packages/1-framework/3-tooling/migration/src/attestation.ts (1)
17-31:⚠️ Potential issue | 🟠 Major
verifyMigration()no longer attests the full manifest.Stripping
fromContract,toContract, andhintsmeans those fields can be edited after attestation andverifyMigration()will still returnok: true. All three are consumed in non-test code:
fromContractandtoContractare used in migration planning and operation resolution (descriptor-planner.ts, planner-strategies.ts, operation-resolver.ts)hintsis displayed in CLI diagnostics (migration-status.ts)Since the migration ID no longer covers data the toolchain trusts, either keep them in the hash or add a separate integrity check for this authorship metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/migration/src/attestation.ts` around lines 17 - 31, computeMigrationId currently strips fromContract, toContract, and hints from the canonicalized manifest which allows those fields to be modified after attestation; update computeMigrationId (and any caller like verifyMigration that relies on it) so the integrity covers authorship metadata: stop destructuring out _fromContract, _toContract, and _hints (only strip migrationId and signature) so canonicalManifest includes fromContract/toContract/hints, then re-run the canonicalizeJson/sha256Hex steps to produce the migration ID; alternatively, if you intentionally want separate attestations, add a companion integrity check in verifyMigration that computes and verifies a hash over the authorship metadata (fromContract/toContract/hints) against a stored signature, but do not leave those fields unverified.packages/1-framework/3-tooling/cli/test/control-api/db-update.test.ts (1)
1-598: 🛠️ Refactor suggestion | 🟠 MajorSplit this test file; it exceeds the 500-line limit.
This file is currently over the readability threshold and mixes multiple concerns. Please split by concern (e.g., basic flow, destructive gate, progress events) into independently runnable test files.
As per coding guidelines,
**/*.test.ts: "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/1-framework/3-tooling/cli/test/control-api/db-update.test.ts` around lines 1 - 598, The test file is too large—split it into multiple smaller test files grouped by concern (e.g., db-update.basic.test.ts for core flows that exercise executeDbUpdate, db-update.destructive.test.ts for the "destructive changes gate" describe block and its helpers, and db-update.progress.test.ts for the "progress events" tests); move or duplicate the shared helpers (createMockDriver, createMockFamilyInstance, createMockMigrations, dummyContract) into a small test helper module or copy them at the top of each new file so each file is independently runnable; ensure each new file imports executeDbUpdate and ControlProgressEvent and retains the same test cases and assertions (names and function references like executeDbUpdate, createMockMigrations, createDestructiveMigrations, and the describe blocks) and that test-level mocks (vi.fn) remain scoped to their file; finally run tests to confirm each new file is under 500 lines and update any import paths accordingly.packages/1-framework/3-tooling/cli/src/commands/migration-new.ts (1)
231-244:⚠️ Potential issue | 🟠 MajorUse
instanceof CliStructuredErrorfor consistent error detection.The code uses a property-based check (
'code' in error && typeof ...) to detectCliStructuredError, but other CLI commands (migration-ref.ts, db-verify.ts, db-sign.ts, contract-emit.ts) consistently useinstanceof CliStructuredError. SinceCliStructuredErroris already imported, replace the property check withinstanceofto align with the established pattern and improve type safety.🤖 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/migration-new.ts` around lines 231 - 244, The catch block is using a property-based check to detect CliStructuredError; replace the "'code' in error && typeof ... === 'string'" check with "error instanceof CliStructuredError" so the branch returns notOk(error as CliStructuredError) consistently; update the catch to use instanceof CliStructuredError (keeping the existing notOk(...) and fallback to notOk(errorUnexpected(...)) behavior) referencing the CliStructuredError class and the notOk and errorUnexpected calls in this catch.
🧹 Nitpick comments (12)
scripts/lint-framework-target-imports.mjs (1)
30-45: Externalize lint policy into repository configThe forbidden prefix and exception lists are embedded in executable logic. Move these policy knobs to a declarative config file and have this script consume them, so rule updates don’t require script edits.
As per coding guidelines, "Prefer declarative data structures (JSON/config files) over hardcoded conditionals for validation rules, constraints, and configuration logic in dependency-cruiser and build configurations" and "Define validation rules and exceptions in declarative format within configuration files (e.g., planeRules, layerOrder) rather than as hardcoded conditionals in generator logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-framework-target-imports.mjs` around lines 30 - 45, The hardcoded policy values (FORBIDDEN_SUBSTRING, FRAMEWORK_ROOT, INCLUDED_EXTENSIONS, EXCLUDED_DIRECTORIES) should be moved to a declarative config file (e.g., lint-framework-targets.config.json) and the script should read/validate those values at startup; update scripts/lint-framework-target-imports.mjs to load the JSON config, validate required keys (forbiddenSubstring, frameworkRoot, includedExtensions, excludedDirectories), use those values in place of the constants, and keep sensible defaults/fallbacks when the config is missing or malformed to preserve current behavior during rollout.packages/3-mongo-target/1-mongo-target/test/data-transform.test.ts (1)
60-74: Consolidate related field assertions withtoMatchObjectThis block checks one object shape via many scalar assertions; prefer a single object matcher for readability and failure grouping.
♻️ Suggested refactor
it('produces correct operation structure with check', () => { const op = dataTransform('backfill-status', { check: { source: () => makeCheckPlan(), }, run: () => makeRunPlan(), }); - expect(op.id).toBe('data_transform.backfill-status'); - expect(op.label).toBe('Data transform: backfill-status'); - expect(op.operationClass).toBe('data'); - expect(op.name).toBe('backfill-status'); - expect(op.precheck).toHaveLength(1); - expect(op.postcheck).toHaveLength(1); + expect(op).toMatchObject({ + id: 'data_transform.backfill-status', + label: 'Data transform: backfill-status', + operationClass: 'data', + name: 'backfill-status', + precheck: [expect.any(Object)], + postcheck: [expect.any(Object)], + }); });As per coding guidelines:
**/*.test.ts: Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/test/data-transform.test.ts` around lines 60 - 74, The test uses multiple scalar expect() assertions for related fields on the operation object; replace those with a single expect(op).toMatchObject({ id: 'data_transform.backfill-status', label: 'Data transform: backfill-status', operationClass: 'data', name: 'backfill-status' }) to consolidate related assertions for the result of dataTransform, and retain the separate length checks for op.precheck and op.postcheck (expect(op.precheck).toHaveLength(1) and expect(op.postcheck).toHaveLength(1)); update the test that constructs op (via dataTransform('backfill-status', ...)) accordingly.packages/1-framework/1-core/errors/test/migration.test.ts (1)
24-39: PrefertoThrowassertions over manual try/catch in this test.This block can be expressed with
expect(() => placeholder('foo')).toThrow(...)to match the repo’s test error-assertion pattern.♻️ Proposed refactor
it('placeholder throws a CliStructuredError that names the slot', () => { - let thrown: unknown; - try { - placeholder('foo'); - } catch (error) { - thrown = error; - } - - expect(CliStructuredError.is(thrown)).toBe(true); - expect(thrown).toMatchObject({ - code: '2001', - domain: 'MIG', - meta: { slot: 'foo' }, - }); - expect((thrown as CliStructuredError).toEnvelope().code).toBe('PN-MIG-2001'); + const invoke = () => placeholder('foo'); + expect(invoke).toThrow(CliStructuredError); + expect(invoke).toThrow('foo'); });As per coding guidelines: "Use
expect(() => ...).toThrow(...)to assert errors instead of manual try/catch blocks in tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/1-core/errors/test/migration.test.ts` around lines 24 - 39, Replace the manual try/catch in the test with Jest's expect(...).toThrow pattern: assert the call with expect(() => placeholder('foo')).toThrow(); then separately assert the error shape by using expect(() => placeholder('foo')).toThrowErrorMatchingInlineSnapshot or another toThrow/toThrowError matcher that verifies the CliStructuredError properties (CliStructuredError.is, code/domain/meta and toEnvelope()) so you remove the manual try/catch while still validating placeholder, CliStructuredError, and the envelope code.examples/retail-store/test/manual-migration.test.ts (1)
52-58: Prefer a single object matcher for related operation assertions.This test checks related operation fields with separate assertions; consolidating to
toMatchObjectmakes failures easier to scan and aligns with repo test style.♻️ Suggested assertion shape
- expect(ops).toHaveLength(2); - expect(ops[0]!.id).toBe('collection.products.setValidation'); - expect(ops[1]!.id).toContain('index.products.create'); + expect(ops).toHaveLength(2); + expect(ops).toMatchObject([ + { id: 'collection.products.setValidation' }, + expect.objectContaining({ id: expect.stringContaining('index.products.create') }), + ]);As per coding guidelines,
**/*.test.ts: "Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/retail-store/test/manual-migration.test.ts` around lines 52 - 58, The test currently asserts related fields of the operations array with multiple expect() calls; replace those with a single object matcher using toMatchObject on the ops array from the AddProductValidation instance. Locate the instantiation of AddProductValidation and the ops variable, then assert ops via toMatchObject with an array shape covering both entries (first entry id equals 'collection.products.setValidation' and second entry id matches a stringContaining 'index.products.create') so failures are reported as a single consolidated diff and follow the repo test style.packages/3-mongo-target/1-mongo-target/src/core/planner-produced-migration.ts (1)
45-52: Consider usingifDefined()for conditional object spreads.Per coding guidelines, prefer
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns.Use ifDefined() utility
+import { ifDefined } from '@prisma-next/utils/defined'; + // ... renderTypeScript(): string { return renderCallsToTypeScript(this.calls, { from: this.meta.from, to: this.meta.to, - ...(this.meta.kind !== undefined ? { kind: this.meta.kind } : {}), - ...(this.meta.labels !== undefined ? { labels: this.meta.labels } : {}), + ...ifDefined('kind', this.meta.kind), + ...ifDefined('labels', this.meta.labels), }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/src/core/planner-produced-migration.ts` around lines 45 - 52, The renderTypeScript method uses inline conditional object spreads to pass meta fields to renderCallsToTypeScript; replace those inline spreads with the ifDefined() helper from `@prisma-next/utils/defined` so optional properties are only included when defined. In renderTypeScript(), import ifDefined and call renderCallsToTypeScript(this.calls, { from: this.meta.from, to: this.meta.to, ...ifDefined('kind', this.meta.kind), ...ifDefined('labels', this.meta.labels) }) (keeping the same property names) so that kind and labels are conditionally added via ifDefined rather than inline ternary spreads.packages/2-mongo-family/9-family/src/core/mongo-emit.ts (1)
94-98: Unreachable branch indescribeValue.Line 96 returns
'an array'for arrays, but this function is only called from line 86 when!Array.isArray(operations)is true, making this branch unreachable. Consider removing it for clarity.Simplify describeValue by removing unreachable branch
function describeValue(value: unknown): string { if (value === null) return 'null'; - if (Array.isArray(value)) return 'an array'; return `a value of type ${typeof value}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/src/core/mongo-emit.ts` around lines 94 - 98, The describeValue function contains an unreachable branch returning 'an array' (Array.isArray(value)) because callers (e.g., the place that checks !Array.isArray(operations) before calling describeValue) already guard against arrays; remove the array branch and simplify describeValue to only handle null and otherwise return `a value of type ${typeof value}` (update function describeValue accordingly so it no longer checks Array.isArray).packages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.ts (1)
72-82: Preferexpect().toThrow()over manual try/catch for error assertions.Per coding guidelines, use
expect(() => ...).toThrow(...)instead of manual try/catch blocks in tests.Refactor to use expect().toThrow()
- let caught: unknown; - try { - success.plan.renderTypeScript(); - } catch (err) { - caught = err; - } - - expect(caught).toBeInstanceOf(CliStructuredError); - expect((caught as CliStructuredError).code).toBe('2010'); - expect((caught as CliStructuredError).meta).toMatchObject({ targetId: 'postgres' }); + expect(() => success.plan.renderTypeScript()).toThrow(CliStructuredError); + + try { + success.plan.renderTypeScript(); + } catch (err) { + expect(err).toBeInstanceOf(CliStructuredError); + const cliErr = err as CliStructuredError; + expect(cliErr.code).toBe('2010'); + expect(cliErr.meta).toMatchObject({ targetId: 'postgres' }); + }Alternatively, if you need to inspect the error properties, consider using a custom matcher or extracting the error in a controlled way:
expect(() => success.plan.renderTypeScript()).toThrowError( expect.objectContaining({ code: '2010', meta: expect.objectContaining({ targetId: 'postgres' }), }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.ts` around lines 72 - 82, Replace the manual try/catch around success.plan.renderTypeScript() with Jest's expect-toThrow style: call expect(() => success.plan.renderTypeScript()).toThrowError(...) and assert the thrown error is a CliStructuredError by using expect.objectContaining to check { code: '2010', meta: expect.objectContaining({ targetId: 'postgres' }) }; this removes the caught variable and aligns the test with Jest error-assertion patterns while still validating the CliStructuredError properties.packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
291-294: Avoid blind-casting planner conflicts into CLI conflicts.Both
as readonly CliErrorConflict[]casts erase the contract between the planner and the CLI layer. If either planner starts returning a new conflict variant, this path will still compile and can emit malformed conflict payloads. Please share the conflict type across the boundary or map the planner result explicitly here.As per coding guidelines, "No blind casts in production code:
as unknown as Xis forbidden outside tests. Prefer type predicates or fix the type surface."Also applies to: 328-330
🤖 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/migration-plan.ts` around lines 291 - 294, The code presently blind-casts descriptorResult.conflicts to readonly CliErrorConflict[] when calling errorMigrationPlanningFailed (and similarly at the other occurrence), which can hide incompatible planner variants; instead either export/share the planner conflict type so the CLI can accept it safely or explicitly map/validate each planner conflict to a CliErrorConflict before passing it into errorMigrationPlanningFailed (use a small mapping function that converts planner conflict discriminants into CliErrorConflict instances and throws/returns a safe default for unknown kinds). Locate the usages around descriptorResult.conflicts in migration-plan.ts and replace the "as readonly CliErrorConflict[]" casts with an explicit mapping/validation step (or use a shared type alias) so the payload shape is guaranteed at compile time and runtime.packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts (1)
133-245: Use rejection assertions instead of manualtry/catchin these error-path tests.These blocks are only checking that
executeCommand(...)rejects, soawait expect(...).rejectswill read cleaner and remove the repeated bookkeeping. You can still keep the JSON-envelope assertions immediately after that.♻️ Representative cleanup
- let thrown = false; - try { - await executeCommand(command, ['--dir', 'migrations/20260101_test', '--json']); - } catch { - thrown = true; - } - - expect(thrown).toBe(true); + await expect( + executeCommand(command, ['--dir', 'migrations/20260101_test', '--json']), + ).rejects.toBeDefined();As per coding guidelines, "Use
expect(() => ...).toThrow(...)to assert errors instead of manual try/catch blocks in tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts` around lines 133 - 245, Several tests (using createMigrationEmitCommand and executeCommand) use manual try/catch to assert rejection; replace each try/catch block with an explicit async rejection assertion such as await expect(executeCommand(command, ['--dir', 'migrations/20260101_test', '--json'])).rejects.toBeDefined() (or .rejects.toThrow() if the rejection is an Error) and leave the subsequent consoleOutput/JSON envelope assertions unchanged; update the tests that reference mocks.emitMigrationMock and mockMigrationCapableConfig accordingly to remove the thrown bookkeeping and rely on the expect(...).rejects pattern for clarity and consistency.examples/retail-store/migrations/20260416_backfill-product-status/migration.ts (1)
1-5: Use the target-owned Mongo migration entrypoint in this example.This new example is demonstrating the current Mongo migration authoring surface, but it still pulls
Migrationfrom@prisma-next/family-mongo/migration. Switching both symbols to@prisma-next/target-mongo/migrationkeeps the example aligned with generated migrations and avoids preserving the old path in user-facing code.♻️ Proposed fix
-import { Migration } from '@prisma-next/family-mongo/migration'; +import { Migration, dataTransform } from '@prisma-next/target-mongo/migration'; import { validateMongoContract } from '@prisma-next/mongo-contract'; import { mongoPipeline } from '@prisma-next/mongo-pipeline-builder'; import { MongoExistsExpr, RawUpdateManyCommand } from '@prisma-next/mongo-query-ast/execution'; -import { dataTransform } from '@prisma-next/target-mongo/migration';As per coding guidelines, "Do not add backward-compatibility shims; update all references immediately to use new approaches instead of maintaining parallel implementations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/retail-store/migrations/20260416_backfill-product-status/migration.ts` around lines 1 - 5, The example imports the Migration symbol from the deprecated '@prisma-next/family-mongo/migration'; update the import so the example uses the target-owned migration entrypoint by importing Migration from '@prisma-next/target-mongo/migration' (and likewise replace any other migration-related symbols that currently come from '@prisma-next/family-mongo/migration' to the same '@prisma-next/target-mongo/migration' module, e.g., the Migration type/constructor used in this file) while leaving unrelated imports (validateMongoContract, mongoPipeline, MongoExistsExpr, RawUpdateManyCommand, dataTransform) as-is.packages/1-framework/3-tooling/cli/src/lib/migration-emit.ts (1)
105-111: Consider adding runtime validation for evaluated descriptors.The cast at line 107 assumes
evaluateMigrationTs(dir)returns a validOperationDescriptor[]. If the evaluated module exports unexpected data, this could lead to silent failures downstream.💡 Proposed defensive check
const pkg = await readMigrationPackage(dir); const descriptors = await evaluateMigrationTs(dir); + if (!Array.isArray(descriptors)) { + throw errorMigrationFileMissing(dir); // or a more specific error + } const operations = migrations.resolveDescriptors(descriptors as OperationDescriptor[], {Alternatively, if
evaluateMigrationTshas its own validation, this cast may be acceptable. The cast is used to bridge the framework-level generic return to the target-specific type.🤖 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/lib/migration-emit.ts` around lines 105 - 111, The code currently casts the result of evaluateMigrationTs(dir) to OperationDescriptor[] before passing to migrations.resolveDescriptors, which can hide malformed exports; add a runtime validation step after calling evaluateMigrationTs (the value assigned to descriptors) that verifies it's an array and that each item has the required OperationDescriptor shape (e.g., required fields used by migrations.resolveDescriptors and pkg.manifest.fromContract / toContract), and if validation fails throw or log a descriptive error referencing evaluateMigrationTs and OperationDescriptor so the failure is explicit; keep using readMigrationPackage and migrations.resolveDescriptors but guard the input to resolveDescriptors with this check to avoid silent downstream failures.packages/2-mongo-family/9-family/test/mongo-emit.test.ts (1)
150-166: Useexpect().rejects.toThrow()for async error assertions.The coding guidelines prefer
expect().toThrow()over manual try/catch blocks. For async functions, use therejectsmodifier.♻️ Proposed refactor
- it('propagates a structured CliStructuredError with code PN-MIG-2001 and the slot in meta', async () => { - await writeFile(join(pkgDir, 'migration.ts'), placeholderMigration); - - let thrown: unknown; - try { - await mongoEmit({ dir: pkgDir, frameworkComponents: [] }); - } catch (error) { - thrown = error; - } - - expect(CliStructuredError.is(thrown)).toBe(true); - expect(thrown).toMatchObject({ - code: '2001', - domain: 'MIG', - meta: { slot: 'backfill-product-status:run' }, - }); - }); + it('propagates a structured CliStructuredError with code PN-MIG-2001 and the slot in meta', async () => { + await writeFile(join(pkgDir, 'migration.ts'), placeholderMigration); + + await expect(mongoEmit({ dir: pkgDir, frameworkComponents: [] })).rejects.toSatisfy( + (error) => + CliStructuredError.is(error) && + error.code === '2001' && + error.domain === 'MIG' && + error.meta?.slot === 'backfill-product-status:run', + ); + });Alternatively, if
toSatisfyis not available, you can use a helper that extracts the error and asserts on it, but the key is to avoid the try/catch pattern when possible. The same pattern applies to the other error tests at lines 170-184, 188-225, and 228-260.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/test/mongo-emit.test.ts` around lines 150 - 166, Replace the manual try/catch in the test with Jest's async rejection assertions: call await expect(mongoEmit({ dir: pkgDir, frameworkComponents: [] })).rejects.toMatchObject({ code: '2001', domain: 'MIG', meta: { slot: 'backfill-product-status:run' } }) and also assert CliStructuredError.is via awaiting the rejected value if needed (e.g., await expect(...).rejects.toSatisfy(err => CliStructuredError.is(err))). Apply the same pattern for the other error tests that currently use try/catch (the tests that reference mongoEmit and validate structured error shapes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@examples/retail-store/migrations/20260416_backfill-product-status/migration.ts`:
- Around line 13-18: The describe() override currently hardcodes to
'sha256:a1b2...' which doesn't match the actual bundled contract hash; update
the to field so it uses the real bundled contract storage hash (e.g. replace the
literal in describe().to with the contract's storage hash reference such as
contract.storage.storageHash or the variable that holds the bundled contract
hash) so migration.describe().to matches the bundled contract and emitted
migration.json uses the correct destination hash.
In `@packages/3-mongo-target/1-mongo-target/test/data-transform.test.ts`:
- Around line 188-225: Replace the manual try/catch blocks in both tests with
Vitest's expect(() => ...).toThrow(CliStructuredError) to assert the error type,
then perform a separate capture to inspect properties; specifically, for the
test cases invoking dataTransform('backfill-product-status', { check: () =>
placeholder(...), ... }) and dataTransform('backfill-product-status', { run: ()
=> placeholder(...) }), call expect(() =>
dataTransform(...)).toThrow(CliStructuredError) and then capture the thrown
error (e.g., via a short try/catch around the same invocation) and assert
CliStructuredError.is(thrown) and the thrown object matches the expected
code/domain/meta values for placeholder slot, keeping references to
dataTransform, placeholder, and CliStructuredError to locate the changes.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.ts`:
- Around line 16-21: The file currently exports the concrete class
MigrationMongoDriver which should be hidden; change the exported API to a
factory that returns the MongoDriver interface: make the class non-exported
(rename to migrationMongoDriver or keep MigrationMongoDriver but remove the
export), add and export a factory function like createMigrationMongoDriver(db:
Db): MongoDriver that instantiates the private class and returns it, and keep
the MongoDriver interface exported for typing; ensure all callers use the new
createMigrationMongoDriver factory instead of constructing the class directly.
In `@packages/3-targets/3-targets/postgres/src/exports/control.ts`:
- Around line 137-140: Remove the blind `as never` cast on the call to
inner.plan and fix the type mismatch by making the planner options structurally
correct: either extend the framework-facing MigrationPlanner options to include
readonly schemaName?: string, or build a properly typed
SqlMigrationPlannerPlanOptions object before calling inner.plan (map the
existing options plus schemaName: options.schemaName ?? 'public') and pass that
to inner.plan; update the call site where inner.plan(options as never) is used
so it passes the correctly typed options object instead of casting.
In `@scripts/lint-framework-target-imports.mjs`:
- Around line 67-74: The current check around FORBIDDEN_SUBSTRING on the lines
array only ignores lines starting with '*' or '//' and therefore still flags
text that appears inside /* ... */ block comments or after trailing inline
comments; update the logic in the loop that examines const line = lines[i] (and
uses const trimmed = line.trimStart()) to first strip or skip block-comment
content (introduce and use an inBlockComment boolean to ignore lines between /*
and */ or remove /*...*/ fragments on the same line) and also remove trailing
inline comments (e.g., drop the portion after '//' when it's not inside a string
or comment you already removed) before checking for FORBIDDEN_SUBSTRING so
violations.push(...) is only executed for real code, not documentation or inline
comments.
---
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-new.ts`:
- Around line 231-244: The catch block is using a property-based check to detect
CliStructuredError; replace the "'code' in error && typeof ... === 'string'"
check with "error instanceof CliStructuredError" so the branch returns
notOk(error as CliStructuredError) consistently; update the catch to use
instanceof CliStructuredError (keeping the existing notOk(...) and fallback to
notOk(errorUnexpected(...)) behavior) referencing the CliStructuredError class
and the notOk and errorUnexpected calls in this catch.
In `@packages/1-framework/3-tooling/cli/test/control-api/db-update.test.ts`:
- Around line 1-598: The test file is too large—split it into multiple smaller
test files grouped by concern (e.g., db-update.basic.test.ts for core flows that
exercise executeDbUpdate, db-update.destructive.test.ts for the "destructive
changes gate" describe block and its helpers, and db-update.progress.test.ts for
the "progress events" tests); move or duplicate the shared helpers
(createMockDriver, createMockFamilyInstance, createMockMigrations,
dummyContract) into a small test helper module or copy them at the top of each
new file so each file is independently runnable; ensure each new file imports
executeDbUpdate and ControlProgressEvent and retains the same test cases and
assertions (names and function references like executeDbUpdate,
createMockMigrations, createDestructiveMigrations, and the describe blocks) and
that test-level mocks (vi.fn) remain scoped to their file; finally run tests to
confirm each new file is under 500 lines and update any import paths
accordingly.
In `@packages/1-framework/3-tooling/migration/src/attestation.ts`:
- Around line 17-31: computeMigrationId currently strips fromContract,
toContract, and hints from the canonicalized manifest which allows those fields
to be modified after attestation; update computeMigrationId (and any caller like
verifyMigration that relies on it) so the integrity covers authorship metadata:
stop destructuring out _fromContract, _toContract, and _hints (only strip
migrationId and signature) so canonicalManifest includes
fromContract/toContract/hints, then re-run the canonicalizeJson/sha256Hex steps
to produce the migration ID; alternatively, if you intentionally want separate
attestations, add a companion integrity check in verifyMigration that computes
and verifies a hash over the authorship metadata (fromContract/toContract/hints)
against a stored signature, but do not leave those fields unverified.
In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts`:
- Around line 165-167: The code blindly casts options.destinationContract and
immediately reads profileHash; add a runtime type-narrowing/validation step for
options.destinationContract before any operations run (before using
options.plan.destination or performing side effects) — implement a type guard
like isDestinationContract(obj) that checks for the expected shape (e.g., object
with optional profileHash string), call it early and either narrow the variable
or throw a clear error, then compute const profileHash = contract.profileHash ??
destination.storageHash only after validation; update references to
options.destinationContract/contract and ensure no operations execute prior to
this check so partial changes cannot occur.
---
Nitpick comments:
In
`@examples/retail-store/migrations/20260416_backfill-product-status/migration.ts`:
- Around line 1-5: The example imports the Migration symbol from the deprecated
'@prisma-next/family-mongo/migration'; update the import so the example uses the
target-owned migration entrypoint by importing Migration from
'@prisma-next/target-mongo/migration' (and likewise replace any other
migration-related symbols that currently come from
'@prisma-next/family-mongo/migration' to the same
'@prisma-next/target-mongo/migration' module, e.g., the Migration
type/constructor used in this file) while leaving unrelated imports
(validateMongoContract, mongoPipeline, MongoExistsExpr, RawUpdateManyCommand,
dataTransform) as-is.
In `@examples/retail-store/test/manual-migration.test.ts`:
- Around line 52-58: The test currently asserts related fields of the operations
array with multiple expect() calls; replace those with a single object matcher
using toMatchObject on the ops array from the AddProductValidation instance.
Locate the instantiation of AddProductValidation and the ops variable, then
assert ops via toMatchObject with an array shape covering both entries (first
entry id equals 'collection.products.setValidation' and second entry id matches
a stringContaining 'index.products.create') so failures are reported as a single
consolidated diff and follow the repo test style.
In `@packages/1-framework/1-core/errors/test/migration.test.ts`:
- Around line 24-39: Replace the manual try/catch in the test with Jest's
expect(...).toThrow pattern: assert the call with expect(() =>
placeholder('foo')).toThrow(); then separately assert the error shape by using
expect(() => placeholder('foo')).toThrowErrorMatchingInlineSnapshot or another
toThrow/toThrowError matcher that verifies the CliStructuredError properties
(CliStructuredError.is, code/domain/meta and toEnvelope()) so you remove the
manual try/catch while still validating placeholder, CliStructuredError, and the
envelope code.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 291-294: The code presently blind-casts descriptorResult.conflicts
to readonly CliErrorConflict[] when calling errorMigrationPlanningFailed (and
similarly at the other occurrence), which can hide incompatible planner
variants; instead either export/share the planner conflict type so the CLI can
accept it safely or explicitly map/validate each planner conflict to a
CliErrorConflict before passing it into errorMigrationPlanningFailed (use a
small mapping function that converts planner conflict discriminants into
CliErrorConflict instances and throws/returns a safe default for unknown kinds).
Locate the usages around descriptorResult.conflicts in migration-plan.ts and
replace the "as readonly CliErrorConflict[]" casts with an explicit
mapping/validation step (or use a shared type alias) so the payload shape is
guaranteed at compile time and runtime.
In `@packages/1-framework/3-tooling/cli/src/lib/migration-emit.ts`:
- Around line 105-111: The code currently casts the result of
evaluateMigrationTs(dir) to OperationDescriptor[] before passing to
migrations.resolveDescriptors, which can hide malformed exports; add a runtime
validation step after calling evaluateMigrationTs (the value assigned to
descriptors) that verifies it's an array and that each item has the required
OperationDescriptor shape (e.g., required fields used by
migrations.resolveDescriptors and pkg.manifest.fromContract / toContract), and
if validation fails throw or log a descriptive error referencing
evaluateMigrationTs and OperationDescriptor so the failure is explicit; keep
using readMigrationPackage and migrations.resolveDescriptors but guard the input
to resolveDescriptors with this check to avoid silent downstream failures.
In `@packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts`:
- Around line 133-245: Several tests (using createMigrationEmitCommand and
executeCommand) use manual try/catch to assert rejection; replace each try/catch
block with an explicit async rejection assertion such as await
expect(executeCommand(command, ['--dir', 'migrations/20260101_test',
'--json'])).rejects.toBeDefined() (or .rejects.toThrow() if the rejection is an
Error) and leave the subsequent consoleOutput/JSON envelope assertions
unchanged; update the tests that reference mocks.emitMigrationMock and
mockMigrationCapableConfig accordingly to remove the thrown bookkeeping and rely
on the expect(...).rejects pattern for clarity and consistency.
In `@packages/2-mongo-family/9-family/src/core/mongo-emit.ts`:
- Around line 94-98: The describeValue function contains an unreachable branch
returning 'an array' (Array.isArray(value)) because callers (e.g., the place
that checks !Array.isArray(operations) before calling describeValue) already
guard against arrays; remove the array branch and simplify describeValue to only
handle null and otherwise return `a value of type ${typeof value}` (update
function describeValue accordingly so it no longer checks Array.isArray).
In `@packages/2-mongo-family/9-family/test/mongo-emit.test.ts`:
- Around line 150-166: Replace the manual try/catch in the test with Jest's
async rejection assertions: call await expect(mongoEmit({ dir: pkgDir,
frameworkComponents: [] })).rejects.toMatchObject({ code: '2001', domain: 'MIG',
meta: { slot: 'backfill-product-status:run' } }) and also assert
CliStructuredError.is via awaiting the rejected value if needed (e.g., await
expect(...).rejects.toSatisfy(err => CliStructuredError.is(err))). Apply the
same pattern for the other error tests that currently use try/catch (the tests
that reference mongoEmit and validate structured error shapes).
In
`@packages/3-mongo-target/1-mongo-target/src/core/planner-produced-migration.ts`:
- Around line 45-52: The renderTypeScript method uses inline conditional object
spreads to pass meta fields to renderCallsToTypeScript; replace those inline
spreads with the ifDefined() helper from `@prisma-next/utils/defined` so optional
properties are only included when defined. In renderTypeScript(), import
ifDefined and call renderCallsToTypeScript(this.calls, { from: this.meta.from,
to: this.meta.to, ...ifDefined('kind', this.meta.kind), ...ifDefined('labels',
this.meta.labels) }) (keeping the same property names) so that kind and labels
are conditionally added via ifDefined rather than inline ternary spreads.
In `@packages/3-mongo-target/1-mongo-target/test/data-transform.test.ts`:
- Around line 60-74: The test uses multiple scalar expect() assertions for
related fields on the operation object; replace those with a single
expect(op).toMatchObject({ id: 'data_transform.backfill-status', label: 'Data
transform: backfill-status', operationClass: 'data', name: 'backfill-status' })
to consolidate related assertions for the result of dataTransform, and retain
the separate length checks for op.precheck and op.postcheck
(expect(op.precheck).toHaveLength(1) and expect(op.postcheck).toHaveLength(1));
update the test that constructs op (via dataTransform('backfill-status', ...))
accordingly.
In
`@packages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.ts`:
- Around line 72-82: Replace the manual try/catch around
success.plan.renderTypeScript() with Jest's expect-toThrow style: call expect(()
=> success.plan.renderTypeScript()).toThrowError(...) and assert the thrown
error is a CliStructuredError by using expect.objectContaining to check { code:
'2010', meta: expect.objectContaining({ targetId: 'postgres' }) }; this removes
the caught variable and aligns the test with Jest error-assertion patterns while
still validating the CliStructuredError properties.
In `@scripts/lint-framework-target-imports.mjs`:
- Around line 30-45: The hardcoded policy values (FORBIDDEN_SUBSTRING,
FRAMEWORK_ROOT, INCLUDED_EXTENSIONS, EXCLUDED_DIRECTORIES) should be moved to a
declarative config file (e.g., lint-framework-targets.config.json) and the
script should read/validate those values at startup; update
scripts/lint-framework-target-imports.mjs to load the JSON config, validate
required keys (forbiddenSubstring, frameworkRoot, includedExtensions,
excludedDirectories), use those values in place of the constants, and keep
sensible defaults/fallbacks when the config is missing or malformed to preserve
current behavior during rollout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a3f62431-5331-4d44-8800-ddf3b17cdca0
⛔ Files ignored due to path filters (8)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/mongo-migration-authoring/specs/class-flow-scaffolding.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/data-migrations.spec.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/data-transform-placeholder.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/migration-emit-unification.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/migration-package-polish.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/planner-dual-output.spec.mdis excluded by!projects/**projects/mongo-migration-authoring/specs/target-owned-migration-scaffolding.mdis excluded by!projects/**
📒 Files selected for processing (92)
examples/mongo-demo/migrations/20260409T1030_migration/migration.jsonexamples/mongo-demo/migrations/20260415_add-posts-author-index/migration.jsonexamples/mongo-demo/migrations/20260415_add-posts-author-index/migration.tsexamples/mongo-demo/test/manual-migration.test.tsexamples/retail-store/migrations/20260415_add-product-validation/migration.tsexamples/retail-store/migrations/20260416_backfill-product-status/contract.d.tsexamples/retail-store/migrations/20260416_backfill-product-status/contract.jsonexamples/retail-store/migrations/20260416_backfill-product-status/migration.tsexamples/retail-store/migrations/20260416_backfill-product-status/ops.jsonexamples/retail-store/test/manual-migration.test.tspackage.jsonpackages/1-framework/1-core/errors/package.jsonpackages/1-framework/1-core/errors/src/control.tspackages/1-framework/1-core/errors/src/exports/migration.tspackages/1-framework/1-core/errors/src/migration.tspackages/1-framework/1-core/errors/test/migration.test.tspackages/1-framework/1-core/errors/tsdown.config.tspackages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/package.jsonpackages/1-framework/3-tooling/cli/src/cli.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.tspackages/1-framework/3-tooling/cli/src/commands/migration-emit.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/commands/migration-verify.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/lib/migration-emit.tspackages/1-framework/3-tooling/cli/src/lib/migration-strategy.tspackages/1-framework/3-tooling/cli/src/utils/cli-errors.tspackages/1-framework/3-tooling/cli/src/utils/formatters/help.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-verify.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-update.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.tspackages/1-framework/3-tooling/cli/test/output.migration-commands.test.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/migration/src/attestation.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/runtime-detection.tspackages/1-framework/3-tooling/migration/test/attestation.test.tspackages/1-framework/3-tooling/migration/test/migration-base.test.tspackages/1-framework/3-tooling/migration/test/migration-ts.test.tspackages/1-framework/3-tooling/migration/test/runtime-detection.test.tspackages/2-mongo-family/9-family/package.jsonpackages/2-mongo-family/9-family/src/core/mongo-emit.tspackages/2-mongo-family/9-family/src/core/mongo-migration.tspackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/2-mongo-family/9-family/test/mongo-emit.test.tspackages/3-mongo-target/1-mongo-target/package.jsonpackages/3-mongo-target/1-mongo-target/src/core/migration-factories.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-planner.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-runner.tspackages/3-mongo-target/1-mongo-target/src/core/planner-produced-migration.tspackages/3-mongo-target/1-mongo-target/src/core/render-typescript.tspackages/3-mongo-target/1-mongo-target/src/exports/control.tspackages/3-mongo-target/1-mongo-target/src/exports/migration.tspackages/3-mongo-target/1-mongo-target/test/data-transform.test.tspackages/3-mongo-target/1-mongo-target/test/migration-e2e.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.test.tspackages/3-mongo-target/1-mongo-target/test/planner-produced-migration.test.tspackages/3-mongo-target/1-mongo-target/test/render-ops.test.tspackages/3-mongo-target/1-mongo-target/test/render-typescript.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/dml-executor.tspackages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.contract-to-schema-ir.test.tspackages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.tsscripts/lint-framework-target-imports.mjstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.tstest/integration/test/mongo/migration-e2e.test.tstest/integration/test/mongo/migration-m2-vocabulary.test.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (4)
- packages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-verify.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-verify.ts
✅ Files skipped from review due to trivial changes (18)
- examples/mongo-demo/migrations/20260415_add-posts-author-index/migration.json
- packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-show.ts
- packages/2-mongo-family/9-family/package.json
- packages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.ts
- packages/1-framework/1-core/errors/src/exports/migration.ts
- examples/mongo-demo/migrations/20260409T1030_migration/migration.json
- examples/retail-store/migrations/20260416_backfill-product-status/ops.json
- packages/1-framework/1-core/errors/tsdown.config.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md
- package.json
- examples/retail-store/migrations/20260416_backfill-product-status/contract.json
- packages/1-framework/3-tooling/migration/src/runtime-detection.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-status.ts
- packages/3-mongo-target/1-mongo-target/src/core/migration-factories.ts
- examples/retail-store/migrations/20260416_backfill-product-status/contract.d.ts
- packages/1-framework/3-tooling/migration/src/migration-ts.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/3-mongo-target/1-mongo-target/package.json
- packages/3-mongo-target/1-mongo-target/src/exports/migration.ts
- packages/2-mongo-family/9-family/src/core/mongo-migration.ts
- packages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts
- packages/3-mongo-target/1-mongo-target/test/migration-e2e.test.ts
- packages/3-mongo-target/2-mongo-adapter/test/dml-executor.test.ts
wmadden
added a commit
that referenced
this pull request
Apr 18, 2026
…ability Surfaces a misconfigured target (registers a migrations capability but implements neither `resolveDescriptors` nor `emit`) at the selector itself, with a structured `errorTargetHasIncompleteMigrationCapabilities` (PN-MIG-2011) that names the offending target. Previously the selector silently fell through to `class-based`, deferring the failure to the emit dispatch site, which then complained about the missing `emit` hook even when the actual intent had been descriptor flow. Threads `targetId` through `migrationStrategy(...)` callsites in `migration plan` and `migration emit` so the error message is honest. Test now asserts the throw rather than the fallthrough. Closes review finding N03 from PR #349 code review.
wmadden
added a commit
that referenced
this pull request
Apr 18, 2026
Records two intentional deviations from the original design: - `MigrationScaffoldContext` carries `fromHash`/`toHash` so class-flow targets can populate `describe()` on the rendered stub. Without these the user would have to fix the manifest identity by hand before `migration emit`. The spec now documents this in §In-scope and §Risks §5 (the latter previously characterised the identity fields as inert dummies). - `TargetMigrationsCapability` retains a narrow optional `renderDescriptorTypeScript?(descriptors, context): string` hook for the descriptor-flow branch of `migration plan`. This is the minimal change that avoids unifying `migration plan` across strategies, which is in §Out of scope. Decision §5 and §Deletion now record the deviation explicitly. Also updates the strategy-selector code snippet, AC §1 framework types, and AC §5 strategy selector to match the shipped signature (`migrationStrategy(migrations, targetId)` throwing PN-MIG-2011). Closes review findings N04 and N05 from PR #349 code review.
wmadden
added a commit
that referenced
this pull request
Apr 18, 2026
The MongoDB Family subsystem doc still listed "migration runner" as a non-goal and claimed schema evolution happens via data invariants only; that has been wrong since the Mongo planner / runner / class-flow authoring surface landed. Reframes migrations as a supported feature on Mongo and points at `migration new`/`migration plan`/`migration emit`. The Migration System subsystem doc described offline planning in SQL-only terms (`SqlSchemaIR` + `PostgresMigrationPlanner`) and made no mention of the class-flow authoring surface. Generalises the contract-to-schema section across families, and adds an §Authoring strategies section that explains the descriptor-flow vs class-flow distinction and the `migrationStrategy(...)` selector. Closes class-flow-scaffolding AC §8 from PR #349 code review.
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
425ac5c to
ba2734e
Compare
wmadden
changed the title
This was referenced Apr 19, 2026
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
d84b34a to
8265d44
Compare
wmadden
changed the title
wmadden
force-pushed
the
tml-2219-b-class-flow-scaffolding
branch
from
d1c595a to
1e0bc67
Compare
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
8265d44 to
824404e
Compare
wmadden
force-pushed
the
tml-2219-b-class-flow-scaffolding
branch
from
1e0bc67 to
f001281
Compare
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
824404e to
f3c391e
Compare
wmadden
changed the title
wmadden
force-pushed
the
tml-2219-b-class-flow-scaffolding
branch
3 times, most recently
from
eca86d0 to
3ca7de9
Compare
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
f3c391e to
54927c2
Compare
Builds on the class-flow scaffolding spine (PR B) to introduce data
transform operations as a first-class MongoDB migration operation kind:
- `dataTransform(name, { check?, run })` factory in
`@prisma-next/target-mongo/migration` produces a serialized
`MongoDataTransformOperation` with optional pre/postcheck and one or
more raw run pipelines.
- `MongoMigrationRunner` learns to dispatch data transform ops through
a new DML executor in the Mongo adapter, distinguishing them from the
DDL operations that the existing planner emits.
- The query-ast layer extends the operation union with
`MongoDataTransformOperation` and `MongoDataTransformCheck`, so
`AnyMongoMigrationOperation` is now a real union instead of a
one-member alias.
- Adds the `backfill-product-status` migration to the retail-store
example as a worked end-to-end demonstration of authoring a data
migration with the contract snapshot.
- Adds CLI journey, ops-serializer, runner, and DML executor coverage,
plus an end-to-end Mongo migration journey test against a
mongo-memory-server replica set.
- Wires `scripts/lint-framework-target-imports.mjs` into the root
`lint:deps` script so CI enforces the import boundary the spine
relies on.
wmadden
force-pushed
the
tml-2219-data-migrations-for-mongodb
branch
from
54927c2 to
cc4bbfd
Compare
The data-transform postcheck message used a different sentence shape,
hyphenated "post-check", and had a trailing period that the three other
runner diagnostics lacked. Align it to the same `Operation ${id} failed
during postcheck` pattern used by DDL precheck, DDL postcheck, and
data-transform precheck.
…t MATCH_ALL_FILTER
run now accepts only () => MongoQueryPlan | Buildable, matching
check.source and the spec decision that closures defer .build() to the
resolver. This makes placeholder("slot") throw at factory invocation
rather than at module load, and removes a silent extra surface nobody
was using.
Added a rationale comment on MATCH_ALL_FILTER explaining why
exists("_id") is used as a match-all filter (the AST has no identity
expression).
Added a check clause to the hand-authored dataTransform that detects mixed-case user names. After the first apply all names are lower-case, so on re-apply the postcheck is satisfied and the runner skips the transform. Second apply exits 0 and data is byte-identical. This pins the AC-E3 retry-skip path against a real mongodb-memory-server instance, closing the only acceptance criterion that was previously verified only via stubbed adapter/driver.
MongoDriverImpl and MigrationMongoDriver had identical dispatch logic for all 9 wire-command kinds. The only differences were lifecycle ownership (client vs no-client) and constructor shape. Every future wire kind or driver bug fix had to be mirrored in both. MongoDriverImpl now supports two construction modes via static factories: - fromConnection(uri, dbName): owns the MongoClient, close() disconnects - fromDb(db): borrows a Db, close() is a no-op createMongoRunnerDeps takes an explicit MongoDriver parameter instead of constructing one internally, keeping the adapter package (layer 2) free of a runtime dependency on the driver package (layer 3). The family descriptor composes the deps with MongoDriverImpl.fromDb(). dml-executor.ts and its test are deleted; the driver test gains two fromDb-specific tests (execute works, close is no-op).
…he real pipeline The backfill-product-status migration had a fabricated to hash (sha256:a1b2c3d4...) that was never produced by the real pipeline. Data transforms do not change storage state, so to == from. Fixed the describe().to value and re-ran both example migrations via Migration.run(), which now produces fully-attested artifacts including a content-addressed migrationId. Both manifests were previously migrationId: null.
…nd attest all five example migrations Add migration.ts class-flow sources for the two planner-scaffolded initial migrations (mongo-demo and retail-store) so users see the canonical authoring surface, not just generated JSON. Co-locate contract.json + contract.d.ts in every migration directory that was missing them (four of five). Re-emit all five migrations via Migration.run() so every migration.json carries a real migrationId hash and complete fromContract/toContract.
…-trip, remove empty contract.prisma F09: deserializeMongoQueryPlan now preserves annotations, refs, projection, and projectionTypes on round-trip instead of silently dropping them. Rename schema field columnTypeIds → projectionTypes to match the PlanMeta interface. Add a round-trip test covering all four fields. F10: delete the zero-byte contract.prisma from the backfill example migration — vanilla data transforms do not need a .prisma file.
… migrationId assertion After F01 changed createMongoRunnerDeps to require a MongoDriver parameter, 12 call sites in integration and example tests still passed only one argument. Pass MongoDriverImpl.fromDb(extractDb(controlDriver)) at each site. Also update retail-store manual-migration.test.ts to expect an attested migrationId (sha256:…) now that Migration.run() produces fully-attested manifests.
…d and 2-arg createMongoRunnerDeps
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts (1)
424-436: UseifDefined()for optionalPlanMetafields.This block reintroduces inline conditional spreads in a package file. Please switch these optional assignments to
ifDefined()soPlanMetaconstruction stays aligned with the repo convention.♻️ Example shape
const meta: PlanMeta = { target: data.meta.target, storageHash: data.meta.storageHash, lane: data.meta.lane, paramDescriptors: data.meta.paramDescriptors as PlanMeta['paramDescriptors'], - ...(data.meta.targetFamily !== undefined && { targetFamily: data.meta.targetFamily }), - ...(data.meta.profileHash !== undefined && { profileHash: data.meta.profileHash }), - ...(data.meta.annotations !== undefined && { - annotations: data.meta.annotations as PlanMeta['annotations'], - }), - ...(data.meta.refs !== undefined && { refs: data.meta.refs as PlanMeta['refs'] }), - ...(data.meta.projection !== undefined && { projection: data.meta.projection }), - ...(data.meta.projectionTypes !== undefined && { projectionTypes: data.meta.projectionTypes }), + ...ifDefined(data.meta.targetFamily, (targetFamily) => ({ targetFamily })), + ...ifDefined(data.meta.profileHash, (profileHash) => ({ profileHash })), + ...ifDefined(data.meta.annotations as PlanMeta['annotations'] | undefined, (annotations) => ({ + annotations, + })), + ...ifDefined(data.meta.refs as PlanMeta['refs'] | undefined, (refs) => ({ refs })), + ...ifDefined(data.meta.projection, (projection) => ({ projection })), + ...ifDefined(data.meta.projectionTypes, (projectionTypes) => ({ projectionTypes })), };Also add
import { ifDefined } from '@prisma-next/utils/defined';.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/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts` around lines 424 - 436, Replace the inline conditional spreads when constructing the PlanMeta object with the repo utility ifDefined: import ifDefined from '@prisma-next/utils/defined' (add the import at top) and wrap each optional field (targetFamily, profileHash, annotations, refs, projection, projectionTypes) using ifDefined(data.meta.<field>, { <fieldName>: data.meta.<field> }) or the library's documented pattern so the PlanMeta construction uses ifDefined(...) instead of ...(condition && { ... }) while keeping required fields (target, storageHash, lane, paramDescriptors) unchanged; ensure types remain cast as before (e.g., as PlanMeta['annotations']).packages/3-mongo-target/3-mongo-driver/src/exports/index.ts (1)
1-1: KeepMongoDriverImploff the public API.Re-exporting the concrete class here makes the implementation part of the supported package surface. Please expose
fromDb()through a factory and keep callers pinned toMongoDriver, notMongoDriverImpl.As per coding guidelines, "Export interfaces and factory functions for public APIs; keep concrete classes as private implementation details".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/3-mongo-driver/src/exports/index.ts` at line 1, Remove MongoDriverImpl from the public exports: stop re-exporting MongoDriverImpl from the barrel (keep only createMongoDriver exported). Replace any usage that relied on MongoDriverImpl with the public factory API by exposing a fromDb factory that returns the MongoDriver interface (either add a fromDb function export or attach fromDb to createMongoDriver as the public factory). Ensure the exported symbol is createMongoDriver (and optionally createMongoDriver.fromDb) and that callers depend on the MongoDriver interface rather than the concrete MongoDriverImpl class.packages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts (1)
414-834: Split the new runner coverage into dedicated test files.These data-transform and round-trip suites push this file far past the 500-line cap and add separate top-level concerns to an already large runner spec. Moving them into files like
mongo-runner.data-transform.test.tsandmongo-runner.round-trip.test.tswill keep failures easier to navigate.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-level
describeblocks 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/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts` around lines 414 - 834, The file exceeds the 500-line guideline because two large top-level suites ("MongoMigrationRunner - data transforms" and "MongoMigrationRunner - E2E round-trip") are bundled in mongo-runner.test.ts; split each suite into its own test file (e.g., mongo-runner.data-transform.test.ts and mongo-runner.round-trip.test.ts) by moving the describe(...) blocks that contain makeDataTransformPlan, makeCheckSource, makePrecheckObj/makePostcheckObj and the E2E round-trip tests (which reference dataTransform, RawUpdateManyCommand, RawAggregateCommand, MongoMigrationPlanner, serializeMongoOps, makeContract, makeRunner, db, etc.) into their respective new files, preserve and export any shared helpers (makeRunner, db setup/teardown) or import them from the original test utilities, and update imports/exports so tests run unchanged aside from their new locations; ensure package test config picks up the new filenames.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/retail-store/migrations/20260413T0314_migration/migration.ts`:
- Around line 29-65: The current collection validators created in
createCollection('carts') (and the similar blocks for orders, products,
invoices) are missing required fields from the contract (e.g., Price.amount in
carts/orders/products and subtotal, tax, total, unitPrice, lineTotal in
invoices); update each validator's $jsonSchema properties and required arrays to
include these missing scalar fields with correct bsonType (e.g., Price.amount as
an int/number, subtotal/tax/total/unitPrice/lineTotal as appropriate numeric
bsonType) and ensure nested objects (like price and invoice line items) list
these properties in their required arrays so the emitted schemas exactly match
the contract.d.ts/contract.json.
- Line 286: The migration creates a non-unique secondary index on carts.userId
which breaks the contract that carts.userId must be unique; update the migration
invocation that calls createIndex for the 'carts' collection (the
createIndex('carts', [{ field: 'userId', direction: 1 }]) call) to create a
unique index instead (use the migration API's unique flag/option or the
createUniqueIndex variant so the carts.userId index enforces uniqueness).
In
`@examples/retail-store/migrations/20260415_add-product-validation/contract.d.ts`:
- Around line 24-31: The emitted types PriceOutput and PriceInput declare amount
as required but the JSON Schema validators for the nested price objects (the
price properties used in the validators around the blocks you noted) only define
currency; update the source contract's price schema so each price.properties
includes an amount with bsonType: 'double' and add "amount" to the corresponding
required arrays for every nested price validator (the three price schema blocks
referenced), then re-run the generator to re-emit the contract.d.ts so the
runtime validators match the PriceInput/PriceOutput types.
- Around line 86-97: The generated contract types mark Invoice.subtotal,
Invoice.tax, Invoice.total and InvoiceLineItem.unitPrice,
InvoiceLineItem.lineTotal as required, but the emitted invoice validator does
not define these properties or mark them required; update the validator schema
to include those properties with the correct codec/type mappings (use the same
CodecTypes entries as in Invoice and InvoiceLineItem) and add them to the
required list, or if they are meant to be optional, change the Invoice and
InvoiceLineItem type declarations to optional for those fields and then
regenerate; ensure the symbol names Invoice, InvoiceLineItem and the invoice
validator (the function/constant producing the Invoice schema) stay in sync
before regenerating the artifacts.
In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts`:
- Around line 274-290: In evaluateDataTransformChecks ensure we never execute
write commands: after obtaining wireCommand = adapter.lower(check.source)
validate it is read-only (e.g., command name is find, count, distinct, aggregate
without $out/$merge, listIndexes, findOne, etc.) and reject/throw an error for
any write-like commands (insert, update, delete, findAndModify, create/drop
indexes, aggregate with $out/$merge or any top-level insert/update/delete
fields) before calling driver.execute; use the wireCommand object returned by
adapter.lower and perform explicit checks for those write indicators and fail
fast rather than executing them.
In `@packages/3-mongo-target/3-mongo-driver/test/mongo-driver.test.ts`:
- Around line 317-321: The test name claims the external client remains usable
after driver.close(), but it doesn't assert that; update the test in
mongo-driver.test.ts (the spec that creates driver via
MongoDriverImpl.fromDb(seedClient.db(dbName)) and calls driver.close()) to
perform an immediate operation on the external seedClient after await
driver.close() (for example call seedClient.db(dbName).admin().ping() or run a
simple find/count) and assert it resolves/returns expected result to prove the
client is still usable.
---
Nitpick comments:
In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts`:
- Around line 424-436: Replace the inline conditional spreads when constructing
the PlanMeta object with the repo utility ifDefined: import ifDefined from
'@prisma-next/utils/defined' (add the import at top) and wrap each optional
field (targetFamily, profileHash, annotations, refs, projection,
projectionTypes) using ifDefined(data.meta.<field>, { <fieldName>:
data.meta.<field> }) or the library's documented pattern so the PlanMeta
construction uses ifDefined(...) instead of ...(condition && { ... }) while
keeping required fields (target, storageHash, lane, paramDescriptors) unchanged;
ensure types remain cast as before (e.g., as PlanMeta['annotations']).
In `@packages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.ts`:
- Around line 414-834: The file exceeds the 500-line guideline because two large
top-level suites ("MongoMigrationRunner - data transforms" and
"MongoMigrationRunner - E2E round-trip") are bundled in mongo-runner.test.ts;
split each suite into its own test file (e.g.,
mongo-runner.data-transform.test.ts and mongo-runner.round-trip.test.ts) by
moving the describe(...) blocks that contain makeDataTransformPlan,
makeCheckSource, makePrecheckObj/makePostcheckObj and the E2E round-trip tests
(which reference dataTransform, RawUpdateManyCommand, RawAggregateCommand,
MongoMigrationPlanner, serializeMongoOps, makeContract, makeRunner, db, etc.)
into their respective new files, preserve and export any shared helpers
(makeRunner, db setup/teardown) or import them from the original test utilities,
and update imports/exports so tests run unchanged aside from their new
locations; ensure package test config picks up the new filenames.
In `@packages/3-mongo-target/3-mongo-driver/src/exports/index.ts`:
- Line 1: Remove MongoDriverImpl from the public exports: stop re-exporting
MongoDriverImpl from the barrel (keep only createMongoDriver exported). Replace
any usage that relied on MongoDriverImpl with the public factory API by exposing
a fromDb factory that returns the MongoDriver interface (either add a fromDb
function export or attach fromDb to createMongoDriver as the public factory).
Ensure the exported symbol is createMongoDriver (and optionally
createMongoDriver.fromDb) and that callers depend on the MongoDriver interface
rather than the concrete MongoDriverImpl class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 38c44707-9fb5-4699-b78d-f9fc763eddb9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
examples/mongo-demo/migrations/20260409T1030_migration/contract.d.tsexamples/mongo-demo/migrations/20260409T1030_migration/contract.jsonexamples/mongo-demo/migrations/20260409T1030_migration/migration.jsonexamples/mongo-demo/migrations/20260409T1030_migration/migration.tsexamples/mongo-demo/migrations/20260415_add-posts-author-index/contract.d.tsexamples/mongo-demo/migrations/20260415_add-posts-author-index/contract.jsonexamples/mongo-demo/migrations/20260415_add-posts-author-index/migration.jsonexamples/mongo-demo/test/manual-migration.test.tsexamples/retail-store/migrations/20260413T0314_migration/contract.d.tsexamples/retail-store/migrations/20260413T0314_migration/contract.jsonexamples/retail-store/migrations/20260413T0314_migration/migration.jsonexamples/retail-store/migrations/20260413T0314_migration/migration.tsexamples/retail-store/migrations/20260413T0314_migration/ops.jsonexamples/retail-store/migrations/20260415_add-product-validation/contract.d.tsexamples/retail-store/migrations/20260415_add-product-validation/contract.jsonexamples/retail-store/migrations/20260415_add-product-validation/migration.jsonexamples/retail-store/migrations/20260416_backfill-product-status/contract.d.tsexamples/retail-store/migrations/20260416_backfill-product-status/contract.jsonexamples/retail-store/migrations/20260416_backfill-product-status/migration.jsonexamples/retail-store/migrations/20260416_backfill-product-status/migration.tsexamples/retail-store/migrations/20260416_backfill-product-status/ops.jsonexamples/retail-store/test/manual-migration.test.tspackage.jsonpackages/2-mongo-family/4-query/query-ast/src/exports/control.tspackages/2-mongo-family/4-query/query-ast/src/migration-operation-types.tspackages/2-mongo-family/9-family/package.jsonpackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/3-mongo-target/1-mongo-target/package.jsonpackages/3-mongo-target/1-mongo-target/src/core/migration-factories.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-runner.tspackages/3-mongo-target/1-mongo-target/src/exports/migration.tspackages/3-mongo-target/1-mongo-target/test/data-transform.test.tspackages/3-mongo-target/1-mongo-target/test/migration-e2e.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.test.tspackages/3-mongo-target/2-mongo-adapter/package.jsonpackages/3-mongo-target/2-mongo-adapter/src/core/runner-deps.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.tspackages/3-mongo-target/3-mongo-driver/src/exports/index.tspackages/3-mongo-target/3-mongo-driver/src/mongo-driver.tspackages/3-mongo-target/3-mongo-driver/test/mongo-driver.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/contract-additive.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/contract-base.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/package.jsontest/integration/test/mongo/migration-authoring-e2e.test.tstest/integration/test/mongo/migration-e2e.test.tstest/integration/test/mongo/migration-m2-vocabulary.test.tstest/integration/test/mongo/migration-psl-authoring.test.ts
✅ Files skipped from review due to trivial changes (21)
- packages/3-mongo-target/2-mongo-adapter/package.json
- packages/2-mongo-family/9-family/package.json
- packages/3-mongo-target/1-mongo-target/package.json
- test/integration/test/fixtures/cli/cli-e2e-test-app/package.json
- packages/3-mongo-target/1-mongo-target/src/exports/migration.ts
- packages/2-mongo-family/4-query/query-ast/src/exports/control.ts
- examples/retail-store/migrations/20260416_backfill-product-status/migration.json
- package.json
- examples/retail-store/migrations/20260413T0314_migration/ops.json
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/prisma-next.config.with-db.ts
- packages/3-mongo-target/1-mongo-target/test/migration-e2e.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/contract-base.ts
- examples/retail-store/migrations/20260415_add-product-validation/migration.json
- examples/mongo-demo/migrations/20260415_add-posts-author-index/contract.json
- examples/mongo-demo/migrations/20260409T1030_migration/contract.json
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.test.ts
- examples/retail-store/migrations/20260413T0314_migration/contract.json
- examples/mongo-demo/migrations/20260409T1030_migration/contract.d.ts
- examples/retail-store/migrations/20260416_backfill-product-status/ops.json
- examples/retail-store/migrations/20260416_backfill-product-status/contract.json
- examples/retail-store/migrations/20260415_add-product-validation/contract.json
🚧 Files skipped from review as they are similar to previous changes (11)
- examples/mongo-demo/migrations/20260415_add-posts-author-index/migration.json
- packages/3-mongo-target/2-mongo-adapter/src/exports/control.ts
- test/integration/test/mongo/migration-authoring-e2e.test.ts
- examples/mongo-demo/migrations/20260409T1030_migration/migration.json
- packages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.test.ts
- packages/2-mongo-family/4-query/query-ast/src/migration-operation-types.ts
- examples/retail-store/migrations/20260416_backfill-product-status/migration.ts
- packages/2-mongo-family/9-family/src/core/mongo-target-descriptor.ts
- test/integration/test/mongo/migration-m2-vocabulary.test.ts
- packages/3-mongo-target/2-mongo-adapter/src/core/runner-deps.ts
- packages/3-mongo-target/1-mongo-target/src/core/migration-factories.ts
…egate commands (A06)
…horing e2e (A14a)
…ileHash predicate (A15a)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts (1)
24-36: Move constant and helper above all imports for consistent organization.The
READ_ONLY_CHECK_COMMAND_KINDSconstant andhasProfileHashfunction are placed between import blocks (after line 22 imports, before lines 35-36 imports), creating inconsistent module organization.♻️ Suggested reordering
import { notOk, ok } from '@prisma-next/utils/result'; +import { FilterEvaluator } from './filter-evaluator'; +import { deserializeMongoOps } from './mongo-ops-serializer'; const READ_ONLY_CHECK_COMMAND_KINDS: ReadonlySet<string> = new Set(['aggregate', 'rawAggregate']); function hasProfileHash(value: unknown): value is { readonly profileHash: string } { return ( typeof value === 'object' && value !== null && Object.hasOwn(value, 'profileHash') && typeof (value as { profileHash: unknown }).profileHash === 'string' ); } - -import { FilterEvaluator } from './filter-evaluator'; -import { deserializeMongoOps } from './mongo-ops-serializer';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts` around lines 24 - 36, Move the READ_ONLY_CHECK_COMMAND_KINDS constant and hasProfileHash type guard so they are declared before any import statements to maintain consistent module organization; specifically relocate the declarations that currently sit between the import groups so they appear above the imports that reference symbols like FilterEvaluator and deserializeMongoOps, ensuring no circular dependency is introduced and running a quick type-check after the move.
🤖 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/3-mongo-target/1-mongo-target/src/core/mongo-runner.ts`:
- Around line 24-36: Move the READ_ONLY_CHECK_COMMAND_KINDS constant and
hasProfileHash type guard so they are declared before any import statements to
maintain consistent module organization; specifically relocate the declarations
that currently sit between the import groups so they appear above the imports
that reference symbols like FilterEvaluator and deserializeMongoOps, ensuring no
circular dependency is introduced and running a quick type-check after the move.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 83aceb65-2b8f-40a1-9113-eccdf9f77ea0
📒 Files selected for processing (8)
examples/retail-store/migrations/20260413T0314_migration/migration.jsonexamples/retail-store/migrations/20260413T0314_migration/migration.tsexamples/retail-store/migrations/20260413T0314_migration/ops.jsonpackages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-runner.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.test.tspackages/3-mongo-target/3-mongo-driver/test/mongo-driver.test.tstest/integration/test/mongo/migration-authoring-e2e.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.test.ts
- packages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/3-mongo-target/3-mongo-driver/test/mongo-driver.test.ts
- examples/retail-store/migrations/20260413T0314_migration/migration.json
- examples/retail-store/migrations/20260413T0314_migration/migration.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary
This is PR C in the stacked split of the original #349 (base: PR B —
tml-2219-b-class-flow-scaffolding).Builds on the class-flow scaffolding spine (PR B) to introduce data
transform operations as a first-class MongoDB migration operation kind.
Highlights
dataTransform(name, { check?, run })factory in@prisma-next/target-mongo/migrationproduces a serializedMongoDataTransformOperationwith optional pre/postcheck and one ormore raw run pipelines.
MongoMigrationRunnerlearns to dispatch data transform ops via anew DML executor in the Mongo adapter, distinguishing them from the
DDL operations the existing planner emits.
MongoDataTransformOperationandMongoDataTransformCheck;AnyMongoMigrationOperationis now a real union.examples/retail-store/migrations/20260416_backfill-product-statusdemonstrates authoring a data migration with the contract snapshot.
Mongo migration journey test against
mongo-memory-serverreplica set.scripts/lint-framework-target-imports.mjsinto rootlint:deps.Stacked PR context
This PR depends on the following stack:
new/apply/planCLI commands #354)Test plan
pnpm typecheck,pnpm lint,pnpm test)examples/retail-storeend-to-end: scaffold, fill placeholders, emit, apply backfillmongo-migration.e2e.test.tspasses against mongo-memory-serverdataTransform()round-trips through ops.json/runnerSummary by CodeRabbit
New Features
Examples
Tooling
Tests