fix(routing): address codex review — complete interactive bypass and accurate banner
resolvePreferredModelConfig now skips routing-ceiling synthesis when isAutoMode=false, preventing interactive dispatches from silently switching to the tier_models.heavy model. The auto-start banner now reflects effective routing state by checking flat-rate provider suppression and using the actual ceiling from tier_models.heavy when configured.
This commit is contained in:
parent
839cd8d55b
commit
66710f2b28
3 changed files with 80 additions and 3 deletions
|
|
@ -25,10 +25,17 @@ export interface ModelSelectionResult {
|
|||
export function resolvePreferredModelConfig(
|
||||
unitType: string,
|
||||
autoModeStartModel: { provider: string; id: string } | null,
|
||||
/** When false, only return explicit per-phase model configs — do not
|
||||
* synthesize a routing ceiling from dynamic_routing.tier_models (#3962). */
|
||||
isAutoMode = true,
|
||||
) {
|
||||
const explicitConfig = resolveModelWithFallbacksForUnit(unitType);
|
||||
if (explicitConfig) return explicitConfig;
|
||||
|
||||
// In interactive mode, don't synthesize a routing-based model config.
|
||||
// The user's session model (/model) should be used as-is (#3962).
|
||||
if (!isAutoMode) return undefined;
|
||||
|
||||
const routingConfig = resolveDynamicRoutingConfig();
|
||||
if (!routingConfig.enabled || !routingConfig.tier_models) return undefined;
|
||||
|
||||
|
|
@ -66,7 +73,7 @@ export async function selectAndApplyModel(
|
|||
* Dynamic routing only applies in auto-mode where cost optimization is expected. (#3962) */
|
||||
isAutoMode = true,
|
||||
): Promise<ModelSelectionResult> {
|
||||
const modelConfig = resolvePreferredModelConfig(unitType, autoModeStartModel);
|
||||
const modelConfig = resolvePreferredModelConfig(unitType, autoModeStartModel, isAutoMode);
|
||||
let routing: { tier: string; modelDowngraded: boolean } | null = null;
|
||||
let appliedModel: Model<Api> | null = null;
|
||||
|
||||
|
|
|
|||
|
|
@ -780,13 +780,28 @@ export async function bootstrapAutoSession(
|
|||
|
||||
// Show dynamic routing status so users know upfront if models will be
|
||||
// downgraded for simple tasks (#3962).
|
||||
// Use the same effective logic as selectAndApplyModel: check flat-rate
|
||||
// provider suppression and resolve the actual ceiling model.
|
||||
const routingConfig = resolveDynamicRoutingConfig();
|
||||
const startModelLabel = s.autoModeStartModel
|
||||
? `${s.autoModeStartModel.provider}/${s.autoModeStartModel.id}`
|
||||
: ctx.model ? `${ctx.model.provider}/${ctx.model.id}` : "default";
|
||||
if (routingConfig.enabled) {
|
||||
|
||||
// Flat-rate providers (e.g. GitHub Copilot, claude-code) suppress routing
|
||||
// at dispatch time (#3453) — reflect that in the banner.
|
||||
const { isFlatRateProvider } = await import("./auto-model-selection.js");
|
||||
const effectiveProvider = s.autoModeStartModel?.provider ?? ctx.model?.provider;
|
||||
const effectivelyEnabled = routingConfig.enabled
|
||||
&& !(effectiveProvider && isFlatRateProvider(effectiveProvider));
|
||||
|
||||
// The actual ceiling may come from tier_models.heavy, not the start model.
|
||||
const effectiveCeiling = (routingConfig.enabled && routingConfig.tier_models?.heavy)
|
||||
? routingConfig.tier_models.heavy
|
||||
: startModelLabel;
|
||||
|
||||
if (effectivelyEnabled) {
|
||||
ctx.ui.notify(
|
||||
`Dynamic routing: enabled — simple tasks may use cheaper models (ceiling: ${startModelLabel})`,
|
||||
`Dynamic routing: enabled — simple tasks may use cheaper models (ceiling: ${effectiveCeiling})`,
|
||||
"info",
|
||||
);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -53,6 +53,36 @@ describe("interactive routing bypass (#3962)", () => {
|
|||
);
|
||||
});
|
||||
|
||||
test("resolvePreferredModelConfig skips routing synthesis when isAutoMode is false", () => {
|
||||
// resolvePreferredModelConfig should accept isAutoMode and bail early
|
||||
// before synthesizing a routing ceiling from tier_models (#3962 codex review)
|
||||
assert.ok(
|
||||
modelSelectionSrc.includes("function resolvePreferredModelConfig"),
|
||||
"resolvePreferredModelConfig should exist",
|
||||
);
|
||||
// The function should check isAutoMode before routing synthesis
|
||||
const fnIdx = modelSelectionSrc.indexOf("function resolvePreferredModelConfig");
|
||||
const fnBody = modelSelectionSrc.slice(fnIdx, fnIdx + 600);
|
||||
assert.ok(
|
||||
fnBody.includes("isAutoMode"),
|
||||
"resolvePreferredModelConfig should accept isAutoMode parameter",
|
||||
);
|
||||
assert.ok(
|
||||
fnBody.includes("if (!isAutoMode) return undefined"),
|
||||
"should return undefined (skip routing synthesis) when not in auto-mode",
|
||||
);
|
||||
});
|
||||
|
||||
test("selectAndApplyModel threads isAutoMode to resolvePreferredModelConfig", () => {
|
||||
// The call to resolvePreferredModelConfig inside selectAndApplyModel
|
||||
// should pass isAutoMode as the third argument
|
||||
const callSite = "resolvePreferredModelConfig(unitType, autoModeStartModel, isAutoMode)";
|
||||
assert.ok(
|
||||
modelSelectionSrc.includes(callSite),
|
||||
"selectAndApplyModel should pass isAutoMode to resolvePreferredModelConfig",
|
||||
);
|
||||
});
|
||||
|
||||
test("guided-flow passes isAutoMode=false", () => {
|
||||
// guided-flow.ts should explicitly pass isAutoMode as false
|
||||
assert.ok(
|
||||
|
|
@ -149,4 +179,29 @@ describe("auto-mode start routing banner (#3962)", () => {
|
|||
"banner should reference the start/ceiling model",
|
||||
);
|
||||
});
|
||||
|
||||
test("banner accounts for flat-rate provider suppression", () => {
|
||||
// The banner should check isFlatRateProvider to accurately reflect
|
||||
// whether routing will actually be active at dispatch time (#3962 codex review)
|
||||
assert.ok(
|
||||
autoStartSrc.includes("isFlatRateProvider"),
|
||||
"banner should check flat-rate provider status",
|
||||
);
|
||||
assert.ok(
|
||||
autoStartSrc.includes("effectivelyEnabled"),
|
||||
"banner should compute effective routing state, not just raw config",
|
||||
);
|
||||
});
|
||||
|
||||
test("banner uses effective ceiling from tier_models.heavy when configured", () => {
|
||||
// The actual ceiling may come from tier_models.heavy, not the start model
|
||||
assert.ok(
|
||||
autoStartSrc.includes("tier_models?.heavy"),
|
||||
"banner should check tier_models.heavy for the effective ceiling",
|
||||
);
|
||||
assert.ok(
|
||||
autoStartSrc.includes("effectiveCeiling"),
|
||||
"banner should compute the effective ceiling model",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue