fix(look-at): preserve variant metadata and block non-vision models#2469
Merged
code-yeongyu merged 2 commits intodevfrom Mar 11, 2026
Merged
fix(look-at): preserve variant metadata and block non-vision models#2469code-yeongyu merged 2 commits intodevfrom
code-yeongyu merged 2 commits intodevfrom
Conversation
…on-vision models - fallback-chain.ts: cache-derived entries inherit variant from matching hardcoded entries - agent-metadata.ts: new isVisionCapableAgentModel() guard blocks non-vision registered models - tools.ts: early vision-capability check before session creation - Added regression tests for variant preservation and non-vision model rejection
- Only block when a resolved model is explicitly not vision-capable - Set up vision cache in model passthrough test for proper isolation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
- Test in
src/tools/look-at/multimodal-agent-metadata.test.tsusesopenai/gpt-5.4as a non-vision model, but it’s in the multimodal fallback chain, so the test may misrepresent behavior - This looks like a moderate, test-scoped issue (severity 6/10) rather than a production regression, so the change feels safe to merge with a small fix
- Pay close attention to
src/tools/look-at/multimodal-agent-metadata.test.ts- choose a truly non-vision model in the test
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/tools/look-at/multimodal-agent-metadata.test.ts">
<violation number="1" location="src/tools/look-at/multimodal-agent-metadata.test.ts:151">
P2: The test uses `openai/gpt-5.4` to represent a non-vision model, but `gpt-5.4` is in the hardcoded multimodal fallback chain (and thus is vision-capable). Replace it with a known non-vision model (e.g., `openai/gpt-5.3-codex`) to properly test rejection of non-vision registered models.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+151
to
+169
| test("does not return a registered model when no vision-capable model is available", async () => { | ||
| // given | ||
| spyOn(modelAvailability, "fetchAvailableModels").mockResolvedValue( | ||
| new Set(["openai/gpt-5.4"]), | ||
| ) | ||
| spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["openai"]) | ||
| const ctx = createPluginInput([ | ||
| { | ||
| name: "multimodal-looker", | ||
| model: { providerID: "openai", modelID: "gpt-5.4" }, | ||
| }, | ||
| ]) | ||
|
|
||
| // when | ||
| const result = await resolveMultimodalLookerAgentMetadata(ctx) | ||
|
|
||
| // then | ||
| expect(result).toEqual({}) | ||
| }) |
There was a problem hiding this comment.
P2: The test uses openai/gpt-5.4 to represent a non-vision model, but gpt-5.4 is in the hardcoded multimodal fallback chain (and thus is vision-capable). Replace it with a known non-vision model (e.g., openai/gpt-5.3-codex) to properly test rejection of non-vision registered models.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/look-at/multimodal-agent-metadata.test.ts, line 151:
<comment>The test uses `openai/gpt-5.4` to represent a non-vision model, but `gpt-5.4` is in the hardcoded multimodal fallback chain (and thus is vision-capable). Replace it with a known non-vision model (e.g., `openai/gpt-5.3-codex`) to properly test rejection of non-vision registered models.</comment>
<file context>
@@ -112,4 +147,24 @@ describe("resolveMultimodalLookerAgentMetadata", () => {
})
})
+
+ test("does not return a registered model when no vision-capable model is available", async () => {
+ // given
+ spyOn(modelAvailability, "fetchAvailableModels").mockResolvedValue(
</file context>
Suggested change
| test("does not return a registered model when no vision-capable model is available", async () => { | |
| // given | |
| spyOn(modelAvailability, "fetchAvailableModels").mockResolvedValue( | |
| new Set(["openai/gpt-5.4"]), | |
| ) | |
| spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["openai"]) | |
| const ctx = createPluginInput([ | |
| { | |
| name: "multimodal-looker", | |
| model: { providerID: "openai", modelID: "gpt-5.4" }, | |
| }, | |
| ]) | |
| // when | |
| const result = await resolveMultimodalLookerAgentMetadata(ctx) | |
| // then | |
| expect(result).toEqual({}) | |
| }) | |
| test("does not return a registered model when no vision-capable model is available", async () => { | |
| // given | |
| spyOn(modelAvailability, "fetchAvailableModels").mockResolvedValue( | |
| new Set(["openai/gpt-5.3-codex"]), | |
| ) | |
| spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["openai"]) | |
| const ctx = createPluginInput([ | |
| { | |
| name: "multimodal-looker", | |
| model: { providerID: "openai", modelID: "gpt-5.3-codex" }, | |
| }, | |
| ]) | |
| // when | |
| const result = await resolveMultimodalLookerAgentMetadata(ctx) | |
| // then | |
| expect(result).toEqual({}) | |
| }) |
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/tools/look-at/tools.test.ts">
<violation number="1" location="src/tools/look-at/tools.test.ts:259">
P2: Global state modification without cleanup risks test pollution.</violation>
</file>
<file name="src/tools/look-at/tools.ts">
<violation number="1" location="src/tools/look-at/tools.ts:151">
P0: Changing the condition to `agentModel && ...` removes the fail-fast behavior when `agentModel` is `undefined`, defeating the PR's goal of blocking non-vision models.
According to the PR description, `resolveMultimodalLookerAgentMetadata` returns empty metadata (`agentModel` is `undefined`) when no vision-capable model is found. Because this condition now evaluates to `false` when `agentModel` is undefined, the tool proceeds to create a session and send a request without a model override. The backend then falls back to using the registered non-vision model anyway.
Restore the `!agentModel` check to properly fail fast when no vision model is available.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+151
to
+156
| if (agentModel && !isVisionCapableResolvedModel(agentModel)) { | ||
| log("[look_at] Resolved model is not vision-capable, blocking", { | ||
| resolvedModel: agentModel, | ||
| }) | ||
| return "Error: Resolved multimodal-looker model is not vision-capable" | ||
| } |
There was a problem hiding this comment.
P0: Changing the condition to agentModel && ... removes the fail-fast behavior when agentModel is undefined, defeating the PR's goal of blocking non-vision models.
According to the PR description, resolveMultimodalLookerAgentMetadata returns empty metadata (agentModel is undefined) when no vision-capable model is found. Because this condition now evaluates to false when agentModel is undefined, the tool proceeds to create a session and send a request without a model override. The backend then falls back to using the registered non-vision model anyway.
Restore the !agentModel check to properly fail fast when no vision model is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/look-at/tools.ts, line 151:
<comment>Changing the condition to `agentModel && ...` removes the fail-fast behavior when `agentModel` is `undefined`, defeating the PR's goal of blocking non-vision models.
According to the PR description, `resolveMultimodalLookerAgentMetadata` returns empty metadata (`agentModel` is `undefined`) when no vision-capable model is found. Because this condition now evaluates to `false` when `agentModel` is undefined, the tool proceeds to create a session and send a request without a model override. The backend then falls back to using the registered non-vision model anyway.
Restore the `!agentModel` check to properly fail fast when no vision model is available.</comment>
<file context>
@@ -148,11 +148,11 @@ Be thorough on what was requested, concise on everything else.
const { agentModel, agentVariant } = await resolveMultimodalLookerAgentMetadata(ctx)
- if (!agentModel || !isVisionCapableResolvedModel(agentModel)) {
- log("[look_at] No vision-capable multimodal-looker model resolved", {
+ if (agentModel && !isVisionCapableResolvedModel(agentModel)) {
+ log("[look_at] Resolved model is not vision-capable, blocking", {
resolvedModel: agentModel,
</file context>
Suggested change
| if (agentModel && !isVisionCapableResolvedModel(agentModel)) { | |
| log("[look_at] Resolved model is not vision-capable, blocking", { | |
| resolvedModel: agentModel, | |
| }) | |
| return "Error: Resolved multimodal-looker model is not vision-capable" | |
| } | |
| if (!agentModel || !isVisionCapableResolvedModel(agentModel)) { | |
| log("[look_at] No vision-capable multimodal-looker model resolved, blocking", { | |
| resolvedModel: agentModel, | |
| }) | |
| return "Error: No vision-capable multimodal-looker model available" | |
| } |
| // when LookAt tool executed | ||
| // then model info should be passed to sync prompt | ||
| test("passes multimodal-looker model to sync prompt when available", async () => { | ||
| setVisionCapableModelsCache(new Map([["google/gemini-3-flash", { providerID: "google", modelID: "gemini-3-flash" }]])) |
There was a problem hiding this comment.
P2: Global state modification without cleanup risks test pollution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/look-at/tools.test.ts, line 259:
<comment>Global state modification without cleanup risks test pollution.</comment>
<file context>
@@ -255,6 +256,8 @@ describe("look-at tool", () => {
// when LookAt tool executed
// then model info should be passed to sync prompt
test("passes multimodal-looker model to sync prompt when available", async () => {
+ setVisionCapableModelsCache(new Map([["google/gemini-3-flash", { providerID: "google", modelID: "gemini-3-flash" }]]))
+
let promptBody: any
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fallback-chain.ts: cache-derived entries inheritvariantfrom matching hardcoded entriesagent-metadata.ts: newisVisionCapableAgentModel()guard blocks non-vision registered modelstools.ts: early vision-capability check before session creation, fail-fast with error messageContext
Oracle regression check on PR #2371 found variant metadata was lost in fallback chain and non-vision models could be returned as vision-capable.
Summary by cubic
Preserves variant metadata in the multimodal fallback chain and ensures the
look-attool only uses vision-capable models. Adds a safe early check that blocks only when a resolved model is explicitly non-vision.isVisionCapableAgentModel; return empty metadata when none.look-at, fail fast only if a resolved model is not vision-capable; do not block when no model is resolved.Written for commit a668860. Summary will update on new commits.