mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🎛️ fix: Sanitize Label Client Options and Bound the Batch Prompt
Round-seventeen review: two fixed; the other two findings repeat the maintainer-decided packages/api extraction (follow-up) and the edited+reconnect index limitation (seventh instance), answered on-thread. - Primary-option strip (host.ts): the label client copied the resolved `llmConfig` wholesale, so an endpoint whose defaults enable extended thinking or carry model-specific output caps forwarded them to the (often cheaper) label model — unsupported options failed every label, and supported thinking spent real tokens and the settlement window on a 4–9 word header. The copy now strips `omitTitleOptions` keys and the `modelKwargs` output caps exactly like the title path, restoring the Anthropic `clientOptions` carrier by reference so proxy `defaultHeaders` still reach label requests. - Batch prompt budget (runtime.ts): per-entry truncation left the batch dimension unbounded — hundreds of parallel calls could build a prompt past the fast model's window. The entries section now has a total budget (8k chars, scaling with `activityCharLimit` so a raised limit still fits several entries); entries past it are skipped without paying their serialization cost, and the list notes how many were omitted. The first entry always renders in full. Tests: option strip with header-carrier survival (host.spec); giant batch bounded with omission marker, small batch untouched (runtime.spec).
This commit is contained in:
parent
4fc63fc2a3
commit
1ff605c401
4 changed files with 116 additions and 4 deletions
|
|
@ -159,6 +159,31 @@ describe('resolveActivityLabelModel model precedence', () => {
|
|||
expect.objectContaining({ model_parameters: { model: 'label-model' } }),
|
||||
);
|
||||
});
|
||||
|
||||
/** The label often runs a cheaper model than the primary generation, so
|
||||
* primary-only options (thinking, output caps) must be stripped exactly
|
||||
* like the title path — while proxy headers survive. */
|
||||
it('strips primary-generation options but keeps the Anthropic header carrier', async () => {
|
||||
mockGetOptions.mockResolvedValueOnce({
|
||||
llmConfig: {
|
||||
model: 'resolved',
|
||||
thinking: { type: 'enabled', budget_tokens: 4096 },
|
||||
maxOutputTokens: 8192,
|
||||
streaming: true,
|
||||
modelKwargs: { max_output_tokens: 8192, service_tier: 'flex' },
|
||||
clientOptions: { defaultHeaders: { 'x-proxy-key': 'abc' } },
|
||||
},
|
||||
} as never);
|
||||
const resolved = await resolve({ activityLabel: true, activityModel: 'label-model' });
|
||||
const clientOptions = resolved.clientOptions as Record<string, unknown>;
|
||||
expect(clientOptions.thinking).toBeUndefined();
|
||||
expect(clientOptions.maxOutputTokens).toBeUndefined();
|
||||
expect(clientOptions.streaming).toBeUndefined();
|
||||
expect(clientOptions.modelKwargs).toEqual({ service_tier: 'flex' });
|
||||
expect(clientOptions.clientOptions).toEqual({
|
||||
defaultHeaders: { 'x-proxy-key': 'abc' },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('mapCollectedMetadataToUsage cache tokens', () => {
|
||||
|
|
|
|||
|
|
@ -175,6 +175,38 @@ describe('buildPrompt', () => {
|
|||
expect(tight).toContain('…');
|
||||
});
|
||||
|
||||
/** Per-entry truncation alone leaves the batch dimension unbounded: a
|
||||
* parallel batch of hundreds of calls must not build a prompt past the
|
||||
* fast model's window. */
|
||||
it('bounds the total entries section for giant parallel batches', () => {
|
||||
const entries = Array.from({ length: 200 }, (_, i) => ({
|
||||
toolName: `tool_${i}`,
|
||||
toolInput: { i },
|
||||
toolUseId: `t${i}`,
|
||||
status: 'success' as const,
|
||||
toolOutput: 'y'.repeat(500),
|
||||
}));
|
||||
const prompt = buildPrompt(entries, 600);
|
||||
expect(prompt.length).toBeLessThan(15_000);
|
||||
expect(prompt).toMatch(/\(\+\d+ more tool calls not shown\)/);
|
||||
/** Prefix semantics: the first entry is always present in full. */
|
||||
expect(prompt).toContain('- tool_0(');
|
||||
expect(prompt).not.toContain('- tool_199(');
|
||||
});
|
||||
|
||||
it('shows every entry when the batch fits the budget', () => {
|
||||
const entries = Array.from({ length: 3 }, (_, i) => ({
|
||||
toolName: `tool_${i}`,
|
||||
toolInput: { i },
|
||||
toolUseId: `t${i}`,
|
||||
status: 'success' as const,
|
||||
toolOutput: 'ok',
|
||||
}));
|
||||
const prompt = buildPrompt(entries, 600);
|
||||
expect(prompt).toContain('- tool_2(');
|
||||
expect(prompt).not.toContain('more tool calls not shown');
|
||||
});
|
||||
|
||||
it('serializes small structured values exactly like JSON.stringify', () => {
|
||||
const toolInput = { q: 'docs', filters: { lang: 'en', page: 2 }, ids: [1, 2] };
|
||||
const prompt = buildPrompt(
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import type { EndpointDbMethods, OpenAIConfiguration, ServerRequest } from '~/ty
|
|||
import type { ActivityLabelLLM } from './runtime';
|
||||
import { getProviderConfig } from '~/endpoints/config/providers';
|
||||
import { resolveConfigHeaders } from '~/utils/headers';
|
||||
import { omitTitleOptions } from '~/agents/client';
|
||||
import { createSafeUser } from '~/utils/env';
|
||||
|
||||
/** Cache-token details in the LangChain-standard normalized shape. */
|
||||
|
|
@ -289,7 +290,36 @@ export async function resolveActivityLabelModel({
|
|||
) {
|
||||
provider = Providers.AZURE;
|
||||
}
|
||||
const clientOptions = { ...(llmConfig ?? {}) } as MaybeAzureConfig;
|
||||
/** Sanitized copy, exactly like the title path: the label often runs a
|
||||
* DIFFERENT (cheaper) model than the primary generation, so
|
||||
* primary-generation options must not ride along. `omitTitleOptions`
|
||||
* drops thinking/streaming/output-cap keys that can make the label
|
||||
* request fail outright on the substitute model — or spend extended
|
||||
* thinking on a 4–9 word header and blow the settlement window. The
|
||||
* `modelKwargs` output caps go for the same reason (copied, not mutated:
|
||||
* `llmConfig` is shared with the memoized provider resolution). */
|
||||
const rawOptions = { ...(llmConfig ?? {}) } as MaybeAzureConfig & {
|
||||
modelKwargs?: Record<string, unknown>;
|
||||
clientOptions?: { defaultHeaders?: unknown };
|
||||
};
|
||||
if (rawOptions.modelKwargs != null) {
|
||||
const modelKwargs = { ...rawOptions.modelKwargs };
|
||||
delete modelKwargs.max_completion_tokens;
|
||||
delete modelKwargs.max_output_tokens;
|
||||
rawOptions.modelKwargs = modelKwargs;
|
||||
}
|
||||
/** The filter drops the Anthropic `clientOptions` carrier (thinking,
|
||||
* streaming), which would also drop its `defaultHeaders` — restore the
|
||||
* SAME object reference so gateway/proxy metadata still reaches label
|
||||
* requests and `resolveConfigHeaders` mutates the object the client is
|
||||
* actually built from. */
|
||||
const anthropicCarrier = rawOptions.clientOptions;
|
||||
const clientOptions = Object.fromEntries(
|
||||
Object.entries(rawOptions).filter(([key]) => !omitTitleOptions.has(key)),
|
||||
) as MaybeAzureConfig & { clientOptions?: { defaultHeaders?: unknown } };
|
||||
if (anthropicCarrier?.defaultHeaders != null && clientOptions.clientOptions == null) {
|
||||
clientOptions.clientOptions = anthropicCarrier;
|
||||
}
|
||||
if (options.configOptions) {
|
||||
clientOptions.configuration = options.configOptions;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -174,6 +174,13 @@ const DEFAULT_CHAR_LIMIT = 600;
|
|||
* reach a distinguishing path or query past the first 200 characters. */
|
||||
const INTENT_CHAR_LIMIT = 200;
|
||||
const SUMMARY_TIMEOUT_MS = 12_000;
|
||||
/** Total budget for the entries section. Per-entry truncation alone leaves
|
||||
* the batch dimension unbounded — a parallel batch of hundreds of calls
|
||||
* would build a prompt past the fast model's window and bill input tokens
|
||||
* far beyond what a one-line header justifies. Scales with the configured
|
||||
* per-entry limit so a raised `activityCharLimit` still fits several
|
||||
* entries; entries past the budget are skipped WITHOUT serializing them. */
|
||||
const ENTRIES_CHAR_BUDGET = 8_000;
|
||||
/** Hard bound on the PERSISTED label. The instruction asks for 4–9 words, but
|
||||
* a model that ignores it — or is steered by injection through untrusted
|
||||
* tool output — could otherwise turn one header into thousands of tokens
|
||||
|
|
@ -348,14 +355,32 @@ export function buildPrompt(
|
|||
.join('\n'),
|
||||
);
|
||||
}
|
||||
const lines = entries.map((entry) => {
|
||||
const budget = Math.max(ENTRIES_CHAR_BUDGET, charLimit * 4);
|
||||
const lines: string[] = [];
|
||||
let used = 0;
|
||||
let omitted = 0;
|
||||
for (const entry of entries) {
|
||||
/** Prefix cut, never a mid-list sample: once the budget is spent the
|
||||
* remaining entries are skipped unserialized (a giant batch must not
|
||||
* even pay the stringify cost for lines that will be dropped). The
|
||||
* first entry always fits, and the line that crosses the budget is
|
||||
* kept, so at least one complete call is always shown. */
|
||||
if (lines.length > 0 && used >= budget) {
|
||||
omitted += 1;
|
||||
continue;
|
||||
}
|
||||
const input = truncate(stringifyUnknown(entry.toolInput, charLimit), charLimit);
|
||||
const outcome =
|
||||
entry.status === 'error'
|
||||
? `ERROR: ${truncate(entry.error ?? 'unknown error', charLimit)}`
|
||||
: truncate(stringifyUnknown(entry.toolOutput, charLimit), charLimit);
|
||||
return `- ${entry.toolName}(${input}) → ${outcome}`;
|
||||
});
|
||||
const line = `- ${entry.toolName}(${input}) → ${outcome}`;
|
||||
lines.push(line);
|
||||
used += line.length;
|
||||
}
|
||||
if (omitted > 0) {
|
||||
lines.push(`- (+${omitted} more tool calls not shown)`);
|
||||
}
|
||||
/** Flagged as reference material: without this the model tends to read the
|
||||
* list as the thing to summarize and hands back a transcription of it. */
|
||||
sections.push(`What it called, and what came back (do not restate these):\n${lines.join('\n')}`);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue