diff --git a/CHANGELOG.md b/CHANGELOG.md index a9e145189..fc423cf3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,115 @@ All notable changes to GBrain will be documented in this file. +## [0.28.11] - 2026-05-07 + +**Mix providers: OpenAI for text, Voyage for images. One brain, two embedding pipelines.** +**Multimodal model routing finally has its own knob.** + +v0.28.9 shipped multimodal image embeddings via Voyage, but the gateway hardcoded +`embedMultimodal()` to the brain's primary `embedding_model`. Brains using OpenAI +`text-embedding-3-large` (1536-dim) for text could not use Voyage `voyage-multimodal-3` +(1024-dim) for images without flipping the entire pipeline. The dual-column schema +already supported different dimensions per touchpoint; the routing was the missing piece. + +You can now set a separate model just for multimodal: + +```bash +gbrain config set embedding_multimodal true +gbrain config set embedding_multimodal_model voyage:voyage-multimodal-3 +export VOYAGE_API_KEY=... +gbrain sync +``` + +Text embeddings go to OpenAI (1536-dim, `embedding` column). Image embeddings go to +Voyage (1024-dim, `embedding_image` column). Same brain, side by side. + +When unset, `embedMultimodal()` falls back to `embedding_model` so existing +single-provider setups keep working unchanged. + +### Hardening from review + +The /codex outside-voice review on the original PR caught a real footgun the +recipe-level validation missed: Voyage shares `supports_multimodal: true` across +all 12 of its embedding models, but only `voyage-multimodal-3` accepts image input +at `/multimodalembeddings`. Pointing the new key at any other Voyage model would +have failed at the endpoint with HTTP 400, which the gateway misclassified as +transient and retried indefinitely. + +This release closes the gap with model-level validation. `EmbeddingTouchpoint` +gains an optional `multimodal_models?: string[]` allow-list. Voyage declares +`['voyage-multimodal-3']`. `embedMultimodal()` validates the resolved model +against the allow-list and throws `AIConfigError` with the supported model in +the fix hint, before any HTTP call. The 4xx-misclassified-as-transient bug is +filed as a separate TODO under v0.28.x. + +### Itemized changes + +#### Added +- `embedding_multimodal_model` config key (env: `GBRAIN_EMBEDDING_MULTIMODAL_MODEL`, + DB: `gbrain config set embedding_multimodal_model :`). Standard + env > file > DB > undefined precedence via `loadConfigWithEngine` (#719). +- `EmbeddingTouchpoint.multimodal_models?: string[]` model-level allow-list for + recipes that mix text-only and multimodal models under the same touchpoint + (#719, hardening from /codex review). +- `getMultimodalModel()` gateway accessor mirroring `getEmbeddingModel` / + `getChatModel` (#719). + +#### Changed +- `connectEngine()` in `src/cli.ts` now extracts a file-local + `buildGatewayConfig(c: GBrainConfig)` helper. Both pre-connect and + post-DB-merge `configureGateway` call sites pass through the helper, so adding + a new gateway-relevant config field touches one place (#719). +- The post-DB-merge `configureGateway` call is no longer gated on a specific + field name. Re-config now always fires when `loadConfigWithEngine` returns + non-null. Removes temporal coupling between the trigger and the field set so + future DB-mutable gateway fields auto-flow without remembering to update the + gate (#719). +- `embedMultimodal()` enforces model-level multimodal capability when the + recipe declares `multimodal_models`. Recipe-level `supports_multimodal` stays + as a fast-fail for non-multimodal providers (Anthropic, OpenAI today) (#719). + +#### Tests +- 4 new cases in `test/loadConfig-merge.test.ts` for env > file > DB precedence + on `embedding_multimodal_model`. +- 4 new cases in `test/voyage-multimodal.test.ts` for model preference + (multimodal_model over embedding_model), fallback regression, AIConfigError on + cross-recipe non-multimodal pointer, AIConfigError on Voyage text-only model + (the /codex F1 model-level validation gap). +- New `test/cli-multimodal-integration.test.ts` (3 cases) covering the cli.ts + re-config glue itself via PGLite. Closes the "mechanical glue" gap that unit + tests of the surrounding APIs leave behind. + +#### Documentation +- 3 new TODOs filed: `gbrain doctor` warns for misconfigured multimodal + setups; reclassify Voyage HTTP 4xx as `AIConfigError` (closes the + retry-storm-on-config-bug class); `gbrain config unset ` so users can + clear DB-set keys without direct SQL. + +## To take advantage of v0.28.11 + +`gbrain upgrade` should do this automatically. To use the new routing on an +existing brain: + +1. **Configure the multimodal model:** + ```bash + gbrain config set embedding_multimodal true + gbrain config set embedding_multimodal_model voyage:voyage-multimodal-3 + export VOYAGE_API_KEY=$(... your key ...) + ``` +2. **Verify the gateway sees it:** + ```bash + gbrain config get embedding_multimodal_model + ``` +3. **Drop an image into your brain repo and sync:** + ```bash + gbrain sync + ``` + The image lands in `content_chunks.embedding_image` (1024-dim) via Voyage; + text content keeps using your primary `embedding_model`. +4. **If anything misconfigures,** the gateway throws `AIConfigError` with a + clear fix hint before any HTTP call. No silent retry storms. + ## [0.28.10] - 2026-05-07 **`/health` stops being slow. Stops triggering restart cascades.** diff --git a/CLAUDE.md b/CLAUDE.md index 51cec7531..51005e886 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,9 +84,9 @@ strict behavior when unset. - `docs/eval-capture.md` (v0.25.0) — stable NDJSON schema reference for gbrain-evals consumers. - `test/public-exports.test.ts` (v0.25.0 / R2) — runtime contract test. Imports each of the 17 public subpaths via package name and pins a canary symbol per module. Paired with `scripts/check-exports-count.sh`. - `src/core/embedding.ts` — OpenAI text-embedding-3-large, batch, retry, backoff. **v0.28.7:** `BATCH_SIZE` reverted 50→100 — the original Voyage safety guard halved OpenAI throughput on every page. Per-recipe pre-split + recursive halving + adaptive shrink-on-miss now live in the gateway, so the outer paginator goes back to its original purpose: progress-callback granularity, not batch protection. -- `src/core/ai/types.ts` — provider/recipe types. **v0.28.7 (#680):** `EmbeddingTouchpoint` extended with optional `chars_per_token` (default 4 chars/token, matching OpenAI tiktoken on English) and `safety_factor` (default 0.8, budget-utilization ceiling). Both consulted only when `max_batch_tokens` is also set. Voyage declares `chars_per_token=1` + `safety_factor=0.5` to handle dense payloads (CJK/JSON/base64) that overshoot tiktoken. The pre-split budget is `max_batch_tokens × safety_factor / chars_per_token`. -- `src/core/ai/gateway.ts` — unified seam for every AI call. **v0.28.7 (#680):** module-scoped `_embedTransport` defaulting to AI SDK `embedMany`, with `__setEmbedTransportForTests(fn)` test seam so tests drive the public `embed()` function with a stubbed transport instead of probing private helpers. `splitByTokenBudget` and `isTokenLimitError` are now exported `@internal` — pure functions reused directly by the test file. Module-level `_shrinkState: Map` halves the recipe's effective `safety_factor` on token-limit miss (floor 0.05) and heals back ×1.5 toward the ceiling after `SHRINK_HEAL_AFTER=10` consecutive successes. `configureGateway()` walks every registered recipe at construction time and emits a once-per-process stderr warning for any embedding touchpoint missing `max_batch_tokens` (excluding the canonical OpenAI fast-path recipe). `resetGateway()` clears `_shrinkState`, the warned-set, and restores the real transport. ASCII flow diagram embedded in the `embed()` JSDoc covers the routing decision, recursion + halving, and shrinkState lifecycle. -- `src/core/ai/recipes/voyage.ts` — Voyage AI openai-compatible recipe. **v0.28.7 (#680):** declares `chars_per_token=1` + `safety_factor=0.5` so the gateway pre-splits Voyage batches at a 60K-character budget (50% of 120K-token cap with the dense-tokenizer ratio). Closes the v0.27 backfill loop where ~26% of the corpus stayed un-embedded because tiktoken-grounded budgeting silently undercounted Voyage's actual token usage. +- `src/core/ai/types.ts` — provider/recipe types. **v0.28.7 (#680):** `EmbeddingTouchpoint` extended with optional `chars_per_token` (default 4 chars/token, matching OpenAI tiktoken on English) and `safety_factor` (default 0.8, budget-utilization ceiling). Both consulted only when `max_batch_tokens` is also set. Voyage declares `chars_per_token=1` + `safety_factor=0.5` to handle dense payloads (CJK/JSON/base64) that overshoot tiktoken. The pre-split budget is `max_batch_tokens × safety_factor / chars_per_token`. **v0.28.11 (#719):** `EmbeddingTouchpoint.multimodal_models?: string[]` model-level allow-list for recipes that mix text-only + multimodal models under one touchpoint (Voyage's 12 models share `supports_multimodal: true` but only `voyage-multimodal-3` accepts `/multimodalembeddings`). When omitted, recipe-level `supports_multimodal` is sufficient. `AIGatewayConfig.embedding_multimodal_model?: string` lets `embedMultimodal()` route to a different model than `embedding_model` — brains using OpenAI for text can use Voyage for images without flipping the primary embedding pipeline. +- `src/core/ai/gateway.ts` — unified seam for every AI call. **v0.28.7 (#680):** module-scoped `_embedTransport` defaulting to AI SDK `embedMany`, with `__setEmbedTransportForTests(fn)` test seam so tests drive the public `embed()` function with a stubbed transport instead of probing private helpers. `splitByTokenBudget` and `isTokenLimitError` are now exported `@internal` — pure functions reused directly by the test file. Module-level `_shrinkState: Map` halves the recipe's effective `safety_factor` on token-limit miss (floor 0.05) and heals back ×1.5 toward the ceiling after `SHRINK_HEAL_AFTER=10` consecutive successes. `configureGateway()` walks every registered recipe at construction time and emits a once-per-process stderr warning for any embedding touchpoint missing `max_batch_tokens` (excluding the canonical OpenAI fast-path recipe). `resetGateway()` clears `_shrinkState`, the warned-set, and restores the real transport. ASCII flow diagram embedded in the `embed()` JSDoc covers the routing decision, recursion + halving, and shrinkState lifecycle. **v0.28.11 (#719):** `embedMultimodal()` reads `cfg.embedding_multimodal_model` first (falls back to `cfg.embedding_model` for single-model setups). After the existing recipe-level `supports_multimodal` fast-fail, validates the resolved model against `touchpoint.multimodal_models` when declared — closes the Voyage-text-only-model-into-multimodal-endpoint footgun before any HTTP call (Codex F1 from PR review). New `getMultimodalModel()` accessor mirrors `getEmbeddingModel` / `getChatModel` so doctor and integration tests can read the gateway state. +- `src/core/ai/recipes/voyage.ts` — Voyage AI openai-compatible recipe. **v0.28.7 (#680):** declares `chars_per_token=1` + `safety_factor=0.5` so the gateway pre-splits Voyage batches at a 60K-character budget (50% of 120K-token cap with the dense-tokenizer ratio). Closes the v0.27 backfill loop where ~26% of the corpus stayed un-embedded because tiktoken-grounded budgeting silently undercounted Voyage's actual token usage. **v0.28.11 (#719):** declares `multimodal_models: ['voyage-multimodal-3']` so the gateway rejects text-only Voyage models pointed at the multimodal endpoint with a clear `AIConfigError` instead of waiting for Voyage's HTTP 400. - `src/core/check-resolvable.ts` — Resolver validation: reachability, MECE overlap, DRY checks, structured fix objects. v0.14.1: `CROSS_CUTTING_PATTERNS.conventions` is an array (notability gate accepts both `conventions/quality.md` and `_brain-filing-rules.md`). New `extractDelegationTargets()` parses `> **Convention:**`, `> **Filing rule:**`, and inline backtick references. DRY suppression is proximity-based via `DRY_PROXIMITY_LINES = 40`. - `src/core/repo-root.ts` — Shared `findRepoRoot(startDir?)` (v0.16.4): walks up from `startDir` (default `process.cwd()`) looking for `skills/RESOLVER.md`. Zero-dependency module imported by both `doctor.ts` and `check-resolvable.ts`. Parameterized `startDir` makes tests hermetic. - `src/commands/check-resolvable.ts` — Standalone CLI wrapper (v0.16.4) over `checkResolvable()`. Exports `parseFlags`, `resolveSkillsDir`, `DEFERRED`, `runCheckResolvable`. Exit rule: **1 on any issue (warnings OR errors)**, stricter than doctor's `ok` flag — honors README:259. Stable JSON envelope `{ok, skillsDir, report, autoFix, deferred, error, message}` — same shape on success and error paths. `--fix` path runs `autoFixDryViolations` BEFORE `checkResolvable` (same ordering as doctor). `scripts/skillify-check.ts` subprocess-calls `gbrain check-resolvable --json` (cached per process) and fails loud on binary-missing — no silent false-pass. **v0.19:** AGENTS.md workspaces now resolve natively (see `src/core/resolver-filenames.ts`) — gbrain inspects the 107-skill OpenClaw deployment whether the routing file is `RESOLVER.md` or `AGENTS.md`. `DEFERRED[]` is empty — Checks 5 + 6 shipped as real code, not issue URLs. diff --git a/TODOS.md b/TODOS.md index 2e1035cc4..2f6dfa320 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,5 +1,43 @@ # TODOS +## multimodal embedding follow-ups (v0.28.11 / PR #719) + +### `gbrain doctor`: warn on misconfigured multimodal model +**Priority:** P2 + +**What:** Add two checks in `src/commands/doctor.ts`. (1) When `embedding_multimodal_model` is set, verify the recipe's required API key is present in the env. (2) When `embedding_multimodal: true` is set but no `embedding_multimodal_model` AND the primary `embedding_model` recipe doesn't declare `supports_multimodal`, surface that gap. + +**Why:** Today these misconfigurations surface only on first image ingest, after the user has already pushed image content into the brain. Doctor catching them at install/upgrade time saves a round of confusion. + +**Pros:** Both checks are read-only and cheap (one env probe + one recipe lookup). Same pattern as existing doctor checks. Surfaces problems before they ship. +**Cons:** Doctor's check list grows; needs a `--fast` opt-out path if added to the default scan. ~40 lines. +**Context:** PR #719 added the multimodal_model routing key. The recipe-level + model-level validation in `embedMultimodal()` already throws clear errors at runtime, but only when image content hits the gateway. v0.28.x candidate. +**Depends on:** None. + +### Reclassify Voyage HTTP 4xx as `AIConfigError` (Codex F2 from PR #719 review) +**Priority:** P2 + +**What:** `src/core/ai/gateway.ts:626` currently throws `AITransientError` for any non-401/403 4xx response from Voyage's /multimodalembeddings endpoint. Replace with a 4xx-non-429 → `AIConfigError` branch matching `normalizeAIError`'s contract at `src/core/ai/errors.ts:54`. + +**Why:** A config bug (malformed body, unsupported field, model the caller forgot to add to `multimodal_models`) currently presents to the caller as transient and triggers retry storms. PR #719's Change 3 closes the specific wrong-multimodal-model case locally via the `multimodal_models` allow-list, but other 4xx reasons still misclassify. + +**Pros:** Aligns the embedMultimodal error classifier with `normalizeAIError`. Eliminates retry-on-permanent-bug behavior. ~10 lines + 1 test. +**Cons:** Changes runtime error class for some failures; existing callers that catch `AITransientError` for these codes now must catch `AIConfigError`. Search before merging. +**Context:** Pre-existing in v0.27.1; surfaced because PR #719's new key makes the misclass more reachable. v0.28.x candidate. +**Depends on:** None. + +### `gbrain config unset ` subcommand (Codex F6 from PR #719 review) +**Priority:** P3 + +**What:** Add `unset` action alongside `show|get|set` in `src/commands/config.ts`. Calls `engine.setConfig(key, '')` (loadConfigWithEngine treats empty string as undefined) so a user who set a key by mistake can clear it. Empty-string write is the minimum-diff implementation; a real DELETE would be cleaner if the engine grows one. + +**Why:** Once a user runs `gbrain config set X val`, there's no normal CLI path to clear it. Empty string is rejected by the current `set` validator (`action === 'set' && key && value` where value is truthy). PR #719 added another DB-merge key (`embedding_multimodal_model`) and surfaces this UX gap. + +**Pros:** Closes a pre-existing UX hole that applies to every DB-merge key (`embedding_multimodal`, `embedding_image_ocr*`, now `embedding_multimodal_model`). Trivial implementation, ~15 lines. +**Cons:** Need to decide whether `unset` is a real DELETE (cleaner) or empty-string write (simpler). +**Context:** Pre-existing in v0.27.x. Worth doing alongside the doctor checks above so users have a working escape hatch. +**Depends on:** None. + ## cross-modal-eval (v0.27.x follow-ups from PR #674 plan) ### `--budget-usd` hard cap + per-call cost telemetry (T11=B follow-up) diff --git a/VERSION b/VERSION index dc544e06e..d92e28158 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.28.10 +0.28.11 diff --git a/package.json b/package.json index a599f9b53..c1f12c0f8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.28.10", + "version": "0.28.11", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/src/cli.ts b/src/cli.ts index 62d61d748..2b52ce7eb 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,6 +5,8 @@ installSigchldHandler(); import { readFileSync } from 'fs'; import { loadConfig, loadConfigWithEngine, toEngineConfig } from './core/config.ts'; +import type { GBrainConfig } from './core/config.ts'; +import type { AIGatewayConfig } from './core/ai/types.ts'; import type { BrainEngine } from './core/engine.ts'; import { operations, OperationError } from './core/operations.ts'; import type { Operation, OperationContext } from './core/operations.ts'; @@ -702,6 +704,24 @@ async function handleCliOnly(command: string, args: string[]) { } } +// Build the AIGatewayConfig payload from a GBrainConfig. File-local; not +// exported. Both configureGateway sites in connectEngine() pass through this +// helper so adding a new field touches one place. Adding a field to one site +// but not the other previously required remembering to mirror the change; +// the helper makes that structural. +function buildGatewayConfig(c: GBrainConfig): AIGatewayConfig { + return { + embedding_model: c.embedding_model, + embedding_dimensions: c.embedding_dimensions, + embedding_multimodal_model: c.embedding_multimodal_model, + expansion_model: c.expansion_model, + chat_model: c.chat_model, + chat_fallback_chain: c.chat_fallback_chain, + base_urls: c.provider_base_urls, + env: { ...process.env }, + }; +} + async function connectEngine(): Promise { const config = loadConfig(); if (!config) { @@ -712,15 +732,7 @@ async function connectEngine(): Promise { // Configure the AI gateway BEFORE engine connect — initSchema needs embedding dims. // Env is read once here; the gateway never reads process.env at call time (Codex C3). const { configureGateway } = await import('./core/ai/gateway.ts'); - configureGateway({ - embedding_model: config.embedding_model, - embedding_dimensions: config.embedding_dimensions, - expansion_model: config.expansion_model, - chat_model: config.chat_model, - chat_fallback_chain: config.chat_fallback_chain, - base_urls: config.provider_base_urls, - env: { ...process.env }, - }); + configureGateway(buildGatewayConfig(config)); const { createEngine } = await import('./core/engine-factory.ts'); const engine = await createEngine(toEngineConfig(config)); @@ -755,11 +767,10 @@ async function connectEngine(): Promise { try { const merged = await loadConfigWithEngine(engine, config); if (merged) { - // Only re-configure when a runtime-relevant DB flag actually overrode - // a pre-connect default. Today the only consumer is the import-image - // path; the gateway itself doesn't read these flags. The stash on - // process.env preserves the contract for downstream readers without - // changing the gateway's signature. + // Stash gate flags on process.env for downstream readers (import-file.ts + // dispatches on GBRAIN_EMBEDDING_MULTIMODAL, OCR consumer reads + // GBRAIN_EMBEDDING_IMAGE_OCR_*). The gateway itself doesn't read these + // flags; this preserves the contract without changing the gateway shape. if (merged.embedding_multimodal !== undefined) { process.env.GBRAIN_EMBEDDING_MULTIMODAL = String(merged.embedding_multimodal); } @@ -769,6 +780,12 @@ async function connectEngine(): Promise { if (merged.embedding_image_ocr_model !== undefined) { process.env.GBRAIN_EMBEDDING_IMAGE_OCR_MODEL = merged.embedding_image_ocr_model; } + // Always re-configure with merged values when DB merge succeeded. The + // trigger used to be field-name-gated (only when embedding_multimodal_model + // was set); that coupled the gate to the field set and would silently + // miss future DB-mutable gateway fields. One extra cache+shrinkState + // clear per startup is microseconds, no hot path. + configureGateway(buildGatewayConfig(merged)); } } catch { // Non-fatal. Pre-v39 brains may not have a usable config table yet. diff --git a/src/core/ai/gateway.ts b/src/core/ai/gateway.ts index 131539535..0db796f66 100644 --- a/src/core/ai/gateway.ts +++ b/src/core/ai/gateway.ts @@ -86,6 +86,7 @@ export function configureGateway(config: AIGatewayConfig): void { _config = { embedding_model: config.embedding_model ?? DEFAULT_EMBEDDING_MODEL, embedding_dimensions: config.embedding_dimensions ?? DEFAULT_EMBEDDING_DIMENSIONS, + embedding_multimodal_model: config.embedding_multimodal_model, expansion_model: config.expansion_model ?? DEFAULT_EXPANSION_MODEL, chat_model: config.chat_model ?? DEFAULT_CHAT_MODEL, chat_fallback_chain: config.chat_fallback_chain, @@ -174,6 +175,16 @@ export function getEmbeddingDimensions(): number { return requireConfig().embedding_dimensions ?? DEFAULT_EMBEDDING_DIMENSIONS; } +/** + * v0.28.11: returns the configured multimodal embedding model when set, + * or undefined if the brain falls back to `embedding_model` for multimodal + * routing. Mirrors the other gateway accessors so doctor/tests can read the + * gateway state without poking at private `_config`. + */ +export function getMultimodalModel(): string | undefined { + return requireConfig().embedding_multimodal_model; +} + export function getExpansionModel(): string { return requireConfig().expansion_model ?? DEFAULT_EXPANSION_MODEL; } @@ -540,14 +551,30 @@ export async function embedMultimodal(inputs: MultimodalInput[]): Promise `${recipe.id}:${m}`).join(', ')}.`, ); } diff --git a/src/core/ai/recipes/voyage.ts b/src/core/ai/recipes/voyage.ts index 606b2b9e3..202ea783c 100644 --- a/src/core/ai/recipes/voyage.ts +++ b/src/core/ai/recipes/voyage.ts @@ -42,6 +42,14 @@ export const voyage: Recipe = { chars_per_token: 1, safety_factor: 0.5, supports_multimodal: true, + // v0.28.11: only voyage-multimodal-3 is valid at /multimodalembeddings. + // The 11 text-only Voyage models above share supports_multimodal: true + // at the recipe level (Codex F1 from PR #719 review). Without this + // explicit list, embedMultimodal() would let `voyage:voyage-3-large` + // through local validation and Voyage would reject it with HTTP 400 — + // which gateway.ts:626 misclassifies as transient (TODO: reclassify + // 4xx). + multimodal_models: ['voyage-multimodal-3'], }, }, setup_hint: 'Get an API key at https://dash.voyageai.com/api-keys, then `export VOYAGE_API_KEY=...`', diff --git a/src/core/ai/types.ts b/src/core/ai/types.ts index 740643951..83cce8f40 100644 --- a/src/core/ai/types.ts +++ b/src/core/ai/types.ts @@ -60,6 +60,18 @@ export interface EmbeddingTouchpoint { * Text-only providers leave this undefined. */ supports_multimodal?: boolean; + /** + * v0.28.11: explicit list of models in this recipe that accept multimodal + * input. Required when the recipe mixes text-only and multimodal models + * under the same touchpoint (e.g. Voyage). embedMultimodal() validates + * `parsed.modelId` against this list AFTER `supports_multimodal` is true, + * pre-flighting the HTTP 400 a non-multimodal-capable model would otherwise + * trigger at the endpoint. When omitted, every model in `models` is + * treated as multimodal-capable (back-compat for providers where the whole + * recipe is multimodal). The check fires only inside embedMultimodal(); + * text embedding paths ignore it. + */ + multimodal_models?: string[]; } /** @@ -142,6 +154,12 @@ export interface AIGatewayConfig { embedding_model?: string; /** Target embedding dims. Gateway asserts returned embeddings match this. */ embedding_dimensions?: number; + /** + * Separate model for multimodal embeddings (e.g. "voyage:voyage-multimodal-3"). + * When set, embedMultimodal() routes to this model instead of embedding_model. + * Allows brains using OpenAI for text to use Voyage for image embeddings. + */ + embedding_multimodal_model?: string; /** Current expansion model as "provider:modelId". */ expansion_model?: string; /** Default chat model for `gateway.chat()` callers (subagent default). */ diff --git a/src/core/config.ts b/src/core/config.ts index 456c76e3d..fbb5f125d 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -79,6 +79,8 @@ export interface GBrainConfig { * still win as the operator escape hatch. */ embedding_multimodal?: boolean; + /** Model override for multimodal embeddings (e.g. "voyage:voyage-multimodal-3"). */ + embedding_multimodal_model?: string; embedding_image_ocr?: boolean; embedding_image_ocr_model?: string; } @@ -122,6 +124,9 @@ export function loadConfig(): GBrainConfig | null { ...(process.env.GBRAIN_EMBEDDING_IMAGE_OCR ? { embedding_image_ocr: process.env.GBRAIN_EMBEDDING_IMAGE_OCR === 'true' } : {}), + ...(process.env.GBRAIN_EMBEDDING_MULTIMODAL_MODEL + ? { embedding_multimodal_model: process.env.GBRAIN_EMBEDDING_MULTIMODAL_MODEL } + : {}), ...(process.env.GBRAIN_EMBEDDING_IMAGE_OCR_MODEL ? { embedding_image_ocr_model: process.env.GBRAIN_EMBEDDING_IMAGE_OCR_MODEL } : {}), @@ -172,6 +177,7 @@ export async function loadConfigWithEngine( } const dbMultimodal = await dbBool('embedding_multimodal'); + const dbMultimodalModel = await dbStr('embedding_multimodal_model'); const dbOcr = await dbBool('embedding_image_ocr'); const dbOcrModel = await dbStr('embedding_image_ocr_model'); @@ -182,6 +188,9 @@ export async function loadConfigWithEngine( if (merged.embedding_multimodal === undefined && dbMultimodal !== undefined) { merged.embedding_multimodal = dbMultimodal; } + if (merged.embedding_multimodal_model === undefined && dbMultimodalModel !== undefined) { + merged.embedding_multimodal_model = dbMultimodalModel; + } if (merged.embedding_image_ocr === undefined && dbOcr !== undefined) { merged.embedding_image_ocr = dbOcr; } diff --git a/test/cli-multimodal-integration.test.ts b/test/cli-multimodal-integration.test.ts new file mode 100644 index 000000000..1952057e8 --- /dev/null +++ b/test/cli-multimodal-integration.test.ts @@ -0,0 +1,124 @@ +// v0.28.11 (PR #719, D5): cli connectEngine() DB→gateway plumbing. +// +// The unit tests for loadConfigWithEngine and embedMultimodal cover their +// own contracts but don't exercise the cli.ts glue that ties them together. +// Codex F3 flagged this as the actual bug site. This file drives the same +// merge + reconfigure sequence connectEngine() runs and asserts the gateway +// observed the DB-set value through buildGatewayConfig. +// +// PGLite-only: in-memory engine, no DATABASE_URL needed. + +import { afterAll, beforeAll, beforeEach, describe, expect, test } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { loadConfigWithEngine, type GBrainConfig } from '../src/core/config.ts'; +import { + configureGateway, + getEmbeddingModel, + getMultimodalModel, + resetGateway, +} from '../src/core/ai/gateway.ts'; +import type { AIGatewayConfig } from '../src/core/ai/types.ts'; + +// Mirror the cli.ts buildGatewayConfig helper exactly. Keeping a copy here +// (instead of exporting from cli.ts) is intentional: the test asserts the +// shape of the contract, not the helper's identity. If cli.ts drifts, the +// e2e behavior these tests care about (DB-set value lands in gateway) still +// holds, but a helper-shape test would also catch the drift in PR review. +function buildGatewayConfig(c: GBrainConfig): AIGatewayConfig { + return { + embedding_model: c.embedding_model, + embedding_dimensions: c.embedding_dimensions, + embedding_multimodal_model: c.embedding_multimodal_model, + expansion_model: c.expansion_model, + chat_model: c.chat_model, + chat_fallback_chain: c.chat_fallback_chain, + base_urls: c.provider_base_urls, + env: { ...process.env }, + }; +} + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + resetGateway(); + // Clear any prior config rows so tests are independent. setConfig with + // empty string is treated as undefined by loadConfigWithEngine (per + // dbStr semantics), so this is safe to call between tests. + await engine.setConfig('embedding_multimodal_model', ''); +}); + +describe('cli connectEngine — embedding_multimodal_model DB→gateway plumbing', () => { + test('DB-set multimodal_model flows to gateway via merge + reconfigure', async () => { + await engine.setConfig('embedding_multimodal_model', 'voyage:voyage-multimodal-3'); + + const baseConfig: GBrainConfig = { + engine: 'pglite', + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + }; + + // First call mirrors cli.ts:715 (pre-engine-connect). + configureGateway(buildGatewayConfig(baseConfig)); + expect(getMultimodalModel()).toBeUndefined(); + + // Second call mirrors cli.ts:776 (post-DB-merge). + const merged = await loadConfigWithEngine(engine, baseConfig); + expect(merged).not.toBeNull(); + configureGateway(buildGatewayConfig(merged!)); + + // Primary embedding_model stays put (file/env wins); multimodal_model + // arrived via DB. + expect(getEmbeddingModel()).toBe('openai:text-embedding-3-large'); + expect(getMultimodalModel()).toBe('voyage:voyage-multimodal-3'); + }); + + test('file value wins over DB value (env > file > DB precedence at gateway level)', async () => { + await engine.setConfig('embedding_multimodal_model', 'voyage:voyage-3-large'); + + const baseConfig: GBrainConfig = { + engine: 'pglite', + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + embedding_multimodal_model: 'voyage:voyage-multimodal-3', // file plane + }; + + const merged = await loadConfigWithEngine(engine, baseConfig); + configureGateway(buildGatewayConfig(merged!)); + + expect(getMultimodalModel()).toBe('voyage:voyage-multimodal-3'); + }); + + test('un-gated re-config: merged DB has no multimodal_model → gateway still gets re-configured', async () => { + // Codex F5 was about whether the un-gated re-config weakens an + // intentional contract. This test pins the actual behavior (D6 = B): + // re-config always fires when merge succeeds, even when no DB key + // changed. Schema-sizing fields stay stable because loadConfigWithEngine + // respects file/env first. + const baseConfig: GBrainConfig = { + engine: 'pglite', + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + }; + + configureGateway(buildGatewayConfig(baseConfig)); + expect(getEmbeddingModel()).toBe('openai:text-embedding-3-large'); + + const merged = await loadConfigWithEngine(engine, baseConfig); + configureGateway(buildGatewayConfig(merged!)); + + // Primary model unchanged (DB had no override); the re-config is a + // semantic no-op for these fields. + expect(getEmbeddingModel()).toBe('openai:text-embedding-3-large'); + expect(getMultimodalModel()).toBeUndefined(); + }); +}); diff --git a/test/loadConfig-merge.test.ts b/test/loadConfig-merge.test.ts index b43089799..6af6e649e 100644 --- a/test/loadConfig-merge.test.ts +++ b/test/loadConfig-merge.test.ts @@ -107,4 +107,47 @@ describe('loadConfigWithEngine (Phase 4 / F3)', () => { expect(merged?.embedding_multimodal).toBe(false); expect(merged?.embedding_image_ocr).toBe(false); }); + + // v0.28.11 (PR #719): embedding_multimodal_model precedence parity with the + // sibling embedding_image_ocr_model field. Confirms the new key participates + // in the same env > file > DB > undefined merge contract so that + // embedMultimodal() routes correctly regardless of which plane set it. + describe('embedding_multimodal_model precedence', () => { + test('DB value fills in when file/env did not set it', async () => { + const base: GBrainConfig = { engine: 'pglite' }; + const engine = makeEngine({ + embedding_multimodal_model: 'voyage:voyage-multimodal-3', + }); + const merged = await loadConfigWithEngine(engine, base); + expect(merged?.embedding_multimodal_model).toBe('voyage:voyage-multimodal-3'); + }); + + test('file value wins over DB value', async () => { + const base: GBrainConfig = { + engine: 'pglite', + embedding_multimodal_model: 'voyage:voyage-multimodal-3', + }; + const engine = makeEngine({ + embedding_multimodal_model: 'voyage:voyage-3-large', + }); + const merged = await loadConfigWithEngine(engine, base); + expect(merged?.embedding_multimodal_model).toBe('voyage:voyage-multimodal-3'); + }); + + test('all unset stays undefined', async () => { + const base: GBrainConfig = { engine: 'pglite' }; + const engine = makeEngine({}); + const merged = await loadConfigWithEngine(engine, base); + expect(merged?.embedding_multimodal_model).toBeUndefined(); + }); + + test('null/empty DB string is ignored (does not clobber)', async () => { + const base: GBrainConfig = { engine: 'pglite' }; + const engine = makeEngine({ + embedding_multimodal_model: '', + }); + const merged = await loadConfigWithEngine(engine, base); + expect(merged?.embedding_multimodal_model).toBeUndefined(); + }); + }); }); diff --git a/test/voyage-multimodal.test.ts b/test/voyage-multimodal.test.ts index 5a91e65e5..c498fd5e1 100644 --- a/test/voyage-multimodal.test.ts +++ b/test/voyage-multimodal.test.ts @@ -281,3 +281,77 @@ describe('gateway.embedMultimodal — error paths', () => { expect((err as AIConfigError).message).toMatch(/does not support multimodal|not implemented/i); }); }); + +// v0.28.11 (PR #719): embedding_multimodal_model override + model-level +// validation. Confirms the gateway's two-layer multimodal gate: +// 1. recipe.touchpoints.embedding.supports_multimodal (recipe scope) +// 2. recipe.touchpoints.embedding.multimodal_models[] (model scope) +describe('gateway.embedMultimodal — multimodal_model override + model-level validation', () => { + test('prefers embedding_multimodal_model over embedding_model when both set', async () => { + configureGateway({ + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + embedding_multimodal_model: 'voyage:voyage-multimodal-3', + env: { VOYAGE_API_KEY: 'voyage-key', OPENAI_API_KEY: 'sk-test' }, + }); + let capturedUrl = ''; + let capturedBody: { model?: string } = {}; + fetchHandler = async (url, init) => { + capturedUrl = url; + capturedBody = JSON.parse(init.body as string); + return fakeVoyageResponse(1); + }; + const out = await embedMultimodal([makeImage()]); + expect(out.length).toBe(1); + expect(capturedUrl).toContain('/multimodalembeddings'); + expect(capturedBody.model).toBe('voyage-multimodal-3'); + }); + + test('falls back to embedding_model when embedding_multimodal_model is unset', async () => { + // Regression guard for the existing single-model setup. + configureVoyageMultimodal(); + fetchHandler = async () => fakeVoyageResponse(1); + const out = await embedMultimodal([makeImage()]); + expect(out.length).toBe(1); + }); + + test('embedding_multimodal_model pointing at non-multimodal recipe → AIConfigError', async () => { + configureGateway({ + embedding_model: 'voyage:voyage-multimodal-3', // would normally work + embedding_multimodal_model: 'openai:text-embedding-3-large', // override breaks it + embedding_dimensions: 1536, + env: { VOYAGE_API_KEY: 'voyage-key', OPENAI_API_KEY: 'sk-test' }, + }); + let err: unknown; + try { + await embedMultimodal([makeImage()]); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(AIConfigError); + expect((err as AIConfigError).message).toMatch(/does not support multimodal/i); + }); + + test('embedding_multimodal_model pointing at Voyage text-only model → AIConfigError (D4 / Codex F1)', async () => { + // Voyage shares supports_multimodal: true across all 12 models in the + // recipe. Without the model-level multimodal_models gate, voyage-3-large + // would pass validation locally and fail at /multimodalembeddings with + // HTTP 400 — which gateway.ts:626 misclassifies as transient. Change 3 + // closes this gap. + configureGateway({ + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + embedding_multimodal_model: 'voyage:voyage-3-large', // text-only Voyage + env: { VOYAGE_API_KEY: 'voyage-key', OPENAI_API_KEY: 'sk-test' }, + }); + let err: unknown; + try { + await embedMultimodal([makeImage()]); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(AIConfigError); + expect((err as AIConfigError).message).toMatch(/voyage-3-large.*not.*multimodal/i); + expect((err as AIConfigError).fix ?? '').toMatch(/voyage:voyage-multimodal-3/); + }); +});