TML-2893: resolve pass + ResolvedDocument semantic layer over the CST#818
Closed
SevInf wants to merge 3 commits into
Closed
TML-2893: resolve pass + ResolvedDocument semantic layer over the CST#818SevInf wants to merge 3 commits into
SevInf wants to merge 3 commits into
Conversation
Introduce a target-agnostic resolved view over the fault-tolerant CST: a `resolve(document): ResolvedDocument` pass that walks the tree once and returns per-namespace, name-keyed `ReadonlyMap` stores (models, enums, composite types, extension blocks), a document-level `namedTypes` map, and a diagnostics list. Every field/named-type reference resolves to a `TypeTarget` (scalar / ref with a kind-ful `DeclCoord` / crossSpace carrying coordinates but no DeclKind / constructor / unresolved). Declaration stores are first-declaration-wins on duplicate, with a collision diagnostic; a dangling reference yields an unresolved target plus a diagnostic. Cross-space references are never flagged unresolved. Attributes are exposed structurally as `ResolvedAttribute`, with arg values as CST `ExpressionAst` nodes and positional/string accessors that subsume the old `getPositionalArgument` / `parseQuotedStringLiteral` helpers. Each resolved entity keeps a CST back-pointer (`syntax`). `parse` and `resolve` stay separate exported entries; the pass is non-throwing on malformed input. Adds two non-semantic diagnostic codes (`PSL_UNRESOLVED_TYPE_REFERENCE`, `PSL_DUPLICATE_DECLARATION`). Bare references resolve current-namespace-then-document; qualified `ns.Type` references resolve against that namespace exactly, matching the live old parser + interpreter resolution policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Add a fixture where a same-named declaration exists in two namespaces and a bare reference resolves to the one in its own namespace, distinguishing current-namespace-first from document-wide first-match. Deleting the current-namespace preference branch in resolve now fails this test. Reword the NameTable JSDoc to state the chosen bare-name policy (current-namespace-first, then document-wide first-match) without the overstated live-interpreter-parity claim, and note that the live SQL interpreter bare path is document-wide last-wins, to be reconciled at interpreter migration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Move the two semantic PSL validators into the resolve pass at message/code
parity with the legacy parser/validator, computed directly over the resolved
CST shape (no PslExtensionBlock adapter shim):
- Extension-block validation (PSL_EXTENSION_* family): descriptor-driven over
GenericBlockDeclaration / KeyValuePair entries, with codec lookup. Covers
unknown parameter, missing-required, option-out-of-set, invalid-value
(unknown codec / non-JSON literal / codec rejection), unresolved-ref, and
first-occurrence-wins duplicate parameter. Populates the per-namespace
extensionBlocks map left empty in the resolved-model dispatch.
- Named-type cross-kind collision (PSL_INVALID_TYPES_MEMBER): scalar / model /
enum conflicts, preserving the legacy scalar->model->enum precedence and
verbatim messages.
resolve now accepts ResolveOptions { pslBlockDescriptors, codecLookup }. The
legacy validateExtensionBlock / parser.ts stay untouched; parse remains purely
syntactic.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
📝 WalkthroughWalkthroughThis PR implements a complete PSL authoring resolve pass that constructs resolved document models from parse trees, validates extension blocks and type references, detects duplicate declarations and collisions, and resolves type annotations into structured targets across namespaces and spaces. ChangesPSL Resolve Pass Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 docstrings
🧪 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. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@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/cli-telemetry
@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: |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/1-framework/2-authoring/psl-parser/test/resolve.test.ts (1)
175-180: ⚡ Quick winMake the cross-space non-unresolved assertion precise.
The current check (
messagenot containing"User") is indirect and brittle. It can fail on unrelated diagnostics and doesn’t directly verify unresolved-type behavior.Suggested tightening
it('does not flag the cross-space reference as unresolved', () => { const doc = resolveSource(source); - for (const diagnostic of doc.diagnostics) { - expect(diagnostic.message).not.toContain('User'); - } + const unresolvedMessages = doc.diagnostics + .filter((d) => d.code === 'PSL_UNRESOLVED_TYPE_REFERENCE') + .map((d) => d.message); + expect(unresolvedMessages).toEqual(['Type "Mystery" does not resolve to a known declaration']); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/2-authoring/psl-parser/test/resolve.test.ts` around lines 175 - 180, The test currently checks doc.diagnostics for messages not containing "User", which is indirect and brittle; replace it with a precise assertion that no diagnostic exists that indicates an unresolved cross-space reference to "User" by using resolveSource(...) and asserting that doc.diagnostics does not contain any entry where diagnostic.code (or diagnostic.message if code is unavailable) matches the unresolved-reference identifier (e.g., 'UNRESOLVED_REFERENCE' or message.includes('unresolved') ) and the diagnostic.message includes "User"; update the it block named 'does not flag the cross-space reference as unresolved' to explicitly check doc.diagnostics.some(...) === false for that specific unresolved diagnostic instead of the broad not-toContain string check.packages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.ts (1)
454-473: ⚡ Quick winAssert the “first occurrence wins” behavior explicitly.
Line 454’s test title claims first-occurrence semantics, but the assertions currently only verify the duplicate-parameter diagnostic. Both
usingvalues are valid strings, so a regression to “last wins” would still pass this test.Suggested tightening
policy_select ReadPosts { target = Post roles = [] using = "first" - using = "second" + using = 42 } `; const diagnostic = diagnosticsFor(source).find( (d) => d.code === 'PSL_EXTENSION_DUPLICATE_PARAMETER', ); expect(diagnostic?.message).toBe( 'Duplicate parameter "using" in "policy_select" block "ReadPosts"; first occurrence wins', ); + expect(diagnosticsFor(source).some((d) => d.code === 'PSL_EXTENSION_INVALID_VALUE')).toBe( + false, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.ts` around lines 454 - 473, The test currently only checks for the PSL_EXTENSION_DUPLICATE_PARAMETER diagnostic but doesn't assert which value is retained; update the test (which already uses diagnosticsFor) to also parse/resolve the source and locate the policy_select block "ReadPosts" and assert that its resolved using parameter is "first" (i.e., first occurrence wins), e.g., retrieve the parsed/validated policy_select node for "ReadPosts" and expect its using value to equal "first".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.ts`:
- Around line 454-473: The test currently only checks for the
PSL_EXTENSION_DUPLICATE_PARAMETER diagnostic but doesn't assert which value is
retained; update the test (which already uses diagnosticsFor) to also
parse/resolve the source and locate the policy_select block "ReadPosts" and
assert that its resolved using parameter is "first" (i.e., first occurrence
wins), e.g., retrieve the parsed/validated policy_select node for "ReadPosts"
and expect its using value to equal "first".
In `@packages/1-framework/2-authoring/psl-parser/test/resolve.test.ts`:
- Around line 175-180: The test currently checks doc.diagnostics for messages
not containing "User", which is indirect and brittle; replace it with a precise
assertion that no diagnostic exists that indicates an unresolved cross-space
reference to "User" by using resolveSource(...) and asserting that
doc.diagnostics does not contain any entry where diagnostic.code (or
diagnostic.message if code is unavailable) matches the unresolved-reference
identifier (e.g., 'UNRESOLVED_REFERENCE' or message.includes('unresolved') ) and
the diagnostic.message includes "User"; update the it block named 'does not flag
the cross-space reference as unresolved' to explicitly check
doc.diagnostics.some(...) === false for that specific unresolved diagnostic
instead of the broad not-toContain string check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6d4ff9a7-672f-489a-b524-2135741fa9c4
📒 Files selected for processing (5)
packages/1-framework/1-core/framework-components/src/shared/psl-extension-block.tspackages/1-framework/2-authoring/psl-parser/src/exports/syntax.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Refs TML-2893. First (stack-root) slice of the PSL parser replacement — introduces the resolved semantic layer the SQL and Mongo authoring paths migrate onto in follow-up slices. Builds on the fault-tolerant CST parser landed in #795.
At a glance
Today the only PSL semantic layer is
PslDocumentAstfrom the legacy regex parser, which half-resolves type references (it leaves a name string each interpreter re-resolves by hand viaallPslModels.find(...)).resolveresolves every reference to a kind-ful coordinate, once.Decision
This PR ships two things in
@prisma-next/psl-parser:A
resolve(document, options?): ResolvedDocumentpass over the CST — a target-agnostic, name-resolved view: per-namespace name-keyedReadonlyMapstores (models/enums/compositeTypes/extensionBlocks) plus document-levelnamedTypes, insertion-ordered (source order preserved), first-declaration-wins on duplicates. Every field / named-type reference resolves to aTypeTarget—scalar/ref(a kind-fulDeclCoord) /crossSpace(coordinates, no kind — bound later against composed contracts) /constructor/unresolved. Attribute arg values are the CSTExpressionAstnodes (no normalized value union);ResolvedAttributeaccessors (positionalArg,stringArg) subsume the oldgetPositionalArgument/parseQuotedStringLiteralhelpers. Every resolved entity keeps asyntaxback-pointer to its CST node for spans.Relocation of the two semantic validators into
resolve, at verbatim message/code parity — extension-block validation (thePSL_EXTENSION_*/PSL_INVALID_EXTENSION_BLOCK_*family, descriptor- and codec-driven) and named-type collision. These were misfiled inside the legacy parser even though they are semantic, not syntactic.parsestays purely syntactic.resolvehas no production consumer yet — the SQL and Mongo interpreters migrate onto it in later slices, and only then is the legacy parser deleted. Nothing is removed here.How it fits together
The target pipeline is
PSL text ─parse─▶ CST ─resolve─▶ ResolvedDocument ─▶ per-target interpreters. This slice builds the middle stage:43e948349). Walk the CST once: bucket declarations into per-namespace name-keyed maps; resolve every type reference to aTypeTarget; follow named-type aliases (types { Email = String }→ anamedTyperef); record the two non-semantic diagnostics this stage owns (unresolved reference, duplicate-declaration collision). Non-throwing on malformed input — diagnostics accumulate,resolvealways returns a document.561f23c89). Bare-name resolution is current-namespace-first, then document-wide first-match; qualifiedns.Typeresolves to that namespace exactly. A regression test pins the current-namespace-first branch as distinct from the document-wide path (a same name declared in two namespaces, referenced from one).a10a46c4b). Populate theextensionBlocksmap; run descriptor-driven extension-block validation and named-type collision insideresolve, surfaced inResolvedDocument.diagnostics.parseperforms neither; the legacyvalidateExtensionBlock/parser.tsare left untouched (their deletion is a later slice). Reuses the new parser's CST directly — no CST→PslExtensionBlockadapter shim.Behavior changes & evidence
resolve+ResolvedDocumentsurface insrc/resolve.ts, exported fromsrc/exports/syntax.tsalongside the existing CST exports.parseandresolveare separate entry points. Whole-shape + duplicate-collision coverage intest/resolve.test.ts.resolveat parity — the fivevalidateExtensionBlockfailure modes (unknown/missing block, option-out-of-set, invalid-value incl. unknown-codec / non-JSON-literal / codec-rejection variants, unresolved-ref withsame namespace/any namespace in the schemascope labels),PSL_EXTENSION_DUPLICATE_PARAMETER, and named-type collision (scalar/model/enum) — verified character-for-character (toBe) against the legacy validator + parser intest/resolve-validation.test.ts.PSL_UNRESOLVED_TYPE_REFERENCE,PSL_DUPLICATE_DECLARATION) in the sharedPslDiagnosticCodeunion (framework-components/src/shared/psl-extension-block.ts) — appended only, no existing code repurposed.Reviewer notes
a10a46c4b(resolve.ts+358, validators + classification moved in). The new parser's CST is purely syntactic, so the param-kind classification (option/value/ref/list) that the old parser did at parse time now happens insideresolveagainst the descriptor — that is whyresolvegained the optionalResolveOptions { pslBlockDescriptors?, codecLookup? }second argument. The signature stays backward-compatible (resolve(document)still works;codecLookupdefaults toemptyCodecLookup, matching the legacy parser's own fallback).parser.tsbuildscompositeTypeNamesbut never reads it in the collision loop, so it emits no composite collision. Matching that exactly is the parity bar; adding a composite collision would invent a diagnostic the old parser never emitted.resolveuses current-namespace-first bare-name resolution (the design-pinned policy); the live SQL interpreter's bare path is flat document-wide last-wins. They differ only when the same bare name is declared in two namespaces and referenced from one. With no consumer yet this is inert; it becomes the decision point under the migration slices' byte-identity check. Tracked in the project plan's open items.info-levelno-bare-castatsrc/tokenizer.ts:69(from feat(psl-parser): fault-tolerant recursive-descent parser (parse + SourceFile) under ./syntax #795, untouched) — lint exits 0.Compatibility / migration / risk
Additive only. No existing export changed or removed;
parsebehavior unchanged; the legacy parser,validateExtensionBlock, and the printer/infer path are untouched. No consumer depends onresolveyet, so the divergence note above carries zero runtime risk in this PR.Testing performed
pnpm --filter @prisma-next/psl-parser typecheck— passpnpm --filter @prisma-next/psl-parser test— pass (15 files, 455 tests; 22 inresolve.test.ts, 25 inresolve-validation.test.ts)pnpm --filter @prisma-next/psl-parser lint— pass (one pre-existinginfo, exit 0)pnpm lint:deps— pass (no cross-layer violation;ResolvedDocumentstays inpsl-parser)Follow-ups
ResolvedDocument(separate slices); the bare-name divergence above is reconciled there under byte-identity.parser.ts/parsePslDocument/./parserexport /validateExtensionBlockdeleted once both interpreters are off them (thedelete-legacy-parserslice).Alternatives considered
parse+resolveinto one call. Rejected — callers that only need syntax (round-trip, formatting) skip resolution; keeping them separate keepsparsepurely syntactic and the stages independently callable.ResolvedValueunion for attribute args. Rejected — consumers already read raw args descriptor-driven, and the CST expression node carries the structured form; a normalized union would be unconsumed and, without the descriptor, often ambiguous.DeclCoordcoordinates. Rejected — coordinates avoid a cyclic object graph (so nodes can be frozen) and mirror the existingentries[kind][name]/ ContractRecord<string, …>keying.PslExtensionBlockadapter so the old validator runs unchanged. Rejected (brief constraint) — relocating the logic over the CST directly avoids a shim that would outlive its purpose.Checklist
TML-NNNN:conventionpnpm lint:depscleanSummary by CodeRabbit
New Features
Tests