mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 10:23:14 +00:00
fix: reject images for text-only models (#1439)
* fix: reject image uploads for text-only models Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: validate queued images against active model Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
30d5d55f0f
commit
5faf190954
5 changed files with 228 additions and 8 deletions
|
|
@ -10,6 +10,7 @@ class ModelOption(TypedDict):
|
|||
label: str
|
||||
efforts: list[str]
|
||||
default_effort: str
|
||||
supports_images: bool
|
||||
|
||||
|
||||
SUPPORTED_MODELS: list[ModelOption] = [
|
||||
|
|
@ -18,36 +19,42 @@ SUPPORTED_MODELS: list[ModelOption] = [
|
|||
"label": "Opus 4.8",
|
||||
"efforts": ["low", "medium", "high", "xhigh", "max"],
|
||||
"default_effort": "high",
|
||||
"supports_images": True,
|
||||
},
|
||||
{
|
||||
"id": "openai:gpt-5.5",
|
||||
"label": "GPT-5.5",
|
||||
"efforts": ["none", "low", "medium", "high", "xhigh"],
|
||||
"default_effort": "xhigh",
|
||||
"supports_images": True,
|
||||
},
|
||||
{
|
||||
"id": "google_genai:gemini-3.5-flash",
|
||||
"label": "Gemini 3.5 Flash",
|
||||
"efforts": ["minimal", "low", "medium", "high"],
|
||||
"default_effort": "medium",
|
||||
"supports_images": True,
|
||||
},
|
||||
{
|
||||
"id": "fireworks:accounts/fireworks/models/kimi-k2p6",
|
||||
"label": "Kimi K2.6",
|
||||
"efforts": ["none", "low", "medium", "high"],
|
||||
"default_effort": "high",
|
||||
"supports_images": False,
|
||||
},
|
||||
{
|
||||
"id": "fireworks:accounts/fireworks/models/deepseek-v4-pro",
|
||||
"label": "DeepSeek V4 Pro",
|
||||
"efforts": ["none", "low", "medium", "high", "xhigh", "max"],
|
||||
"default_effort": "high",
|
||||
"supports_images": False,
|
||||
},
|
||||
{
|
||||
"id": "fireworks:accounts/fireworks/models/glm-5p1",
|
||||
"label": "GLM 5.1",
|
||||
"efforts": ["none", "low", "medium", "high"],
|
||||
"default_effort": "high",
|
||||
"supports_images": False,
|
||||
},
|
||||
]
|
||||
|
||||
|
|
@ -64,6 +71,13 @@ def model_supports_effort(model_id: str, effort: str) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
def model_supports_images(model_id: str) -> bool:
|
||||
for m in SUPPORTED_MODELS:
|
||||
if m["id"] == model_id:
|
||||
return m["supports_images"]
|
||||
return False
|
||||
|
||||
|
||||
def _provider_of(model_id: str) -> str | None:
|
||||
provider, _, rest = model_id.partition(":")
|
||||
return provider if rest else None
|
||||
|
|
|
|||
|
|
@ -18,9 +18,11 @@ from langgraph_sdk.errors import InternalServerError
|
|||
from pydantic import BaseModel, ConfigDict, Field
|
||||
|
||||
from ..utils.thread_ops import is_thread_active, langgraph_client, queue_message_for_thread
|
||||
from .agent_overrides import normalize_profile_overrides
|
||||
from .message_adapter import state_messages_to_ui
|
||||
from .options import SUPPORTED_MODEL_IDS, model_supports_effort
|
||||
from .options import SUPPORTED_MODEL_IDS, model_supports_effort, model_supports_images
|
||||
from .profiles import get_profile, get_valid_access_token
|
||||
from .team_settings import get_team_default_model
|
||||
from .user_mappings import email_for_login
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -86,6 +88,21 @@ def _normalize_model_choice(
|
|||
return model_id, effort
|
||||
|
||||
|
||||
async def _resolve_agent_model_choice(
|
||||
profile: dict[str, Any],
|
||||
model_id: str | None,
|
||||
effort: str | None,
|
||||
) -> tuple[str, str]:
|
||||
resolved_model, resolved_effort = await get_team_default_model("agent")
|
||||
profile_model, profile_effort = normalize_profile_overrides(profile)
|
||||
if profile_model and profile_effort:
|
||||
resolved_model, resolved_effort = profile_model, profile_effort
|
||||
chosen_model, chosen_effort = _normalize_model_choice(model_id, effort)
|
||||
if chosen_model and chosen_effort:
|
||||
resolved_model, resolved_effort = chosen_model, chosen_effort
|
||||
return resolved_model, resolved_effort
|
||||
|
||||
|
||||
def _now_ms() -> int:
|
||||
return int(datetime.now(UTC).timestamp() * 1000)
|
||||
|
||||
|
|
@ -114,9 +131,14 @@ def _decode_dashboard_image(image: DashboardImageBody) -> bytes:
|
|||
return data
|
||||
|
||||
|
||||
def _image_blocks(images: list[DashboardImageBody]) -> list[dict[str, Any]]:
|
||||
def _image_blocks(
|
||||
images: list[DashboardImageBody], *, model_id: str | None
|
||||
) -> list[dict[str, Any]]:
|
||||
if len(images) > _MAX_DASHBOARD_IMAGES:
|
||||
raise HTTPException(422, f"at most {_MAX_DASHBOARD_IMAGES} images are supported")
|
||||
if images and (not model_id or not model_supports_images(model_id)):
|
||||
model_label = model_id or "the current model"
|
||||
raise HTTPException(422, f"model {model_label} does not support image input")
|
||||
return [
|
||||
create_image_block(
|
||||
base64=base64.b64encode(_decode_dashboard_image(image)).decode("ascii"),
|
||||
|
|
@ -127,14 +149,17 @@ def _image_blocks(images: list[DashboardImageBody]) -> list[dict[str, Any]]:
|
|||
|
||||
|
||||
def _user_message_content(
|
||||
prompt: str, images: list[DashboardImageBody]
|
||||
prompt: str, images: list[DashboardImageBody], *, model_id: str | None = None
|
||||
) -> str | list[dict[str, Any]]:
|
||||
text = prompt.strip()
|
||||
if not text and not images:
|
||||
raise HTTPException(422, "prompt or image required")
|
||||
if not images:
|
||||
return text
|
||||
return [*_image_blocks(images), *([{"type": "text", "text": text}] if text else [])]
|
||||
return [
|
||||
*_image_blocks(images, model_id=model_id),
|
||||
*([{"type": "text", "text": text}] if text else []),
|
||||
]
|
||||
|
||||
|
||||
async def _ensure_dashboard_github_token(login: str) -> None:
|
||||
|
|
@ -158,6 +183,14 @@ def _thread_source(metadata: dict[str, Any]) -> str:
|
|||
return source if isinstance(source, str) and source else _DASHBOARD_SOURCE
|
||||
|
||||
|
||||
def _metadata_model_id(metadata: dict[str, Any]) -> str | None:
|
||||
for key in ("resolved_model", "model"):
|
||||
model = metadata.get(key)
|
||||
if isinstance(model, str) and model in SUPPORTED_MODEL_IDS:
|
||||
return model
|
||||
return None
|
||||
|
||||
|
||||
def _user_owns_thread(metadata: dict[str, Any], login: str, email: str | None) -> bool:
|
||||
if _thread_source(metadata) not in _SURFACED_SOURCES:
|
||||
return False
|
||||
|
|
@ -343,7 +376,8 @@ async def _start_agent_run(
|
|||
profile = await get_profile(login) or {}
|
||||
now_ms = _now_ms()
|
||||
prompt = prompt.strip()
|
||||
content = _user_message_content(prompt, images or [])
|
||||
resolved_model, resolved_effort = await _resolve_agent_model_choice(profile, model_id, effort)
|
||||
content = _user_message_content(prompt, images or [], model_id=resolved_model)
|
||||
chosen_model, chosen_effort = _normalize_model_choice(model_id, effort)
|
||||
metadata_model = chosen_model or profile.get("default_model") or "Default"
|
||||
metadata_effort = chosen_effort or profile.get("reasoning_effort")
|
||||
|
|
@ -356,6 +390,8 @@ async def _start_agent_run(
|
|||
"branch_prefix": profile.get("branch_prefix"),
|
||||
"model": metadata_model,
|
||||
"effort": metadata_effort,
|
||||
"resolved_model": resolved_model,
|
||||
"resolved_effort": resolved_effort,
|
||||
"created_at_ms": now_ms,
|
||||
"updated_at_ms": now_ms,
|
||||
}
|
||||
|
|
@ -433,16 +469,17 @@ async def send_dashboard_message(
|
|||
owner, name, _ = _metadata_repo(metadata)
|
||||
|
||||
prompt = body.content.strip()
|
||||
content = _user_message_content(prompt, body.images)
|
||||
now_ms = _now_ms()
|
||||
chosen_model, chosen_effort = _normalize_model_choice(body.model_id, body.effort)
|
||||
metadata_update: dict[str, Any] = {"source": _DASHBOARD_SOURCE, "updated_at_ms": now_ms}
|
||||
if chosen_model and chosen_effort:
|
||||
metadata_update["model"] = chosen_model
|
||||
metadata_update["effort"] = chosen_effort
|
||||
await client.threads.update(thread_id=thread_id, metadata=metadata_update)
|
||||
|
||||
if await is_thread_active(thread_id):
|
||||
active_model = _metadata_model_id(metadata) if body.images else None
|
||||
content = _user_message_content(prompt, body.images, model_id=active_model)
|
||||
await client.threads.update(thread_id=thread_id, metadata=metadata_update)
|
||||
queue_payload: dict[str, Any] = {"text": prompt, "source": _DASHBOARD_SOURCE}
|
||||
if isinstance(content, list):
|
||||
queue_payload["images"] = [
|
||||
|
|
@ -461,8 +498,16 @@ async def send_dashboard_message(
|
|||
thread if isinstance(thread, dict) else {"thread_id": thread_id, "metadata": metadata}
|
||||
)
|
||||
|
||||
await _ensure_dashboard_github_token(login)
|
||||
profile = await get_profile(login) or {}
|
||||
resolved_model, resolved_effort = await _resolve_agent_model_choice(
|
||||
profile, body.model_id, body.effort
|
||||
)
|
||||
metadata_update["resolved_model"] = resolved_model
|
||||
metadata_update["resolved_effort"] = resolved_effort
|
||||
content = _user_message_content(prompt, body.images, model_id=resolved_model)
|
||||
await client.threads.update(thread_id=thread_id, metadata=metadata_update)
|
||||
|
||||
await _ensure_dashboard_github_token(login)
|
||||
configurable: dict[str, Any] = {
|
||||
"thread_id": thread_id,
|
||||
"source": _DASHBOARD_SOURCE,
|
||||
|
|
|
|||
92
tests/test_dashboard_thread_api.py
Normal file
92
tests/test_dashboard_thread_api.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
import base64
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
from agent.dashboard import thread_api
|
||||
from agent.dashboard.options import model_supports_images
|
||||
|
||||
_TEXT_ONLY_MODEL = "fireworks:accounts/fireworks/models/deepseek-v4-pro"
|
||||
_VISION_MODEL = "openai:gpt-5.5"
|
||||
|
||||
|
||||
def _image() -> thread_api.DashboardImageBody:
|
||||
return thread_api.DashboardImageBody(
|
||||
base64=base64.b64encode(b"image").decode("ascii"),
|
||||
mimeType="image/png",
|
||||
)
|
||||
|
||||
|
||||
def test_model_supports_images_marks_text_only_fireworks_models() -> None:
|
||||
assert not model_supports_images(_TEXT_ONLY_MODEL)
|
||||
assert model_supports_images(_VISION_MODEL)
|
||||
|
||||
|
||||
def test_user_message_content_rejects_images_for_text_only_model() -> None:
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
thread_api._user_message_content("see attached", [_image()], model_id=_TEXT_ONLY_MODEL)
|
||||
|
||||
assert exc_info.value.status_code == 422
|
||||
assert "does not support image input" in exc_info.value.detail
|
||||
|
||||
|
||||
def test_user_message_content_allows_images_for_vision_model() -> None:
|
||||
content = thread_api._user_message_content("see attached", [_image()], model_id=_VISION_MODEL)
|
||||
|
||||
assert isinstance(content, list)
|
||||
assert content[-1] == {"type": "text", "text": "see attached"}
|
||||
assert any(block.get("type") != "text" for block in content)
|
||||
|
||||
|
||||
async def test_resolve_agent_model_choice_applies_profile_before_team_default(monkeypatch) -> None:
|
||||
async def fake_team_default(role: str) -> tuple[str, str]:
|
||||
assert role == "agent"
|
||||
return _VISION_MODEL, "medium"
|
||||
|
||||
monkeypatch.setattr(thread_api, "get_team_default_model", fake_team_default)
|
||||
|
||||
model_id, effort = await thread_api._resolve_agent_model_choice(
|
||||
{"default_model": _TEXT_ONLY_MODEL, "reasoning_effort": "high"},
|
||||
None,
|
||||
None,
|
||||
)
|
||||
|
||||
assert (model_id, effort) == (_TEXT_ONLY_MODEL, "high")
|
||||
|
||||
|
||||
async def test_resolve_agent_model_choice_applies_request_before_profile(monkeypatch) -> None:
|
||||
async def fake_team_default(role: str) -> tuple[str, str]:
|
||||
assert role == "agent"
|
||||
return _VISION_MODEL, "medium"
|
||||
|
||||
monkeypatch.setattr(thread_api, "get_team_default_model", fake_team_default)
|
||||
|
||||
model_id, effort = await thread_api._resolve_agent_model_choice(
|
||||
{"default_model": _TEXT_ONLY_MODEL, "reasoning_effort": "high"},
|
||||
"anthropic:claude-opus-4-8",
|
||||
"high",
|
||||
)
|
||||
|
||||
assert (model_id, effort) == ("anthropic:claude-opus-4-8", "high")
|
||||
|
||||
|
||||
async def test_create_dashboard_thread_rejects_images_for_resolved_text_only_model(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
async def fake_profile(login: str) -> dict[str, str]:
|
||||
assert login == "octocat"
|
||||
return {"default_model": _TEXT_ONLY_MODEL, "reasoning_effort": "high"}
|
||||
|
||||
async def fake_team_default(role: str) -> tuple[str, str]:
|
||||
assert role == "agent"
|
||||
return _VISION_MODEL, "medium"
|
||||
|
||||
monkeypatch.setattr(thread_api, "get_profile", fake_profile)
|
||||
monkeypatch.setattr(thread_api, "get_team_default_model", fake_team_default)
|
||||
|
||||
body = thread_api.ThreadCreateBody(prompt="see attached", images=[_image()])
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await thread_api.create_dashboard_thread("octocat", body)
|
||||
|
||||
assert exc_info.value.status_code == 422
|
||||
assert "does not support image input" in exc_info.value.detail
|
||||
|
|
@ -3,6 +3,7 @@ from __future__ import annotations
|
|||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
from agent.dashboard import thread_api
|
||||
|
||||
|
|
@ -174,6 +175,7 @@ async def test_dashboard_followup_on_busy_thread_queues_images(
|
|||
metadata = {
|
||||
"source": "dashboard",
|
||||
"github_login": "octocat",
|
||||
"resolved_model": "openai:gpt-5.5",
|
||||
}
|
||||
client = _FakeClient(metadata)
|
||||
queued_messages: list[object] = []
|
||||
|
|
@ -209,6 +211,72 @@ async def test_dashboard_followup_on_busy_thread_queues_images(
|
|||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dashboard_followup_on_busy_text_only_thread_rejects_images(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
metadata = {
|
||||
"source": "dashboard",
|
||||
"github_login": "octocat",
|
||||
"resolved_model": "fireworks:accounts/fireworks/models/deepseek-v4-pro",
|
||||
}
|
||||
client = _FakeClient(metadata)
|
||||
queued_messages: list[object] = []
|
||||
|
||||
async def fake_queue_message_for_thread(thread_id: str, message_content: object) -> bool:
|
||||
queued_messages.append(message_content)
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(thread_api, "langgraph_client", lambda: client)
|
||||
monkeypatch.setattr(thread_api, "is_thread_active", _active_thread)
|
||||
monkeypatch.setattr(thread_api, "queue_message_for_thread", fake_queue_message_for_thread)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await thread_api.send_dashboard_message(
|
||||
"thread-1",
|
||||
"octocat",
|
||||
thread_api.ThreadMessageBody(
|
||||
content="continue in web",
|
||||
images=[thread_api.DashboardImageBody(base64="aW1hZ2U=", mimeType="image/png")],
|
||||
model_id="openai:gpt-5.5",
|
||||
effort="medium",
|
||||
),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 422
|
||||
assert "does not support image input" in exc_info.value.detail
|
||||
assert queued_messages == []
|
||||
assert client.threads.updates == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dashboard_followup_on_busy_unknown_model_rejects_images(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
metadata = {
|
||||
"source": "dashboard",
|
||||
"github_login": "octocat",
|
||||
}
|
||||
client = _FakeClient(metadata)
|
||||
|
||||
monkeypatch.setattr(thread_api, "langgraph_client", lambda: client)
|
||||
monkeypatch.setattr(thread_api, "is_thread_active", _active_thread)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await thread_api.send_dashboard_message(
|
||||
"thread-1",
|
||||
"octocat",
|
||||
thread_api.ThreadMessageBody(
|
||||
content="continue in web",
|
||||
images=[thread_api.DashboardImageBody(base64="aW1hZ2U=", mimeType="image/png")],
|
||||
),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 422
|
||||
assert "does not support image input" in exc_info.value.detail
|
||||
assert client.threads.updates == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dashboard_followup_preserves_explicit_repo_less_thread(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ export interface ModelOption {
|
|||
label: string;
|
||||
efforts: Array<string>;
|
||||
default_effort: string;
|
||||
supports_images: boolean;
|
||||
}
|
||||
|
||||
export interface OptionsPayload {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue