feat(pgvector): add cosineSimilarity operation#271
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdded support for a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/runtime-executor
@prisma-next/mongo-core
@prisma-next/mongo-orm
@prisma-next/mongo-runtime
@prisma-next/sql-runtime
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract-authoring
@prisma-next/contract-ts
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/eslint-plugin
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@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/family-sql
@prisma-next/sql-kysely-lane
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder-new
@prisma-next/sql-lane
@prisma-next/target-postgres
@prisma-next/adapter-postgres
@prisma-next/driver-postgres
@prisma-next/core-control-plane
@prisma-next/core-execution-plane
@prisma-next/config
@prisma-next/contract
@prisma-next/operations
@prisma-next/plan
@prisma-next/utils
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/3-extensions/pgvector/test/result-type.types.test-d.ts (1)
321-325: Add an explicitexpectTypeOfassertion forRow['similarity'].The current assignment check works, but a direct type assertion will better protect against accidental widening (
any/unknown) regressions.♻️ Suggested type assertion
type Row = ResultType<typeof _plan>; const similarityValue = 0 as Row['similarity']; const similarityAsExpected: number = similarityValue; void similarityAsExpected; + expectTypeOf({} as Row).toExtend<{ similarity: number }>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/pgvector/test/result-type.types.test-d.ts` around lines 321 - 325, The test currently infers Row via ResultType<typeof _plan> and checks assignment to number indirectly; add an explicit expectTypeOf assertion for Row['similarity'] to lock its static type to number. Locate the Row/ResultType/_plan usage in the test and add an expectTypeOf assertion that compares Row['similarity'] to number (e.g., expectTypeOf<Row['similarity']>().toEqualTypeOf<number>()) so the type cannot widen to any/unknown.packages/2-sql/4-lanes/sql-builder-new/test/integration/extension-functions.test.ts (1)
42-53: Strengthen the WHERE-filter test with an explicit negative assertion.Right now the test only proves inclusion of
id=1. Adding an exclusion check (e.g., orthogonal row) makes regressions easier to catch.♻️ Suggested test hardening
expect(rows.length).toBeGreaterThan(0); expect(rows.some((r) => r.id === 1)).toBe(true); + expect(rows.some((r) => r.id === 3)).toBe(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/4-lanes/sql-builder-new/test/integration/extension-functions.test.ts` around lines 42 - 53, The test "cosineSimilarity filters in WHERE" only asserts presence of the matching row (id === 1) and should also assert exclusion of an orthogonal row to catch regressions; update the test that calls db().posts.select('id').where((f, fns) => fns.gt(fns.cosineSimilarity(f.embedding, [1, 0, 0]), 0.5)).all() to include an explicit negative assertion such as expect(rows.some(r => r.id === 3)).toBe(false) (or equivalent) so the orthogonal embedding is confirmed filtered out.packages/3-extensions/pgvector/src/core/descriptor-meta.ts (1)
37-64: Consider reusing the lowering constants to avoid duplication.The
cosineDistanceLoweringandcosineSimilarityLoweringconstants are defined at lines 6-16, but the same template strings are duplicated inline inpgvectorQueryOperations. Reusing the constants improves maintainability and ensures consistency.♻️ Suggested refactor to reuse lowering constants
export const pgvectorQueryOperations: readonly QueryOperationDescriptor[] = [ { method: 'cosineDistance', args: [ { codecId: pgvectorTypeId, nullable: false }, { codecId: pgvectorTypeId, nullable: false }, ], returns: { codecId: 'pg/float8@1', nullable: false }, - lowering: { - targetFamily: 'sql', - strategy: 'function', - template: '{{self}} <=> {{arg0}}', - }, + lowering: cosineDistanceLowering, }, { method: 'cosineSimilarity', args: [ { codecId: pgvectorTypeId, nullable: false }, { codecId: pgvectorTypeId, nullable: false }, ], returns: { codecId: 'pg/float8@1', nullable: false }, - lowering: { - targetFamily: 'sql', - strategy: 'function', - template: '1 - ({{self}} <=> {{arg0}})', - }, + lowering: cosineSimilarityLowering, }, ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/pgvector/src/core/descriptor-meta.ts` around lines 37 - 64, The pgvectorQueryOperations array duplicates the lowering objects inline for cosineDistance and cosineSimilarity; replace those inline lowering objects by referencing the existing constants cosineDistanceLowering and cosineSimilarityLowering instead (update the entries in pgvectorQueryOperations so each operation's lowering property is assigned the corresponding constant), ensuring the types remain QueryOperationDescriptor and no other fields change.packages/3-extensions/pgvector/test/operations.test.ts (1)
29-49: PrefertoMatchObjectfor multiple related assertions on the same object.The coding guidelines recommend using object matchers over multiple individual
expect().toBe()orexpect().toEqual()calls when checking 2 or more related values. Both operation blocks could be consolidated.♻️ Suggested refactor using toMatchObject
const cosineDistanceOp = operations.find((op) => op.method === 'cosineDistance'); - expect(cosineDistanceOp).toBeDefined(); - expect(cosineDistanceOp?.forTypeId).toBe('pg/vector@1'); - expect(cosineDistanceOp?.args).toEqual([{ kind: 'param' }]); - expect(cosineDistanceOp?.returns).toEqual({ kind: 'builtin', type: 'number' }); - expect(cosineDistanceOp?.lowering).toEqual({ - targetFamily: 'sql', - strategy: 'function', - template: '{{self}} <=> {{arg0}}', - }); + expect(cosineDistanceOp).toMatchObject({ + method: 'cosineDistance', + forTypeId: 'pg/vector@1', + args: [{ kind: 'param' }], + returns: { kind: 'builtin', type: 'number' }, + lowering: { + targetFamily: 'sql', + strategy: 'function', + template: '{{self}} <=> {{arg0}}', + }, + }); const cosineSimilarityOp = operations.find((op) => op.method === 'cosineSimilarity'); - expect(cosineSimilarityOp).toBeDefined(); - expect(cosineSimilarityOp?.forTypeId).toBe('pg/vector@1'); - expect(cosineSimilarityOp?.args).toEqual([{ kind: 'param' }]); - expect(cosineSimilarityOp?.returns).toEqual({ kind: 'builtin', type: 'number' }); - expect(cosineSimilarityOp?.lowering).toEqual({ - targetFamily: 'sql', - strategy: 'function', - template: '1 - ({{self}} <=> {{arg0}})', - }); + expect(cosineSimilarityOp).toMatchObject({ + method: 'cosineSimilarity', + forTypeId: 'pg/vector@1', + args: [{ kind: 'param' }], + returns: { kind: 'builtin', type: 'number' }, + lowering: { + targetFamily: 'sql', + strategy: 'function', + template: '1 - ({{self}} <=> {{arg0}})', + }, + });As per coding guidelines: "Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/pgvector/test/operations.test.ts` around lines 29 - 49, Consolidate the multiple assertions for each operation into a single object matcher: replace the repeated expects on cosineDistanceOp and cosineSimilarityOp (found via operations.find) with expect(cosineDistanceOp).toMatchObject({...}) and expect(cosineSimilarityOp).toMatchObject({...}) respectively, including forTypeId, args, returns and lowering (with lowering.template, lowering.strategy, lowering.targetFamily) so each block verifies the related fields in one matcher instead of multiple toBe/toEqual calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/2-sql/4-lanes/sql-builder-new/test/integration/extension-functions.test.ts`:
- Around line 42-53: The test "cosineSimilarity filters in WHERE" only asserts
presence of the matching row (id === 1) and should also assert exclusion of an
orthogonal row to catch regressions; update the test that calls
db().posts.select('id').where((f, fns) =>
fns.gt(fns.cosineSimilarity(f.embedding, [1, 0, 0]), 0.5)).all() to include an
explicit negative assertion such as expect(rows.some(r => r.id ===
3)).toBe(false) (or equivalent) so the orthogonal embedding is confirmed
filtered out.
In `@packages/3-extensions/pgvector/src/core/descriptor-meta.ts`:
- Around line 37-64: The pgvectorQueryOperations array duplicates the lowering
objects inline for cosineDistance and cosineSimilarity; replace those inline
lowering objects by referencing the existing constants cosineDistanceLowering
and cosineSimilarityLowering instead (update the entries in
pgvectorQueryOperations so each operation's lowering property is assigned the
corresponding constant), ensuring the types remain QueryOperationDescriptor and
no other fields change.
In `@packages/3-extensions/pgvector/test/operations.test.ts`:
- Around line 29-49: Consolidate the multiple assertions for each operation into
a single object matcher: replace the repeated expects on cosineDistanceOp and
cosineSimilarityOp (found via operations.find) with
expect(cosineDistanceOp).toMatchObject({...}) and
expect(cosineSimilarityOp).toMatchObject({...}) respectively, including
forTypeId, args, returns and lowering (with lowering.template,
lowering.strategy, lowering.targetFamily) so each block verifies the related
fields in one matcher instead of multiple toBe/toEqual calls.
In `@packages/3-extensions/pgvector/test/result-type.types.test-d.ts`:
- Around line 321-325: The test currently infers Row via ResultType<typeof
_plan> and checks assignment to number indirectly; add an explicit expectTypeOf
assertion for Row['similarity'] to lock its static type to number. Locate the
Row/ResultType/_plan usage in the test and add an expectTypeOf assertion that
compares Row['similarity'] to number (e.g.,
expectTypeOf<Row['similarity']>().toEqualTypeOf<number>()) so the type cannot
widen to any/unknown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 3443e60d-92b4-4dff-886c-5c6aad5e6093
📒 Files selected for processing (11)
packages/2-sql/4-lanes/sql-builder-new/test/integration/extension-functions.test.tspackages/3-extensions/pgvector/README.mdpackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/src/exports/control.tspackages/3-extensions/pgvector/src/exports/runtime.tspackages/3-extensions/pgvector/src/types/operation-types.tspackages/3-extensions/pgvector/test/manifest.test.tspackages/3-extensions/pgvector/test/operations.test.tspackages/3-extensions/pgvector/test/result-type.types.test-d.tstest/integration/test/pgvector.test.tstest/utils/src/operation-descriptors.ts
e365e2b to
6ad46fb
Compare
95f2de4 to
a283845
Compare
Follow up to <#270>. Add a separate `cosineSimilarity` operation with `1 - ({{self}} <=> {{arg0}})` lowering because it is also useful.
6ad46fb to
7fc9d6f
Compare
Follow up to <#270>. Add a separate `cosineSimilarity` operation with `1 - ({{self}} <=> {{arg0}})` lowering because it is also useful. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `cosineSimilarity` operation for pgvector extension, enabling vector similarity calculations computed as 1 minus cosine distance, with results ranging from 0 to 1. * **Documentation** * Updated pgvector README with `cosineSimilarity` operation documentation, including TypeScript signatures and SQL rendering examples. * **Tests** * Added integration tests validating `cosineSimilarity` operation behavior and query results. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Follow up to <#270>. Add a separate `cosineSimilarity` operation with `1 - ({{self}} <=> {{arg0}})` lowering because it is also useful. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `cosineSimilarity` operation for pgvector extension, enabling vector similarity calculations computed as 1 minus cosine distance, with results ranging from 0 to 1. * **Documentation** * Updated pgvector README with `cosineSimilarity` operation documentation, including TypeScript signatures and SQL rendering examples. * **Tests** * Added integration tests validating `cosineSimilarity` operation behavior and query results. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Follow up to #270.
Add a separate
cosineSimilarityoperation with1 - ({{self}} <=> {{arg0}})lowering because it is also useful.Summary by CodeRabbit
Release Notes
New Features
cosineSimilarityoperation for pgvector extension, enabling vector similarity calculations computed as 1 minus cosine distance, with results ranging from 0 to 1.Documentation
cosineSimilarityoperation documentation, including TypeScript signatures and SQL rendering examples.Tests
cosineSimilarityoperation behavior and query results.