Open
Conversation
📝 WalkthroughWalkthroughReplaces the aggregation-focused pipeline-builder package with a broader typed Mongo query builder ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryRoot as "mongoQuery / QueryRoot"
participant Collection as "CollectionHandle"
participant Filtered as "FilteredCollection"
participant Driver as "Mongo Driver / Wire"
participant DB as "MongoDB"
Client->>QueryRoot: mongoQuery(contractJson)
QueryRoot->>Collection: from('orders')
alt Unqualified insert
Collection->>Driver: insertOne command (plan.meta.lane = "mongo-query")
Driver->>DB: run insertOne(...)
DB-->>Driver: Insert result
Driver-->>Client: plan with InsertOneResult
end
alt Filtered update / find-and-modify
Collection->>Filtered: match(filter)
Filtered->>Driver: findOneAndUpdate / updateMany (includes upsert, sort?, skip?, returnDocument?)
Driver->>DB: call findOneAndUpdate/update with options
DB-->>Driver: Result (matched/modified and optional upsertedId/count)
Driver-->>Client: plan with UpdateResult
end
alt Raw command escape hatch
Client->>QueryRoot: rawCommand(AnyMongoCommand)
QueryRoot->>Driver: wrap and forward command (requires storageHash)
Driver->>DB: execute raw command
DB-->>Driver: raw response
Driver-->>Client: raw plan result
end
sequenceDiagram
participant Dev as Developer
participant FieldAccessor
participant Expression
participant UpdateOps
Dev->>FieldAccessor: f('a.b') or f.a.b
FieldAccessor->>Expression: produce TypedAggExpr with _path
alt Filter usage
Expression->>Expression: .eq/.nin -> MongoFilterExpr
Expression-->>Dev: filter callback yields MongoFieldFilter
end
alt Update usage
Expression->>UpdateOps: .set/.inc/... -> TypedUpdateOp[]
UpdateOps->>UpdateOps: resolveUpdaterResult / foldUpdateOps -> MongoUpdateSpec or pipeline stages
UpdateOps-->>Dev: resolved update payload for terminal
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.ts (1)
60-66:⚠️ Potential issue | 🟡 MinorPass
command.upsertto wire command constructors in raw update lowering paths.Lines 101 and 103 construct raw update wire commands without the
upsertparameter, while the typed update paths (lines 61–66, 73–78) correctly passcommand.upsert. BothRawUpdateOneCommandandRawUpdateManyCommandcarry theupsertfield, so raw lowering must propagate it to maintain consistent behavior.Required fix
case 'rawUpdateOne': - return new UpdateOneWireCommand(command.collection, command.filter, command.update); + return new UpdateOneWireCommand( + command.collection, + command.filter, + command.update, + command.upsert, + ); case 'rawUpdateMany': - return new UpdateManyWireCommand(command.collection, command.filter, command.update); + return new UpdateManyWireCommand( + command.collection, + command.filter, + command.update, + command.upsert, + );🤖 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/mongo-adapter.ts` around lines 60 - 66, The raw-update lowering path is not propagating the upsert flag: when creating RawUpdateOneCommand and RawUpdateManyCommand you must pass command.upsert (just like the typed paths pass it to UpdateOneWireCommand/UpdateManyWireCommand). Update the places that construct RawUpdateOneCommand and RawUpdateManyCommand to include command.upsert as the upsert argument (and ensure any helper like this.#lowerRawUpdate forwards that value) so raw and typed update behavior is consistent.
🧹 Nitpick comments (5)
packages/2-mongo-family/5-query-builders/query-builder/src/update-ops.ts (2)
136-140: Note theas unknown as MongoValuecasts for protocol-specific values.These casts handle MongoDB's wire protocol requirements where
$unsetexpects an empty string and$renameexpects a string value, butMongoValuemay not include plain strings. The casts are pragmatic given the protocol constraints, though per coding guidelines blind casts are generally discouraged.If feasible, consider widening
MongoValueor using a dedicated wire-format type to avoid the casts, but this may be acceptable given MongoDB's protocol quirks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/src/update-ops.ts` around lines 136 - 140, The code in update-ops.ts uses blind casts "as unknown as MongoValue" for $unset and $rename in the switch handling (see the case blocks for '$unset' and '$rename'), which is brittle; either widen the MongoValue type to include plain string values expected by the wire protocol or introduce a dedicated wire-format type (e.g., WireMongoValue) and use that in the ensure(...) return type for update builders; update the signatures/typedefs referenced by ensure(...) and the surrounding update builder logic so you can assign '' and entry.newName without unsafe casts, or if widening types is not feasible, add a short typed helper (e.g., castToWireValue) used by the '$unset' and '$rename' cases to centralize the unsafe conversion and document why it's necessary.
151-160: Consider documenting the type coercion rationale.The final cast
buckets as unknown as Record<string, MongoValue>and the intermediate casts for$pullAll,$currentDateare necessary because the internal bucket structure (Record<string, Record<string, MongoValue>>) doesn't directly match the return type. This is correct at runtime since MongoDB expects this exact shape.A brief inline comment explaining "bucket structure matches MongoUpdateSpec's non-pipeline form" would help future maintainers understand why the cast is safe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/src/update-ops.ts` around lines 151 - 160, Add a brief inline comment near the final cast of buckets (and the `$pullAll` / `$currentDate` assignments) explaining that the internal type is Record<string, Record<string, MongoValue>> but we cast to Record<string, MongoValue> because the assembled bucket structure matches MongoDB's non-pipeline MongoUpdateSpec shape at runtime; reference the local variable `buckets` and the update-op handling in this function (`$pullAll`, `$currentDate`) so maintainers understand the coercion is intentional and safe.examples/mongo-demo/src/db.ts (1)
13-13: Preferquerynaming here too, notpipeline.This file switched to
mongoQuerybut still exportspipeline, which keeps old semantics in the example API and makes migration less clear.Suggested rename
-const pipeline = mongoQuery<Contract>({ contractJson }); +const query = mongoQuery<Contract>({ contractJson }); ... - return { orm, runtime, pipeline, contract }; + return { orm, runtime, query, contract };As per coding guidelines: “Do not add backward-compatibility shims; update call sites instead.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/mongo-demo/src/db.ts` at line 13, The example exports a variable named pipeline while using mongoQuery, causing mixed semantics; rename the exported variable pipeline to query where it's declared (the line using mongoQuery<Contract>({ contractJson })) and update any local exports or usages to reference query so the API matches the new mongoQuery naming and avoids keeping the old "pipeline" shim.packages/3-extensions/mongo/src/runtime/mongo.ts (1)
39-39: Buildqueryfrom the validated contract to keep surfaces consistent.
ormandruntimeusecontractfromvalidateMongoContract, butqueryuses rawoptions.contractJson. If validation normalizes/enriches the contract shape, these surfaces can diverge.Suggested fix
- const query = mongoQuery<TContract>({ contractJson: options.contractJson }); + const query = mongoQuery<TContract>({ contractJson: contract });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/mongo/src/runtime/mongo.ts` at line 39, Replace the use of raw options.contractJson when constructing the query with the validated/normalized contract returned by validateMongoContract: after calling validateMongoContract(...) use the returned variable (e.g., contract) as the input to mongoQuery<TContract> instead of options.contractJson so that the query, runtime and orm all operate on the same validated/enriched contract shape.examples/retail-store/src/db.ts (1)
13-13: Renamepipelinetoqueryto avoid carrying legacy API naming.Now that this uses
mongoQuery, keeping the exported field aspipelinelooks like a compatibility shim and makes the example API inconsistent with the rest of the Mongo runtime surface.Suggested rename
-const pipeline = mongoQuery<Contract>({ contractJson }); +const query = mongoQuery<Contract>({ contractJson }); ... - return { orm, runtime, pipeline, raw, contract }; + return { orm, runtime, query, raw, contract };As per coding guidelines: “Do not add backward-compatibility shims; update call sites instead.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/retail-store/src/db.ts` at line 13, Rename the exported variable named pipeline to query to remove legacy naming: change the const pipeline = mongoQuery<Contract>({ contractJson }) binding to const query = mongoQuery<Contract>({ contractJson }), update any local references and the module export that still exposes pipeline to use query instead, and run a quick search/replace for usages of pipeline in this example to update call sites to query so there are no backward-compatibility shims left.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-mongo-family/5-query-builders/query-builder/README.md`:
- Line 5: Replace the transient project link
"projects/mongo-pipeline-builder/specs/query-builder-unification.spec.md" in the
README sentence about write terminals with a stable documentation path (for
example an architecture or package docs page under docs/ or packages/ that
covers the query-builder unification design); update the sentence to point to
that durable doc and remove dependence on the projects/ directory while keeping
the reference to TML-2267 if desired (edit the line containing the existing link
text to the new stable URL).
In `@packages/2-mongo-family/5-query-builders/query-builder/src/builder.ts`:
- Around line 563-577: Remove the backwards-compatibility alias that resurfaces
the deprecated pipeline-builder name: delete the PipelineBuilder type alias and
the const PipelineBuilder = PipelineChain assignment so only PipelineChain
remains; then remove any re-exports of PipelineBuilder from your public exports
(so callers can no longer import the deprecated name) and update any internal
import sites that still reference PipelineBuilder to use PipelineChain (or
CollectionHandle/FilteredCollection) instead.
- Around line 81-93: The phantom-type markers U and F on PipelineChain are
currently unused because no conditional write terminals exist; either implement
the promised terminals (add methods updateMany, updateOne, findOneAndUpdate,
findOneAndDelete to class PipelineChain with signatures that are gated by the U
and F phantom parameters so their availability is reflected in the return types
and TypeScript constraints) or remove the marker plumbing entirely (strip U/F
from PipelineChain and all stage methods that toggle them). Also remove the
backward-compatibility alias PipelineBuilder and update any call sites to use
PipelineChain instead to comply with the guideline against shims.
In `@packages/2-mongo-family/5-query-builders/query-builder/src/query.ts`:
- Around line 30-33: Change the mongoQuery API to require a typed contract at
the boundary: update the function signature of mongoQuery to accept
contractJson: TContract (not unknown) so callers must pass a validated contract;
inside mongoQuery remove the unsafe blind-cast and the secondary re-cast
(replace the `contract as unknown as MongoContract` usage with the already-typed
`contract` after you perform the storage-hash check), ensuring callers get
proper compile-time typing rather than a later runtime failure in from() or
rawCommand().
In `@packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts`:
- Around line 474-519: The findOneAndUpdate and findOneAndDelete signatures
currently claim they return MongoQueryPlan<ModelToDocShape<...>> but omit the
legitimate no-match/null case; update both return types to
MongoQueryPlan<ModelToDocShape<TContract, ModelName> | null> (i.e. include |
null) so callers are correctly typed for a missing document, and scan for any
other find-and-modify helpers in this file (e.g. any other findOne* methods) to
apply the same change to their return types.
In
`@packages/2-mongo-family/5-query-builders/query-builder/test/state-machine.test-d.ts`:
- Around line 16-40: Replace the current GetU and GetF helper types that use
explicit any placeholders and biome-ignore comments by using bare infer
placeholders so the type parameters are inferred directly from PipelineChain;
specifically update type GetU<T> to pattern-match T extends PipelineChain<infer
_TContract, infer _Shape, infer U, infer _F> ? U : never and update type GetF<T>
to pattern-match T extends PipelineChain<infer _TContract, infer _Shape, infer
_U, infer F> ? F : never, removing the any usages and the biome-ignore comments
while keeping the same exported type names.
---
Outside diff comments:
In `@packages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.ts`:
- Around line 60-66: The raw-update lowering path is not propagating the upsert
flag: when creating RawUpdateOneCommand and RawUpdateManyCommand you must pass
command.upsert (just like the typed paths pass it to
UpdateOneWireCommand/UpdateManyWireCommand). Update the places that construct
RawUpdateOneCommand and RawUpdateManyCommand to include command.upsert as the
upsert argument (and ensure any helper like this.#lowerRawUpdate forwards that
value) so raw and typed update behavior is consistent.
---
Nitpick comments:
In `@examples/mongo-demo/src/db.ts`:
- Line 13: The example exports a variable named pipeline while using mongoQuery,
causing mixed semantics; rename the exported variable pipeline to query where
it's declared (the line using mongoQuery<Contract>({ contractJson })) and update
any local exports or usages to reference query so the API matches the new
mongoQuery naming and avoids keeping the old "pipeline" shim.
In `@examples/retail-store/src/db.ts`:
- Line 13: Rename the exported variable named pipeline to query to remove legacy
naming: change the const pipeline = mongoQuery<Contract>({ contractJson })
binding to const query = mongoQuery<Contract>({ contractJson }), update any
local references and the module export that still exposes pipeline to use query
instead, and run a quick search/replace for usages of pipeline in this example
to update call sites to query so there are no backward-compatibility shims left.
In `@packages/2-mongo-family/5-query-builders/query-builder/src/update-ops.ts`:
- Around line 136-140: The code in update-ops.ts uses blind casts "as unknown as
MongoValue" for $unset and $rename in the switch handling (see the case blocks
for '$unset' and '$rename'), which is brittle; either widen the MongoValue type
to include plain string values expected by the wire protocol or introduce a
dedicated wire-format type (e.g., WireMongoValue) and use that in the
ensure(...) return type for update builders; update the signatures/typedefs
referenced by ensure(...) and the surrounding update builder logic so you can
assign '' and entry.newName without unsafe casts, or if widening types is not
feasible, add a short typed helper (e.g., castToWireValue) used by the '$unset'
and '$rename' cases to centralize the unsafe conversion and document why it's
necessary.
- Around line 151-160: Add a brief inline comment near the final cast of buckets
(and the `$pullAll` / `$currentDate` assignments) explaining that the internal
type is Record<string, Record<string, MongoValue>> but we cast to Record<string,
MongoValue> because the assembled bucket structure matches MongoDB's
non-pipeline MongoUpdateSpec shape at runtime; reference the local variable
`buckets` and the update-op handling in this function (`$pullAll`,
`$currentDate`) so maintainers understand the coercion is intentional and safe.
In `@packages/3-extensions/mongo/src/runtime/mongo.ts`:
- Line 39: Replace the use of raw options.contractJson when constructing the
query with the validated/normalized contract returned by validateMongoContract:
after calling validateMongoContract(...) use the returned variable (e.g.,
contract) as the input to mongoQuery<TContract> instead of options.contractJson
so that the query, runtime and orm all operate on the same validated/enriched
contract shape.
🪄 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: 195ea9d8-3158-47e7-b67a-a00ac60a6144
⛔ Files ignored due to path filters (6)
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-migrations.spec.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/query-builder-unification-plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/specs/query-builder-unification.spec.mdis excluded by!projects/**projects/one-package-install/plan.mdis excluded by!projects/**
📒 Files selected for processing (58)
docs/Architecture Overview.mddocs/architecture docs/Package-Layering.mddocs/reference/Package Naming Conventions.mdexamples/mongo-demo/package.jsonexamples/mongo-demo/src/db.tsexamples/mongo-demo/src/server.tsexamples/retail-store/package.jsonexamples/retail-store/src/data/events.tsexamples/retail-store/src/db.tsexamples/retail-store/test/setup.tspackages/2-mongo-family/4-query/query-ast/src/commands.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/builder.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/exports/index.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/field-proxy.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/filter-proxy.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/pipeline.tspackages/2-mongo-family/5-query-builders/query-builder/DEVELOPING.mdpackages/2-mongo-family/5-query-builders/query-builder/README.mdpackages/2-mongo-family/5-query-builders/query-builder/package.jsonpackages/2-mongo-family/5-query-builders/query-builder/src/accumulator-helpers.tspackages/2-mongo-family/5-query-builders/query-builder/src/builder.tspackages/2-mongo-family/5-query-builders/query-builder/src/exports/index.tspackages/2-mongo-family/5-query-builders/query-builder/src/expression-helpers.tspackages/2-mongo-family/5-query-builders/query-builder/src/field-accessor.tspackages/2-mongo-family/5-query-builders/query-builder/src/markers.tspackages/2-mongo-family/5-query-builders/query-builder/src/query.tspackages/2-mongo-family/5-query-builders/query-builder/src/state-classes.tspackages/2-mongo-family/5-query-builders/query-builder/src/types.tspackages/2-mongo-family/5-query-builders/query-builder/src/update-ops.tspackages/2-mongo-family/5-query-builders/query-builder/test/accumulator-helpers.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/accumulator-helpers.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder-new-stages.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/expression-helpers.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/expression-helpers.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/find-and-modify.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/fixtures/test-contract.tspackages/2-mongo-family/5-query-builders/query-builder/test/raw-command.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/state-machine-surface.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/state-machine.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/types.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/writes.test.tspackages/2-mongo-family/5-query-builders/query-builder/tsconfig.jsonpackages/2-mongo-family/5-query-builders/query-builder/tsconfig.prod.jsonpackages/2-mongo-family/5-query-builders/query-builder/tsdown.config.tspackages/2-mongo-family/5-query-builders/query-builder/vitest.config.tspackages/2-mongo-family/6-transport/mongo-wire/src/results.tspackages/2-mongo-family/6-transport/mongo-wire/src/wire-commands.tspackages/2-mongo-family/7-runtime/package.jsonpackages/2-mongo-family/7-runtime/test/query-builder.test.tspackages/2-mongo-family/7-runtime/test/runtime-types.test-d.tspackages/3-extensions/mongo/package.jsonpackages/3-extensions/mongo/src/runtime/mongo.tspackages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.tspackages/3-mongo-target/3-mongo-driver/src/mongo-driver.tstest/integration/package.jsontest/integration/test/mongo/query-builder.test.ts
💤 Files with no reviewable changes (5)
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/field-proxy.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/filter-proxy.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/exports/index.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/builder.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/pipeline.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts (1)
479-513:⚠️ Potential issue | 🟡 MinorInclude
| nullin find-and-modify return types for no-match case.Both
findOneAndUpdateandfindOneAndDeletecan legitimately return no document when the filter doesn't match any documents. The current return types omit this case, which could lead to runtime surprises.🩹 Suggested fix
findOneAndUpdate( updaterFn: ( fields: FieldAccessor<ModelToDocShape<TContract, ModelName>>, ) => ReadonlyArray<UpdaterItem>, opts: { readonly upsert?: boolean; readonly returnDocument?: 'before' | 'after' } = {}, - ): MongoQueryPlan<ModelToDocShape<TContract, ModelName>> { + ): MongoQueryPlan<ModelToDocShape<TContract, ModelName> | null> { ... - override findOneAndDelete(): MongoQueryPlan<ModelToDocShape<TContract, ModelName>> { + override findOneAndDelete(): MongoQueryPlan<ModelToDocShape<TContract, ModelName> | null> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts` around lines 479 - 513, The return type of both methods findOneAndUpdate and findOneAndDelete should include the possibility of no-match by allowing null; update their signatures to return MongoQueryPlan<ModelToDocShape<TContract, ModelName> | null> (or adjust the contained command/result type used by MongoQueryPlan to accept null) so callers know a find-and-modify can yield null when the filter matches nothing; update any related type references or factory usage for FindOneAndUpdateCommand and FindOneAndDeleteCommand to accommodate the nullable result type.packages/2-mongo-family/5-query-builders/query-builder/src/builder.ts (1)
797-811: 🛠️ Refactor suggestion | 🟠 MajorRemove the
PipelineBuilderbackward-compatibility alias.Per coding guidelines: "Do not add backward-compatibility shims" and "Remove old code entirely instead of preserving it with backward-compatibility shims." The
PipelineBuildertype alias and const assignment re-expose the deprecated name as a public API.♻️ Suggested removal
-/** - * Backwards-compatible alias for the previous read-only builder class. New - * call sites should reference `PipelineChain` (or the higher-level - * `CollectionHandle` / `FilteredCollection`) directly. - * - * `@deprecated` Use `PipelineChain` (or `CollectionHandle` / `FilteredCollection`) - * directly. Retained for the M0/M1 transition; will be removed once all - * internal references are migrated. - */ -export type PipelineBuilder< - TContract extends MongoContractWithTypeMaps<MongoContract, MongoTypeMaps>, - Shape extends DocShape, -> = PipelineChain<TContract, Shape, 'compat', 'compat'>; - -export const PipelineBuilder = PipelineChain;Also remove the re-export from
packages/2-mongo-family/5-query-builders/query-builder/src/exports/index.tsand update any internal call sites to usePipelineChaindirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/src/builder.ts` around lines 797 - 811, Remove the backwards-compatibility alias by deleting the PipelineBuilder declarations (the type alias and the const assignment) so only PipelineChain remains; also remove any export of PipelineBuilder from the package's exports/index.ts and update all internal call sites that reference PipelineBuilder to use PipelineChain directly (search for the symbol "PipelineBuilder" across the repo and replace usages with "PipelineChain", ensuring generic parameters and import paths still match).
🧹 Nitpick comments (1)
examples/mongo-demo/test/query-builder-writes.test.ts (1)
148-156: Consider usingtoMatchObjectfor related assertions.Lines 150-152 use separate
expect().toBe()calls forbioandname. Per coding guidelines, prefer object matchers when checking 2+ related values.♻️ Suggested refactor
- const doc = rows[0] as Record<string, unknown>; - expect(doc.bio).toBe('original'); - expect(doc.name).toBe('Alice'); + expect(rows[0]).toMatchObject({ bio: 'original', name: 'Alice' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/mongo-demo/test/query-builder-writes.test.ts` around lines 148 - 156, Replace the two separate assertions on the returned document's fields with a single object matcher: after obtaining rows via runtime.execute(plan) and casting rows[0] to doc, use expect(doc).toMatchObject({ name: 'Alice', bio: 'original' }) instead of expect(doc.bio).toBe('original') and expect(doc.name).toBe('Alice'); keep the length check and the persisted usersCol().findOne assertion as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/2-mongo-family/5-query-builders/query-builder/src/builder.ts`:
- Around line 797-811: Remove the backwards-compatibility alias by deleting the
PipelineBuilder declarations (the type alias and the const assignment) so only
PipelineChain remains; also remove any export of PipelineBuilder from the
package's exports/index.ts and update all internal call sites that reference
PipelineBuilder to use PipelineChain directly (search for the symbol
"PipelineBuilder" across the repo and replace usages with "PipelineChain",
ensuring generic parameters and import paths still match).
In `@packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts`:
- Around line 479-513: The return type of both methods findOneAndUpdate and
findOneAndDelete should include the possibility of no-match by allowing null;
update their signatures to return MongoQueryPlan<ModelToDocShape<TContract,
ModelName> | null> (or adjust the contained command/result type used by
MongoQueryPlan to accept null) so callers know a find-and-modify can yield null
when the filter matches nothing; update any related type references or factory
usage for FindOneAndUpdateCommand and FindOneAndDeleteCommand to accommodate the
nullable result type.
---
Nitpick comments:
In `@examples/mongo-demo/test/query-builder-writes.test.ts`:
- Around line 148-156: Replace the two separate assertions on the returned
document's fields with a single object matcher: after obtaining rows via
runtime.execute(plan) and casting rows[0] to doc, use
expect(doc).toMatchObject({ name: 'Alice', bio: 'original' }) instead of
expect(doc.bio).toBe('original') and expect(doc.name).toBe('Alice'); keep the
length check and the persisted usersCol().findOne assertion as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4f37e528-e4a5-4f3e-9d69-72e9bb81ae97
⛔ Files ignored due to path filters (2)
projects/mongo-pipeline-builder/plans/query-builder-unification-followups-plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/query-builder-unification-plan.mdis excluded by!projects/**
📒 Files selected for processing (13)
examples/mongo-demo/test/query-builder-writes.test.tspackages/2-mongo-family/4-query/query-ast/src/commands.tspackages/2-mongo-family/4-query/query-ast/src/raw-commands.tspackages/2-mongo-family/5-query-builders/query-builder/src/builder.tspackages/2-mongo-family/5-query-builders/query-builder/src/field-accessor.tspackages/2-mongo-family/5-query-builders/query-builder/src/state-classes.tspackages/2-mongo-family/5-query-builders/query-builder/src/update-ops.tspackages/2-mongo-family/5-query-builders/query-builder/test/find-and-modify.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/pipeline-updates.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/state-machine-surface.test-d.tspackages/2-mongo-family/6-transport/mongo-wire/src/wire-commands.tspackages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.tspackages/3-mongo-target/3-mongo-driver/src/mongo-driver.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/find-and-modify.test.ts
- packages/2-mongo-family/6-transport/mongo-wire/src/wire-commands.ts
- packages/2-mongo-family/4-query/query-ast/src/commands.ts
- packages/2-mongo-family/5-query-builders/query-builder/src/update-ops.ts
Contributor
Author
|
Addressed review feedback in 5304fdf: Will address (done):
Won't address:
Not actionable:
|
Capture TML-2267 design decisions: rename @prisma-next/mongo-pipeline-builder to @prisma-next/mongo-query-builder; split the single PipelineBuilder into a three-state machine (CollectionHandle, FilteredCollection, PipelineChain) with phantom UpdateCompat / FindAndModifyCompat markers; unify FieldProxy and FilterProxy into the ADR-180 accessor; add typed terminals for inserts, updates, deletes, upserts, find-and-modify, update-with-pipeline, and $merge/$out. Plan organises the work into six milestones (M0 rename through M5 close-out) with a per-criterion test coverage table.
… (M0) Mechanical rename ahead of the query-builder unification (TML-2267): - packages/2-mongo-family/5-query-builders/pipeline-builder/ -> .../query-builder/ - @prisma-next/mongo-pipeline-builder -> @prisma-next/mongo-query-builder - mongoPipeline() -> mongoQuery(); PipelineRoot -> QueryRoot - src/pipeline.ts -> src/query.ts - @prisma-next/mongo extension: pipeline: -> query: on MongoClient/ConnectedMongoClient - Update runtime tests, integration tests, examples, migration-authoring docs, Package-Layering, Architecture Overview, Naming Conventions Read-side surface is unchanged. typecheck/lint/lint:deps clean; mongo-query-builder (271 tests) and mongo-runtime (51 tests) suites pass.
…d field accessor (M1) Splits the read-only PipelineBuilder into a three-state state machine — CollectionHandle, FilteredCollection, PipelineChain — gated by phantom UpdateCompat / FindAndModifyCompat markers per the spec marker table. CollectionHandle and FilteredCollection extend PipelineChain so the existing read-side surface keeps working unchanged. Also replaces the separate FieldProxy and FilterProxy with a single ADR-180 unified FieldAccessor that exposes filter operators, the full set of update operators, and is callable as a dot-path accessor. Adds TypedUpdateOp + foldUpdateOps for the M2 write terminals. State-machine type tests assert transitions and the marker table for representative pipeline-stage methods.
CollectionHandle gains the insert/unqualified-write surface (insertOne, insertMany, updateAll, deleteAll). FilteredCollection gains the filtered-write surface (updateMany, updateOne, deleteMany, deleteOne) which AND-folds the accumulated filters into the wire commands filter slot. All terminals produce a MongoQueryPlan wrapping the existing AST commands (already supported end-to-end by mongo-adapter). Update terminals consume TypedUpdateOp callbacks and fold them via foldUpdateOps, which throws on operator+path collisions. Tautological match-all is represented as _id $exists: true pending the Open Item #2 decision; centralised so the eventual switch is a one-line change. Adds writes.test.ts asserting AST emission for every terminal and state-machine-surface.test-d.ts asserting that insert/unqualified-write methods are absent from FilteredCollection and filtered-write methods are absent from CollectionHandle.
- Extend `UpdateOneCommand`/`UpdateManyCommand` and the corresponding wire
commands with an `upsert` flag; thread it through the adapter and driver.
`Update{One,Many}Result` gains optional `upsertedCount`/`upsertedId`.
- `FilteredCollection.findOneAndUpdate(updaterFn, opts?)` and
`.findOneAndDelete()` produce `FindOneAndUpdate{,Delete}Command` over the
AND-folded filter. `returnDocument` is hardcoded `after` at the driver
layer; making it caller-controllable is deferred.
- `CollectionHandle.upsertOne(filterFn, updaterFn)` and
`FilteredCollection.upsertOne(updaterFn)` produce `UpdateOneCommand` with
`upsert: true`. `upsertMany` deferred per the design discussion.
- Type tests assert find-and-modify is absent on `CollectionHandle`.
Deferred from the M3 scope:
- `PipelineChain<…, _, compat>.findOneAndUpdate/findOneAndDelete` — needs
`sort`/`skip` slots on the find-and-modify AST/wire commands.
- `mongo-memory-server` integration coverage — to land in the wider
M2/M3/M4 integration sweep.
- `PipelineChain.merge(opts)` and `.out(coll)` now return `MongoQueryPlan<unknown>` (lane `mongo-write`) directly rather than another `PipelineChain`. The `$merge`/`$out` stage is appended internally so the wire `AggregateCommand` ends with the terminal stage as Mongo requires. - Existing tests updated to drop the trailing `.build()` and assert the new lane. Deferred from M4: - Pipeline-stage emitters on `FieldAccessor` (`f.set`, `f.replaceRoot`, ...) for the update-with-pipeline form (4.1/4.2). - `PipelineChain<…, compat, _>.updateMany()` / `.updateOne()` no-arg consumption form (4.3) — marker gating already in place. - `mongo-memory-server` integration coverage (4.6) — to land alongside the wider M2/M3/M4 integration sweep.
`mongoQuery(...).rawCommand(cmd)` packages any `AnyMongoCommand` (typed CRUD or `RawMongoCommand` of `Document`s) into a `MongoQueryPlan<unknown>` with `lane: mongo-raw`. The contract `storageHash` is validated eagerly inside `rawCommand` so the type-test surface (which passes an `unknown` contract for shape-only checks) continues to typecheck without a runtime guard fight. Deferred to the close-out PR: - mongo-memory-server integration coverage for M2.7/M3.6/M4.6. - Long-lived design content migration into the subsystem doc. - `projects/mongo-pipeline-builder/` retirement.
…ocument (F1) Add sort, skip, and returnDocument fields to FindOneAndUpdate/Delete commands across the AST, raw-commands, wire-commands, adapter, and driver layers. All fields default so existing call sites stay green. The driver conditionally spreads sort into MongoDB options (skip is carried on the wire command for F2 chain-deconstruction validation but is not passed to the MongoDB driver since findAndModify does not support skip at the wire level). Surface returnDocument on FilteredCollection.findOneAndUpdate opts.
…gating (F2) Add findOneAndUpdate/findOneAndDelete to PipelineChain, gated by the FindAndModifyCompat marker via the this-parameter idiom. Stages that clear the marker (group, limit, addFields, project, unwind, etc.) make these terminals invisible at the type level. Chain deconstruction walks accumulated stages, validates they are all $match/$sort/$skip, AND-folds matches into a filter, and threads sort/skip into the wire-command slots. Brand fields (__updateCompat, __findAndModifyCompat) anchor the phantom type parameters so structural typing respects the markers. Extract foldUpdateOps into builder.ts imports to avoid a cyclic dep between builder.ts and state-classes.ts.
…terminals (F3) Add f.stage.set/unset/replaceRoot/replaceWith emitters to FieldAccessor for producing MongoUpdatePipelineStage nodes in updater callbacks. Add resolveUpdaterResult dispatcher that classifies updater arrays as either TypedUpdateOp (folded into operator form) or pipeline stages (passed through as-is), throwing on mixed arrays. Add no-arg updateMany()/updateOne() on PipelineChain (gated on U=compat) that deconstruct leading $match stages into a filter and validate remaining stages are valid update-pipeline stages ($addFields, $project, $replaceRoot).
14 end-to-end tests against mongo-memory-server covering M2 CRUD, M3 find-and-modify/upserts, and M4 pipeline-style updates plus merge/out. Reuses the existing mongo-demo harness rather than standing up a new one. Updates both the follow-ups plan and parent plan to mark F1–F4 tasks as completed.
- Remove deprecated PipelineBuilder alias from public surface - Add | null to findOneAndUpdate/findOneAndDelete return types - Replace transient projects/ spec link in README with stable path - Refactor GetU/GetF helper types to use bare infer (drop biome-ignore) - Rename pipeline → query in mongo-demo and retail-store examples - Use validated contract in mongo extension runtime - Use toMatchObject in integration test assertions
- Add MongoFieldFilter.nin() factory; use it in field-accessor instead of raw constructor - Refactor deconstructUpdateChain loop from indexed for to for…of - Improve PipelineChain.updateMany/updateOne error messages to explain the no-arg form - Delete orphan FieldProxy/FilterProxy/FilterHandle types and their re-exports
UpdateCompat → UpdateEnabled (values: update-ok | update-cleared) FindAndModifyCompat → FindAndModifyEnabled (values: fam-ok | fam-cleared) The old shared literal "compat" was ambiguous in hovers and error messages where both slots appeared as the same string. The new per-slot literals are greppable and self-locating without counting comma positions.
Aligns with the repo-wide package-per-lane convention (mongo-orm uses mongo-orm, sql-family uses sql). The three-lane split (mongo-pipeline, mongo-write, mongo-raw) duplicated intent information already available on the plan command discriminated union.
- Extract result interfaces to result-types.ts (breaks circular import) - PipelineChain.updateMany/updateOne return UpdateResult instead of unknown - PipelineChain.findOneAndUpdate/findOneAndDelete return ResolveRow|null - FilteredCollection find-and-modify returns ResolveRow|null (matching parent) - Add UpdaterResult type alias (union of homogeneous arrays) so mixed-shape updaters are a compile error, not just a runtime one - Add @ts-expect-error rows for no-arg updateMany/updateOne unavailability after group/limit/sort stages
upsertMany is explicitly non-goal: MongoDB updateMany with upsert:true inserts at most one document, making the name misleading. Also updates all marker references in the spec to use the new UpdateEnabled / FindAndModifyEnabled naming with self-identifying value literals.
Result interfaces (InsertOneResult, UpdateResult, DeleteResult, etc.) are part of the primitive language definition and belong in the query layer, not the transport layer. The transport layer (mongo-wire) now re-exports from mongo-query-ast rather than owning the definitions. Deletes the intermediate result-types.ts from query-builder.
Thread the specific command type through every terminal signature so plan.command is narrowly typed without casts. The second type parameter defaults to AnyMongoCommand, so existing consumers are unaffected.
Now that MongoQueryPlan carries the Command type parameter, plan.command is narrowly typed at each terminal. The casts that were needed before are redundant and their removal is the regression guard for the new type parameter.
Replaces the read-only-focused README with a complete guide covering the three-state machine, write terminals, field accessor, update operators, pipeline-style updates, marker gating, rawCommand, and observability. Reference tables for fn/acc helpers migrate to DEVELOPING.md scope.
wmadden
force-pushed
the
tml-2267-query-builder-unification
branch
from
4e98ce8 to
7188cb1
Compare
Use query instead of pipeline in setupTestDb so Db matches createClient.
out() and merge() already return MongoQueryPlan; calling build() was a type error.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts (1)
280-286: Redundant defensive check after length validation.Lines 280-282 already ensure
filters.length > 0, making thefilters[0] === undefinedcheck at lines 284-286 unreachable. TypeScript can't narrow array element access after length checks, so the guard is harmless, but consider using a non-null assertion instead:const first = this.#filters[0]!;That said, the explicit defensive check is acceptable as defense-in-depth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts` around lines 280 - 286, The code in FilteredCollection redundantly checks filters[0] after already throwing when filters.length === 0; replace the defensive runtime check by using a non-null assertion when reading the first element (e.g. assign first from this.#filters[0]!) or otherwise rely on the prior length check to avoid the unreachable throw; update the reference to the private array (this.#filters) and the variable first in the constructor/initializer so TypeScript knows the element is non-null without the extra runtime branch.packages/2-mongo-family/5-query-builders/query-builder/test/builder-new-stages.test.ts (1)
47-60: Consider consolidating related assertions withtoMatchObject.Per coding guidelines, prefer object matchers over multiple individual
expect().toBe()calls when checking 2+ related values. Theout()test could consolidate:expect(plan).toMatchObject({ meta: { lane: 'mongo-query' }, command: { pipeline: expect.arrayContaining([expect.any(MongoOutStage)]) }, }); expect((plan.command.pipeline[0] as MongoOutStage).collection).toBe('results');That said, the current assertions are clear and acceptable for terminal behavior tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/5-query-builders/query-builder/test/builder-new-stages.test.ts` around lines 47 - 60, Consolidate the related assertions in the out() test by using an object matcher: replace the multiple expects that check plan.meta.lane and that pipeline contains a MongoOutStage with a single expect(plan).toMatchObject({ meta: { lane: 'mongo-query' }, command: { pipeline: expect.arrayContaining([expect.any(MongoOutStage)]) } }) to assert structure, and keep a focused assertion for the MongoOutStage.collection (accessed via createOrdersBuilder().out(...).command.pipeline[0] as MongoOutStage) to verify 'results' (and similarly for the optional db parameter test assert stage.db === 'archive'); this uses the existing createOrdersBuilder, out, MongoOutStage, plan, and stage symbols to reduce repetitive checks while preserving intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-mongo-family/4-query/query-ast/src/raw-commands.ts`:
- Around line 105-107: The types RawFindOneAndUpdateCommand and
RawFindOneAndDeleteCommand currently expose a skip field that is never used by
the driver; remove the skip property from those command interfaces (and from any
other raw-find-one command types referenced around the same blocks) and delete
any related references/uses so callers can't pass a no-op option; ensure
findOneAndUpdate and findOneAndDelete keep using only returnDocument/upsert/sort
as already implemented (update type names: RawFindOneAndUpdateCommand,
RawFindOneAndDeleteCommand, and any related definitions at the nearby sections)
so the type surface matches the actual driver behavior.
---
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts`:
- Around line 280-286: The code in FilteredCollection redundantly checks
filters[0] after already throwing when filters.length === 0; replace the
defensive runtime check by using a non-null assertion when reading the first
element (e.g. assign first from this.#filters[0]!) or otherwise rely on the
prior length check to avoid the unreachable throw; update the reference to the
private array (this.#filters) and the variable first in the
constructor/initializer so TypeScript knows the element is non-null without the
extra runtime branch.
In
`@packages/2-mongo-family/5-query-builders/query-builder/test/builder-new-stages.test.ts`:
- Around line 47-60: Consolidate the related assertions in the out() test by
using an object matcher: replace the multiple expects that check plan.meta.lane
and that pipeline contains a MongoOutStage with a single
expect(plan).toMatchObject({ meta: { lane: 'mongo-query' }, command: { pipeline:
expect.arrayContaining([expect.any(MongoOutStage)]) } }) to assert structure,
and keep a focused assertion for the MongoOutStage.collection (accessed via
createOrdersBuilder().out(...).command.pipeline[0] as MongoOutStage) to verify
'results' (and similarly for the optional db parameter test assert stage.db ===
'archive'); this uses the existing createOrdersBuilder, out, MongoOutStage,
plan, and stage symbols to reduce repetitive checks while preserving intent.
🪄 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: 1b498810-33af-4fe4-b20f-2c98c25362d2
⛔ Files ignored due to path filters (7)
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-migrations.spec.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/query-builder-unification-followups-plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/query-builder-unification-plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/specs/query-builder-unification.spec.mdis excluded by!projects/**projects/one-package-install/plan.mdis excluded by!projects/**
📒 Files selected for processing (68)
docs/Architecture Overview.mddocs/architecture docs/Package-Layering.mddocs/reference/Package Naming Conventions.mdexamples/mongo-demo/package.jsonexamples/mongo-demo/src/db.tsexamples/mongo-demo/src/server.tsexamples/mongo-demo/test/query-builder-writes.test.tsexamples/retail-store/package.jsonexamples/retail-store/src/data/events.tsexamples/retail-store/src/data/products.tsexamples/retail-store/src/db.tsexamples/retail-store/test/setup.tspackages/2-mongo-family/4-query/query-ast/src/commands.tspackages/2-mongo-family/4-query/query-ast/src/exports/execution.tspackages/2-mongo-family/4-query/query-ast/src/filter-expressions.tspackages/2-mongo-family/4-query/query-ast/src/query-plan.tspackages/2-mongo-family/4-query/query-ast/src/raw-commands.tspackages/2-mongo-family/4-query/query-ast/src/result-types.tspackages/2-mongo-family/5-query-builders/pipeline-builder/README.mdpackages/2-mongo-family/5-query-builders/pipeline-builder/src/builder.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/exports/index.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/field-proxy.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/filter-proxy.tspackages/2-mongo-family/5-query-builders/pipeline-builder/src/pipeline.tspackages/2-mongo-family/5-query-builders/query-builder/DEVELOPING.mdpackages/2-mongo-family/5-query-builders/query-builder/README.mdpackages/2-mongo-family/5-query-builders/query-builder/package.jsonpackages/2-mongo-family/5-query-builders/query-builder/src/accumulator-helpers.tspackages/2-mongo-family/5-query-builders/query-builder/src/builder.tspackages/2-mongo-family/5-query-builders/query-builder/src/exports/index.tspackages/2-mongo-family/5-query-builders/query-builder/src/expression-helpers.tspackages/2-mongo-family/5-query-builders/query-builder/src/field-accessor.tspackages/2-mongo-family/5-query-builders/query-builder/src/markers.tspackages/2-mongo-family/5-query-builders/query-builder/src/query.tspackages/2-mongo-family/5-query-builders/query-builder/src/state-classes.tspackages/2-mongo-family/5-query-builders/query-builder/src/types.tspackages/2-mongo-family/5-query-builders/query-builder/src/update-ops.tspackages/2-mongo-family/5-query-builders/query-builder/test/accumulator-helpers.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/accumulator-helpers.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder-new-stages.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/builder.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/expression-helpers.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/expression-helpers.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/find-and-modify.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/fixtures/test-contract.tspackages/2-mongo-family/5-query-builders/query-builder/test/pipeline-updates.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/raw-command.test.tspackages/2-mongo-family/5-query-builders/query-builder/test/state-machine-surface.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/state-machine.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/types.test-d.tspackages/2-mongo-family/5-query-builders/query-builder/test/writes.test.tspackages/2-mongo-family/5-query-builders/query-builder/tsconfig.jsonpackages/2-mongo-family/5-query-builders/query-builder/tsconfig.prod.jsonpackages/2-mongo-family/5-query-builders/query-builder/tsdown.config.tspackages/2-mongo-family/5-query-builders/query-builder/vitest.config.tspackages/2-mongo-family/6-transport/mongo-wire/package.jsonpackages/2-mongo-family/6-transport/mongo-wire/src/results.tspackages/2-mongo-family/6-transport/mongo-wire/src/wire-commands.tspackages/2-mongo-family/7-runtime/package.jsonpackages/2-mongo-family/7-runtime/test/query-builder.test.tspackages/2-mongo-family/7-runtime/test/runtime-types.test-d.tspackages/3-extensions/mongo/package.jsonpackages/3-extensions/mongo/src/runtime/mongo.tspackages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.tspackages/3-mongo-target/3-mongo-driver/src/mongo-driver.tstest/integration/package.jsontest/integration/test/mongo/query-builder.test.ts
💤 Files with no reviewable changes (6)
- packages/2-mongo-family/5-query-builders/pipeline-builder/README.md
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/field-proxy.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/builder.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/pipeline.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/exports/index.ts
- packages/2-mongo-family/5-query-builders/pipeline-builder/src/filter-proxy.ts
✅ Files skipped from review due to trivial changes (17)
- examples/mongo-demo/package.json
- docs/reference/Package Naming Conventions.md
- packages/3-extensions/mongo/package.json
- packages/2-mongo-family/6-transport/mongo-wire/package.json
- packages/2-mongo-family/7-runtime/package.json
- docs/Architecture Overview.md
- examples/retail-store/test/setup.ts
- examples/mongo-demo/src/server.ts
- test/integration/test/mongo/query-builder.test.ts
- packages/2-mongo-family/5-query-builders/query-builder/DEVELOPING.md
- packages/2-mongo-family/7-runtime/test/query-builder.test.ts
- packages/2-mongo-family/7-runtime/test/runtime-types.test-d.ts
- docs/architecture docs/Package-Layering.md
- test/integration/package.json
- packages/2-mongo-family/6-transport/mongo-wire/src/results.ts
- packages/2-mongo-family/4-query/query-ast/src/result-types.ts
- packages/2-mongo-family/5-query-builders/query-builder/src/exports/index.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- examples/retail-store/package.json
- packages/2-mongo-family/5-query-builders/query-builder/package.json
- packages/3-mongo-target/3-mongo-driver/src/mongo-driver.ts
- examples/retail-store/src/db.ts
- packages/3-mongo-target/2-mongo-adapter/src/mongo-adapter.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/builder.test.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/raw-command.test.ts
- packages/3-extensions/mongo/src/runtime/mongo.ts
- examples/retail-store/src/data/events.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/state-machine.test-d.ts
- packages/2-mongo-family/5-query-builders/query-builder/README.md
- examples/mongo-demo/test/query-builder-writes.test.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/pipeline-updates.test.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/state-machine-surface.test-d.ts
- examples/mongo-demo/src/db.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/writes.test.ts
- packages/2-mongo-family/4-query/query-ast/src/commands.ts
- packages/2-mongo-family/5-query-builders/query-builder/test/find-and-modify.test.ts
Comment on lines
+105
to
+107
| readonly sort: Record<string, 1 | -1> | undefined; | ||
| readonly skip: number | undefined; | ||
| readonly returnDocument: 'before' | 'after'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm skip is present in raw and wire command models"
rg -n --type=ts -C2 '\breadonly skip\b|constructor\(.+skip\?: number' \
packages/2-mongo-family/4-query/query-ast/src/raw-commands.ts \
packages/2-mongo-family/6-transport/mongo-wire/src/wire-commands.ts
echo
echo "2) Check mongo-driver execution paths for skip usage"
rg -n --type=ts -C4 'findOneAndUpdate|findOneAndDelete|cmd\.skip|skip:' \
packages/3-mongo-target/3-mongo-driver/src/mongo-driver.tsRepository: prisma/prisma-next
Length of output: 4158
Remove skip from find-and-modify command types or implement its handling in the driver.
skip is defined in both RawFindOneAndUpdateCommand (line 106) and RawFindOneAndDeleteCommand (line 135), but the MongoDB driver execution paths ignore it entirely:
findOneAndUpdatepassesreturnDocument,upsert, and conditionallysort, but never usescmd.skipfindOneAndDeletepasses only conditionalsort, ignoringcmd.skip
This creates a misleading API where callers can provide skip with no effect. Either remove skip from these command types (since MongoDB's single-document operations don't support it) or implement proper handling in the driver.
Also applies to: 114-116, 123-125, 134-135, 137-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-mongo-family/4-query/query-ast/src/raw-commands.ts` around lines
105 - 107, The types RawFindOneAndUpdateCommand and RawFindOneAndDeleteCommand
currently expose a skip field that is never used by the driver; remove the skip
property from those command interfaces (and from any other raw-find-one command
types referenced around the same blocks) and delete any related references/uses
so callers can't pass a no-op option; ensure findOneAndUpdate and
findOneAndDelete keep using only returnDocument/upsert/sort as already
implemented (update type names: RawFindOneAndUpdateCommand,
RawFindOneAndDeleteCommand, and any related definitions at the nearby sections)
so the type surface matches the actual driver behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/integration/test/mongo/query-builder.test.ts (2)
107-115: Optional: renamepipelinetoqueryfor DSL consistency.The code works, but the old local name now conflicts with the new DSL terminology and can cause minor cognitive friction.
♻️ Suggested rename
-const pipeline = mongoQuery<TContract>({ contractJson }); +const query = mongoQuery<TContract>({ contractJson }); function products() { - return pipeline.from('products'); + return query.from('products'); } function orders() { - return pipeline.from('orders'); + return query.from('orders'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/mongo/query-builder.test.ts` around lines 107 - 115, Rename the local variable pipeline to query to match the DSL and avoid naming friction: change the declaration const pipeline = mongoQuery<TContract>({ contractJson }) to const query = mongoQuery<TContract>({ contractJson }) and update all usages inside the helper functions (e.g., pipeline.from in products() and orders()) to use query.from instead; ensure tests import/expectations refer to the new local name if any references exist.
169-740: Split this test file; it exceeds the 500-line limit.This file is now 740 lines and should be split by existing functional
describegroups (e.g., basic flow, aggregation, write terminals, advanced stages) to keep navigation and maintenance manageable.As per coding guidelines, “Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/mongo/query-builder.test.ts` around lines 169 - 740, Current test file exceeds 500 lines; split it into multiple smaller test files by grouping related describe blocks (e.g., "match + sort + limit + skip" for basic pipeline flow, "group with accumulators" and other aggregation tests, "out" and "merge" as write-terminal tests, and advanced stages like "graphLookup", "setWindowFields", "redact", etc.). Extract the shared helpers seed and exec and the describeWithMongoDB invocation into a small test helper module so each new file can import them; move each logical describe(...) block (refer to names like "match + sort + limit + skip", "group with accumulators", "addFields", "project", "count", "unwind", "lookup", "facet", "bucket", "unionWith", "sortByCount", "sample", "out", "merge", "bucketAuto", "redact", "graphLookup", "setWindowFields", "complex pipeline", "named-args expression helpers", "named-args accumulator helpers") into its own test file until each file is under 500 lines, and update imports in those files to use the extracted seed/exec helpers and describeWithMongoDB.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/integration/test/mongo/query-builder.test.ts`:
- Around line 107-115: Rename the local variable pipeline to query to match the
DSL and avoid naming friction: change the declaration const pipeline =
mongoQuery<TContract>({ contractJson }) to const query = mongoQuery<TContract>({
contractJson }) and update all usages inside the helper functions (e.g.,
pipeline.from in products() and orders()) to use query.from instead; ensure
tests import/expectations refer to the new local name if any references exist.
- Around line 169-740: Current test file exceeds 500 lines; split it into
multiple smaller test files by grouping related describe blocks (e.g., "match +
sort + limit + skip" for basic pipeline flow, "group with accumulators" and
other aggregation tests, "out" and "merge" as write-terminal tests, and advanced
stages like "graphLookup", "setWindowFields", "redact", etc.). Extract the
shared helpers seed and exec and the describeWithMongoDB invocation into a small
test helper module so each new file can import them; move each logical
describe(...) block (refer to names like "match + sort + limit + skip", "group
with accumulators", "addFields", "project", "count", "unwind", "lookup",
"facet", "bucket", "unionWith", "sortByCount", "sample", "out", "merge",
"bucketAuto", "redact", "graphLookup", "setWindowFields", "complex pipeline",
"named-args expression helpers", "named-args accumulator helpers") into its own
test file until each file is under 500 lines, and update imports in those files
to use the extracted seed/exec helpers and describeWithMongoDB.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f6dbe063-35c9-4b0b-afe4-fe96035138da
📒 Files selected for processing (2)
examples/retail-store/test/setup.tstest/integration/test/mongo/query-builder.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/retail-store/test/setup.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests