chroma-core / chroma

the AI-native open-source embedding database
https://www.trychroma.com/
Apache License 2.0
15.41k stars 1.29k forks source link

[Bug]: Type Error with QueryResponse - 'number[]' is not assignable to type 'number'. #660

Closed Russell-Pollari closed 1 year ago

Russell-Pollari commented 1 year ago

What happened?

Came across while trying to update the langchainjs integration here

    const result = await collection.query({
      queryEmbeddings: query,
      nResults: k,
      where: { ..._filter },
    });

    const { ids, distances, documents, metadatas } = result;
    if (!ids || !distances || !documents || !metadatas) {
      return [];
    }
    // get the result data from the first and only query vector
    const [firstIds] = ids;
    const [firstDistances] = distances;
    const [firstDocuments] = documents;
    const [firstMetadatas] = metadatas;

    const results: [Document, number][] = [];
    for (let i = 0; i < firstIds.length; i += 1) {
      results.push([
        new Document({
          pageContent: firstDocuments?.[i] ?? "",
          metadata: firstMetadatas?.[i] ?? {},
        }),
        firstDistances[i],
      ]);
    }
    return results;

pushing firstDistances[i] into results gives this error

Type 'number[]' is not assignable to type 'number'.ts(2322)

Looks like QueryResult has distances typed to number[][][], but langchain's expectation (and mine) is number[][]

Confirmed in chroma/clients/js/src/types.ts,

export type QueryResponse = {
  ids: IDs[];
  embeddings: null | Embeddings[][];
  documents: (null | Document)[][];
  metadatas: (null | Metadata)[][];
  distances: null | number[][][];
}

Isn't this incorrect? I think it should be:

 export type QueryResponse = {
   ids: IDs[];
-  embeddings: null | Embeddings[][];
+  embeddings: null | Embeddings[];
   documents: (null | Document)[][];
   metadatas: (null | Metadata)[][];
-  distances: null | number[][][];
+  distances: null | number[][];
 }

New to typescript so might be missing something

Versions

chromadb v1.5.1 node v18.16.0

Relevant log output

No response

jeffchuber commented 1 year ago

@Russell-Pollari I think you are right.

Embeddings is already an array of numbers. https://github.com/chroma-core/chroma/blob/main/clients/js/src/types.ts#L10

so we just need an array of that.

Distances I think should be number[][]; as you said as well.

Thanks @Russell-Pollari !

Russell-Pollari commented 1 year ago

fix incoming