TML-2962: extension-aware contract infer omits pack-claimed public tables#901
Closed
wmadden-electric wants to merge 1 commit into
Closed
TML-2962: extension-aware contract infer omits pack-claimed public tables#901wmadden-electric wants to merge 1 commit into
contract infer omits pack-claimed public tables#901wmadden-electric wants to merge 1 commit into
Conversation
…bles contract infer introspects the public schema only, so a table an extension pack owns in public leaked into the app's inferred contract.prisma. Make inferPslContract omit any introspected table a stack pack claims in its public namespace (matched by namespace .id, not record key), and strip surviving tables' foreign keys that reference an omitted table so no dangling relation is emitted. Pack claims in non-public namespaces (auth, storage) never suppress a same-named public app table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Contributor
📝 WalkthroughWalkthroughThis PR adds table filtering to SQL-to-PSL schema inference. ChangesClaimed table filtering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ControlInstance as control-instance.ts
participant Collector as collectClaimedPublicTableNames
participant Converter as sqlSchemaIrToPslAst
ControlInstance->>Collector: extensions
Collector-->>ControlInstance: claimedTableNames set
ControlInstance->>Converter: schemaIR, { claimedTableNames }
Converter->>Converter: filterClaimedTables(schemaIR)
Converter-->>ControlInstance: PslDocumentAst (filtered)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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/config-loader
@prisma-next/emitter
@prisma-next/language-server
@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/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts (1)
205-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a direct unit test for FK-stripping in
filterClaimedTables.Both new tests here cover table omission only; dangling foreign-key removal is exercised solely at the extension-aware integration layer. A focused unit test in this file (a table with a FK to a claimed table, asserting the FK is stripped while the table survives) would make failures in
filterClaimedTableseasier to isolate from family-instance wiring.🤖 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/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts` around lines 205 - 259, Add a focused unit test around `filterClaimedTables` in this spec file: create a schema where a kept table has a foreign key pointing to a claimed table, then assert the kept table still appears in the AST but its foreign key is removed. Use the existing `sqlSchemaIrToPslAst` and `flatPslModels` helpers to locate the behavior, and keep the test separate from the extension-aware integration coverage so FK-stripping failures are isolated.packages/2-sql/9-family/src/core/psl-contract-infer/sql-schema-ir-to-psl-ast.ts (1)
111-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
.map()return-tuple inference likely widens toanyviaObject.fromEntries.The callback in
.map(([tableName, table]) => { ... return [tableName, table]; ... return [tableName, {...}]; })has a block body with bare array-literal returns and no explicit return type, so TypeScript infers the element type as(string | SqlTableIR)[]rather than a[string, SqlTableIR]tuple.Object.fromEntriesfalls back to its(entries: Iterable<readonly any[]>) => anyoverload in that case, sotables(and thereforefilteredSchemaIR) loses type-checking and becomes effectivelyany. It still works at runtime, but silently defeats compile-time safety for this construction.Please verify with
tscwhether this is actually inferred asany, and consider annotating the tuple explicitly.♻️ Proposed fix to preserve tuple typing
const tables = Object.fromEntries( Object.entries(schemaIR.tables) .filter(([tableName]) => !claimedTableNames.has(tableName)) - .map(([tableName, table]) => { + .map(([tableName, table]): [string, SqlSchemaIR['tables'][string]] => { const survivingForeignKeys = table.foreignKeys.filter( (fk) => !claimedTableNames.has(fk.referencedTable), ); if (survivingForeignKeys.length === table.foreignKeys.length) { return [tableName, table]; } return [tableName, { ...table, foreignKeys: survivingForeignKeys }]; }), );🤖 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/2-sql/9-family/src/core/psl-contract-infer/sql-schema-ir-to-psl-ast.ts` around lines 111 - 134, The `filterClaimedTables` construction is losing tuple typing in the `Object.fromEntries` pipeline, so `tables` can degrade to `any` and weaken type safety. Update the `.map()` callback in `filterClaimedTables` to return an explicitly typed `[tableName, table]`-style tuple (or annotate the callback return type) so `Object.fromEntries` selects the typed overload and preserves the `SqlSchemaIR` shape. Verify the inferred type with `tsc` after the change.
🤖 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/2-sql/9-family/src/core/psl-contract-infer/sql-schema-ir-to-psl-ast.ts`:
- Around line 111-134: The `filterClaimedTables` construction is losing tuple
typing in the `Object.fromEntries` pipeline, so `tables` can degrade to `any`
and weaken type safety. Update the `.map()` callback in `filterClaimedTables` to
return an explicitly typed `[tableName, table]`-style tuple (or annotate the
callback return type) so `Object.fromEntries` selects the typed overload and
preserves the `SqlSchemaIR` shape. Verify the inferred type with `tsc` after the
change.
In
`@packages/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts`:
- Around line 205-259: Add a focused unit test around `filterClaimedTables` in
this spec file: create a schema where a kept table has a foreign key pointing to
a claimed table, then assert the kept table still appears in the AST but its
foreign key is removed. Use the existing `sqlSchemaIrToPslAst` and
`flatPslModels` helpers to locate the behavior, and keep the test separate from
the extension-aware integration coverage so FK-stripping failures are isolated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7624bf72-a8b4-4992-99c2-236d854eb27a
⛔ Files ignored due to path filters (1)
projects/extension-supabase/slices/g-extension-aware-infer/spec.mdis excluded by!projects/**
📒 Files selected for processing (4)
packages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/psl-contract-infer/sql-schema-ir-to-psl-ast.tspackages/2-sql/9-family/test/psl-contract-infer/infer-psl-contract.extension-aware.test.tspackages/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts
Contributor
Author
|
Closing: built prematurely on the flat |
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.
What
contract inferintrospects thepublicschema only, so a table an extension pack owns inpublicleaked into the app's inferredcontract.prisma. This makes infer extension-aware: it omits any introspected table a stack extension pack claims in itspublicnamespace.Slice G of the extension-supabase project (TML-2962). Independent of native enums, RLS, and the complete-contract work (Slice F).
How
inferPslContract(SQL family instance) derives the set of table names each stack pack claims in itspublicnamespace — matched by namespace.id, not record key, because the on-disk hydration path (hydrateSqlNamespaceEntry) does not reconcile the two.sqlSchemaIrToPslAstgains an optionalclaimedTableNamesfilter. It drops claimed tables and also strips surviving tables' foreign keys that reference an omitted table, so no dangling relation (a@relationto a non-existent model) is emitted.auth.users/storage.objects(non-public) never suppresses a same-namedpublicapp table.fixtures:checkclean).Tests
claimedTableNamesfilters the named table; no-options path keeps all tables.publicnamespace → omitted; app table kept.publicnamespace → app table kept..ideven when keyed under a different record key.Verification
build · typecheck · family-sql tests (384/384) · fixtures:check (byte-identical) · lint:deps · lint:casts (delta 0) · check:upgrade-coverage — all green locally.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests