mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-21 15:45:22 +00:00
🎯 refactor: Infer Agents Endpoint for Model Specs Naming an Agent (#14889)
* 🎯 fix: Infer Agents Endpoint for Model Specs Naming an Agent A model spec whose preset names an `agent_id` but omits `endpoint` was unusable. `isModelSpecEndpointMatch` compares the request's endpoint to `preset.endpoint` by strict equality, so an undefined endpoint matched nothing and every request selecting the spec was rejected with a bare `Model spec mismatch` — an error naming neither the spec nor the missing field. The selector had the matching half of the same gap: `handleSelectSpec` read `preset.endpoint` directly, so it sent no endpoint and skipped assigning `agent_id` to `model`. Fixing only the server would leave the request malformed, so the resolution is shared between both. - Add `resolveModelSpecEndpoint` to `librechat-data-provider`, inferring the agents endpoint when a preset names an agent and none is set. An explicit `endpoint` always wins, so configured specs are unaffected. - Use it for endpoint matching and in the selector, so the menu and the request pipeline resolve a spec identically. * 🔁 refactor: Materialize Inferred Spec Endpoints at Config Load The review showed the lazy-resolver approach was unsound end to end: config validation rejected an endpoint-less spec before the resolver could ever run (`tPresetSchema` requires the `endpoint` key), and the resolver was applied at 2 of ~8 read sites, leaving selection handlers, startup presets, access filters, and provider-key reachability reading the raw preset. Materialize once at the boundary instead: - `tModelSpecPresetSchema` now makes `endpoint` optional (`nullish`). This is barely a widening — `endpoint: null` already validated — and only for model-spec presets; `tPresetSchema` is untouched. - `materializeModelSpecEndpoints` writes each spec's resolved endpoint back onto its preset. `createAppConfigService` applies it at both effective-config assembly points — YAML base load and DB-override merge — so admin-panel specs stored in override documents are covered. Identity-preserving, so cached configs see no new references when nothing needs filling in. - Every consumer now reads complete specs; the client's lazy resolve in `handleSelectSpec` is reverted to a raw read. `getModelSpecPreset` and the two hand-rolled preset constructions resolve the endpoint explicitly, which the narrowed preset type now enforces at compile time for any `TPreset`-shaped destination. - `isModelSpecEndpointMatch` keeps the resolver as request-time defense. * 🩹 fix: Materialize Spec Endpoints Before the YAML Missing-Endpoint Guard `processModelSpecs` warns and skips any spec whose preset lacks an endpoint, and it runs inside `loadBaseConfig` — so the previous commit's materialization received a YAML list from which the inferable spec had already been dropped. Only DB-override specs (merged after the guard) actually benefited. - Materialize at the entry of `processModelSpecs`, so inference happens before the guard and YAML agent specs survive it. The guard keeps skipping genuinely endpoint-less specs. The `createAppConfigService` calls stay: the base-path one guards alternate `loadBaseConfig` implementations, the merged-path one covers override documents, and both are identity-preserving no-ops when specs are already complete. - Constrain the widened schema: omitting `endpoint` is only legal when the preset names an `agent_id`. A preset with neither validated as a hard error before the key became optional, and silently accepting it would trade that startup-time error for a dead spec. An explicit `endpoint: null` (valid before this PR) keeps validating. * 🩹 fix: Infer Only From Non-Empty Agent IDs, Never Over Explicit Null Two edge cases in the inference contract: - `agent_id: ''` (what a form-backed writer persists for an untouched field) passed the nullish checks, validating and materializing a spec that names no agent. Both the refinement and the resolver now require a non-empty id, so such config fails validation loudly instead of producing a selectable spec that cannot work. - `endpoint: null` alongside an `agent_id` was treated as inferable, silently activating a spec that validated — and was skipped — before this PR. An explicit null is a statement, not an omission: the resolver now infers only when the key is absent, preserving prior behavior for previously valid configs.
This commit is contained in:
parent
ed081964d6
commit
bce93f9c55
13 changed files with 437 additions and 29 deletions
|
|
@ -114,6 +114,51 @@ describe('createAppConfigService', () => {
|
|||
expect(deps.getApplicableConfigs).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('materializes inferred model-spec endpoints in the base config', async () => {
|
||||
const deps = createDeps({
|
||||
loadBaseConfig: jest.fn().mockResolvedValue({
|
||||
modelSpecs: {
|
||||
enforce: false,
|
||||
prioritize: true,
|
||||
list: [{ name: 'agent-spec', label: 'Agent Spec', preset: { agent_id: 'agent_abc' } }],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
||||
const config = await getAppConfig({ baseOnly: true });
|
||||
|
||||
expect(config.modelSpecs?.list?.[0]?.preset?.endpoint).toBe('agents');
|
||||
});
|
||||
|
||||
/**
|
||||
* Admin-panel specs arrive through DB override documents the base config
|
||||
* never saw, so materialization must also run on the merged result.
|
||||
*/
|
||||
it('materializes inferred model-spec endpoints contributed by DB overrides', async () => {
|
||||
const deps = createDeps({
|
||||
getApplicableConfigs: jest.fn().mockResolvedValue([
|
||||
{
|
||||
priority: 10,
|
||||
isActive: true,
|
||||
overrides: {
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{ name: 'agent-spec', label: 'Agent Spec', preset: { agent_id: 'agent_abc' } },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
]),
|
||||
});
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
||||
const config = (await getAppConfig({ role: 'USER' })) as TestConfig;
|
||||
|
||||
expect(config.modelSpecs?.list?.[0]?.preset?.endpoint).toBe('agents');
|
||||
expect(config.modelSpecs?.list?.[0]?.preset?.agent_id).toBe('agent_abc');
|
||||
});
|
||||
|
||||
it('caches empty result — does not re-query DB on second call', async () => {
|
||||
const deps = createDeps({ getApplicableConfigs: jest.fn().mockResolvedValue([]) });
|
||||
const { getAppConfig } = createAppConfigService(deps);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { PrincipalType } from 'librechat-data-provider';
|
||||
import { PrincipalType, materializeModelSpecEndpoints } from 'librechat-data-provider';
|
||||
import {
|
||||
logger,
|
||||
getTenantId,
|
||||
|
|
@ -10,6 +10,20 @@ import type { Types } from 'mongoose';
|
|||
|
||||
const BASE_CONFIG_KEY = '_BASE_';
|
||||
|
||||
/**
|
||||
* Materializes inferable model-spec fields (an omitted `preset.endpoint` for
|
||||
* agent specs) so every consumer of the effective config reads complete specs.
|
||||
* Runs at both assembly points — YAML base load and DB-override merge — because
|
||||
* override documents contribute specs the base config never saw.
|
||||
*/
|
||||
function materializeConfigModelSpecs(config: AppConfig): AppConfig {
|
||||
const modelSpecs = materializeModelSpecEndpoints(config.modelSpecs);
|
||||
if (modelSpecs === config.modelSpecs) {
|
||||
return config;
|
||||
}
|
||||
return { ...config, modelSpecs };
|
||||
}
|
||||
|
||||
export const DEFAULT_OVERRIDE_CACHE_TTL = 60_000;
|
||||
|
||||
// ── Types ────────────────────────────────────────────────────────────
|
||||
|
|
@ -169,6 +183,8 @@ export function createAppConfigService(deps: AppConfigServiceDeps): {
|
|||
throw new Error('Failed to initialize app configuration through AppService.');
|
||||
}
|
||||
|
||||
baseConfig = materializeConfigModelSpecs(baseConfig);
|
||||
|
||||
if (baseConfig.availableTools) {
|
||||
await setCachedTools(baseConfig.availableTools);
|
||||
}
|
||||
|
|
@ -241,7 +257,7 @@ export function createAppConfigService(deps: AppConfigServiceDeps): {
|
|||
return baseConfig;
|
||||
}
|
||||
|
||||
const merged = mergeConfigOverrides(baseConfig, configs);
|
||||
const merged = materializeConfigModelSpecs(mergeConfigOverrides(baseConfig, configs));
|
||||
await cache.set(cacheKey, merged, overrideCacheTtl);
|
||||
return merged;
|
||||
} catch (error) {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import {
|
||||
parseCompactConvo,
|
||||
replaceSpecialVars,
|
||||
resolveModelSpecEndpoint,
|
||||
type EModelEndpoint,
|
||||
type TConversation,
|
||||
type TModelSpec,
|
||||
|
|
@ -130,7 +131,7 @@ export function isModelSpecEndpointMatch(
|
|||
modelSpec: Pick<TModelSpec, 'preset'> | undefined,
|
||||
endpoint: string | null | undefined,
|
||||
): boolean {
|
||||
return Boolean(modelSpec && endpoint === modelSpec.preset?.endpoint);
|
||||
return Boolean(modelSpec && endpoint === resolveModelSpecEndpoint(modelSpec));
|
||||
}
|
||||
|
||||
export function applyModelSpecPreset({
|
||||
|
|
|
|||
|
|
@ -183,6 +183,47 @@ describe('modelSpecs helpers', () => {
|
|||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.google)).toBe(false);
|
||||
});
|
||||
|
||||
/**
|
||||
* A preset naming an `agent_id` can only be served by the agents endpoint, so
|
||||
* omitting `endpoint` previously left the spec matching nothing at all.
|
||||
*/
|
||||
it('should infer the agents endpoint when a preset omits it but names an agent', () => {
|
||||
const modelSpec: TModelSpec = {
|
||||
name: 'agent-spec',
|
||||
label: 'Agent Spec',
|
||||
preset: {
|
||||
agent_id: 'agent_abc',
|
||||
},
|
||||
} as TModelSpec;
|
||||
|
||||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.agents)).toBe(true);
|
||||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.openAI)).toBe(false);
|
||||
});
|
||||
|
||||
it('should keep an explicit endpoint over the inferred one', () => {
|
||||
const modelSpec: TModelSpec = {
|
||||
name: 'explicit-spec',
|
||||
label: 'Explicit Spec',
|
||||
preset: {
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
agent_id: 'agent_abc',
|
||||
},
|
||||
} as TModelSpec;
|
||||
|
||||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.openAI)).toBe(true);
|
||||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.agents)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not infer an endpoint for presets without an agent', () => {
|
||||
const modelSpec: TModelSpec = {
|
||||
name: 'bare-spec',
|
||||
label: 'Bare Spec',
|
||||
preset: {},
|
||||
} as TModelSpec;
|
||||
|
||||
expect(isModelSpecEndpointMatch(modelSpec, EModelEndpoint.agents)).toBe(false);
|
||||
});
|
||||
|
||||
it('should resolve special variables in model spec prompt prefixes', () => {
|
||||
expect(
|
||||
resolveModelSpecPromptPrefixVariables({ promptPrefix: 'Help {{current_user}}.' }, {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue