feat(mongo-query-ast): add core pipeline stage extensions#306
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds seven aggregation stages, unifies stage rewrite APIs under a MongoStageRewriterContext (filter + aggExpr), extends visitors and public exports, updates lowering to handle new stage shapes and expressions, and expands type and runtime tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Stage as MongoStageNode
participant Context as MongoStageRewriterContext
participant AggRewriter as MongoAggExprRewriter
participant Lowerer as LoweringAdapter
participant Runtime as MongoDB Driver
Stage->>Context: rewrite(context)
Context->>AggRewriter: (optional) rewrite agg expressions
AggRewriter-->>Stage: return rewritten agg-expr nodes
Stage-->>Lowerer: provide lowered stage shape
Lowerer->>Runtime: emit BSON stage for execution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/2-mongo-family/4-query/query-ast/test/stages.test-d.ts (1)
154-154: Use an explicit mock-boundary double cast here.Direct cast on a synthetic mock object is discouraged in test files; use
unknownas the unsafe boundary.♻️ Suggested change
- const stage = {} as MongoMatchStage; + const stage = {} as unknown as MongoMatchStage;As per coding guidelines:
**/*.{test.ts,test-d.ts}: “Use double casts (as unknown as X) for mocks and dynamic proxies in tests instead of direct casts”.🤖 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/test/stages.test-d.ts` at line 154, Replace the direct cast of the synthetic mock object to MongoMatchStage by introducing the mock boundary using unknown; change the construction of the stage mock (symbol: stage) so the empty object is first cast to unknown and then to MongoMatchStage (i.e., use a double cast) to follow test mock guidelines.packages/2-mongo-family/4-query/query-ast/src/stages.ts (1)
180-184: Constructor validation allowslet_withoutpipeline, which may be invalid MongoDB syntax.The validation permits constructing a
MongoLookupStagewith onlylet_(nopipeline,localField, orforeignField), which would be invalid when executed against MongoDB. Consider tightening the validation to requirepipelinewhenlet_is provided.Suggested stricter validation
- if (!options.localField && !options.foreignField && !options.pipeline && !options.let_) { + const hasEqualityFields = options.localField && options.foreignField; + const hasPipeline = !!options.pipeline; + if (!hasEqualityFields && !hasPipeline) { throw new Error( 'MongoLookupStage requires either equality fields (localField/foreignField) or a pipeline', ); } + if (options.let_ && !hasPipeline) { + throw new Error('MongoLookupStage let_ requires a pipeline'); + }🤖 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/stages.ts` around lines 180 - 184, The constructor validation for MongoLookupStage currently allows options.let_ without options.pipeline; update the check in the MongoLookupStage constructor so that if options.let_ is present it also requires options.pipeline (i.e., reject cases where options.let_ is set but options.pipeline is falsy), while keeping the existing requirement that at least one of (localField/foreignField) or pipeline is present; adjust the thrown Error message accordingly to mention that let_ requires a pipeline when provided.
🤖 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/4-query/query-ast/src/stages.ts`:
- Around line 180-184: The constructor validation for MongoLookupStage currently
allows options.let_ without options.pipeline; update the check in the
MongoLookupStage constructor so that if options.let_ is present it also requires
options.pipeline (i.e., reject cases where options.let_ is set but
options.pipeline is falsy), while keeping the existing requirement that at least
one of (localField/foreignField) or pipeline is present; adjust the thrown Error
message accordingly to mention that let_ requires a pipeline when provided.
In `@packages/2-mongo-family/4-query/query-ast/test/stages.test-d.ts`:
- Line 154: Replace the direct cast of the synthetic mock object to
MongoMatchStage by introducing the mock boundary using unknown; change the
construction of the stage mock (symbol: stage) so the empty object is first cast
to unknown and then to MongoMatchStage (i.e., use a double cast) to follow test
mock guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0ad0429f-0123-4cff-9e8f-ea5a03005865
⛔ Files ignored due to path filters (3)
projects/mongo-pipeline-builder/plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/aggregation-expression-ast-plan.mdis excluded by!projects/**projects/orm-consolidation/plans/aggregation-expression-ast-design.mdis excluded by!projects/**
📒 Files selected for processing (17)
packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.tspackages/2-mongo-family/4-query/query-ast/src/exports/index.tspackages/2-mongo-family/4-query/query-ast/src/filter-expressions.tspackages/2-mongo-family/4-query/query-ast/src/stages.tspackages/2-mongo-family/4-query/query-ast/src/visitors.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test.tspackages/2-mongo-family/4-query/query-ast/test/stages.test-d.tspackages/2-mongo-family/4-query/query-ast/test/stages.test.tspackages/2-mongo-family/4-query/query-ast/vitest.config.tspackages/2-mongo-family/7-runtime/test/aggregate.test.tspackages/3-mongo-target/2-mongo-adapter/src/exports/index.tspackages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.tstest/integration/test/mongo/expr-filter.test.ts
wmadden
force-pushed
the
tml-2210-core-pipeline-stage-extensions-group-end-to-end
branch
from
6aab003 to
a47a548
Compare
@prisma-next/mongo-orm
@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/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-emitter
@prisma-next/mongo-query-ast
@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/adapter-postgres
@prisma-next/driver-postgres
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/2-mongo-family/4-query/query-ast/test/stages.test.ts (2)
295-611: Split this test file to satisfy the 500-line limit.Line [611] shows this file now exceeds the repository cap for test size. The new stage-specific blocks provide natural split points (e.g., stage construction/rewrite vs visitor dispatch), so this should be split into focused test files for maintainability.
As per coding guidelines, "
**/*.test.ts: Keep test files under 500 lines ... Split test files when they exceed 500 lines ...`"🤖 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/test/stages.test.ts` around lines 295 - 611, This file exceeds the 500-line test limit; split it into smaller, focused test files by grouping related describe blocks (e.g., one file per stage or a file for stage behavior tests and another for visitor dispatch tests). Move the describe blocks for MongoGroupStage, MongoAddFieldsStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage, MongoSampleStage, MongoRedactStage (and their rewrite/is-frozen tests) into one or more stage-specific test files, and move the MongoStageVisitor dispatch tests (the kindVisitor and its accept assertions) into a separate visitor test file; update test suite imports/exports if any references exist so symbols like MongoGroupStage, MongoAddFieldsStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage, MongoSampleStage, MongoRedactStage, and MongoStageVisitor remain resolvable. Ensure each new file stays under 500 lines and retains the same test bodies and descriptions.
329-523: Factor repeated aggExpr rewriter setup into a small helper.The same
MongoAggExprRewriterobject is repeated across multiple tests (Line [330], Line [342], Line [382], Line [408], Line [451], Line [511]). A local helper would reduce duplication.♻️ Suggested refactor
+const prefixFieldRefRewriter = (prefix: string): MongoAggExprRewriter => ({ + fieldRef: (expr) => MongoAggFieldRef.of(`${prefix}${expr.path}`), +}); ... -const aggExpr: MongoAggExprRewriter = { - fieldRef: (expr) => MongoAggFieldRef.of(`r.${expr.path}`), -}; +const aggExpr = prefixFieldRefRewriter('r.');🤖 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/test/stages.test.ts` around lines 329 - 523, Multiple tests repeat the same MongoAggExprRewriter setup; create a small helper function (e.g., makeAggExprRewriter or aggExprRewriter) that returns a MongoAggExprRewriter with fieldRef: (expr) => MongoAggFieldRef.of(`r.${expr.path}`) and use it in tests that call stage.rewrite({ aggExpr }) (references: MongoAggExprRewriter, fieldRef, MongoAggFieldRef.of, and rewrite methods on MongoGroupStage, MongoAddFieldsStage, MongoReplaceRootStage, MongoSortByCountStage, MongoRedactStage); add the helper near the top of the describe block (or test file) and replace inline aggExpr objects with a call to the helper to remove duplication while preserving types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/3-mongo-target/2-mongo-adapter/src/lowering.ts`:
- Around line 152-159: The current discriminator in lowerGroupId uses the
presence of the string key 'kind' to decide if a MongoGroupId is a MongoAggExpr,
which misclassifies document-shaped _id values like {_id: { kind: expr }} and
causes lowerAggExpr to be called on plain objects (leading to missing accept()).
Replace the `'kind' in groupId` check with a proper AST node guard that detects
an actual MongoAggExpr node (e.g., check for the node shape/property that real
AST nodes expose such as a type tag or an isNode/isAggExpr predicate used
elsewhere), update the corresponding helper in stages.ts to use the same guard,
and add a regression test that lowers a composite group _id containing a key
named "kind" to ensure it stays a document and does not call lowerAggExpr.
Ensure references to lowerAggExpr and accept() are preserved so the guard
prevents passing plain objects into lowerAggExpr.
- Around line 192-204: MongoLookupStage currently allows half-specified
equality-style lookups; update MongoLookupStage.constructor() validation so it
rejects instances where only one of localField or foreignField is provided.
Require either both localField AND foreignField (equality syntax) or a pipeline
(pipeline present) — let_ may be present only when pipeline is used. Modify the
constructor to throw/raise on the invalid case (localField xor foreignField) and
keep existing acceptance for pipeline (+ optional let_).
---
Nitpick comments:
In `@packages/2-mongo-family/4-query/query-ast/test/stages.test.ts`:
- Around line 295-611: This file exceeds the 500-line test limit; split it into
smaller, focused test files by grouping related describe blocks (e.g., one file
per stage or a file for stage behavior tests and another for visitor dispatch
tests). Move the describe blocks for MongoGroupStage, MongoAddFieldsStage,
MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage, MongoSampleStage,
MongoRedactStage (and their rewrite/is-frozen tests) into one or more
stage-specific test files, and move the MongoStageVisitor dispatch tests (the
kindVisitor and its accept assertions) into a separate visitor test file; update
test suite imports/exports if any references exist so symbols like
MongoGroupStage, MongoAddFieldsStage, MongoReplaceRootStage, MongoCountStage,
MongoSortByCountStage, MongoSampleStage, MongoRedactStage, and MongoStageVisitor
remain resolvable. Ensure each new file stays under 500 lines and retains the
same test bodies and descriptions.
- Around line 329-523: Multiple tests repeat the same MongoAggExprRewriter
setup; create a small helper function (e.g., makeAggExprRewriter or
aggExprRewriter) that returns a MongoAggExprRewriter with fieldRef: (expr) =>
MongoAggFieldRef.of(`r.${expr.path}`) and use it in tests that call
stage.rewrite({ aggExpr }) (references: MongoAggExprRewriter, fieldRef,
MongoAggFieldRef.of, and rewrite methods on MongoGroupStage,
MongoAddFieldsStage, MongoReplaceRootStage, MongoSortByCountStage,
MongoRedactStage); add the helper near the top of the describe block (or test
file) and replace inline aggExpr objects with a call to the helper to remove
duplication while preserving types.
🪄 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: aa6148c8-4f46-4054-a061-8cf4ba53bdb0
📒 Files selected for processing (8)
packages/2-mongo-family/4-query/query-ast/src/exports/index.tspackages/2-mongo-family/4-query/query-ast/src/stages.tspackages/2-mongo-family/4-query/query-ast/src/visitors.tspackages/2-mongo-family/4-query/query-ast/test/stages.test-d.tspackages/2-mongo-family/4-query/query-ast/test/stages.test.tspackages/2-mongo-family/7-runtime/test/aggregate.test.tspackages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/2-mongo-family/7-runtime/test/aggregate.test.ts
- packages/2-mongo-family/4-query/query-ast/test/stages.test-d.ts
- packages/2-mongo-family/4-query/query-ast/src/stages.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/2-mongo-family/4-query/query-ast/src/exports/index.ts
- packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
- packages/2-mongo-family/4-query/query-ast/src/visitors.ts
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/2-mongo-family/4-query/query-ast/test/stages-extended.test.ts (1)
50-57: Strengthen compoundgroupIdassertions to validate values, not just presence.
toBeDefined()here can pass even if the wrong expressions are assigned. Assert the resolved field paths to validate wiring.Proposed test tightening
it('accepts compound groupId', () => { const stage = new MongoGroupStage( { dept: MongoAggFieldRef.of('department'), year: MongoAggFieldRef.of('year') }, { total: MongoAggAccumulator.sum(MongoAggFieldRef.of('amount')) }, ); - expect((stage.groupId as Record<string, MongoAggExpr>)['dept']).toBeDefined(); - expect((stage.groupId as Record<string, MongoAggExpr>)['year']).toBeDefined(); + const groupId = stage.groupId as Record<string, MongoAggExpr>; + expect({ + dept: (groupId['dept'] as MongoAggFieldRef).path, + year: (groupId['year'] as MongoAggFieldRef).path, + }).toMatchObject({ + dept: 'department', + year: 'year', + }); });As per coding guidelines "
**/*.test.ts: Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/4-query/query-ast/test/stages-extended.test.ts` around lines 50 - 57, Replace the loose presence checks on stage.groupId with a stronger object equality assertion: instead of two expect(...).toBeDefined() calls, assert that stage.groupId matches the expected mapping (use expect(stage.groupId).toMatchObject(...)) and compare the values to the exact MongoAggFieldRef/MongoAggExpr instances you expect (e.g., dept should equal MongoAggFieldRef.of('department') and year should equal MongoAggFieldRef.of('year')), so the test verifies the resolved field paths/expressions rather than just their existence; refer to MongoGroupStage, groupId, MongoAggFieldRef, and MongoAggExpr to locate the code to change.packages/2-mongo-family/4-query/query-ast/test/stages.test.ts (2)
66-82: Add the no-op branch for expression-valued projections.The new
MongoAggExprpath is only exercised withaggExprpresent. A regression that assumescontext.aggExprexists for expression-bearing projections would still pass because the only identity check here is the scalar-only case.Suggested test addition
it('rewrite() returns this for scalar-only projections', () => { const stage = new MongoProjectStage({ name: 1 }); expect(stage.rewrite({})).toBe(stage); }); + it('rewrite() returns this when no aggExpr rewriter is provided', () => { + const stage = new MongoProjectStage({ + fullName: MongoAggFieldRef.of('name'), + _id: 0, + }); + expect(stage.rewrite({})).toBe(stage); + }); + it('rewrite() recurses into expression projection values', () => {🤖 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/test/stages.test.ts` around lines 66 - 82, Add a test that exercises the no-op branch when projections contain expression values but no aggExpr is provided: create a MongoProjectStage whose projection includes a MongoAggFieldRef (e.g., fullName: MongoAggFieldRef.of('name')), call stage.rewrite({}) and assert it returns the same instance (toBe(stage)). This ensures MongoProjectStage.rewrite does not assume context.aggExpr exists for expression-valued projections and covers the MongoAggExpr/MongoAggFieldRef path without providing aggExpr.
211-225: Cover{ aggExpr }propagation through$lookup.pipeline.This proves nested
filterrewrites, and the laterlet_case proves top-levellet_rewrites, but the suite still never places an expression-bearing stage insidepipeline. A bug that skipsrewrite(context)for sub-pipeline stages when onlyaggExpris supplied would slip through.Suggested test addition
it('rewrite() rewrites nested pipeline stages', () => { const filter: MongoFilterRewriter = { field: (expr) => MongoFieldFilter.of(expr.field, '$ne', expr.value), }; @@ const match = rewritten.pipeline![0] as MongoMatchStage; expect((match.filter as MongoFieldFilter).op).toBe('$ne'); }); + + it('rewrite() propagates aggExpr into nested pipeline stages', () => { + const aggExpr: MongoAggExprRewriter = { + fieldRef: (expr) => MongoAggFieldRef.of(`r.${expr.path}`), + }; + const stage = new MongoLookupStage({ + from: 'orders', + as: 'matchingOrders', + pipeline: [new MongoProjectStage({ userId: MongoAggFieldRef.of('_id') })], + }); + const rewritten = stage.rewrite({ aggExpr }) as MongoLookupStage; + expect( + ((rewritten.pipeline![0] as MongoProjectStage).projection['userId'] as MongoAggFieldRef).path, + ).toBe('r._id'); + });🤖 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/test/stages.test.ts` around lines 211 - 225, The test case for MongoLookupStage rewrite misses verifying that aggExpr-only rewrite contexts propagate into nested lookup.pipeline; update the 'rewrite() rewrites nested pipeline stages' test (referencing MongoLookupStage, MongoMatchStage, and MongoFieldFilter) to include a pipeline stage that carries an expression (e.g., a MongoProjectStage or a stage with an aggExpr-bearing field) and assert that its rewrite(context with only aggExpr) is applied (for example, transform an agg expression or resulting operator and assert the rewritten pipeline stage reflects the change), ensuring rewrite(context) is invoked for sub-pipeline stages when only aggExpr is provided.
🤖 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/4-query/query-ast/test/stages-extended.test.ts`:
- Around line 50-57: Replace the loose presence checks on stage.groupId with a
stronger object equality assertion: instead of two expect(...).toBeDefined()
calls, assert that stage.groupId matches the expected mapping (use
expect(stage.groupId).toMatchObject(...)) and compare the values to the exact
MongoAggFieldRef/MongoAggExpr instances you expect (e.g., dept should equal
MongoAggFieldRef.of('department') and year should equal
MongoAggFieldRef.of('year')), so the test verifies the resolved field
paths/expressions rather than just their existence; refer to MongoGroupStage,
groupId, MongoAggFieldRef, and MongoAggExpr to locate the code to change.
In `@packages/2-mongo-family/4-query/query-ast/test/stages.test.ts`:
- Around line 66-82: Add a test that exercises the no-op branch when projections
contain expression values but no aggExpr is provided: create a MongoProjectStage
whose projection includes a MongoAggFieldRef (e.g., fullName:
MongoAggFieldRef.of('name')), call stage.rewrite({}) and assert it returns the
same instance (toBe(stage)). This ensures MongoProjectStage.rewrite does not
assume context.aggExpr exists for expression-valued projections and covers the
MongoAggExpr/MongoAggFieldRef path without providing aggExpr.
- Around line 211-225: The test case for MongoLookupStage rewrite misses
verifying that aggExpr-only rewrite contexts propagate into nested
lookup.pipeline; update the 'rewrite() rewrites nested pipeline stages' test
(referencing MongoLookupStage, MongoMatchStage, and MongoFieldFilter) to include
a pipeline stage that carries an expression (e.g., a MongoProjectStage or a
stage with an aggExpr-bearing field) and assert that its rewrite(context with
only aggExpr) is applied (for example, transform an agg expression or resulting
operator and assert the rewritten pipeline stage reflects the change), ensuring
rewrite(context) is invoked for sub-pipeline stages when only aggExpr is
provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9b49cf55-3a3e-48d8-9198-5cfb20036f20
📒 Files selected for processing (6)
packages/2-mongo-family/4-query/query-ast/src/stages.tspackages/2-mongo-family/4-query/query-ast/test/stages-extended.test.tspackages/2-mongo-family/4-query/query-ast/test/stages.test-d.tspackages/2-mongo-family/4-query/query-ast/test/stages.test.tspackages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/2-mongo-family/4-query/query-ast/test/stages.test-d.ts
- packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
- packages/2-mongo-family/4-query/query-ast/src/stages.ts
wmadden
force-pushed
the
tml-2210-core-pipeline-stage-extensions-group-end-to-end
branch
from
90ff16c to
44fe826
Compare
Stage rewrite() now accepts a context with optional filter and aggExpr rewriters, enabling new stages that contain MongoAggExpr to recurse into both filter and aggregation expression trees.
MongoGroupStage, MongoAddFieldsStage, MongoReplaceRootStage, MongoCountStage, MongoSortByCountStage, MongoSampleStage, and MongoRedactStage — each with kind discriminant, immutability, accept/rewrite, and unit tests. MongoStageVisitor and MongoReadStage union expanded to cover all new kinds.
MongoProjectStage accepts MongoAggExpr for computed projections. MongoLookupStage supports correlated pipeline form with let_ and optional localField/foreignField. MongoUnwindStage gains optional includeArrayIndex. All extensions are backward compatible.
All 7 new stage classes and 2 new type aliases now exported from @prisma-next/mongo-query-ast barrel.
lowerStage() now handles group, addFields, replaceRoot, count, sortByCount, sample, and redact stages. Project stage lowering calls lowerAggExpr for computed projection values. Lookup lowering emits let for correlated pipelines. Unwind lowering includes includeArrayIndex.
Replace raw $group/$sort objects with MongoGroupStage and MongoSortStage AST nodes, proving typed stages execute correctly through the full runtime stack against mongodb-memory-server.
- F01: Add exhaustiveness guard default case to lowerStage() - F02: Document MongoAggAccumulator.rewrite() cast limitation - F03: Add constructor validation to MongoLookupStage (equality or pipeline required) - F04: Add stages.test-d.ts with compile-time type tests for visitor/union contracts - F05: Document structural discrimination convention for isAggExpr - AC10: Add MongoMatchStage to integration test pipeline to match spec
- Use accept-based guard to discriminate MongoGroupId instead of kind check, preventing misclassification of compound _id records with a "kind" key - Tighten MongoLookupStage validation: require localField/foreignField as a pair, require pipeline when let_ is provided - Use double cast (as unknown as) for mock objects in type tests - Split stages.test.ts into stages.test.ts (308 lines) and stages-extended.test.ts (349 lines) to stay under 500-line limit - Extract prefixFieldRefRewriter helper to reduce test duplication - Add regression tests for compound groupId with "kind" key in both query-ast and adapter-mongo lowering
wmadden
force-pushed
the
tml-2210-core-pipeline-stage-extensions-group-end-to-end
branch
from
44fe826 to
bf0154d
Compare
wmadden
deleted the
tml-2210-core-pipeline-stage-extensions-group-end-to-end
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-2210
Key snippet (new capability)
Intent
Extend the MongoDB pipeline stage AST with the core data-transformation stages (
$group,$addFields,$replaceRoot,$count,$sortByCount,$sample,$redact) and widen three existing stages ($projectwith computed expressions,$lookupwith correlated pipelines,$unwindwith array index), so that typed AST pipelines can express grouping, computed projections, and document reshaping without falling back to rawRecord<string, unknown>. This is Milestone 3 of the Mongo Pipeline Builder project and unblocks the typed pipeline builder (TML-2211).Change map
MongoStageRewriterContext, extendedMongoStageVisitor)The story
Introduce a composite rewriter context: The existing
rewrite(MongoFilterRewriter)signature cannot recurse intoMongoAggExprnodes. A newMongoStageRewriterContextinterface bundles bothfilter?andaggExpr?rewriters, and all existing stages are migrated to accept it. This resolves Open Question 1 from the spec with Option (a).Add 7 new pipeline stage classes:
MongoGroupStage,MongoAddFieldsStage,MongoReplaceRootStage,MongoCountStage,MongoSortByCountStage,MongoSampleStage,MongoRedactStage— all following the establishedMongoStageNodepattern (kind discriminant, frozen, accept/rewrite). Each stage'srewrite()recurses into its embeddedMongoAggExprfields using the context'saggExprrewriter.Widen 3 existing stages for richer usage:
MongoProjectStagenow acceptsMongoAggExprvalues alongside0 | 1(computed projections).MongoLookupStagesupports the correlated pipeline form (let_/pipelinewithoutlocalField/foreignField).MongoUnwindStagegains optionalincludeArrayIndex.Wire adapter lowering:
lowerStage()inadapter-mongogets cases for all 7 new stage kinds plus updated lowering for the 3 widened stages. Expression-bearing stages call throughlowerAggExpr(),lowerGroupId(),lowerExprRecord(), andlowerProjectionValue()— no raw pass-through.Prove the full stack end-to-end: The integration test replaces the raw-object
$grouppipeline with typedMongoMatchStage→MongoGroupStage→MongoSortStageand verifies correct aggregation results against mongodb-memory-server.Harden with type-level tests and invariant checks: Adds
stages.test-d.tsverifying compile-time contracts (visitor requires all 14 methods, kind union exhaustiveness, accumulator type constraint). Adds constructor validation toMongoLookupStage(at least one of equality or pipeline form required). Adds exhaustiveness guard tolowerStage().Behavior changes & evidence
Adds 7 typed pipeline stage classes that can represent grouping, computed fields, document replacement, counting, sampling, and redaction without the raw escape hatch
AggregatePipelineEntry(Record<string, unknown>) escape hatch loses all type safety. With the aggregation expression AST (TML-2209) available, these stages can embed typedMongoAggExprnodes.Stage rewriting now composes filter and agg-expr rewriters via
MongoStageRewriterContext:rewrite()changed from acceptingMongoFilterRewritertoMongoStageRewriterContext { filter?, aggExpr? }, recursing into both expression types.MongoAggExprnodes, notMongoFilterExpr. The composite context lets each stage pick the rewriter it needs.MongoProjectStageaccepts computed projection values (0 | 1 | MongoAggExpr): enables expressions like{ fullName: { $concat: ["$first", " ", "$last"] } }.MongoLookupStagesupports correlated pipeline form:localField/foreignFieldnow optional;let_supported. Constructor validates at least one form is present.Adapter
lowerStage()handles all new and widened stage kinds with exhaustiveness guard.Compatibility / migration / risk
compile.tscompiles unchanged.rewrite()signature change is internal — all call sites updated, no external consumers.MongoReadStageunion is widened. Existingswitchstatements will get compile errors for unhandled members — intended forcing function.Follow-ups / open questions
as MongoAggAccumulatorcast inMongoGroupStage.rewrite()should be addressed by narrowingMongoAggAccumulator.rewrite()return type — tracked as a follow-up for the aggregation expression AST.Non-goals / intentionally out of scope
$facet,$graphLookup,$unionWith,$bucket, etc.) — deferred to Milestone 5MongoReadStage→MongoPipelineStage— deferred to Milestone 5AggregatePipelineEntryescape hatch — deferred to Milestone 5Summary by CodeRabbit
New Features
Improvements
Tests