refactor: execution metadata lives on AST (ADR 205)#393
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (86)
📝 WalkthroughWalkthroughThis pull request implements ADR 205 and ADR 012, moving execution metadata from Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 57 minutes and 45 seconds.Comment |
SevInf
force-pushed
the
ast-codec-ids-v2
branch
from
c0e7d51 to
8245deb
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
@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/ts-render
@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/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: |
SevInf
commented
Apr 29, 2026
SevInf
force-pushed
the
ast-codec-ids-v2
branch
from
3d6422f to
3720761
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts (1)
19-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate RETURNING columns before stamping
codecId.This path now silently builds
ProjectionItems withcodecId: undefinedwhentableNameis unknown or a requested returning column is missing. That defers the failure to SQL execution and also disables decode-time codec lookup for that projection. Please mirror the validation style used intoParamAssignments()/resolveTableColumns()and then accesstable.columns[column]directly.Suggested fix
function buildReturningColumns( contract: Contract<SqlStorage>, tableName: string, returningColumns: readonly string[] | undefined, ): ReadonlyArray<ProjectionItem> { const columns = returningColumns && returningColumns.length > 0 ? [...returningColumns] : resolveTableColumns(contract, tableName); - const table = contract.storage.tables[tableName]; - return columns.map((column) => - ProjectionItem.of(column, ColumnRef.of(tableName, column), table?.columns[column]?.codecId), - ); + const table = contract.storage.tables[tableName]; + if (!table) { + throw new Error(`Unknown table "${tableName}"`); + } + + return columns.map((column) => { + const codecId = table.columns[column]?.codecId; + if (!codecId) { + throw new Error(`Unknown column "${column}" in table "${tableName}"`); + } + return ProjectionItem.of(column, ColumnRef.of(tableName, column), codecId); + }); }As per coding guidelines, "Remove optional chaining and conditional property spreading for properties that are guaranteed to exist after validation - access required properties directly without
?.operators".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts` around lines 19 - 32, In buildReturningColumns, validate that the table exists and each requested returning column exists before creating ProjectionItem instances: first resolve the table via contract.storage.tables[tableName] and throw an error if missing (matching resolveTableColumns/toParamAssignments style), then iterate the chosen columns and verify table.columns[column] exists, throwing if a column is unknown; only after these validations access table.columns[column].codecId directly (remove the optional chaining/table? usage) when constructing ProjectionItem.of.packages/2-sql/5-runtime/src/middleware/before-compile-chain.ts (1)
13-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve meta-only
beforeCompilerewrites.This now treats
astidentity as the only change signal, so a middleware that returns the same AST with updatedmetais dropped on the floor. That breaks both downstream middleware chaining and the final draft returned toSqlRuntimeImpl.runBeforeCompile(). Please keep the new draft when eitherastormetachanges, or narrow the middleware contract to AST-only rewrites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/5-runtime/src/middleware/before-compile-chain.ts` around lines 13 - 28, The middleware loop in before-compile-chain.ts currently ignores rewrites that only change meta because it only compares result.ast === current.ast; update the logic in the loop that handles mw.beforeCompile (the comparison around result.ast and current.ast) to treat the draft as changed when either result.ast !== current.ast OR result.meta !== current.meta (or use a deep/appropriate equality check for meta), so that current is replaced with result when meta-only updates occur; keep the ctx.log.debug middleware.rewrite behavior and ensure this preserves drafts passed back to SqlRuntimeImpl.runBeforeCompile() and downstream middleware chaining.
🧹 Nitpick comments (3)
docs/architecture docs/adrs/ADR 205 - Execution metadata lives on AST.md (1)
108-117: ⚡ Quick winMove implementation mechanics out of the ADR body.
This section reads like migration/implementation guidance (specific producers, visitors, adapter lowering changes). In this repo, ADRs are expected to stay at decision/constraint level; detailed rollout mechanics are better in subsystem docs or follow-up implementation notes.
Based on learnings: “ADRs in prisma/prisma-next should document architectural design decisions and the key constraints/assumptions only... move implementation guidance to subsystem documentation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/adrs/ADR 205 - Execution metadata lives on AST.md around lines 108 - 117, The ADR includes low-level implementation and migration guidance that should be removed from the decision document and moved into subsystem implementation notes; specifically, remove or relocate the procedural details referencing buildQueryPlan, buildOrmQueryPlan, the row decoder change to walk projection lists, the RETURNING lowering changes (InsertAst/UpdateAst/DeleteAst and ProjectionItem), the PlanMeta pruning, and the test-migration guidance about annotations.codecs and refs/paramDescriptors/raw-plan options—retain only the architectural decision, constraints, and key assumptions in this ADR and create a separate implementation doc (or link) containing the detailed steps, visitor/rewriter edits, adapter lowering changes, and test migration instructions.packages/2-sql/4-lanes/relational-core/test/ast/common.test.ts (1)
116-119: ⚡ Quick winPrefer a single object matcher for related
stampedfield assertions.This keeps failure output and style consistent with the test guidelines.
♻️ Suggested change
- expect(stamped.codecId).toBe('pg/int4@1'); - expect(stamped.alias).toBe('id'); - expect(stamped.expr).toBe(expr); + expect(stamped).toMatchObject({ + codecId: 'pg/int4@1', + alias: 'id', + expr, + });As per coding guidelines, "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-sql/4-lanes/relational-core/test/ast/common.test.ts` around lines 116 - 119, Replace the three separate assertions on the stamped object with a single object matcher: after creating stamped via withoutCodec.withCodecId('pg/int4@1'), assert stamped matches an object containing codecId: 'pg/int4@1', alias: 'id', and expr: expr (use toMatchObject or equivalent) so the related fields are checked together; update the test that references stamped, withoutCodec.withCodecId, codecId, alias, and expr accordingly.packages/2-sql/4-lanes/relational-core/src/ast/types.ts (1)
1071-1087: ⚡ Quick winExpose
codecIdthrough the incremental projection API too.
ProjectionItemnow carries execution metadata, butSelectAst.addProjection()still has no way to supply it. That makes the common incremental builder path lossy and easy to misuse after ADR 205.Follow-up change
- addProjection(alias: string, expr: ProjectionExpr): SelectAst { + addProjection(alias: string, expr: ProjectionExpr, codecId?: string): SelectAst { return new SelectAst({ ...this, - projection: [...this.projection, new ProjectionItem(alias, expr)], + projection: [...this.projection, new ProjectionItem(alias, expr, codecId)], }); }As per coding guidelines: Keep public API stable while moving internals during refactoring of large monolithic files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/4-lanes/relational-core/src/ast/types.ts` around lines 1071 - 1087, SelectAst.addProjection currently cannot accept ProjectionItem.execution metadata (codecId) causing loss when building incrementally; update the SelectAst.addProjection signature to accept an optional codecId: string | undefined, and pass that codecId through when constructing ProjectionItem (use new ProjectionItem(alias, expr, codecId) or ProjectionItem.of/withCodecId as appropriate). Also update any incremental builder helpers that call addProjection to accept/forward codecId (and adjust call sites that construct ProjectionItem via ProjectionItem.of or withCodecId) so the execution metadata is preserved across the API boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/adrs/ADR 012 - Raw SQL Escape Hatch.md:
- Around line 3-4: The ADR text is inconsistent: the banner says the optional
structured-annotations branch (`refs`, `projection`, `codecs`,
`paramDescriptors`) was removed but the body still documents them as current;
update ADR 012 - Raw SQL Escape Hatch to either (a) mark all sections that
describe `refs`/`projection`/`codecs`/`paramDescriptors` as historical/retired
(prepend a "Deprecated / Historical" note and link to ADR 205) or (b) delete
those sections and rewrite any runtime behavior text to describe current
behavior (i.e., raw plans no longer provide those fields, they pass parameters
as supplied and surface wire-level row values), while ensuring the minimal
annotation schema (`intent`, `isMutation`, `hasWhere`, `hasLimit`) is clearly
presented as the active spec.
In `@packages/2-sql/5-runtime/src/codecs/decoding.ts`:
- Around line 47-55: The current logic treats an empty projection list
(projection === []) as valid metadata, causing aliases to be set to an empty
array and decodeRow to produce empty objects; update the early-return after
calling projectionListFromAst(plan.ast) so that it treats both falsy and
empty-array results as "no projection metadata" (i.e., when projection is
null/undefined OR projection.length === 0) and return aliases: undefined (and
the same empty codecs/columnRefs/includeAliases) so decodeRow preserves raw row
keys; locate the check around projectionListFromAst and adjust the condition
accordingly for projection and downstream use in decodeRow/withProjection
handling.
In `@packages/2-sql/5-runtime/src/codecs/encoding.ts`:
- Around line 85-88: When a stamped codec is missing, fail fast instead of
returning the raw value: in the branch where you call
registry.get(metadata.codecId) and receive undefined, check whether
metadata.codecId is set and, if so, throw a descriptive error (including the
missing metadata.codecId and relevant param/ref identifier) rather than
returning value; keep the existing behavior of returning value only when no
codecId is present. This change should be applied where codec is obtained
(registry.get), using the symbols metadata.codecId, codec and the surrounding
encoding/encode function logic.
In `@packages/2-sql/5-runtime/test/budgets.test.ts`:
- Around line 223-234: The test currently sets a limit via
SelectAst.withLimit(5) so the estimated rows will be 5 regardless of whether
budgets() uses tableRows.user or defaultTableRows; update the test to make the
assertion depend on the FROM lookup by removing or changing the limit and
adjusting budgets() inputs — for example, remove .withLimit(5) (or set a higher
limit) and set budgets({ maxRows: 4, defaultTableRows: 10_000, tableRows: {
user: 5 }}) or keep maxRows high and set defaultTableRows < tableRows.user so
that the planner’s estimate (from createPlan and budgets) differs when FROM
lookup is broken; ensure you reference SelectAst.from, .withProjection,
.withLimit, createPlan, and budgets in the change.
---
Outside diff comments:
In `@packages/2-sql/5-runtime/src/middleware/before-compile-chain.ts`:
- Around line 13-28: The middleware loop in before-compile-chain.ts currently
ignores rewrites that only change meta because it only compares result.ast ===
current.ast; update the logic in the loop that handles mw.beforeCompile (the
comparison around result.ast and current.ast) to treat the draft as changed when
either result.ast !== current.ast OR result.meta !== current.meta (or use a
deep/appropriate equality check for meta), so that current is replaced with
result when meta-only updates occur; keep the ctx.log.debug middleware.rewrite
behavior and ensure this preserves drafts passed back to
SqlRuntimeImpl.runBeforeCompile() and downstream middleware chaining.
In `@packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts`:
- Around line 19-32: In buildReturningColumns, validate that the table exists
and each requested returning column exists before creating ProjectionItem
instances: first resolve the table via contract.storage.tables[tableName] and
throw an error if missing (matching resolveTableColumns/toParamAssignments
style), then iterate the chosen columns and verify table.columns[column] exists,
throwing if a column is unknown; only after these validations access
table.columns[column].codecId directly (remove the optional chaining/table?
usage) when constructing ProjectionItem.of.
---
Nitpick comments:
In `@docs/architecture` docs/adrs/ADR 205 - Execution metadata lives on AST.md:
- Around line 108-117: The ADR includes low-level implementation and migration
guidance that should be removed from the decision document and moved into
subsystem implementation notes; specifically, remove or relocate the procedural
details referencing buildQueryPlan, buildOrmQueryPlan, the row decoder change to
walk projection lists, the RETURNING lowering changes
(InsertAst/UpdateAst/DeleteAst and ProjectionItem), the PlanMeta pruning, and
the test-migration guidance about annotations.codecs and
refs/paramDescriptors/raw-plan options—retain only the architectural decision,
constraints, and key assumptions in this ADR and create a separate
implementation doc (or link) containing the detailed steps, visitor/rewriter
edits, adapter lowering changes, and test migration instructions.
In `@packages/2-sql/4-lanes/relational-core/src/ast/types.ts`:
- Around line 1071-1087: SelectAst.addProjection currently cannot accept
ProjectionItem.execution metadata (codecId) causing loss when building
incrementally; update the SelectAst.addProjection signature to accept an
optional codecId: string | undefined, and pass that codecId through when
constructing ProjectionItem (use new ProjectionItem(alias, expr, codecId) or
ProjectionItem.of/withCodecId as appropriate). Also update any incremental
builder helpers that call addProjection to accept/forward codecId (and adjust
call sites that construct ProjectionItem via ProjectionItem.of or withCodecId)
so the execution metadata is preserved across the API boundary.
In `@packages/2-sql/4-lanes/relational-core/test/ast/common.test.ts`:
- Around line 116-119: Replace the three separate assertions on the stamped
object with a single object matcher: after creating stamped via
withoutCodec.withCodecId('pg/int4@1'), assert stamped matches an object
containing codecId: 'pg/int4@1', alias: 'id', and expr: expr (use toMatchObject
or equivalent) so the related fields are checked together; update the test that
references stamped, withoutCodec.withCodecId, codecId, alias, and expr
accordingly.
🪄 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: 6c3adf38-5887-4752-b166-cce9405c55e6
📒 Files selected for processing (83)
docs/architecture docs/adrs/ADR 012 - Raw SQL Escape Hatch.mddocs/architecture docs/adrs/ADR 205 - Execution metadata lives on AST.mdexamples/prisma-next-demo/test/runtime.offline.integration.test.tspackages/1-framework/0-foundation/contract/README.mdpackages/1-framework/0-foundation/contract/src/exports/types.tspackages/1-framework/0-foundation/contract/src/types.tspackages/1-framework/0-foundation/contract/test/types.test.tspackages/1-framework/1-core/framework-components/test/mock-family.test.tspackages/1-framework/1-core/framework-components/test/query-plan.types.test-d.tspackages/1-framework/1-core/framework-components/test/run-with-middleware.test.tspackages/1-framework/1-core/framework-components/test/runtime-core.test.tspackages/1-framework/1-core/framework-components/test/runtime-core.types.test-d.tspackages/2-mongo-family/4-query/query-ast/test/query-plan.types.test-d.tspackages/2-mongo-family/5-query-builders/orm/src/collection.tspackages/2-mongo-family/5-query-builders/orm/src/compile.tspackages/2-mongo-family/5-query-builders/orm/src/mongo-raw.tspackages/2-mongo-family/5-query-builders/query-builder/src/builder.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/7-runtime/test/mongo-execution-plan.types.test-d.tspackages/2-mongo-family/7-runtime/test/mongo-middleware.test.tspackages/2-mongo-family/7-runtime/test/raw-commands.test.tspackages/2-mongo-family/7-runtime/test/setup.tspackages/2-sql/4-lanes/relational-core/README.mdpackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/src/plan.tspackages/2-sql/4-lanes/relational-core/src/types.tspackages/2-sql/4-lanes/relational-core/test/ast/builders.test.tspackages/2-sql/4-lanes/relational-core/test/ast/common.test.tspackages/2-sql/4-lanes/relational-core/test/ast/delete.test.tspackages/2-sql/4-lanes/relational-core/test/ast/insert.test.tspackages/2-sql/4-lanes/relational-core/test/ast/rich-ast.test.tspackages/2-sql/4-lanes/relational-core/test/ast/test-helpers.tspackages/2-sql/4-lanes/relational-core/test/ast/update.test.tspackages/2-sql/4-lanes/relational-core/test/plan.test.tspackages/2-sql/4-lanes/relational-core/test/sql-execution-plan.types.test-d.tspackages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.tspackages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.tspackages/2-sql/5-runtime/src/codecs/decoding.tspackages/2-sql/5-runtime/src/codecs/encoding.tspackages/2-sql/5-runtime/src/guardrails/raw.tspackages/2-sql/5-runtime/src/middleware/before-compile-chain.tspackages/2-sql/5-runtime/src/middleware/budgets.tspackages/2-sql/5-runtime/src/sql-runtime.tspackages/2-sql/5-runtime/test/async-iterable-result.test.tspackages/2-sql/5-runtime/test/before-compile-chain.test.tspackages/2-sql/5-runtime/test/budgets.test.tspackages/2-sql/5-runtime/test/codec-async.test.tspackages/2-sql/5-runtime/test/json-schema-validation.test.tspackages/2-sql/5-runtime/test/lints.test.tspackages/2-sql/5-runtime/test/sql-family-adapter.test.tspackages/2-sql/5-runtime/test/sql-runtime.test.tspackages/3-extensions/middleware-telemetry/test/telemetry-middleware.test.tspackages/3-extensions/sql-orm-client/src/query-plan-aggregate.tspackages/3-extensions/sql-orm-client/src/query-plan-meta.tspackages/3-extensions/sql-orm-client/src/query-plan-mutations.tspackages/3-extensions/sql-orm-client/src/query-plan-select.tspackages/3-extensions/sql-orm-client/src/where-binding.tspackages/3-extensions/sql-orm-client/test/collection-mutation-dispatch.test.tspackages/3-extensions/sql-orm-client/test/query-plan-aggregate.test.tspackages/3-extensions/sql-orm-client/test/query-plan-meta.test.tspackages/3-extensions/sql-orm-client/test/query-plan-mutations.test.tspackages/3-extensions/sql-orm-client/test/query-plan-select.test.tspackages/3-extensions/sql-orm-client/test/raw-compiled-query.test.tspackages/3-extensions/sql-orm-client/test/rich-collection.test.tspackages/3-extensions/sql-orm-client/test/rich-query-plans.test.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.tspackages/3-mongo-target/1-mongo-target/test/data-transform.test.tspackages/3-mongo-target/1-mongo-target/test/migration-e2e.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-ops-serializer.dml.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-adapter.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.tspackages/3-targets/6-adapters/postgres/src/core/sql-renderer.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter-lower-parity.test.tspackages/3-targets/6-adapters/postgres/test/migrations/data-transform.test.tspackages/3-targets/6-adapters/postgres/test/rich-adapter.test.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.tstest/e2e/framework/vitest.config.tstest/integration/test/cross-package/cross-family-middleware.test.tstest/integration/test/mongo/expr-filter.test.ts
💤 Files with no reviewable changes (35)
- packages/2-mongo-family/7-runtime/test/mongo-middleware.test.ts
- packages/1-framework/1-core/framework-components/test/runtime-core.types.test-d.ts
- packages/2-mongo-family/7-runtime/test/setup.ts
- packages/2-mongo-family/5-query-builders/orm/src/compile.ts
- test/integration/test/mongo/expr-filter.test.ts
- packages/2-sql/5-runtime/test/lints.test.ts
- packages/2-mongo-family/5-query-builders/query-builder/src/query.ts
- packages/2-mongo-family/5-query-builders/query-builder/src/state-classes.ts
- packages/2-mongo-family/5-query-builders/orm/src/mongo-raw.ts
- packages/1-framework/1-core/framework-components/test/runtime-core.test.ts
- packages/2-mongo-family/5-query-builders/orm/src/collection.ts
- packages/3-mongo-target/2-mongo-adapter/test/mongo-adapter.test.ts
- packages/3-mongo-target/1-mongo-target/test/data-transform.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/rich-ast.test.ts
- packages/1-framework/1-core/framework-components/test/run-with-middleware.test.ts
- packages/2-mongo-family/4-query/query-ast/test/query-plan.types.test-d.ts
- packages/3-targets/6-adapters/postgres/test/migrations/data-transform.test.ts
- packages/3-extensions/middleware-telemetry/test/telemetry-middleware.test.ts
- packages/3-extensions/sql-orm-client/test/rich-collection.test.ts
- packages/2-mongo-family/7-runtime/test/mongo-execution-plan.types.test-d.ts
- packages/2-sql/5-runtime/test/async-iterable-result.test.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.test.ts
- packages/3-extensions/sql-orm-client/test/raw-compiled-query.test.ts
- packages/1-framework/0-foundation/contract/src/exports/types.ts
- packages/3-extensions/sql-orm-client/test/collection-mutation-dispatch.test.ts
- test/integration/test/cross-package/cross-family-middleware.test.ts
- packages/1-framework/1-core/framework-components/test/mock-family.test.ts
- packages/2-sql/4-lanes/relational-core/test/plan.test.ts
- packages/2-sql/5-runtime/test/sql-family-adapter.test.ts
- packages/2-mongo-family/7-runtime/test/raw-commands.test.ts
- packages/2-sql/4-lanes/relational-core/src/plan.ts
- packages/3-mongo-target/1-mongo-target/src/core/mongo-ops-serializer.ts
- packages/2-mongo-family/5-query-builders/query-builder/src/builder.ts
- packages/2-sql/4-lanes/relational-core/test/sql-execution-plan.types.test-d.ts
- packages/1-framework/1-core/framework-components/test/query-plan.types.test-d.ts
Comment on lines
+47
to
55
| const projection = projectionListFromAst(plan.ast); | ||
| if (!projection) { | ||
| return { | ||
| aliases: undefined, | ||
| codecs: new Map(), | ||
| columnRefs: new Map(), | ||
| includeAliases: EMPTY_INCLUDE_ALIASES, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Treat an empty projection list like “no projection metadata”.
When projectionListFromAst(plan.ast) is [], this builds aliases: [], and decodeRow() returns {} for every row because it iterates zero aliases. That breaks AST-backed plans that intentionally use withProjection([]) as a valid shape, including fixtures in packages/2-sql/5-runtime/test/sql-runtime.test.ts and packages/2-sql/5-runtime/test/before-compile-chain.test.ts. Falling back to aliases: undefined for empty projections preserves the raw row keys until there is explicit projection metadata.
Suggested fix
const projection = projectionListFromAst(plan.ast);
- if (!projection) {
+ if (!projection || projection.length === 0) {
return {
aliases: undefined,
codecs: new Map(),
columnRefs: new Map(),
includeAliases: EMPTY_INCLUDE_ALIASES,
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/5-runtime/src/codecs/decoding.ts` around lines 47 - 55, The
current logic treats an empty projection list (projection === []) as valid
metadata, causing aliases to be set to an empty array and decodeRow to produce
empty objects; update the early-return after calling
projectionListFromAst(plan.ast) so that it treats both falsy and empty-array
results as "no projection metadata" (i.e., when projection is null/undefined OR
projection.length === 0) and return aliases: undefined (and the same empty
codecs/columnRefs/includeAliases) so decodeRow preserves raw row keys; locate
the check around projectionListFromAst and adjust the condition accordingly for
projection and downstream use in decodeRow/withProjection handling.
Contributor
Author
There was a problem hiding this comment.
withProjection([]) will produce invalid SQL, it does not matter what metadata it has
There was a problem hiding this comment.
@SevInf, that's a fair point — if withProjection([]) produces invalid SQL, the query never executes and decodeRow is never reached with an empty projection. My concern was a false positive. The current code is correct as-is.
🧠 Learnings used
Learnt from: wmadden
Repo: prisma/prisma-next PR: 38
File: packages/2-sql/3-tooling/family/src/core/migrations/plan-helpers.ts:40-47
Timestamp: 2025-12-26T23:02:40.679Z
Learning: In Prisma-next's TypeScript codebase, prefer using well-established utility libraries (e.g., Lodash) rather than building custom utilities for complex object operations such as deep freezing or deep cloning. Favor reliable, battle-tested functions and ensure dependencies are reviewed for tree-shaking and bundle size. If a lightweight utility suffices, consider modern native approaches or smaller libs.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 191
File: packages/1-framework/3-tooling/cli/src/commands/contract-emit.ts:5-5
Timestamp: 2026-03-01T13:54:21.863Z
Learning: In the Prisma-next repository, prefer importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files). Apply this in TypeScript files, replacing imports from 'node:path' with 'pathe' and adjusting API usage accordingly. Ensure consistent usage across all modules and update any affected tests or tooling imports when refactoring.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 234
File: packages/2-sql/4-lanes/sql-lane/test/rich-mutation.test.ts:2-2
Timestamp: 2026-03-23T10:01:15.075Z
Learning: In the prisma/prisma-next repo, prefer using `pathe` over Node’s built-in `node:path`. For any TypeScript file (including tests, test helpers, and integration tooling) replace imports like `import { dirname, join } from 'node:path'` with `import { dirname, join } from 'pathe'`—especially when code reads files/fixtures from disk.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 294
File: packages/2-mongo-family/2-query/query-ast/src/read-plan.ts:4-10
Timestamp: 2026-04-04T10:08:34.220Z
Learning: In prisma/prisma-next, don’t flag declaration-emit/typecheck warnings/errors when a file uses an intentional nominal/branding pattern with a non-exported `declare const <brand>: unique symbol` as a computed property key on an exported interface (e.g., `readonly [__mongoReadPlanRow]: ...` / `readonly [aggregateResultBrand]: ...`). The `unique symbol` must remain unexported to prevent external forging, while the branded property key staying on the exported interface provides compile-time discrimination. If the build correctly emits declaration files for this pattern, treat it as intentional rather than a declaration-emit error.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 293
File: packages/1-framework/1-core/shared/contract/src/canonicalization.ts:203-216
Timestamp: 2026-04-04T12:19:05.250Z
Learning: In the prisma/prisma-next repository, do not treat `Array.prototype.sort` comparators as “potentially non-deterministic” solely because the comparator does not include explicit tie-breaking keys. This is because the repo’s required runtime is Node >= 24, where `Array.prototype.sort` is stable (equal elements retain their original relative order via a stable sort such as TimSort). Therefore, equal-comparing elements should remain deterministic without additional tie-break logic.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 334
File: packages/2-mongo-family/9-family/src/core/control-instance.ts:204-228
Timestamp: 2026-04-13T15:54:52.337Z
Learning: When reviewing TypeScript code in this repo, do not assume a property is potentially `undefined` at a call site just because an arktype schema definition uses the DSL syntax `'key?': 'type'` (a quoted string key with a trailing `?`). This DSL notation is not the same as TypeScript optional properties (`key?: type`). To determine whether `key` is truly optional/undefined-capable, verify the actual exported TypeScript type declaration (e.g., `contract-types.ts`) and the runtime validation schema (e.g., `validate-contract.ts`) for that property; only then should the review flag call-site handling for potential `undefined`.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 339
File: test/integration/test/cli.init-templates.e2e.test.ts:96-96
Timestamp: 2026-04-15T19:33:48.733Z
Learning: In prisma/prisma-next, when writing tests that run TypeScript compilation (e.g., via `tsc --noEmit`), avoid hardcoded timeout numbers (such as `30_000`). Instead, use `prisma-next/test-utils` timeout helpers—specifically `timeouts.typeScriptCompilation` (implemented in `test/utils/src/timeouts.ts`) for `tsc` compilation scenarios. Use `timeouts.spinUpPpgDev` for PostgreSQL server startup and `timeouts.databaseOperation` for database operation scenarios.
Learnt from: saevarb
Repo: prisma/prisma-next PR: 394
File: packages/1-framework/3-tooling/migration/src/graph-ops.ts:50-54
Timestamp: 2026-04-29T12:44:59.376Z
Learning: In TypeScript overload implementations, an internal “implementation-only” cast (e.g., `state as unknown as <ReturnType>`) can be safe and intentional when the public overload signatures and their structural typing prevent callers from reaching the implementation with an incompatible type. In these cases, reviewers should not flag the cast as a blind/unsafe cast, since compile-time overload enforcement substitutes for a runtime type guard. Don’t require an added `typeof`/runtime check solely to silence the cast when the overload design already prevents invalid inputs.
Comment on lines
+85
to
88
| const codec: Codec | undefined = registry.get(metadata.codecId); | ||
| if (!codec) { | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Fail fast when a stamped codec is missing.
If ParamRef.codecId is present, returning the raw value here turns a runtime/configuration error into silent passthrough. That can skip required transforms like encryption or normalization and send the wrong wire shape to the driver.
Possible fix
const codec: Codec | undefined = registry.get(metadata.codecId);
if (!codec) {
- return value;
+ const label = paramLabel(metadata, paramIndex);
+ throw runtimeError(
+ 'RUNTIME.ENCODE_FAILED',
+ `Failed to encode parameter ${label} with codec '${metadata.codecId}': codec is not registered`,
+ { label, codec: metadata.codecId, paramIndex },
+ );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/5-runtime/src/codecs/encoding.ts` around lines 85 - 88, When a
stamped codec is missing, fail fast instead of returning the raw value: in the
branch where you call registry.get(metadata.codecId) and receive undefined,
check whether metadata.codecId is set and, if so, throw a descriptive error
(including the missing metadata.codecId and relevant param/ref identifier)
rather than returning value; keep the existing behavior of returning value only
when no codecId is present. This change should be applied where codec is
obtained (registry.get), using the symbols metadata.codecId, codec and the
surrounding encoding/encode function logic.
Contributor
Author
There was a problem hiding this comment.
Eventually we will want to make codecId required and the problem will go away
There was a problem hiding this comment.
@SevInf, understood — the current soft passthrough is consistent with codecId being intentionally optional for now (as noted in the PR description). Once codecId is made required on ProjectionItem (and ParamRef), the "codec stamped but not registered" case becomes a genuine misconfiguration and the fail-fast path should be added at that point.
It might be worth dropping a // TODO: throw when codecId is present but codec is not registered (once codecId is required) comment in the if (!codec) branch so the future hardening is discoverable, but that's entirely up to you.
✏️ Learnings added
Learnt from: SevInf
Repo: prisma/prisma-next PR: 393
File: packages/2-sql/5-runtime/src/codecs/encoding.ts:85-88
Timestamp: 2026-04-30T09:39:58.142Z
Learning: In prisma/prisma-next, `ProjectionItem.codecId` and `ParamRef.codecId` are intentionally optional for now (per ADR 205 and PR `#393`). The encoding path (`encodeParamValue` in `packages/2-sql/5-runtime/src/codecs/encoding.ts`) silently passes through the raw value when a stamped `codecId` cannot be resolved in the registry — this is by design until `codecId` is made required. Do not flag this as an unconditional bug; the fail-fast behaviour should only be enforced once `codecId` is required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: wmadden
Repo: prisma/prisma-next PR: 38
File: packages/2-sql/3-tooling/family/src/core/migrations/plan-helpers.ts:40-47
Timestamp: 2025-12-26T23:02:40.679Z
Learning: In Prisma-next's TypeScript codebase, prefer using well-established utility libraries (e.g., Lodash) rather than building custom utilities for complex object operations such as deep freezing or deep cloning. Favor reliable, battle-tested functions and ensure dependencies are reviewed for tree-shaking and bundle size. If a lightweight utility suffices, consider modern native approaches or smaller libs.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 191
File: packages/1-framework/3-tooling/cli/src/commands/contract-emit.ts:5-5
Timestamp: 2026-03-01T13:54:21.863Z
Learning: In the Prisma-next repository, prefer importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files). Apply this in TypeScript files, replacing imports from 'node:path' with 'pathe' and adjusting API usage accordingly. Ensure consistent usage across all modules and update any affected tests or tooling imports when refactoring.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 234
File: packages/2-sql/4-lanes/sql-lane/test/rich-mutation.test.ts:2-2
Timestamp: 2026-03-23T10:01:15.075Z
Learning: In the prisma/prisma-next repo, prefer using `pathe` over Node’s built-in `node:path`. For any TypeScript file (including tests, test helpers, and integration tooling) replace imports like `import { dirname, join } from 'node:path'` with `import { dirname, join } from 'pathe'`—especially when code reads files/fixtures from disk.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 294
File: packages/2-mongo-family/2-query/query-ast/src/read-plan.ts:4-10
Timestamp: 2026-04-04T10:08:34.220Z
Learning: In prisma/prisma-next, don’t flag declaration-emit/typecheck warnings/errors when a file uses an intentional nominal/branding pattern with a non-exported `declare const <brand>: unique symbol` as a computed property key on an exported interface (e.g., `readonly [__mongoReadPlanRow]: ...` / `readonly [aggregateResultBrand]: ...`). The `unique symbol` must remain unexported to prevent external forging, while the branded property key staying on the exported interface provides compile-time discrimination. If the build correctly emits declaration files for this pattern, treat it as intentional rather than a declaration-emit error.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 293
File: packages/1-framework/1-core/shared/contract/src/canonicalization.ts:203-216
Timestamp: 2026-04-04T12:19:05.250Z
Learning: In the prisma/prisma-next repository, do not treat `Array.prototype.sort` comparators as “potentially non-deterministic” solely because the comparator does not include explicit tie-breaking keys. This is because the repo’s required runtime is Node >= 24, where `Array.prototype.sort` is stable (equal elements retain their original relative order via a stable sort such as TimSort). Therefore, equal-comparing elements should remain deterministic without additional tie-break logic.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 334
File: packages/2-mongo-family/9-family/src/core/control-instance.ts:204-228
Timestamp: 2026-04-13T15:54:52.337Z
Learning: When reviewing TypeScript code in this repo, do not assume a property is potentially `undefined` at a call site just because an arktype schema definition uses the DSL syntax `'key?': 'type'` (a quoted string key with a trailing `?`). This DSL notation is not the same as TypeScript optional properties (`key?: type`). To determine whether `key` is truly optional/undefined-capable, verify the actual exported TypeScript type declaration (e.g., `contract-types.ts`) and the runtime validation schema (e.g., `validate-contract.ts`) for that property; only then should the review flag call-site handling for potential `undefined`.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 339
File: test/integration/test/cli.init-templates.e2e.test.ts:96-96
Timestamp: 2026-04-15T19:33:48.733Z
Learning: In prisma/prisma-next, when writing tests that run TypeScript compilation (e.g., via `tsc --noEmit`), avoid hardcoded timeout numbers (such as `30_000`). Instead, use `prisma-next/test-utils` timeout helpers—specifically `timeouts.typeScriptCompilation` (implemented in `test/utils/src/timeouts.ts`) for `tsc` compilation scenarios. Use `timeouts.spinUpPpgDev` for PostgreSQL server startup and `timeouts.databaseOperation` for database operation scenarios.
Learnt from: saevarb
Repo: prisma/prisma-next PR: 394
File: packages/1-framework/3-tooling/migration/src/graph-ops.ts:50-54
Timestamp: 2026-04-29T12:44:59.376Z
Learning: In TypeScript overload implementations, an internal “implementation-only” cast (e.g., `state as unknown as <ReturnType>`) can be safe and intentional when the public overload signatures and their structural typing prevent callers from reaching the implementation with an incompatible type. In these cases, reviewers should not flag the cast as a blind/unsafe cast, since compile-time overload enforcement substitutes for a runtime type guard. Don’t require an added `typeof`/runtime check solely to silence the cast when the overload design already prevents invalid inputs.
wmadden
approved these changes
Apr 30, 2026
Contributor
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
Approving preemptively. The change does what ADR 205 describes: AST as the single source of execution metadata, runBeforeCompileChain correct-by-construction with no paramDescriptors re-derivation, sidecars deleted at the source with no compat shims. The new alias-rename test in before-compile-chain.test.ts is a strong end-to-end pin for the rewrite-then-decode story.
Four structural items are worth addressing or responding to before merge — left as inline comments on the relevant lines. None of them block merge in my view, so no re-review needed: address inline or reply with a deferral, then merge.
Structural (S1–S4):
- S1:
decodeRowsilently masks alias/row mismatch for AST-backed plans (decoded row getsundefinedfor absent keys, no error). - S2: Encoder maps
paramRefsByValueOrder(ast)[i] → plan.params[i]by index without verifyingrefs.length === paramCount— silent miscoding if any of the three independent walks ofcollectParamRefs()ever drift. - S3: Aggregate
ProjectionItems carry nocodecId; the decoder hands wire values through (Postgrescount(*)arrives as a string"42"). Pre-existing, not a regression — but the PR's design (codec-on-projection) makes it fixable in exactly the place it belongs. - S4: No test exercises the alias-differs RETURNING SQL text or codec-decoded RETURNING via
ProjectionItem.codecId. The renderer's<table>.<column> AS <alias>branch is implemented but unverified; AC22 (codec-decoded RETURNING) is reachable in production but unasserted.
Full review (system-design + code-review + walkthrough) is local under wip/review-code/pr-393/. Acceptance-criteria verification: 23 PASS, 2 WEAK (AC14, AC15 — both closed by S4), 1 NOT VERIFIED (AC22 — also closed by S4), 0 FAIL.
Follow-ups already noted in the PR description (e.g. promoting ProjectionItem.codecId to required) are out of scope here.
| @@ -164,6 +167,6 @@ export function compileGroupedAggregate( | |||
| ast = ast.withHaving(validateGroupedHavingExpr(havingExpr)); | |||
Contributor
There was a problem hiding this comment.
S3 — Aggregate ProjectionItems carry no codecId; the decoder passes wire values through.
Line 119 (compileAggregate) and line 154 (compileGroupedAggregate) emit ProjectionItem.of(alias, toAggregateExpr(tableName, selector)) with no codec. The decoder's path for !codec → return wire value as-is (decoding.ts:171–174) means the application sees whatever the driver hands back.
For Postgres, the pg driver returns bigint (count, sum-of-int) as a JavaScript string by default. The application sees "42" instead of 42 / 42n. For min/max/sum/avg of a typed column, the result type is inferable from the underlying column's codec; today it isn't propagated.
This is not a regression introduced by this PR — pre-PR compileAggregate also did not populate codec info for aggregate aliases (the old projectionTypes map likewise did not include them). But the PR's design (codec-on-ProjectionItem) makes this fixable in exactly the place it belongs, and the spec's text — "every ProjectionItem emitted by either producer carries codecId whenever the producer's existing scope-field machinery has type information for that alias" — reads as if it should be addressed here.
Suggestion: Either (a) stamp known aggregate codec IDs at the aggregate construction sites (count → pg/int8@1; sum/avg/min/max → propagate from the underlying column codec via the operations registry the runtime already consults for SQL operations); or (b) explicitly defer with a code comment naming the follow-up. The current state — silently passing aggregate strings back to the application — is a real footgun, even though it pre-dates this PR.
SevInf
force-pushed
the
ast-codec-ids-v2
branch
from
393e62d to
cf5187b
Compare
Make the SQL AST the single source of truth for execution metadata. Remove the `refs`, `paramDescriptors`, `annotations.codecs`, and `projectionTypes` sidecars from `PlanMeta`; promote `ProjectionItem` to carry an optional `codecId`; promote `Insert/Update/Delete.returning` from `ColumnRef[]` to `ProjectionItem[]`; migrate the runtime decoder, encoder, lints middleware, budgets middleware, and Postgres / SQLite RETURNING lowering to read from the AST. AST + contract types - `ProjectionItem` gains an optional `codecId?: string` (constructor, `static of`, `withCodecId`). - `Insert/Update/Delete.returning` is `ReadonlyArray<ProjectionItem>`; the mutation ASTs gain `rewrite()` methods that descend into each `ProjectionItem.expr` of `returning`, mirroring `SelectAst.projection`. `collectParamRefs()` walks the new shape. - `PlanMeta` shrinks to identification + policy fields only; the `paramDescriptors` / `refs` / `projection` / `projectionTypes` / `annotations.codecs` fields are gone, along with `ParamDescriptor` and `PlanRefs` from `@prisma-next/contract`. - `RawTemplateOptions` / `RawFunctionOptions` no longer accept `refs` or `projection`; raw plans pass parameters to the driver as supplied and surface wire-level row values back without codec transformation. - The dead AST traversal helpers (`QueryAst.collectRefs`, `FromSource.collectRefs`, mutation/select `collectRefs` overrides, and the supporting `mergeRefsInto` / `addColumnRefToRefSets` / `sortRefs` helpers) are removed; nothing in production reads them after the sidecars are gone. Producers - `buildQueryPlan` (sql-builder) and `buildOrmQueryPlan` (sql-orm-client) stamp `codecId` onto each emitted `ProjectionItem` (SELECT projections and INSERT/UPDATE/DELETE RETURNING items) using the same scope-field / contract-column lookup that previously populated `projectionTypes`. Both producers stop emitting the four sidecar fields. Runtime - The row decoder builds its alias→codec lookup by walking the AST's projection list (or `returning` list for mutation plans). For plans without an AST (raw plans) the decoder hands wire-level row values to the caller without codec-based transformation. - The parameter encoder reads `codecId` from each `ParamRef` collected off the AST. Raw plans pass parameters to the driver as supplied. - `runBeforeCompileChain` no longer re-derives `paramDescriptors` after AST rewrites — the AST is now the source of truth, so middleware that introduces ParamRefs is correct-by-construction. - The lints middleware runs structural lints only on AST-backed plans; raw plans get the existing SQL-string heuristics. The budgets middleware derives the primary table from `ast.from`; the raw-plan heuristic collapses to a SQL-text LIMIT regex. Raw guardrails drop the refs-based `evaluateIndexCoverage` path. Adapter - The Postgres and SQLite SQL renderers lower RETURNING from `ProjectionItem[]`. For column-ref projections they emit `<table>.<column>` when the alias matches the column name and `<table>.<column> AS <alias>` when it differs; non-`ColumnRef` projection expressions render through the expression renderer with `AS <alias>`. Cross-package cleanup - Mongo query-builder / ORM / state-classes producers and the Mongo plan-JSON serializer drop `paramDescriptors`. Test fixtures across framework-components, middleware-telemetry, mongo-family, mongo-target, and cross-package integration tests are updated to the narrowed `PlanMeta` shape and AST-backed plans. Documentation - ADR 012 carries a banner marking its optional structured-annotations branch (`refs`, `projection`, codec maps) as retired by ADR 205. The minimal annotation schema (`intent`, `isMutation`, `hasWhere`, `hasLimit`) is unchanged. - The driving `projects/execution-metadata-on-ast/` workspace (spec + plan) is included; ADR 205 itself remains canonical under `docs/architecture docs/adrs/`.
Address review feedback on the ADR 205 PR: - budgets middleware: drop the raw-plan SQL-string row-count heuristic (`evaluateWithHeuristics`). After ADR 205, raw plans get no budgets validation; only AST-backed SELECTs get the row-count check. Three obsolete tests in budgets.test.ts removed. - sql-runtime.ts: restore a JSDoc on `runBeforeCompile` describing the no-sidecar reconciliation contract (paramDescriptors re-derivation is no longer needed because the encoder reads from the AST directly). - before-compile-chain.test.ts: rewrite the explanatory comment above the alias-swap end-to-end test to be concrete about the test mechanics (rename, codec effect, what the assertion would prove false). - contract/README.md, relational-core/README.md: drop references to deleted `ParamDescriptor` / `PlanRefs` / `augmentDescriptorWithColumnMeta`; point readers at ADR 205 and the AST-borne codec IDs instead.
Strip "(ADR 205)" / "Per ADR 205, ..." references from the JSDoc on `runBeforeCompile` and the test explanation above the alias-swap end-to-end test in `before-compile-chain.test.ts`. The comments now explain the invariant directly without pointing readers off-site.
- Delete `projects/execution-metadata-on-ast/{plan,spec}.md` from the PR
per the project close-out task; ADR 205 is canonical under
`docs/architecture docs/adrs/`.
- Remove the multi-line explanatory comment above the alias-swap
end-to-end test in `before-compile-chain.test.ts` (PR review).
The DSL builder now stamps `codecId` onto every `ProjectionItem` it emits, so the offline integration test's `.toEqual(ProjectionItem.of( alias, expr))` assertion no longer matches. Update both projection expectations to include the contract codec IDs (`sql/char@1` for `user.id`, `pg/text@1` for `user.email`).
The e2e framework suite ran with `timeouts.default` (100ms × multiplier), which produced 200ms test timeouts on CI workers. Three SQLite migration tests (additive, default-drift, fk-preservation) intermittently exceeded that budget despite running in 11–69ms locally. e2e tests exercise real databases (SQLite/Postgres migrations + DML), so `timeouts.databaseOperation` (5s × multiplier) is the correct semantic ceiling.
Semgrep's `javascript.express.security.audit.remote-property-injection` rule fires on legitimate bracket access where the index is a numeric loop counter (e.g. `metadata[i] ?? NO_METADATA` in `encoding.ts:125`). The rule is rooted in untrusted-input scenarios for Express handlers — not relevant to this repo, which has no HTTP request-borne indices in the surfaces this rule scans. Add it to the existing `--exclude-rule` list alongside the wkhtmltoimage rule so PR review noise doesn't accumulate.
The previous version of the budgets-middleware "reads from FROM clause" test asserted no warning under `maxRows: 10_000, defaultTableRows: 10_000, tableRows.user: 5, limit: 5`. At those values the estimate is 5 whether the FROM lookup hits `tableRows.user` (returning 5) or short-circuits to null (table unresolved). The "no warning" assertion held either way, so the test did not actually verify the invariant in its name. Re-pick parameters so the assertion only passes when the FROM lookup hits `tableRows.user`: `limit: 10`, `maxRows: 4`, `tableRows.user: 5`, `defaultTableRows: 10_000`. Estimate becomes `min(limit=10, tableRows.user=5) = 5`, exceeds maxRows=4, and emits `BUDGET.ROWS_EXCEEDED` with `details.estimatedRows: 5`. If the FROM lookup ever broke, the estimate would be null (no emit) or fall back to defaultTableRows (`min(10, 10_000) = 10`), and the `estimatedRows: 5` assertion would fail.
…ked plans
decodeRow derives `aliases` from the AST projection list and reads
`row[alias]`. Before this change, a row whose keys didn't match the AST
aliases (driver bug, adapter bug, identifier-quoting drift) silently
produced a decoded row with every aliased key set to `undefined` — the
`decodeField` short-circuit treated `wireValue === undefined` the same
as a SQL NULL.
For AST-backed plans only, assert presence of every alias on the row
via `Object.hasOwn` and throw `RUNTIME.DECODE_FAILED` on miss with
`{ alias, expectedAliases, presentKeys }`. Tighten `decodeField`'s
early short-circuit to `wireValue === null` so SQL NULL stays distinct
from "key absent".
Two new tests cover the alias-mismatch error and the preserved-null
behavior. Raw plans (no AST) still fall through to `Object.keys(row)`
and pass-through decode.
…entity-deduped walks Four call sites independently dedupe `ast.collectParamRefs()` by ParamRef identity in first-encounter order to align `plan.params[i]` with metadata-by-index. The encoder relies on all four agreeing on order and count; copy-pasted loops made that invariant fragile. Extract `collectOrderedParamRefs(ast)` in `relational-core/src/ast/util.ts`, exported via `@prisma-next/sql-relational-core/ast`. Switch the four sites to consume it: `deriveParamsFromAst` (orm-client), `buildQueryPlan` (sql-builder), `encodeParams` (sql-runtime, replacing the now-deleted local `paramRefsByValueOrder`), and the Postgres renderer's `$N` index map. SQLite's `?`-placeholder renderer keeps its non-deduped walk: it needs one params entry per occurrence in the SQL.
For min/max aggregates, the result type is identical to the input
column's type, so propagate the underlying column's codecId from the
contract onto the emitted `ProjectionItem`. Both `compileAggregate`
and `compileGroupedAggregate` route through the same
`toAggregateProjection` helper that returns `{ expr, codecId }`.
count, sum, avg are intentionally left with `codecId: undefined`:
- count returns a target-specific bigint (mapping not derivable in
the orm-client without target coupling).
- sum widens (int4 → int8 in Postgres) and avg → numeric; both need
target+input-aware mapping that doesn't yet exist.
Three new tests pin the propagation for min/max (plain + grouped) and
the deferred-undefined state for count/sum/avg, so a future change to
add them is forced through tests.
Two coverage gaps in the previous test suite:
1. Postgres + SQLite renderers' `<table>.<column> AS <alias>`
RETURNING branch was reached only by alias-equals-column tests, so
the AS-suffix path was implemented but unverified.
2. The decoder treats SELECT projections and mutation RETURNING
symmetrically, but no test specifically constructed a mutation AST
with a codec on a `returning` ProjectionItem and asserted decodeRow
applied that codec to the wire value.
Add three tests:
- adapter-postgres `rich-adapter.test.ts`: assert
`RETURNING "user"."id" AS "user_id"` for an Insert AST whose
returning alias differs from the underlying column.
- adapter-sqlite `adapter.test.ts`: SQLite parity (same shape with `?`
placeholders).
- sql-runtime `before-compile-chain.test.ts`: build an Insert AST
with `ProjectionItem.of('id', ColumnRef.of(...), 'pg/int4@1')` on
returning, run decodeRow against a wire row, and assert the codec
fired (decode adds 100, so wire 7 → decoded 107).
The new helper extracted in the previous commit had no direct unit tests, dropping `src/ast/util.ts` coverage below the package thresholds (statements 50% < 96%, branches 80% < 91%, functions 50% < 95%). Add four tests covering depth-first ordering, identity deduplication, distinct instances with equal values, and the empty-AST frozen-array path. Coverage now reads 100% across all metrics.
SevInf
force-pushed
the
ast-codec-ids-v2
branch
from
83eea8c to
f8ad1c1
Compare
cursor Bot
pushed a commit
that referenced
this pull request
Apr 30, 2026
Biome organize-imports rule expects type/value imports interleaved alphabetically rather than grouped. Resolves CI Lint failure introduced by the post-#393 rebase merge of three import sources.
cursor Bot
pushed a commit
that referenced
this pull request
May 1, 2026
Biome organize-imports rule expects type/value imports interleaved alphabetically rather than grouped. Resolves CI Lint failure introduced by the post-#393 rebase merge of three import sources.
cursor Bot
pushed a commit
that referenced
this pull request
May 1, 2026
Biome organize-imports rule expects type/value imports interleaved alphabetically rather than grouped. Resolves CI Lint failure introduced by the post-#393 rebase merge of three import sources.
wmadden
pushed a commit
that referenced
this pull request
May 1, 2026
Biome organize-imports rule expects type/value imports interleaved alphabetically rather than grouped. Resolves CI Lint failure introduced by the post-#393 rebase merge of three import sources.
wmadden
pushed a commit
that referenced
this pull request
May 4, 2026
closes [TML-2311](https://linear.app/prisma-company/issue/TML-2311/pn-execution-metadata-lives-on-ast) ## Intent Make the SQL AST the single source of truth for execution metadata. Today two channels carry it: the AST itself, and a sidecar bag on `PlanMeta` (`refs`, `paramDescriptors`, `annotations.codecs`, `projectionTypes`). Both builder paths populate the sidecar by walking the AST and flattening per-node info into per-alias and per-index maps; the runtime then chooses inconsistently which channel to read. With the `beforeCompile` middleware hook now in main (PR #373), AST rewrites silently invalidate every alias-keyed or index-keyed sidecar entry, and the recently merged `re-derive paramDescriptors after middleware AST rewrite` workaround lives exactly to paper over that drift. ADR 205 collapses the two channels into one. `ProjectionItem` carries the projection's output codec; `ParamRef` already carries its parameter codec. The decoder builds its alias→codec lookup by walking the AST; the encoder reads from `ParamRef.codecId`. `PlanMeta` shrinks to identification + policy fields. Raw plans — which have no AST — pass parameters and rows through to the driver and back, consistent with the escape-hatch contract from ADR 012. ## Change map - **AST + contract types** — `packages/1-framework/0-foundation/contract/src/types.ts`, `packages/2-sql/4-lanes/relational-core/src/ast/types.ts`, `packages/2-sql/4-lanes/relational-core/src/types.ts`. `ProjectionItem.codecId` is now optional. `Insert/Update/Delete.returning` is `ProjectionItem[]`. Mutation ASTs gain `rewrite()` methods that descend into `returning`. `PlanMeta` loses `paramDescriptors`/`refs`/`projection`/`projectionTypes`/`annotations.codecs`. `ParamDescriptor` and `PlanRefs` are deleted from the contract package. `RawTemplateOptions` loses `refs` and `projection`. The dead AST traversal helpers (`collectRefs` on `QueryAst`/`FromSource`/all subclasses, plus `mergeRefsInto`/`addColumnRefToRefSets`/`sortRefs`) are removed too. - **Producers** — `packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts`, `packages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.ts`, `packages/3-extensions/sql-orm-client/src/query-plan-{meta,select,mutations,aggregate}.ts`, `packages/3-extensions/sql-orm-client/src/where-binding.ts`. Every emitted `ProjectionItem` is now stamped with the codec ID the producer already had in scope (column lookup for SELECTs and RETURNINGs, scope-field lookup for the DSL builder). Both producers stop emitting the four sidecar fields. - **Runtime** — `packages/2-sql/5-runtime/src/codecs/{decoding,encoding}.ts`, `packages/2-sql/5-runtime/src/middleware/{before-compile-chain,budgets}.ts`, `packages/2-sql/5-runtime/src/guardrails/raw.ts`. The decoder walks the AST's projection/returning list to build its alias→codec lookup; the encoder reads `codecId` off each `ParamRef` collected from the AST. `runBeforeCompileChain` no longer re-derives `paramDescriptors`. The budgets middleware derives the primary table from `ast.from` instead of `meta.refs.tables[0]`; the raw heuristic collapses to a SQL-text LIMIT regex. Raw guardrails drop the refs-based index-coverage path. - **Adapter** — `packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts`, `packages/3-targets/6-adapters/sqlite/src/core/adapter.ts`. Postgres and SQLite render RETURNING from `ProjectionItem[]` with alias-aware emission: `<table>.<column>` when alias matches column, `<table>.<column> AS <alias>` when it differs, `<expr> AS <alias>` for non-`ColumnRef` projections. - **Cross-package fixture cleanup** — Mongo query-builder/ORM/state-classes producers and the Mongo plan-JSON serializer drop `paramDescriptors`. Test fixtures across framework-components, middleware-telemetry, mongo-family, mongo-target, and cross-package integration tests are updated to the narrowed `PlanMeta` shape. - **Docs + project workspace** — `docs/architecture docs/adrs/ADR 012 - Raw SQL Escape Hatch.md` carries a banner marking its optional structured-annotations branch retired by ADR 205. The driving spec + plan ship under `projects/execution-metadata-on-ast/`. ## The story The change starts at the AST. `ProjectionItem` previously carried only `(alias, expr)`; ADR 205 makes its codec ID a first-class field on the node. The mutation `returning` lists were `ColumnRef[]` — no alias, no codec — and now they are `ProjectionItem[]`, putting RETURNING outputs on the same footing as SELECT projections. To keep `beforeCompile` rewrites correct-by-construction, the mutation ASTs gain `rewrite()` methods that descend into each `ProjectionItem.expr` of `returning`, mirroring `SelectAst.projection`'s long-standing rewrite path. Once the AST can carry the metadata, the producers stop emitting the sidecar. `buildOrmQueryPlan` previously called `resolveProjectionCodecs(contract, ast)` to flatten projections into a `projectionTypes` map; that work moves up to the construction sites where the codec is already in hand (`buildProjection`, `buildReturningColumns`, the aggregate-projection builder, the DSL `select(...)` resolver). `buildQueryPlan` in the SQL builder lane likewise stamps codecs at projection-construction time and stops walking the row-fields map for sidecar purposes. `where-binding`'s `bindSelectAst` preserves `codecId` when it rebuilds projections so the rebound AST stays equivalent. The runtime then collapses two consumers to one source. The decoder used to read `meta.annotations.codecs` first, then fall back to `meta.projectionTypes`, then build a column-ref index from `meta.refs.columns` for error messages and JSON-Schema validator keys. After this PR it walks the AST's projection (or returning) list once per plan, builds an alias→codec lookup and an alias→`{table, column}` map from `ProjectionItem.expr` when the expression is a `ColumnRef`, and recognises subquery / json-array-agg projections as include aggregates (replacing the old `include:`-prefixed string convention in `meta.projection`). For raw plans — no AST — the decoder hands wire values straight back. The encoder follows the same pattern: ordered, deduplicated `ast.collectParamRefs()` produces metadata aligned with `plan.params`, raw plans pass values through. `runBeforeCompileChain` had a comment-block workaround explaining that AST rewrites invalidate `paramDescriptors` and that we re-derive them. With descriptors gone, that whole workaround is gone — the AST is what the encoder reads, so middleware that introduces ParamRefs cannot drift. The lints and budgets middleware lose their `refs`-driven paths. `evaluateAstLints` already covered the AST-backed cases; raw plans now get only the SQL-string heuristics ADR 205 expressly degrades to. The budgets middleware reads the primary table off `ast.from`; the raw fallback collapses to a LIMIT-regex check rather than chasing `meta.refs.tables[0]`. The Postgres and SQLite renderers swap their `RETURNING <ColumnRef>[]` lowering for `ProjectionItem[]`-aware rendering, emitting `AS <alias>` only when the alias diverges from the column name to keep the snapshot diff minimal. Test fixtures that hand-constructed plans with `meta: { paramDescriptors: [...], projectionTypes: {...}, refs: {...} }` migrate to either pass an AST holding the equivalent `ParamRef.codecId` / `ProjectionItem.codecId`, or — where they were exercising raw-plan encoding — drop the codec-driven assertion since raw plans no longer codec-encode. ## Behavior changes & evidence - **Decoded values, encoded parameters, lints, and budget evaluations** for AST-backed plans are unchanged. The decoder finds the same codec for the same alias; the encoder finds the same codec for the same param. ([codecs/decoding.ts](packages/2-sql/5-runtime/src/codecs/decoding.ts), [codecs/encoding.ts](packages/2-sql/5-runtime/src/codecs/encoding.ts), [middleware/budgets.ts](packages/2-sql/5-runtime/src/middleware/budgets.ts)) - **RETURNING SQL text changes only when an alias differs from the underlying column name** — the `AS <alias>` suffix is now emitted in that case. The alias-matches-column case is byte-identical to the old output. ([postgres/src/core/sql-renderer.ts](packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts), [sqlite/src/core/adapter.ts](packages/3-targets/6-adapters/sqlite/src/core/adapter.ts)) - **Raw plans drop per-parameter codec encoding and per-alias codec decoding.** Index-coverage lints and the `refs.tables[0]` row-count heuristic that depended on raw-plan annotations are removed; `select-star`, `missing LIMIT`, and `mutation-without-WHERE` SQL-string heuristics still run on raw plans. ([guardrails/raw.ts](packages/2-sql/5-runtime/src/guardrails/raw.ts)) - **`beforeCompile` AST rewrites that introduce, swap, or remove ParamRefs no longer require any sidecar maintenance.** A new test in `before-compile-chain.test.ts` asserts that a `beforeCompile` rewriter that swaps a projection alias decodes correctly end-to-end through the AST-driven decoder. ([before-compile-chain.test.ts](packages/2-sql/5-runtime/test/before-compile-chain.test.ts)) ## Compatibility / migration / risk - **No backwards-compatibility shims.** Per repo policy, every in-repo call site that constructed a plan with the removed sidecars is updated at the point of change. `ParamDescriptor` and `PlanRefs` are deleted outright, not deprecated. - **Plan identity is unaffected.** ADR 013 already excluded the removed fields from identity hashing. - **Telemetry is unaffected.** Timing, row counts, and error codes flow through paths independent of the removed sidecars. - **Mongo family changes are limited to dropping `paramDescriptors` from PlanMeta call sites.** Cross-family invariant ("AST is the source when one exists") applies family-wide; specific Mongo-family migration of ProjectionItem-style metadata is separate work. ## Follow-ups / open questions - The `projects/execution-metadata-on-ast/` workspace ships with the PR for reviewability; it can be deleted in a follow-up close-out commit. ADR 205 is canonical under `docs/architecture docs/adrs/`. - `ProjectionItem.codecId` is optional. Once all in-repo producers stamp it (this PR), it's a candidate for promotion to required in a future change. Out of scope here. ## Non-goals / intentionally out of scope - Restoring the unindexed-predicate lint or the refs-based row-count budget for raw plans. ADR 205 accepts this as a known degradation. - New lints on top of the AST. Lint coverage stays at parity (or strictly degrades for raw plans only). - The ADR 012 minimal annotation schema (`intent`, `isMutation`, `hasWhere`, `hasLimit`). Unchanged. - Cross-family Mongo migration of execution metadata onto the Mongo AST. Tracked separately. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Execution metadata (codec IDs, param info, projection mapping) is now carried on AST nodes, not plan metadata. * **Behavior Changes** * Raw SQL plans forward caller parameters and return wire rows unchanged (no automatic codec encoding/decoding). * Linting and row-budget heuristics now run only for AST-backed plans; raw plans use existing SQL-text heuristics. * **Documentation** * ADR and architecture notes updated to reflect the new metadata and behavioral expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
wmadden
pushed a commit
that referenced
this pull request
May 4, 2026
Biome organize-imports rule expects type/value imports interleaved alphabetically rather than grouped. Resolves CI Lint failure introduced by the post-#393 rebase merge of three import sources.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes TML-2311
Intent
Make the SQL AST the single source of truth for execution metadata. Today two channels carry it: the AST itself, and a sidecar bag on
PlanMeta(refs,paramDescriptors,annotations.codecs,projectionTypes). Both builder paths populate the sidecar by walking the AST and flattening per-node info into per-alias and per-index maps; the runtime then chooses inconsistently which channel to read. With thebeforeCompilemiddleware hook now in main (PR #373), AST rewrites silently invalidate every alias-keyed or index-keyed sidecar entry, and the recently mergedre-derive paramDescriptors after middleware AST rewriteworkaround lives exactly to paper over that drift.ADR 205 collapses the two channels into one.
ProjectionItemcarries the projection's output codec;ParamRefalready carries its parameter codec. The decoder builds its alias→codec lookup by walking the AST; the encoder reads fromParamRef.codecId.PlanMetashrinks to identification + policy fields. Raw plans — which have no AST — pass parameters and rows through to the driver and back, consistent with the escape-hatch contract from ADR 012.Change map
packages/1-framework/0-foundation/contract/src/types.ts,packages/2-sql/4-lanes/relational-core/src/ast/types.ts,packages/2-sql/4-lanes/relational-core/src/types.ts.ProjectionItem.codecIdis now optional.Insert/Update/Delete.returningisProjectionItem[]. Mutation ASTs gainrewrite()methods that descend intoreturning.PlanMetalosesparamDescriptors/refs/projection/projectionTypes/annotations.codecs.ParamDescriptorandPlanRefsare deleted from the contract package.RawTemplateOptionslosesrefsandprojection. The dead AST traversal helpers (collectRefsonQueryAst/FromSource/all subclasses, plusmergeRefsInto/addColumnRefToRefSets/sortRefs) are removed too.packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts,packages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.ts,packages/3-extensions/sql-orm-client/src/query-plan-{meta,select,mutations,aggregate}.ts,packages/3-extensions/sql-orm-client/src/where-binding.ts. Every emittedProjectionItemis now stamped with the codec ID the producer already had in scope (column lookup for SELECTs and RETURNINGs, scope-field lookup for the DSL builder). Both producers stop emitting the four sidecar fields.packages/2-sql/5-runtime/src/codecs/{decoding,encoding}.ts,packages/2-sql/5-runtime/src/middleware/{before-compile-chain,budgets}.ts,packages/2-sql/5-runtime/src/guardrails/raw.ts. The decoder walks the AST's projection/returning list to build its alias→codec lookup; the encoder readscodecIdoff eachParamRefcollected from the AST.runBeforeCompileChainno longer re-derivesparamDescriptors. The budgets middleware derives the primary table fromast.frominstead ofmeta.refs.tables[0]; the raw heuristic collapses to a SQL-text LIMIT regex. Raw guardrails drop the refs-based index-coverage path.packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts,packages/3-targets/6-adapters/sqlite/src/core/adapter.ts. Postgres and SQLite render RETURNING fromProjectionItem[]with alias-aware emission:<table>.<column>when alias matches column,<table>.<column> AS <alias>when it differs,<expr> AS <alias>for non-ColumnRefprojections.paramDescriptors. Test fixtures across framework-components, middleware-telemetry, mongo-family, mongo-target, and cross-package integration tests are updated to the narrowedPlanMetashape.docs/architecture docs/adrs/ADR 012 - Raw SQL Escape Hatch.mdcarries a banner marking its optional structured-annotations branch retired by ADR 205. The driving spec + plan ship underprojects/execution-metadata-on-ast/.The story
The change starts at the AST.
ProjectionItempreviously carried only(alias, expr); ADR 205 makes its codec ID a first-class field on the node. The mutationreturninglists wereColumnRef[]— no alias, no codec — and now they areProjectionItem[], putting RETURNING outputs on the same footing as SELECT projections. To keepbeforeCompilerewrites correct-by-construction, the mutation ASTs gainrewrite()methods that descend into eachProjectionItem.exprofreturning, mirroringSelectAst.projection's long-standing rewrite path.Once the AST can carry the metadata, the producers stop emitting the sidecar.
buildOrmQueryPlanpreviously calledresolveProjectionCodecs(contract, ast)to flatten projections into aprojectionTypesmap; that work moves up to the construction sites where the codec is already in hand (buildProjection,buildReturningColumns, the aggregate-projection builder, the DSLselect(...)resolver).buildQueryPlanin the SQL builder lane likewise stamps codecs at projection-construction time and stops walking the row-fields map for sidecar purposes.where-binding'sbindSelectAstpreservescodecIdwhen it rebuilds projections so the rebound AST stays equivalent.The runtime then collapses two consumers to one source. The decoder used to read
meta.annotations.codecsfirst, then fall back tometa.projectionTypes, then build a column-ref index frommeta.refs.columnsfor error messages and JSON-Schema validator keys. After this PR it walks the AST's projection (or returning) list once per plan, builds an alias→codec lookup and an alias→{table, column}map fromProjectionItem.exprwhen the expression is aColumnRef, and recognises subquery / json-array-agg projections as include aggregates (replacing the oldinclude:-prefixed string convention inmeta.projection). For raw plans — no AST — the decoder hands wire values straight back. The encoder follows the same pattern: ordered, deduplicatedast.collectParamRefs()produces metadata aligned withplan.params, raw plans pass values through.runBeforeCompileChainhad a comment-block workaround explaining that AST rewrites invalidateparamDescriptorsand that we re-derive them. With descriptors gone, that whole workaround is gone — the AST is what the encoder reads, so middleware that introduces ParamRefs cannot drift.The lints and budgets middleware lose their
refs-driven paths.evaluateAstLintsalready covered the AST-backed cases; raw plans now get only the SQL-string heuristics ADR 205 expressly degrades to. The budgets middleware reads the primary table offast.from; the raw fallback collapses to a LIMIT-regex check rather than chasingmeta.refs.tables[0]. The Postgres and SQLite renderers swap theirRETURNING <ColumnRef>[]lowering forProjectionItem[]-aware rendering, emittingAS <alias>only when the alias diverges from the column name to keep the snapshot diff minimal.Test fixtures that hand-constructed plans with
meta: { paramDescriptors: [...], projectionTypes: {...}, refs: {...} }migrate to either pass an AST holding the equivalentParamRef.codecId/ProjectionItem.codecId, or — where they were exercising raw-plan encoding — drop the codec-driven assertion since raw plans no longer codec-encode.Behavior changes & evidence
AS <alias>suffix is now emitted in that case. The alias-matches-column case is byte-identical to the old output. (postgres/src/core/sql-renderer.ts, sqlite/src/core/adapter.ts)refs.tables[0]row-count heuristic that depended on raw-plan annotations are removed;select-star,missing LIMIT, andmutation-without-WHERESQL-string heuristics still run on raw plans. (guardrails/raw.ts)beforeCompileAST rewrites that introduce, swap, or remove ParamRefs no longer require any sidecar maintenance. A new test inbefore-compile-chain.test.tsasserts that abeforeCompilerewriter that swaps a projection alias decodes correctly end-to-end through the AST-driven decoder. (before-compile-chain.test.ts)Compatibility / migration / risk
ParamDescriptorandPlanRefsare deleted outright, not deprecated.paramDescriptorsfrom PlanMeta call sites. Cross-family invariant ("AST is the source when one exists") applies family-wide; specific Mongo-family migration of ProjectionItem-style metadata is separate work.Follow-ups / open questions
projects/execution-metadata-on-ast/workspace ships with the PR for reviewability; it can be deleted in a follow-up close-out commit. ADR 205 is canonical underdocs/architecture docs/adrs/.ProjectionItem.codecIdis optional. Once all in-repo producers stamp it (this PR), it's a candidate for promotion to required in a future change. Out of scope here.Non-goals / intentionally out of scope
intent,isMutation,hasWhere,hasLimit). Unchanged.Summary by CodeRabbit
New Features
Behavior Changes
Documentation