Files
gbrain/test/loadConfig-merge.test.ts
T
bfab1ded08 v0.28.11 feat: embedding_multimodal_model — separate model routing for multimodal embeddings (#719)
* feat: embedding_multimodal_model — separate model routing for multimodal embeddings

v0.28.9 shipped multimodal image embeddings via Voyage, but
embedMultimodal() hardcodes to the primary embedding_model. Brains
using OpenAI text-embedding-3-large (1536-dim) for text cannot use
Voyage voyage-multimodal-3 (1024-dim) for images without switching
their entire embedding pipeline.

This adds embedding_multimodal_model as a distinct config key that
embedMultimodal() prefers over embedding_model when set. The dual-
column schema (embedding vs embedding_image) already supports
different dimensions — this patch completes the routing.

Config surface:
  - gbrain config set embedding_multimodal_model voyage:voyage-multimodal-3
  - env: GBRAIN_EMBEDDING_MULTIMODAL_MODEL=voyage:voyage-multimodal-3

Files changed:
  - core/ai/types.ts: AIGatewayConfig gains embedding_multimodal_model
  - core/ai/gateway.ts: configureGateway stores it; embedMultimodal reads it
  - core/config.ts: GBrainConfig type + env loader + DB merge path
  - cli.ts: threads config into gateway; reconfigures after DB merge

Tested on a 96K-page brain with OpenAI text + Voyage multimodal
running side by side. Voyage returns 1024-dim vectors into
embedding_image column; text embeddings unchanged.

* refactor(cli): extract buildGatewayConfig + always re-config after DB merge

Two related changes co-located so the un-gate doesn't leave the duplicated
configureGateway shapes drifting:

1. Extract file-local `buildGatewayConfig(c: GBrainConfig): AIGatewayConfig`
   helper. Both configureGateway sites in connectEngine() now pass through
   it; future fields touch one place.

2. Drop the field-name-gated re-config trigger. The previous gate fired
   only when `merged.embedding_multimodal_model` was truthy, coupling the
   trigger to one field name. Future DB-mutable gateway fields would
   silently miss it. Re-config now always fires when loadConfigWithEngine
   returns non-null. One extra cache+shrinkState clear per startup is
   microseconds, no hot path.

Schema-sizing fields stay stable because loadConfigWithEngine respects
file/env first; merged.embedding_dimensions equals config.embedding_dimensions
when no DB override exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(ai): model-level multimodal validation + getMultimodalModel accessor

Codex review of PR #719 (F1) caught a real footgun: the Voyage recipe
shares supports_multimodal: true across all 12 models in its embedding
touchpoint, of which only voyage-multimodal-3 is valid at
/multimodalembeddings. A user setting embedding_multimodal_model to a
text-only Voyage model (e.g. voyage-3-large) passes local validation and
fails at the endpoint with HTTP 400 — which gateway.ts:626 misclassifies
as transient (TODO: reclassify, tracked in TODOS.md).

Adds:
- EmbeddingTouchpoint.multimodal_models?: string[] (optional, model-level
  allow-list inside a recipe that mixes text-only + multimodal models).
- Voyage declares multimodal_models: ['voyage-multimodal-3'].
- embedMultimodal() validates parsed.modelId against the allow-list AFTER
  the existing recipe-level supports_multimodal check. Throws AIConfigError
  with the full multimodal_models list in the fix hint.
- getMultimodalModel() public accessor mirroring getEmbeddingModel /
  getChatModel — needed by the cli-multimodal-integration test and useful
  for future doctor checks.

Recipe-level fast-fail stays so non-multimodal providers (Anthropic /
OpenAI today) keep their AIConfigError path unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: cover embedding_multimodal_model precedence + gateway override + cli integration

PR #719 originally shipped zero tests for the new code paths. Closes
that gap with three layers:

1. test/loadConfig-merge.test.ts — extends the existing env > file > DB
   precedence pattern (which already covers embedding_image_ocr_model)
   with four cases for embedding_multimodal_model: DB-only fills in,
   file wins over DB, all-unset stays undefined, null/empty DB ignored.

2. test/voyage-multimodal.test.ts — four cases for embedMultimodal model
   resolution: prefers multimodal_model over embedding_model, falls back
   to embedding_model when unset (regression guard), AIConfigError on
   non-multimodal recipe, AIConfigError on Voyage text-only model
   (Codex F1 model-level validation).

3. test/cli-multimodal-integration.test.ts (NEW) — three PGLite-based
   integration tests for the cli.ts re-config glue itself (Codex F3:
   the actual bug site that "mechanical glue" claims hide). Drives the
   loadConfigWithEngine + buildGatewayConfig + configureGateway sequence
   connectEngine() runs and asserts the gateway observed the DB-set value.

11 new test cases total. All pass against the production code in this
PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(todos): follow-ups from PR #719 codex review

Three items surfaced during /codex outside-voice review of PR #719's plan
that are out of scope for the current PR but worth tracking:

- gbrain doctor: warn on misconfigured multimodal model (P2). Two checks:
  multimodal_model set without recipe API key; embedding_multimodal flag
  on without a multimodal-capable embedding_model.

- Reclassify Voyage HTTP 4xx as AIConfigError (P2, Codex F2). Today
  gateway.ts:626 throws AITransientError for any non-401/403 4xx, so
  permanent config bugs (malformed body, model not in multimodal_models)
  trigger retry storms. Aligns with normalizeAIError's contract.

- gbrain config unset <key> (P3, Codex F6). Once a user sets a key in DB
  there's no normal CLI path to clear it. Pre-existing UX gap; PR #719's
  new key surfaces it again.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump version and changelog (v0.28.11)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: v0.28.11 annotations for ai/types, ai/gateway, voyage recipe

Updates the Key Files section so the per-file annotations reflect the
multimodal_model routing + model-level validation that landed in #719.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: garrytan-agents <garrytan-agents@users.noreply.github.com>
Co-authored-by: Garry Tan <garrytan@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 13:41:46 -07:00

154 lines
5.7 KiB
TypeScript

// Phase 4 (F3): loadConfigWithEngine() DB-merge contract.
//
// Verifies precedence (env > file > DB > defaults) for the new v0.27.1
// multimodal flags so `gbrain config set embedding_multimodal true`
// actually flips the runtime gate even when the file plane is silent.
import { describe, expect, test } from 'bun:test';
import { loadConfigWithEngine, type GBrainConfig } from '../src/core/config.ts';
interface FakeEngine {
getConfig(key: string): Promise<string | null | undefined>;
}
function makeEngine(map: Record<string, string | null | undefined>): FakeEngine {
return {
async getConfig(key: string) {
return map[key];
},
};
}
describe('loadConfigWithEngine (Phase 4 / F3)', () => {
test('returns null when base config is null', async () => {
const result = await loadConfigWithEngine(makeEngine({}), null);
expect(result).toBeNull();
});
test('DB flag fills in when file/env did not set it', async () => {
const base: GBrainConfig = { engine: 'pglite' };
const engine = makeEngine({
embedding_multimodal: 'true',
embedding_image_ocr: 'false',
embedding_image_ocr_model: 'openai:gpt-4o-mini',
});
const merged = await loadConfigWithEngine(engine, base);
expect(merged?.embedding_multimodal).toBe(true);
expect(merged?.embedding_image_ocr).toBe(false);
expect(merged?.embedding_image_ocr_model).toBe('openai:gpt-4o-mini');
});
test('file/env precedence: file value wins over DB value', async () => {
const base: GBrainConfig = {
engine: 'pglite',
embedding_multimodal: false,
embedding_image_ocr_model: 'file-set-model',
};
const engine = makeEngine({
embedding_multimodal: 'true',
embedding_image_ocr_model: 'db-set-model',
});
const merged = await loadConfigWithEngine(engine, base);
expect(merged?.embedding_multimodal).toBe(false);
expect(merged?.embedding_image_ocr_model).toBe('file-set-model');
});
test('partial DB merge: only undefined fields fall through', async () => {
const base: GBrainConfig = {
engine: 'pglite',
embedding_multimodal: true,
// embedding_image_ocr NOT set in file plane
};
const engine = makeEngine({
embedding_multimodal: 'false',
embedding_image_ocr: 'true',
});
const merged = await loadConfigWithEngine(engine, base);
// file/env wins for multimodal
expect(merged?.embedding_multimodal).toBe(true);
// DB fills in for ocr
expect(merged?.embedding_image_ocr).toBe(true);
});
test('engine.getConfig throwing is non-fatal — file/env config still returned', async () => {
const base: GBrainConfig = {
engine: 'pglite',
embedding_multimodal: true,
};
const engine: FakeEngine = {
async getConfig() {
throw new Error('config table missing');
},
};
const merged = await loadConfigWithEngine(engine, base);
expect(merged?.embedding_multimodal).toBe(true);
});
test('null/empty DB values are ignored (not coerced to false)', async () => {
const base: GBrainConfig = { engine: 'pglite' };
const engine = makeEngine({
embedding_multimodal: null,
embedding_image_ocr: '',
embedding_image_ocr_model: undefined,
});
const merged = await loadConfigWithEngine(engine, base);
expect(merged?.embedding_multimodal).toBeUndefined();
expect(merged?.embedding_image_ocr).toBeUndefined();
expect(merged?.embedding_image_ocr_model).toBeUndefined();
});
test('non-"true" DB string values resolve to false (strict equality)', async () => {
const base: GBrainConfig = { engine: 'pglite' };
const engine = makeEngine({
embedding_multimodal: 'TRUE', // wrong case
embedding_image_ocr: '1', // wrong format
});
const merged = await loadConfigWithEngine(engine, base);
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();
});
});
});