From 40b1d94cbd347f14883779774876549b97bb5460 Mon Sep 17 00:00:00 2001 From: ikelvingo Date: Sat, 25 Jul 2026 12:54:27 -0300 Subject: [PATCH] fix(compression): restore auto-combo scoring/routing features dropped by stale rebase PR #8438 (session-dedup OOM-guard rewrite) was built against a stale local copy of scoring.ts/autoStrategy.ts predating #8008 and #7301: merging it as-is silently deleted the cacheAffinity scoring factor + normalizeScoringWeights (#8008), the cached getCachedProviderConnections read path, and the #COMBO-REF candidate-pool guard (#7301), breaking the TypeScript build. Restore all three while keeping the PR's own legitimate additions (computePoolMaxima/PoolMaxima pool-maxima memoization). Also fix a real perf regression the PR introduced in session-dedup's single-message intra-dedup path: dedupeWithinMessage dynamically built and ran a global RegExp per candidate suffix block to count/replace occurrences, which is pathologically slow on large single-message inputs (the exact "huge tool result" shape the #7849 O(n^2) guard targets). Replace it with indexOf-based counting/replacement (same semantics, no regex compile/match overhead). Update the #7849 regression suite's assertions to match the new MAX_SUFFIX_STARTS/MAX_TOTAL_BLOCK_BYTES bound (which truncates suffix-block enumeration silently/best-effort instead of failing the whole request open with the old "suffix work budget exceeded" warning) instead of asserting on the removed shared-budget mechanism. Co-authored-by: diegosouzapw --- open-sse/services/autoCombo/scoring.ts | 26 +++++++ open-sse/services/combo/autoStrategy.ts | 15 +++- .../engines/session-dedup/index.ts | 71 +++++++++++++------ .../session-dedup-memory-7849.test.ts | 46 +++++++----- 4 files changed, 117 insertions(+), 41 deletions(-) diff --git a/open-sse/services/autoCombo/scoring.ts b/open-sse/services/autoCombo/scoring.ts index 2012319874..a8758df00c 100644 --- a/open-sse/services/autoCombo/scoring.ts +++ b/open-sse/services/autoCombo/scoring.ts @@ -19,6 +19,7 @@ export interface ScoringFactors { tierAffinity: number; specificityMatch: number; contextAffinity: number; + cacheAffinity?: number; resetWindowAffinity: number; connectionDensity: number; } @@ -34,6 +35,7 @@ export interface ScoringWeights { tierAffinity: number; specificityMatch: number; contextAffinity: number; + cacheAffinity?: number; resetWindowAffinity: number; connectionDensity: number; } @@ -49,10 +51,30 @@ export const DEFAULT_WEIGHTS: ScoringWeights = { tierAffinity: 0.05, specificityMatch: 0.05, contextAffinity: 0.05, + cacheAffinity: 0, resetWindowAffinity: 0, connectionDensity: 0.05, }; +/** Normalize independently configured UI weights into a scoring distribution. */ +export function normalizeScoringWeights( + weights: Partial | null | undefined +): ScoringWeights { + if (!weights) return { ...DEFAULT_WEIGHTS }; + const entries = Object.keys(DEFAULT_WEIGHTS) as Array; + const sanitized = Object.fromEntries( + entries.map((key) => { + const value = Number(weights?.[key]); + return [key, Number.isFinite(value) && value >= 0 ? value : 0]; + }) + ) as unknown as ScoringWeights; + const total = entries.reduce((sum, key) => sum + Number(sanitized[key] ?? 0), 0); + if (total <= 0) return { ...DEFAULT_WEIGHTS }; + return Object.fromEntries( + entries.map((key) => [key, Number(sanitized[key] ?? 0) / total]) + ) as unknown as ScoringWeights; +} + export interface ProviderCandidate { provider: string; model: string; @@ -77,6 +99,8 @@ export interface ProviderCandidate { quotaResetIntervalSecs?: number; /** Score [0..1] for staying on the current session's provider/account/model path. */ contextAffinity?: number; + /** Score [0..1] for the account selected by the stable prompt-cache key. */ + cacheAffinity?: number; /** Score [0..1] for quota reset-window preference; sooner selected reset windows score higher. */ resetWindowAffinity?: number; connectionPoolSize?: number; @@ -110,6 +134,7 @@ export function calculateScore(factors: ScoringFactors, weights: ScoringWeights) (weights.tierAffinity ?? 0) * factors.tierAffinity + (weights.specificityMatch ?? 0) * factors.specificityMatch + (weights.contextAffinity ?? 0) * factors.contextAffinity + + (weights.cacheAffinity ?? 0) * (factors.cacheAffinity ?? 0) + (weights.resetWindowAffinity ?? 0) * factors.resetWindowAffinity + (weights.connectionDensity ?? 0) * factors.connectionDensity ); @@ -234,6 +259,7 @@ export function calculateFactors( tierAffinity: calculateTierAffinity(candidate, manifestHint), specificityMatch: calculateSpecificityMatch(candidate, manifestHint), contextAffinity: clamp01(candidate.contextAffinity ?? 0.5), + cacheAffinity: clamp01(candidate.cacheAffinity ?? 0), resetWindowAffinity: clamp01(candidate.resetWindowAffinity ?? 0.5), connectionDensity: clamp01(((candidate.connectionPoolSize ?? 1) - 1) / 10), }; diff --git a/open-sse/services/combo/autoStrategy.ts b/open-sse/services/combo/autoStrategy.ts index 1e4a472740..8683ca9450 100644 --- a/open-sse/services/combo/autoStrategy.ts +++ b/open-sse/services/combo/autoStrategy.ts @@ -38,7 +38,7 @@ import { type ScoringWeights, } from "../autoCombo/scoring.ts"; import type { RoutingHint } from "../manifestAdapter"; -import { getProviderConnections } from "../../../src/lib/db/providers"; +import { getCachedProviderConnections } from "../../../src/lib/db/readCache"; import { getProviderModels } from "../../config/providerModels.ts"; import { getConnectionRoutingTags, @@ -249,7 +249,7 @@ export async function applyRequestTagRouting( await Promise.all( providerIds.map(async (providerId) => { try { - const connections = await getProviderConnections({ provider: providerId, isActive: true }); + const connections = await getCachedProviderConnections({ provider: providerId, isActive: true }); providerConnections.set( providerId, Array.isArray(connections) ? (connections as Array>) : [] @@ -426,8 +426,17 @@ export async function expandAutoComboCandidatePool( if (Array.isArray(localAutoConfig?.candidatePool) && localAutoConfig.candidatePool.length > 0) return eligibleTargets; + // #COMBO-REF: if the combo references other combos via kind:"combo-ref" entries, + // the resolved eligibleTargets already represent the operator's intended pool. + // Expanding to ALL providers would defeat the purpose of the combo-ref constraint + // (e.g. an "auto" combo delegating to a "priority" sub-combo should not pull in + // every model from every active provider). + const rawModels = (combo as Record | null | undefined)?.models; + if (Array.isArray(rawModels) && rawModels.some((m) => isRecord(m) && m.kind === "combo-ref")) + return eligibleTargets; + try { - const allConnections = await getProviderConnections({ isActive: true }); + const allConnections = await getCachedProviderConnections({ isActive: true }); const providerIds = [ ...new Set( (allConnections as Array<{ provider?: unknown }>) diff --git a/open-sse/services/compression/engines/session-dedup/index.ts b/open-sse/services/compression/engines/session-dedup/index.ts index 67424e1662..a7d4916b5d 100644 --- a/open-sse/services/compression/engines/session-dedup/index.ts +++ b/open-sse/services/compression/engines/session-dedup/index.ts @@ -109,6 +109,47 @@ function findSuffixBlocks( // ─── two-pass dedup on message texts ───────────────────────────────────────── +/** + * Replace every occurrence of `needle` in `haystack` after the first with + * `marker`, keeping the first occurrence intact. Returns the new string and + * how many replacements were made. + * + * Uses plain `indexOf`/slice instead of a dynamically-built global RegExp: + * escaping an up-to-multi-hundred-KB literal block into a RegExp and running + * it (once per candidate block, once for counting + once for replacing) is + * pathologically slow on large single-message inputs — the exact + * multi-thousand-line "huge tool result" shape the O(n²) guard above targets + * (#OOM incident) — so a single call could still take tens of seconds even + * though findSuffixBlocks itself is bounded. indexOf-based scanning is O(n) + * per pass and has no compile step. + */ +function replaceAllButFirst( + haystack: string, + needle: string, + marker: string +): { result: string; occurrences: number } { + if (needle.length === 0) return { result: haystack, occurrences: 0 }; + + const firstIdx = haystack.indexOf(needle); + if (firstIdx === -1) return { result: haystack, occurrences: 0 }; + + let occurrences = 1; + let searchFrom = firstIdx + needle.length; + let result = haystack.slice(0, searchFrom); + let cursor = searchFrom; + + for (;;) { + const idx = haystack.indexOf(needle, cursor); + if (idx === -1) break; + occurrences++; + result += haystack.slice(cursor, idx) + marker; + cursor = idx + needle.length; + } + result += haystack.slice(cursor); + + return { result, occurrences }; +} + /** * Deduplicates repeated lines within a single message (intra-message dedup). * Replaces repeated suffix blocks with markers. @@ -122,34 +163,22 @@ function dedupeWithinMessage( if (blocks.length < 2) return { deduped: text, changed: false }; - // Find the most common block (likely candidate for intra-message dedup). - const blockFreq = new Map(); - for (const { block } of blocks) { - blockFreq.set(block, (blockFreq.get(block) || 0) + 1); - } - - // Sort by frequency descending, then by length descending (prefer replacing more common, longer blocks first). - const sortedBlocks = [...blocks].sort((a, b) => { - const freqDiff = (blockFreq.get(b.block) || 0) - (blockFreq.get(a.block) || 0); - return freqDiff !== 0 ? freqDiff : b.block.length - a.block.length; - }); + // findSuffixBlocks already de-duplicates by exact block content (its `seen` + // set), so every entry here is already frequency-1 by construction. Sort by + // length descending so the longest candidate blocks are tried first. + const sortedBlocks = [...blocks].sort((a, b) => b.block.length - a.block.length); let result = text; let changed = false; for (const { block } of sortedBlocks) { - // Only dedup blocks that appear 2+ times in the text. - const occurrences = (result.match(new RegExp(block.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "g")) || []).length; - if (occurrences < 2) continue; - const sha = hashBlock(block); const marker = `[dedup:ref sha=${sha}]`; - // Replace ALL occurrences except the first (keep the original once). - let count = 0; - result = result.replace(new RegExp(block.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "g"), () => { - count++; - return count === 1 ? block : marker; - }); + // Only dedup blocks that appear 2+ times in the text; keep the first + // occurrence intact and replace the rest. + const { result: replaced, occurrences } = replaceAllButFirst(result, block, marker); + if (occurrences < 2) continue; + result = replaced; changed = true; } diff --git a/tests/unit/compression/session-dedup-memory-7849.test.ts b/tests/unit/compression/session-dedup-memory-7849.test.ts index 3a64606603..cc3e6cbaea 100644 --- a/tests/unit/compression/session-dedup-memory-7849.test.ts +++ b/tests/unit/compression/session-dedup-memory-7849.test.ts @@ -9,7 +9,14 @@ import { sessionDedupEngine } from "../../../open-sse/services/compression/engin const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), "../../.."); const FIXTURE = join(REPO_ROOT, "tests/fixtures/compression/session-dedup-memory-7849.ts"); const SUFFIX_WORK_BUDGET = 32 * 1024 * 1024; -const SUFFIX_WORK_BUDGET_WARNING = "session-dedup: skipped (suffix work budget exceeded)"; +// The original #7849 fix used a shared cross-message "suffix work" char budget +// that failed the whole request open (with this warning) once exceeded. PR +// #8438 replaced that mechanism with a simpler, per-message bound +// (MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES in session-dedup/index.ts) that +// silently truncates suffix-block enumeration instead of failing the request +// open — dedup is best-effort, so truncating never changes output +// correctness, it only forgoes some compression. There is no longer an +// equivalent "budget exceeded" warning for inputs of this size. function makeFixedWidthText(lineCount: number, lineChars: number, tag: string): string { return Array.from({ length: lineCount }, (_, index) => { @@ -38,7 +45,7 @@ function makeSharedBudgetBody(): Record { }; } -test("#7849: shares the two-pass suffix-work budget across all messages", () => { +test("#7849: a shape that used to exceed the shared suffix-work budget now completes cleanly under the per-message bound", () => { const body = makeSharedBudgetBody(); const messages = body.messages as Array<{ content: string }>; const perMessageWork = messages.map(({ content }) => projectedSuffixWork(content, 2)); @@ -47,14 +54,9 @@ test("#7849: shares the two-pass suffix-work budget across all messages", () => perMessageWork.every((work) => work < SUFFIX_WORK_BUDGET), "each message must fit the two-pass budget on its own" ); - assert.ok( - messages.reduce((total, { content }) => total + projectedSuffixWork(content, 1), 0) < - SUFFIX_WORK_BUDGET, - "the pair must fit if incorrectly charged for only one pass" - ); assert.ok( perMessageWork.reduce((total, work) => total + work, 0) > SUFFIX_WORK_BUDGET, - "the pair must exceed the shared budget when correctly charged for two passes" + "the pair would have exceeded the old shared budget when charged for two passes" ); for (const message of messages) { @@ -64,20 +66,23 @@ test("#7849: shares the two-pass suffix-work budget across all messages", () => assert.equal(individualResult.stats, null, "each message must be accepted individually"); } + // The two messages use distinct tags ("first"/"second"), so they share no + // duplicate content — under the new per-message MAX_SUFFIX_STARTS / + // MAX_TOTAL_BLOCK_BYTES bound (well within budget at this size), the engine + // finds nothing to dedup and returns the body unchanged, with no warnings. const result = sessionDedupEngine.apply(body); - assert.deepEqual(result.stats?.validationWarnings, [SUFFIX_WORK_BUDGET_WARNING]); + assert.strictEqual(result.body, body, "no duplicates found: body must be returned by identity"); + assert.equal(result.compressed, false); + assert.equal(result.stats, null); }); -test("#7849: exhausted suffix-work budget fails open with exact zero-savings stats", () => { +test("#7849: the previously budget-exhausting shape produces no false-positive compression or warnings", () => { const body = makeSharedBudgetBody(); const result = sessionDedupEngine.apply(body); - assert.strictEqual(result.body, body, "budget exhaustion must return the input body by identity"); + assert.strictEqual(result.body, body, "no duplicates found: body must be returned by identity"); assert.equal(result.compressed, false); - assert.ok(result.stats, "budget exhaustion must return explanatory stats"); - assert.equal(result.stats.originalTokens, result.stats.compressedTokens); - assert.equal(result.stats.savingsPercent, 0); - assert.deepEqual(result.stats.validationWarnings, [SUFFIX_WORK_BUDGET_WARNING]); + assert.equal(result.stats, null, "no dedup work occurred, so no stats/warnings are produced"); }); test("#7849: near-boundary under-budget request still deduplicates", () => { @@ -130,9 +135,16 @@ test( warnings: string[]; }; assert.deepEqual(output.enginesRun, ["session-dedup", "lite", "rtk", "headroom", "caveman"]); + // The fixture's lines are all unique (no repeated content), so under the + // new per-message MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES bound + // session-dedup finds nothing to dedup and reports "no eligible content" — + // there is no longer a distinct "suffix work budget exceeded" warning. + // The regression this test guards against is the O(n²) OOM/hang itself + // (asserted above via `child.status === 0` within the heap/time budget), + // not this specific warning string. assert.ok( - output.warnings.includes("session-dedup: skipped (suffix work budget exceeded)"), - `expected an explicit session-dedup work-budget warning, got ${JSON.stringify(output.warnings)}` + output.warnings.includes("session-dedup: skipped (no eligible content)"), + `expected session-dedup to report no eligible content, got ${JSON.stringify(output.warnings)}` ); } );