From ee53f54414fd3f063f2441504cc6976aa319f795 Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Fri, 10 Jan 2025 13:12:57 +0000 Subject: [PATCH 1/8] Improve generative/reranker UX: - Allow updating of generative and reranker configs dynamically - Fix typing issues with generative and reranker configs --- ci/docker-compose.yml | 2 +- src/collections/config/classes.ts | 44 +++++++++++++++- src/collections/config/integration.test.ts | 48 +++++++++++++++++ src/collections/config/types/index.ts | 10 ++-- src/collections/config/unit.test.ts | 60 +++++++++++++++++++--- src/collections/configure/index.ts | 2 + 6 files changed, 154 insertions(+), 12 deletions(-) diff --git a/ci/docker-compose.yml b/ci/docker-compose.yml index 74746f97..b762e87c 100644 --- a/ci/docker-compose.yml +++ b/ci/docker-compose.yml @@ -21,7 +21,7 @@ services: AUTHENTICATION_ANONYMOUS_ACCESS_ENABLED: 'true' PERSISTENCE_DATA_PATH: '/var/lib/weaviate' DEFAULT_VECTORIZER_MODULE: 'text2vec-contextionary' - ENABLE_MODULES: text2vec-contextionary,backup-filesystem,img2vec-neural + ENABLE_MODULES: text2vec-contextionary,backup-filesystem,img2vec-neural,generative-cohere,reranker-cohere BACKUP_FILESYSTEM_PATH: "/tmp/backups" CLUSTER_GOSSIP_BIND_PORT: "7100" CLUSTER_DATA_BIND_PORT: "7101" diff --git a/src/collections/config/classes.ts b/src/collections/config/classes.ts index 1b023269..7c7fdc65 100644 --- a/src/collections/config/classes.ts +++ b/src/collections/config/classes.ts @@ -3,6 +3,7 @@ import { WeaviateInvalidInputError } from '../../errors.js'; import { WeaviateClass, WeaviateInvertedIndexConfig, + WeaviateModuleConfig, WeaviateMultiTenancyConfig, WeaviateReplicationConfig, WeaviateVectorIndexConfig, @@ -17,7 +18,15 @@ import { VectorIndexConfigFlatUpdate, VectorIndexConfigHNSWUpdate, } from '../configure/types/index.js'; -import { CollectionConfigUpdate, VectorIndexType } from './types/index.js'; +import { + CollectionConfigUpdate, + GenerativeConfig, + GenerativeSearch, + ModuleConfig, + Reranker, + RerankerConfig, + VectorIndexType, +} from './types/index.js'; export class MergeWithExisting { static schema( @@ -27,6 +36,8 @@ export class MergeWithExisting { ): WeaviateClass { if (update === undefined) return current; if (update.description !== undefined) current.description = update.description; + if (update.generative !== undefined) + current.moduleConfig = MergeWithExisting.generative(current.moduleConfig, update.generative); if (update.invertedIndex !== undefined) current.invertedIndexConfig = MergeWithExisting.invertedIndex( current.invertedIndexConfig, @@ -42,6 +53,8 @@ export class MergeWithExisting { current.replicationConfig!, update.replication ); + if (update.reranker !== undefined) + current.moduleConfig = MergeWithExisting.reranker(current.moduleConfig, update.reranker); if (update.vectorizers !== undefined) { if (Array.isArray(update.vectorizers)) { current.vectorConfig = MergeWithExisting.vectors(current.vectorConfig, update.vectorizers); @@ -61,6 +74,35 @@ export class MergeWithExisting { return current; } + static generative( + current: WeaviateModuleConfig, + update: ModuleConfig + ): WeaviateModuleConfig { + if (current === undefined) throw Error('Module config is missing from the class schema.'); + if (update === undefined) return current; + const generative = update.name === 'generative-azure-openai' ? 'generative-openai' : update.name; + const currentGenerative = current[generative] as Record; + current[generative] = { + ...currentGenerative, + ...update.config, + }; + return current; + } + + static reranker( + current: WeaviateModuleConfig, + update: ModuleConfig + ): WeaviateModuleConfig { + if (current === undefined) throw Error('Module config is missing from the class schema.'); + if (update === undefined) return current; + const reranker = current[update.name] as Record; + current[update.name] = { + ...reranker, + ...update.config, + }; + return current; + } + static invertedIndex( current: WeaviateInvertedIndexConfig, update: InvertedIndexConfigUpdate diff --git a/src/collections/config/integration.test.ts b/src/collections/config/integration.test.ts index e930a0ef..3585957c 100644 --- a/src/collections/config/integration.test.ts +++ b/src/collections/config/integration.test.ts @@ -2,8 +2,11 @@ import { WeaviateUnsupportedFeatureError } from '../../errors.js'; import weaviate, { WeaviateClient, weaviateV2 } from '../../index.js'; import { + GenerativeCohereConfig, + ModuleConfig, MultiTenancyConfig, PropertyConfig, + RerankerCohereConfig, VectorIndexConfigDynamic, VectorIndexConfigHNSW, } from './types/index.js'; @@ -621,4 +624,49 @@ describe('Testing of the collection.config namespace', () => { expect(config.vectorizers.default.indexType).toEqual('hnsw'); expect(config.vectorizers.default.vectorizer.name).toEqual('none'); }); + + it.only('should be able to update the generative & reranker configs of a collection', async () => { + const collectionName = 'TestCollectionConfigUpdateGenerative'; + const collection = client.collections.get(collectionName); + await client.collections.create({ + name: collectionName, + vectorizers: weaviate.configure.vectorizer.none(), + }); + let config = await collection.config.get(); + expect(config.generative).toBeUndefined(); + + await collection.config.update({ + generative: weaviate.reconfigure.generative.cohere({ + model: 'model', + }), + }); + + config = await collection.config.get(); + expect(config.generative).toEqual>({ + name: 'generative-cohere', + config: { + model: 'model', + }, + }); + + await collection.config.update({ + reranker: weaviate.reconfigure.reranker.cohere({ + model: 'model', + }), + }); + + config = await collection.config.get(); + expect(config.generative).toEqual>({ + name: 'generative-cohere', + config: { + model: 'model', + }, + }); + expect(config.reranker).toEqual>({ + name: 'reranker-cohere', + config: { + model: 'model', + }, + }); + }); }); diff --git a/src/collections/config/types/index.ts b/src/collections/config/types/index.ts index 8e5e1fb2..04630b1c 100644 --- a/src/collections/config/types/index.ts +++ b/src/collections/config/types/index.ts @@ -9,8 +9,8 @@ import { ReplicationConfigUpdate, VectorConfigUpdate, } from '../../configure/types/index.js'; -import { GenerativeConfig } from './generative.js'; -import { RerankerConfig } from './reranker.js'; +import { GenerativeConfig, GenerativeSearch } from './generative.js'; +import { Reranker, RerankerConfig } from './reranker.js'; import { VectorIndexType } from './vectorIndex.js'; import { VectorConfig } from './vectorizer.js'; @@ -93,22 +93,24 @@ export type ShardingConfig = { export type CollectionConfig = { name: string; description?: string; - generative?: GenerativeConfig; + generative?: ModuleConfig; invertedIndex: InvertedIndexConfig; multiTenancy: MultiTenancyConfig; properties: PropertyConfig[]; references: ReferenceConfig[]; replication: ReplicationConfig; - reranker?: RerankerConfig; + reranker?: ModuleConfig; sharding: ShardingConfig; vectorizers: VectorConfig; }; export type CollectionConfigUpdate = { description?: string; + generative?: ModuleConfig; invertedIndex?: InvertedIndexConfigUpdate; multiTenancy?: MultiTenancyConfigUpdate; replication?: ReplicationConfigUpdate; + reranker?: ModuleConfig; vectorizers?: | VectorConfigUpdate | VectorConfigUpdate[]; diff --git a/src/collections/config/unit.test.ts b/src/collections/config/unit.test.ts index fdc83c5d..ad27f61c 100644 --- a/src/collections/config/unit.test.ts +++ b/src/collections/config/unit.test.ts @@ -1,9 +1,11 @@ import { WeaviateInvertedIndexConfig, + WeaviateModuleConfig, WeaviateMultiTenancyConfig, WeaviateVectorsConfig, } from '../../openapi/types'; import { MergeWithExisting } from './classes'; +import { GenerativeCohereConfig, RerankerCohereConfig } from './types'; describe('Unit testing of the MergeWithExisting class', () => { const invertedIndex: WeaviateInvertedIndexConfig = { @@ -122,8 +124,22 @@ describe('Unit testing of the MergeWithExisting class', () => { autoTenantCreation: false, }; + const moduleConfig: WeaviateModuleConfig = { + 'generative-cohere': { + kProperty: 0.1, + model: 'model', + maxTokensProperty: '5', + returnLikelihoodsProperty: 'likelihoods', + stopSequencesProperty: ['and'], + temperatureProperty: 5.2, + }, + 'reranker-cohere': {}, + }; + + const parse = (config: any) => JSON.parse(JSON.stringify(config)); + it('should merge a partial invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(JSON.parse(JSON.stringify(invertedIndex)), { + const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { b: 0.9, }, @@ -147,7 +163,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a no quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(hnswVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -196,7 +212,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a PQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(hnswVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -245,7 +261,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(hnswVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -280,7 +296,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a SQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(hnswVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -353,7 +369,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge full multi tenancy config with existing schema', () => { - const merged = MergeWithExisting.multiTenancy(JSON.parse(JSON.stringify(multiTenancyConfig)), { + const merged = MergeWithExisting.multiTenancy(parse(multiTenancyConfig), { autoTenantActivation: true, autoTenantCreation: true, }); @@ -363,4 +379,36 @@ describe('Unit testing of the MergeWithExisting class', () => { autoTenantCreation: true, }); }); + + it('should merge a generative config with existing schema', () => { + const merged = MergeWithExisting.generative(parse(moduleConfig), { + name: 'generative-cohere', + config: { + kProperty: 0.2, + } as GenerativeCohereConfig, + }); + expect(merged).toEqual({ + ...moduleConfig, + 'generative-cohere': { + ...(moduleConfig['generative-cohere'] as any), + kProperty: 0.2, + } as GenerativeCohereConfig, + }); + }); + + it('should merge a reranker config with existing schema', () => { + const merged = MergeWithExisting.reranker(parse(moduleConfig), { + name: 'reranker-cohere', + config: { + model: 'other', + } as RerankerCohereConfig, + }); + expect(merged).toEqual({ + ...moduleConfig, + 'reranker-cohere': { + ...(moduleConfig['reranker-cohere'] as any), + model: 'other', + } as RerankerCohereConfig, + }); + }); }); diff --git a/src/collections/configure/index.ts b/src/collections/configure/index.ts index 9619fe11..7a1ca15c 100644 --- a/src/collections/configure/index.ts +++ b/src/collections/configure/index.ts @@ -261,6 +261,8 @@ const reconfigure = { autoTenantCreation: options.autoTenantCreation, }; }, + generative: configure.generative, + reranker: configure.reranker, }; export { From af87430e3fc4caee7975329a93442ce9cce54ec2 Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Fri, 10 Jan 2025 13:23:09 +0000 Subject: [PATCH 2/8] Skip test for <1.25.0 --- src/collections/config/integration.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/collections/config/integration.test.ts b/src/collections/config/integration.test.ts index 3585957c..254c6112 100644 --- a/src/collections/config/integration.test.ts +++ b/src/collections/config/integration.test.ts @@ -625,7 +625,11 @@ describe('Testing of the collection.config namespace', () => { expect(config.vectorizers.default.vectorizer.name).toEqual('none'); }); - it.only('should be able to update the generative & reranker configs of a collection', async () => { + it('should be able to update the generative & reranker configs of a collection', async () => { + if ((await client.getWeaviateVersion()).isLowerThan(1, 25, 0)) { + console.warn('Skipping test because Weaviate version is lower than 1.25.0'); + return; + } const collectionName = 'TestCollectionConfigUpdateGenerative'; const collection = client.collections.get(collectionName); await client.collections.create({ From 9d27f2f0b27f7136cd5840c66221989418dcdc0c Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 11:47:21 +0100 Subject: [PATCH 3/8] Rearrange `parse` method --- src/collections/config/unit.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/collections/config/unit.test.ts b/src/collections/config/unit.test.ts index ad27f61c..5006d20e 100644 --- a/src/collections/config/unit.test.ts +++ b/src/collections/config/unit.test.ts @@ -8,6 +8,8 @@ import { MergeWithExisting } from './classes'; import { GenerativeCohereConfig, RerankerCohereConfig } from './types'; describe('Unit testing of the MergeWithExisting class', () => { + const parse = (config: any) => JSON.parse(JSON.stringify(config)); + const invertedIndex: WeaviateInvertedIndexConfig = { bm25: { b: 0.8, @@ -64,7 +66,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }; it('should merge a full invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(JSON.parse(JSON.stringify(invertedIndex)), { + const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { b: 0.9, k1: 1.4, @@ -136,8 +138,6 @@ describe('Unit testing of the MergeWithExisting class', () => { 'reranker-cohere': {}, }; - const parse = (config: any) => JSON.parse(JSON.stringify(config)); - it('should merge a partial invertedIndexUpdate with existing schema', () => { const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { @@ -333,7 +333,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer Flat vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(flatVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(flatVectorConfig), [ { name: 'name', vectorIndex: { From 9c1ea29087c441f06b96fdea0291224f928948a8 Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 14:41:48 +0100 Subject: [PATCH 4/8] Make typing changes in response to review --- src/collections/aggregate/index.ts | 54 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/collections/aggregate/index.ts b/src/collections/aggregate/index.ts index 63a41496..e1fb9e62 100644 --- a/src/collections/aggregate/index.ts +++ b/src/collections/aggregate/index.ts @@ -10,33 +10,35 @@ import { Aggregator } from '../../graphql/index.js'; import { toBase64FromMedia } from '../../index.js'; import { Serialize } from '../serialize/index.js'; -export type AggregateBaseOptions = { +export type AggregateBaseOptions = { filters?: FilterValue; returnMetrics?: M; }; -export type AggregateGroupByOptions = AggregateOptions & { - groupBy: (keyof T & string) | GroupByAggregate; +export type PropertyOf = T extends undefined ? string : keyof T & string; + +export type AggregateGroupByOptions = AggregateOptions & { + groupBy: PropertyOf | GroupByAggregate; }; export type GroupByAggregate = { - property: keyof T & string; + property: PropertyOf; limit?: number; }; -export type AggregateOptions = AggregateBaseOptions; +export type AggregateOptions = AggregateBaseOptions; -export type AggregateBaseOverAllOptions = AggregateBaseOptions; +export type AggregateBaseOverAllOptions = AggregateBaseOptions; -export type AggregateNearOptions = AggregateBaseOptions & { +export type AggregateNearOptions = AggregateBaseOptions & { certainty?: number; distance?: number; objectLimit?: number; targetVector?: string; }; -export type AggregateGroupByNearOptions = AggregateNearOptions & { - groupBy: (keyof T & string) | GroupByAggregate; +export type AggregateGroupByNearOptions = AggregateNearOptions & { + groupBy: PropertyOf | GroupByAggregate; }; export type AggregateBoolean = { @@ -126,11 +128,11 @@ export type AggregateMetrics = { [K in keyof M]: M[K] extends true ? number : never; }; -export type MetricsProperty = T extends undefined ? string : keyof T & string; +export type MetricsProperty = PropertyOf; export const metrics = () => { return { - aggregate:

>(property: P) => new MetricsManager(property), + aggregate:

>(property: P) => new MetricsManager(property), }; }; @@ -143,10 +145,10 @@ export interface Metrics { See [the docs](https://weaviate.io/developers/weaviate/search/aggregate) for more details! */ - aggregate:

>(property: P) => MetricsManager; + aggregate:

>(property: P) => MetricsManager; } -export class MetricsManager> { +export class MetricsManager> { private propertyName: P; constructor(property: P) { @@ -419,11 +421,7 @@ class AggregateManager implements Aggregate { return new Aggregator(this.connection); } - base( - metrics?: PropertiesMetrics, - filters?: FilterValue, - groupBy?: (keyof T & string) | GroupByAggregate - ) { + base(metrics?: PropertiesMetrics, filters?: FilterValue, groupBy?: PropertyOf | GroupByAggregate) { let fields = 'meta { count }'; let builder = this.query().withClassName(this.name); if (metrics) { @@ -491,7 +489,7 @@ class AggregateManager implements Aggregate { async nearImage>( image: string | Buffer, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearImage({ image: await toBase64FromMedia(image), @@ -507,7 +505,7 @@ class AggregateManager implements Aggregate { nearObject>( id: string, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearObject({ id: id, @@ -523,7 +521,7 @@ class AggregateManager implements Aggregate { nearText>( query: string | string[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearText({ concepts: Array.isArray(query) ? query : [query], @@ -539,7 +537,7 @@ class AggregateManager implements Aggregate { nearVector>( vector: number[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearVector({ vector: vector, @@ -553,7 +551,7 @@ class AggregateManager implements Aggregate { return this.do(builder); } - overAll>(opts?: AggregateOptions): Promise> { + overAll>(opts?: AggregateOptions): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters); return this.do(builder); } @@ -615,7 +613,7 @@ export interface Aggregate { */ nearImage>( image: string | Buffer, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near object search on this collection. @@ -630,7 +628,7 @@ export interface Aggregate { */ nearObject>( id: string, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near vector search on this collection. @@ -645,7 +643,7 @@ export interface Aggregate { */ nearText>( query: string | string[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near vector search on this collection. @@ -660,7 +658,7 @@ export interface Aggregate { */ nearVector>( vector: number[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over all the objects in this collection without any vector search. @@ -668,7 +666,7 @@ export interface Aggregate { * @param {AggregateOptions} [opts] The options for the request. * @returns {Promise[]>} The aggregated metrics for the objects in the collection. */ - overAll>(opts?: AggregateOptions): Promise>; + overAll>(opts?: AggregateOptions): Promise>; } export interface AggregateGroupBy { From 866f99e32d959c2c557cd0ae87d36a07c784bbcb Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 14:43:21 +0100 Subject: [PATCH 5/8] Revert "Rearrange `parse` method" This reverts commit 9d27f2f0b27f7136cd5840c66221989418dcdc0c. --- src/collections/config/unit.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/collections/config/unit.test.ts b/src/collections/config/unit.test.ts index 5006d20e..ad27f61c 100644 --- a/src/collections/config/unit.test.ts +++ b/src/collections/config/unit.test.ts @@ -8,8 +8,6 @@ import { MergeWithExisting } from './classes'; import { GenerativeCohereConfig, RerankerCohereConfig } from './types'; describe('Unit testing of the MergeWithExisting class', () => { - const parse = (config: any) => JSON.parse(JSON.stringify(config)); - const invertedIndex: WeaviateInvertedIndexConfig = { bm25: { b: 0.8, @@ -66,7 +64,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }; it('should merge a full invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { + const merged = MergeWithExisting.invertedIndex(JSON.parse(JSON.stringify(invertedIndex)), { bm25: { b: 0.9, k1: 1.4, @@ -138,6 +136,8 @@ describe('Unit testing of the MergeWithExisting class', () => { 'reranker-cohere': {}, }; + const parse = (config: any) => JSON.parse(JSON.stringify(config)); + it('should merge a partial invertedIndexUpdate with existing schema', () => { const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { @@ -333,7 +333,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer Flat vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(flatVectorConfig), [ + const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(flatVectorConfig)), [ { name: 'name', vectorIndex: { From d930391d1c8c0d8aa5c9cc3b452b349d4851f2f4 Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 14:43:46 +0100 Subject: [PATCH 6/8] Revert "Make typing changes in response to review" This reverts commit 9c1ea29087c441f06b96fdea0291224f928948a8. --- src/collections/aggregate/index.ts | 54 ++++++++++++++++-------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/collections/aggregate/index.ts b/src/collections/aggregate/index.ts index e1fb9e62..63a41496 100644 --- a/src/collections/aggregate/index.ts +++ b/src/collections/aggregate/index.ts @@ -10,35 +10,33 @@ import { Aggregator } from '../../graphql/index.js'; import { toBase64FromMedia } from '../../index.js'; import { Serialize } from '../serialize/index.js'; -export type AggregateBaseOptions = { +export type AggregateBaseOptions = { filters?: FilterValue; returnMetrics?: M; }; -export type PropertyOf = T extends undefined ? string : keyof T & string; - -export type AggregateGroupByOptions = AggregateOptions & { - groupBy: PropertyOf | GroupByAggregate; +export type AggregateGroupByOptions = AggregateOptions & { + groupBy: (keyof T & string) | GroupByAggregate; }; export type GroupByAggregate = { - property: PropertyOf; + property: keyof T & string; limit?: number; }; -export type AggregateOptions = AggregateBaseOptions; +export type AggregateOptions = AggregateBaseOptions; -export type AggregateBaseOverAllOptions = AggregateBaseOptions; +export type AggregateBaseOverAllOptions = AggregateBaseOptions; -export type AggregateNearOptions = AggregateBaseOptions & { +export type AggregateNearOptions = AggregateBaseOptions & { certainty?: number; distance?: number; objectLimit?: number; targetVector?: string; }; -export type AggregateGroupByNearOptions = AggregateNearOptions & { - groupBy: PropertyOf | GroupByAggregate; +export type AggregateGroupByNearOptions = AggregateNearOptions & { + groupBy: (keyof T & string) | GroupByAggregate; }; export type AggregateBoolean = { @@ -128,11 +126,11 @@ export type AggregateMetrics = { [K in keyof M]: M[K] extends true ? number : never; }; -export type MetricsProperty = PropertyOf; +export type MetricsProperty = T extends undefined ? string : keyof T & string; export const metrics = () => { return { - aggregate:

>(property: P) => new MetricsManager(property), + aggregate:

>(property: P) => new MetricsManager(property), }; }; @@ -145,10 +143,10 @@ export interface Metrics { See [the docs](https://weaviate.io/developers/weaviate/search/aggregate) for more details! */ - aggregate:

>(property: P) => MetricsManager; + aggregate:

>(property: P) => MetricsManager; } -export class MetricsManager> { +export class MetricsManager> { private propertyName: P; constructor(property: P) { @@ -421,7 +419,11 @@ class AggregateManager implements Aggregate { return new Aggregator(this.connection); } - base(metrics?: PropertiesMetrics, filters?: FilterValue, groupBy?: PropertyOf | GroupByAggregate) { + base( + metrics?: PropertiesMetrics, + filters?: FilterValue, + groupBy?: (keyof T & string) | GroupByAggregate + ) { let fields = 'meta { count }'; let builder = this.query().withClassName(this.name); if (metrics) { @@ -489,7 +491,7 @@ class AggregateManager implements Aggregate { async nearImage>( image: string | Buffer, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearImage({ image: await toBase64FromMedia(image), @@ -505,7 +507,7 @@ class AggregateManager implements Aggregate { nearObject>( id: string, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearObject({ id: id, @@ -521,7 +523,7 @@ class AggregateManager implements Aggregate { nearText>( query: string | string[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearText({ concepts: Array.isArray(query) ? query : [query], @@ -537,7 +539,7 @@ class AggregateManager implements Aggregate { nearVector>( vector: number[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters).withNearVector({ vector: vector, @@ -551,7 +553,7 @@ class AggregateManager implements Aggregate { return this.do(builder); } - overAll>(opts?: AggregateOptions): Promise> { + overAll>(opts?: AggregateOptions): Promise> { const builder = this.base(opts?.returnMetrics, opts?.filters); return this.do(builder); } @@ -613,7 +615,7 @@ export interface Aggregate { */ nearImage>( image: string | Buffer, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near object search on this collection. @@ -628,7 +630,7 @@ export interface Aggregate { */ nearObject>( id: string, - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near vector search on this collection. @@ -643,7 +645,7 @@ export interface Aggregate { */ nearText>( query: string | string[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over the objects returned by a near vector search on this collection. @@ -658,7 +660,7 @@ export interface Aggregate { */ nearVector>( vector: number[], - opts?: AggregateNearOptions + opts?: AggregateNearOptions ): Promise>; /** * Aggregate metrics over all the objects in this collection without any vector search. @@ -666,7 +668,7 @@ export interface Aggregate { * @param {AggregateOptions} [opts] The options for the request. * @returns {Promise[]>} The aggregated metrics for the objects in the collection. */ - overAll>(opts?: AggregateOptions): Promise>; + overAll>(opts?: AggregateOptions): Promise>; } export interface AggregateGroupBy { From b3fcd16d2861993660b6f1742a0793fc8c2d6c0f Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 14:43:57 +0100 Subject: [PATCH 7/8] Revert "Revert "Rearrange `parse` method"" This reverts commit 866f99e32d959c2c557cd0ae87d36a07c784bbcb. --- src/collections/config/unit.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/collections/config/unit.test.ts b/src/collections/config/unit.test.ts index ad27f61c..5006d20e 100644 --- a/src/collections/config/unit.test.ts +++ b/src/collections/config/unit.test.ts @@ -8,6 +8,8 @@ import { MergeWithExisting } from './classes'; import { GenerativeCohereConfig, RerankerCohereConfig } from './types'; describe('Unit testing of the MergeWithExisting class', () => { + const parse = (config: any) => JSON.parse(JSON.stringify(config)); + const invertedIndex: WeaviateInvertedIndexConfig = { bm25: { b: 0.8, @@ -64,7 +66,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }; it('should merge a full invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(JSON.parse(JSON.stringify(invertedIndex)), { + const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { b: 0.9, k1: 1.4, @@ -136,8 +138,6 @@ describe('Unit testing of the MergeWithExisting class', () => { 'reranker-cohere': {}, }; - const parse = (config: any) => JSON.parse(JSON.stringify(config)); - it('should merge a partial invertedIndexUpdate with existing schema', () => { const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { bm25: { @@ -333,7 +333,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer Flat vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(JSON.parse(JSON.stringify(flatVectorConfig)), [ + const merged = MergeWithExisting.vectors(parse(flatVectorConfig), [ { name: 'name', vectorIndex: { From 892ec1265b37f75259a2f73625c73580e3e1a22d Mon Sep 17 00:00:00 2001 From: Tommy Smith Date: Mon, 13 Jan 2025 14:52:46 +0100 Subject: [PATCH 8/8] Make changes in response to review --- src/collections/config/unit.test.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/collections/config/unit.test.ts b/src/collections/config/unit.test.ts index 5006d20e..b8fc8596 100644 --- a/src/collections/config/unit.test.ts +++ b/src/collections/config/unit.test.ts @@ -8,7 +8,7 @@ import { MergeWithExisting } from './classes'; import { GenerativeCohereConfig, RerankerCohereConfig } from './types'; describe('Unit testing of the MergeWithExisting class', () => { - const parse = (config: any) => JSON.parse(JSON.stringify(config)); + const deepCopy = (config: any) => JSON.parse(JSON.stringify(config)); const invertedIndex: WeaviateInvertedIndexConfig = { bm25: { @@ -66,7 +66,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }; it('should merge a full invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { + const merged = MergeWithExisting.invertedIndex(deepCopy(invertedIndex), { bm25: { b: 0.9, k1: 1.4, @@ -139,7 +139,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }; it('should merge a partial invertedIndexUpdate with existing schema', () => { - const merged = MergeWithExisting.invertedIndex(parse(invertedIndex), { + const merged = MergeWithExisting.invertedIndex(deepCopy(invertedIndex), { bm25: { b: 0.9, }, @@ -163,7 +163,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a no quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ + const merged = MergeWithExisting.vectors(deepCopy(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -212,7 +212,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a PQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ + const merged = MergeWithExisting.vectors(deepCopy(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -261,7 +261,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ + const merged = MergeWithExisting.vectors(deepCopy(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -296,7 +296,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a SQ quantizer HNSW vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(hnswVectorConfig), [ + const merged = MergeWithExisting.vectors(deepCopy(hnswVectorConfig), [ { name: 'name', vectorIndex: { @@ -333,7 +333,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a BQ quantizer Flat vectorIndexConfig with existing schema', () => { - const merged = MergeWithExisting.vectors(parse(flatVectorConfig), [ + const merged = MergeWithExisting.vectors(deepCopy(flatVectorConfig), [ { name: 'name', vectorIndex: { @@ -369,7 +369,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge full multi tenancy config with existing schema', () => { - const merged = MergeWithExisting.multiTenancy(parse(multiTenancyConfig), { + const merged = MergeWithExisting.multiTenancy(deepCopy(multiTenancyConfig), { autoTenantActivation: true, autoTenantCreation: true, }); @@ -381,7 +381,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a generative config with existing schema', () => { - const merged = MergeWithExisting.generative(parse(moduleConfig), { + const merged = MergeWithExisting.generative(deepCopy(moduleConfig), { name: 'generative-cohere', config: { kProperty: 0.2, @@ -397,7 +397,7 @@ describe('Unit testing of the MergeWithExisting class', () => { }); it('should merge a reranker config with existing schema', () => { - const merged = MergeWithExisting.reranker(parse(moduleConfig), { + const merged = MergeWithExisting.reranker(deepCopy(moduleConfig), { name: 'reranker-cohere', config: { model: 'other',