pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-08-14 03:02:14 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(combo): decouple failoverBeforeRetry same-model guard from the skipUpstreamRetry default

Browse Source 
Audit found that DEFAULT_COMBO_CONFIG.failoverBeforeRetry has defaulted to
true since before #10217 (predates #2417), and that value also feeds the
independent skipUpstreamRetry mechanism (src/sse/handlers/chat.ts:859,1126).
The previous commit on this branch flipped that default to false to fix the
#10217 same-model retry guard, which silently disabled skipUpstreamRetry's
own default-on behavior for every combo without an opt-in — a regression in
the opposite direction (executor-level retries before the loop's own
failover, changing latency/failure behavior).

Revert the default back to true and decouple the two mechanisms instead:
resolveComboConfig/resolveComboSetupConfig now also compute
failoverBeforeRetryExplicit, true only when a cascade layer (combo/provider/
global) literally sets failoverBeforeRetry to true — not merely inherited
from the default. The #10217 same-model retry guards in combo.ts (priority/
auto and round-robin loops) now read failoverBeforeRetryExplicit instead of
config.failoverBeforeRetry, restoring opt-in-only behavior for that guard
while the skipUpstreamRetry pass-through (config.failoverBeforeRetry at
combo.ts:1297,2865) is untouched and keeps its historical default-on.
This commit is contained in:
diegosouzapw
2026-08-13 14:19:34 -03:00
committed by Xiangzhe
parent f2b9c40590
commit 104afeda4e
2 changed files with 67 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
open-sse/services/combo.ts
View File

@@ -1919,11 +1919,18 @@ export async function handleComboChat({
// skip the same-model retry when `nextTarget` (computed above)
// actually gives us somewhere to fail over to — with no sibling
// left, skipping just burns the last attempt for nothing.
//
// #10217 round-4 fix: this guard reads `failoverBeforeRetryExplicit`
// (opt-in only), NOT `config.failoverBeforeRetry` — that field
// defaults to true for the separate skipUpstreamRetry mechanism
// (see DEFAULT_COMBO_CONFIG comment in comboConfig.ts) and reading
// it here would silently skip the same-model retry for every combo,
// not just ones that explicitly opted in.
if (
retry < maxRetries &&
isTransient &&
!providerExhausted &&
(!config.failoverBeforeRetry || !nextTarget)
(!config.failoverBeforeRetryExplicit || !nextTarget)
) {
if (
!protectedPriorityTarget &&
@@ -2438,7 +2445,15 @@ async function handleRoundRobinCombo({
}: HandleRoundRobinOptions): Promise<Response> {
const config = settings
? resolveComboConfig(combo, settings)
: { ...getDefaultComboConfig(), ...(combo.config || {}) };
: {
...getDefaultComboConfig(),
...(combo.config || {}),
// See resolveComboConfig's failoverBeforeRetryExplicit comment in
// comboConfig.ts (no `settings` here, so only the combo's own config
// can opt in).
failoverBeforeRetryExplicit:
(combo.config as Record<string, unknown> | undefined)?.failoverBeforeRetry === true,
};
// #9158: clamp combo-level concurrency to a sane bound — a config carrying a
// huge or negative value would otherwise open an unbounded semaphore and
// flood targets (or deadlock at 0).
@@ -3163,12 +3178,14 @@ async function handleRoundRobinCombo({
// just the lower-level skipUpstreamRetry mechanism. Only skip when
// `offset + 1 < modelCount` means a sibling target is actually left
// in this rotation; with none left, skipping just wastes the attempt.
// #10217 round-4 fix: opt-in only — read failoverBeforeRetryExplicit,
// not config.failoverBeforeRetry (see comboConfig.ts comment).
const hasNextRrTarget = offset + 1 < modelCount;
if (
retry < maxRetries &&
isTransient &&
!providerExhausted &&
(!config.failoverBeforeRetry || !hasNextRrTarget)
(!config.failoverBeforeRetryExplicit || !hasNextRrTarget)
) {
continue;
}

59
open-sse/services/comboConfig.ts
View File

@@ -126,12 +126,23 @@ const DEFAULT_COMBO_CONFIG = {
resetAwareWeeklyWeight: 0.65,
resetAwareTieBandPercent: 5,
resetAwareExhaustionGuardPercent: 10,
// Opt-in (#2417/#10217): when unset, transient errors retry the same model
// up to maxRetries before falling over to the next target — the historical
// default. Defaulting this to true silently flipped that behavior for every
// combo that never touched the setting, breaking same-model retry semantics
// (round 4 base-red bisect: 06f41cda63 vs d2fd88dfbc).
failoverBeforeRetry: false,
// Historical default (predates #2417/#10217) — true. This value feeds TWO
// independent mechanisms and must stay true-by-default for one of them:
// 1. skipUpstreamRetry (src/sse/handlers/chat.ts:859,1126) — the
// lower-level executor retry skip. Always default-on; changing this
// default flips that mechanism's behavior for every combo, not just
// opted-in ones.
// 2. The #10217 same-model retry guard in this file's combo.ts callers
// (priority/auto + round-robin loops) — meant to be OPT-IN only. That
// guard must NOT read this field directly; it consults the sibling
// `failoverBeforeRetryExplicit` flag computed below in
// resolveComboConfig/resolveComboSetupConfig, which is true only when
// an actual cascade layer (combo/provider/global) set the flag to
// true, not merely inherited from this default. See round-4 base-red
// bisect (06f41cda63 vs d2fd88dfbc) — flipping THIS default to false
// "fixed" mechanism 2 but silently broke mechanism 1's default-on
// behavior for every combo without an explicit opt-in.
failoverBeforeRetry: true,
// Feature 4985: configurable response-body validation predicate (per-combo). When set,
// a 200 OK whose body fails the predicate fails over to the next target.
responseValidation: undefined as ResponseValidationConfig | undefined,
@@ -289,15 +300,32 @@ export function resolveComboConfig(
)
);
const cleanGlobal = clean(global);
const cleanProviderOverride = clean(providerOverride);
const cleanComboConfig = clean(comboConfig);
const merged = {
...DEFAULT_COMBO_CONFIG,
...clean(global),
...clean(providerOverride),
...clean(comboConfig),
...cleanGlobal,
...cleanProviderOverride,
...cleanComboConfig,
};
// #10217 round-4 fix: `failoverBeforeRetry` defaults to true (see comment on
// DEFAULT_COMBO_CONFIG above) and feeds two independent mechanisms. Callers
// that gate the OPT-IN same-model retry guard (combo.ts) must NOT read
// `merged.failoverBeforeRetry` directly — that stays true unless a layer
// explicitly disables it, which can't distinguish "inherited default" from
// "operator opted in". This flag is true only when some cascade layer
// literally set the value to true, i.e. a genuine opt-in.
const failoverBeforeRetryExplicit =
cleanComboConfig.failoverBeforeRetry === true ||
cleanProviderOverride.failoverBeforeRetry === true ||
cleanGlobal.failoverBeforeRetry === true;
return {
...merged,
failoverBeforeRetryExplicit,
shadowRouting: {
...DEFAULT_COMBO_CONFIG.shadowRouting,
...(isRecord(global.shadowRouting) ? clean(global.shadowRouting) : {}),
@@ -327,7 +355,14 @@ export function getDefaultComboConfig() {
* return type is the single source of truth for ComboContext.config (combo/context.ts).
*/
export function resolveComboSetupConfig(combo: ComboConfigLike, settings: ComboSettingsLike) {
return settings
? resolveComboConfig(combo, settings)
: { ...getDefaultComboConfig(), ...((combo?.config as Record<string, unknown>) || {}) };
if (settings) return resolveComboConfig(combo, settings);
const comboConfig = (combo?.config as Record<string, unknown>) || {};
return {
...getDefaultComboConfig(),
...comboConfig,
// See resolveComboConfig's failoverBeforeRetryExplicit comment — same
// distinction applies here (no `settings`, so only the combo's own config
// can opt in).
failoverBeforeRetryExplicit: comboConfig.failoverBeforeRetry === true,
};
}