Open
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 ignored due to path filters (1)
📒 Files selected for processing (70)
📝 WalkthroughWalkthroughThis PR implements an index-type registry, renames index metadata from using/config to type/options, threads pack-provided index-types through authoring and PSL, validates types/options at contract build, and updates Postgres DDL/introspection, emitter, ParadeDB extension, and tests. ChangesIndex-Type Registry Foundation
Index IR Shape Transformation
Validation and Semantics
TypeScript Contract Authoring
PSL Contract Authoring
ParadeDB Extension
Schema Verification and Migrations
Code Emission
E2E and Integration Tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/2-sql/2-authoring/contract-ts/test/contract-builder.normalization.test.ts (1)
220-229:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlso assert plain indexes omit
type/options.After moving to
type/options, this test still only checks legacyusing/config. At Line [220], add assertions that plain indexes do not carrytypeoroptions, so default metadata leaks are caught.Suggested patch
const idx = contract.storage.tables.user.indexes[0]!; expect(idx.columns).toEqual(['email']); expect(idx).not.toHaveProperty('using'); expect(idx).not.toHaveProperty('config'); + expect(idx).not.toHaveProperty('type'); + expect(idx).not.toHaveProperty('options');🤖 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/2-authoring/contract-ts/test/contract-builder.normalization.test.ts` around lines 220 - 229, The test should also assert that plain indexes do not include the new metadata keys; after retrieving idx from contract.storage.tables.user.indexes[0] (the existing idx variable), add two assertions that idx does not have properties 'type' and 'options' (e.g., expect(idx).not.toHaveProperty('type') and expect(idx).not.toHaveProperty('options')) so plain indexes omit type/options alongside the existing using/config checks.
🧹 Nitpick comments (6)
packages/2-sql/2-authoring/contract-ts/src/build-contract.ts (1)
81-90: ⚡ Quick winConsider replacing the structural cast with a type predicate.
(pack as { readonly indexTypes?: IndexTypeRegistration<IndexTypeMap> })is a widening cast to probe a property not in the formal pack type. A narrow type predicate avoids the cast and lets the compiler narrow cleanly:♻️ Suggested refactor
+function hasIndexTypes( + pack: object, +): pack is { readonly indexTypes: IndexTypeRegistration<IndexTypeMap> } { + return 'indexTypes' in pack; +} const indexTypeRegistry = createIndexTypeRegistry(); for (const pack of [definition.target, ...Object.values(definition.extensionPacks ?? {})]) { - const registration = (pack as { readonly indexTypes?: IndexTypeRegistration<IndexTypeMap> }) - .indexTypes; - if (!registration) continue; - for (const entry of registration.entries) { + if (!hasIndexTypes(pack)) continue; + for (const entry of pack.indexTypes.entries) { indexTypeRegistry.register(entry); } }🤖 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/2-authoring/contract-ts/src/build-contract.ts` around lines 81 - 90, The code uses a structural cast on "pack" to access "indexTypes"; replace it with a type predicate function (e.g., hasIndexTypes) that asserts "pack is { readonly indexTypes?: IndexTypeRegistration<IndexTypeMap> }" so the compiler can narrow the type without a widening cast; update the loop over "definition.target" and "definition.extensionPacks" to call hasIndexTypes(pack) before reading "pack.indexTypes", then assign to "registration" and call "indexTypeRegistry.register(entry)" as before and keep the subsequent call to "validateIndexTypes(contract, indexTypeRegistry)" unchanged.packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts (1)
509-515: ⚡ Quick winUse
ifDefined()instead of inline conditional spreads fortype/options.Line 513 and Line 514 currently use inline conditional spread patterns, which diverges from the project convention.
♻️ Suggested patch
const indexes: readonly SqlIndexIR[] = Array.from(indexesMap.values()).map((idx) => ({ columns: Object.freeze([...idx.columns]) as readonly string[], name: idx.name, unique: idx.unique, - ...(idx.type !== undefined && { type: idx.type }), - ...(idx.options !== undefined && { options: idx.options }), + ...ifDefined('type', idx.type), + ...ifDefined('options', idx.options), }));As per coding guidelines: "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 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/3-targets/6-adapters/postgres/src/core/control-adapter.ts` around lines 509 - 515, The inline conditional spreads for type/options inside the mapping that builds the readonly SqlIndexIR array (the indexes variable created from indexesMap.values()) should be replaced with the project-convention helper ifDefined from `@prisma-next/utils/defined`; update the mapping that produces each SqlIndexIR (referencing SqlIndexIR, indexesMap, and the indexes constant) to import and call ifDefined for idx.type and idx.options so the conditional properties are spread using ifDefined instead of the current ...(idx.type !== undefined && ...) and ...(idx.options !== undefined && ...) patterns.packages/2-sql/1-core/contract/test/validate.test.ts (1)
899-985: ⚡ Quick win
describe('validateIndexTypes', ...)is nested insidedescribe('validateContract', ...)— wrong grouping
validateIndexTypesis a separate validator. Its test suite should be a top-leveldescribeblock (sibling todescribe('validateContract', ...)), not a child of it. As-is, test runners report these asvalidateContract > validateIndexTypes > ..., which is misleading.🔧 Proposed fix
- describe('validateIndexTypes', () => { +}); + +describe('validateIndexTypes', () => { function makeContractWithIndex(index: Record<string, unknown>) { ... } ... - }); -}); +});🤖 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/1-core/contract/test/validate.test.ts` around lines 899 - 985, The tests for validateIndexTypes are incorrectly nested inside describe('validateContract', ...) — move the entire describe('validateIndexTypes', ...) block (the function makeContractWithIndex and all its it(...) cases) out so it is a top-level describe sibling to validateContract; ensure validateIndexTypes, createIndexTypeRegistry, makeContract (and any helpers used inside makeContractWithIndex) remain in scope or are imported/declared above the new top-level describe; verify braces/exports so the test file still parses and run tests to confirm the suite now reports "validateIndexTypes > ..." as a top-level group.packages/2-sql/1-core/contract/src/index-types.ts (1)
39-43: 💤 Low valueMinor: needless widening cast.
entry.options as Type<unknown>widens fromType<TOpts>toType<unknown>. Since the entries array is typed asReadonlyArray<IndexTypeEntry>(whose defaultTOptionsisunknown), assignment is structurally fine without the cast —Type<TOpts>flows intoType<unknown>via the variance arktype already provides on its inferred output. If TS still complains, prefer constructing the entry without an explicit annotation rather than masking with anas-cast.🤖 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/1-core/contract/src/index-types.ts` around lines 39 - 43, The cast entry.options as Type<unknown> is unnecessary and widens types; remove the explicit cast and push the entry as { type: typeLiteral, options: entry.options } so the structural assignment into the ReadonlyArray<IndexTypeEntry> works via Type variance; update the return in IndexTypeBuilderImpl (the constructor call creating new IndexTypeBuilderImpl<TMap & Record<TLit, { readonly options: TOpts }>>([...this.entries, { type: typeLiteral, options: entry.options }])) to eliminate the cast and let Type<TOpts> flow naturally into the entries array.packages/2-sql/2-authoring/contract-ts/src/contract-dsl.ts (1)
712-721: 💤 Low valuePrefer
ifDefinedfor conditional object spreads.
ifDefinedis already imported at the top of the file (line 19). Per repo conventions, conditional object spreads should use it instead of the inline... !== undefined ? { x } : {}pattern. The rest of the file pre-dates this guideline, but the new index impl is fresh code worth aligning.♻️ Suggested refactor
return { kind: 'index', fields: normalizeFieldRefInput(fields), - ...(options?.name !== undefined ? { name: options.name } : {}), - ...(options?.type !== undefined ? { type: options.type } : {}), - ...(options?.options !== undefined - ? { options: options.options as Record<string, unknown> } - : {}), + ...ifDefined('name', options?.name), + ...ifDefined('type', options?.type), + ...ifDefined('options', options?.options as Record<string, unknown> | undefined), };As per coding guidelines: "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 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/2-authoring/contract-ts/src/contract-dsl.ts` around lines 712 - 721, The returned index object uses inline conditional spreads for options.name, options.type and options.options; replace these with ifDefined(...) calls from `@prisma-next/utils/defined` (already imported) so the return becomes a single object with fields: kind: 'index', fields: normalizeFieldRefInput(fields), and conditional properties inserted via ifDefined({ name: options.name }), ifDefined({ type: options.type }), and ifDefined({ options: options.options as Record<string, unknown> }) (keep the cast and the normalizeFieldRefInput call intact) so the code follows the repo convention for conditional spreads.packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts (1)
209-260: 💤 Low valueNaive backslash-escape handling in quote tracking.
In
splitObjectLiteralEntriesandfindTopLevelColon, the closing-quote checkbody[index - 1] !== '\\'does not handle escaped backslashes: an even number of trailing backslashes means the quote is unescaped. For example,{ key: "a\\", key2: "b" }would be parsed as one continuous quote because the"after\\is treated as escaped.For V1 (string-leaf only with simple values like
"id"), this is unlikely to trigger in practice, but worth tightening to count consecutive preceding backslashes.🤖 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/2-authoring/contract-psl/src/psl-attribute-parsing.ts` around lines 209 - 260, The quote-closing logic in splitObjectLiteralEntries and findTopLevelColon incorrectly treats a quote as escaped if the immediately preceding char is a backslash; instead, count consecutive backslashes immediately before the quote and treat the quote as escaped only if the count is odd. Update the closing-quote checks in splitObjectLiteralEntries (the block that currently does if (ch === quote && body[index - 1] !== '\\')) and in findTopLevelColon (if (ch === quote && entry[index - 1] !== '\\')) to compute the number of consecutive backslashes before index and only clear quote when that count is even (i.e., the quote is not escaped).
🤖 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.
Inline comments:
In `@docs/architecture` docs/adrs/ADR 210 - Index-type registry.md:
- Line 7: The ADR currently links a transient project file via the "Spec:" line
pointing to projects/index-type-registry/spec.md; update ADR 210 - Index-type
registry to avoid referencing ephemeral project artifacts by either removing the
"Spec:" link entirely, replacing it with a stable/versioned document URL (or a
tag/commit-based link) or by inlining the essential spec content into the ADR
body so the ADR stands alone (locate the "Spec:" line in the ADR and modify it
to one of these three options).
In `@packages/2-sql/2-authoring/contract-ts/src/composed-authoring-helpers.ts`:
- Around line 101-108: The MergeAllPackIndexTypes helper currently injects a
__family property causing asymmetry with the contract types; update
MergeAllPackIndexTypes so it no longer adds { readonly __family: Family } and
instead only carries { readonly __target: Target } plus ExtensionPacks before
passing into MergeExtensionIndexTypes, so its shape matches
AllPacks/IndexTypesFromDefinition (remove __family from the intersection and
keep __target and ExtensionPacks intact).
In `@packages/3-extensions/paradedb/README.md`:
- Around line 53-60: The README example calls constraints.index with a single
ColumnRef argument which no longer typechecks because constraints.index now
requires an array/tuple parameter (see the new overload in contract-dsl.ts that
expects { readonly [K in keyof FieldNames]: ColumnRef<...> }). Update the
example and any single-column usages to pass a one-element array/tuple (e.g.,
replace constraints.index(cols.body, {...}) with constraints.index([cols.body],
{...})) and audit other uses of constraints.index to ensure they supply arrays
per the new ColumnRef tuple type.
In `@packages/3-targets/3-targets/postgres/test/migrations/issue-planner.test.ts`:
- Around line 530-589: The tests that call planIssues (in the two index-mismatch
cases) only assert result.value.calls[0], which allows extra unexpected planner
operations; update both tests to also assert the planner produced exactly one
call by adding expect(result.value.calls).toHaveLength(1) after the result.ok
checks so any extra calls are caught (look for the planIssues invocations in the
two index-mismatch tests using makeContract and add the toHaveLength(1)
assertion before matching calls[0]).
---
Outside diff comments:
In
`@packages/2-sql/2-authoring/contract-ts/test/contract-builder.normalization.test.ts`:
- Around line 220-229: The test should also assert that plain indexes do not
include the new metadata keys; after retrieving idx from
contract.storage.tables.user.indexes[0] (the existing idx variable), add two
assertions that idx does not have properties 'type' and 'options' (e.g.,
expect(idx).not.toHaveProperty('type') and
expect(idx).not.toHaveProperty('options')) so plain indexes omit type/options
alongside the existing using/config checks.
---
Nitpick comments:
In `@packages/2-sql/1-core/contract/src/index-types.ts`:
- Around line 39-43: The cast entry.options as Type<unknown> is unnecessary and
widens types; remove the explicit cast and push the entry as { type:
typeLiteral, options: entry.options } so the structural assignment into the
ReadonlyArray<IndexTypeEntry> works via Type variance; update the return in
IndexTypeBuilderImpl (the constructor call creating new
IndexTypeBuilderImpl<TMap & Record<TLit, { readonly options: TOpts
}>>([...this.entries, { type: typeLiteral, options: entry.options }])) to
eliminate the cast and let Type<TOpts> flow naturally into the entries array.
In `@packages/2-sql/1-core/contract/test/validate.test.ts`:
- Around line 899-985: The tests for validateIndexTypes are incorrectly nested
inside describe('validateContract', ...) — move the entire
describe('validateIndexTypes', ...) block (the function makeContractWithIndex
and all its it(...) cases) out so it is a top-level describe sibling to
validateContract; ensure validateIndexTypes, createIndexTypeRegistry,
makeContract (and any helpers used inside makeContractWithIndex) remain in scope
or are imported/declared above the new top-level describe; verify braces/exports
so the test file still parses and run tests to confirm the suite now reports
"validateIndexTypes > ..." as a top-level group.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts`:
- Around line 209-260: The quote-closing logic in splitObjectLiteralEntries and
findTopLevelColon incorrectly treats a quote as escaped if the immediately
preceding char is a backslash; instead, count consecutive backslashes
immediately before the quote and treat the quote as escaped only if the count is
odd. Update the closing-quote checks in splitObjectLiteralEntries (the block
that currently does if (ch === quote && body[index - 1] !== '\\')) and in
findTopLevelColon (if (ch === quote && entry[index - 1] !== '\\')) to compute
the number of consecutive backslashes before index and only clear quote when
that count is even (i.e., the quote is not escaped).
In `@packages/2-sql/2-authoring/contract-ts/src/build-contract.ts`:
- Around line 81-90: The code uses a structural cast on "pack" to access
"indexTypes"; replace it with a type predicate function (e.g., hasIndexTypes)
that asserts "pack is { readonly indexTypes?:
IndexTypeRegistration<IndexTypeMap> }" so the compiler can narrow the type
without a widening cast; update the loop over "definition.target" and
"definition.extensionPacks" to call hasIndexTypes(pack) before reading
"pack.indexTypes", then assign to "registration" and call
"indexTypeRegistry.register(entry)" as before and keep the subsequent call to
"validateIndexTypes(contract, indexTypeRegistry)" unchanged.
In `@packages/2-sql/2-authoring/contract-ts/src/contract-dsl.ts`:
- Around line 712-721: The returned index object uses inline conditional spreads
for options.name, options.type and options.options; replace these with
ifDefined(...) calls from `@prisma-next/utils/defined` (already imported) so the
return becomes a single object with fields: kind: 'index', fields:
normalizeFieldRefInput(fields), and conditional properties inserted via
ifDefined({ name: options.name }), ifDefined({ type: options.type }), and
ifDefined({ options: options.options as Record<string, unknown> }) (keep the
cast and the normalizeFieldRefInput call intact) so the code follows the repo
convention for conditional spreads.
In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts`:
- Around line 509-515: The inline conditional spreads for type/options inside
the mapping that builds the readonly SqlIndexIR array (the indexes variable
created from indexesMap.values()) should be replaced with the project-convention
helper ifDefined from `@prisma-next/utils/defined`; update the mapping that
produces each SqlIndexIR (referencing SqlIndexIR, indexesMap, and the indexes
constant) to import and call ifDefined for idx.type and idx.options so the
conditional properties are spread using ifDefined instead of the current
...(idx.type !== undefined && ...) and ...(idx.options !== undefined && ...)
patterns.
🪄 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: 5907af77-0930-4006-84b5-1ef4d43910ae
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (66)
docs/architecture docs/adrs/ADR 210 - Index-type registry.mdpackages/1-framework/2-authoring/contract/src/descriptors.tspackages/1-framework/2-authoring/contract/test/descriptors.test.tspackages/2-sql/1-core/contract/package.jsonpackages/2-sql/1-core/contract/src/exports/index-types.tspackages/2-sql/1-core/contract/src/index-types.tspackages/2-sql/1-core/contract/src/index.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/1-core/contract/src/validators.tspackages/2-sql/1-core/contract/test/index-types.test.tspackages/2-sql/1-core/contract/test/validate.test.tspackages/2-sql/1-core/contract/tsdown.config.tspackages/2-sql/1-core/schema-ir/src/types.tspackages/2-sql/2-authoring/contract-psl/package.jsonpackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/src/build-contract.tspackages/2-sql/2-authoring/contract-ts/src/composed-authoring-helpers.tspackages/2-sql/2-authoring/contract-ts/src/contract-definition.tspackages/2-sql/2-authoring/contract-ts/src/contract-dsl.tspackages/2-sql/2-authoring/contract-ts/src/contract-lowering.tspackages/2-sql/2-authoring/contract-ts/src/contract-types.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.constraints.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.dsl.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.index-types.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.normalization.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-dsl.runtime.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-lowering.runtime.test.tspackages/2-sql/2-authoring/contract-ts/test/helpers/test-index-pack.tspackages/2-sql/3-tooling/emitter/src/index.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.generation.basic.test.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.structure.test.tspackages/2-sql/9-family/src/core/schema-verify/verify-helpers.tspackages/2-sql/9-family/test/schema-verify.helpers.tspackages/2-sql/9-family/test/schema-verify.semantic-satisfaction.test.tspackages/3-extensions/paradedb/README.mdpackages/3-extensions/paradedb/package.jsonpackages/3-extensions/paradedb/src/core/constants.tspackages/3-extensions/paradedb/src/core/descriptor-meta.tspackages/3-extensions/paradedb/src/exports/index-types.tspackages/3-extensions/paradedb/src/types/index-types.tspackages/3-extensions/paradedb/test/index-types.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/indexes.tspackages/3-targets/3-targets/postgres/test/migrations/index-ddl.test.tspackages/3-targets/3-targets/postgres/test/migrations/issue-planner.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/migrations/index-introspection.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/op-factory-call.rendering.test.tstest/e2e/framework/test/sqlite/migrations/additive.test.tstest/e2e/framework/test/sqlite/migrations/destructive.test.tstest/e2e/framework/test/sqlite/migrations/fk-preservation.test.tstest/integration/package.jsontest/integration/test/authoring/paradedb-bm25-narrowing.test.tstest/integration/test/authoring/parity/core-surface/contract.tstest/integration/test/authoring/parity/map-attributes/contract.tstest/integration/test/authoring/psl-index-type-options.integration.test.tstest/integration/test/family.schema-verify.basic.integration.test.tstest/integration/test/family.schema-verify.basic.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-scenarios/contract-add-project-slug.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-scenarios/contract.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/contract.parity.tstest/integration/test/referential-actions.integration.test.ts
💤 Files with no reviewable changes (1)
- packages/3-extensions/paradedb/src/core/constants.ts
Comment on lines
+530
to
+589
| const result = planIssues({ | ||
| ...defaultCtx, | ||
| issues, | ||
| toContract, | ||
| fromContract: null, | ||
| storageTypes: toContract.storage.types ?? {}, | ||
| }); | ||
|
|
||
| expect(result.ok).toBe(true); | ||
| if (!result.ok) throw new Error('expected ok'); | ||
| expect(result.value.calls[0]).toMatchObject({ | ||
| factoryName: 'createIndex', | ||
| tableName: 'doc', | ||
| indexName: 'doc_body_bm25_idx', | ||
| indexType: 'bm25', | ||
| options: { key_field: 'id' }, | ||
| }); | ||
| }); | ||
|
|
||
| it('falls back to a default index name when the contract index has no name', () => { | ||
| const toContract = makeContract({ | ||
| tables: { | ||
| doc: { | ||
| columns: { | ||
| id: { nativeType: 'uuid', codecId: 'pg/uuid@1', nullable: false }, | ||
| body: { nativeType: 'text', codecId: 'pg/text@1', nullable: false }, | ||
| }, | ||
| primaryKey: { columns: ['id'] }, | ||
| uniques: [], | ||
| indexes: [{ columns: ['body'] }], | ||
| foreignKeys: [], | ||
| }, | ||
| }, | ||
| }); | ||
| const issues: SchemaIssue[] = [ | ||
| { | ||
| kind: 'index_mismatch', | ||
| table: 'doc', | ||
| expected: 'body', | ||
| message: 'Table "doc" is missing index: body', | ||
| }, | ||
| ]; | ||
|
|
||
| const result = planIssues({ | ||
| ...defaultCtx, | ||
| issues, | ||
| toContract, | ||
| fromContract: null, | ||
| storageTypes: toContract.storage.types ?? {}, | ||
| }); | ||
|
|
||
| expect(result.ok).toBe(true); | ||
| if (!result.ok) throw new Error('expected ok'); | ||
| expect(result.value.calls[0]).toMatchObject({ | ||
| factoryName: 'createIndex', | ||
| tableName: 'doc', | ||
| indexName: 'doc_body_idx', | ||
| indexType: undefined, | ||
| options: undefined, | ||
| }); |
There was a problem hiding this comment.
Assert single-call planning in the last two index-mismatch cases.
At Line [540] and Line [583], the tests only validate calls[0]. Add toHaveLength(1) so extra unexpected planner operations are caught.
Suggested patch
expect(result.ok).toBe(true);
if (!result.ok) throw new Error('expected ok');
+ expect(result.value.calls).toHaveLength(1);
expect(result.value.calls[0]).toMatchObject({
factoryName: 'createIndex',
tableName: 'doc',
indexName: 'doc_body_bm25_idx',
indexType: 'bm25',
options: { key_field: 'id' },
});
@@
expect(result.ok).toBe(true);
if (!result.ok) throw new Error('expected ok');
+ expect(result.value.calls).toHaveLength(1);
expect(result.value.calls[0]).toMatchObject({
factoryName: 'createIndex',
tableName: 'doc',
indexName: 'doc_body_idx',
indexType: undefined,
options: undefined,
});🤖 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/3-targets/3-targets/postgres/test/migrations/issue-planner.test.ts`
around lines 530 - 589, The tests that call planIssues (in the two
index-mismatch cases) only assert result.value.calls[0], which allows extra
unexpected planner operations; update both tests to also assert the planner
produced exactly one call by adding expect(result.value.calls).toHaveLength(1)
after the result.ok checks so any extra calls are caught (look for the
planIssues invocations in the two index-mismatch tests using makeContract and
add the toHaveLength(1) assertion before matching calls[0]).
Establishes the index-type registry primitive: factory builder declaring the type literal once, phantom-typed __indexTypes pack threading, validation seam in validateSqlStorage, single Postgres-style renderer, and the lockstep using/config → type/options field-name rename.
Adds defineIndexTypes() factory builder and createIndexTypeRegistry() in @prisma-next/sql-contract/index-types. The builder declares each type literal exactly once and exposes both a phantom IndexTypes map (for type-level narrowing) and a runtime entry list. The registry is instance-based — created per contract assembly, not module-global — so two contracts with different pack lists see different valid type sets. No consumers wired yet. Also clarifies ADR 210: the per-contract registry is assembled from each pack's entries during contract definition, not via global register() calls at module load.
Lockstep rename across the IR, the SQL family validator, the schema-IR, the TS authoring surface (IndexOptions, IndexConstraint, IndexNode, contract-lowering, build-contract), and the contract emitter. The schema-IR SqlIndexIR gains the new fields. The ParadeDB bm25Index() helper writes the new field names but is otherwise untouched (full removal happens when the registry is wired in M3). The placeholder fields are inert in every active code path today, so this rename is observationally silent. No backwards-compatibility shim.
…ntract definition ExtractIndexTypesFromPack pulls the phantom __indexTypes off any pack that carries one. AllIndexTypeLiterals walks all literal keys across a record of packs, and MergeExtensionIndexTypes builds the merged map by looking each literal up in its contributing pack. IndexTypesFromDefinition folds target + extension packs into a single record and feeds it through MergeExtensionIndexTypes. SqlContractResult exposes the merged map as a phantom __indexTypes? field. The implementation deliberately avoids UnionToIntersection so the empty case resolves cleanly to Record<never, never> rather than the UnionToIntersection<never> = unknown footgun. Type tests cover single-pack extraction, fall-through for packs without __indexTypes, multi-pack merging, definition-level resolution, and the end-to-end defineContract path.
validateContract grows an optional indexTypeRegistry option. When supplied, validateSqlStorage looks up each index's type in the registry, validates options against the registered arktype validator, and rejects unknown types or options that fail validation. Even when no registry is supplied, options-without-type is still rejected. Test coverage: registered+valid passes, unregistered type rejects with the type name, invalid option fails with arktype path, strict-mode extra key rejects, options-without-type rejects, no type/options accepts, no registry skips type lookup but still rejects options-without-type.
…ions are set createIndex grows an optional extras argument carrying type and options. When supplied, it emits CREATE INDEX <name> ON <table> USING <method> (<cols>) WITH (<key> = <literal>, ...). The framework owns the renderer: strings are escapeLiteral-quoted, finite numbers stringify, booleans emit true/false, and any other leaf shape (null, NaN, objects) throws. CreateIndexCall and renderTypeScript carry the new args verbatim; the issue planner threads contract index.type / index.options through. Existing callers without extras get the same plain CREATE INDEX as before. Tests: 8 DDL emission cases in index-ddl.test.ts (plain, USING-only, USING+WITH, empty options, mixed scalars, single-quote escape, null rejection, NaN rejection) plus a renderTypeScript case for the new extras argument shape.
…ema indexes verifyIndexes now treats type and options as part of an index's identity. A contract index with type "gin" no longer matches a schema index that has columns the same but type "btree" (or no type at all); instead the verifier emits index_mismatch (the contract index needs creation) plus extra_index in strict mode (the schema index needs to be dropped). The issue planner naturally translates this pair into DROP + CREATE — the spec'd behavior for any change to columns / type / options. Unique constraints can only satisfy a contract index requirement when that requirement does not specify a type or options, since a unique constraint carries no index-method or storage-parameter information of its own. Tests: three new cases in schema-verify.semantic-satisfaction.test.ts — type differs (mismatch + extra), options differ (mismatch + extra), and type/options match exactly (no issues). The createContractTable / createSchemaTable helpers grow optional type / options fields so the index inputs accept the new shape.
…ry; drop helpers
Replace the bespoke bm25Index() helper, the bm25 namespace
(text/numeric/boolean/json/datetime/range/expression builders), the
Bm25*FieldConfig / *FieldOptions types, the TokenizerId catalog, and
the rich Bm25IndexConfig with a single defineIndexTypes().add("bm25",
{ options }) registration. The arktype validator narrows options to
{ key_field: string } in strict mode; per-field tokenizer / column
configuration is deferred to expression-index support.
paradedbPackMeta exposes the resulting __indexTypes phantom map so
SqlContractResult can narrow constraints.index({ type: "bm25" }) at
the call site, and exposes the registry entries on indexTypes so
contract assembly can register them at validation time.
Tests rewritten around the new shape: pack identity + capability
checks, single-entry verification, and the four validator paths
(valid options, missing key_field, extra key, wrong type).
Adds @prisma-next/sql-contract and arktype to paradedb deps.
…pe, options })
Threads the merged IndexTypes map all the way to the constraints DSL
inside .sql(({ constraints }) => ...) so wrong index type literals and
wrong option shapes surface as TS errors on the offending
constraints.index(...) line — not buried in a deep defineContract type.
Wiring:
- ContractModelBuilder gains a sixth generic IndexTypes (default wildcard)
- SqlContext<Fields, IndexTypes> picks PackAwareSqlConstraints<IndexTypes>
- A discriminated-union IndexInput<Name, IndexTypes> rejects unregistered
types and bad option shapes; "options without type" is also rejected
via the type?: never branch
- ComposedAuthoringHelpers.model becomes PackAwareModel<MergeAllPackIndexTypes>
so the helpers form (defineContract({...}, ({ model }) => ...)) carries
the merged map; the bare model() import keeps the wildcard default to
preserve backward compat
The runtime is unchanged — narrowing is pure type machinery.
e2e: test/integration/test/authoring/paradedb-bm25-narrowing.test.ts
exercises the real paradedbPack and asserts six cases via vitest +
@ts-expect-error: well-formed bm25 typechecks; unknown options key
rejects; missing key_field rejects; unregistered type rejects; options
without type rejects; bare model() degraded path still accepts (no
narrowing).
Adds extension-paradedb to integration-tests deps.
Adds parseObjectLiteralStringMap to psl-attribute-parsing — parses
brace-balanced { key: "value", ... } argument values into
Record<string, string>. Bare-identifier keys, quoted-string-literal
values; boolean / number / nested-object leaves are rejected with a
clear diagnostic (V1 is string-leaves-only per the spec).
The PSL grammar itself needs no change: splitTopLevelSegments is
already brace-aware, so the existing parser preserves the raw
{...} value through to the interpreter.
The @@index interpreter now extracts the type and options named
arguments alongside the existing @Map handling. Validates that
options requires a surrounding type, that type is a quoted string
literal, and lowers everything into IndexNode.type / IndexNode.options.
Tests cover: documented example shape, multi-key options, boolean
leaf rejection, number leaf rejection, options-without-type, malformed
object literal, and the unchanged "no type or options" path.
…pack
Adds an integration test that exercises the full PSL -> IR -> registry
validation path with the real paradedbPack:
1. The documented spec example (@@index([body], type: "bm25",
options: { key_field: "id" }, map: "doc_body_bm25_idx")) lowers
to a Contract IR index node carrying type, options, and name.
2. The lowered contract validates clean against a registry built from
paradedbIndexTypes.entries.
3. A PSL-authored bm25 index whose options miss the required key_field
is rejected by the registry with a clear error.
This complements the unit tests in contract-psl by going through the
actual pack metadata and validateContract path.
…exIR
The Postgres introspection query joins pg_am and reads
pg_class.reloptions so introspected SqlIndexIR now carries:
- type: the index method (dropped to undefined when amname is "btree",
since that is the Postgres default and a contract index without an
explicit type should match a default-method introspected index)
- options: pg_class.reloptions parsed from {key=value, ...} into
Record<string, string> (Postgres returns reloption values as raw
text regardless of the underlying scalar type)
The family verifier's indexExtrasMatch now compares contract options
to introspected options via String() coercion so a contract
`fillfactor: 70` matches an introspected `fillfactor: "70"` without
forcing a spurious DROP+CREATE.
Without this wiring the migration planner would treat any contract
index with type set as different from any introspected index on the
same columns — forcing DROP+CREATE on every plan even when the live
index already matches the contract.
Tests: four PGlite integration cases covering the matrix — default
btree (type/options unset), non-default gin (type set, options unset),
default btree with WITH options (type unset, options set), and gin
with WITH options (both set).
…clause Hoist extras?.options into a local so the WITH-clause check uses optional chaining instead of !-assertions, which biome rejects under lint/style/noNonNullAssertion.
…x_mismatch planner branch The issue-planner index_mismatch branch reconstructed the contract index from issue.expected (column names only), so adding a typed index to an existing table emitted CREATE INDEX without USING/WITH and rebuilt as a default btree. Look up the contract index by columns and pass type, options, and an explicit name through to CreateIndexCall.
…uild registry from packs in family validateContract The internal validateSqlStorage previously short-circuited the type lookup when no registry was supplied, and the SQL family runtime never passed one. Net result: the CLI validateContract codepath silently accepted any index type literal, defeating the design-time gate the registry is meant to be (per ADR 210). - Internal validateSqlStorage now takes a non-undefined IndexTypeRegistry. - Public validateContract defaults a missing options.indexTypeRegistry to an empty registry, so unregistered type literals fail validation regardless of caller setup. - createSqlFamilyInstance walks [adapter, target, ...extensions] for indexTypes entries via buildIndexTypeRegistry(), caches the registry once per family instance, and passes it to all four sqlValidateContract call sites (validateContract, verify, schemaVerify, sign).
…rant-opt-in, not framework-enforced The previous wording implied validateSqlStorage validates options "in strict mode", which read as a framework guarantee. arktype is loose-by-default and the framework merely invokes whatever validator the registrant constructed; strictness is a property of that validator. Make the recommendation explicit without claiming the framework imposes it.
Two gaps in the PSL integration coverage for the index-type registry:
unregistered type (TS DSL covered both compile-time and runtime, PSL
only covered compile-time via parser; runtime side untested) and an
empty options literal (parser produces {}; pipeline through to the
validator was unasserted). Both authored as @@index attributes, lowered
through the real paradedb pack, and asserted at the runtime registry
boundary.
…== undefined The contract-dsl `index()` factory and the emitter index serialization used truthy checks for `name`, `type`, and `options`, while the rest of the framework (validate.ts, verify-helpers.ts, op-factory-call.ts, issue-planner.ts) reads these fields with `=== undefined`. Align both authoring sites on the framework convention.
…pes to Record<never, never>
The bare model() import previously defaulted IndexTypes to a wildcard
(Record<string, { options: Record<string, unknown> }>), so any string
literal typechecked at constraints.index({ type, options }) — even when
no packs were attached and no index types were registered. Drop the
wildcard. With no attached packs, only the default-index form
constraints.index(cols.x) (no type/options) typechecks; type literals
are restricted to whatever attached packs registered.
The IndexInput discriminated union already had the right structure for
the empty case (keyof IndexTypes extends never → ConstraintOptions<Name>);
the wildcard was the inconsistency.
Three lowering/runtime tests authored typed indexes via the bare model()
to exercise IR pass-through. They now use createComposedAuthoringHelpers
with a small test pack (test/helpers/test-index-pack.ts) that registers
bm25 and hash with permissive option shapes. The paradedb narrowing test
flips: bare model() now @ts-expect-errors on type: "made-up" and accepts
the no-options default-index form.
contract-builder.normalization.test.ts: simplified the "extension index
config with expression fields" test to a single pass-through case using
the real { key_field: "id" } shape; the old nested-array fields[]
payload was a leftover from the deleted bm25Index() helper.
…ten bare index overloads
Three small cleanups enabled by the wildcard removal:
- composed-authoring-helpers.ts dropped its local copies of
ExtractIndexTypesFromPack, AllPackIndexTypeLiterals, and
MergedPackIndexTypes. They were near-duplicates of the canonical
versions in contract-types.ts. MergeAllPackIndexTypes now reuses
MergeExtensionIndexTypes directly.
- contract-types.ts MergeExtensionIndexTypes now clamps the value side
with Extract<..., { readonly options: unknown }>. The clamp tells the
type-checker the resulting map structurally extends IndexTypeMap, so
MergeAllPackIndexTypes no longer needs the extends infer M extends
IndexTypeMap ? M : Record<never, never> coercion at use sites.
- contract-dsl.ts bare index() overloads use ConstraintOptions<Name>
directly instead of IndexInput<Name, Record<never, never>>. With no
packs attached IndexInput already evaluates to ConstraintOptions, so
using it directly is self-documenting.
…es IndexTypeRegistration directly The previous design carried two parallel artifacts on every pack: the runtime indexTypes entries array and a __indexTypes phantom typed map. Both had to be kept in sync by hand, with no compile-time link between them — adding .add(...) to the builder would update one and silently leave the other behind. Introduce a read-only IndexTypeRegistration<TMap> interface that exposes just entries (runtime) and IndexTypes (phantom). IndexTypeBuilder extends it. The pack stores the builder verbatim in its indexTypes field; both halves flow from a single source of truth, and the read-only interface prevents the pack from being misused as a mutable registry. Consumer-side adjustments: - ExtractIndexTypesFromPack<P> reads P[indexTypes] typed as IndexTypeRegistration<infer M>. - buildIndexTypeRegistry iterates descriptor.indexTypes.entries. - __indexTypes? dropped from SqlContractResult (only tests read it; narrowing is verified end-to-end via paradedb-bm25-narrowing). - ParadeDB descriptor-meta.ts collapsed from base+intersection to a plain as const literal. - contract-builder.index-types.test.ts updated; the two SqlContractResult.__indexTypes tests removed (redundant with upstream MergeExtensionIndexTypes tests). ADR 210 §3 and the matching/lookup/dispatch prose updated to drop the "phantom-typed __indexTypes field" framing in favor of "single registration value carrying both halves".
…Types; remove buildSqlSpec cast
The cast at buildSqlSpec(createSqlConstraintsDsl() as unknown as
SqlContext<Fields, IndexTypes>[constraints]) was bridging from a bare
DSL value to one with pack-aware narrowing. Both are the same runtime
function; only the type-level signature differs.
Make createConstraintsDsl and createSqlConstraintsDsl generic over
IndexTypes, with the inner index overloads using IndexInput<Name,
IndexTypes> directly. buildSqlSpec passes its IndexTypes generic to
createSqlConstraintsDsl<IndexTypes>(); no cast needed.
The implementation signatures inner options field was loosened from
Record<string, unknown> to unknown, because per-pack option shapes
(like { key_field: string }) are closed object types that arent
strictly assignable to Record<string, unknown>. A single property-
scoped as Record<string, unknown> cast at the spread point stores the
value into the IR.
Net: one as unknown as cast removed, one minimal as cast introduced at
the storage boundary.
…p dead branches F08: paradedb-bm25-narrowing.test.ts gains two positive expectTypeOf assertions on Doc.__indexTypes — bare model() resolves to Record<never, never>, helpers-bound model() (with paradedb attached) carries the bm25 shape. The boundary is now locked at the type level rather than relying on absence of TS errors. F10: psl-index-type-options.integration.test.ts "rejects bm25 with bad options" case tightened from .toThrow(/key_field|bm25/) to instanceof ContractValidationError plus message-contains-both bm25 AND key_field. Either-word match was loose enough to pass on incidental wording. F14: dropped the hasExtras ternary in CreateIndexCall.toOp and the two issue-planner.ts branches (missing_table, index_mismatch). Calling createIndex(..., extras) with an empty extras object behaves the same as omitting the arg, so the conditional was dead. Postgres tests still 155/155.
…r-property error positions Collapsing constraints.index to a single multi-field overload makes TypeScript pin error positions to the offending property (an unknown options key, a wrong type literal, missing required option fields) instead of reporting a generic "no overload matches this call" at the call expression. The trade-off: callers always pass the field list as an array, even for single-column indexes: constraints.index(cols.body, ...) // before constraints.index([cols.body], ...) // after Migrated all in-repo callers (tests, integration test fixtures, CLI test fixtures, e2e migrations) in lockstep. The paradedb narrowing test now lands @ts-expect-error directives on the offending property line inside the options literal, matching the new TS error positions. Also fixed obsolete MySQL/MariaDB/MSSQL adapter mentions in ADR 210 and the PR description; only postgres and sqlite SQL adapters exist.
…owering, not at runtime
Index-type validation now runs inside buildSqlContractFromDefinition,
the shared lowering both TS (defineContract) and PSL
(interpretPslDocumentToSqlContract) authoring funnel through. Bad
types or option shapes throw at authoring time.
- packages/2-sql/1-core/contract/src/validators.ts: validateIndexTypes
lives here next to validateStorageSemantics. Both validate the
storage IR; build-contract.ts assertStorageSemantics calls them in
one step.
- packages/2-sql/1-core/contract/src/validate.ts: drop validateIndexTypes
(moved to validators.ts) and the indexTypeRegistry option from the
public validateContract API. Runtime keeps structural / referential /
semantic checks; no registry needed.
- packages/2-sql/2-authoring/contract-ts/src/build-contract.ts: build a
per-contract IndexTypeRegistry from definition.target +
definition.extensionPacks inline in assertStorageSemantics.
- packages/2-sql/9-family/src/core/control-instance.ts: strip the family
registry build, the DescriptorWithIndexTypes interface, and the
{ indexTypeRegistry } argument at all four sqlValidateContract call
sites.
Tests:
- validate.test.ts cases call validateIndexTypes directly from
validators.ts.
- contract-builder.constraints.test.ts asserts defineContract throws
at authoring time on an unregistered type.
- contract-psl/test/interpreter.test.ts attaches a permissive bm25
test pack so the existing PSL parser tests can lower successfully.
- paradedb-bm25-narrowing.test.ts wraps the @ts-expect-error cases in
expect(...).toThrow(...) since defineContract now throws synchronously.
- psl-index-type-options.integration.test.ts: rewrite negative cases
to assert interpret() throws directly (validation happens inside
the interpreter, not in a separate validateContract call).
Docs:
- ADR 210 §4 retitled "Validation seam at the ContractIR -> Contract
boundary" and rewritten around the new seam.
- PR description: "Authoring-time validator" bullet replaces the old
"Validator hook" + "Family runtime wiring" pair.
…switch Wraps the single-field column reference in an array to match the post-rename API. The single-field overload of constraints.index(...) was removed in lockstep with the array-only switch; in-repo callers were migrated but the user-facing README example was missed and would not compile against this branch. Addresses F01.
…validators Splits validateIndexTypes into a sibling module (index-type-validation.ts) to reflect its real layering. The other validators in validators.ts only check internal consistency of the loaded contract JSON; validateIndexTypes checks the contract against an external IndexTypeRegistry assembled from extension packs that are not part of the JSON, so it correctly belongs at the ContractIR -> Contract lowering seam (where the packs are in scope) rather than next to validators called from validateContract. The file boundary surfaces this distinction in a way a docstring would not: validators.ts becomes the natural home for things validateContract would call, while index-type-validation.ts is clearly a registry-dependent check called only from the authoring layer. Updates the import in build-contract.ts and the test in validate.test.ts. Adds a new ./index-type-validation export entry to the package and the tsdown bundle. Pure file move + import update; no behavior change. Addresses F04.
…, and PSL diagnostics Six focused tweaks following PR review: - contract-lowering.ts: switch index name/type/options spreads from truthy checks to ifDefined() so the lowering matches build-contract.ts semantics. Empty-string type and empty-record options are not valid today, but the divergence between IR writers in the same authoring layer is the kind of thing that drifts and bites later. (F02) - build-contract.ts: replace the inline structural cast on pack.indexTypes with a positive type guard that throws a contextual error naming the pack id when the value is not a real IndexTypeRegistration. A misconfigured third-party pack now produces a useful error instead of an opaque "entries is not iterable" stack trace. (F03) - op-factory-call.ts: comment why the field is named indexType rather than typeName — typeName is read by the planner location helper and identifies a CREATE TYPE target on CreateEnumTypeCall. (F05) - control-adapter.ts: parsePgReloptions now throws on a malformed entry with no "=" separator, naming both the entry and the index. The previous silent skip would surface downstream as a confusing index_mismatch with no breadcrumb back to the malformed catalog row. The function takes an indexName parameter threaded from the introspection call site and is exported so it can be tested directly. (F06) - validators.ts: sort options keys before serializing the duplicate-index signature so two indexes whose options differ only in key order are detected as duplicates. JSON.stringify preserves insertion order, which was reachable through TS authoring across two source files. (F07) - psl-attribute-parsing.ts: append a short hint to the "must be a quoted string literal" diagnostic explaining that V1 PSL @@index options support string leaves only and pointing at the TS authoring surface for non-string options. (F09) Adds three unit tests, written before each fix: - validators.test.ts: detects two indexes whose options differ only in key order as duplicates. - contract-builder.constraints.test.ts: a pack with indexTypes set to a non-registration value triggers a contextual error naming the pack id. - control-adapter.test.ts: parsePgReloptions throws on malformed entry, parses well-formed entries, returns undefined for null/empty. Existing PSL interpreter tests use a regex substring match against "must be a quoted string literal" and continue to pass with the appended hint. Addresses F02, F03, F05, F06, F07, F09.
…n-first, no project state Restructure ADR 210 to follow the write-architecture-docs conventions applied across the recent ADRs (e.g. ADR 209): one-sentence decision callout above the fold, complete grounding example (ParadeDB bm25 declaration + TS/PSL authoring + the three failure modes), narrative buildup of the registry primitive, per-contract composition, lowering seam, strictness, rendering, identity, and authoring surfaces. Drop project-state references unsuitable for a long-lived doc: the Status / Date / Spec headers, the link to a transient projects/ spec file, the using/config rename narrative, and the V1/follow-up framing in non-goals. Reframe non-goals as architectural boundaries (no built-in entries — the registry is open by design; default B-tree is expressed by omitting type). Add an explicit subsection on the load-bearing layering between validateContract (JSON-internal-consistency only) and the authoring-time validateIndexTypes (registry-dependent, lives at the ContractIR -> Contract lowering seam). Clarify that PSL string-leaf-only options is a grammar property, not a registry property. No design changes; same decisions, same trade-offs.
wmadden
approved these changes
May 9, 2026
Contributor
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
I made a few small corrections, I think this is good to go 👍🏻
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-2390
Intent
Make
@@index(type:, options:)a real first-class authoring surface across PSL and TS, replacing the inertusing/configplaceholders that round-tripped silently. Authors can now register their own index types via an extension pack — the registered shape narrows the TS DSL at the call site, validates the contract IR at runtime, drivesCREATE INDEX … USING <method> WITH (…)DDL, and survives a round-trip through Postgres introspection. ParadeDB's bespokebm25Index()helper is replaced with a registry entry; the helper and its column-builder namespace (bm25.text,.numeric, …) are deleted.Change map
defineIndexTypes().add(literal, { options })returns anIndexTypeRegistration<TMap>carrying both runtimeentriesand the type-levelIndexTypesphantom map;createIndexTypeRegistry()builds the per-contract runtime registry (instance-based, not module-global; two contracts in the same workspace see different registries).using/config→type/optionsin lockstep across IR, schema-IR, validators, lowering, emitter, and the ParadeDB helper.ContractIR → Contractlowering seam (buildSqlContractFromDefinition), where both TS (defineContract) and PSL (interpretPslDocumentToSqlContract) authoring converge. The lowering builds a per-contractIndexTypeRegistryfrom the definition's attached packs and runsvalidateIndexTypesagainst the in-memory contract; bad types or option shapes throw at authoring time, not at runtime. RuntimevalidateContractkeeps structural / referential / semantic validation but no longer needs a registry.createIndexemitsUSING <method> WITH (key = literal, ...); framework-owned scalar renderer withescapeLiteralquoting.missing_tableandindex_mismatchbranches now look up the contract index by columns and threadname/type/optionsthrough toCreateIndexCall. Without this, adding a typed index to an existing table emitted a default-btree.(columns, type, options); mismatch on any one emitsindex_mismatch+extra_index(= DROP + CREATE).ExtractIndexTypesFromPack<P>readsP['indexTypes']typed asIndexTypeRegistration<infer M>(no separate__indexTypesphantom); the merged map flows throughComposedAuthoringHelpers.model: PackAwareModel<...>intoContractModelBuilder(sixthIndexTypesgeneric) andSqlContext<Fields, IndexTypes>to a discriminated-unionIndexInput<Name, IndexTypes>soconstraints.index({ type, options })narrows at the call site.@@index([cols], type: "...", options: { k: "v" })parses; V1 admits string-literal leaves only.defineIndexTypes().add('bm25', { options: type({ '+': 'reject', key_field: 'string' }) })replaces ~190 lines of bespoke helpers; the pack metadata stores the registration directly inindexTypes:(no separate phantom field).pg_am, readspg_class.reloptions; dropsbtreetoundefined(Postgres default) so a default-method contract index matches without spurious DROP+CREATE.The story
The contract IR already had inert
using/configfields. The shape of the registry came first: rename totype/options(matching Prisma's@@index(type:)), put the registered shape in one place, and make every consumer derive from that one place.The registry is per-contract, not module-global. A pack stores a single registration value (the output of
defineIndexTypes()) on its metadata; contract assembly creates a fresh registry and registers each pack's entries into it. Two contracts in the same workspace with different pack lists see different valid type sets. The first attempt at this used a module-levelMap; the maintainer flagged that as wrong (WHY IS REGISTRY A GLOBAL?) and the design moved to the instance-based shape it has now.One field, both halves. The factory builder produces a value with two surfaces: a runtime
entriesarray and a TypeScript-onlyIndexTypesphantom map. The pack stores that value verbatim in itsindexTypesfield; the builder type extends a read-onlyIndexTypeRegistration<TMap>interface so the pack can't be misused as a mutable registry from outside. Both halves stay in lockstep automatically — adding.add('hnsw', …)to the builder updates runtime registration and call-site narrowing in one step.Type-level threading lands at the call site, not at
defineContract. The registered shape flows through three legs: (1)indexTypesregistration on the pack, (2)MergeAllPackIndexTypesover family + target + extension packs inComposedAuthoringHelpers, (3) a sixthIndexTypesgeneric onContractModelBuilderthat threads throughSqlContext<Fields, IndexTypes>to a discriminated-unionIndexInput<Name, IndexTypes>. The result: insidedefineContract({...}, ({ model }) => …),constraints.index(cols.body, { type: 'made-up', options: {} })is a TS error on that line — not buried fifty levels deep in adefineContracttype. The baremodel()import (no packs available) defaultsIndexTypestoRecord<never, never>, so only the no-options default-index form typechecks; there is noWildcardIndexTypesescape hatch.Migration diff falls out of identity. Schema-verify already grouped indexes by columns; once
typeandoptionsare part of identity, a contractginagainst a schemabtreesimply doesn't match —verifyIndexesemitsindex_mismatchfor the new contract index and (in strict mode)extra_indexfor the stale schema index. The Postgres planner then looks up the contract index by columns fromctx.toContractand emits a fully-typedCreateIndexCall(DROP + CREATE). NoALTER INDEXpath; Postgres has no clean ALTER for index method orWITH-key changes anyway.Introspection had to come along. Without it, every plan against a live DB would force DROP+CREATE on any contract index whose
typeis set. The Postgres adapter now joinspg_am, readspg_class.reloptions, and the family verifier'sindexExtrasMatchdoes string-coerced option comparison so contractfillfactor: 70matches an introspectedfillfactor: '70'(Postgres returns reloption values as raw text regardless of the underlying scalar).btreeintrospects toundefined— it's the Postgres default — so a default-method contract index matches a default-method live index without ceremony.ParadeDB shrinks dramatically. The old
bm25Index()helper plus the entirebm25.{text, numeric, json, expression, …}namespace and theBm25*FieldConfig/TokenizerIdtypes are deleted (~470 lines). What's left is onedefineIndexTypes().add('bm25', { options: type({ '+': 'reject', key_field: 'string' }) })declaration plus a plain constparadedbPackMetathat stores it underindexTypes:. Per-field tokenizer / column configuration is deferred to a future expression-index surface.Behavior changes & evidence
Authoring:
constraints.index(cols.body, { type: 'bm25', options: { key_field: 'id' } })now typechecks against the merged pack registry;type: 'made-up', missing requiredoptionskeys, and unknown options keys are TS errors at the offending line. PSL's@@index([body], type: "bm25", options: { key_field: "id" })lowers to the same IR shape. Contract validation rejects unregistered types and bad options at runtime via arktype (strict-key rejection is registrant-opt-in via'+': 'reject'). Postgres DDL emission producesCREATE INDEX … USING <method> WITH (…)on both new tables and existing tables (typed indexes added later). Schema verification treats type/options as part of identity. Postgres introspection populates the new fields; the verifier's loose option comparison handles the contract/reloptionstyping gap.Evidence:
index_mismatchthreading: 3 cases in packages/3-targets/3-targets/postgres/test/migrations/issue-planner.test.ts (extras threaded; named index honored; default-name fallback).Compatibility / migration / risk
using/config. The fields were inert (no PSL or TS surface populated them), and the only in-repo writer wasbm25Index()which is removed in the same PR. Out-of-repo consumers using the old field names will need to rename in lockstep.ALTER INDEXfor type/options changes. Any change to columns, type, or options isDROP INDEX+CREATE INDEX. This is a property of how Postgres handles index method andWITH-key changes, not a regression introduced here. Lock and rebuild cost on large tables is non-trivial.options: { fastupdate: false }doesn't parse; onlyoptions: { key: "value" }(string leaves) works from PSL. TS authoring is unrestricted. The PSL grammar lift ships with the built-in-entries follow-up that actually needs non-string options.USING/WITH).constraints.index(...)is array-only. The single-field shortcut (constraints.index(cols.x, { type, options })) is removed; callers always pass the field list as an array (constraints.index([cols.x], { type, options })). Collapsing to one signature was what let TypeScript pin per-property errors at the offending key (e.g. an unknownoptionskey, a wrongtypeliteral, missing required option fields) instead of reporting a generic "no overload matches this call" at the call expression. In-repo callers were migrated in lockstep.Follow-ups / open questions
btree,hash,gin,gist,brin,spgist(tracked separately).options— needed once built-in entries land (fastupdate: false,fillfactor: 70).gist's per-column operator classes) — out of scope; v1 carriesoptionsas a single object on the index, not per-column.pdb.*query-builder operator surface (@@@, etc.) for ParadeDB BM25 search — separate workstream.Non-goals / intentionally out of scope
ALTER INDEXrendering paths.type/options.Summary by CodeRabbit
New Features
typeandoptions; PSL and authoring DSL accept and validate them.USING <type> WITH (...).Refactor
constraints.index([ ... ]).Documentation