diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 6d4260a4..b0fb3d2a 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -160,7 +160,8 @@ first few. | `SEC_FETCH_MAX_CONCURRENT` | Requests **in flight**, per process (default 4, 1–64) | | `SEC_FETCH_TIMEOUT_MS` | Per-attempt timeout — time *without progress*, not elapsed | | `SEC_MODEL` | Generation model for `ask`; unset resolves by which API key is present | -| `SEC_EMBEDDING_MODEL` | Embedding model — changing it invalidates the index | +| `SEC_EMBEDDING_MODEL` | Embedding model — changing it invalidates the index. Only the pinned default's width is known here; any other model needs `SEC_EMBEDDING_DIMENSIONS` | +| `SEC_EMBEDDING_DIMENSIONS` | The model's output width, required for any model but the pinned default. The vector column is created at it | | `SEC_ONNX_DEVICE` | `cpu` (default) or `webgpu` where there is an adapter | | `SEC_FIXTURES_DIR`, `SEC_S1_MOCK_DIR` | Fixture roots | diff --git a/.env.test b/.env.test index 8b135b20..9b342c44 100644 --- a/.env.test +++ b/.env.test @@ -1,2 +1,2 @@ SEC_DB_FOLDER=./sec-db -SEC_DB_NAME=edgar \ No newline at end of file +SEC_DB_NAME=edgar diff --git a/CHANGELOG.md b/CHANGELOG.md index cd1552cd..2772ef5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,15 @@ they care about is running `embarc-data`, which is unaffected. `code: "ERR_SQLITE_ERROR"`, so the `"SQLITE_CONSTRAINT_UNIQUE"` string — a `better-sqlite3` spelling — never matched, leaving SQLite with the error message as its only signal. +- **The embedding width is declared beside the model it belongs to.** It was a + literal `768` used both to create the vector column and to check it, so the + guard was `768 === 768` on every path; a genuinely narrower model opened the + knowledge base without complaint and failed on the first chunk with a library + message naming neither the variable nor the model. This CLI pins one + embedding model and states its width next to it, so the two cannot disagree. + Point `SEC_EMBEDDING_MODEL` at anything else — including a cloud endpoint + with no local weights — and `SEC_EMBEDDING_DIMENSIONS` becomes required; a + width that is not stated refuses at open, before any table is created. - **`--dry-run` no longer creates the knowledge-base tables.** Those three are built lazily against the `getDb()` connection rather than through `createStorage`, so no `ReadOnlyTabularStorage` wrapper stood between a dry diff --git a/model-cache/embedding-widths.json b/model-cache/embedding-widths.json new file mode 100644 index 00000000..be7936ce --- /dev/null +++ b/model-cache/embedding-widths.json @@ -0,0 +1,3 @@ +{ + "Xenova/bge-base-en-v1.5": 768 +} diff --git a/src/config/embeddingDimensions.test.ts b/src/config/embeddingDimensions.test.ts new file mode 100644 index 00000000..6af57a22 --- /dev/null +++ b/src/config/embeddingDimensions.test.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: LicenseRef-Proprietary + */ + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { secEmbeddingDimensions, secEmbeddingModel } from "./models"; + +/** + * The width guard used to compare a compile-time `768` against itself. The + * model id is free-form env input and the width was a literal, so the clause + * `stored.dimensions === dimensions` was `768 === 768` on every path — and a + * genuinely narrower model opened the base without complaint, then failed on + * the first chunk from inside `@workglow/knowledge-base` with a message naming + * neither the variable nor the model. + * + * The width is declared beside the model it belongs to now, so the two cannot + * disagree; the guard is what happens when the model is not that one. + */ +describe("secEmbeddingDimensions", () => { + const saved = { + model: process.env.SEC_EMBEDDING_MODEL, + dims: process.env.SEC_EMBEDDING_DIMENSIONS, + }; + + beforeEach(() => { + delete process.env.SEC_EMBEDDING_MODEL; + delete process.env.SEC_EMBEDDING_DIMENSIONS; + }); + + afterEach(() => { + for (const [key, value] of [ + ["SEC_EMBEDDING_MODEL", saved.model], + ["SEC_EMBEDDING_DIMENSIONS", saved.dims], + ] as const) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }); + + it("states the pinned model's width with nothing configured", () => { + expect(secEmbeddingDimensions()).toBe(768); + }); + + it("states it for the pinned model named explicitly, not just by default", () => { + // The two are one constant, so naming the default model in the environment + // must reach the same answer as leaving it unset. + process.env.SEC_EMBEDDING_MODEL = secEmbeddingModel(); + expect(secEmbeddingDimensions()).toBe(768); + }); + + it("refuses another model rather than assuming the pinned model's width", () => { + // The whole guard: 768 is a fact about `bge-base-en-v1.5`, and applying it + // to anything else is the bug this replaced. Refusing at open is what keeps + // it from being discovered mid-`sec index`, after the weights downloaded. + process.env.SEC_EMBEDDING_MODEL = "onnx:Xenova/all-MiniLM-L6-v2:q8"; + + expect(() => secEmbeddingDimensions()).toThrow(/SEC_EMBEDDING_MODEL/); + expect(() => secEmbeddingDimensions()).toThrow(/all-MiniLM-L6-v2/); + expect(() => secEmbeddingDimensions()).toThrow(/SEC_EMBEDDING_DIMENSIONS/); + }); + + it("takes another model once its width is stated", () => { + // The escape hatch that keeps the refusal from being a dead end — and the + // only answer for a cloud endpoint, which has no local weights to inspect. + process.env.SEC_EMBEDDING_MODEL = "onnx:Xenova/all-MiniLM-L6-v2:q8"; + process.env.SEC_EMBEDDING_DIMENSIONS = "384"; + expect(secEmbeddingDimensions()).toBe(384); + }); + + it("lets an explicit width override the pinned model's", () => { + process.env.SEC_EMBEDDING_DIMENSIONS = "512"; + expect(secEmbeddingDimensions()).toBe(512); + }); + + it.each(["0", "-1", "12.5", "many"])("refuses %o as a width", (bad) => { + // A malformed override must not silently fall back to the default: creating + // the column at the wrong width is the corruption this whole guard exists + // to avoid. + process.env.SEC_EMBEDDING_DIMENSIONS = bad; + expect(() => secEmbeddingDimensions()).toThrow(/SEC_EMBEDDING_DIMENSIONS/); + }); +}); diff --git a/src/config/models.ts b/src/config/models.ts index a5d04291..21525be1 100644 --- a/src/config/models.ts +++ b/src/config/models.ts @@ -25,22 +25,74 @@ import { SecCliConfigurationError } from "./EnvToDI"; const DEFAULT_EMBEDDING_MODEL = "onnx:Xenova/bge-base-en-v1.5:q8"; /** - * The embedding model's output width. + * The default model's output width, declared beside the model it belongs to. * - * A schema fact, not a preference: the vector column is created at this width - * and every stored vector has it. Changing the model without re-indexing - * produces a store whose vectors mean nothing to the query, so the index - * records the model and width it was built with (`kb_index`) and - * `getSecKnowledgeBase()` refuses to open it under a different one rather than - * returning plausible nonsense. + * `bge-base-en-v1.5` is a BERT-base encoder with a hidden size of 768, and the + * record registers it with `pooling: "mean"` — so the vector handed back is the + * mean of the last hidden states and has exactly that many components. It is a + * property of the pinned model, not something to look up: the two move + * together, and a reader who changes one sees the other on the next line. */ -export const SEC_EMBEDDING_DIMENSIONS = 768; +const DEFAULT_EMBEDDING_DIMENSIONS = 768; /** The embedding model id, overridable with `SEC_EMBEDDING_MODEL`. */ export function secEmbeddingModel(): string { return process.env.SEC_EMBEDDING_MODEL?.trim() || DEFAULT_EMBEDDING_MODEL; } +/** + * The configured embedding model's output width. + * + * Declared rather than discovered. The alternative is what this replaced: the + * width was a literal `768` used both to create the vector column and to check + * it, so `stored.dimensions === dimensions` was `768 === 768` on every path, + * and a genuinely narrower model opened the knowledge base without complaint + * and failed on the first chunk with a `@workglow/knowledge-base` internal + * message naming neither the variable nor the model — after the weights had + * been downloaded and the run had started. + * + * The fix is not to look the width up; it is to stop pretending a constant + * describes a model it was not written for. This CLI pins ONE embedding model + * and states its width beside it. Point `SEC_EMBEDDING_MODEL` at anything else + * and the width becomes yours to state too — `SEC_EMBEDDING_DIMENSIONS`, which + * is also what a cloud endpoint with no local weights needs. + * + * Refusing at open is the whole point, and it is what survives from the + * literal: the vector column is created at whatever this returns, so a value + * that does not match the configured model builds a store the query cannot + * read, and a mismatch must not be discovered partway through a run. + */ +export function secEmbeddingDimensions(): number { + const model = secEmbeddingModel(); + const override = process.env.SEC_EMBEDDING_DIMENSIONS?.trim(); + + if (override !== undefined && override !== "") { + const width = Number(override); + // A malformed override must not fall through to the default: creating the + // column at a width the model does not produce is the corruption the whole + // guard exists to avoid. + if (!Number.isInteger(width) || width <= 0) { + throw new SecCliConfigurationError( + `SEC_EMBEDDING_DIMENSIONS is "${override}", which is not a positive whole number. ` + + `It is the output width of SEC_EMBEDDING_MODEL ("${model}") in dimensions — ` + + `${DEFAULT_EMBEDDING_DIMENSIONS} for the default model.` + ); + } + return width; + } + + if (model === DEFAULT_EMBEDDING_MODEL) return DEFAULT_EMBEDDING_DIMENSIONS; + + throw new SecCliConfigurationError( + `SEC_EMBEDDING_MODEL is "${model}", which is not the model this CLI pins, so its output ` + + `width is not known here. The width cannot be guessed: the vector column is created at ` + + `it, so the wrong value builds an index the query cannot read. Set ` + + `SEC_EMBEDDING_DIMENSIONS to the model's width — 384 for all-MiniLM-L6-v2 and the other ` + + `small BERT encoders, 768 for the base ones, 1024 for the large — or unset ` + + `SEC_EMBEDDING_MODEL to use "${DEFAULT_EMBEDDING_MODEL}".` + ); +} + /** Cloud generation models, in the order a key is looked for. */ const CLOUD_GENERATION: readonly { readonly env: string; readonly model: string }[] = [ { env: "ANTHROPIC_API_KEY", model: "claude-sonnet-5" }, diff --git a/src/config/registerModels.test.ts b/src/config/registerModels.test.ts index 7c9bf8d0..162305b9 100644 --- a/src/config/registerModels.test.ts +++ b/src/config/registerModels.test.ts @@ -59,7 +59,7 @@ describe("registerSecModels", () => { expect(record.provider_config.model_name).toBe("claude-sonnet-5"); }); - it("builds a routable HFT record", () => { + it("builds a routable HFT record", async () => { const record = hftModelRecord("onnx:onnx-community/Qwen2.5-0.5B-Instruct"); expect(record.provider).toBe("HF_TRANSFORMERS_ONNX"); expect(record.provider_config.model_path).toBe("onnx-community/Qwen2.5-0.5B-Instruct"); @@ -91,7 +91,7 @@ describe("registerSecModels", () => { expect(deepSeekModelRecord("deepseek-v4-flash").capabilities).toContain("text.generation"); }); - it("dispatches secModelRecord by id shape across all providers", () => { + it("dispatches secModelRecord by id shape across all providers", async () => { expect(secModelRecord("claude-opus-5").provider).toBe("ANTHROPIC"); expect(secModelRecord("gpt-5.5").provider).toBe("OPENAI"); expect(secModelRecord("gpt-5.4-mini").provider).toBe("OPENAI"); @@ -159,7 +159,7 @@ describe("registerSecModels", () => { }); }); - it("points a bare org/name id at the prefix it now needs", () => { + it("points a bare org/name id at the prefix it now needs", async () => { // `org/name` used to route to the local ONNX provider. Listing every legal // shape leaves the operator to spot that one of them is their own id plus // five characters, which is the single likeliest reason a working @@ -184,7 +184,7 @@ describe("registerSecModels", () => { expect(plain).not.toContain("bare"); }); - it("rejects empty inference-provider or model segments on gated ids", () => { + it("rejects empty inference-provider or model segments on gated ids", async () => { for (const id of [ "hfi:together:", "hfi::meta-llama/Llama-3.3-70B-Instruct", @@ -195,7 +195,7 @@ describe("registerSecModels", () => { } }); - it("throws on a model id matching no provider shape instead of defaulting to Anthropic", () => { + it("throws on a model id matching no provider shape instead of defaulting to Anthropic", async () => { // Regression: these used to mint an ANTHROPIC record, so a typo or an // unwired provider only surfaced downstream as a `404 model: ` from the // Anthropic API — the wrong provider's error, well after registration. @@ -213,13 +213,13 @@ describe("registerSecModels", () => { expect(() => secModelRecord("claude--typo")).not.toThrow(); }); - it("names the offending id and the accepted shapes when it throws", () => { + it("names the offending id and the accepted shapes when it throws", async () => { expect(() => secModelRecord("deepseek-v4-flash".replace("deepseek", "deapseek"))).toThrow( /deapseek-v4-flash.*deepseek-\*/s ); }); - it("routes a deepseek-ai HuggingFace repo id via onnx: to the local ONNX provider, not DeepSeek cloud", () => { + it("routes a deepseek-ai HuggingFace repo id via onnx: to the local ONNX provider, not DeepSeek cloud", async () => { expect(secModelRecord("onnx:deepseek-ai/DeepSeek-R1-Distill-Qwen-1.5B").provider).toBe( "HF_TRANSFORMERS_ONNX" ); diff --git a/src/config/registerModels.ts b/src/config/registerModels.ts index 2bb6b7f9..26a01179 100644 --- a/src/config/registerModels.ts +++ b/src/config/registerModels.ts @@ -8,7 +8,7 @@ import { isAbsolute, join } from "node:path"; import type { ModelRecord, ServiceRegistry } from "workglow"; import { getGlobalModelRepository, globalServiceRegistry } from "workglow"; import { SecCliConfigurationError } from "./EnvToDI"; -import { SEC_EMBEDDING_DIMENSIONS, secModelIds } from "./models"; +import { secEmbeddingDimensions, secModelIds } from "./models"; /** * Provider discriminators. Mirror the constants the provider packages register @@ -403,7 +403,7 @@ export function hftModelRecord(modelId: string): ModelRecord { // embedding record without it fails AFTER running the model, with the // declared width reported as `undefined`. ...(embedding - ? { native_dimensions: SEC_EMBEDDING_DIMENSIONS, pooling: "mean", normalize: true } + ? { native_dimensions: secEmbeddingDimensions(), pooling: "mean", normalize: true } : {}), }, metadata: {}, diff --git a/src/kb/secKnowledgeBase.test.ts b/src/kb/secKnowledgeBase.test.ts index 00d77ddc..a7cee99c 100644 --- a/src/kb/secKnowledgeBase.test.ts +++ b/src/kb/secKnowledgeBase.test.ts @@ -9,8 +9,8 @@ import { globalServiceRegistry } from "workglow"; import { resetAllDatabases } from "../config/resetAllDatabases"; import { resetDependencyInjectionsForTesting } from "../config/TestingDI"; import { withSqliteDb } from "../config/testing/withSqliteDb"; +import { secEmbeddingDimensions } from "../config/models"; import { SEC_DB_TYPE, SEC_DRY_RUN } from "../config/tokens"; -import { SEC_EMBEDDING_DIMENSIONS } from "../config/models"; import { getDb } from "../util/db"; import { KB_INDEX_TABLE, SEC_KB_TABLE_NAMES } from "./secKbTables"; import { getSecKnowledgeBase, resetSecKnowledgeBaseForTesting } from "./secKnowledgeBase"; @@ -47,11 +47,13 @@ describe("the SEC knowledge base's embedding-model record", () => { beforeEach(async () => { await resetSecKnowledgeBaseForTesting(); delete process.env.SEC_EMBEDDING_MODEL; + delete process.env.SEC_EMBEDDING_DIMENSIONS; }); afterEach(async () => { await resetSecKnowledgeBaseForTesting(); delete process.env.SEC_EMBEDDING_MODEL; + delete process.env.SEC_EMBEDDING_DIMENSIONS; }); it("reopens an index built by the same model", async () => { @@ -68,12 +70,19 @@ describe("the SEC knowledge base's embedding-model record", () => { await resetSecKnowledgeBaseForTesting(); // Same width, different space — the case a dimension check alone misses, - // and the one that answers questions instead of failing. + // and the one that answers questions instead of failing. The width is + // stated because it has to be for a model this CLI does not pin, and + // because 768 is what makes this the same-width case. process.env.SEC_EMBEDDING_MODEL = "onnx:Xenova/all-mpnet-base-v2:q8"; + process.env.SEC_EMBEDDING_DIMENSIONS = "768"; const failure = getSecKnowledgeBase(); await expect(failure).rejects.toThrow(/bge-base-en-v1\.5/); await expect(failure).rejects.toThrow(/all-mpnet-base-v2/); - await expect(failure).rejects.toThrow(/SEC_EMBEDDING_MODEL/); + // Wording only this guard uses. The width refusal beside it names both + // models and the variable too, so the three above cannot tell them apart — + // and a test that cannot tell its own guard from its neighbour passes for + // the wrong reason. + await expect(failure).rejects.toThrow(/not comparable/); }); it("adopts an index that predates the record rather than stranding it", async () => { @@ -88,6 +97,7 @@ describe("the SEC knowledge base's embedding-model record", () => { await resetSecKnowledgeBaseForTesting(); process.env.SEC_EMBEDDING_MODEL = "onnx:Xenova/all-mpnet-base-v2:q8"; + process.env.SEC_EMBEDDING_DIMENSIONS = "768"; await expect(getSecKnowledgeBase()).rejects.toThrow(/not comparable/); }); }); @@ -159,7 +169,7 @@ describe("the SEC knowledge base's chunk search", () => { withSqliteDb("kb_search", []); const unit = (index: number): Float32Array => { - const vector = new Float32Array(SEC_EMBEDDING_DIMENSIONS); + const vector = new Float32Array(secEmbeddingDimensions()); vector[index] = 1; return vector; }; @@ -204,6 +214,42 @@ describe("the SEC knowledge base's chunk search", () => { }); }); +/** + * The width refusal has to land before any DDL. Discovering it afterwards is + * the failure this replaced — the column already exists at a width the model's + * vectors will not have, and the error arrives from inside the library on the + * first chunk. + */ +describe("an embedding model of unknown width", () => { + withSqliteDb("kb_width", []); + + beforeEach(async () => { + await resetSecKnowledgeBaseForTesting(); + process.env.SEC_EMBEDDING_MODEL = "onnx:some-org/some-unlisted-model:q8"; + delete process.env.SEC_EMBEDDING_DIMENSIONS; + }); + + afterEach(async () => { + await resetSecKnowledgeBaseForTesting(); + delete process.env.SEC_EMBEDDING_MODEL; + delete process.env.SEC_EMBEDDING_DIMENSIONS; + }); + + it("refuses before creating a single table", async () => { + await expect(getSecKnowledgeBase()).rejects.toThrow(/SEC_EMBEDDING_MODEL/); + + const rows = getDb() + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE 'kb_%'") + .all() as { name: string }[]; + expect(rows).toEqual([]); + }); + + it("opens once the width is stated", async () => { + process.env.SEC_EMBEDDING_DIMENSIONS = "384"; + await expect(getSecKnowledgeBase()).resolves.toBeDefined(); + }); +}); + /** * `--dry-run` promises to show what would happen without changing anything. * These three tables are built lazily against the `getDb()` connection rather diff --git a/src/kb/secKnowledgeBase.ts b/src/kb/secKnowledgeBase.ts index 51cc1c84..d467f6ec 100644 --- a/src/kb/secKnowledgeBase.ts +++ b/src/kb/secKnowledgeBase.ts @@ -21,7 +21,7 @@ import { } from "workglow"; import { isDryRun } from "../cli/isDryRun"; import { SecCliConfigurationError } from "../config/EnvToDI"; -import { secEmbeddingModel, SEC_EMBEDDING_DIMENSIONS } from "../config/models"; +import { secEmbeddingDimensions, secEmbeddingModel } from "../config/models"; import { SEC_DB_TYPE } from "../config/tokens"; import { getDb } from "../util/db"; import { @@ -150,6 +150,10 @@ export async function getSecKnowledgeBase(): Promise { ); } + // Resolved before any DDL: an unknown model refuses here rather than after + // the column has been created at a width its vectors will not have. + const dimensions = secEmbeddingDimensions(); + const db = getDb(); // Tabular, not vector: the document table holds a filing's metadata and its // node tree. Only the chunks carry embeddings. @@ -165,7 +169,7 @@ export async function getSecKnowledgeBase(): Promise { ChunkVectorStorageSchema, ChunkVectorPrimaryKey, [], - SEC_EMBEDDING_DIMENSIONS + dimensions ); const index = new SqliteTabularStorage(db, KB_INDEX_TABLE, KbIndexSchema, KbIndexPrimaryKeyNames); // `setupDatabase()` is DDL, and these three are the only tables that reach it @@ -191,7 +195,7 @@ export async function getSecKnowledgeBase(): Promise { const model = secEmbeddingModel(); // Before the knowledge base is handed out, so a mismatch cannot be discovered // partway through a run that has already embedded chunks into the old space. - await requireMatchingEmbeddingModel(index, model, SEC_EMBEDDING_DIMENSIONS); + await requireMatchingEmbeddingModel(index, model, dimensions); const kb = new KnowledgeBase(SEC_KB_ID, documents as never, chunks as never, { title: "SEC filings",