mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-10 07:12:30 +00:00
fix(nodejs)!: key parsed embedding configs by vector column (#4003)
Two bugs in Node's reading of the embedding_functions schema metadata. First, parseFunctions keyed its result map by function name, so a table whose metadata configures the same function for two vector columns came back with only the last one. It now keys by the vector column, the convention Python's parser already uses. Second, Node could not read metadata written by the Python bindings at all, which spell the keys snake_case: configs parsed with both columns undefined, breaking embedding application on add() and leaving only query-side embedding working. The parse now accepts both spellings. Both fixes land in one shared parser used by every reader -- parseFunctions and the makeArrowTable schema validator, which had its own private camelCase-only parse -- so the wire contract cannot fork between entry points. A config naming no source or vector column is an error at the boundary rather than a default downstream, as are two configs claiming one column. The "vector" fallback remains only on the optional field of user-supplied configs. Breaking: parseFunctions is exported and its map keys change from function name to vector column. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5c3bc7f643
commit
7fd881bbe3
@@ -25,9 +25,12 @@
|
||||
### Type Aliases
|
||||
|
||||
- [CreateReturnType](type-aliases/CreateReturnType.md)
|
||||
- [EmbeddingMetadataEntry](type-aliases/EmbeddingMetadataEntry.md)
|
||||
- [ResolvedEmbeddingFunctionConfig](type-aliases/ResolvedEmbeddingFunctionConfig.md)
|
||||
|
||||
### Functions
|
||||
|
||||
- [LanceSchema](functions/LanceSchema.md)
|
||||
- [getRegistry](functions/getRegistry.md)
|
||||
- [parseEmbeddingMetadata](functions/parseEmbeddingMetadata.md)
|
||||
- [register](functions/register.md)
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
[**@lancedb/lancedb**](../../../README.md) • **Docs**
|
||||
|
||||
***
|
||||
|
||||
[@lancedb/lancedb](../../../globals.md) / [embedding](../README.md) / parseEmbeddingMetadata
|
||||
|
||||
# Function: parseEmbeddingMetadata()
|
||||
|
||||
```ts
|
||||
function parseEmbeddingMetadata(json): EmbeddingMetadataEntry[]
|
||||
```
|
||||
|
||||
The single parser for `embedding_functions` schema metadata: every reader
|
||||
goes through here, so the wire contract cannot fork between them.
|
||||
|
||||
## Parameters
|
||||
|
||||
* **json**: `string`
|
||||
|
||||
## Returns
|
||||
|
||||
[`EmbeddingMetadataEntry`](../type-aliases/EmbeddingMetadataEntry.md)[]
|
||||
@@ -0,0 +1,40 @@
|
||||
[**@lancedb/lancedb**](../../../README.md) • **Docs**
|
||||
|
||||
***
|
||||
|
||||
[@lancedb/lancedb](../../../globals.md) / [embedding](../README.md) / EmbeddingMetadataEntry
|
||||
|
||||
# Type Alias: EmbeddingMetadataEntry
|
||||
|
||||
```ts
|
||||
type EmbeddingMetadataEntry: object;
|
||||
```
|
||||
|
||||
One entry of the `embedding_functions` schema metadata, with the column
|
||||
keys normalized across the bindings' spellings.
|
||||
|
||||
## Type declaration
|
||||
|
||||
### model
|
||||
|
||||
```ts
|
||||
model: EmbeddingFunction["TOptions"];
|
||||
```
|
||||
|
||||
### name
|
||||
|
||||
```ts
|
||||
name: string;
|
||||
```
|
||||
|
||||
### sourceColumn
|
||||
|
||||
```ts
|
||||
sourceColumn: string;
|
||||
```
|
||||
|
||||
### vectorColumn
|
||||
|
||||
```ts
|
||||
vectorColumn: string;
|
||||
```
|
||||
@@ -0,0 +1,22 @@
|
||||
[**@lancedb/lancedb**](../../../README.md) • **Docs**
|
||||
|
||||
***
|
||||
|
||||
[@lancedb/lancedb](../../../globals.md) / [embedding](../README.md) / ResolvedEmbeddingFunctionConfig
|
||||
|
||||
# Type Alias: ResolvedEmbeddingFunctionConfig
|
||||
|
||||
```ts
|
||||
type ResolvedEmbeddingFunctionConfig: EmbeddingFunctionConfig & object;
|
||||
```
|
||||
|
||||
An [EmbeddingFunctionConfig] read back from table metadata, where the
|
||||
vector column is always recorded.
|
||||
|
||||
## Type declaration
|
||||
|
||||
### vectorColumn
|
||||
|
||||
```ts
|
||||
vectorColumn: string;
|
||||
```
|
||||
@@ -173,6 +173,36 @@ describe.each([arrow15, arrow16, arrow17, arrow18])(
|
||||
}
|
||||
|
||||
describe("The function makeArrowTable", function () {
|
||||
it("accepts snake_case embedding metadata like camelCase", function () {
|
||||
const spellings = [
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
{ source_column: "text", vector_column: "vector" },
|
||||
{ sourceColumn: "text", vectorColumn: "vector" },
|
||||
];
|
||||
for (const columns of spellings) {
|
||||
const schema = new Schema(
|
||||
[
|
||||
new Field("text", new Utf8(), false),
|
||||
new Field(
|
||||
"vector",
|
||||
new FixedSizeList(3, new Field("item", new Float32(), true)),
|
||||
false,
|
||||
),
|
||||
],
|
||||
new Map([
|
||||
[
|
||||
"embedding_functions",
|
||||
JSON.stringify([{ name: "mock", model: {}, ...columns }]),
|
||||
],
|
||||
]),
|
||||
);
|
||||
// The vector field is non-nullable and absent from the data; only a
|
||||
// recognized embedding config makes that acceptable.
|
||||
const table = makeArrowTable([{ text: "hello" }], { schema });
|
||||
expect(table.numRows).toBe(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("will use data types from a provided schema instead of inference", async function () {
|
||||
const schema = new Schema([
|
||||
new Field("a", new Int32(), false),
|
||||
|
||||
@@ -106,6 +106,77 @@ describe.each([arrow15, arrow16, arrow17, arrow18])("Registry", (arrow) => {
|
||||
'Embedding function with alias "mock-embedding" already exists',
|
||||
);
|
||||
});
|
||||
test("parseFunctions keeps entries sharing a function name", async () => {
|
||||
class MockEmbeddingFunction extends EmbeddingFunction<string> {
|
||||
ndims() {
|
||||
return 3;
|
||||
}
|
||||
embeddingDataType() {
|
||||
return new arrow.Float32() as apiArrow.Float;
|
||||
}
|
||||
async computeSourceEmbeddings(data: string[]) {
|
||||
return data.map(() => [1, 2, 3]);
|
||||
}
|
||||
}
|
||||
register("mock-embedding")(MockEmbeddingFunction);
|
||||
const parsed = await getRegistry().parseFunctions(
|
||||
new Map([
|
||||
[
|
||||
"embedding_functions",
|
||||
JSON.stringify([
|
||||
{
|
||||
name: "mock-embedding",
|
||||
sourceColumn: "text",
|
||||
vectorColumn: "vector_a",
|
||||
model: {},
|
||||
},
|
||||
{
|
||||
name: "mock-embedding",
|
||||
sourceColumn: "text",
|
||||
vectorColumn: "vector_b",
|
||||
model: {},
|
||||
},
|
||||
]),
|
||||
],
|
||||
]),
|
||||
);
|
||||
expect([...parsed.values()].map((f) => f.vectorColumn)).toEqual([
|
||||
"vector_a",
|
||||
"vector_b",
|
||||
]);
|
||||
|
||||
// The Python bindings write snake_case keys.
|
||||
const snake = await getRegistry().parseFunctions(
|
||||
new Map([
|
||||
[
|
||||
"embedding_functions",
|
||||
JSON.stringify([
|
||||
{
|
||||
name: "mock-embedding",
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
source_column: "text",
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
vector_column: "vector_a",
|
||||
model: {},
|
||||
},
|
||||
{
|
||||
name: "mock-embedding",
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
source_column: "text",
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
vector_column: "vector_b",
|
||||
model: {},
|
||||
},
|
||||
]),
|
||||
],
|
||||
]),
|
||||
);
|
||||
expect([...snake.keys()]).toEqual(["vector_a", "vector_b"]);
|
||||
expect([...snake.values()].map((f) => f.sourceColumn)).toEqual([
|
||||
"text",
|
||||
"text",
|
||||
]);
|
||||
});
|
||||
test("schema should contain correct metadata", async () => {
|
||||
class MockEmbeddingFunction extends EmbeddingFunction<string> {
|
||||
constructor(args: FunctionOptions = {}) {
|
||||
|
||||
@@ -48,7 +48,11 @@ import {
|
||||
} from "apache-arrow";
|
||||
import { Buffers } from "apache-arrow/data";
|
||||
import { type EmbeddingFunction } from "./embedding/embedding_function";
|
||||
import { EmbeddingFunctionConfig, getRegistry } from "./embedding/registry";
|
||||
import {
|
||||
EmbeddingFunctionConfig,
|
||||
getRegistry,
|
||||
parseEmbeddingMetadata,
|
||||
} from "./embedding/registry";
|
||||
import {
|
||||
sanitizeField,
|
||||
sanitizeSchema,
|
||||
@@ -933,7 +937,7 @@ async function applyEmbeddingsFromMetadata(
|
||||
|
||||
for (const functionEntry of functions.values()) {
|
||||
const sourceColumn = columns[functionEntry.sourceColumn];
|
||||
const destColumn = functionEntry.vectorColumn ?? "vector";
|
||||
const destColumn = functionEntry.vectorColumn;
|
||||
if (sourceColumn === undefined) {
|
||||
throw new Error(
|
||||
`Cannot apply embedding function because the source column '${functionEntry.sourceColumn}' was not present in the data`,
|
||||
@@ -1385,11 +1389,10 @@ function validateSchemaEmbeddings(
|
||||
|
||||
// Check schema metadata for embedding functions
|
||||
if (schema.metadata.has("embedding_functions")) {
|
||||
const embeddings = JSON.parse(
|
||||
const entries = parseEmbeddingMetadata(
|
||||
schema.metadata.get("embedding_functions")!,
|
||||
);
|
||||
// biome-ignore lint/suspicious/noExplicitAny: we don't know the type of `f`
|
||||
if (embeddings.find((f: any) => f["vectorColumn"] === field.name)) {
|
||||
if (entries.some((f) => f.vectorColumn === field.name)) {
|
||||
hasEmbeddingFunction = true;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -104,41 +104,29 @@ export class EmbeddingFunctionRegistry {
|
||||
async parseFunctions(
|
||||
this: EmbeddingFunctionRegistry,
|
||||
metadata: Map<string, string>,
|
||||
): Promise<Map<string, EmbeddingFunctionConfig>> {
|
||||
): Promise<Map<string, ResolvedEmbeddingFunctionConfig>> {
|
||||
if (!metadata.has("embedding_functions")) {
|
||||
return new Map();
|
||||
} else {
|
||||
type FunctionConfig = {
|
||||
name: string;
|
||||
sourceColumn: string;
|
||||
vectorColumn: string;
|
||||
model: EmbeddingFunction["TOptions"];
|
||||
};
|
||||
|
||||
const functions = <FunctionConfig[]>(
|
||||
JSON.parse(metadata.get("embedding_functions")!)
|
||||
);
|
||||
|
||||
const items: [string, EmbeddingFunctionConfig][] = await Promise.all(
|
||||
functions.map(async (f) => {
|
||||
const fn = this.get(f.name);
|
||||
if (!fn) {
|
||||
throw new Error(`Function "${f.name}" not found in registry`);
|
||||
}
|
||||
const func = await this.get(f.name)!.create(f.model);
|
||||
return [
|
||||
f.name,
|
||||
{
|
||||
sourceColumn: f.sourceColumn,
|
||||
vectorColumn: f.vectorColumn,
|
||||
function: func,
|
||||
},
|
||||
];
|
||||
}),
|
||||
);
|
||||
|
||||
return new Map(items);
|
||||
}
|
||||
const entries = parseEmbeddingMetadata(
|
||||
metadata.get("embedding_functions")!,
|
||||
);
|
||||
const items = await Promise.all(
|
||||
entries.map(async (f): Promise<ResolvedEmbeddingFunctionConfig> => {
|
||||
const fn = this.get(f.name);
|
||||
if (!fn) {
|
||||
throw new Error(`Function "${f.name}" not found in registry`);
|
||||
}
|
||||
const func = await fn.create(f.model);
|
||||
return {
|
||||
sourceColumn: f.sourceColumn,
|
||||
vectorColumn: f.vectorColumn,
|
||||
function: func,
|
||||
};
|
||||
}),
|
||||
);
|
||||
// Keyed by output column: one function may serve several columns.
|
||||
return new Map(items.map((config) => [config.vectorColumn, config]));
|
||||
}
|
||||
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
|
||||
functionToMetadata(conf: EmbeddingFunctionConfig): Record<string, any> {
|
||||
@@ -218,3 +206,52 @@ export interface EmbeddingFunctionConfig {
|
||||
vectorColumn?: string;
|
||||
function: EmbeddingFunction;
|
||||
}
|
||||
|
||||
/** An [EmbeddingFunctionConfig] read back from table metadata, where the
|
||||
* vector column is always recorded. */
|
||||
export type ResolvedEmbeddingFunctionConfig = EmbeddingFunctionConfig & {
|
||||
vectorColumn: string;
|
||||
};
|
||||
|
||||
/** One entry of the `embedding_functions` schema metadata, with the column
|
||||
* keys normalized across the bindings' spellings. */
|
||||
export type EmbeddingMetadataEntry = {
|
||||
name: string;
|
||||
sourceColumn: string;
|
||||
vectorColumn: string;
|
||||
model: EmbeddingFunction["TOptions"];
|
||||
};
|
||||
|
||||
/** The single parser for `embedding_functions` schema metadata: every reader
|
||||
* goes through here, so the wire contract cannot fork between them. */
|
||||
export function parseEmbeddingMetadata(json: string): EmbeddingMetadataEntry[] {
|
||||
// The wire format, honestly: the Python bindings write snake_case keys.
|
||||
type Raw = {
|
||||
name: string;
|
||||
sourceColumn?: string;
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
source_column?: string;
|
||||
vectorColumn?: string;
|
||||
// biome-ignore lint/style/useNamingConvention: the Python wire spelling
|
||||
vector_column?: string;
|
||||
model: EmbeddingFunction["TOptions"];
|
||||
};
|
||||
const entries = <Raw[]>JSON.parse(json);
|
||||
const seen = new Set<string>();
|
||||
return entries.map((f) => {
|
||||
const sourceColumn = f.sourceColumn ?? f.source_column;
|
||||
const vectorColumn = f.vectorColumn ?? f.vector_column;
|
||||
if (sourceColumn === undefined || vectorColumn === undefined) {
|
||||
throw new Error(
|
||||
`Embedding function "${f.name}" metadata names no source or vector column`,
|
||||
);
|
||||
}
|
||||
if (seen.has(vectorColumn)) {
|
||||
throw new Error(
|
||||
`Multiple embedding configs claim vector column "${vectorColumn}"`,
|
||||
);
|
||||
}
|
||||
seen.add(vectorColumn);
|
||||
return { name: f.name, sourceColumn, vectorColumn, model: f.model };
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user