diff --git a/packages/2-sql/2-authoring/contract-ts/src/build-contract.ts b/packages/2-sql/2-authoring/contract-ts/src/build-contract.ts index 5447eed1255e..f99bd0effce6 100644 --- a/packages/2-sql/2-authoring/contract-ts/src/build-contract.ts +++ b/packages/2-sql/2-authoring/contract-ts/src/build-contract.ts @@ -75,6 +75,7 @@ import { computeCheckContentHash, derivedCheckPrefixes, } from '@internal/sql-schema-ir/naming'; +import { invariant } from '@internal/utils/assertions'; import { blindCast } from '@internal/utils/casts'; import { ifDefined } from '@internal/utils/defined'; import { InternalError } from '@internal/utils/internal-error'; @@ -1345,6 +1346,10 @@ export function buildSqlContractFromDefinition( relation.toNamespaceId, 'Relation', ); + invariant( + relation.toTable !== undefined, + `Relation "${semanticModel.modelName}.${relation.fieldName}" is local but carries no target table; only cross-space relations may leave it unset.`, + ); assertTargetTableMatches(semanticModel.modelName, targetModel, relation.toTable, 'Relation'); const targetColumnToField = new Map( diff --git a/packages/2-sql/2-authoring/contract-ts/src/contract-definition.ts b/packages/2-sql/2-authoring/contract-ts/src/contract-definition.ts index 3a46c19c8157..a0a76f10c463 100644 --- a/packages/2-sql/2-authoring/contract-ts/src/contract-definition.ts +++ b/packages/2-sql/2-authoring/contract-ts/src/contract-definition.ts @@ -144,7 +144,11 @@ export interface ForeignKeyNode { export interface RelationNode { readonly fieldName: string; readonly toModel: string; - readonly toTable: string; + /** + * Physical table of the related model. Undefined only for a cross-space + * relation whose handle carries no static table name. + */ + readonly toTable: string | undefined; /** * Namespace coordinate of the related model. When omitted the assembler * resolves the coordinate from the referenced model node's own @@ -173,7 +177,7 @@ export interface RelationNode { readonly on: { readonly parentTable: string; readonly parentColumns: readonly string[]; - readonly childTable: string; + readonly childTable: string | undefined; readonly childColumns: readonly string[]; }; readonly through?: { diff --git a/packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts b/packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts index 3d114887c2da..ec7df13e5e33 100644 --- a/packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts +++ b/packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts @@ -453,7 +453,6 @@ function lowerBelongsToRelation( relation.spaceId, `Relation "${currentSpec.modelName}.${relationName}"`, ); - const targetTable = relation.tableName ?? targetModelName.toLowerCase(); const parentColumns = mapFieldNamesToColumnNames( currentSpec.modelName, fromFields, @@ -461,12 +460,10 @@ function lowerBelongsToRelation( ); // For cross-space relations, the `to` field names map directly to column // names because we have no fieldToColumn map for the remote model. - // (The brand carries the table name; field→column resolution on the remote - // side is deferred to the planner which has access to the remote contract.) return { fieldName: relationName, toModel: targetModelName, - toTable: targetTable, + toTable: relation.tableName, cardinality: 'N:1', nullable: belongsToNullable(relationName, relation.optional, currentSpec, fromFields), spaceId: relation.spaceId, @@ -474,7 +471,7 @@ function lowerBelongsToRelation( on: { parentTable: currentSpec.tableName, parentColumns, - childTable: targetTable, + childTable: relation.tableName, childColumns: toFields, }, }; @@ -712,11 +709,24 @@ function lowerCrossSpaceForeignKeyNode( readonly index?: boolean | undefined; }, ): ForeignKeyNode { + if (foreignKey.targetTableName === undefined) { + throw contractError( + 'CONTRACT.FOREIGN_KEY_INVALID', + `Foreign key on "${spec.modelName}" references model "${foreignKey.targetModel}" in contract space "${foreignKey.targetSpaceId}" but the target table name is unknown: the handle's .sql() stage is a factory function, so its table cannot be read statically. Declare the target model's .sql() stage with a static object carrying \`table\`.`, + { + meta: { + sourceModel: spec.modelName, + targetModel: foreignKey.targetModel, + spaceId: foreignKey.targetSpaceId, + }, + }, + ); + } return { columns: mapFieldNamesToColumnNames(spec.modelName, foreignKey.fields, spec.fieldToColumn), references: { model: foreignKey.targetModel, - table: foreignKey.targetTableName ?? foreignKey.targetModel.toLowerCase(), + table: foreignKey.targetTableName, columns: foreignKey.targetFields, ...(foreignKey.targetNamespaceId !== undefined ? { namespaceId: foreignKey.targetNamespaceId } @@ -917,7 +927,15 @@ function resolveModelNode( }; } -function collectRuntimeModelSpecs(definition: ContractInput): RuntimeCollection { +/** + * `ContractInput`'s `Extensions` parameter defaults to `undefined`, but lowering + * reads the extension-pack record at runtime, so the input is widened here. + */ +type LoweringInput = Omit & { + readonly extensions?: Record> | undefined; +}; + +function collectRuntimeModelSpecs(definition: LoweringInput): RuntimeCollection { const storageTypes = { ...(definition.types ?? {}) } as Record; const models = { ...(definition.models ?? {}) } as Record; @@ -1033,7 +1051,7 @@ function lowerModels( * No entity kind is named anywhere in this walk. */ function lowerPackEntityHandles( - definition: ContractInput, + definition: LoweringInput, modelSpecs: ReadonlyMap, ): AttachedEntities | undefined { const entities = definition.entities; @@ -1158,7 +1176,7 @@ function lowerPackEntityHandles( return pack; } -export function buildContractDefinition(definition: ContractInput): ContractDefinition { +export function buildContractDefinition(definition: LoweringInput): ContractDefinition { const collection = collectRuntimeModelSpecs(definition); const models = lowerModels(collection, definition.extensions); const attachedEntities = lowerPackEntityHandles(definition, collection.modelSpecs); diff --git a/packages/2-sql/2-authoring/contract-ts/test/cross-space-fk.test.ts b/packages/2-sql/2-authoring/contract-ts/test/cross-space-fk.test.ts index f0dbd03d39e0..86bcf4d7e761 100644 --- a/packages/2-sql/2-authoring/contract-ts/test/cross-space-fk.test.ts +++ b/packages/2-sql/2-authoring/contract-ts/test/cross-space-fk.test.ts @@ -216,6 +216,61 @@ describe('cross-space FK via constraints.foreignKey in sql()', () => { // Missing-pack fail-fast (AC5 TS half) // --------------------------------------------------------------------------- +/** + * Synthetic supabase OrderItem handle whose `.sql()` stage is a factory + * function, so the handle carries no statically readable table name. + */ +function buildSyntheticSupabaseOrderItem() { + return new ContractModelBuilder( + { + modelName: 'OrderItem' as const, + namespace: 'auth', + fields: { + id: field.column(int4Column).id(), + sku: field.column(textColumn), + }, + relations: {}, + }, + undefined, + undefined, + 'supabase' as const, + ).sql(() => ({ table: 'order_items' })); +} + +describe('cross-space FK to a handle with no statically readable table name', () => { + const defineLineNoteContract = () => { + const ExtOrderItem = buildSyntheticSupabaseOrderItem(); + + const LineNote = model('LineNote', { + fields: { + id: field.column(int4Column).id(), + orderItemId: field.column(int4Column), + }, + }).sql(({ cols, constraints }) => ({ + table: 'line_note', + foreignKeys: [constraints.foreignKey(cols.orderItemId, ExtOrderItem.refs.id)], + })); + + return defineContract({ + family: bareFamilyPack, + target: postgresTargetPack, + createNamespace: createTestSqlNamespace, + extensions: { supabase: supabasePack }, + models: { LineNote }, + }); + }; + + it('throws a structured error instead of guessing the target table', () => { + expect(defineLineNoteContract).toThrow( + expect.objectContaining({ + code: 'CONTRACT.FOREIGN_KEY_INVALID', + message: + 'Foreign key on "LineNote" references model "OrderItem" in contract space "supabase" but the target table name is unknown: the handle\'s .sql() stage is a factory function, so its table cannot be read statically. Declare the target model\'s .sql() stage with a static object carrying `table`.', + }), + ); + }); +}); + describe('missing-pack fail-fast diagnostic', () => { it('throws when the referenced spaceId is not in extensions', () => { const ExtUser = buildSyntheticSupabaseAuthUser(); diff --git a/packages/2-sql/2-authoring/contract-ts/test/cross-space-relation.test.ts b/packages/2-sql/2-authoring/contract-ts/test/cross-space-relation.test.ts index eb8165dfa605..bf5125bdca07 100644 --- a/packages/2-sql/2-authoring/contract-ts/test/cross-space-relation.test.ts +++ b/packages/2-sql/2-authoring/contract-ts/test/cross-space-relation.test.ts @@ -18,6 +18,7 @@ import { describe, expect, it } from 'vitest'; import { createTestSqlNamespace } from '../../../1-core/contract/test/test-support'; import { defineContract, field, model, rel } from '../src/contract-builder'; import { ContractModelBuilder } from '../src/contract-dsl'; +import { buildContractDefinition } from '../src/contract-lowering'; import { modelsOf } from './contract-test-helpers'; import { columnDescriptor } from './helpers/column-descriptor'; @@ -182,6 +183,57 @@ describe('cross-space belongsTo relation lowering', () => { }); }); +/** + * Synthetic supabase OrderItem handle whose `.sql()` stage is a factory + * function, so the handle carries no statically readable table name. + */ +function buildSyntheticSupabaseOrderItem() { + return new ContractModelBuilder( + { + modelName: 'OrderItem' as const, + namespace: 'auth', + fields: { + id: field.column(int4Column).id(), + sku: field.column(textColumn), + }, + relations: {}, + }, + undefined, + undefined, + 'supabase' as const, + ).sql(() => ({ table: 'order_items' })); +} + +describe('cross-space belongsTo relation with no statically readable target table', () => { + it('leaves the target table unset instead of fabricating one from the model name', () => { + const ExtOrderItem = buildSyntheticSupabaseOrderItem(); + + const LineNote = model('LineNote', { + fields: { + id: field.column(int4Column).id(), + orderItemId: field.column(int4Column), + }, + }).relations({ + orderItem: rel.belongsTo(ExtOrderItem, { from: 'orderItemId', to: 'id' }), + }); + + const definition = buildContractDefinition({ + family: bareFamilyPack, + target: postgresTargetPack, + createNamespace: createTestSqlNamespace, + extensions: { supabase: supabasePack }, + models: { LineNote }, + }); + + const relation = definition.models.find((m) => m.modelName === 'LineNote')?.relations?.[0]; + expect(relation).toMatchObject({ toModel: 'OrderItem', spaceId: 'supabase' }); + expect({ toTable: relation?.toTable, childTable: relation?.on.childTable }).toEqual({ + toTable: undefined, + childTable: undefined, + }); + }); +}); + // --------------------------------------------------------------------------- // Cross-space relation — missing-pack fail-fast (AC5 TS half) // --------------------------------------------------------------------------- diff --git a/projects/psl-verbatim-table-names/slices/ts-dsl-relation-fallback/spec.md b/projects/psl-verbatim-table-names/slices/ts-dsl-relation-fallback/spec.md new file mode 100644 index 000000000000..68cf4939d370 --- /dev/null +++ b/projects/psl-verbatim-table-names/slices/ts-dsl-relation-fallback/spec.md @@ -0,0 +1,38 @@ +# Slice spec — TS DSL cross-space relation table fallback + +**Project:** `projects/psl-verbatim-table-names/` · **Slice 3** · **Branch:** `psl-verbatim-ts-dsl-relation-fallback` (from `main`) + +## At a glance + +In the TypeScript authoring DSL, a `belongsTo` relation whose target model lives in another contract space resolves the target table as `relation.tableName ?? targetModelName.toLowerCase()` in `packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts` (around line 456). `OrderItem` becomes `orderitem`, which is neither the DSL's identity naming default nor any naming strategy the DSL offers. It is the same class of defect as the PSL default this project removed: an implicit case transform. + +## Chosen design + +Never guess a table name. Two paths carried the lowercase guess: + +- The relation node. For a cross-space relation with no `tableName`, `toTable` and `on.childTable` are left undefined rather than fabricated. Their only reader in `build-contract.ts` already skips cross-space relations, so no consumer changes behaviour. +- The foreign-key node (`lowerCrossSpaceForeignKeyNode`). Its target table is written into `contract.json` and into the `REFERENCES` clause of the DDL, and nothing resolves it against the remote contract later. A guessed name there is a foreign key to a table that may not exist. So when a cross-space foreign key targets a handle with no statically readable table, lowering throws `CONTRACT.FOREIGN_KEY_INVALID`, naming the source model, the target model and the space, and telling the author to declare the target model's `.sql()` stage with a static object carrying `table`. That is the only way a cross-space handle can carry its table today; there is no per-relation table option. + +Amended after review: the original text claimed the foreign-key path already left the table unset and the planner resolved it. The reviewer traced the value into the emitted contract and the DDL; neither claim held. + +## Why `tableName` can be undefined + +`ContractModelBuilder.tableName` is only populated when `.sql()` receives a static object with `table`. A model whose `.sql()` stage is a factory function (`sql(({ cols }) => ({ table: ..., ... }))`) has no statically readable table, so a cross-space handle to it carries no `tableName`. No existing test exercises that shape; every cross-space fixture uses a static `.sql({ table: 'users' })`. + +## Scope + +**In:** the lowering change; the relation-node type change if the primary design is taken; a test in `packages/2-sql/2-authoring/contract-ts/test/cross-space-relation.test.ts` (or `cross-space-fk.test.ts`) with a branded cross-space handle whose `.sql()` is a factory function and whose model is a two-word PascalCase name such as `OrderItem`, asserting the relation node carries no fabricated table; and a test in `cross-space-fk.test.ts` asserting the structured error for a cross-space foreign key to such a handle. Both must fail on `main`. + +**Out:** same-space relations; the naming-strategy machinery (`applyNaming`); any change to how the planner resolves remote tables. + +## Done conditions + +- The new test is red on `main` and green on the branch. +- No `.toLowerCase()` on a model name remains in `packages/2-sql/2-authoring/contract-ts/src`. +- `@internal/sql-contract-ts` build, typecheck, test, and lint pass; `pnpm lint:deps` passes; `pnpm test:packages` passes once on the final tree. +- Upgrade coverage: run `pnpm check:upgrade-coverage --mode pr --prev origin/main --head HEAD`; no example or extension should change, so no fragment is expected. If the check demands one, report rather than write a `changes: []` fragment without checking it is truthful. + +## Dispatch plan + +1. Red test only, committed alone. +2. The change, gates, commit. Push with `git push -u bot psl-verbatim-ts-dsl-relation-fallback`; do not open the PR (the orchestrator writes the PR text).