mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🧢 fix: Cap Label Generation, Order the Flag Persist, Detach Settled Listeners
Round-nineteen review: three fixed; the fourth is the ninth instance of the edited+reconnect index limitation, answered on-thread. - Generation cap (host.ts): stripping the primary output caps left label calls with NO cap at all — `normalizeLabelOutput` bounds what persists, not what the provider generates and bills, so a model ignoring the 4–9-word instruction (or steered by injected tool output) could emit its provider-default output per batch. The sanitize step now installs a 256-token label cap (per provider family: `maxOutputTokens` for Google-style wrappers, `maxTokens` otherwise), after the filter so the omit set cannot remove it. - Flag-persist ordering (client.js): the `markActivityLabels` write was fire-and-forget, so an immediate cross-replica reconnect could read the job between the write and the first claim, see neither flag nor snapshot label, and skip gap reconciliation. Label emission now awaits the (settled-on-failure) persist chain, making "a label event exists" imply "the flag is durable" — the race window is gone; only the documented double-write-failure residual remains. - Listener detach (client.js): each HITL approval cycle's wiring adds a `once` abort listener to the shared job signal that only an actual abort removes; settled segments now detach theirs in `settleActivityLabels`, so long multi-approval runs cannot accumulate dead closures toward the listener-limit warning. Tests: the primary cap is REPLACED by the 256-token label cap (host.spec).
This commit is contained in:
parent
98409ecce3
commit
0928ac7d7f
3 changed files with 50 additions and 6 deletions
|
|
@ -658,6 +658,12 @@ class AgentClient extends BaseClient {
|
|||
scope.abort.abort();
|
||||
}
|
||||
});
|
||||
/** Settled either way — the abort listener has no remaining work, and
|
||||
* leaving it attached across HITL approval cycles accumulates dead
|
||||
* closures on the shared job signal. Idempotent on double settle. */
|
||||
for (const scope of this.activityLabelScopes ?? []) {
|
||||
scope.detach?.();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -712,7 +718,10 @@ class AgentClient extends BaseClient {
|
|||
* content itself recorded perfectly well. One retry costs nothing at run
|
||||
* setup and removes the only realistic way the gate goes stale.
|
||||
*/
|
||||
void GenerationJobManager.markActivityLabels(streamId).catch(() =>
|
||||
/** Retained (not fire-and-forget): label emission is ORDERED after this
|
||||
* persist, below. The chain settles on failure (warned retry), so a
|
||||
* lost write can never wedge label emission. */
|
||||
const activityLabelsMarked = GenerationJobManager.markActivityLabels(streamId).catch(() =>
|
||||
GenerationJobManager.markActivityLabels(streamId).catch(() => {
|
||||
logger.warn(
|
||||
`[AgentClient] Could not flag activity labels for ${streamId}; a label resolving during a resume gap may not be reconciled.`,
|
||||
|
|
@ -756,6 +765,12 @@ class AgentClient extends BaseClient {
|
|||
closeOnAbort();
|
||||
} else {
|
||||
abortSignal.addEventListener('abort', closeOnAbort, { once: true });
|
||||
/** Detached once this segment settles: HITL runs rebuild a wiring
|
||||
* per approval cycle on the SAME job signal, and `once` only
|
||||
* removes the listener if an abort actually fires — long
|
||||
* multi-approval runs would otherwise accumulate obsolete
|
||||
* closures toward the listener-limit warning. */
|
||||
labelScope.detach = () => abortSignal.removeEventListener('abort', closeOnAbort);
|
||||
}
|
||||
}
|
||||
/** Thin wrapper: slot claiming, lane stamping, emit ordering, and settle
|
||||
|
|
@ -770,8 +785,15 @@ class AgentClient extends BaseClient {
|
|||
bumpIndexOffset: () => {
|
||||
this.steerOffsetState.offset += 1;
|
||||
},
|
||||
emitLabelEvent: (index, part) =>
|
||||
GenerationJobManager.emitChunk(
|
||||
emitLabelEvent: async (index, part) => {
|
||||
/** ORDERED after the flag persist: resume-gap reconciliation is
|
||||
* gated on the flag, so a label event must never exist before the
|
||||
* flag does — an immediate cross-replica reconnect could otherwise
|
||||
* read the job between the two writes, see neither flag nor
|
||||
* snapshot label, and skip reconciling a label claimed in the
|
||||
* snapshot→subscribe window. */
|
||||
await activityLabelsMarked;
|
||||
return GenerationJobManager.emitChunk(
|
||||
streamId,
|
||||
{
|
||||
event: ActivityLabelEvents.ON_ACTIVITY_LABEL,
|
||||
|
|
@ -788,7 +810,8 @@ class AgentClient extends BaseClient {
|
|||
* the new response — invisibly, since an empty label renders
|
||||
* nothing — overwriting whatever occupies that slot. */
|
||||
{ durable: true, expectedCreatedAt: this.jobCreatedAt },
|
||||
),
|
||||
);
|
||||
},
|
||||
trackPendingFill: (fillDone) => {
|
||||
this.pendingActivityLabelFills = this.pendingActivityLabelFills ?? [];
|
||||
this.pendingActivityLabelFills.push(fillDone);
|
||||
|
|
|
|||
|
|
@ -185,12 +185,16 @@ describe('resolveActivityLabelModel model precedence', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('strips the primary maxTokens like the title path', async () => {
|
||||
/** The primary cap is not merely stripped — it is REPLACED with a small
|
||||
* label-specific one, so a model ignoring the 4–9-word instruction (or
|
||||
* steered by injected tool output) cannot generate and bill its
|
||||
* provider-default output for a header. */
|
||||
it('replaces the primary maxTokens with the small label cap', async () => {
|
||||
mockGetOptions.mockResolvedValueOnce({
|
||||
llmConfig: { model: 'resolved', maxTokens: 64_000 },
|
||||
} as never);
|
||||
const resolved = await resolve({ activityLabel: true, activityModel: 'label-model' });
|
||||
expect((resolved.clientOptions as Record<string, unknown>).maxTokens).toBeUndefined();
|
||||
expect((resolved.clientOptions as Record<string, unknown>).maxTokens).toBe(256);
|
||||
});
|
||||
|
||||
/** The Anthropic carrier holds client CONSTRUCTION options — for
|
||||
|
|
|
|||
|
|
@ -116,6 +116,14 @@ type MaybeAzureConfig = ClientOptions & {
|
|||
configuration?: OpenAIConfiguration;
|
||||
};
|
||||
|
||||
/** Generation cap for label calls — ~25x the largest legitimate 4–9 word
|
||||
* header, so truncation can never clip a real label. Replaces the stripped
|
||||
* primary caps: with NO cap, a model that ignores the instruction (or is
|
||||
* steered by injection in untrusted tool output) generates and BILLS its
|
||||
* provider-default output on every batch; `normalizeLabelOutput` bounds
|
||||
* only what persists, not what the provider generates. */
|
||||
const LABEL_MAX_OUTPUT_TOKENS = 256;
|
||||
|
||||
/** Effective activity-label settings for one endpoint. */
|
||||
export interface ResolvedActivityConfig {
|
||||
enabled: boolean;
|
||||
|
|
@ -330,6 +338,15 @@ export async function resolveActivityLabelModel({
|
|||
if (anthropicCarrier != null && clientOptions.clientOptions == null) {
|
||||
clientOptions.clientOptions = anthropicCarrier;
|
||||
}
|
||||
/** Replace the stripped primary caps with a SMALL one — see
|
||||
* {@link LABEL_MAX_OUTPUT_TOKENS}. Installed after the filter so the
|
||||
* omit set cannot remove it; keyed per provider family (Google-style
|
||||
* wrappers read `maxOutputTokens`, the rest `maxTokens`). */
|
||||
if (provider === Providers.GOOGLE || provider === Providers.VERTEXAI) {
|
||||
(clientOptions as { maxOutputTokens?: number }).maxOutputTokens = LABEL_MAX_OUTPUT_TOKENS;
|
||||
} else {
|
||||
(clientOptions as { maxTokens?: number }).maxTokens = LABEL_MAX_OUTPUT_TOKENS;
|
||||
}
|
||||
if (options.configOptions) {
|
||||
clientOptions.configuration = options.configOptions;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue