mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
Explain effective auto-combo scoring weights (#7087)
* fix(inspector): report effective auto scoring weights * fix(inspector): check options.combos before the health-signal short-circuit resolveConfiguredCombos() returned [] unconditionally whenever healthResponse, forecastResponse, and either skipAutopilot or autopilotReport were all supplied -- before it ever looked at options.combos. That is exactly the call shape comboHealthDashboard.ts::buildComboHealthDashboardResponse() always uses (it resolves combos/health/forecast/autopilot once, then passes all of them into buildComboScoringInspectorResponse together), so through the real dashboard integration the caller-supplied combos were silently discarded every time. Since combosById/combosByName (built from resolveConfiguredCombos()'s return value) are what resolveInspectorWeights() uses to report a combo's actual configured modePack/weights, this meant the dashboard's weightSource/modePack fields always came back "default", even for a combo with an explicit mode pack configured. Check options.combos first, unconditionally, and only fall back to the health-signals short-circuit (skip an unnecessary getCombos() DB round-trip) or a fresh getCombos() call when the caller didn't supply combos at all. Adds a regression test that drives the real buildComboHealthDashboardResponse() end-to-end with a combo configured for modePack "ship-fast", proving the inspector now reports the correct weightSource/modePack through that call path -- the exact scenario the PR's own tests didn't cover (they only exercised buildComboScoringInspectorResponse() directly with comboId+combos, never combos alongside a pre-resolved healthResponse+forecastResponse). Also reconciles the existing "skipAutopilot avoids rebuilding autopilot report" test: it asserted options.combos was never even read in this scenario via a throwing property getter, which pinned the exact short-circuit-wins-always bug this fix removes. The test's options object never supplied a real combos array in the first place, so the fixed code still takes the same DB-free path -- only the poison-pill mechanism (which trapped a mere property read rather than an actual getCombos() call) no longer applies. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * refactor(usage): split resolveInspectorWeights below complexity gate resolveInspectorWeights had cyclomatic complexity 16 (gate max is 15). Extract the auto-config precedence resolution (autoConfig -> config.auto -> config -> {}), the mode-pack name lookup, and the explicit-weights validation into small pure helpers, each returning early instead of nesting ternaries. Behavior is unchanged, including the fallback warning that fires when a mode pack or explicit weights were configured but could not be resolved. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { buildComboHealthAutopilotReport } from "@/lib/monitoring/comboHealthAutopilot";
|
||||
import { getCombos } from "@/lib/db/combos";
|
||||
import { getProviderConnections } from "@/lib/db/providers";
|
||||
import { buildComboForecastResponse } from "@/lib/usage/comboForecast";
|
||||
import { buildComboHealthResponse } from "@/lib/usage/comboHealth";
|
||||
@@ -14,7 +15,9 @@ import {
|
||||
type ProviderCandidate,
|
||||
type ScoringFactors,
|
||||
type ScoringWeights,
|
||||
validateWeights,
|
||||
} from "@omniroute/open-sse/services/autoCombo/scoring.ts";
|
||||
import { getModePack } from "@omniroute/open-sse/services/autoCombo/modePacks.ts";
|
||||
import { getTaskFitness } from "@omniroute/open-sse/services/autoCombo/taskFitness.ts";
|
||||
import type {
|
||||
ComboAutopilotCombo,
|
||||
@@ -32,6 +35,7 @@ import type {
|
||||
ComboScoringInspectorResponse,
|
||||
ComboScoringInspectorSource,
|
||||
ComboScoringInspectorTarget,
|
||||
ComboScoringInspectorWeightSource,
|
||||
UtilizationTimeRange,
|
||||
} from "@/shared/types/utilization";
|
||||
|
||||
@@ -58,6 +62,13 @@ type CandidateContext = {
|
||||
notes: Partial<Record<ComboScoringInspectorFactorKey, string>>;
|
||||
};
|
||||
|
||||
type InspectorWeights = {
|
||||
weights: ScoringWeights;
|
||||
source: ComboScoringInspectorWeightSource;
|
||||
modePack: string | null;
|
||||
warning?: string;
|
||||
};
|
||||
|
||||
const FACTOR_KEYS: ComboScoringInspectorFactorKey[] = [
|
||||
"quota",
|
||||
"health",
|
||||
@@ -81,6 +92,82 @@ function normalizeTaskType(taskType: string | undefined): string {
|
||||
return typeof taskType === "string" && taskType.trim().length > 0 ? taskType.trim() : "default";
|
||||
}
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the auto-strategy config object a combo's weights should be read from,
|
||||
* honoring the same precedence the runtime auto-combo strategy uses: a dedicated
|
||||
* `autoConfig`, then a nested `config.auto`, then the raw `config`, then an empty
|
||||
* fallback so callers can probe optional fields without further null-checks.
|
||||
*/
|
||||
function resolveInspectorAutoConfig(combo: ComboRecord | undefined): Record<string, unknown> {
|
||||
if (isRecord(combo?.autoConfig)) return combo.autoConfig;
|
||||
if (isRecord(combo?.config)) {
|
||||
if (isRecord(combo.config.auto)) return combo.config.auto;
|
||||
return combo.config;
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
/** Extracts the mode-pack name referenced by the config, if any. */
|
||||
function resolveModePackName(config: Record<string, unknown>): string | null {
|
||||
return typeof config.modePack === "string" ? config.modePack : null;
|
||||
}
|
||||
|
||||
/** Resolves an explicit, validated `weights` object from the config, if present. */
|
||||
function resolveExplicitWeights(config: Record<string, unknown>): ScoringWeights | undefined {
|
||||
const explicitWeights = isRecord(config.weights) ? (config.weights as ScoringWeights) : undefined;
|
||||
return explicitWeights && validateWeights(explicitWeights) ? explicitWeights : undefined;
|
||||
}
|
||||
|
||||
function resolveInspectorWeights(combo: ComboRecord | undefined): InspectorWeights {
|
||||
const config = resolveInspectorAutoConfig(combo);
|
||||
|
||||
const modePack = resolveModePackName(config);
|
||||
const resolvedModePack = modePack ? getModePack(modePack) : undefined;
|
||||
if (resolvedModePack) {
|
||||
return { weights: resolvedModePack, source: "mode_pack", modePack };
|
||||
}
|
||||
|
||||
const explicitWeights = resolveExplicitWeights(config);
|
||||
if (explicitWeights) {
|
||||
return { weights: explicitWeights, source: "explicit", modePack: null };
|
||||
}
|
||||
|
||||
return {
|
||||
weights: DEFAULT_WEIGHTS,
|
||||
source: "default",
|
||||
modePack: null,
|
||||
warning:
|
||||
modePack || isRecord(config.weights)
|
||||
? "Configured auto weights are invalid or unavailable; default weights were used."
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
async function resolveConfiguredCombos(
|
||||
options: ComboScoringInspectorOptions
|
||||
): Promise<ComboRecord[]> {
|
||||
// #7087: `options.combos`, when supplied, must always win — regardless of the
|
||||
// health/forecast/autopilot short-circuit below. The dashboard integration
|
||||
// (comboHealthDashboard.ts::buildComboHealthDashboardResponse) always calls this
|
||||
// with `combos` AND `healthResponse`/`forecastResponse`/`autopilotReport` set
|
||||
// together (it already fetched everything once upstream to avoid redundant work),
|
||||
// which used to hit the short-circuit FIRST and silently discard the caller's
|
||||
// `combos` — so `combosById`/`combosByName` (used by resolveInspectorWeights() to
|
||||
// report the combo's real configured modePack/weights) came back empty and every
|
||||
// combo's weightSource fell back to "default" through that call path.
|
||||
if (options.combos) return options.combos;
|
||||
const alreadyHaveHealthSignals =
|
||||
options.healthResponse &&
|
||||
options.forecastResponse &&
|
||||
(options.skipAutopilot || options.autopilotReport);
|
||||
if (alreadyHaveHealthSignals) return [];
|
||||
return (await getCombos()) as ComboRecord[];
|
||||
}
|
||||
|
||||
function buildEmptyAutopilotReport(options: ComboScoringInspectorOptions): ComboAutopilotReport {
|
||||
return {
|
||||
status: "healthy",
|
||||
@@ -260,9 +347,11 @@ async function buildInspectorCombo(
|
||||
forecastTargets: Map<string, ComboForecastTarget>,
|
||||
autopilotCombo: ComboAutopilotCombo | undefined,
|
||||
taskType: string,
|
||||
weights: ScoringWeights
|
||||
inspectorWeights: InspectorWeights
|
||||
): Promise<ComboScoringInspectorCombo> {
|
||||
const warnings: string[] = [];
|
||||
const { weights } = inspectorWeights;
|
||||
if (inspectorWeights.warning) warnings.push(inspectorWeights.warning);
|
||||
const targets = combo.targetHealth ?? [];
|
||||
if (combo.strategy !== "auto") {
|
||||
warnings.push(
|
||||
@@ -349,6 +438,8 @@ async function buildInspectorCombo(
|
||||
strategy: combo.strategy,
|
||||
taskType,
|
||||
weights: weights as Record<ComboScoringInspectorFactorKey, number>,
|
||||
weightSource: inspectorWeights.source,
|
||||
modePack: inspectorWeights.modePack,
|
||||
selectedExecutionKey: inspectorTargets[0]?.executionKey ?? null,
|
||||
targets: inspectorTargets,
|
||||
warnings,
|
||||
@@ -359,14 +450,14 @@ export async function buildComboScoringInspectorResponse(
|
||||
options: ComboScoringInspectorOptions
|
||||
): Promise<ComboScoringInspectorResponse> {
|
||||
const taskType = normalizeTaskType(options.taskType);
|
||||
const weights = DEFAULT_WEIGHTS;
|
||||
const configuredCombos = await resolveConfiguredCombos(options);
|
||||
const [health, forecast] = await Promise.all([
|
||||
options.healthResponse ??
|
||||
buildComboHealthResponse({
|
||||
range: options.range,
|
||||
comboId: options.comboId,
|
||||
now: options.now,
|
||||
combos: options.combos,
|
||||
combos: configuredCombos,
|
||||
}),
|
||||
options.forecastResponse ??
|
||||
buildComboForecastResponse({
|
||||
@@ -374,7 +465,7 @@ export async function buildComboScoringInspectorResponse(
|
||||
horizon: options.horizon,
|
||||
comboId: options.comboId,
|
||||
now: options.now,
|
||||
combos: options.combos,
|
||||
combos: configuredCombos,
|
||||
}),
|
||||
]);
|
||||
const autopilot =
|
||||
@@ -388,13 +479,19 @@ export async function buildComboScoringInspectorResponse(
|
||||
includeHealthy: true,
|
||||
includeActions: false,
|
||||
now: options.now,
|
||||
combos: options.combos,
|
||||
combos: configuredCombos,
|
||||
healthResponse: health,
|
||||
forecastResponse: forecast,
|
||||
}));
|
||||
|
||||
const forecastByComboId = new Map(forecast.combos.map((combo) => [combo.comboId, combo]));
|
||||
const autopilotByComboId = new Map(autopilot.combos.map((combo) => [combo.comboId, combo]));
|
||||
const combosById = new Map(
|
||||
configuredCombos.flatMap((combo) => (combo.id ? [[combo.id, combo]] : []))
|
||||
);
|
||||
const combosByName = new Map(
|
||||
configuredCombos.flatMap((combo) => (combo.name ? [[combo.name, combo]] : []))
|
||||
);
|
||||
|
||||
return {
|
||||
asOf: new Date(options.now ?? Date.now()).toISOString(),
|
||||
@@ -408,7 +505,9 @@ export async function buildComboScoringInspectorResponse(
|
||||
targetForecastMap(forecastByComboId.get(combo.comboId)?.targets ?? []),
|
||||
autopilotByComboId.get(combo.comboId),
|
||||
taskType,
|
||||
weights
|
||||
resolveInspectorWeights(
|
||||
combosById.get(combo.comboId) ?? combosByName.get(combo.comboName)
|
||||
)
|
||||
)
|
||||
)
|
||||
),
|
||||
|
||||
@@ -82,6 +82,8 @@ export interface ComboRecord {
|
||||
name?: string;
|
||||
strategy?: string;
|
||||
models?: unknown[];
|
||||
autoConfig?: unknown;
|
||||
config?: unknown;
|
||||
}
|
||||
|
||||
export type UtilizationTimeRange = "1h" | "24h" | "7d" | "30d";
|
||||
@@ -270,11 +272,9 @@ export type ComboScoringInspectorFactorKey =
|
||||
| "resetWindowAffinity";
|
||||
|
||||
export type ComboScoringInspectorSource =
|
||||
| "combo_health"
|
||||
| "combo_forecast"
|
||||
| "combo_autopilot"
|
||||
| "runtime"
|
||||
| "default";
|
||||
"combo_health" | "combo_forecast" | "combo_autopilot" | "runtime" | "default";
|
||||
|
||||
export type ComboScoringInspectorWeightSource = "default" | "explicit" | "mode_pack";
|
||||
|
||||
export interface ComboScoringInspectorFactor {
|
||||
key: ComboScoringInspectorFactorKey;
|
||||
@@ -376,6 +376,8 @@ export interface ComboScoringInspectorCombo {
|
||||
strategy: string;
|
||||
taskType: string;
|
||||
weights: Record<ComboScoringInspectorFactorKey, number>;
|
||||
weightSource: ComboScoringInspectorWeightSource;
|
||||
modePack: string | null;
|
||||
selectedExecutionKey: string | null;
|
||||
targets: ComboScoringInspectorTarget[];
|
||||
warnings: string[];
|
||||
|
||||
@@ -21,11 +21,14 @@ const quotaSnapshotsDb = await import("../../src/lib/db/quotaSnapshots.ts");
|
||||
const callLogs = await import("../../src/lib/usage/callLogs.ts");
|
||||
const comboMetrics = await import("../../open-sse/services/comboMetrics.ts");
|
||||
const inspector = await import("../../src/lib/usage/comboScoringInspector.ts");
|
||||
const comboHealthDashboard = await import("../../src/lib/usage/comboHealthDashboard.ts");
|
||||
const route = await import("../../src/app/api/usage/combo-scoring-inspector/route.ts");
|
||||
const { normalizeComboStep } = await import("../../src/lib/combos/steps.ts");
|
||||
const { lockModel, clearAllModelLockouts } =
|
||||
await import("../../open-sse/services/accountFallback.ts");
|
||||
const { resetAllCircuitBreakers } = await import("../../src/shared/utils/circuitBreaker.ts");
|
||||
const { DEFAULT_WEIGHTS } = await import("../../open-sse/services/autoCombo/scoring.ts");
|
||||
const { MODE_PACKS } = await import("../../open-sse/services/autoCombo/modePacks.ts");
|
||||
|
||||
async function resetStorage() {
|
||||
comboMetrics.resetAllComboMetrics();
|
||||
@@ -41,10 +44,11 @@ async function enableManagementAuth() {
|
||||
await settingsDb.updateSettings({ requireLogin: true, password: "" });
|
||||
}
|
||||
|
||||
async function seedAutoCombo() {
|
||||
async function seedAutoCombo(comboOverrides: Record<string, unknown> = {}) {
|
||||
const comboInput = {
|
||||
name: "combo-scoring-auto",
|
||||
strategy: "auto",
|
||||
...comboOverrides,
|
||||
models: [
|
||||
{
|
||||
kind: "model",
|
||||
@@ -165,6 +169,8 @@ test("scoring inspector ranks targets and explains score contributions", async (
|
||||
assert.equal(response.method, "read_only_recompute");
|
||||
assert.equal(response.combos.length, 1);
|
||||
assert.equal(response.combos[0].strategy, "auto");
|
||||
assert.equal(response.combos[0].weightSource, "default");
|
||||
assert.equal(response.combos[0].modePack, null);
|
||||
assert.equal(response.combos[0].targets.length, 2);
|
||||
assert.equal(response.combos[0].selectedExecutionKey, firstStep.id);
|
||||
assert.equal(response.combos[0].targets[0].executionKey, firstStep.id);
|
||||
@@ -181,6 +187,60 @@ test("scoring inspector ranks targets and explains score contributions", async (
|
||||
);
|
||||
});
|
||||
|
||||
test("scoring inspector reports mode packs over explicit auto weights", async () => {
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "combo-scoring-mode-pack",
|
||||
strategy: "auto",
|
||||
models: ["openai/gpt-4o-mini"],
|
||||
config: {
|
||||
auto: {
|
||||
modePack: "ship-fast",
|
||||
weights: { ...DEFAULT_WEIGHTS },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const response = await inspector.buildComboScoringInspectorResponse({
|
||||
range: "24h",
|
||||
horizon: "7d",
|
||||
comboId: String(combo.id),
|
||||
combos: [combo],
|
||||
skipAutopilot: true,
|
||||
});
|
||||
|
||||
assert.equal(response.combos.length, 1);
|
||||
assert.equal(response.combos[0].weightSource, "mode_pack");
|
||||
assert.equal(response.combos[0].modePack, "ship-fast");
|
||||
assert.deepEqual(response.combos[0].weights, MODE_PACKS["ship-fast"]);
|
||||
});
|
||||
|
||||
test("scoring inspector reports valid explicit auto weights", async () => {
|
||||
const explicitWeights = {
|
||||
...DEFAULT_WEIGHTS,
|
||||
latencyInv: 0.2,
|
||||
costInv: 0.07,
|
||||
};
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "combo-scoring-explicit-weights",
|
||||
strategy: "auto",
|
||||
models: ["openai/gpt-4o-mini"],
|
||||
autoConfig: { weights: explicitWeights },
|
||||
});
|
||||
|
||||
const response = await inspector.buildComboScoringInspectorResponse({
|
||||
range: "24h",
|
||||
horizon: "7d",
|
||||
comboId: String(combo.id),
|
||||
combos: [combo],
|
||||
skipAutopilot: true,
|
||||
});
|
||||
|
||||
assert.equal(response.combos.length, 1);
|
||||
assert.equal(response.combos[0].weightSource, "explicit");
|
||||
assert.equal(response.combos[0].modePack, null);
|
||||
assert.deepEqual(response.combos[0].weights, explicitWeights);
|
||||
});
|
||||
|
||||
test("scoring inspector marks non-auto combos as explanatory recompute", async () => {
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "combo-scoring-priority",
|
||||
@@ -267,11 +327,15 @@ test("scoring inspector skipAutopilot avoids rebuilding autopilot report", async
|
||||
},
|
||||
skipAutopilot: true,
|
||||
};
|
||||
Object.defineProperty(options, "combos", {
|
||||
get() {
|
||||
throw new Error("autopilot should not read combos when skipped");
|
||||
},
|
||||
});
|
||||
// #7087: this used to assert that `options.combos` is never even READ when
|
||||
// health/forecast/skipAutopilot are all supplied (via a getter that throws on
|
||||
// access) — but that was pinning the exact bug: resolveConfiguredCombos() must
|
||||
// check `options.combos` FIRST, unconditionally, so a caller-supplied `combos`
|
||||
// array (e.g. from buildComboHealthDashboardResponse()) is never silently
|
||||
// discarded just because health/forecast/autopilot were already resolved too.
|
||||
// This test's own `options` genuinely has no `combos` (undefined), which still
|
||||
// exercises the "no combos supplied + already have health signals -> skip the
|
||||
// getCombos() DB round-trip" branch — no getter/trap needed for that.
|
||||
|
||||
const response = await inspector.buildComboScoringInspectorResponse(options);
|
||||
|
||||
@@ -417,3 +481,41 @@ test("scoring inspector route requires auth, validates query, and returns 404",
|
||||
assert.equal(body.combos.length, 1);
|
||||
assert.equal(body.combos[0].targets.length, 2);
|
||||
});
|
||||
|
||||
// #7087 follow-up: resolveConfiguredCombos() used to unconditionally return `[]`
|
||||
// whenever healthResponse + forecastResponse + (skipAutopilot || autopilotReport)
|
||||
// were ALL supplied, regardless of whether options.combos was also populated. That
|
||||
// is exactly the call shape comboHealthDashboard.ts::buildComboHealthDashboardResponse
|
||||
// always uses (it fetches combos once, then threads combos/healthResponse/
|
||||
// forecastResponse/autopilotReport into buildComboScoringInspectorResponse
|
||||
// together) -- so through the real dashboard integration, `combosById`/`combosByName`
|
||||
// (which resolveInspectorWeights() reads to report a combo's real configured
|
||||
// modePack/weights) always came back empty, and every combo's weightSource silently
|
||||
// fell back to "default" no matter what was actually configured. This test drives
|
||||
// the real buildComboHealthDashboardResponse() end-to-end (not
|
||||
// buildComboScoringInspectorResponse() directly, which the PR's own tests already
|
||||
// covered) with a combo configured for modePack "ship-fast".
|
||||
test("scoring inspector reports the configured weight source through the dashboard integration", async () => {
|
||||
const { combo } = await seedAutoCombo({
|
||||
config: { auto: { modePack: "ship-fast" } },
|
||||
});
|
||||
|
||||
const dashboard = await comboHealthDashboard.buildComboHealthDashboardResponse({
|
||||
range: "24h",
|
||||
horizon: "7d",
|
||||
comboId: String(combo.id),
|
||||
taskType: "coding",
|
||||
combos: [combo],
|
||||
});
|
||||
|
||||
assert.equal(dashboard.errors.scoring, undefined);
|
||||
assert.ok(dashboard.scoring, "expected a scoring inspector response, not null");
|
||||
assert.equal(dashboard.scoring?.combos.length, 1);
|
||||
assert.equal(
|
||||
dashboard.scoring?.combos[0].weightSource,
|
||||
"mode_pack",
|
||||
"dashboard's own combos should drive the reported weight source, not silently fall back to default"
|
||||
);
|
||||
assert.equal(dashboard.scoring?.combos[0].modePack, "ship-fast");
|
||||
assert.deepEqual(dashboard.scoring?.combos[0].weights, MODE_PACKS["ship-fast"]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user