dispatchHookUnit() previously used a simplistic lookup that only matched exact model IDs or "provider/id" against ctx.modelRegistry.getAvailable(). Non-Claude models (e.g. openrouter/openai/gpt-5.4-codex) silently fell back to the session model with no warning. - Replace simplistic lookup in dispatchHookUnit with resolveModelId() which handles provider/model, bare-id, and OpenRouter org/model formats - Add warning notification when a hook model can't be resolved instead of silent fallback - Wire sidecar hook model override through runUnitPhase so post-unit hook model fields are applied after standard model selection - Consume pre-dispatch hook model field (hookModelOverride) in IterationData so it reaches the dispatch phase - Export resolveModelId from auto-model-selection.ts for reuse - Add resolveModelId to LoopDeps interface for testability Closes #1720 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
27916344df
commit
d14e67cad8
6 changed files with 140 additions and 5 deletions
|
|
@ -164,7 +164,7 @@ export async function selectAndApplyModel(
|
|||
* Resolve a model ID string to a model object from the available models list.
|
||||
* Handles formats: "provider/model", "bare-id", "org/model-name" (OpenRouter).
|
||||
*/
|
||||
function resolveModelId<T extends { id: string; provider: string }>(
|
||||
export function resolveModelId<T extends { id: string; provider: string }>(
|
||||
modelId: string,
|
||||
availableModels: T[],
|
||||
currentProvider: string | undefined,
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ import {
|
|||
import { closeoutUnit } from "./auto-unit-closeout.js";
|
||||
import { recoverTimedOutUnit } from "./auto-timeout-recovery.js";
|
||||
import { selfHealRuntimeRecords } from "./auto-recovery.js";
|
||||
import { selectAndApplyModel } from "./auto-model-selection.js";
|
||||
import { selectAndApplyModel, resolveModelId } from "./auto-model-selection.js";
|
||||
import {
|
||||
syncProjectRootToWorktree,
|
||||
syncStateToProjectRoot,
|
||||
|
|
@ -912,6 +912,7 @@ function buildLoopDeps(): LoopDeps {
|
|||
|
||||
// Model selection + supervision
|
||||
selectAndApplyModel,
|
||||
resolveModelId,
|
||||
startUnitSupervision,
|
||||
|
||||
// Prompt helpers
|
||||
|
|
@ -1307,15 +1308,19 @@ export async function dispatchHookUnit(
|
|||
|
||||
if (hookModel) {
|
||||
const availableModels = ctx.modelRegistry.getAvailable();
|
||||
const match = availableModels.find(
|
||||
(m) => m.id === hookModel || `${m.provider}/${m.id}` === hookModel,
|
||||
);
|
||||
const match = resolveModelId(hookModel, availableModels, ctx.model?.provider);
|
||||
if (match) {
|
||||
try {
|
||||
await pi.setModel(match);
|
||||
} catch {
|
||||
/* non-fatal */
|
||||
}
|
||||
} else {
|
||||
ctx.ui.notify(
|
||||
`Hook model "${hookModel}" not found in available models. Falling back to current session model. ` +
|
||||
`Ensure the model is defined in models.json and has auth configured.`,
|
||||
"warning",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -236,6 +236,11 @@ export interface LoopDeps {
|
|||
startModel: { provider: string; id: string } | null,
|
||||
retryContext?: { isRetry: boolean; previousTier?: string },
|
||||
) => Promise<{ routing: { tier: string; modelDowngraded: boolean } | null }>;
|
||||
resolveModelId: <T extends { id: string; provider: string }>(
|
||||
modelId: string,
|
||||
availableModels: T[],
|
||||
currentProvider: string | undefined,
|
||||
) => T | undefined;
|
||||
startUnitSupervision: (sctx: {
|
||||
s: AutoSession;
|
||||
ctx: ExtensionContext;
|
||||
|
|
|
|||
|
|
@ -642,6 +642,7 @@ export async function runDispatch(
|
|||
pauseAfterUatDispatch, observabilityIssues,
|
||||
state, mid, midTitle,
|
||||
isRetry: false, previousTier: undefined,
|
||||
hookModelOverride: preDispatchResult.model,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
@ -928,6 +929,30 @@ export async function runUnitPhase(
|
|||
s.currentUnitRouting =
|
||||
modelResult.routing as AutoSession["currentUnitRouting"];
|
||||
|
||||
// Apply sidecar/pre-dispatch hook model override (takes priority over standard model selection)
|
||||
const hookModelOverride = sidecarItem?.model ?? iterData.hookModelOverride;
|
||||
if (hookModelOverride) {
|
||||
const availableModels = ctx.modelRegistry.getAvailable();
|
||||
const match = deps.resolveModelId(hookModelOverride, availableModels, ctx.model?.provider);
|
||||
if (match) {
|
||||
const ok = await pi.setModel(match, { persist: false });
|
||||
if (ok) {
|
||||
ctx.ui.notify(`Hook model override: ${match.provider}/${match.id}`, "info");
|
||||
} else {
|
||||
ctx.ui.notify(
|
||||
`Hook model "${hookModelOverride}" found but setModel failed. Using default.`,
|
||||
"warning",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
ctx.ui.notify(
|
||||
`Hook model "${hookModelOverride}" not found in available models. Falling back to current session model. ` +
|
||||
`Ensure the model is defined in models.json and has auth configured.`,
|
||||
"warning",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Start unit supervision
|
||||
deps.clearUnitTimeout();
|
||||
deps.startUnitSupervision({
|
||||
|
|
|
|||
|
|
@ -94,6 +94,8 @@ export interface IterationData {
|
|||
midTitle: string | undefined;
|
||||
isRetry: boolean;
|
||||
previousTier: string | undefined;
|
||||
/** Model override from pre-dispatch hooks (applied after standard model selection). */
|
||||
hookModelOverride?: string;
|
||||
}
|
||||
|
||||
export type WindowEntry = { key: string; error?: string };
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
/**
|
||||
* Tests for hook model resolution (#1720).
|
||||
*
|
||||
* Verifies that resolveModelId handles all model ID formats correctly,
|
||||
* including OpenRouter-style "org/model" IDs, provider-prefixed IDs,
|
||||
* and bare IDs.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { resolveModelId } from "../auto-model-selection.js";
|
||||
|
||||
// ─── Test Models ─────────────────────────────────────────────────────────────
|
||||
|
||||
type TestModel = { id: string; provider: string };
|
||||
|
||||
const AVAILABLE_MODELS: TestModel[] = [
|
||||
{ id: "claude-sonnet-4-6", provider: "anthropic" },
|
||||
{ id: "claude-opus-4-6", provider: "anthropic" },
|
||||
{ id: "claude-haiku-4-5", provider: "anthropic" },
|
||||
{ id: "openai/gpt-5.4-codex", provider: "openrouter" },
|
||||
{ id: "google/gemini-2.5-pro", provider: "openrouter" },
|
||||
{ id: "gpt-4o", provider: "openai" },
|
||||
{ id: "gpt-4o", provider: "azure" },
|
||||
];
|
||||
|
||||
// ─── Bare model ID ───────────────────────────────────────────────────────────
|
||||
|
||||
test("resolveModelId: bare ID resolves to current provider first", () => {
|
||||
const match = resolveModelId("gpt-4o", AVAILABLE_MODELS, "openai");
|
||||
assert.ok(match);
|
||||
assert.equal(match.provider, "openai");
|
||||
assert.equal(match.id, "gpt-4o");
|
||||
});
|
||||
|
||||
test("resolveModelId: bare ID falls back to first available when no current provider match", () => {
|
||||
const match = resolveModelId("claude-sonnet-4-6", AVAILABLE_MODELS, "openai");
|
||||
assert.ok(match);
|
||||
assert.equal(match.provider, "anthropic");
|
||||
assert.equal(match.id, "claude-sonnet-4-6");
|
||||
});
|
||||
|
||||
// ─── Provider-prefixed ID ────────────────────────────────────────────────────
|
||||
|
||||
test("resolveModelId: provider/model resolves correctly", () => {
|
||||
const match = resolveModelId("anthropic/claude-opus-4-6", AVAILABLE_MODELS, undefined);
|
||||
assert.ok(match);
|
||||
assert.equal(match.provider, "anthropic");
|
||||
assert.equal(match.id, "claude-opus-4-6");
|
||||
});
|
||||
|
||||
test("resolveModelId: provider/model case-insensitive", () => {
|
||||
const match = resolveModelId("Anthropic/Claude-Sonnet-4-6", AVAILABLE_MODELS, undefined);
|
||||
assert.ok(match);
|
||||
assert.equal(match.provider, "anthropic");
|
||||
});
|
||||
|
||||
// ─── OpenRouter-style model IDs (org/model as the ID) ───────────────────────
|
||||
|
||||
test("resolveModelId: openrouter/org/model resolves full string as ID", () => {
|
||||
const match = resolveModelId("openrouter/openai/gpt-5.4-codex", AVAILABLE_MODELS, undefined);
|
||||
assert.ok(match, "should find the OpenRouter model with org/model ID");
|
||||
assert.equal(match.provider, "openrouter");
|
||||
assert.equal(match.id, "openai/gpt-5.4-codex");
|
||||
});
|
||||
|
||||
test("resolveModelId: openrouter org/model resolves when used as bare ID", () => {
|
||||
// When the user specifies "openai/gpt-5.4-codex" without provider prefix,
|
||||
// and "openai" is not a known provider, it should try matching the full
|
||||
// string as a model ID.
|
||||
const modelsWithoutOpenai = AVAILABLE_MODELS.filter(m => m.provider !== "openai" && m.provider !== "azure");
|
||||
const match = resolveModelId("openai/gpt-5.4-codex", modelsWithoutOpenai, undefined);
|
||||
assert.ok(match, "should find the model when openai is not a known provider");
|
||||
assert.equal(match.provider, "openrouter");
|
||||
assert.equal(match.id, "openai/gpt-5.4-codex");
|
||||
});
|
||||
|
||||
// ─── Disambiguation with multiple providers ──────────────────────────────────
|
||||
|
||||
test("resolveModelId: azure/gpt-4o resolves to azure provider", () => {
|
||||
const match = resolveModelId("azure/gpt-4o", AVAILABLE_MODELS, undefined);
|
||||
assert.ok(match);
|
||||
assert.equal(match.provider, "azure");
|
||||
assert.equal(match.id, "gpt-4o");
|
||||
});
|
||||
|
||||
// ─── Missing model ───────────────────────────────────────────────────────────
|
||||
|
||||
test("resolveModelId: returns undefined for unknown model", () => {
|
||||
const match = resolveModelId("nonexistent-model", AVAILABLE_MODELS, "anthropic");
|
||||
assert.equal(match, undefined);
|
||||
});
|
||||
|
||||
test("resolveModelId: returns undefined for unknown provider/model combo", () => {
|
||||
const match = resolveModelId("fakeprovider/fake-model", AVAILABLE_MODELS, undefined);
|
||||
assert.equal(match, undefined);
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue