Conversation
…artifacts Extract consolidated framework limitations from code reviews into a standalone document at projects/mongo-example-apps/framework-limitations.md. Covers type ergonomics (FL-01–FL-03), query capabilities (FL-04–FL-08), schema/migration gaps (FL-09–FL-12), and missing capabilities (FL-13–FL-15). Tracks 6 items addressed by this branch (FL-A1–FL-A6). Also commits the PR #327 review artifacts (system-design-review, code-review, walkthrough), the round 2 spec, and the round 2 plan.
Review artifacts under projects/**/reviews/ are useful during development but should not ship in PRs. Added a gitignore rule to prevent re-addition.
The where() codec gap applies to any field whose codec has a non-identity encode, not just ObjectId. Updated framework-limitations.md and next-steps.md to reflect the corrected root cause and scope.
Captures design decisions from discussion: codec-generic where() encoding, full ADR 180 field accessor with dot-path traversal, 1:N back-relation test coverage, and retail store cleanup scope.
4 milestones: 1:N back-relation tests, codec-aware where(), ADR 180 field accessor with dot-path traversal, retail store cleanup. All acceptance criteria mapped to unit or type tests.
Add User.tasks 1:N reference relation to the ORM test fixture and verify that include() produces $lookup without $unwind. Type tests confirm the return type for 1:N included relations is an array.
where() now accepts a plain object { fieldName: value } in addition to
MongoFilterExpr. Values are encoded through codecs automatically via
the field codecId from the contract. Multiple fields produce an AND
of equality filters. Chainable with MongoFilterExpr where() calls.
Also fixes MongoWhereFilter type to correctly navigate the contract
field type structure (field.type.codecId instead of field.codecId).
Add value objects (HomeAddress), numeric field (loginCount), and array field (tags) to the ORM test fixture for testing mutation operators and dot-path access. Implement createFieldAccessor() with Proxy-based field expressions supporting $set, $unset, $inc, $mul, $push, $pull, $addToSet, $pop. Add compile-time DotPath and ResolveDotPathType types for type-safe dot-path access into value objects (ADR 180).
update(), updateAll(), updateCount(), and upsert() now accept a callback (u => [...ops]) that uses the Proxy-based field accessor to produce typed mutation operators ($set, $inc, $push, etc.). Field operations are compiled into a grouped update document and codec-encoded using the contract field definitions. Dot-path access resolves codecs through value object field chains.
Export FieldAccessor, FieldExpression, FieldOperation, DotPath, ResolveDotPathType, UpdateOperator types and createFieldAccessor from the ORM package public API.
…mics
Replace all objectIdEq() calls with object-based where({ field: value })
in carts, orders, products, users, and invoices data files.
Replace db.raw addToCart/removeFromCart/updateOrderStatus with ORM
callback-based update() and upsert() using field accessor operators
($push, $pull). Delete object-id-filter.ts.
Fix upsert $setOnInsert to exclude fields referenced by any update
operator (not just $set), preventing MongoDB path conflicts.
…ec output types change
…rn type, add updateAll callback test Address code review findings F02, F03, F05: - F02: skip codec wrapping for $unset sentinel values - F03: add missing updateAll() callback form test - F05: return Record<string, Record<string, MongoValue>> from compileFieldOperations, eliminating unsound cast
Addresses code review finding F06: all three features now have integration tests against a real MongoDB instance (MongoMemoryReplSet) proving codec encoding, BSON round-trip, and pipeline correctness.
… with a kind field
#isFilterExpr used a structural check ("kind" in filter) which would
misroute user data objects that legitimately have a "kind" field.
Replace with a symbol brand on the MongoFilterExpression base class
that plain objects can never carry.
FieldExpression<T> now intersects with NumericOps only when T extends number, so calling inc()/mul() on a string field is a type error. The runtime implementation still carries all methods via RuntimeFieldExpression; gating is purely at the type level.
Dot-path operations like u("homeAddress.city").set("LA") conflict with
$setOnInsert on the insert path: MongoDB rejects overlapping paths, and
excluding the parent field from $setOnInsert creates an incomplete
document. Add a runtime guard that throws with a clear error message
when dot-path operations are detected in upsert callbacks.
📝 WalkthroughWalkthroughAdds codec-aware object-shaped Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Collection as ORM.Collection
participant Accessor as FieldAccessor
participant Compiler as compileFieldOperations
participant Codec as Codec/Wrap
participant Driver as MongoDriver
participant DB as MongoDB
Client->>Collection: update((u) => { ... })
Collection->>Accessor: createFieldAccessor() and invoke callback
Accessor-->>Collection: returns FieldOperation[]
Collection->>Compiler: compileFieldOperations(ops, wrapValue)
Compiler->>Codec: wrap values (attach codecId / MongoParamRef)
Codec-->>Driver: send operator-grouped update document
Driver->>DB: execute update command
DB-->>Driver: ack/result
Driver-->>Collection: result
Collection-->>Client: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@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/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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts (1)
1-1080: Consider splitting this test file.This test file is now ~1080 lines, exceeding the 500-line guideline. Consider splitting by functionality into separate files such as:
collection.chaining.test.tscollection.where.test.tscollection.mutations.test.tscollection.field-accessor.test.ts🤖 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/orm/test/collection.test.ts` around lines 1 - 1080, The file is too large; split the big test suite into multiple focused test files by moving the describe blocks into new files (e.g., move the "MongoCollection chaining" block to collection.chaining.test.ts, the "MongoCollection object-based where()" plus related where chaining tests to collection.where.test.ts, the "createFieldAccessor()" and "compileFieldOperations()" blocks to collection.field-accessor.test.ts, and the large "MongoCollection write methods" block (including create/createAll/update/updateAll/upsert/delete/*mutation-related* tests) to collection.mutations.test.ts); preserve imports used across blocks (MongoFieldFilter, MongoParamRef, createMongoCollection, createFieldAccessor, compileFieldOperations, createMockExecutor helper and the contract/defaultUserData), and ensure each new file exports/uses the same helper functions (move createMockExecutor and shared constants to a small test-utils module or duplicate them minimally) so tests compile and run unchanged aside from file splits.
🤖 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/orm/src/collection.ts`:
- Around line 351-364: Callback-based updates currently allow modifying _id
because compileFieldOperations invoked from callback paths doesn't reject _id
like object-based updates do via `#toSetFields`; update the `#resolveUpdateDoc`
method to inspect the compiled FieldOperation[] (from FieldAccessor callbacks)
and throw the same error string ('Mutation payloads cannot modify `_id`') if any
operation targets _id, thereby matching the protection provided for object
updates in `#toSetFields` and covering the update, updateAll, and updateCount
callback flows.
---
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts`:
- Around line 1-1080: The file is too large; split the big test suite into
multiple focused test files by moving the describe blocks into new files (e.g.,
move the "MongoCollection chaining" block to collection.chaining.test.ts, the
"MongoCollection object-based where()" plus related where chaining tests to
collection.where.test.ts, the "createFieldAccessor()" and
"compileFieldOperations()" blocks to collection.field-accessor.test.ts, and the
large "MongoCollection write methods" block (including
create/createAll/update/updateAll/upsert/delete/*mutation-related* tests) to
collection.mutations.test.ts); preserve imports used across blocks
(MongoFieldFilter, MongoParamRef, createMongoCollection, createFieldAccessor,
compileFieldOperations, createMockExecutor helper and the
contract/defaultUserData), and ensure each new file exports/uses the same helper
functions (move createMockExecutor and shared constants to a small test-utils
module or duplicate them minimally) so tests compile and run unchanged aside
from file splits.
🪄 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: 8f93eca9-2c82-45d1-8dea-1a803205cce2
⛔ Files ignored due to path filters (5)
projects/mongo-example-apps/framework-limitations.mdis excluded by!projects/**projects/mongo-example-apps/plans/orm-query-mutation-ergonomics-plan.mdis excluded by!projects/**projects/mongo-example-apps/plans/retail-store-round-2-plan.mdis excluded by!projects/**projects/mongo-example-apps/specs/orm-query-mutation-ergonomics.spec.mdis excluded by!projects/**projects/mongo-example-apps/specs/retail-store-round-2.spec.mdis excluded by!projects/**
📒 Files selected for processing (22)
docs/planning/mongo-target/next-steps.mdexamples/retail-store/src/data/carts.tsexamples/retail-store/src/data/invoices.tsexamples/retail-store/src/data/object-id-filter.tsexamples/retail-store/src/data/orders.tsexamples/retail-store/src/data/products.tsexamples/retail-store/src/data/users.tsexamples/retail-store/test/order-lifecycle.test.tspackages/2-mongo-family/1-foundation/mongo-contract/test/fixtures/orm-contract.d.tspackages/2-mongo-family/1-foundation/mongo-contract/test/fixtures/orm-contract.jsonpackages/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/test/filter-expressions.test.tspackages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/src/exports/index.tspackages/2-mongo-family/5-query-builders/orm/src/field-accessor.tspackages/2-mongo-family/5-query-builders/orm/src/types.tspackages/2-mongo-family/5-query-builders/orm/test/collection.test.tspackages/2-mongo-family/5-query-builders/orm/test/integration/orm-ergonomics.test.tspackages/2-mongo-family/5-query-builders/orm/test/integration/polymorphism.test.tspackages/2-mongo-family/5-query-builders/orm/test/orm-types.test-d.tspackages/2-mongo-family/7-runtime/test/orm-contract-types.test-d.ts
💤 Files with no reviewable changes (1)
- examples/retail-store/src/data/object-id-filter.ts
…essor Replace non-null assertions with nullish-coalescing fallbacks and explicit local variables to satisfy biome strict mode.
Object-based updates reject _id via #toSetFields, but callback-based updates (update, updateAll, updateCount, upsert) bypassed that check. Add _id validation in #resolveUpdateDoc and the upsert callback path so u._id.set() throws the same error as object mutations.
Contributor
Author
|
Addressed all review findings:
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/2-mongo-family/5-query-builders/orm/src/collection.ts (1)
460-473:⚠️ Potential issue | 🟠 MajorReject
_idin callback mutations too.
#toSetFields()blocks_idfor plain objects, but these callback branches compile arbitraryFieldOperation[]directly. A callback can still target_idhere, so the new overloads no longer enforce the same invariant as the existing object path. Add the same top-level_idcheck beforecompileFieldOperations().Also applies to: 641-652
🤖 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/orm/src/collection.ts` around lines 460 - 473, The callback-based update branch in upsert() (the path using createFieldAccessor, ops, and compileFieldOperations with this.#wrapFieldOpValue) currently allows operations targeting the top-level _id field; add the same top-level _id rejection that `#toSetFields` enforces before calling compileFieldOperations so `_id` updates are rejected in callback mutations, and apply the identical fix to the second callback branch (the similar compileFieldOperations usage around the other upsert/update handling) to ensure consistency.
🤖 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/orm/src/collection.ts`:
- Around line 646-652: When the callback path (dataOrCallback function) produces
no operations, normalize that result to match the object overload by handling
empty ops returned from compileFieldOperations; inside the branch that calls
createFieldAccessor<TContract, ModelName>() and compileFieldOperations(...
this.#wrapFieldOpValue ...), check if the generated ops is empty (e.g., length
=== 0 or deep-empty) and then return the same no-op shape used by the object
overload (for example { $set: {} }) or throw a clear error; update the behavior
in the dataOrCallback branch so update(), updateAll(), and updateCount() see
identical no-op semantics as the object overload.
- Around line 656-692: The current `#wrapFieldOpValue` and `#wrapDotPathValue` only
re-wrap scalar leaves, so when an operation value is a value-object (e.g.,
callback payloads for $set/$push/$pull) its nested scalar fields skip codec
wrapping; update both `#wrapFieldOpValue` and `#wrapDotPathValue` to detect when the
target field type is a valueObject and the provided value is an object (not a
MongoParamRef) and then pass that object through the same value-object wrapping
logic used elsewhere (call the existing `#wrapValueObject` or the same wrapping
routine used by `#wrapFieldValue`) so nested scalar fields get wrapped with their
codecIds; ensure you handle dot-path resolution in `#wrapDotPathValue` to obtain
the correct ContractValueObject (voDef) and invoke the wrapper before returning.
---
Duplicate comments:
In `@packages/2-mongo-family/5-query-builders/orm/src/collection.ts`:
- Around line 460-473: The callback-based update branch in upsert() (the path
using createFieldAccessor, ops, and compileFieldOperations with
this.#wrapFieldOpValue) currently allows operations targeting the top-level _id
field; add the same top-level _id rejection that `#toSetFields` enforces before
calling compileFieldOperations so `_id` updates are rejected in callback
mutations, and apply the identical fix to the second callback branch (the
similar compileFieldOperations usage around the other upsert/update handling) to
ensure consistency.
🪄 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: 928fccf1-aed1-4e90-bed4-d2220b9aed71
📒 Files selected for processing (2)
packages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/src/field-accessor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-mongo-family/5-query-builders/orm/src/field-accessor.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts (1)
1-1112: Consider splitting this test file in a follow-up.This file is 1112 lines, exceeding the 500-line guideline. Natural split boundaries exist:
collection.chaining.test.ts— chaining and terminal methodscollection.field-accessor.test.ts—createFieldAccessor()andcompileFieldOperations()testscollection.write.test.ts— create/update/delete methodsAs per coding guidelines, "Keep test files under 500 lines to maintain readability and navigability."
🤖 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/orm/test/collection.test.ts` around lines 1 - 1112, Test file is too large (>1100 lines); split into smaller files to follow the 500-line guideline. Create three new test files and move the described blocks: 1) collection.chaining.test.ts — move the top-level "MongoCollection chaining" and "MongoCollection terminal methods" describe blocks (and related tests that assert plan/stage compilation like all(), first()); 2) collection.field-accessor.test.ts — move the "createFieldAccessor()" and "compileFieldOperations()" describe blocks and their tests (functions: createFieldAccessor, compileFieldOperations, FieldAccessor, FieldOperation); 3) collection.write.test.ts — move all write-related describe blocks ("MongoCollection write methods", including create/createAll/createCount, update/updateAll/updateCount/upsert, include rejection on write terminals, _id rejection on update paths, undefined normalization, immutability) and their tests (refer to methods create, createAll, update, updateAll, updateCount, delete, deleteCount, upsert). Ensure each new file imports the same helpers used here (createMongoCollection, createMockExecutor, MongoFieldFilter, MongoParamRef, contract fixture, etc.), update import paths as needed, and run tests to verify no inter-file dependencies remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts`:
- Around line 1-1112: Test file is too large (>1100 lines); split into smaller
files to follow the 500-line guideline. Create three new test files and move the
described blocks: 1) collection.chaining.test.ts — move the top-level
"MongoCollection chaining" and "MongoCollection terminal methods" describe
blocks (and related tests that assert plan/stage compilation like all(),
first()); 2) collection.field-accessor.test.ts — move the
"createFieldAccessor()" and "compileFieldOperations()" describe blocks and their
tests (functions: createFieldAccessor, compileFieldOperations, FieldAccessor,
FieldOperation); 3) collection.write.test.ts — move all write-related describe
blocks ("MongoCollection write methods", including create/createAll/createCount,
update/updateAll/updateCount/upsert, include rejection on write terminals, _id
rejection on update paths, undefined normalization, immutability) and their
tests (refer to methods create, createAll, update, updateAll, updateCount,
delete, deleteCount, upsert). Ensure each new file imports the same helpers used
here (createMongoCollection, createMockExecutor, MongoFieldFilter,
MongoParamRef, contract fixture, etc.), update import paths as needed, and run
tests to verify no inter-file dependencies remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 052a950b-f64f-4d9b-a095-166e75bacc32
📒 Files selected for processing (2)
packages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/test/collection.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-mongo-family/5-query-builders/orm/src/collection.ts
…eld operations
Empty callback updates now produce { $set: {} } matching object-based update
behavior. Value-object payloads in callback mutations (set/push/pull) are now
recursively wrapped through their codecs, matching the wrapping applied to
object-based updates and create paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts (2)
29-35: Use a fixture factory instead of a shared mutable object.
defaultUserDatais reused across many tests and contains a mutable array (tags). A factory avoids hidden coupling and mutation bleed between tests.Refactor sketch
-const defaultUserData = { - name: 'Alice', - email: 'a@b.c', - loginCount: 0, - tags: [] as string[], - homeAddress: null, -}; +const createDefaultUserData = () => ({ + name: 'Alice', + email: 'a@b.c', + loginCount: 0, + tags: [] as string[], + homeAddress: null, +});-const result = await col.create(defaultUserData); +const result = await col.create(createDefaultUserData());Also applies to: 529-530, 578-580, 760-761, 1064-1065
🤖 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/orm/test/collection.test.ts` around lines 29 - 35, defaultUserData is a shared mutable fixture (contains tags: string[]) causing tests to leak state; replace it with a factory function (e.g., makeDefaultUserData or defaultUserDataFactory) that returns a fresh object each call and update all tests that reference defaultUserData to call the factory instead (also replace other occurrences of the same shared object). Ensure the factory clones/creates new primitives and a new array for tags so tests no longer share mutable state.
162-1112: Please split this test file by concern.This file is now very large and mixes multiple feature areas (chaining/read paths, includes, object-where, field-accessor unit tests, callback mutation guards). Splitting will improve reviewability and failure triage.
As per coding guidelines,
*.test.tsfiles should stay under 500 lines and be split by logical boundaries using descriptive names.🤖 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/orm/test/collection.test.ts` around lines 162 - 1112, The single large test file should be split into smaller files by concern: create separate test files for (1) "object-based where" and related MongoCollection read/terminal tests (describe 'MongoCollection object-based where()', 'MongoCollection variant()', 'MongoCollection include()', 'MongoCollection terminal methods'), (2) "field accessor & compileFieldOperations" (describe 'createFieldAccessor()', 'compileFieldOperations()'), (3) "write methods and mutation guards" (describe blocks under 'MongoCollection write methods', including create/createAll/update/updateAll/upsert/delete and related rejection/immutability tests), and (4) any "include rejection on write terminals" and "undefined/_id normalization" tests if not already in write group; for each new file move the matching describe(...) blocks (e.g., 'createFieldAccessor()', 'compileFieldOperations()', 'MongoCollection variant()', etc.) into their respective files, ensure shared helpers/fixtures like createMockExecutor, contract, defaultUserData, MongoFieldFilter, MongoParamRef are imported from the original test utils module, update any relative imports, and keep each resulting *.test.ts under ~500 lines so CI passes; do not change test logic—only relocate blocks and adjust imports/exports.
🤖 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/orm/test/collection.test.ts`:
- Around line 722-753: The test names claim they validate the "correct update
doc" but only assert row/modified counts; update the tests that use
createMockExecutor, createMongoCollection, MongoFieldFilter.eq,
col.updateAll(...) and col.updateCount(...) to also assert the actual generated
update document (available as executor.lastPlan) matches the expected Mongo
update shape (e.g., $inc/$set structure and field paths), or else change the
test names to reflect they only check counts; ensure assertions reference
executor.lastPlan and verify operator grouping/encoding for u.loginCount.inc(1)
and u.name.set('X').
---
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts`:
- Around line 29-35: defaultUserData is a shared mutable fixture (contains tags:
string[]) causing tests to leak state; replace it with a factory function (e.g.,
makeDefaultUserData or defaultUserDataFactory) that returns a fresh object each
call and update all tests that reference defaultUserData to call the factory
instead (also replace other occurrences of the same shared object). Ensure the
factory clones/creates new primitives and a new array for tags so tests no
longer share mutable state.
- Around line 162-1112: The single large test file should be split into smaller
files by concern: create separate test files for (1) "object-based where" and
related MongoCollection read/terminal tests (describe 'MongoCollection
object-based where()', 'MongoCollection variant()', 'MongoCollection include()',
'MongoCollection terminal methods'), (2) "field accessor &
compileFieldOperations" (describe 'createFieldAccessor()',
'compileFieldOperations()'), (3) "write methods and mutation guards" (describe
blocks under 'MongoCollection write methods', including
create/createAll/update/updateAll/upsert/delete and related
rejection/immutability tests), and (4) any "include rejection on write
terminals" and "undefined/_id normalization" tests if not already in write
group; for each new file move the matching describe(...) blocks (e.g.,
'createFieldAccessor()', 'compileFieldOperations()', 'MongoCollection
variant()', etc.) into their respective files, ensure shared helpers/fixtures
like createMockExecutor, contract, defaultUserData, MongoFieldFilter,
MongoParamRef are imported from the original test utils module, update any
relative imports, and keep each resulting *.test.ts under ~500 lines so CI
passes; do not change test logic—only relocate blocks and adjust
imports/exports.
🪄 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: c19b9c15-3ca4-49c6-8a57-ae6a9a0af31b
📒 Files selected for processing (1)
packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/2-mongo-family/5-query-builders/orm/src/collection.ts (1)
698-730: Dot-path traversal does not handle array indices.The current implementation treats all dot-path segments as field names. If a user specifies a path like
"items.0.name"(array index access), it will fail to resolve the field definition and return the value unwrapped.This is likely acceptable for now since array index paths are uncommon in typical MongoDB updates, but worth noting for future consideration.
🤖 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/orm/src/collection.ts` around lines 698 - 730, The traversal in `#wrapDotPathValue` treats every dot segment as an object field and fails on numeric array indices (e.g. "items.0.name"); update the for-loop in `#wrapDotPathValue` to detect numeric segments (e.g. /^\d+$/) and skip treating them as object keys (continue to next segment) so array indices don't break resolution, and additionally handle arrays of value objects by, when currentField?.type.kind === 'array' and the array item type is a valueObject, drilling into the array item type (set currentField to the array item definition) before attempting the voDef lookup so value-object wrapping still works for paths like "items.0.name". Ensure you reference currentField, parts[i], voDef, and use the existing `#wrapValueObject` for final wrapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/orm/src/collection.ts`:
- Around line 698-730: The traversal in `#wrapDotPathValue` treats every dot
segment as an object field and fails on numeric array indices (e.g.
"items.0.name"); update the for-loop in `#wrapDotPathValue` to detect numeric
segments (e.g. /^\d+$/) and skip treating them as object keys (continue to next
segment) so array indices don't break resolution, and additionally handle arrays
of value objects by, when currentField?.type.kind === 'array' and the array item
type is a valueObject, drilling into the array item type (set currentField to
the array item definition) before attempting the voDef lookup so value-object
wrapping still works for paths like "items.0.name". Ensure you reference
currentField, parts[i], voDef, and use the existing `#wrapValueObject` for final
wrapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a6f03164-0b0f-48b9-b6a9-a61898cfdf92
📒 Files selected for processing (2)
packages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/test/collection.test.ts
Symbol-based brands break under dual-package installs because each copy of the module creates its own unique symbol. A string-keyed, non-enumerable prototype property survives this scenario since string in-checks compare by value, not reference identity.
… type maps ResolveFieldType only handled scalar fields, causing value-object fields (e.g. homeAddress) to fall through to unknown. This silently accepted any value in set()/push()/pull() for those fields. Now uses ResolvedModelRow (ExtractMongoFieldOutputTypes with InferModelRow fallback), matching the same pattern as ResolvedOutputRow in types.ts. Value-object fields resolve to their concrete emitted types, restoring type safety.
…act types Use CartItemInput, OrderLineItemInput, StatusEntryInput, InvoiceLineItemInput, FieldInputTypes from contract.d.ts instead of inline object type literals. This ensures the data layer stays in sync with schema changes automatically and demonstrates the value of contract-emitted types.
wmadden
commented
Apr 15, 2026
| MongoFieldFilter, | ||
| MongoNotExpr, | ||
| MongoOrExpr, | ||
| mongoFilterBrand, |
Contributor
Author
There was a problem hiding this comment.
This should not be exported
…d definition Replace exported mongoFilterBrand constant with isMongoFilterExpr() predicate defined alongside the brand. The brand is now an implementation detail; consumers use the predicate instead of performing raw in-checks against an exported constant.
wmadden
commented
Apr 15, 2026
| const ops = input.update(accessor); | ||
| const idOp = ops.find((op) => op.field === '_id'); | ||
| if (idOp) { | ||
| throw new Error('Mutation payloads cannot modify `_id`'); |
Contributor
Author
There was a problem hiding this comment.
Do we have a better runtime error type/envelope defined? We need one, we can't throw raw errors on a meaningful user execution path
…oint The filter brand predicate is an implementation detail, not a public API. Move it from the execution barrel to a dedicated ./internal entry point so only internal consumers (mongo-orm) can access it.
The predicate belongs alongside the types it discriminates. Remove the bogus ./internal entry point — it was not a real access control boundary and had no precedent in this repo.
wmadden
commented
Apr 15, 2026
| currentField = partKey ? voDef.fields[partKey] : undefined; | ||
| } | ||
|
|
||
| if (currentField?.type.kind === 'scalar' && value instanceof MongoParamRef) { |
Contributor
Author
There was a problem hiding this comment.
DOn't use instanceof
wmadden
commented
Apr 15, 2026
Comment on lines
+721
to
+724
| const voDef = (this.#contract as { valueObjects?: Record<string, ContractValueObject> }) | ||
| .valueObjects?.[voName]; | ||
| if (voDef) { | ||
| return this.#wrapValueObject(raw as Record<string, unknown>, voDef); |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts (1)
1-1139: Test file exceeds 500-line limit.At ~1139 lines, this file exceeds the 500-line guideline for test files. The author acknowledged this will be tracked separately as a refactor. Consider splitting by functionality:
collection.chaining.test.ts- chaining/immutability testscollection.where.test.ts- object-based and filter-based where testscollection.mutations.test.ts- create/update/delete testscollection.field-accessor.test.ts- field accessor and compilation 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/orm/test/collection.test.ts` around lines 1 - 1139, The test file is too large (~1139 lines); split it into focused test files to meet the 500-line guideline by extracting related suites: move chaining/immutability tests (describe 'MongoCollection chaining' and the 'immutability' suite) into collection.chaining.test.ts; move object-based where tests (describe 'MongoCollection object-based where()' and related include/windowing rejection cases) into collection.where.test.ts; move all mutation and write-path tests (describe 'MongoCollection write methods' including create/createAll/createCount/update/updateAll/updateCount/upsert/delete/deleteCount and the include rejection/undefined/_id rejection/windowing rejection suites) into collection.mutations.test.ts; and move field-accessor/compileFieldOperations suites (describe 'createFieldAccessor()' and 'compileFieldOperations()') into collection.field-accessor.test.ts; ensure each new file retains imports for createMongoCollection, MongoFieldFilter, createFieldAccessor, compileFieldOperations, MongoParamRef, AsyncIterableResult and the contract fixture, and preserve individual it() blocks and any helper createMockExecutor function and defaultUserData so tests still reference createMongoCollection, MongoFieldFilter, createFieldAccessor, compileFieldOperations, and FieldAccessor correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/2-mongo-family/5-query-builders/orm/test/collection.test.ts`:
- Around line 1-1139: The test file is too large (~1139 lines); split it into
focused test files to meet the 500-line guideline by extracting related suites:
move chaining/immutability tests (describe 'MongoCollection chaining' and the
'immutability' suite) into collection.chaining.test.ts; move object-based where
tests (describe 'MongoCollection object-based where()' and related
include/windowing rejection cases) into collection.where.test.ts; move all
mutation and write-path tests (describe 'MongoCollection write methods'
including
create/createAll/createCount/update/updateAll/updateCount/upsert/delete/deleteCount
and the include rejection/undefined/_id rejection/windowing rejection suites)
into collection.mutations.test.ts; and move
field-accessor/compileFieldOperations suites (describe 'createFieldAccessor()'
and 'compileFieldOperations()') into collection.field-accessor.test.ts; ensure
each new file retains imports for createMongoCollection, MongoFieldFilter,
createFieldAccessor, compileFieldOperations, MongoParamRef, AsyncIterableResult
and the contract fixture, and preserve individual it() blocks and any helper
createMockExecutor function and defaultUserData so tests still reference
createMongoCollection, MongoFieldFilter, createFieldAccessor,
compileFieldOperations, and FieldAccessor correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 95c6aafb-0214-45d0-9620-18bb8d45f95a
📒 Files selected for processing (15)
examples/retail-store/src/data/carts.tsexamples/retail-store/src/data/events.tsexamples/retail-store/src/data/invoices.tsexamples/retail-store/src/data/orders.tsexamples/retail-store/src/data/users.tsexamples/retail-store/src/seed.tsexamples/retail-store/test/aggregation.test.tsexamples/retail-store/test/crud-lifecycle.test.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/test/filter-expressions.test.tspackages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/src/field-accessor.tspackages/2-mongo-family/5-query-builders/orm/test/collection.test.tspackages/2-mongo-family/5-query-builders/orm/test/orm-types.test-d.ts
✅ Files skipped from review due to trivial changes (1)
- examples/retail-store/test/aggregation.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/2-mongo-family/4-query/query-ast/src/exports/execution.ts
- examples/retail-store/src/data/users.ts
- examples/retail-store/src/data/orders.ts
- examples/retail-store/src/data/carts.ts
- packages/2-mongo-family/5-query-builders/orm/test/orm-types.test-d.ts
- packages/2-mongo-family/5-query-builders/orm/src/collection.ts
- examples/retail-store/src/data/invoices.ts
wmadden
deleted the
tml-2246-orm-query-and-mutation-ergonomics-fl-04-fl-06-fl-08
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
closes TML-2246
Intent
Three ORM ergonomics gaps in the Mongo family forced the retail store example to drop to raw MongoDB commands for common patterns: typed array/numeric mutations required
mongoRaw, ObjectId filter setup required a hand-rolled helper, and 1:N relation includes were incorrectly unwinding arrays. This PR closes all three gaps (FL-04, FL-06, FL-08) and removes all resulting workarounds from the example app.Key snippet
Change map
where()overload, callback-based update methods, upsert dot-path guardmongoFilterBrandsymbol onMongoFilterExpressionbase classmongoRawwithupdate(u => [...])callbackobjectIdEqwith objectwhere()MongoMemoryReplSetfor all three FLswhere(), field accessor operators, dot-path guardMongoWhereFilter,DotPath,FieldExpression<T>numeric constraintThe story
New
field-accessor.tsintroduces the typed mutation surface (FL-04). AFieldAccessor<TContract, ModelName>proxy exposes per-fieldFieldExpression<T>objects, each with typed operators:set,unset,push,pull,addToSet,pop, andinc/mul(restricted toT extends number). A callable signatureu("homeAddress.city")handles dot-path access into value objects, with the path type-checked at compile time via recursive template literal types (DotPath<TContract, ModelName>).compileFieldOperationsgroups the resultingFieldOperation[]by MongoDB operator key, passing each value through the codec'swrapValuecallback so encoding stays consistent withupdate(plainObject).MongoCollection.where()gains an object overload (FL-06). Passing a plain{ field: value }object compiles each key into aMongoFieldFilter.eqexpression, with values run through#wrapFieldValuefor codec encoding. The#isFilterExprguard is backed by aunique symbolbrand onMongoFilterExpressionrather than a structural'kind' in filtercheck, so user data objects with akindfield are never misrouted.update(),updateAll(),updateCount(), andupsert()accept callbacks. All four mutation methods now accept either a plain data object (existing path) or a(u: FieldAccessor) => FieldOperation[]callback. Theupsert()path additionally guards against dot-path operations in callbacks — MongoDB rejects overlapping paths between$setand$setOnInsert, which would silently produce incomplete documents on insert.FL-08 required no implementation changes — 1:N reference relation
$lookupwithout$unwindwas already correct. The gap was test coverage; the ORM fixture contract is enriched with atasks1:N relation and the integration suite proves it.Retail store cleanup.
object-id-filter.tsis deleted. All call sites acrosscarts.ts,orders.ts,products.ts,users.ts, andinvoices.tsuse objectwhere()orupdate(u => [...])callbacks. NomongoRawremains in any data access file.Behavior changes & evidence
Adds object-based
where()overload:where({ userId })encodes values through codecs and produces aMongoFieldFilter.eqper field.Adds
FieldAccessorcallback form toupdate(),updateAll(),updateCount(),upsert(): typed$set,$unset,$push,$pull,$addToSet,$pop,$inc,$mul— values codec-encoded.upsert()rejects dot-path operations in callbacks with a clear error naming the offending path. MongoDB rejects overlapping paths in$set/$setOnInsert.#isFilterExpruses a symbol brand:mongoFilterBrand in filterreplaces'kind' in filter.inc()/mul()restricted toT extends number: string-typed fields don't expose arithmetic operators at compile time.1:N
include()test coverage added (FL-08): no implementation changes.Retail store data layer no longer uses
mongoRawor ObjectId helpers (no behavior change;object-id-filter.tsdeleted).Compatibility / migration / risk
None. All new method forms are additive overloads; existing plain-object
update()calls are unchanged.Follow-ups / open questions
FieldExpression<T>gatesinc()/mul()onT extends number. ADR 170 specifies the fuller mechanism viaCodecTypes[Id]['traits']. Current shortcut is correct for all existing Mongo codecs.upsert()callbacks is runtime-only; a type-level restriction is possible as a follow-up.Non-goals / intentionally out of scope
Summary by CodeRabbit
New Features
Improvements
Tests