diff --git a/raven/agent/context/builder.py b/raven/agent/context/builder.py index 30ae4f49..cda51600 100644 --- a/raven/agent/context/builder.py +++ b/raven/agent/context/builder.py @@ -1,7 +1,5 @@ """Context builder for assembling agent prompts.""" -import base64 -import mimetypes import platform import time from datetime import datetime @@ -12,7 +10,7 @@ from raven.memory_engine.skill_forge import LocalSkillCatalog from raven.memory_engine.skill_local.types import SkillMeta from raven.security.trust import wrap_untrusted, wrap_untrusted_blocks -from raven.utils.helpers import build_assistant_message, detect_image_mime, image_block +from raven.utils.helpers import build_assistant_message if TYPE_CHECKING: from raven.providers.base import LLMProvider @@ -280,35 +278,26 @@ def build_messages( {"role": "user", "content": merged}, ] - def _build_user_content(self, text: str, media: list[str] | None) -> str | list[dict[str, Any]]: - """Build user message content with optional base64-encoded images. - - Every image's path is named in the text as well: the base64 survives only - this turn (the session stores a placeholder), so the path is what lets a - later turn re-read the picture instead of only learning one existed. + def _build_user_content( + self, + text: str, + media: list[str] | None, + *, + can_see_images: bool = True, + describe_tool: str | None = None, + ) -> str | list[dict[str, Any]]: + """Build user message content with optional attachments. + + Delegates to the one implementation rather than keeping a second: this + builder only feeds MemoryConsolidator's token estimation today, so a + divergence here would be invisible until someone routed a real turn + through it, and by then the two would have drifted. The vision-aware + arguments are carried for that day rather than used now -- the estimator + passes no media at all, so nothing reaches the attachment path yet. """ - if not media: - return text - - images = [] - notes = [] - for path in media: - p = Path(path) - if not p.is_file(): - continue - raw = p.read_bytes() - # Detect real MIME type from magic bytes; fallback to filename guess - mime = detect_image_mime(raw) or mimetypes.guess_type(path)[0] - if not mime or not mime.startswith("image/"): - continue - b64 = base64.b64encode(raw).decode() - images.append(image_block(f"data:{mime};base64,{b64}")) - notes.append(f"[Image: {p.name} (path: {p}) — re-read it with read_file if you need another look]") - - if not images: - return text - body = (f"{text}\n\n" if text else "") + "\n".join(notes) - return images + [{"type": "text", "text": body}] + from raven.context_engine.segments import render + + return render.build_user_content(text, media, can_see_images=can_see_images, describe_tool=describe_tool) def add_tool_result( self, diff --git a/raven/agent/loop/main.py b/raven/agent/loop/main.py index 1ddadc64..ee809bcc 100644 --- a/raven/agent/loop/main.py +++ b/raven/agent/loop/main.py @@ -44,7 +44,7 @@ from raven.memory_engine.base import TokenBudget from raven.memory_engine.consolidate.consolidator import MemoryConsolidator, MemoryStore from raven.providers.base import LLMProvider, LLMResponse, ToolCallRequest -from raven.providers.capabilities import image_placeholder_text, supports_image_tool_result +from raven.providers.capabilities import image_placeholder_text, supports_image_tool_result, vision_verdict from raven.sandbox import SandboxConfig, SandboxExecutor, SandboxInitError, build_executor from raven.session.manager import Session, SessionManager from raven.spine.turn import Origin @@ -373,6 +373,7 @@ def __init__( # takes a per-call model (strategies rewrite it, and the model chain # falls back), so one model's verdict must not answer for another's. self._image_tool_result_ok: dict[str, bool] = {} + self._vision_ok: dict[str, bool] = {} self.max_iterations = max_iterations # Empty-response recovery budgets. None → enabled defaults. self._recovery_limits = empty_recovery if empty_recovery is not None else RecoveryLimits() @@ -799,6 +800,94 @@ def _supports_image_tool_result(self, model: str | None = None) -> bool: ) return self._image_tool_result_ok[key] + # The tool that can read an attachment for a model that cannot see it. + # Contributed by the EverOS plugin, so absent on a default install. + _DESCRIBE_TOOL = "understand_media" + + def _describe_tool_name(self) -> str | None: + """The description tool's name if it is registered, else ``None``. + + Checked rather than assumed: the note that replaces a picture points at + this tool, and pointing at one the model was never given is an + instruction it cannot follow. + """ + return self._DESCRIBE_TOOL if self.tools.get(self._DESCRIBE_TOOL) else None + + def _route_result_images( + self, + model_text: str, + blocks: list[dict[str, Any]] | None, + model: str, + ) -> tuple[str, list[dict[str, Any]] | None, list[dict[str, Any]] | None]: + """Decide how a tool result's pictures reach ``model``. + + Returns the text the tool result carries, the blocks to put *in* it, and + the blocks to attach to a following user message. Exactly one of the last + two is ever populated. + + Three outcomes, and the wording differs because the model's next move + differs. No vision at all: nothing follows, so the note must not promise + an attachment, and it names the description tool when one is registered. + Vision and a transport that carries images in a ``role="tool"`` message: + the blocks ride along untouched. Vision but a transport that cannot (every + OpenAI-style Chat Completions endpoint -- image is excluded from the tool + role at the schema level): the result carries text and the picture follows + in a user message, the shape OpenClaw uses. + + A method rather than a branch inside the loop so it can be tested at all: + the loop reaches this point only through a live provider and a real tool + call, and the wrong choice here is silent -- the model answers about a + picture it never received. + """ + if not blocks: + return model_text, blocks, None + if not self._supports_vision(model): + return image_placeholder_text(blocks, blind=True, describe_tool=self._describe_tool_name()), None, None + if self._supports_image_tool_result(model): + return model_text, blocks, None + attach = [b for b in blocks if b.get("type") == "image_url"] + return image_placeholder_text(blocks), None, attach + + def _supports_vision(self, model: str | None = None) -> bool: + """Cached per model: whether this model can see a picture at all. + + Asked once per turn and once per tool result that returns an image, and + the lookup joins the model string against the gateway catalogue -- same + reason the sibling probe above is cached. + + Only a real verdict is cached. ``vision_verdict`` returns ``None`` while + the catalog has no answer -- a cold install, before the background warm + lands -- and that is optimism rather than knowledge: caching it would + freeze the guess for the life of this loop, which is the life of the + process, and the warm would then fill a table nothing re-reads. An + unknown model is re-asked each turn, which costs a dict lookup. + + The verdict is the routed primary's. A fallback further down the chain is + sent the same message list, so a vision-capable primary with a blind + fallback hands the blind endpoint an image block it will refuse; that + refusal classifies as fatal and stops the chain rather than answering + blind. Pre-existing in the sibling probe too, and it needs the fallback + chain to be assembled per candidate to fix properly. + """ + key = model or self.model + cached = self._vision_ok.get(key) + if cached is not None: + return cached + + spec = None + try: + from raven.providers.registry import find_by_model + + spec = find_by_model(key) + except Exception: + pass + verdict = vision_verdict(key, spec, self.provider) + logger.debug("vision support for {}: {}", key, verdict) + if verdict is None: + return True + self._vision_ok[key] = verdict + return verdict + # ── Context engine helpers ────────────────────────────────────────── def _context_messages_for_session(self, session: Session) -> list[dict[str, Any]]: @@ -865,8 +954,15 @@ async def _assemble_context_messages( channel: str | None = None, chat_id: str | None = None, selected_skills: list[Any] | None = None, + model: str | None = None, ) -> list[dict[str, Any]]: - """Ask the active context engine for the main-agent message window.""" + """Ask the active context engine for the main-agent message window. + + ``model`` is the id the request will actually reach (the router's pick, + when there is one). It decides whether an attachment is inlined as a + picture, so defaulting it to ``self.model`` would let the configured + model answer for a routed one. + """ from raven.context_engine import TurnContext # deferred — see module note # Phase A / Phase C tidy: reset the metadata stash BEFORE calling @@ -883,6 +979,8 @@ async def _assemble_context_messages( turn=TurnContext( current_message=current_message, media=media, + can_see_images=self._supports_vision(model), + describe_tool=self._describe_tool_name(), channel=channel, chat_id=chat_id, selected_skills=selected_skills, @@ -1863,19 +1961,9 @@ async def _run_agent_loop( "truncated": len(display_src) > 200, }, ) - blocks = getattr(result, "blocks", None) - attach_blocks: list[dict[str, Any]] | None = None - if blocks: - if self._supports_image_tool_result(call_model or effective_model): - pass # blocks ride in the tool result itself - else: - # This transport cannot put an image in a tool result, - # so the tool result carries text naming the image and - # the picture follows in a user message. Same shape - # OpenClaw uses against Chat Completions endpoints. - model_text = image_placeholder_text(blocks) - attach_blocks = [b for b in blocks if b.get("type") == "image_url"] - blocks = None + model_text, blocks, attach_blocks = self._route_result_images( + model_text, getattr(result, "blocks", None), call_model or effective_model + ) if blocks: messages = self.context.add_tool_result( messages, tool_call.id, tool_call.name, model_text, blocks @@ -2380,17 +2468,12 @@ async def _extract(): content, context_messages, ) - initial_messages = await self._assemble_context_messages( - session=session, - session_key=key, - current_message=content, - media=media_paths if media_paths else None, - channel=channel, - chat_id=chat_id, - selected_skills=selected_skills or None, - ) - # ── Model routing (EcoClaw-style) ──────────────────────────────────── + # Ahead of assembly, not after it: assembly decides whether an + # attachment is inlined as a picture or described in text, and that + # question is about the model the request will actually reach. Routing + # needs only ``content``, so asking first costs nothing and stops one + # model's verdict from shaping a message another model receives. routed_model: str | None = None fallback_models: list[str] = [] if self.router is not None: @@ -2400,6 +2483,17 @@ async def _extract(): if fallback_models: logger.info("Router fallback chain: {}", fallback_models) + initial_messages = await self._assemble_context_messages( + session=session, + session_key=key, + current_message=content, + media=media_paths if media_paths else None, + channel=channel, + chat_id=chat_id, + selected_skills=selected_skills or None, + model=routed_model, + ) + extraction_sid = None # Phase B-1: embedded extraction removed; always None now. turn_start_idx = len(initial_messages) - 1 final_content, _, all_msgs, outcome = await self._run_agent_loop( diff --git a/raven/agent/tools/filesystem.py b/raven/agent/tools/filesystem.py index 1255948b..b09f4633 100644 --- a/raven/agent/tools/filesystem.py +++ b/raven/agent/tools/filesystem.py @@ -54,7 +54,7 @@ def description(self) -> str: return ( "Read the contents of a file. Text files return numbered lines — use offset and limit to " "paginate through large ones. Image files (PNG, JPEG, GIF, WebP, and other common formats) " - "return the picture itself when the active model can see images, downscaled if needed." + "return the picture itself, downscaled if needed." ) @property diff --git a/raven/context_engine/assembler.py b/raven/context_engine/assembler.py index a2c990dd..a44707fa 100644 --- a/raven/context_engine/assembler.py +++ b/raven/context_engine/assembler.py @@ -76,6 +76,8 @@ async def assemble( session_key=session_key, current_message=turn.current_message, media=turn.media, + can_see_images=turn.can_see_images, + describe_tool=turn.describe_tool, channel=turn.channel, chat_id=turn.chat_id, session_messages=session_messages, @@ -144,7 +146,12 @@ async def after_turn( def _build_user(self, ctx: AssemblyContext) -> dict[str, Any]: """The single structural user message: runtime context + content.""" runtime_ctx = render.build_runtime_context(self._now_fn, ctx.channel, ctx.chat_id) - user_content = render.build_user_content(ctx.current_message, ctx.media) + user_content = render.build_user_content( + ctx.current_message, + ctx.media, + can_see_images=ctx.can_see_images, + describe_tool=ctx.describe_tool, + ) if isinstance(user_content, str): merged: Any = f"{runtime_ctx}\n\n{user_content}" else: diff --git a/raven/context_engine/base.py b/raven/context_engine/base.py index 398387ff..0f8f0d26 100644 --- a/raven/context_engine/base.py +++ b/raven/context_engine/base.py @@ -79,6 +79,8 @@ class AssemblyContext: session_messages: list[dict[str, Any]] budget: TokenBudget prefix: AssembledPrefix | None = None + can_see_images: bool = True + describe_tool: str | None = None @dataclass diff --git a/raven/context_engine/curator.py b/raven/context_engine/curator.py index 9864b213..539fae1a 100644 --- a/raven/context_engine/curator.py +++ b/raven/context_engine/curator.py @@ -40,6 +40,15 @@ class TurnContext: channel: str | None = None chat_id: str | None = None selected_skills: list[Any] | None = None + # Whether this turn's model can see a picture. Decided by the loop (it owns + # the provider and the model id) and carried here because the message is + # built down in render, which knows neither. Defaults True so a caller that + # does not set it keeps the old inline-everything behavior. + can_see_images: bool = True + # Name of a registered tool that can read an attachment the model cannot, + # or None when none is (it comes from an optional plugin). Naming a tool the + # model does not have reads as an instruction it cannot follow. + describe_tool: str | None = None @dataclass diff --git a/raven/context_engine/segments/curator.py b/raven/context_engine/segments/curator.py index c443021b..44cde03f 100644 --- a/raven/context_engine/segments/curator.py +++ b/raven/context_engine/segments/curator.py @@ -95,6 +95,8 @@ async def build(self, ctx: AssemblyContext) -> Segment | None: turn = TurnContext( current_message=ctx.current_message, media=ctx.media, + can_see_images=ctx.can_see_images, + describe_tool=ctx.describe_tool, channel=ctx.channel, chat_id=ctx.chat_id, ) diff --git a/raven/context_engine/segments/render.py b/raven/context_engine/segments/render.py index 58104670..6edba944 100644 --- a/raven/context_engine/segments/render.py +++ b/raven/context_engine/segments/render.py @@ -16,9 +16,29 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Callable +from loguru import logger + from raven.security.trust import wrap_untrusted from raven.utils.helpers import detect_image_mime, image_block +# Ceilings on what one message may carry. ``prepare_image`` caps each image on +# its own (1568 tokens, 4.5MB of base64); nothing capped the whole message, and a +# caller may hand over an arbitrarily long media list. Both a count and a byte +# budget are needed: 16 images is about 25k image tokens, which is affordable, +# while 16 images at the per-image byte cap is a ~72MB request body, which every +# major provider refuses outright -- so a legitimate batch would fail the turn +# instead of degrading. The input ceiling is per image and checked by ``stat`` +# before the file is read whole. +_MAX_INLINE_IMAGES = 16 +_MAX_INLINE_BASE64_BYTES = 16 * 1024 * 1024 +_MAX_IMAGE_BYTES = 64 * 1024 * 1024 +# Enough for every magic number ``detect_image_mime`` looks for. +_SNIFF_BYTES = 64 +# What to say when the model can reach the file itself. Named rather than +# interpolated from ``describe_tool``: read_file is always registered, so unlike +# the description tool this hint is never a promise the model cannot keep. +_READ_FILE_HINT = " — use the read_file tool to see it" + if TYPE_CHECKING: from raven.memory_engine.backend import Memory @@ -192,41 +212,140 @@ def build_runtime_context( return RUNTIME_CONTEXT_TAG + "\n" + "\n".join(lines) -def build_user_content(text: str, media: list[str] | None) -> str | list[dict[str, Any]]: +def build_user_content( + text: str, + media: list[str] | None, + *, + can_see_images: bool = True, + describe_tool: str | None = None, +) -> str | list[dict[str, Any]]: """User message content with attachments. - Images are inlined as base64 ``image_url`` blocks so a vision-capable - model sees them directly. Non-image attachments (PDF, audio, Office - docs, …) can't ride in the message, so their paths are surfaced as a - text note — the model reads them on demand via the ``understand_media`` - tool (contributed by the EverOS plugin). Returns a plain ``str`` when - there are no image blocks. + Images are inlined as base64 ``image_url`` blocks so a vision-capable model + sees them directly, downscaled and recompressed first by the same + preprocessing ``read_file`` uses: a phone photo is several megabytes and + thousands of patch tokens, and every target either refuses it or downsizes it + server-side and bills for the original. Returns a plain ``str`` when there + are no image blocks. + + Non-image attachments (PDF, audio, Office docs, …) can't ride in the message, + so their paths are surfaced as a text note for the model to read on demand. Each image also gets its path named in the text, the same way non-image attachments already do. The base64 lives for exactly this turn — it is replaced by a placeholder on the way into the session — so without the path the model loses any way to look at the picture again, and a follow-up question about it has nothing to work from. + + ``can_see_images=False`` (the model has no vision) turns a picture into the + same kind of note the other attachments get. Said out loud rather than + dropped: a text-only endpoint handed an image block either rejects the + request or, worse, discards the picture and answers anyway. Lazy on purpose — + describing every attachment up front would spend a vision call on the ones a + turn only means to move or rename. + + ``describe_tool`` names the tool that can read an attachment, or is ``None`` + when no such tool is registered (it is contributed by the EverOS plugin and + absent on a default install). Pointing at a tool the model does not have + reads as an instruction it cannot follow, so the note then says only what is + there and leaves the path. + + Anything refused -- an unreadable file, one too large, an image past a + ceiling -- becomes a note as well. This runs deep inside turn assembly, where + a raised ``OSError`` surfaces as a failed turn rather than as a sentence about + one attachment. """ if not media: return text images: list[dict[str, Any]] = [] notes: list[str] = [] + inlined_bytes = 0 + hint = f" — use the {describe_tool} tool to read its contents" if describe_tool else "" for path in media: p = Path(path) if not p.is_file(): continue - raw = p.read_bytes() - mime = detect_image_mime(raw) or mimetypes.guess_type(path)[0] - if mime and mime.startswith("image/"): - b64 = base64.b64encode(raw).decode() - images.append(image_block(f"data:{mime};base64,{b64}")) - notes.append(f"[Image: {p.name} (path: {p}) — re-read it with read_file if you need another look]") - else: - notes.append(f"[Attachment: {p.name} (path: {p}) — use the understand_media tool to read its contents]") + try: + size = p.stat().st_size + with p.open("rb") as handle: + # Sniffed from the header alone. Only an image is ever read whole: + # a non-image is named in a note, and reading a 60MB PDF in full to + # look at its first bytes buys nothing. + head = handle.read(_SNIFF_BYTES) + mime = detect_image_mime(head) or mimetypes.guess_type(path)[0] + is_image = bool(mime and mime.startswith("image/")) + if not is_image: + # No fallback hint when there is no description tool. The + # obvious candidate, read_file, decodes text and images and + # errors on a real PDF ("'utf-8' codec can't decode byte + # 0xff"), so naming it here would just be a different + # instruction the model cannot follow. + notes.append(f"[Attachment: {p.name} (path: {p}){hint}]") + continue + # Every reason to refuse is settled before the file is read + # whole. The bytes exist only to inline a picture, so a model + # that cannot see one, or a message with no room left, must not + # pay to load it -- the header already answered the only + # question the note needs. + if not can_see_images: + notes.append(f"[Image: {p.name} (path: {p}) — you cannot see images directly{hint}]") + continue + if size > _MAX_IMAGE_BYTES: + # Past the blind check, so this model can see: read_file is + # the tool that would hand it the picture, and it downscales + # rather than refusing on size. + notes.append( + f"[Image: {p.name} (path: {p}) — too large to read into this message{_READ_FILE_HINT}]" + ) + continue + if len(images) >= _MAX_INLINE_IMAGES or inlined_bytes >= _MAX_INLINE_BASE64_BYTES: + # ``read_file``, not the description tool: this model can + # see, so the useful next step is to fetch the picture + # itself in a later turn. + notes.append( + f"[Image: {p.name} (path: {p}) — not shown, this message is already carrying " + f"{len(images)} images{_READ_FILE_HINT}]" + ) + continue + raw = head + handle.read() + except OSError as e: + # Resolution only proved the path pointed at a file. Between that and + # here it can have lost its permissions or gone away entirely, and an + # unreadable attachment must cost its own note, not the turn. + notes.append(f"[Attachment: {p.name} (path: {p}) — could not be read: {e.strerror or e}]") + continue + block = _inline_image(raw, mime, p, notes) + if block is not None: + images.append(block) + inlined_bytes += len(block.get("image_url", {}).get("url", "")) body = text if notes: body = (f"{text}\n\n" if text else "") + "\n".join(notes) if not images: return body return images + [{"type": "text", "text": body}] + + +def _inline_image(raw: bytes, mime: str, path: Path, notes: list[str]) -> dict[str, Any] | None: + """One image, preprocessed and encoded, with its note appended. + + Preprocessing can fail (a truncated upload, a format Pillow cannot decode, + an image that will not fit the size ceiling at a usable resolution). An + attachment is the user's own doing, so a failure is reported in the note + rather than silently dropping the file or failing the turn. + """ + from raven.agent.tools import media as media_prep + + try: + payload, out_mime, meta = media_prep.prepare_image(raw, mime) + except Exception as e: + logger.warning("attachment {} could not be prepared ({}); naming it instead", path.name, e) + notes.append(f"[Image: {path.name} (path: {path}) — could not be prepared for viewing: {e}]") + return None + + detail = f"{meta['width']}x{meta['height']}px" + if meta.get("resized"): + detail += f", downscaled from {meta['original_width']}x{meta['original_height']}" + notes.append(f"[Image: {path.name} (path: {path}) | {detail} — re-read it with read_file if you need another look]") + b64 = base64.b64encode(payload).decode() + return image_block(f"data:{out_mime};base64,{b64}") diff --git a/raven/plugin/memory/everos/tools.py b/raven/plugin/memory/everos/tools.py index cb375dda..265b24c6 100644 --- a/raven/plugin/memory/everos/tools.py +++ b/raven/plugin/memory/everos/tools.py @@ -38,8 +38,8 @@ def description(self) -> str: "text: PDFs, audio (transcription), Office documents " "(docx/xlsx/pptx), and http(s) URLs (fetched and parsed by " "content type). Pass the file path(s) shown in the " - "'[Attachment: ...]' notes of the user message, and/or http(s) " - "URLs. For an image prefer read_file, which hands you the picture " + "'[Attachment: ...]' or '[Image: ...]' notes of the user message, " + "and/or http(s) URLs. For an image prefer read_file, which hands you the picture " "itself; reach for this tool on an image only when you cannot see " "images or when a scan needs OCR — it returns another model's " "transcription, not the original. Video is not supported." @@ -55,7 +55,8 @@ def parameters(self) -> dict[str, Any]: "items": {"type": "string"}, "description": ( "File path(s) to understand, exactly as shown in the " - "'[Attachment: (path: )]' notes, and/or " + "'[Attachment: (path: )]' or " + "'[Image: (path: )]' notes, and/or " "http(s) URL(s) to fetch and read." ), }, diff --git a/raven/providers/capabilities.py b/raven/providers/capabilities.py index 02766042..b8974fe1 100644 --- a/raven/providers/capabilities.py +++ b/raven/providers/capabilities.py @@ -13,6 +13,11 @@ The second question decides whether ``read_file`` hands the model a picture or a text placeholder plus a follow-up attachment, so it is answered per target, not per model. + +The first is :func:`supports_vision`, answered per model from the gateway +catalog. Both have to be asked: a model that cannot see is not helped by a +transport that could have carried the picture, and a model that can see still +loses it over a transport that cannot. """ from __future__ import annotations @@ -66,6 +71,11 @@ # model is not blind -- the picture is discarded in transit. A refusal is # recoverable (ErrorClassification's should_drop_tool_images retries on the # placeholder path); a silent drop is undetectable by any mechanism. +# Route prefixes whose second segment is a name the user chose rather than a +# model id. LiteLLM's spelling for an Azure deployment; the registry has no +# route by this name, so nothing else recognizes it. +_DEPLOYMENT_NAME_PREFIXES = frozenset({"azure", "azure_ai", "azure_text"}) + GATEWAY_TARGETS = frozenset({"openrouter"}) GATEWAY_IMAGE_TOOL_RESULT_PREFIXES = ("anthropic/claude-", "google/gemini-") @@ -114,6 +124,116 @@ def supports_image_tool_result(provider: Any, model: str, spec: "ProviderSpec | return target in IMAGE_TOOL_RESULT_TARGETS +def _model_id_is_caller_chosen(model: str, provider: Any, spec: "ProviderSpec | None") -> bool: + """Does this route's model string name a deployment rather than a model? + + Azure takes the name of a deployment the user created, and a local runtime + takes whatever tag the user pulled or served under. Either can be spelled + exactly like a vendor id it does not serve -- ``gpt-4`` is the deployment name + Azure's own quickstarts use, and teams keep the name while repointing the + deployment at a newer model -- so joining it against a vendor catalogue + answers about somebody else's model. Only a *denial* does damage (a grant is + what absence already gives), and a denial here is the silent failure this + module exists to avoid, so the catalogue is not consulted for these at all. + + Asked three ways because Azure arrives three ways. Configured as a Raven + provider it is served by ``AzureOpenAIProvider`` and the model string is a + bare deployment name -- no prefix resolves ``find_by_model`` to the Azure + spec, and ``gpt-4`` alone is indistinguishable from OpenAI's own id, so only + the live provider instance knows. Routed through LiteLLM instead it carries + LiteLLM's ``azure/`` prefix, which the registry does not answer to and which + always introduces a deployment name. And a local runtime is named by a spec + that says so. + """ + from raven.providers.azure_openai_provider import AzureOpenAIProvider + from raven.providers.registry import split_model_id + + if isinstance(provider, AzureOpenAIProvider): + return True + if split_model_id(model)[0] in _DEPLOYMENT_NAME_PREFIXES: + return True + if spec is None: + return False + return bool(spec.is_local) or spec.client == "azure" + + +def vision_verdict( + model: str, + spec: "ProviderSpec | None" = None, + provider: Any = None, +) -> bool | None: + """What is *known* about ``model`` seeing images: True, False, or unknown. + + ``None`` is the load-bearing case and the reason this sits under + :func:`supports_vision` rather than inside it. It means no answer exists yet + -- an unlisted model, a deployment name, or a catalog not warm -- which a + caller must not memoize: caching the optimistic default that ``None`` becomes + would freeze a cold-start guess for the life of the process and the warm + behind it would fill a table nobody re-reads. + """ + if spec is not None and spec.vision_override is not None: + return spec.vision_override + if _model_id_is_caller_chosen(model, provider, spec): + return None + + # Imported inside the call: pricing reaches back into this package, so a + # module-level import here would close the loop. + from raven.token_wise.pricing import openrouter_input_modalities, warm_catalog_in_background + + try: + mods = openrouter_input_modalities(model) + except Exception as e: + # The catalog degrades rather than raising, but a capability probe must + # never be the thing that fails a turn. + logger.debug("vision_verdict: catalog lookup failed for {}: {}", model, e) + return None + if mods is None: + warm_catalog_in_background() + return None + return "image" in mods + + +def supports_vision( + model: str, + spec: "ProviderSpec | None" = None, + provider: Any = None, +) -> bool: + """Whether ``model`` can see an image at all. + + The other half of this module's opening question, and the one that decides + whether a picture is inlined into the user message or replaced by a note + telling the model to read it another way. + + Answered from the gateway catalog Raven already fetches and caches for + pricing (:func:`raven.token_wise.pricing.openrouter_input_modalities`), which + publishes ``input_modalities`` for every model it lists. That completeness is + the reason it is the source rather than LiteLLM's price table: the table + states ``supports_vision`` on under a third of its rows, and reading the + silence on the other two thirds as a denial would take a picture that reaches + Grok, Llama 4 and the Qwen-VL family today and replace it with prose. + + No entry at all means yes, which leaves the model exactly where it was before + this function existed. Being wrong that way is loud -- the endpoint refuses + the request and the turn fails. Being wrong the other way is silent: the + picture never arrives and the model answers from the surrounding text as + though it had seen one. There is no automatic recovery in either direction on + the attachment path (``should_drop_tool_images`` rescues an image out of a + *tool result*, not out of a user message), so the choice is between a visible + failure and an invisible one. :attr:`ProviderSpec.vision_override` settles a + model the catalog gets wrong or never lists. + + The catalog is read from cache only, never fetched here, so on a cold install + the first answers are the optimistic default while a background warm fills + it. The pricing path cannot be left to do that warming -- it reaches this + catalog only for models LiteLLM's static table misses, which excludes every + model Raven ships a default for. + + A caller that caches this answer wants :func:`vision_verdict` instead, which + says whether there was an answer to cache. + """ + return vision_verdict(model, spec, provider) is not False + + def _gateway_route(model: str) -> str: """The gateway's own model id, with the gateway prefix stripped. @@ -125,17 +245,41 @@ def _gateway_route(model: str) -> str: return route.lower() -def image_placeholder_text(blocks: list[dict[str, Any]]) -> str: - """Text standing in for images the current transport cannot carry. +def image_placeholder_text( + blocks: list[dict[str, Any]], + *, + blind: bool = False, + describe_tool: str | None = None, +) -> str: + """Text standing in for images the model will not receive. Keeps the tool's own text (it already names the file and its geometry) and appends a line per dropped image so the model knows a picture exists and where it came from, rather than silently seeing nothing. + + Two different reasons, and the model has to be told them apart. By default + the transport cannot put an image in a tool result, so the picture follows + in a user message and the note says so. With ``blind=True`` the model has + no vision at all: nothing follows, and saying it did would leave the model + waiting for a picture that never arrives -- so the note points at the tool + that can read the image for it instead. + + ``describe_tool`` names that tool, or is ``None`` when the caller has none to + offer: it is contributed by the EverOS plugin and absent on a default + install, and naming a tool the model was never given is an instruction it + cannot follow. The note then says only that a picture exists. """ texts = [b.get("text", "") for b in blocks if isinstance(b, dict) and b.get("type") == "text"] images = sum(1 for b in blocks if isinstance(b, dict) and b.get("type") == "image_url") body = "\n".join(t for t in texts if t) if images: noun = "image" if images == 1 else "images" - body += f"\n[{images} {noun} attached to the following message — this endpoint cannot carry images in a tool result]" + if blind: + hint = f"; use the {describe_tool} tool to read the file" if describe_tool else "" + body += f"\n[{images} {noun} not shown — you cannot see images directly{hint}]" + else: + body += ( + f"\n[{images} {noun} attached to the following message — " + "this endpoint cannot carry images in a tool result]" + ) return body.strip() diff --git a/raven/providers/registry.py b/raven/providers/registry.py index a27b7a2b..e22aee8a 100644 --- a/raven/providers/registry.py +++ b/raven/providers/registry.py @@ -98,6 +98,15 @@ class ProviderSpec: # attach the picture to a following user message. image_tool_result_override: bool | None = None + # Whether the models behind this provider can see images at all. None = ask + # the gateway catalogue per model (see supports_vision), which answers "yes" + # whenever it has no entry -- so a provider it does not carry needs nothing + # set here. Set it to False for a provider whose models the catalogue lists + # as vision-capable but this route does not serve that way, and to True to + # overrule a listing that is wrong in the other direction. Source-level, like + # image_tool_result_override: there is no config surface for either. + vision_override: bool | None = None + # Onboard wizard fallback for agents.defaults.model when /v1/models is empty default_model: str = "" diff --git a/raven/token_wise/model_catalog_cache.py b/raven/token_wise/model_catalog_cache.py index 731a93b1..e7b46e85 100644 --- a/raven/token_wise/model_catalog_cache.py +++ b/raven/token_wise/model_catalog_cache.py @@ -24,7 +24,12 @@ from raven.config.paths import get_cache_dir # Bump to force-invalidate every on-disk file after a schema change. -CACHE_VERSION = 1 +# v2 added input_modalities to each entry: a v1 file carries no modality data, +# and reading its silence as "no modalities" would read as "cannot see images". +# v3 dropped the punctuation-stripped join keys v2 also filed, which a +# case-folded lookup can now match against a deployment name -- exactly the +# false "cannot see images" the key was removed to prevent. +CACHE_VERSION = 3 CACHE_FILENAME = "model-catalog.json" # Test seam: when set, overrides the on-disk location so tests never touch the diff --git a/raven/token_wise/pricing.py b/raven/token_wise/pricing.py index 6d4fa441..ab52119d 100644 --- a/raven/token_wise/pricing.py +++ b/raven/token_wise/pricing.py @@ -24,6 +24,7 @@ from __future__ import annotations import pathlib +import threading import time from functools import lru_cache @@ -50,6 +51,11 @@ _OPENROUTER_CACHE_TTL = 3600 _OPENROUTER_CACHE: dict[str, dict] = {} _OPENROUTER_CACHE_TIME: float = 0.0 +# Monotonic stamp of the last background warm attempt (0 = never), and how +# long a failed one waits before another is allowed. See +# warm_catalog_in_background. +_WARM_AT: float = 0.0 +_WARM_RETRY_SECONDS = 300.0 def _litellm_price_table() -> dict: @@ -247,9 +253,17 @@ def _fetch_openrouter_models() -> dict[str, dict]: model_id = model.get("id", "") if not model_id: continue + arch = model.get("architecture") or {} + mods = arch.get("input_modalities") entry = { "pricing": model.get("pricing") or {}, "context_length": model.get("context_length"), + # What the model accepts as input ("text" / "image" / "audio" / + # "file" / "video"). The catalog is fetched for prices, and it is + # also the only published answer to "can this model see" that + # states itself for every model it lists -- see + # ``capabilities.supports_vision``. + "input_modalities": list(mods) if isinstance(mods, list) and mods else None, } cache[model_id] = entry if "/" in model_id: @@ -261,6 +275,115 @@ def _fetch_openrouter_models() -> dict[str, dict]: return cache +def warm_catalog_in_background() -> None: + """Start filling the catalog off the request path, without blocking a turn. + + The pricing path cannot be relied on to do it. It asks LiteLLM's static + table first and only reaches this catalog when that table *misses*, so for + every model LiteLLM does carry -- which is every model Raven ships a default + for -- the catalog is never fetched and a reader like + :func:`openrouter_input_modalities` has nothing to read, forever. + + Called instead of fetching inline because the fetch is synchronous with a + 10s timeout: on a machine that cannot reach the host, doing it in the turn + would stall the turn. A cold caller therefore degrades until the fetch lands. + + Retried on a cooldown rather than attempted once. An attempt that fails + proves nothing about the next one -- the first turn of a session routinely + runs before a VPN is up or a proxy has authenticated -- and a single latched + attempt would leave the reader answering from an empty catalog for the whole + process. A success needs no cooldown: the filled cache is itself the guard. + """ + global _WARM_AT + + if _OPENROUTER_CACHE: + return + now = time.monotonic() + if _WARM_AT and now - _WARM_AT < _WARM_RETRY_SECONDS: + return + _WARM_AT = now + + # Resolved here rather than inside the thread. A thread body that looks the + # name up on entry can lose a race with whoever patched it -- a test seam + # restored between ``start()`` and the thread's first bytecode would send a + # real request from inside the suite and write the real cache file. + fetch = _fetch_openrouter_models + + def _run() -> None: + try: + fetch() + except Exception as exc: # the fetch degrades internally; a thread must not die loudly + logger.debug("pricing: background catalog warm failed ({})", exc) + + threading.Thread(target=_run, name="raven-model-catalog-warm", daemon=True).start() + + +def _cached_catalog_only() -> dict[str, dict]: + """Whatever catalog is already in hand, at any age, without fetching. + + ``_fetch_openrouter_models`` is synchronous with a one-hour TTL, so calling + it from a request path would hand one turn a stall whenever the hour rolls + over. Prices are why that TTL is short; a model's input modalities are not, + so this reader takes a stale table happily and an absent one as "no answer". + Filling an absent one is :func:`warm_catalog_in_background`'s job. + """ + global _OPENROUTER_CACHE + + if _OPENROUTER_CACHE: + return _OPENROUTER_CACHE + disk = model_catalog_cache.load() + if disk is None: + return {} + # Re-checked after the read, not just before it: ``load()`` touches the + # filesystem and releases the GIL, so a background warm can land in that + # window with both a fresher table and a fresh ``_OPENROUTER_CACHE_TIME``. + # Overwriting it with this stale copy would leave that timestamp vouching for + # the wrong table, and the fetch's TTL check would then skip the refetch. + if _OPENROUTER_CACHE: + return _OPENROUTER_CACHE + # Kept so the next lookup does not re-read and re-parse the file. + # ``_OPENROUTER_CACHE_TIME`` is deliberately left alone: the fetch reads it + # to decide freshness, and this table is of unknown age -- good enough for a + # modality question, not to be mistaken for fresh pricing. + _OPENROUTER_CACHE = disk[0] + return _OPENROUTER_CACHE + + +def openrouter_input_modalities(model: str) -> tuple[str, ...] | None: + """What the catalog says ``model`` accepts as input, or ``None``. + + ``None`` means the catalog has no entry (or one written before this field + was kept), never "text only": this source states itself for every model it + lists, so silence is absence rather than a denial. + + Matched on the full id and then the bare alias, case-folded -- the catalog + spells every id it publishes in lower case, while a routed id need not + (``minimax/MiniMax-M2``). Punctuation is *not* normalized away, and that + restraint is the point: an id that survives only a fuzzier match is an id + this catalog does not actually list, and the only thing a wrong match can do + here is deny vision to a model that has it. ``azure/`` and the local + runtimes take a user-chosen deployment or tag name where every other + provider takes a model id, so ``azure/gpt4`` and ``ollama/phi4`` would join + against ``openai/gpt-4`` and ``microsoft/phi-4`` on a punctuation-stripping + key and lose every picture, silently, on a deployment that may well serve a + vision model. Losing the fuzzy tier costs nothing measurable: on the live + catalog every model it additionally matched either already answers "can see" + (the default when there is no answer at all) or is one of these false + denials. + + Reads only what is already cached -- see :func:`_cached_catalog_only`. + """ + key = model.removeprefix("openrouter/").lower() + table = _cached_catalog_only() + entry = table.get(key) + if entry is None and "/" in key: + entry = table.get(key.split("/", 1)[1]) + if not entry: + return None + mods = entry.get("input_modalities") + return tuple(mods) if isinstance(mods, list) and mods else None + + def _lookup_openrouter_entry(model: str) -> dict | None: """Resolve a model to its OpenRouter catalog entry. @@ -397,6 +520,9 @@ def reset_openrouter_cache() -> None: Only useful for tests — pair it with the ``model_catalog_cache._CACHE_PATH`` seam to exercise the disk tiers without touching the real ~/.raven/cache/. """ - global _OPENROUTER_CACHE, _OPENROUTER_CACHE_TIME + global _OPENROUTER_CACHE, _OPENROUTER_CACHE_TIME, _WARM_AT _OPENROUTER_CACHE = {} _OPENROUTER_CACHE_TIME = 0.0 + # Reset too, or a warm attempt from an earlier test leaves this one on a + # cooldown it never asked for. + _WARM_AT = 0.0 diff --git a/raven/tui_rpc/methods/turn.py b/raven/tui_rpc/methods/turn.py index c597012e..e75fa42f 100644 --- a/raven/tui_rpc/methods/turn.py +++ b/raven/tui_rpc/methods/turn.py @@ -17,12 +17,14 @@ from __future__ import annotations +from pathlib import Path from typing import TYPE_CHECKING, Any from uuid import uuid4 +from loguru import logger from pydantic import ValidationError -from raven.spine import ChatType, Origin, Source, TurnHandle, TurnRequest +from raven.spine import ChatType, Media, Origin, Source, TurnHandle, TurnRequest from raven.spine.scheduler import Scheduler, SchedulerDrainingError from raven.tui_rpc.errors import RpcError, TurnInProgressError from raven.tui_rpc.models import ( @@ -38,6 +40,57 @@ _TURN_FAILED_CODE = -32099 + +def _resolve_media(paths: list[str] | None) -> tuple[Media, ...]: + """Turn the front end's attachment paths into ``Media`` for the spine. + + Resolved with the filesystem tools' own policy rather than against the + process cwd. A caller sends what it holds, and what it holds is a workspace + path (``uploads/shot.png``) -- the same spelling every file tool takes, and + one that resolves to nothing from wherever ``raven serve`` happens to have + been started. The downstream check is a bare ``is_file()`` that drops a miss + in silence, so a cwd-relative resolve loses the attachment with no error + anywhere. + + The mime is left generic on purpose: ``render.build_user_content`` sniffs + the magic bytes, and the channels' own intake does the same thing here. + A path that does not resolve, or resolves outside the allowed directory, + is dropped with a log line -- one bad attachment must not fail the turn. + """ + if not paths: + return () + from raven.agent.tools.filesystem import _resolve_path + from raven.config import load_config + + try: + cfg = load_config() + workspace = Path(cfg.agents.defaults.workspace).expanduser() + allowed = workspace if cfg.tools.restrict_to_workspace else None + except Exception as exc: + logger.warning("turn.send: cannot resolve the workspace ({}); attachments dropped", exc) + return () + + out: list[Media] = [] + for raw in paths: + if not isinstance(raw, str) or not raw.strip(): + continue + try: + resolved = _resolve_path(raw.strip(), workspace, allowed) + if not resolved.is_file(): + logger.warning("turn.send: attachment {} does not resolve to a file", raw) + continue + except Exception as exc: + # Every failure shape lands here on purpose. A path can be refused + # (PermissionError), embed a null byte or an unknown ~user + # (ValueError / RuntimeError), or exceed the filesystem's name + # limit (OSError) -- and each of those escaping would turn one bad + # attachment into a turn that never runs. + logger.warning("turn.send: attachment {} rejected: {}", raw, exc) + continue + out.append(Media(path=str(resolved), mime="application/octet-stream", kind="file")) + return tuple(out) + + # --------------------------------------------------------------------------- # Module-level state # --------------------------------------------------------------------------- @@ -149,6 +202,7 @@ async def turn_send( chat_type=ChatType.DM, ), text=parsed.content, + media=_resolve_media(parsed.media), # conversation == the front-end subscription key, so the runner's stream # and the sink's message.complete reach the right subscription. conversation=parsed.session_key, diff --git a/raven/tui_rpc/models.py b/raven/tui_rpc/models.py index 6355bf09..aa2557ec 100644 --- a/raven/tui_rpc/models.py +++ b/raven/tui_rpc/models.py @@ -421,6 +421,14 @@ class TurnSendParams(_Strict): channel: str | None = None chat_id: str | None = None sender_id: str | None = None + # Attachment paths, workspace-relative or absolute. The same lane channels + # already use (``TurnRequest.media``): a vision-capable model gets the + # picture inlined in the user message, anything else gets a note naming it. + # Paths rather than bytes -- the caller has already put the file in the + # workspace, and every file tool is workspace-scoped. Bounded here so a + # malformed caller is refused at the schema rather than resolving thousands + # of paths; the renderer caps how many are inlined regardless. + media: list[str] | None = Field(default=None, max_length=64) class TurnSendResult(_Strict): diff --git a/tests/test_plugin_tools.py b/tests/test_plugin_tools.py index 0c625774..4bb1831d 100644 --- a/tests/test_plugin_tools.py +++ b/tests/test_plugin_tools.py @@ -250,10 +250,16 @@ def test_non_image_surfaced_as_note(self, tmp_path: Path) -> None: pdf = tmp_path / "report.pdf" pdf.write_bytes(b"%PDF-1.4 data") - out = render.build_user_content("summarize this", [str(pdf)]) + # Named explicitly: the hint is only emitted when a description tool + # is actually registered, and it is absent on a default install. + out = render.build_user_content("summarize this", [str(pdf)], describe_tool="understand_media") assert isinstance(out, str) assert "report.pdf" in out assert "understand_media" in out + + bare = render.build_user_content("summarize this", [str(pdf)]) + assert "report.pdf" in bare and f"(path: {pdf})" in bare + assert "understand_media" not in bare assert "summarize this" in out def test_image_inlined_as_block(self, tmp_path: Path) -> None: @@ -292,7 +298,7 @@ def test_mixed_image_and_doc(self, tmp_path: Path) -> None: ) pdf = tmp_path / "d.pdf" pdf.write_bytes(b"%PDF-1.4") - out = render.build_user_content("q", [str(png), str(pdf)]) + out = render.build_user_content("q", [str(png), str(pdf)], describe_tool="understand_media") assert isinstance(out, list) assert out[0]["type"] == "image_url" text_block = out[-1]["text"] @@ -303,6 +309,31 @@ def test_no_media_returns_text(self) -> None: assert render.build_user_content("hi", None) == "hi" + def test_image_becomes_a_note_when_the_model_cannot_see(self, tmp_path: Path) -> None: + """A text-only model must be told the picture exists and how to read it. + + Inlining it instead is the one outcome with no recovery: the endpoint + either rejects the request or drops the image and answers from the text + alone, which reads as a correct reply built on nothing. + """ + import base64 as _b64 + + from raven.context_engine.segments import render + + png = tmp_path / "a.png" + png.write_bytes( + _b64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==" + ) + ) + out = render.build_user_content("look", [str(png)], can_see_images=False, describe_tool="understand_media") + + assert isinstance(out, str) + assert "base64" not in out + assert f"(path: {png})" in out + assert "understand_media" in out + assert out.startswith("look\n\n") + def test_legacy_builder_names_the_image_path_too(self, tmp_path: Path) -> None: """There are two content builders (context engine and legacy ContextBuilder); a path dropped in either one is a path the model cannot diff --git a/tests/test_read_file_image.py b/tests/test_read_file_image.py index d282211c..dd1749c6 100644 --- a/tests/test_read_file_image.py +++ b/tests/test_read_file_image.py @@ -7,7 +7,10 @@ from __future__ import annotations import asyncio +import base64 import json +import threading +import time from pathlib import Path import pytest @@ -18,13 +21,25 @@ from raven.agent.tools.filesystem import ReadFileTool from raven.agent.tools.registry import ToolRegistry from raven.providers.base import LLMProvider -from raven.providers.capabilities import ( +from raven.token_wise import pricing as _pricing + +# Captured before the autouse _no_openrouter_network fixture swaps it out. +_REAL_FETCH = _pricing._fetch_openrouter_models + +from raven.providers.capabilities import ( # noqa: E402 IMAGE_TOOL_RESULT_TARGETS, image_placeholder_text, supports_image_tool_result, ) +def _join_warm() -> None: + """Wait out any background catalog warm this test started.""" + for thread in threading.enumerate(): + if thread.name == "raven-model-catalog-warm": + thread.join(timeout=10) + + def _write_image(path: Path, size: tuple[int, int], fmt: str = "PNG") -> Path: from PIL import Image @@ -270,6 +285,41 @@ def test_image_placeholder_text_keeps_the_path_and_never_leaks_base64() -> None: assert "AAAA" not in out and len(out) < 300 +def test_blind_placeholder_does_not_promise_a_picture_that_never_arrives() -> None: + """Two reasons for the same substitution, and they must not share wording. + + The transport case attaches the picture to the next message, so the note + says so. A model with no vision gets nothing afterwards -- telling it to + expect an attachment leaves it waiting, and the useful thing to say instead + is which tool can read the file for it. + """ + blocks = [ + {"type": "text", "text": "[image: /tmp/x.png] | 300x200px | ~88 tokens"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64," + "A" * 5000}}, + ] + out = image_placeholder_text(blocks, blind=True, describe_tool="understand_media") + + assert "/tmp/x.png" in out + assert "attached to the following message" not in out + assert "understand_media" in out + assert "AAAA" not in out and len(out) < 300 + + +def test_the_blind_placeholder_names_no_tool_when_none_is_registered() -> None: + """The description tool ships with the EverOS plugin and is absent on a + default install. Naming it anyway is an instruction the model cannot follow, + so the note then says only that a picture exists and stops.""" + blocks = [ + {"type": "text", "text": "[image: /tmp/x.png] | 300x200px"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + ] + out = image_placeholder_text(blocks, blind=True) + + assert "you cannot see images directly" in out + assert "tool" not in out + assert "/tmp/x.png" in out + + # -------------------------------------------------------------------------- # persistence # -------------------------------------------------------------------------- @@ -1080,3 +1130,829 @@ async def spy(key, payload): seen = captured[-1]["messages"] assert not any(m.get("_attached_image") for m in seen) assert "base64" not in json.dumps(seen) + + +# -------------------------------------------------------------------------- +# vision capability — can the model see a picture at all +# -------------------------------------------------------------------------- + + +def _catalog(monkeypatch, models: dict[str, list[str] | None]) -> None: + """Install a catalog table directly, keyed the way the fetch keys it. + + Never the live one: LiteLLM and the gateway catalog are both fetched over + the network at import/first-use, and the gateway's file changes several + times a day, so an assertion against it is an assertion about someone + else's deploy. + """ + from raven.token_wise import pricing + + built: dict[str, dict] = {} + for model_id, mods in models.items(): + entry = {"pricing": {}, "context_length": 1, "input_modalities": mods} + built[model_id] = entry + if "/" in model_id: + built.setdefault(model_id.split("/", 1)[1], entry) + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", built) + + +def test_a_routed_id_matches_the_catalog_across_case(monkeypatch) -> None: + """The catalog spells every id it publishes in lower case; a routed id need + not. Case is the only spelling difference normalized away.""" + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"minimax/minimax-m2": ["text"]}) + assert supports_vision("minimax/MiniMax-M2") is False + assert supports_vision("minimax-global/MiniMax-M2") is False + + +def test_a_deployment_name_is_never_matched_by_stripping_punctuation(monkeypatch) -> None: + """Azure and the local runtimes take a user-chosen deployment or tag name + where every other provider takes a model id, so a fuzzier join would answer + for a model the caller never named. + + ``azure/gpt4`` may well serve gpt-4o. A key that dropped the hyphen would + join it to text-only ``openai/gpt-4`` and lose every picture with no error + anywhere -- the one failure this module is built to avoid. Punctuation + therefore stays significant: the fuzzy tier can only ever manufacture a + denial, since a match that grants vision is what absence already gives. + """ + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"openai/gpt-4": ["text"], "microsoft/phi-4": ["text"]}) + assert supports_vision("azure/gpt4") is True + assert supports_vision("azure/GPT4") is True + assert supports_vision("ollama/phi4") is True + # The real vendor spelling still resolves, and still denies. + assert supports_vision("openai/gpt-4") is False + + +def test_the_catalog_is_warmed_in_the_background_when_it_has_no_answer(monkeypatch) -> None: + """The pricing path asks LiteLLM's static table first and only reaches this + catalog when that misses, so for every model Raven ships a default for it + would never be fetched at all and this probe would answer optimistically + forever. Warmed off the request path because the fetch takes a 10s timeout. + """ + from raven.providers.capabilities import supports_vision + from raven.token_wise import pricing + + calls: list[str] = [] + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_WARM_AT", 0.0) + monkeypatch.setattr(pricing, "_fetch_openrouter_models", lambda: calls.append("fetch") or {}) + monkeypatch.setattr(pricing.model_catalog_cache, "load", lambda: None) + + assert supports_vision("deepseek/deepseek-v4-pro") is True + _join_warm() + assert calls == ["fetch"] + + # A second cold answer inside the cooldown must not start a second fetch. + assert supports_vision("some-other/model") is True + _join_warm() + assert calls == ["fetch"] + + +def test_a_failed_warm_is_retried_once_the_cooldown_passes(monkeypatch) -> None: + """A machine whose first turn runs before the VPN is up must not be left + answering from an empty catalog for the rest of the process -- an attempt + that failed says nothing about the next one.""" + from raven.providers.capabilities import supports_vision + from raven.token_wise import pricing + + calls: list[str] = [] + + def _fail() -> dict: + calls.append("fetch") + return {} + + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_WARM_AT", 0.0) + monkeypatch.setattr(pricing, "_fetch_openrouter_models", _fail) + monkeypatch.setattr(pricing.model_catalog_cache, "load", lambda: None) + + assert supports_vision("deepseek/deepseek-v4-pro") is True + _join_warm() + assert calls == ["fetch"] + + # Still on cooldown. + assert supports_vision("deepseek/deepseek-v4-pro") is True + _join_warm() + assert calls == ["fetch"] + + # Cooldown elapsed -> tried again. + monkeypatch.setattr(pricing, "_WARM_AT", time.monotonic() - pricing._WARM_RETRY_SECONDS - 1) + assert supports_vision("deepseek/deepseek-v4-pro") is True + _join_warm() + assert calls == ["fetch", "fetch"] + + +def test_the_warm_resolves_its_fetch_before_the_thread_starts(monkeypatch) -> None: + """``Thread.start()`` returns before the thread runs its first bytecode. A + body that looked the fetch up on entry could therefore lose a race with + whoever patched it -- a restored test seam would send a real request from + inside the suite and write the real cache file.""" + from raven.token_wise import pricing + + calls: list[str] = [] + captured: dict[str, object] = {} + + class _CapturedThread: + """Holds the body at the starting line so the window can be closed by + hand. Racing a real thread would make the assertion depend on which side + of the window the scheduler happened to land on.""" + + def __init__(self, target=None, name=None, daemon=None) -> None: + captured["target"] = target + + def start(self) -> None: + pass + + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_WARM_AT", 0.0) + monkeypatch.setattr(pricing, "_fetch_openrouter_models", lambda: calls.append("stub") or {}) + monkeypatch.setattr(threading, "Thread", _CapturedThread) + + pricing.warm_catalog_in_background() + # Exactly the window: start() has returned, the body has not run. + monkeypatch.setattr(pricing, "_fetch_openrouter_models", lambda: calls.append("REAL") or {}) + captured["target"]() + + assert calls == ["stub"] + + +def test_a_warm_that_cannot_reach_the_host_does_not_raise(monkeypatch) -> None: + """The fetch degrades internally, but a thread that dies loudly writes a + traceback into a user's terminal for a probe that has already answered.""" + from raven.token_wise import pricing + + def _boom() -> dict: + raise RuntimeError("no route to host") + + seen: list[BaseException] = [] + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_WARM_AT", 0.0) + monkeypatch.setattr(pricing, "_fetch_openrouter_models", _boom) + monkeypatch.setattr(threading, "excepthook", lambda args: seen.append(args.exc_value)) + + pricing.warm_catalog_in_background() + _join_warm() + + # Asserted, not merely "did not blow up in the caller": the raise happens on + # another thread, where pytest downgrades an escape to a warning and a test + # with no assertion passes either way. + assert seen == [] + assert pricing._OPENROUTER_CACHE == {} + + +def test_a_model_the_catalog_calls_text_only_is_blind(monkeypatch) -> None: + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"deepseek/deepseek-v4-pro": ["text"]}) + assert supports_vision("deepseek/deepseek-v4-pro") is False + # The gateway spelling of the same model resolves to the same entry -- the + # case this whole lookup exists for. + assert supports_vision("openrouter/deepseek/deepseek-v4-pro") is False + + +def test_a_provider_prefix_the_catalog_never_uses_still_resolves(monkeypatch) -> None: + """dashscope/ and gemini/ are routing names; the catalog files the same + models under qwen/ and google/.""" + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"qwen/qwen-plus": ["text"], "google/gemini-2.5-flash": ["text", "image"]}) + assert supports_vision("dashscope/qwen-plus") is False + assert supports_vision("gemini/gemini-2.5-flash") is True + + +def test_a_model_the_catalog_never_heard_of_keeps_its_pictures(monkeypatch) -> None: + """Absent is not declared blind. An unlisted model is left exactly where it + was before this check existed -- being wrong this way fails loudly at the + endpoint, while being wrong the other way silently turns images into prose + with nothing to notice.""" + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"google/gemini-2.5-flash": ["text", "image"]}) + assert supports_vision("no-such-vendor/no-such-model-9000") is True + + +def test_an_entry_written_before_modalities_were_kept_is_not_a_denial(monkeypatch) -> None: + """A cache file from an older Raven carries no modality data. Reading that + silence as "no modalities" would read as "cannot see".""" + from raven.providers.capabilities import supports_vision + + _catalog(monkeypatch, {"some/model": None}) + assert supports_vision("some/model") is True + + +def test_a_catalog_that_blows_up_does_not_break_the_probe(monkeypatch) -> None: + """A capability probe must never be the thing that fails a turn.""" + from raven.providers import capabilities + from raven.token_wise import pricing + + def _boom(model): + raise RuntimeError("catalog on fire") + + monkeypatch.setattr(pricing, "openrouter_input_modalities", _boom) + assert capabilities.supports_vision("gpt-4o") is True + + +def test_provider_spec_override_beats_the_catalogue(monkeypatch) -> None: + """The escape hatch, both directions: a model the catalog gets wrong, and + one it never lists.""" + from raven.providers.capabilities import supports_vision + from raven.providers.registry import ProviderSpec + + _catalog(monkeypatch, {"openai/gpt-4o": ["text", "image"], "some/text-model": ["text"]}) + + seeing = ProviderSpec(name="selfhost", keywords=("selfhost",), env_key="", vision_override=True) + assert supports_vision("some/text-model", seeing) is True + + blind = ProviderSpec(name="textonly", keywords=("textonly",), env_key="", vision_override=False) + assert supports_vision("gpt-4o", blind) is False + + +# -------------------------------------------------------------------------- +# wiring — one-line hand-offs a refactor can drop with every other test green +# -------------------------------------------------------------------------- + + +def test_the_loop_hands_the_verdict_to_the_context_engine(monkeypatch) -> None: + """`can_see_images` is decided in the loop and consumed three layers down. + Nothing else asserts the hand-off, so dropping it would be silent.""" + from raven.agent.loop.main import AgentLoop + + loop = object.__new__(AgentLoop) + loop._vision_ok = {} + loop.model = "some/model" + monkeypatch.setattr(AgentLoop, "_supports_vision", lambda self, m=None: False) + monkeypatch.setattr(AgentLoop, "_describe_tool_name", lambda self: "understand_media") + + seen = {} + + class _Engine: + owns_compaction = True + + async def assemble(self, session_key, session_messages, budget, *, turn): + seen["can_see_images"] = turn.can_see_images + seen["describe_tool"] = turn.describe_tool + raise _Stop + + class _Stop(Exception): + pass + + loop.context_engine = _Engine() + monkeypatch.setattr(AgentLoop, "_context_messages_for_session", lambda self, s: []) + monkeypatch.setattr(AgentLoop, "_make_token_budget", lambda self, s=None: None) + loop._last_injected_skill_ids = None + + with pytest.raises(_Stop): + asyncio.run( + loop._assemble_context_messages(session=object(), session_key="s", current_message="hi", media=["/x.png"]) + ) + assert seen == {"can_see_images": False, "describe_tool": "understand_media"} + + +def test_the_verdict_is_asked_of_the_routed_model_not_the_configured_one(monkeypatch) -> None: + """Assembly is handed the model the request will actually reach. + + The router can send the turn somewhere other than ``self.model``, and the + tool-result probe already asks about the routed id -- so asking about the + configured one here would let a blind primary's verdict shape a message a + vision model receives, or the reverse. Asserted because the argument is the + whole fix: without it the two halves of one turn disagree and nothing fails. + """ + from raven.agent.loop.main import AgentLoop + + asked: list[str | None] = [] + loop = object.__new__(AgentLoop) + loop._vision_ok = {} + loop.model = "configured/model" + monkeypatch.setattr(AgentLoop, "_supports_vision", lambda self, m=None: asked.append(m) or True) + monkeypatch.setattr(AgentLoop, "_describe_tool_name", lambda self: None) + + class _Stop(Exception): + pass + + class _Engine: + owns_compaction = True + + async def assemble(self, session_key, session_messages, budget, *, turn): + raise _Stop + + loop.context_engine = _Engine() + monkeypatch.setattr(AgentLoop, "_context_messages_for_session", lambda self, s: []) + monkeypatch.setattr(AgentLoop, "_make_token_budget", lambda self, s=None: None) + loop._last_injected_skill_ids = None + + with pytest.raises(_Stop): + asyncio.run( + loop._assemble_context_messages( + session=object(), + session_key="s", + current_message="hi", + media=["/x.png"], + model="routed/vision-model", + ) + ) + assert asked == ["routed/vision-model"] + + +def test_the_assembler_forwards_the_verdict_to_the_renderer(monkeypatch) -> None: + from raven.context_engine.assembler import ContextAssembler + from raven.context_engine.base import AssemblyContext + + seen = {} + + def _spy(text, media, *, can_see_images=True, describe_tool=None): + seen.update(can_see_images=can_see_images, describe_tool=describe_tool) + return text + + from raven.context_engine.segments import render as render_mod + + monkeypatch.setattr(render_mod, "build_user_content", _spy) + engine = ContextAssembler([], lambda: []) + engine._build_user( + AssemblyContext( + session_key="s", + current_message="hi", + media=["/x.png"], + channel=None, + chat_id=None, + session_messages=[], + budget=None, + can_see_images=False, + describe_tool="understand_media", + ) + ) + assert seen == {"can_see_images": False, "describe_tool": "understand_media"} + + +def test_an_unregistered_describe_tool_is_not_named(monkeypatch) -> None: + """Pointing a model at a tool it was never given is an instruction it cannot + follow. The tool ships with an optional plugin, so absence is the default.""" + from raven.agent.loop.main import AgentLoop + + loop = object.__new__(AgentLoop) + + class _Registry: + def __init__(self, has): + self._has = has + + def get(self, name): + return object() if self._has else None + + loop.tools = _Registry(False) + assert loop._describe_tool_name() is None + loop.tools = _Registry(True) + assert loop._describe_tool_name() == "understand_media" + + +# -------------------------------------------------------------------------- +# attachment preprocessing — the inline path used to skip it entirely +# -------------------------------------------------------------------------- + + +def test_an_attachment_is_downscaled_before_it_is_inlined(tmp_path: Path) -> None: + """A phone photo is several megabytes and thousands of patch tokens. Inlined + raw it is refused, or downsized server-side and billed at full size.""" + from raven.context_engine.segments import render + + big = _write_image(tmp_path / "photo.jpg", (4032, 3024), fmt="JPEG") + out = render.build_user_content("look", [str(big)], can_see_images=True) + + b64 = out[0]["image_url"]["url"].split(",", 1)[1] + raw_b64 = len(base64.b64encode(big.read_bytes())) + assert len(b64) < raw_b64 / 4 + note = out[-1]["text"] + assert "downscaled from 4032x3024" in note + assert "px" in note + + +def test_an_attachment_that_cannot_be_prepared_is_named_not_dropped(tmp_path: Path) -> None: + """The user chose this file. A silent drop leaves them believing the model + saw something it never received.""" + from raven.context_engine.segments import render + + broken = tmp_path / "broken.png" + broken.write_bytes(b"\x89PNG\r\n\x1a\n" + b"garbage") + out = render.build_user_content("look", [str(broken)], can_see_images=True) + + assert isinstance(out, str) + assert "broken.png" in out + assert "could not be prepared" in out + + +def test_an_attachment_that_cannot_be_read_costs_a_note_not_the_turn(tmp_path: Path) -> None: + """Resolution only proved the path pointed at a file. Permissions can change + between then and the read, and the file can be gone -- and this renderer runs + deep inside turn assembly, where an ``OSError`` reaches the caller as a failed + turn rather than as a message about one attachment. + """ + import os + + from raven.context_engine.segments import render + + locked = _write_image(tmp_path / "locked.png", (60, 40)) + os.chmod(locked, 0o000) + try: + out = render.build_user_content("look", [str(locked)], can_see_images=True) + finally: + os.chmod(locked, 0o644) + + assert isinstance(out, str) + assert "locked.png" in out + assert "could not be read" in out + + +def test_a_media_list_cannot_inline_an_unbounded_number_of_images(tmp_path: Path) -> None: + """``prepare_image`` caps each picture; nothing capped the count. A caller is + free to hand over any number of paths, and each survivor still costs its own + patch tokens -- so the overflow is named in the text instead of inlined.""" + from raven.context_engine.segments import render + + paths = [str(_write_image(tmp_path / f"p{i}.png", (40, 30))) for i in range(render._MAX_INLINE_IMAGES + 3)] + out = render.build_user_content("look", paths, can_see_images=True) + + blocks = [b for b in out if b["type"] == "image_url"] + assert len(blocks) == render._MAX_INLINE_IMAGES + note = out[-1]["text"] + assert note.count("not shown, this message is already carrying") == 3 + # read_file, not the description tool: this model can see, so the useful + # next step is fetching the picture itself. + assert "read_file" in note and "understand_media" not in note + + +def test_an_image_too_large_is_refused_instead_of_read_whole(tmp_path: Path, monkeypatch) -> None: + """A caller may name a file of any size, and the bytes are only needed to + inline a picture -- so the ceiling is checked from ``stat`` and the file is + never read past its header.""" + from raven.context_engine.segments import render + + monkeypatch.setattr(render, "_MAX_IMAGE_BYTES", 16) + fat = _write_image(tmp_path / "fat.png", (200, 200)) + assert fat.stat().st_size > 16 + + out = render.build_user_content("look", [str(fat)], can_see_images=True) + + assert isinstance(out, str) + assert "too large" in out and "fat.png" in out + + +def test_a_non_image_attachment_is_never_read_past_its_header(tmp_path: Path) -> None: + """The bytes exist only to sniff the magic number. Reading a 60MB PDF in full + to look at its first 8 bytes is pure waste, and with a media list of 64 it is + gigabytes of it per turn.""" + from raven.context_engine.segments import render + + doc = tmp_path / "report.pdf" + doc.write_bytes(b"%PDF-1.4" + b"\0" * (4 * 1024 * 1024)) + + reads: list[int | None] = [] + real_read = render.Path.open + + class _CountingHandle: + def __init__(self, inner): + self._inner = inner + + def read(self, n=None): + reads.append(n) + return self._inner.read(n) if n is not None else self._inner.read() + + def __enter__(self): + self._inner.__enter__() + return self + + def __exit__(self, *exc): + return self._inner.__exit__(*exc) + + def _open(self, *a, **k): + return _CountingHandle(real_read(self, *a, **k)) + + render.Path.open = _open + try: + out = render.build_user_content("summarize", [str(doc)]) + finally: + render.Path.open = real_read + + assert "report.pdf" in out + # Exactly one bounded read: the header. No unbounded read() followed. + assert reads == [render._SNIFF_BYTES] + + +def test_the_inlined_payload_is_bounded_in_bytes_not_only_in_count(tmp_path: Path, monkeypatch) -> None: + """The count ceiling alone permits 16 images at the per-image byte cap, which + is a request body every major provider refuses -- so a legitimate batch would + fail the turn rather than degrade.""" + from raven.context_engine.segments import render + + monkeypatch.setattr(render, "_MAX_INLINE_BASE64_BYTES", 4000) + paths = [str(_write_image(tmp_path / f"p{i}.png", (300, 300))) for i in range(6)] + out = render.build_user_content("look", paths, can_see_images=True) + + blocks = [b for b in out if b["type"] == "image_url"] + assert 0 < len(blocks) < 6 + total = sum(len(b["image_url"]["url"]) for b in blocks) + # Stops at the first image that crosses the budget, so the overshoot is + # bounded by one image rather than by the list length. + assert total < 4000 + len(blocks[-1]["image_url"]["url"]) + assert "not shown, this message is already carrying" in out[-1]["text"] + + +def test_the_attachment_note_names_no_tool_when_none_is_registered(tmp_path: Path) -> None: + """Mirror of the tool-result placeholder: ``describe_tool=None`` means the + default install has no such tool, and the note must not invent one.""" + from raven.context_engine.segments import render + + pic = _write_image(tmp_path / "chart.png", (60, 40)) + with_tool = render.build_user_content("look", [str(pic)], can_see_images=False, describe_tool="understand_media") + without = render.build_user_content("look", [str(pic)], can_see_images=False, describe_tool=None) + + assert "understand_media" in with_tool + assert "tool" not in without + assert "chart.png" in without and "you cannot see images directly" in without + + +def test_a_blind_model_gets_no_image_in_a_tool_result(tmp_path: Path) -> None: + """The third branch at the routing point, and the only one with no picture + anywhere afterwards -- so its wording must not promise one. + + The transport branch says "attached to the following message" because the + image really does follow. Saying that here would leave the model waiting for + something that was never sent. + """ + _write_image(tmp_path / "chart.png", (300, 200)) + result = _read(tmp_path, "chart.png") + + blind = image_placeholder_text(result.blocks, blind=True, describe_tool="understand_media") + transport = image_placeholder_text(result.blocks) + + assert "attached to the following message" in transport + assert "attached to the following message" not in blind + assert "understand_media" in blind + # Both keep the tool's own text, which is the only thing naming the file. + assert str(tmp_path / "chart.png") in blind + assert "base64" not in blind and "iVBOR" not in blind + + +# -------------------------------------------------------------------------- +# round-3: the branches and constraints a mutation test found unguarded +# -------------------------------------------------------------------------- + + +def _loop_for_routing(monkeypatch, *, sees: bool, tool_result_ok: bool, describe: str | None): + from raven.agent.loop.main import AgentLoop + + loop = object.__new__(AgentLoop) + monkeypatch.setattr(AgentLoop, "_supports_vision", lambda self, m=None: sees) + monkeypatch.setattr(AgentLoop, "_supports_image_tool_result", lambda self, m=None: tool_result_ok) + monkeypatch.setattr(AgentLoop, "_describe_tool_name", lambda self: describe) + return loop + + +_ROUTING_BLOCKS = [ + {"type": "text", "text": "[image: /w/shot.png] | 300x200px"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64," + "A" * 500}}, +] + + +def test_a_blind_model_is_told_the_picture_is_not_coming(monkeypatch) -> None: + """The one branch with no picture anywhere afterwards. Saying "attached to + the following message" here leaves the model waiting for something that is + never sent, and nothing downstream can notice.""" + loop = _loop_for_routing(monkeypatch, sees=False, tool_result_ok=True, describe="understand_media") + text, blocks, attach = loop._route_result_images("orig", list(_ROUTING_BLOCKS), "some/blind-model") + + assert blocks is None and attach is None + assert "attached to the following message" not in text + assert "you cannot see images directly" in text + assert "understand_media" in text + assert "AAAA" not in text + + +def test_a_blind_model_with_no_description_tool_is_promised_nothing(monkeypatch) -> None: + loop = _loop_for_routing(monkeypatch, sees=False, tool_result_ok=True, describe=None) + text, _, _ = loop._route_result_images("orig", list(_ROUTING_BLOCKS), "some/blind-model") + + assert "you cannot see images directly" in text + assert "tool" not in text + + +def test_a_transport_that_carries_images_keeps_them_in_the_tool_result(monkeypatch) -> None: + loop = _loop_for_routing(monkeypatch, sees=True, tool_result_ok=True, describe=None) + text, blocks, attach = loop._route_result_images("orig", list(_ROUTING_BLOCKS), "anthropic/claude") + + assert text == "orig" + assert blocks == _ROUTING_BLOCKS and attach is None + + +def test_a_transport_that_cannot_carry_images_attaches_them_after(monkeypatch) -> None: + loop = _loop_for_routing(monkeypatch, sees=True, tool_result_ok=False, describe=None) + text, blocks, attach = loop._route_result_images("orig", list(_ROUTING_BLOCKS), "openai/gpt-4o") + + assert blocks is None + assert attach == [_ROUTING_BLOCKS[1]] + assert "attached to the following message" in text + assert "AAAA" not in text + + +def test_a_text_result_is_passed_through_untouched(monkeypatch) -> None: + loop = _loop_for_routing(monkeypatch, sees=False, tool_result_ok=False, describe=None) + assert loop._route_result_images("plain", None, "m") == ("plain", None, None) + + +def test_the_fetch_files_no_normalized_join_key_in_the_shared_table(monkeypatch) -> None: + """Regression on the write side, which is where the defect lived. + + v2 filed a punctuation-stripped key beside each id. That key is reachable by + an exact lookup, so ``ollama/phi4`` (bare alias ``phi4``) joined + ``microsoft/phi-4`` and inherited its prices, its context window and its + text-only verdict. Asserted through the real fetch: a hand-built table cannot + see this, which is exactly why the mutation went unnoticed. + """ + from raven.providers.capabilities import supports_vision + from raven.token_wise import model_catalog_cache, pricing + + payload = { + "data": [ + { + "id": "openai/gpt-4", + "pricing": {"prompt": "0.00003", "completion": "0.00006"}, + "context_length": 8192, + "architecture": {"input_modalities": ["text"]}, + }, + { + "id": "microsoft/phi-4", + "pricing": {"prompt": "0.00000007", "completion": "0.00000014"}, + "context_length": 16384, + "architecture": {"input_modalities": ["text"]}, + }, + ] + } + + class _Resp: + def raise_for_status(self) -> None: + pass + + def json(self) -> dict: + return payload + + class _Client: + def __init__(self, *a, **k) -> None: + pass + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def get(self, *a, **k): + return _Resp() + + monkeypatch.setattr(pricing.httpx, "Client", _Client) + monkeypatch.setattr(model_catalog_cache, "save", lambda models: None) + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE_TIME", 0.0) + + table = _REAL_FETCH() + + assert set(table) == {"openai/gpt-4", "gpt-4", "microsoft/phi-4", "phi-4"} + assert "gpt4" not in table and "phi4" not in table + + # And the consequence the key set exists to prevent. + assert supports_vision("ollama/phi4") is True + assert supports_vision("azure/gpt4") is True + assert pricing.resolve_context_window("ollama/phi4") is None + + +def test_a_deployment_name_is_never_answered_from_the_vendor_catalog(monkeypatch) -> None: + """Azure takes the name of a deployment the user created and a local runtime + takes whatever tag they pulled, either of which can be spelled exactly like a + vendor id it does not serve -- ``gpt-4`` is the name Azure's own quickstarts + use, and a team keeps the name while repointing the deployment at gpt-4o. + + Only a denial does damage here (a grant is what absence already gives), and a + denial is the silent failure this module exists to avoid, so these providers + do not consult the catalog at all. + """ + from raven.providers.azure_openai_provider import AzureOpenAIProvider + from raven.providers.capabilities import supports_vision, vision_verdict + from raven.providers.registry import find_by_model + + _catalog(monkeypatch, {"openai/gpt-4": ["text"], "qwen/qwen-plus": ["text"]}) + azure = object.__new__(AzureOpenAIProvider) + + # Azure takes a bare deployment name, so no prefix resolves it to a spec -- + # the live provider is the only thing that knows. The alias matches the + # catalog verbatim, so this is not about punctuation either. + assert find_by_model("gpt-4") is not None, "resolves to OpenAI's spec, which is the trap" + assert vision_verdict("gpt-4", find_by_model("gpt-4"), azure) is None + assert supports_vision("gpt-4", find_by_model("gpt-4"), azure) is True + + # Routed through LiteLLM instead, Azure carries a prefix the registry does + # not answer to, so neither the spec nor the provider identifies it. + assert find_by_model("azure/gpt-4") is None + assert supports_vision("azure/gpt-4", find_by_model("azure/gpt-4")) is True + assert supports_vision("azure_ai/gpt-4", find_by_model("azure_ai/gpt-4")) is True + + # A local runtime does carry a resolvable prefix. + assert supports_vision("ollama/qwen-plus", find_by_model("ollama/qwen-plus")) is True + + # The same id routed to the vendor itself is still answered, and denied. + assert supports_vision("gpt-4", find_by_model("gpt-4")) is False + + +def test_a_cold_verdict_is_not_cached_for_the_life_of_the_loop(monkeypatch) -> None: + """``AgentLoop`` is built once per process. Caching the optimistic answer the + catalog gives before it is warm would freeze that guess forever and leave the + background warm filling a table nothing re-reads -- so only a real verdict is + remembered.""" + from raven.agent.loop.main import AgentLoop + from raven.token_wise import pricing + + loop = object.__new__(AgentLoop) + loop._vision_ok = {} + loop.model = "deepseek/deepseek-v4-pro" + loop.provider = None + + monkeypatch.setattr(pricing, "_OPENROUTER_CACHE", {}) + monkeypatch.setattr(pricing, "_WARM_AT", 0.0) + monkeypatch.setattr(pricing, "_fetch_openrouter_models", lambda: {}) + monkeypatch.setattr(pricing.model_catalog_cache, "load", lambda: None) + + assert loop._supports_vision() is True + assert loop._vision_ok == {}, "a cold guess must not be remembered" + _join_warm() + + _catalog(monkeypatch, {"deepseek/deepseek-v4-pro": ["text"]}) + assert loop._supports_vision() is False + assert loop._vision_ok == {"deepseek/deepseek-v4-pro": False} + + +def test_turn_send_refuses_an_unbounded_attachment_list() -> None: + """Nothing downstream counts the list, and each survivor costs its own patch + tokens, so the schema is where an absurd one is refused.""" + import pydantic + + from raven.tui_rpc.models import TurnSendParams + + ok = TurnSendParams(session_key="cli:local", content="hi", media=["a.png"] * 64) + assert len(ok.media) == 64 + with pytest.raises(pydantic.ValidationError, match="at most 64"): + TurnSendParams(session_key="cli:local", content="hi", media=["a.png"] * 65) + + +def test_a_blind_model_never_pays_to_read_the_picture(tmp_path: Path) -> None: + """The bytes exist only to inline a picture. A model that cannot see one + gets a note built from the path alone, so loading the file -- up to the + 64MB ceiling, per attachment -- buys nothing and must not happen.""" + from raven.context_engine.segments import render + + pic = _write_image(tmp_path / "chart.png", (400, 300)) + reads: list[int | None] = [] + real_open = render.Path.open + + class _CountingHandle: + def __init__(self, inner): + self._inner = inner + + def read(self, n=None): + reads.append(n) + return self._inner.read(n) if n is not None else self._inner.read() + + def __enter__(self): + self._inner.__enter__() + return self + + def __exit__(self, *exc): + return self._inner.__exit__(*exc) + + render.Path.open = lambda self, *a, **k: _CountingHandle(real_open(self, *a, **k)) + try: + blind = render.build_user_content("look", [str(pic)], can_see_images=False) + blind_reads = list(reads) + reads.clear() + render.build_user_content("look", [str(pic)], can_see_images=True) + sighted_reads = list(reads) + finally: + render.Path.open = real_open + + assert "you cannot see images directly" in blind + # Header only for the blind model; the sighted one goes on to read the rest. + assert blind_reads == [render._SNIFF_BYTES] + assert sighted_reads == [render._SNIFF_BYTES, None] + + +def test_the_oversize_note_points_at_the_tool_that_could_still_help(tmp_path: Path, monkeypatch) -> None: + """Only a model that can see images reaches this branch, and read_file + downscales rather than refusing on size -- so it, not the description tool, + is the useful next step.""" + from raven.context_engine.segments import render + + monkeypatch.setattr(render, "_MAX_IMAGE_BYTES", 16) + fat = _write_image(tmp_path / "fat.png", (200, 200)) + out = render.build_user_content("look", [str(fat)], can_see_images=True, describe_tool="understand_media") + + assert "too large" in out + assert "read_file" in out + assert "understand_media" not in out diff --git a/tests/test_tui_rpc_turn_send.py b/tests/test_tui_rpc_turn_send.py index ad4a4fda..181153d3 100644 --- a/tests/test_tui_rpc_turn_send.py +++ b/tests/test_tui_rpc_turn_send.py @@ -260,3 +260,163 @@ async def test_turn_send_dispatcher_returns_minus_32003_on_concurrent_send( assert "error" in resp assert resp["error"]["code"] == -32003 assert resp["error"]["message"] == "turn_in_progress" + + +# --- Attachments --- +# +# ``media`` carries paths, not bytes: the front end has already put the file in +# the workspace. What these cover is the resolution policy, because the failure +# mode downstream is silent -- ``build_user_content`` drops a path that is not a +# file without a word, so a wrong resolve loses the attachment with no error +# anywhere in the stack. + + +def _workspace_cfg(tmp_path, *, restrict: bool = True): + """Patch load_config so the resolver sees ``tmp_path`` as the workspace.""" + from unittest.mock import MagicMock + + cfg = MagicMock() + cfg.agents.defaults.workspace = str(tmp_path) + cfg.tools.restrict_to_workspace = restrict + return patch("raven.config.load_config", return_value=cfg) + + +async def test_turn_send_resolves_a_workspace_relative_attachment(tmp_path) -> None: + scheduler = FakeScheduler() + (tmp_path / "uploads").mkdir() + shot = tmp_path / "uploads" / "shot.png" + shot.write_bytes(b"\x89PNG\r\n\x1a\n") + + with _workspace_cfg(tmp_path): + await turn_send( + {"session_key": "tui:default", "content": "look", "media": ["uploads/shot.png"]}, + scheduler=scheduler, + turn_ids={}, + ) + + # Resolved against the workspace, not the process cwd -- the front end sends + # back exactly what fs.upload returned, which is workspace-relative. + assert [m.path for m in scheduler.submitted[0].media] == [str(shot)] + + +async def test_turn_send_accepts_an_absolute_attachment(tmp_path) -> None: + scheduler = FakeScheduler() + shot = tmp_path / "shot.png" + shot.write_bytes(b"\x89PNG\r\n\x1a\n") + + with _workspace_cfg(tmp_path): + await turn_send( + {"session_key": "tui:default", "content": "look", "media": [str(shot)]}, + scheduler=scheduler, + turn_ids={}, + ) + + assert [m.path for m in scheduler.submitted[0].media] == [str(shot)] + + +async def test_turn_send_drops_a_missing_attachment_without_failing_the_turn(tmp_path) -> None: + scheduler = FakeScheduler() + kept = tmp_path / "kept.png" + kept.write_bytes(b"\x89PNG\r\n\x1a\n") + + with _workspace_cfg(tmp_path): + result = await turn_send( + { + "session_key": "tui:default", + "content": "look", + "media": ["uploads/gone.png", str(kept)], + }, + scheduler=scheduler, + turn_ids={}, + ) + + # One bad path must not cost the user the whole message. + assert result["accepted"] is True + assert [m.path for m in scheduler.submitted[0].media] == [str(kept)] + + +async def test_turn_send_refuses_an_attachment_outside_the_workspace(tmp_path) -> None: + scheduler = FakeScheduler() + ws = tmp_path / "ws" + ws.mkdir() + outside = tmp_path / "secret.png" + outside.write_bytes(b"\x89PNG\r\n\x1a\n") + + with _workspace_cfg(ws): + await turn_send( + {"session_key": "tui:default", "content": "look", "media": [str(outside)]}, + scheduler=scheduler, + turn_ids={}, + ) + + # The viewer and the file tools refuse this path; the attachment lane may + # not become the way around them. + assert scheduler.submitted[0].media == () + + +async def test_turn_send_without_media_submits_none(tmp_path) -> None: + scheduler = FakeScheduler() + await turn_send({"session_key": "tui:default", "content": "hi"}, scheduler=scheduler, turn_ids={}) + assert scheduler.submitted[0].media == () + + +async def test_turn_send_accepts_an_absolute_path_when_the_workspace_is_not_enforced(tmp_path) -> None: + """`restrict_to_workspace` defaults to False, so this is the shipped path. + + The attachment lane deliberately matches the filesystem tools rather than + inventing a second policy: a file the agent may read is a file the user may + hand it. + """ + scheduler = FakeScheduler() + outside = tmp_path / "elsewhere.png" + outside.write_bytes(b"\x89PNG\r\n\x1a\n") + ws = tmp_path / "ws" + ws.mkdir() + + with _workspace_cfg(ws, restrict=False): + await turn_send( + {"session_key": "tui:default", "content": "look", "media": [str(outside)]}, + scheduler=scheduler, + turn_ids={}, + ) + + assert [m.path for m in scheduler.submitted[0].media] == [str(outside)] + + +@pytest.mark.parametrize( + "bad", + [ + "with\x00null.png", # ValueError from the OS layer + "~nosuchuser42/x.png", # RuntimeError from expanduser + "x" * 300 + ".png", # OSError: name too long + ], +) +async def test_a_path_the_os_rejects_drops_the_attachment_not_the_turn(tmp_path, bad) -> None: + """Every rejection shape has to be caught here. Escaping this function turns + one unusable attachment into a turn that never runs at all.""" + scheduler = FakeScheduler() + kept = tmp_path / "kept.png" + kept.write_bytes(b"\x89PNG\r\n\x1a\n") + + with _workspace_cfg(tmp_path): + result = await turn_send( + {"session_key": "tui:default", "content": "look", "media": [bad, str(kept)]}, + scheduler=scheduler, + turn_ids={}, + ) + + assert result["accepted"] is True + assert [m.path for m in scheduler.submitted[0].media] == [str(kept)] + + +async def test_a_broken_config_drops_attachments_without_failing_the_turn(tmp_path) -> None: + scheduler = FakeScheduler() + with patch("raven.config.load_config", side_effect=RuntimeError("config on fire")): + result = await turn_send( + {"session_key": "tui:default", "content": "look", "media": ["uploads/x.png"]}, + scheduler=scheduler, + turn_ids={}, + ) + + assert result["accepted"] is True + assert scheduler.submitted[0].media == () diff --git a/ui-tui/rpc-schema/openrpc.json b/ui-tui/rpc-schema/openrpc.json index 6baa90c9..65a84d69 100644 --- a/ui-tui/rpc-schema/openrpc.json +++ b/ui-tui/rpc-schema/openrpc.json @@ -411,6 +411,11 @@ "name": "sender_id", "required": false, "schema": { "type": "string" } + }, + { + "name": "media", + "required": false, + "schema": { "type": "array", "items": { "type": "string" }, "maxItems": 64 } } ], "result": { diff --git a/ui-tui/src/rpc/generated.ts b/ui-tui/src/rpc/generated.ts index 6ee8f91b..d8fa6050 100644 --- a/ui-tui/src/rpc/generated.ts +++ b/ui-tui/src/rpc/generated.ts @@ -628,6 +628,10 @@ export interface TurnSendParams { channel?: string; chat_id?: string; sender_id?: string; + /** + * @maxItems 64 + */ + media?: string[]; } /** * This interface was referenced by `RavenRpcRoot`'s JSON-Schema