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 <diegosouza.pw@gmail.com>
This commit is contained in:
ikelvingo
2026-07-25 12:54:27 -03:00
parent c1f1e3d9c4
commit 40b1d94cbd
4 changed files with 117 additions and 41 deletions

View File

@@ -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<ScoringWeights> | null | undefined
): ScoringWeights {
if (!weights) return { ...DEFAULT_WEIGHTS };
const entries = Object.keys(DEFAULT_WEIGHTS) as Array<keyof ScoringWeights>;
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),
};

View File

@@ -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<Record<string, unknown>>) : []
@@ -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<string, unknown> | 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 }>)

View File

@@ -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<string, number>();
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;
}

View File

@@ -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<string, unknown> {
};
}
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)}`
);
}
);