From 0bbdb839d9fad8ecbe078009a468d131b0e92dc3 Mon Sep 17 00:00:00 2001 From: KooshaPari <42529354+KooshaPari@users.noreply.github.com> Date: Sat, 18 Jul 2026 11:13:26 -0700 Subject: [PATCH] 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> --- src/lib/usage/comboScoringInspector.ts | 111 ++++++++++++++++++-- src/shared/types/utilization.ts | 12 ++- tests/unit/combo-scoring-inspector.test.ts | 114 +++++++++++++++++++-- 3 files changed, 220 insertions(+), 17 deletions(-) diff --git a/src/lib/usage/comboScoringInspector.ts b/src/lib/usage/comboScoringInspector.ts index c61b3f7d12..9f6c9eb0b3 100644 --- a/src/lib/usage/comboScoringInspector.ts +++ b/src/lib/usage/comboScoringInspector.ts @@ -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>; }; +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 { + 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 { + 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 | null { + return typeof config.modePack === "string" ? config.modePack : null; +} + +/** Resolves an explicit, validated `weights` object from the config, if present. */ +function resolveExplicitWeights(config: Record): 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 { + // #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, autopilotCombo: ComboAutopilotCombo | undefined, taskType: string, - weights: ScoringWeights + inspectorWeights: InspectorWeights ): Promise { 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, + 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 { 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) + ) ) ) ), diff --git a/src/shared/types/utilization.ts b/src/shared/types/utilization.ts index 2a8e3cfeb7..7a257e5470 100644 --- a/src/shared/types/utilization.ts +++ b/src/shared/types/utilization.ts @@ -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; + weightSource: ComboScoringInspectorWeightSource; + modePack: string | null; selectedExecutionKey: string | null; targets: ComboScoringInspectorTarget[]; warnings: string[]; diff --git a/tests/unit/combo-scoring-inspector.test.ts b/tests/unit/combo-scoring-inspector.test.ts index 4f6d63c580..473dc1e4ff 100644 --- a/tests/unit/combo-scoring-inspector.test.ts +++ b/tests/unit/combo-scoring-inspector.test.ts @@ -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 = {}) { 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"]); +});