Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixes/15458-priority-order-vs-stickiness.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **fix(combo):** keep declared priority order ahead of session stickiness ([#15458](https://github.com/diegosouzapw/OmniRoute/pull/15458)) — thanks @skygunner
29 changes: 22 additions & 7 deletions open-sse/services/combo/sessionStickiness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ interface StickyEntry {
* Injectable saturation fetcher seam (for unit tests).
* Returns HeadroomSaturation or undefined when unknown.
*/
export type SaturationFetcher = (
connectionId: string
) => Promise<HeadroomSaturation | undefined>;
export type SaturationFetcher = (connectionId: string) => Promise<HeadroomSaturation | undefined>;

// ─── Saturation fetcher seam ─────────────────────────────────────────────────

Expand Down Expand Up @@ -187,9 +185,7 @@ export type QuotaExhaustionChecker = (connectionId: string) => boolean;
let _quotaExhaustionOverride: QuotaExhaustionChecker | null = null;

/** Test-only: inject the quota-exhaustion checker; pass null to restore default. */
export function __setStickinessQuotaCheckerForTests(
checker: QuotaExhaustionChecker | null
): void {
export function __setStickinessQuotaCheckerForTests(checker: QuotaExhaustionChecker | null): void {
_quotaExhaustionOverride = checker;
}

Expand Down Expand Up @@ -450,6 +446,16 @@ export interface ApplyStickinessResult {
stuck: boolean;
}

/**
* #15241: when the caller needs to preserve the combo's declared first target,
* a sticky binding pointing at a mid-list connection must not silently become
* runtime try-slot #1. The binding's hygiene gates below still run; only the
* promotion is barred.
*/
export interface ApplyStickinessOptions {
respectDeclaredOrder?: boolean;
}

/**
* Attempt to promote the sticky connection to the front of `orderedTargets`.
*
Expand All @@ -469,12 +475,15 @@ export interface ApplyStickinessResult {
* @param orderedTargets Targets already ordered by the combo strategy.
* @param messages Request body.messages.
* @param namespace Combo identity that owns this sticky binding.
* @param options #15241 — `respectDeclaredOrder` bars a mid-list promotion
* for strategies whose first target is operator-declared.
* @returns Result with (possibly reordered) targets.
*/
export async function applySessionStickiness(
orderedTargets: ResolvedComboTarget[],
messages: Array<{ role?: string; content?: unknown }> | null | undefined,
namespace?: string
namespace?: string,
options?: ApplyStickinessOptions
): Promise<ApplyStickinessResult> {
const noOp: ApplyStickinessResult = { targets: orderedTargets, messageHash: null, stuck: false };

Expand Down Expand Up @@ -530,6 +539,12 @@ export async function applySessionStickiness(
return { targets: orderedTargets, messageHash, stuck: false };
}

// #15241: a caller preserving an operator-declared head keeps its order —
// a mid-list sticky connection stays mid-list.
if (options?.respectDeclaredOrder && stickyIdx > 0) {
return { targets: orderedTargets, messageHash, stuck: false };
}

// Promote the sticky target to position 0
const reordered = [
orderedTargets[stickyIdx],
Expand Down
7 changes: 6 additions & 1 deletion open-sse/services/combo/targetResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,12 @@ async function applyContinuityFilters(
// #7270: normalize both wire shapes (.messages / Responses-API .input) so the
// stickiness key is derivable on the /v1/responses surface, not just Chat Completions.
normalizeStickinessMessages(body as { messages?: unknown; input?: unknown }),
combo.name
combo.name,
// #15241: priority is an operator-declared failover order. A mid-list
// success must not silently become runtime try-slot #1 while the stored
// hop list still names another head. Other strategies keep their existing
// session-stickiness behavior.
{ respectDeclaredOrder: strategy === "priority" }
);
let orderedTargets = sticky.targets;
if (!cacheStrategyAffinityApplied) {
Expand Down
49 changes: 48 additions & 1 deletion tests/unit/combo-session-stickiness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const {

// ─── helpers ─────────────────────────────────────────────────────────────────

function makeTarget(connectionId: string): import("../../open-sse/services/combo/types.ts").ResolvedComboTarget {
function makeTarget(
connectionId: string
): import("../../open-sse/services/combo/types.ts").ResolvedComboTarget {
return {
kind: "model",
stepId: `step-${connectionId}`,
Expand Down Expand Up @@ -466,3 +468,48 @@ test("peekStickyConnectionId: reflects the current binding without mutating it",
// Peeking again must not clear or otherwise mutate the binding.
assert.equal(peekStickyConnectionId(hash), "conn-peek");
});

// ─── #15241: operator-declared order outranks stickiness promotion ───────────

test("#15241: respectDeclaredOrder keeps the declared head over a mid-list sticky connection", async () => {
const messages = [{ role: "user", content: "Priority order" }];
const hash = deriveMessageHash(messages)!;
recordStickyBinding(hash, "conn-B");

const targets = [makeTarget("conn-A"), makeTarget("conn-B"), makeTarget("conn-C")];
const result = await applySessionStickiness(targets, messages, undefined, {
respectDeclaredOrder: true,
});

assert.equal(result.stuck, false, "nothing was promoted, so sticky must not report stuck");
assert.equal(result.targets[0].connectionId, "conn-A", "declared head stays first");
assert.equal(result.targets[1].connectionId, "conn-B");
// the binding survives — only the reorder is barred, a later success refreshes it
assert.equal(peekStickyConnectionId(hash), "conn-B");
});

test("#15241: respectDeclaredOrder leaves a sticky connection already at the head untouched", async () => {
const messages = [{ role: "user", content: "Already head" }];
const hash = deriveMessageHash(messages)!;
recordStickyBinding(hash, "conn-A");

const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
const result = await applySessionStickiness(targets, messages, undefined, {
respectDeclaredOrder: true,
});

assert.equal(result.stuck, true, "declared head equals the sticky pin — sticky still applied");
assert.equal(result.targets[0].connectionId, "conn-A");
});

test("#15241: without respectDeclaredOrder a mid-list sticky connection still promotes", async () => {
const messages = [{ role: "user", content: "Legacy behavior" }];
const hash = deriveMessageHash(messages)!;
recordStickyBinding(hash, "conn-B");

const targets = [makeTarget("conn-A"), makeTarget("conn-B"), makeTarget("conn-C")];
const result = await applySessionStickiness(targets, messages);

assert.equal(result.stuck, true, "strategies without a declared head keep the reorder");
assert.equal(result.targets[0].connectionId, "conn-B");
});
33 changes: 33 additions & 0 deletions tests/unit/combo-target-resolution-split.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const { saveModelsDevCapabilities, clearModelsDevCapabilities } =
await import("../../src/lib/modelsDevSync.ts");
const { resolveComboTargetPipeline } =
await import("../../open-sse/services/combo/targetResolution.ts");
const { clearAllStickyBindings, recordStickyBinding } =
await import("../../open-sse/services/combo/sessionStickiness.ts");

test.after(() => {
core.resetDbInstance();
Expand All @@ -35,6 +37,7 @@ test.after(() => {

test.beforeEach(() => {
clearModelsDevCapabilities();
clearAllStickyBindings();
});

const noopLog = { info() {}, warn() {}, error() {}, debug() {} } as never;
Expand Down Expand Up @@ -94,6 +97,36 @@ test("priority strategy resolves combo models into orderedTargets in declared or
);
});

test("#15241 priority pipeline keeps its declared head after a mid-list sticky success", async () => {
const priorityWithConnections = {
id: "c-sticky-priority",
name: "sticky-priority",
strategy: "priority",
models: [
{ model: "openai/gpt-4o", connectionId: "conn-priority-a" },
{ model: "anthropic/claude-3", connectionId: "conn-priority-b" },
],
config: {},
};
const first = await resolveComboTargetPipeline(deps({ combo: priorityWithConnections }));
assert.ok(!("earlyResponse" in first));
if ("earlyResponse" in first) return;
assert.ok(first.sticky.messageHash, "pipeline derives the conversation stickiness key");
assert.ok(first.orderedTargets[1]?.connectionId, "second target has a connection id to pin");

recordStickyBinding(first.sticky.messageHash!, first.orderedTargets[1]!.connectionId!);

const repeated = await resolveComboTargetPipeline(deps({ combo: priorityWithConnections }));
assert.ok(!("earlyResponse" in repeated));
if ("earlyResponse" in repeated) return;
assert.deepEqual(
repeated.orderedTargets.map((target) => target.modelStr),
["openai/gpt-4o", "anthropic/claude-3"],
"a prior mid-list success must not become runtime try-slot #1 for priority"
);
assert.equal(repeated.sticky.stuck, false);
});

test("returns the derived values the attempt loop consumes", async () => {
const result = await resolveComboTargetPipeline(deps());
assert.ok(!("earlyResponse" in result));
Expand Down
Loading