mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-26 17:12:27 +03:00
fix(sse): stop re-summarizing a universal handoff that never parses
A universal handoff whose summary comes back unparseable persists nothing, so shouldGenerateUniversalHandoff keeps answering "generate" and the very next model switch in the same session re-issues the same full-history summarization call and discards the answer again — forever. With a switch-heavy combo strategy (weighted, random, round-robin, p2c) the models alternate on almost every turn, so that background call lands on a large fraction of requests: an upstream call whose response nobody reads is real money on a paid provider and real quota on a metered one. Measured on the weighted 70/30 matrix, that inflated the observed openai share to 0.895 where routing actually delivered 0.70, and at the unit level 199 of 200 model switches issued a fresh discarded summarization call. Back off per (session, combo) after an answer that is not a usable handoff: exponential 5min -> 1h, cleared on the first successful generation, capped at 500 tracked keys. Deliberately narrow — a transient upstream failure (!response.ok) is NOT tracked, so it still retries on the next switch, which is the behavior the context-relay path already depends on. After the fix, same harness at n=200: 201 upstream calls for 200 requests (1 extra, 0.5%), and the measured openai share equals the delivered share (0.725). The unit guard drops 199 discarded calls to 1. Refs #11552
This commit is contained in:
@@ -230,7 +230,10 @@ function formatMessagesForPrompt(messages: MessageLike[]): string {
|
||||
.join("\n\n");
|
||||
}
|
||||
|
||||
export function selectMessagesForSummary(messages: MessageLike[], maxMessages: number): MessageLike[] {
|
||||
export function selectMessagesForSummary(
|
||||
messages: MessageLike[],
|
||||
maxMessages: number
|
||||
): MessageLike[] {
|
||||
const validMessages = messages.filter((m) => m && typeof m === "object");
|
||||
const system = validMessages.filter(
|
||||
(m) => typeof m.role === "string" && (m.role === "system" || m.role === "developer")
|
||||
@@ -602,6 +605,69 @@ export function shouldGenerateUniversalHandoff(options: {
|
||||
return "generate";
|
||||
}
|
||||
|
||||
// #11552 — universal-handoff regeneration backoff.
|
||||
//
|
||||
// A universal handoff whose summary comes back unparseable persists NOTHING, so
|
||||
// `shouldGenerateUniversalHandoff` keeps answering "generate" and the very next
|
||||
// model switch in the same session re-issues the same full-history
|
||||
// summarization call and throws the answer away again. With a switch-heavy
|
||||
// strategy (weighted, random, round-robin, p2c) the models alternate on almost
|
||||
// every turn, so that becomes an extra discarded upstream call on a large
|
||||
// fraction of requests — real money on a paid provider, real quota on a metered
|
||||
// one. Back off per (session, combo) instead of hammering.
|
||||
//
|
||||
// Scope is deliberately narrow: only the "responded, but the content is not a
|
||||
// usable handoff" outcome is tracked. A transient upstream failure
|
||||
// (`!response.ok`) is NOT — that one is worth retrying on the next switch, and
|
||||
// the context-relay path relies on exactly that behavior
|
||||
// (tests/unit/context-handoff.test.ts → "allows a new attempt after a failed
|
||||
// in-flight generation").
|
||||
const HANDOFF_UNPARSEABLE_BASE_COOLDOWN_MS = 5 * 60 * 1000;
|
||||
const HANDOFF_UNPARSEABLE_MAX_COOLDOWN_MS = 60 * 60 * 1000;
|
||||
const MAX_TRACKED_HANDOFF_COOLDOWNS = 500;
|
||||
|
||||
type HandoffCooldownState = { consecutive: number; retryAfter: number };
|
||||
const universalHandoffCooldowns = new Map<string, HandoffCooldownState>();
|
||||
|
||||
type UniversalHandoffOutcome = "generated" | "unparseable" | "unavailable";
|
||||
|
||||
function isUniversalHandoffCoolingDown(key: string): boolean {
|
||||
const entry = universalHandoffCooldowns.get(key);
|
||||
if (!entry) return false;
|
||||
return Date.now() < entry.retryAfter;
|
||||
}
|
||||
|
||||
function pruneUniversalHandoffCooldowns(): void {
|
||||
if (universalHandoffCooldowns.size <= MAX_TRACKED_HANDOFF_COOLDOWNS) return;
|
||||
const now = Date.now();
|
||||
for (const [key, entry] of universalHandoffCooldowns) {
|
||||
if (entry.retryAfter <= now) universalHandoffCooldowns.delete(key);
|
||||
}
|
||||
// Map iterates in insertion order, so this evicts the least recently touched
|
||||
// keys first (every record re-inserts its key at the tail).
|
||||
while (universalHandoffCooldowns.size > MAX_TRACKED_HANDOFF_COOLDOWNS) {
|
||||
const oldest = universalHandoffCooldowns.keys().next();
|
||||
if (oldest.done) break;
|
||||
universalHandoffCooldowns.delete(oldest.value);
|
||||
}
|
||||
}
|
||||
|
||||
function recordUniversalHandoffUnparseable(key: string): void {
|
||||
const consecutive = (universalHandoffCooldowns.get(key)?.consecutive ?? 0) + 1;
|
||||
const cooldownMs = Math.min(
|
||||
HANDOFF_UNPARSEABLE_BASE_COOLDOWN_MS * 2 ** (consecutive - 1),
|
||||
HANDOFF_UNPARSEABLE_MAX_COOLDOWN_MS
|
||||
);
|
||||
universalHandoffCooldowns.delete(key);
|
||||
universalHandoffCooldowns.set(key, { consecutive, retryAfter: Date.now() + cooldownMs });
|
||||
pruneUniversalHandoffCooldowns();
|
||||
}
|
||||
|
||||
/** Test seam: drop all universal-handoff regeneration cooldowns. */
|
||||
export function resetUniversalHandoffCooldowns(): void {
|
||||
universalHandoffCooldowns.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a universal handoff summary for any model/provider switch.
|
||||
*/
|
||||
@@ -616,13 +682,13 @@ async function generateUniversalHandoffAsync(options: {
|
||||
maxMessages: number;
|
||||
providerAllowlist: string[];
|
||||
handleSingleModel: (body: Record<string, unknown>, modelStr: string) => Promise<Response>;
|
||||
}): Promise<void> {
|
||||
}): Promise<UniversalHandoffOutcome> {
|
||||
const selectedMessages = selectMessagesForSummary(
|
||||
Array.isArray(options.messages) ? options.messages : [],
|
||||
options.maxMessages
|
||||
);
|
||||
const historyText = formatMessagesForPrompt(selectedMessages);
|
||||
if (!historyText) return;
|
||||
if (!historyText) return "unavailable";
|
||||
|
||||
const summaryPrompt = HANDOFF_PROMPT_TEMPLATE.replace("{HISTORY}", historyText);
|
||||
const summaryModel = options.handoffModel || options.currModel;
|
||||
@@ -637,7 +703,7 @@ async function generateUniversalHandoffAsync(options: {
|
||||
};
|
||||
|
||||
const response = await options.handleSingleModel(summaryBody, summaryModel);
|
||||
if (!response.ok) return;
|
||||
if (!response.ok) return "unavailable";
|
||||
|
||||
let content = "";
|
||||
try {
|
||||
@@ -652,7 +718,7 @@ async function generateUniversalHandoffAsync(options: {
|
||||
}
|
||||
|
||||
const parsed = parseHandoffJSON(content);
|
||||
if (!parsed) return;
|
||||
if (!parsed) return "unparseable";
|
||||
|
||||
upsertHandoff({
|
||||
sessionId: options.sessionId,
|
||||
@@ -669,6 +735,7 @@ async function generateUniversalHandoffAsync(options: {
|
||||
generatedAt: new Date().toISOString(),
|
||||
expiresAt: new Date(Date.now() + options.ttlMs).toISOString(),
|
||||
});
|
||||
return "generated";
|
||||
}
|
||||
|
||||
export function maybeGenerateUniversalHandoff(options: {
|
||||
@@ -692,6 +759,10 @@ export function maybeGenerateUniversalHandoff(options: {
|
||||
if (!options.sessionId) return;
|
||||
|
||||
const inflightKey = getInflightKey(options.sessionId, options.comboName);
|
||||
// #11552: the previous attempt for this session/combo answered with something
|
||||
// that is not a usable handoff. Re-asking on every model switch just burns an
|
||||
// upstream call whose response is discarded — wait out the backoff instead.
|
||||
if (isUniversalHandoffCoolingDown(inflightKey)) return;
|
||||
if (inflightHandoffGenerations.has(inflightKey)) return;
|
||||
inflightHandoffGenerations.add(inflightKey);
|
||||
|
||||
@@ -710,6 +781,10 @@ export function maybeGenerateUniversalHandoff(options: {
|
||||
providerAllowlist: options.universalConfig.providerAllowlist,
|
||||
handleSingleModel: options.handleSingleModel,
|
||||
})
|
||||
.then((outcome) => {
|
||||
if (outcome === "unparseable") recordUniversalHandoffUnparseable(inflightKey);
|
||||
else if (outcome === "generated") universalHandoffCooldowns.delete(inflightKey);
|
||||
})
|
||||
.catch((err) => {
|
||||
if (process.env.NODE_ENV !== "test") {
|
||||
console.warn("[universal-handoff] Generation failed:", err?.message || err);
|
||||
|
||||
@@ -420,3 +420,106 @@ test("selectMessagesForSummary with no system messages and oversized single rema
|
||||
.join("\n\n");
|
||||
assert.ok(historyText.length > 0, "historyText must be non-empty so the handoff is generated");
|
||||
});
|
||||
|
||||
// ── #11552: universal-handoff regeneration backoff ───────────────────────────
|
||||
// A switch-heavy combo strategy (weighted / random / round-robin) alternates
|
||||
// models on almost every turn, so `maybeGenerateUniversalHandoff` is consulted
|
||||
// constantly. When the summarizer answers with something that is not a usable
|
||||
// handoff, nothing is persisted — and before the fix the very next switch
|
||||
// re-issued the same full-history summarization call and discarded the answer
|
||||
// again, on and on. That is the extra upstream call issue #11552 measured.
|
||||
|
||||
function universalHandoffOptions(sessionId, handleSingleModel) {
|
||||
return {
|
||||
sessionId,
|
||||
comboName: "weighted-combo",
|
||||
messages: [{ role: "user", content: "Ship the weighted combo fix" }],
|
||||
prevModel: "openai/gpt-4o-mini",
|
||||
currModel: "claude/claude-3-5-sonnet-20241022",
|
||||
universalConfig: contextHandoff.resolveUniversalHandoffConfig(null, null),
|
||||
handleSingleModel,
|
||||
};
|
||||
}
|
||||
|
||||
function handoffJSONResponse(summary) {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
choices: [
|
||||
{
|
||||
message: {
|
||||
content: JSON.stringify({
|
||||
summary,
|
||||
keyDecisions: ["backoff on unparseable handoffs"],
|
||||
taskProgress: "done",
|
||||
activeEntities: ["contextHandoff.ts"],
|
||||
}),
|
||||
},
|
||||
},
|
||||
],
|
||||
}),
|
||||
{ status: 200, headers: { "content-type": "application/json" } }
|
||||
);
|
||||
}
|
||||
|
||||
test("maybeGenerateUniversalHandoff stops re-summarizing after an unparseable answer", async () => {
|
||||
contextHandoff.resetUniversalHandoffCooldowns();
|
||||
let calls = 0;
|
||||
// 200 upstream OK responses that carry no handoff JSON — exactly what the
|
||||
// weighted combo matrix sees.
|
||||
const options = universalHandoffOptions("sess-unparseable", async () => {
|
||||
calls += 1;
|
||||
return new Response(JSON.stringify({ choices: [{ message: { content: "ok" } }] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
});
|
||||
|
||||
for (let i = 0; i < 200; i++) {
|
||||
contextHandoff.maybeGenerateUniversalHandoff(options);
|
||||
await new Promise((resolve) => setTimeout(resolve, 1));
|
||||
}
|
||||
|
||||
// Positive anchor: the feature still runs — the first switch DID generate.
|
||||
assert.equal(calls, 1, `expected exactly one summarization call, got ${calls}`);
|
||||
assert.equal(handoffDb.getHandoff("sess-unparseable", "weighted-combo"), null);
|
||||
});
|
||||
|
||||
test("maybeGenerateUniversalHandoff still generates and persists a usable handoff", async () => {
|
||||
contextHandoff.resetUniversalHandoffCooldowns();
|
||||
let calls = 0;
|
||||
const options = universalHandoffOptions("sess-usable", async () => {
|
||||
calls += 1;
|
||||
return handoffJSONResponse("Weighted combo handoff");
|
||||
});
|
||||
|
||||
contextHandoff.maybeGenerateUniversalHandoff(options);
|
||||
const saved = await waitFor(() => handoffDb.getHandoff("sess-usable", "weighted-combo"));
|
||||
assert.ok(saved, "a parseable summary must still be persisted");
|
||||
assert.equal(saved.summary, "Weighted combo handoff");
|
||||
assert.equal(calls, 1);
|
||||
|
||||
// A persisted handoff makes the next switch "inject", not "generate".
|
||||
contextHandoff.maybeGenerateUniversalHandoff(options);
|
||||
await new Promise((resolve) => setTimeout(resolve, 40));
|
||||
assert.equal(calls, 1);
|
||||
});
|
||||
|
||||
test("a transient upstream failure does not arm the unparseable backoff", async () => {
|
||||
contextHandoff.resetUniversalHandoffCooldowns();
|
||||
let calls = 0;
|
||||
const options = universalHandoffOptions("sess-transient", async () => {
|
||||
calls += 1;
|
||||
if (calls === 1) return new Response("upstream down", { status: 503 });
|
||||
return handoffJSONResponse("Recovered handoff");
|
||||
});
|
||||
|
||||
contextHandoff.maybeGenerateUniversalHandoff(options);
|
||||
await new Promise((resolve) => setTimeout(resolve, 40));
|
||||
assert.equal(handoffDb.getHandoff("sess-transient", "weighted-combo"), null);
|
||||
|
||||
contextHandoff.maybeGenerateUniversalHandoff(options);
|
||||
const saved = await waitFor(() => handoffDb.getHandoff("sess-transient", "weighted-combo"));
|
||||
assert.ok(saved, "a 503 must stay retryable on the next model switch");
|
||||
assert.equal(saved.summary, "Recovered handoff");
|
||||
assert.equal(calls, 2);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user