mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
The ollama recipe declared a single `default_dims: 768` (nomic-embed-text's width) while serving models spanning 384..4096. Every non-nomic model resolved to 768, so `gbrain init --embedding-model ollama:bge-m3` built a 768-wide `content_chunks.embedding` column for a model that emits 1024. The schema looked fine and only failed at first insert with `expected 768 dimensions, not 1024`. Adds an optional `model_dims` map to `EmbeddingTouchpoint` and an `embeddingDimsForModel()` resolver that prefers the per-model entry and falls back to `default_dims`. The ollama recipe declares real widths for the models it lists; bge-m3 is added to that list. The three `init` call sites that read `default_dims` now resolve per model. Partial by design: unlisted models still fall back to `default_dims`, and `trust_custom_dims` keeps an explicit `--embedding-dimensions` override working. `user_provided_models` recipes (litellm, llama-server) still resolve to 0, so they continue to require explicit dimensions. Verified end to end against an OpenAI-compatible stub standing in for Ollama, using an isolated GBRAIN_HOME: before: config 768, content_chunks.embedding vector(768), insert fails after: config 1024, content_chunks.embedding vector(1024), insert succeeds
This commit is contained in:
+13
-4
@@ -337,7 +337,9 @@ async function resolveAIOptions(opts: ResolveAIOptionsArgs): Promise<ResolvedAIO
|
||||
process.exit(1);
|
||||
}
|
||||
out.embedding_model = `${shorthand}:${firstModel}`;
|
||||
out.embedding_dimensions = recipe.touchpoints.embedding!.default_dims;
|
||||
// #2051: width follows the model actually chosen, not the recipe default.
|
||||
const { embeddingDimsForModel } = await import('../core/ai/model-resolver.ts');
|
||||
out.embedding_dimensions = embeddingDimsForModel(recipe, firstModel);
|
||||
}
|
||||
|
||||
if (dimsArg !== null && !Number.isNaN(dimsArg) && dimsArg > 0) {
|
||||
@@ -361,8 +363,13 @@ async function resolveAIOptions(opts: ResolveAIOptionsArgs): Promise<ResolvedAIO
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
if (recipe?.touchpoints.embedding?.default_dims) {
|
||||
out.embedding_dimensions = recipe.touchpoints.embedding.default_dims;
|
||||
// #2051: resolve the width from the SPECIFIC model, not the recipe-wide
|
||||
// default. `--embedding-model ollama:bge-m3` must yield 1024, not Ollama's
|
||||
// nomic-shaped 768.
|
||||
if (recipe) {
|
||||
const { embeddingDimsForModel } = await import('../core/ai/model-resolver.ts');
|
||||
const dims = embeddingDimsForModel(recipe, out.embedding_model);
|
||||
if (dims > 0) out.embedding_dimensions = dims;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -525,9 +532,11 @@ async function resolveEmbeddingByEnv(out: ResolvedAIOptions, nonInteractive: boo
|
||||
// legacy OpenAI 1536), not the recipe's 2560.
|
||||
const { DEFAULT_EMBEDDING_MODEL, DEFAULT_EMBEDDING_DIMENSIONS } =
|
||||
await import('../core/ai/defaults.ts');
|
||||
const { embeddingDimsForModel } = await import('../core/ai/model-resolver.ts');
|
||||
// #2051: non-canonical models resolve per-model, not recipe-wide.
|
||||
const dims = fullModel === DEFAULT_EMBEDDING_MODEL
|
||||
? DEFAULT_EMBEDDING_DIMENSIONS
|
||||
: tp.default_dims;
|
||||
: embeddingDimsForModel(r, model);
|
||||
out.embedding_model = fullModel;
|
||||
out.embedding_dimensions = dims;
|
||||
console.error(
|
||||
|
||||
@@ -144,3 +144,35 @@ export function assertTouchpoint(
|
||||
export function knownProviderIds(): string[] {
|
||||
return [...RECIPES.keys()];
|
||||
}
|
||||
|
||||
/**
|
||||
* Native embedding width for `modelId` under `recipe`.
|
||||
*
|
||||
* Resolution: the recipe's `model_dims` entry for this model, else the
|
||||
* recipe-wide `default_dims`. Returns 0 when neither is known (the
|
||||
* user-provided-model recipes declare `default_dims: 0` to force an explicit
|
||||
* `--embedding-dimensions`), so callers keep their existing falsy checks.
|
||||
*
|
||||
* Accepts a bare model id (`bge-m3`) or a qualified one (`ollama:bge-m3`);
|
||||
* the provider prefix is stripped before lookup so call sites can pass
|
||||
* whichever they hold.
|
||||
*
|
||||
* Fixes #2051: a recipe-wide default silently picked 768 for every Ollama
|
||||
* model, so `init --embedding-model ollama:bge-m3` built a 768-wide column
|
||||
* for a model that emits 1024 and only failed at first insert.
|
||||
*/
|
||||
export function embeddingDimsForModel(
|
||||
recipe: Recipe,
|
||||
modelId: string | undefined,
|
||||
): number {
|
||||
const tp = recipe.touchpoints.embedding;
|
||||
if (!tp) return 0;
|
||||
if (!modelId) return tp.default_dims ?? 0;
|
||||
// Strip a leading `provider:` so both forms resolve. Slash-form ids
|
||||
// (openrouter nested) are left intact — they're the model id.
|
||||
const colon = modelId.indexOf(':');
|
||||
const bare = colon === -1 ? modelId : modelId.slice(colon + 1);
|
||||
const declared = tp.model_dims?.[bare];
|
||||
if (typeof declared === 'number' && declared > 0) return declared;
|
||||
return tp.default_dims ?? 0;
|
||||
}
|
||||
|
||||
@@ -14,17 +14,29 @@ export const ollama: Recipe = {
|
||||
touchpoints: {
|
||||
embedding: {
|
||||
// #2271: modern local embed models added so assertTouchpoint accepts them.
|
||||
// Each carries its own native dim (qwen3-embed-8b=4096, arctic-l-v2=1024);
|
||||
// the recipe-wide default_dims below is only the nomic fallback, so users
|
||||
// of the larger models pass --embedding-dimensions (allowed via
|
||||
// trust_custom_dims). Per-model dims metadata is a tracked follow-up.
|
||||
models: [
|
||||
'nomic-embed-text',
|
||||
'mxbai-embed-large',
|
||||
'all-minilm',
|
||||
'qwen3-embed-8b',
|
||||
'snowflake-arctic-embed-l-v2',
|
||||
'bge-m3',
|
||||
],
|
||||
// #2051: per-model native dims. Ollama serves models spanning 384..4096,
|
||||
// so the recipe-wide default_dims below is only correct for nomic. Without
|
||||
// this map `init --embedding-model ollama:bge-m3` built a 768-wide column
|
||||
// for a model that emits 1024, and the mismatch only surfaced at first
|
||||
// insert. Resolved via `embeddingDimsForModel()`; unlisted models still
|
||||
// fall back to default_dims, and trust_custom_dims keeps an explicit
|
||||
// --embedding-dimensions override working for models not named here.
|
||||
model_dims: {
|
||||
'nomic-embed-text': 768,
|
||||
'mxbai-embed-large': 1024,
|
||||
'all-minilm': 384,
|
||||
'qwen3-embed-8b': 4096,
|
||||
'snowflake-arctic-embed-l-v2': 1024,
|
||||
'bge-m3': 1024,
|
||||
},
|
||||
default_dims: 768, // nomic-embed-text native dim
|
||||
trust_custom_dims: true, // #2271: local models carry varied native dims
|
||||
cost_per_1m_tokens_usd: 0,
|
||||
|
||||
@@ -28,6 +28,21 @@ export type Implementation =
|
||||
export interface EmbeddingTouchpoint {
|
||||
models: string[];
|
||||
default_dims: number;
|
||||
/**
|
||||
* Per-model native dimensions, keyed by bare model id (no `provider:`
|
||||
* prefix). Consulted before `default_dims` when resolving schema width
|
||||
* for a specific model.
|
||||
*
|
||||
* Local recipes (ollama, llama-server) serve models with very different
|
||||
* native widths — nomic-embed-text is 768, bge-m3 and mxbai-embed-large
|
||||
* are 1024, qwen3-embed-8b is 4096. A single recipe-wide `default_dims`
|
||||
* silently picks the wrong width for every model except the one it was
|
||||
* chosen for, producing a schema that only fails at first insert (#2051).
|
||||
*
|
||||
* Partial by design: a model absent from this map falls back to
|
||||
* `default_dims`, so a recipe can declare only the models it knows.
|
||||
*/
|
||||
model_dims?: Readonly<Record<string, number>>;
|
||||
dims_options?: number[]; // for Matryoshka-aware providers
|
||||
cost_per_1m_tokens_usd?: number;
|
||||
price_last_verified?: string; // ISO date
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
/**
|
||||
* #2051 — per-model embedding dimensions for local recipes.
|
||||
*
|
||||
* Ollama serves models spanning 384..4096 dims, but the recipe declared a
|
||||
* single `default_dims: 768` (nomic-embed-text's width). Every other model
|
||||
* resolved to 768, so `gbrain init --embedding-model ollama:bge-m3` created a
|
||||
* 768-wide column for a model emitting 1024 and the mismatch only surfaced at
|
||||
* first insert.
|
||||
*
|
||||
* `embeddingDimsForModel()` consults the recipe's `model_dims` map first and
|
||||
* falls back to `default_dims`, so:
|
||||
* 1. Known models resolve to their true native width.
|
||||
* 2. Unlisted models still fall back (no regression for arbitrary pulls).
|
||||
* 3. `user_provided_models` recipes keep returning 0, which is what forces
|
||||
* an explicit `--embedding-dimensions`.
|
||||
*/
|
||||
|
||||
import { test, expect, describe } from 'bun:test';
|
||||
import { getRecipe } from '../src/core/ai/recipes/index.ts';
|
||||
import { embeddingDimsForModel } from '../src/core/ai/model-resolver.ts';
|
||||
|
||||
describe('embeddingDimsForModel — per-model dims (#2051)', () => {
|
||||
const ollama = getRecipe('ollama')!;
|
||||
|
||||
test('bge-m3 resolves to its native 1024, not the recipe default 768', () => {
|
||||
expect(embeddingDimsForModel(ollama, 'bge-m3')).toBe(1024);
|
||||
});
|
||||
|
||||
test('accepts a provider-qualified id', () => {
|
||||
expect(embeddingDimsForModel(ollama, 'ollama:bge-m3')).toBe(1024);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['nomic-embed-text', 768],
|
||||
['mxbai-embed-large', 1024],
|
||||
['all-minilm', 384],
|
||||
['qwen3-embed-8b', 4096],
|
||||
['snowflake-arctic-embed-l-v2', 1024],
|
||||
])('%s resolves to %i', (model, dims) => {
|
||||
expect(embeddingDimsForModel(ollama, model as string)).toBe(dims as number);
|
||||
});
|
||||
|
||||
test('every declared model_dims entry is also a listed model', () => {
|
||||
const listed = new Set(ollama.touchpoints.embedding!.models);
|
||||
for (const model of Object.keys(ollama.touchpoints.embedding!.model_dims ?? {})) {
|
||||
expect(listed.has(model)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test('an unlisted model falls back to the recipe default', () => {
|
||||
expect(embeddingDimsForModel(ollama, 'some-model-pulled-locally')).toBe(768);
|
||||
});
|
||||
|
||||
test('a missing model id falls back to the recipe default', () => {
|
||||
expect(embeddingDimsForModel(ollama, undefined)).toBe(768);
|
||||
});
|
||||
|
||||
test('llama-server still returns 0 so explicit dimensions stay required', () => {
|
||||
const llamaServer = getRecipe('llama-server')!;
|
||||
expect(embeddingDimsForModel(llamaServer, 'anything')).toBe(0);
|
||||
});
|
||||
|
||||
test('fixed-dim hosted recipes are unaffected', () => {
|
||||
const openai = getRecipe('openai')!;
|
||||
const tp = openai.touchpoints.embedding!;
|
||||
expect(embeddingDimsForModel(openai, tp.models[0])).toBe(tp.default_dims);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user