fix(base-red): align least-used combo tests with executionKey usage keying (#7015)

sortTargetsByUsage (open-sse/services/combo/targetSorters.ts, since
#7015/#7059) keys usage lookups by the per-target executionKey
(combo-name + step-id), not by the bare model string, so accounts
sharing a modelStr don't collapse into a single usage bucket.

Three tests called recordComboRequest() directly without a `target`,
so the recorded usage landed under a modelStr fallback key that never
matches the real executionKey computed at combo-resolution time —
every target read back as 0 usage and the original combo order won,
failing the "prefers the least-used model" assertions. Production is
unaffected: every real combo.ts call site already passes
`target: toRecordedTarget(target)`.

Fixed by priming usage through real handleComboChat calls (which
route recordComboRequest through the actual resolved target) instead
of calling recordComboRequest() directly with an unlinked target.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-18 22:22:51 -03:00
parent f048d98b46
commit dbc9f60818
2 changed files with 73 additions and 34 deletions

View File

@@ -667,31 +667,48 @@ test("handleComboChat p2c selects the better of two random choices by metrics",
});
test("handleComboChat least-used strategy prefers the model with fewer recorded requests", async () => {
recordComboRequest("least-used-combo", "model-a", {
success: true,
latencyMs: 100,
// Least-used sorts by the per-target executionKey (combo-name + step-id),
// not by the bare model string (#7015/#7059 — sortTargetsByUsage keys
// byTarget[executionKey] so accounts sharing a modelStr don't collapse into
// one shared bucket). Calling recordComboRequest() directly without a
// `target` falls back to keying by modelStr, which never matches the real
// executionKey computed at combo-resolution time. Drive usage through real
// handleComboChat calls instead, so recordComboRequest is invoked with the
// actual resolved target (matching production behavior).
const comboDef = {
name: "least-used-combo",
strategy: "least-used",
});
recordComboRequest("least-used-combo", "model-a", {
success: true,
latencyMs: 100,
strategy: "least-used",
});
recordComboRequest("least-used-combo", "model-b", {
success: true,
latencyMs: 100,
strategy: "least-used",
});
models: ["model-a", "model-b", "model-c"],
};
const primingCalls: any[] = [];
const runPrimingCall = () =>
handleComboChat({
body: {},
combo: comboDef,
handleSingleModel: async (_body: any, modelStr: any) => {
primingCalls.push(modelStr);
return okResponse();
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
relayOptions: null as any,
allCombos: null,
});
// Tied at 0 usage, order is preserved: 1st priming call picks model-a,
// 2nd priming call (model-a now at 1) picks model-b (tied with model-c at
// 0, but model-b sorts first). That leaves model-a and model-b each used
// once, model-c untouched.
await runPrimingCall();
await runPrimingCall();
assert.deepEqual(primingCalls, ["model-a", "model-b"]);
const calls: any[] = [];
await handleComboChat({
body: {},
combo: {
name: "least-used-combo",
strategy: "least-used",
models: ["model-a", "model-b", "model-c"],
},
combo: comboDef,
handleSingleModel: async (_body: any, modelStr: any) => {
calls.push(modelStr);
return okResponse();
@@ -2022,20 +2039,37 @@ test("handleComboChat eval-driven routing can match bare model eval target ids",
});
test("handleComboChat normalizes legacy strategy names at runtime", async () => {
const usageCalls: any[] = [];
recordComboRequest("legacy-usage-combo", "model-a", {
success: true,
latencyMs: 100,
strategy: "least-used",
// See the least-used test above: recordComboRequest() must be driven
// through a real handleComboChat call so it records against the actual
// resolved executionKey, not a bare modelStr fallback that never matches
// sortTargetsByUsage's lookup (#7015/#7059).
const comboDef = {
name: "legacy-usage-combo",
strategy: "usage",
models: ["model-a", "model-b"],
};
const primingCalls: any[] = [];
await handleComboChat({
body: {},
combo: comboDef,
handleSingleModel: async (_body: any, modelStr: any) => {
primingCalls.push(modelStr);
return okResponse();
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
relayOptions: null as any,
allCombos: null,
});
// Tied at 0 usage, order preserved: model-a (first in the list) wins.
assert.deepEqual(primingCalls, ["model-a"]);
const usageCalls: any[] = [];
await handleComboChat({
body: {},
combo: {
name: "legacy-usage-combo",
strategy: "usage",
models: ["model-a", "model-b"],
},
combo: comboDef,
handleSingleModel: async (_body: any, modelStr: any) => {
usageCalls.push(modelStr);
return okResponse();

View File

@@ -213,11 +213,16 @@ test("least-used strategy prefers the model with fewer recorded combo requests",
models: [busyModel, idleModel],
});
recordComboRequest(name, busyModel, {
success: true,
latencyMs: 10,
strategy: "least-used",
});
// Prime usage through a real handleComboChat call rather than calling
// recordComboRequest() directly: least-used sorts by the per-target
// executionKey (combo-name + step-id), not by the bare model string
// (#7015/#7059 — sortTargetsByUsage keys byTarget[executionKey] so accounts
// sharing a modelStr don't collapse into one bucket). Recording without a
// `target` falls back to keying by modelStr, which never matches the real
// executionKey and made this assertion flaky against the intended fix.
// With no prior usage, least-used ties at 0 and keeps combo order, so this
// priming call always lands on busyModel (first in the models array).
assert.equal(await selectedModelFor(combo, reqBodyTextArray), busyModel);
assert.equal(await selectedModelFor(combo, reqBodyTextArray), idleModel);
});