Merge pull request #4348 from Yeachan-Heo/omc-team/you-are-one-of-5-parallel-work/worker-4
fix: trust user-configured multimodal-looker model for vision (#4209)
This commit is contained in:
@@ -13,6 +13,18 @@ import { clearFormatterCache } from "../tools/hashline-edit/formatter-trigger"
|
|||||||
|
|
||||||
export { resolveCategoryConfig } from "./category-config-resolver";
|
export { resolveCategoryConfig } from "./category-config-resolver";
|
||||||
|
|
||||||
|
function collectTrustedVisionCapableModels(
|
||||||
|
pluginConfig: OhMyOpenCodeConfig,
|
||||||
|
): string[] {
|
||||||
|
const trusted: string[] = []
|
||||||
|
const multimodalLookerOverride = pluginConfig.agents?.["multimodal-looker"]
|
||||||
|
const configuredModel = multimodalLookerOverride?.model
|
||||||
|
if (typeof configuredModel === "string" && configuredModel.includes("/")) {
|
||||||
|
trusted.push(configuredModel)
|
||||||
|
}
|
||||||
|
return trusted
|
||||||
|
}
|
||||||
|
|
||||||
export interface ConfigHandlerDeps {
|
export interface ConfigHandlerDeps {
|
||||||
ctx: { directory: string; client?: any };
|
ctx: { directory: string; client?: any };
|
||||||
pluginConfig: OhMyOpenCodeConfig;
|
pluginConfig: OhMyOpenCodeConfig;
|
||||||
@@ -26,7 +38,11 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
|
|||||||
const formatterConfig = config.formatter;
|
const formatterConfig = config.formatter;
|
||||||
|
|
||||||
setAdditionalAllowedMcpEnvVars(pluginConfig.mcp_env_allowlist ?? [])
|
setAdditionalAllowedMcpEnvVars(pluginConfig.mcp_env_allowlist ?? [])
|
||||||
applyProviderConfig({ config, modelCacheState });
|
applyProviderConfig({
|
||||||
|
config,
|
||||||
|
modelCacheState,
|
||||||
|
trustedVisionCapableModels: collectTrustedVisionCapableModels(pluginConfig),
|
||||||
|
});
|
||||||
clearFormatterCache()
|
clearFormatterCache()
|
||||||
|
|
||||||
const pluginComponents = await loadPluginComponents({ pluginConfig });
|
const pluginComponents = await loadPluginComponents({ pluginConfig });
|
||||||
|
|||||||
@@ -97,6 +97,92 @@ describe("applyProviderConfig", () => {
|
|||||||
])
|
])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("trusts user-configured multimodal-looker model even when provider config omits modalities", () => {
|
||||||
|
// given - user configures glm-5.1 as multimodal-looker but provider model entry has no modalities/capabilities
|
||||||
|
const modelCacheState = createModelCacheState()
|
||||||
|
const visionCapableModelsCache = modelCacheState.visionCapableModelsCache
|
||||||
|
if (!visionCapableModelsCache) {
|
||||||
|
throw new Error("visionCapableModelsCache should be initialized")
|
||||||
|
}
|
||||||
|
const config = {
|
||||||
|
provider: {
|
||||||
|
"zhipuai-coding-plan": {
|
||||||
|
models: {
|
||||||
|
"glm-5.1": {
|
||||||
|
limit: { context: 200000 },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} satisfies Record<string, unknown>
|
||||||
|
|
||||||
|
// when
|
||||||
|
applyProviderConfig({
|
||||||
|
config,
|
||||||
|
modelCacheState,
|
||||||
|
trustedVisionCapableModels: ["zhipuai-coding-plan/glm-5.1"],
|
||||||
|
})
|
||||||
|
|
||||||
|
// then - trusted model is in cache even though provider config did not declare image support
|
||||||
|
expect(Array.from(visionCapableModelsCache.keys())).toEqual([
|
||||||
|
"zhipuai-coding-plan/glm-5.1",
|
||||||
|
])
|
||||||
|
expect(readVisionCapableModelsCache()).toEqual([
|
||||||
|
{ providerID: "zhipuai-coding-plan", modelID: "glm-5.1" },
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
test("does not duplicate a trusted model already discovered via provider modalities", () => {
|
||||||
|
// given
|
||||||
|
const modelCacheState = createModelCacheState()
|
||||||
|
const visionCapableModelsCache = modelCacheState.visionCapableModelsCache
|
||||||
|
if (!visionCapableModelsCache) {
|
||||||
|
throw new Error("visionCapableModelsCache should be initialized")
|
||||||
|
}
|
||||||
|
const config = {
|
||||||
|
provider: {
|
||||||
|
google: {
|
||||||
|
models: {
|
||||||
|
"gemini-3-flash": {
|
||||||
|
modalities: { input: ["text", "image"] },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} satisfies Record<string, unknown>
|
||||||
|
|
||||||
|
// when
|
||||||
|
applyProviderConfig({
|
||||||
|
config,
|
||||||
|
modelCacheState,
|
||||||
|
trustedVisionCapableModels: ["google/gemini-3-flash"],
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(Array.from(visionCapableModelsCache.keys())).toEqual([
|
||||||
|
"google/gemini-3-flash",
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
test("ignores malformed trusted vision-capable model strings", () => {
|
||||||
|
// given - entries missing provider or model are skipped silently
|
||||||
|
const modelCacheState = createModelCacheState()
|
||||||
|
const visionCapableModelsCache = modelCacheState.visionCapableModelsCache
|
||||||
|
if (!visionCapableModelsCache) {
|
||||||
|
throw new Error("visionCapableModelsCache should be initialized")
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
applyProviderConfig({
|
||||||
|
config: { provider: {} },
|
||||||
|
modelCacheState,
|
||||||
|
trustedVisionCapableModels: ["no-slash", "/missing-provider", "provider-only/"],
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(visionCapableModelsCache.size).toBe(0)
|
||||||
|
})
|
||||||
|
|
||||||
test("clears stale vision-capable models when provider config changes", () => {
|
test("clears stale vision-capable models when provider config changes", () => {
|
||||||
// given
|
// given
|
||||||
const modelCacheState = createModelCacheState()
|
const modelCacheState = createModelCacheState()
|
||||||
|
|||||||
@@ -26,9 +26,19 @@ function supportsImageInput(modelConfig: ProviderModelConfig | undefined): boole
|
|||||||
return modelConfig?.capabilities?.input?.image === true
|
return modelConfig?.capabilities?.input?.image === true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function parseTrustedModel(modelString: string): VisionCapableModel | undefined {
|
||||||
|
const [providerID, ...modelIDParts] = modelString.split("/")
|
||||||
|
const modelID = modelIDParts.join("/")
|
||||||
|
if (!providerID || modelID.length === 0) {
|
||||||
|
return undefined
|
||||||
|
}
|
||||||
|
return { providerID, modelID }
|
||||||
|
}
|
||||||
|
|
||||||
export function applyProviderConfig(params: {
|
export function applyProviderConfig(params: {
|
||||||
config: Record<string, unknown>;
|
config: Record<string, unknown>;
|
||||||
modelCacheState: ModelCacheState;
|
modelCacheState: ModelCacheState;
|
||||||
|
trustedVisionCapableModels?: string[];
|
||||||
}): void {
|
}): void {
|
||||||
const providers = params.config.provider as
|
const providers = params.config.provider as
|
||||||
| Record<string, ProviderConfig>
|
| Record<string, ProviderConfig>
|
||||||
@@ -47,27 +57,35 @@ export function applyProviderConfig(params: {
|
|||||||
visionCapableModelsCache.clear()
|
visionCapableModelsCache.clear()
|
||||||
setVisionCapableModelsCache(visionCapableModelsCache)
|
setVisionCapableModelsCache(visionCapableModelsCache)
|
||||||
|
|
||||||
if (!providers) return;
|
if (providers) {
|
||||||
|
for (const [providerID, providerConfig] of Object.entries(providers)) {
|
||||||
|
const models = providerConfig?.models;
|
||||||
|
if (!models) continue;
|
||||||
|
|
||||||
for (const [providerID, providerConfig] of Object.entries(providers)) {
|
for (const [modelID, modelConfig] of Object.entries(models)) {
|
||||||
const models = providerConfig?.models;
|
if (supportsImageInput(modelConfig)) {
|
||||||
if (!models) continue;
|
visionCapableModelsCache.set(
|
||||||
|
`${providerID}/${modelID}`,
|
||||||
|
{ providerID, modelID },
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
for (const [modelID, modelConfig] of Object.entries(models)) {
|
const contextLimit = modelConfig?.limit?.context;
|
||||||
if (supportsImageInput(modelConfig)) {
|
if (!contextLimit) continue;
|
||||||
visionCapableModelsCache.set(
|
|
||||||
|
modelContextLimitsCache.set(
|
||||||
`${providerID}/${modelID}`,
|
`${providerID}/${modelID}`,
|
||||||
{ providerID, modelID },
|
contextLimit,
|
||||||
)
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
const contextLimit = modelConfig?.limit?.context;
|
|
||||||
if (!contextLimit) continue;
|
|
||||||
|
|
||||||
modelContextLimitsCache.set(
|
|
||||||
`${providerID}/${modelID}`,
|
|
||||||
contextLimit,
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for (const trustedModelString of params.trustedVisionCapableModels ?? []) {
|
||||||
|
const trustedModel = parseTrustedModel(trustedModelString)
|
||||||
|
if (!trustedModel) continue
|
||||||
|
const key = `${trustedModel.providerID}/${trustedModel.modelID}`
|
||||||
|
if (visionCapableModelsCache.has(key)) continue
|
||||||
|
visionCapableModelsCache.set(key, trustedModel)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user