mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-18 05:02:15 +03:00
fix(combo): keep operator-defined model order for deterministic strategies (#9420)
Merge-train validated (tip 6ce4effef8). Vitest failures confirmed as base-red (#9679).
This commit is contained in:
@@ -205,7 +205,7 @@ export async function applyStrategyOrdering(
|
||||
if (resolvePromptCacheAffinityKey(body)) {
|
||||
orderedTargets = await expandPromptCacheAffinityTargets(orderedTargets);
|
||||
}
|
||||
const affinity = applyPromptCacheAffinity(orderedTargets, body);
|
||||
const affinity = applyPromptCacheAffinity(orderedTargets, body, true, "global");
|
||||
orderedTargets = affinity.targets;
|
||||
log.info(
|
||||
"COMBO",
|
||||
|
||||
@@ -266,14 +266,32 @@ export function shouldProtectOriginalFirst(
|
||||
}
|
||||
|
||||
/**
|
||||
* Order eligible targets using rendezvous hashing. The original order is used
|
||||
* as the final tie-breaker, so targets sharing one account identity remain
|
||||
* stable without using modelStr as the affinity identity.
|
||||
* Extract the base model identity from a target's executionKey or modelStr.
|
||||
* This strips any per-connection suffix (@connectionId) to identify the model itself.
|
||||
*/
|
||||
function getBaseModelIdentity(target: ResolvedComboTarget): string {
|
||||
// executionKey format: "stepId@connectionId" when expanded, or just "stepId"
|
||||
const executionKey = target.executionKey || "";
|
||||
const baseExecutionKey = executionKey.split("@")[0];
|
||||
|
||||
// modelStr format: "provider/model" or "provider/model:version"
|
||||
const modelStr = target.modelStr || "";
|
||||
|
||||
// Use executionKey as primary (preserves stepId grouping), fall back to modelStr
|
||||
return baseExecutionKey || modelStr;
|
||||
}
|
||||
|
||||
/**
|
||||
* Order eligible targets using rendezvous hashing.
|
||||
* @param scope - "model": sort only within same-model groups, preserving inter-model order;
|
||||
* "global": sort across all targets (original behavior).
|
||||
* Defaults to "global" for backward compatibility.
|
||||
*/
|
||||
export function applyPromptCacheAffinity(
|
||||
targets: ResolvedComboTarget[],
|
||||
body: Record<string, unknown> | null | undefined,
|
||||
enabled: boolean = true
|
||||
enabled: boolean = true,
|
||||
scope: "model" | "global" = "global"
|
||||
): PromptCacheAffinityResult {
|
||||
const resolution = enabled ? resolvePromptCacheAffinityKey(body) : null;
|
||||
if (!resolution || targets.length <= 1) {
|
||||
@@ -290,19 +308,61 @@ export function applyPromptCacheAffinity(
|
||||
index,
|
||||
identity: promptCacheTargetIdentity(target),
|
||||
score: rendezvousScore(resolution.key, promptCacheTargetIdentity(target)),
|
||||
baseModel: scope === "model" ? getBaseModelIdentity(target) : null,
|
||||
}));
|
||||
|
||||
ranked.sort((a, b) => {
|
||||
if (a.score > b.score) return -1;
|
||||
if (a.score < b.score) return 1;
|
||||
const identityOrder = a.identity.localeCompare(b.identity);
|
||||
return identityOrder !== 0 ? identityOrder : a.index - b.index;
|
||||
});
|
||||
if (scope === "model") {
|
||||
// Group by base model identity, preserving original group order
|
||||
const groups = new Map<string, typeof ranked>();
|
||||
const groupOrder: string[] = [];
|
||||
|
||||
return {
|
||||
targets: ranked.map((entry) => entry.target),
|
||||
applied: true,
|
||||
source: resolution.source,
|
||||
fingerprint: resolution.fingerprint,
|
||||
};
|
||||
for (const entry of ranked) {
|
||||
// baseModel is guaranteed non-null when scope === "model" (see map above)
|
||||
const baseModel = entry.baseModel as string;
|
||||
if (!groups.has(baseModel)) {
|
||||
groups.set(baseModel, []);
|
||||
groupOrder.push(baseModel);
|
||||
}
|
||||
groups.get(baseModel)!.push(entry);
|
||||
}
|
||||
|
||||
// Sort within each group by score, then identity, then original index
|
||||
const sortedGroups = groupOrder.map((baseModel) => {
|
||||
const group = groups.get(baseModel)!;
|
||||
return group.sort((a, b) => {
|
||||
if (a.score > b.score) return -1;
|
||||
if (a.score < b.score) return 1;
|
||||
const identityOrder = a.identity.localeCompare(b.identity);
|
||||
return identityOrder !== 0 ? identityOrder : a.index - b.index;
|
||||
});
|
||||
});
|
||||
|
||||
// Flatten groups in original order
|
||||
const sortedTargets = sortedGroups.flatMap((group) => group.map((entry) => entry.target));
|
||||
|
||||
// Check if the order actually changed (for applied flag)
|
||||
const orderChanged = !targets.every((target, i) => target === sortedTargets[i]);
|
||||
|
||||
return {
|
||||
targets: sortedTargets,
|
||||
applied: orderChanged, // Only true if the order actually changed
|
||||
source: resolution.source,
|
||||
fingerprint: resolution.fingerprint,
|
||||
};
|
||||
} else {
|
||||
// Original global sorting behavior
|
||||
ranked.sort((a, b) => {
|
||||
if (a.score > b.score) return -1;
|
||||
if (a.score < b.score) return 1;
|
||||
const identityOrder = a.identity.localeCompare(b.identity);
|
||||
return identityOrder !== 0 ? identityOrder : a.index - b.index;
|
||||
});
|
||||
|
||||
return {
|
||||
targets: ranked.map((entry) => entry.target),
|
||||
applied: true,
|
||||
source: resolution.source,
|
||||
fingerprint: resolution.fingerprint,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -658,10 +658,24 @@ async function applyPromptCacheStage(
|
||||
promptCacheAffinityEnabled && resolvePromptCacheAffinityKey(body)
|
||||
? await expandPromptCacheAffinityTargets(orderedTargets)
|
||||
: orderedTargets;
|
||||
|
||||
// Determine affinity scope: restrict to model-level for deterministic strategies
|
||||
// to preserve operator-defined model order; keep global for cross-model
|
||||
// strategies. Per #8370, lkgp/auto/cache-optimized explicitly support promoting
|
||||
// a previously-successful model ahead of the declared order, so they must stay
|
||||
// cross-model ("global") rather than be locked into a single model step.
|
||||
const modelOrderPreservingStrategies = new Set<string>([
|
||||
"priority",
|
||||
"weighted",
|
||||
"fill-first",
|
||||
"quota-share",
|
||||
]);
|
||||
const isDeterministicStrategy = modelOrderPreservingStrategies.has(strategy);
|
||||
const promptCacheAffinity = applyPromptCacheAffinity(
|
||||
promptCacheAffinityTargets,
|
||||
body,
|
||||
promptCacheAffinityEnabled
|
||||
promptCacheAffinityEnabled,
|
||||
isDeterministicStrategy ? "model" : "global"
|
||||
);
|
||||
if (!promptCacheAffinity.applied) return orderedTargets;
|
||||
const protectedOriginal =
|
||||
|
||||
@@ -49,7 +49,7 @@ function applyComboLikeAffinityPin(
|
||||
orderedTargets,
|
||||
connectionsByProvider
|
||||
);
|
||||
const affinity = applyPromptCacheAffinity(expanded, body, true);
|
||||
const affinity = applyPromptCacheAffinity(expanded, body, true, "global");
|
||||
if (!affinity.applied) return affinity.targets;
|
||||
|
||||
const protectedOriginal = shouldProtectOriginalFirst(false, false, strategy) && orderedTargets[0];
|
||||
@@ -176,3 +176,83 @@ test("round-robin combo is NOT protected — it still gets full cross-model affi
|
||||
"round-robin combo must still let prompt-cache affinity pick the winning account across models"
|
||||
);
|
||||
});
|
||||
|
||||
// New test: model-scoped affinity preserves inter-model order
|
||||
function buildModelScopedScenario() {
|
||||
// Three models, each with multiple accounts
|
||||
const orderedTargets = [
|
||||
modelTarget("step-a", "antigravity/gemini-3-pro", "antigravity"),
|
||||
modelTarget("step-b", "ollamacloud/minimax-m3", "ollamacloud"),
|
||||
modelTarget("step-c", "oc/deepseek-v4", "oc"),
|
||||
];
|
||||
const connectionsByProvider = new Map<string, Array<Record<string, unknown>>>([
|
||||
[
|
||||
"antigravity",
|
||||
[{ id: "antigravity-acct-1" }, { id: "antigravity-acct-2" }, { id: "antigravity-acct-3" }],
|
||||
],
|
||||
["ollamacloud", [{ id: "minimax-acct-1" }, { id: "minimax-acct-2" }]],
|
||||
["oc", [{ id: "deepseek-acct-1" }, { id: "deepseek-acct-2" }]],
|
||||
]);
|
||||
return { orderedTargets, connectionsByProvider };
|
||||
}
|
||||
|
||||
test("model-scoped affinity preserves inter-model order while sorting within models", async () => {
|
||||
const { orderedTargets, connectionsByProvider } = buildModelScopedScenario();
|
||||
|
||||
// Find a key that makes deepseek-acct-1 win within its model group
|
||||
const key = "test-key-that-wins-deepseek";
|
||||
const body = { prompt_cache_key: key };
|
||||
|
||||
// Apply model-scoped affinity
|
||||
const expanded = expandPromptCacheAffinityTargetsFromConnections(
|
||||
orderedTargets,
|
||||
connectionsByProvider
|
||||
);
|
||||
|
||||
// Verify global scope still reorders across models
|
||||
const globalAffinity = applyPromptCacheAffinity(expanded, body, true, "global");
|
||||
assert.equal(globalAffinity.applied, true);
|
||||
// The winning account should be from any model (could be deepseek)
|
||||
|
||||
// Apply model-scoped affinity
|
||||
const modelAffinity = applyPromptCacheAffinity(expanded, body, true, "model");
|
||||
assert.equal(modelAffinity.applied, true);
|
||||
|
||||
// Extract base model identities from the result
|
||||
const resultBaseModels = modelAffinity.targets.map((target) => {
|
||||
const executionKey = target.executionKey || "";
|
||||
return executionKey.split("@")[0]; // step-a, step-b, step-c
|
||||
});
|
||||
|
||||
// The first appearance of each model should be in original order
|
||||
const firstAppearance: string[] = [];
|
||||
const seenModels = new Set<string>();
|
||||
for (const baseModel of resultBaseModels) {
|
||||
if (!seenModels.has(baseModel)) {
|
||||
seenModels.add(baseModel);
|
||||
firstAppearance.push(baseModel);
|
||||
}
|
||||
}
|
||||
|
||||
// Should preserve the original model order: step-a, step-b, step-c
|
||||
assert.deepEqual(firstAppearance, ["step-a", "step-b", "step-c"]);
|
||||
|
||||
// Within each model group, the winning account should be sorted first
|
||||
const antigravityGroup = modelAffinity.targets.filter((target) =>
|
||||
target.executionKey.startsWith("step-a")
|
||||
);
|
||||
const ollamacloudGroup = modelAffinity.targets.filter((target) =>
|
||||
target.executionKey.startsWith("step-b")
|
||||
);
|
||||
const ocGroup = modelAffinity.targets.filter((target) =>
|
||||
target.executionKey.startsWith("step-c")
|
||||
);
|
||||
|
||||
// Verify that within the oc group, the winning account is first
|
||||
// (since we chose a key that makes deepseek-acct-1 win)
|
||||
const ocFirstTarget = ocGroup[0];
|
||||
assert.ok(
|
||||
ocFirstTarget.executionKey.includes("deepseek-acct-1"),
|
||||
"Within oc model, the winning account should be first"
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user