From d4411b0bd3d8e0755b834dccb9834d0876ef57d3 Mon Sep 17 00:00:00 2001 From: arelchan <204152633+arelchan@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:36:32 +0800 Subject: [PATCH 1/2] feat(*): send user attachments to the model, gated on vision support turn.send gained an optional media param, so a caller can hand the spine attachment paths the way channels already do. Paths resolve with the filesystem tools' own policy, and one that does not resolve is dropped with a log line rather than failing the turn. A picture then reaches the model as an inline base64 image_url block when the model can see it, and as a note naming the file when it cannot. The verdict comes from the gateway catalog Raven already fetches for pricing, which publishes input_modalities for every model it lists, and it is warmed in the background because the pricing path only ever reaches that catalog for models LiteLLM's static table misses. Absence, staleness, an exception and a deployment name all read as "no answer", which resolves to optimism and is never cached. Being wrong that way fails loudly at the endpoint; being wrong the other way silently turns a picture into prose, and there is no recovery on the attachment path. Co-authored-by: Claude (claude-opus-5) --- raven/agent/context/builder.py | 51 +- raven/agent/loop/main.py | 144 +++- raven/context_engine/assembler.py | 9 +- raven/context_engine/base.py | 2 + raven/context_engine/curator.py | 9 + raven/context_engine/segments/curator.py | 2 + raven/context_engine/segments/render.py | 137 +++- raven/providers/capabilities.py | 150 ++++- raven/providers/registry.py | 9 + raven/token_wise/model_catalog_cache.py | 7 +- raven/token_wise/pricing.py | 128 +++- raven/tui_rpc/methods/turn.py | 56 +- raven/tui_rpc/models.py | 8 + tests/test_plugin_tools.py | 35 +- tests/test_read_file_image.py | 822 ++++++++++++++++++++++- tests/test_tui_rpc_turn_send.py | 160 +++++ ui-tui/rpc-schema/openrpc.json | 5 + ui-tui/src/rpc/generated.ts | 4 + 18 files changed, 1657 insertions(+), 81 deletions(-) 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/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..71ac70fc 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,128 @@ 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 + # fails on a PDF or an audio file, so naming it here would + # just be a different instruction the model cannot follow. + notes.append(f"[Attachment: {p.name} (path: {p}){hint}]") + continue + if size > _MAX_IMAGE_BYTES: + notes.append(f"[Image: {p.name} (path: {p}) — too large to read into this message{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 + if not can_see_images: + notes.append(f"[Image: {p.name} (path: {p}) — you cannot see images directly{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 + 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/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..f416e7e7 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,773 @@ 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) 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 From 6e7cb0552a8bff34904097e71416b6b96473c00e Mon Sep 17 00:00:00 2001 From: arelchan <204152633+arelchan@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:06:12 +0800 Subject: [PATCH 2/2] fix(*): stop naming a tool that cannot help with the picture at hand Three loose ends from the delivery change, all about what the model is told to do next. read_file is the tool that hands a model the picture, so the note it gets when a message cannot carry one now names read_file wherever read_file would work: past the blind check the model can see, and read_file downscales rather than refusing on size. The description tool is named only where it is the one that can help -- a non-image attachment, or a model with no vision at all -- and understand_media's own description now recognises the '[Image: ...]' note as a source of paths, which it had to after that note started pointing at it. read_file's description drops "when the active model can see images". It described an implementation detail the model cannot evaluate about itself, and the runtime now says so directly in the result instead. The blind branch also stops reading the file. Every reason to refuse is settled from the header and the path, so a model that cannot see a picture no longer pays to load one, up to the per-attachment ceiling. Co-authored-by: Claude (claude-opus-5) --- raven/agent/tools/filesystem.py | 2 +- raven/context_engine/segments/render.py | 40 +++++++++++------- raven/plugin/memory/everos/tools.py | 7 ++-- tests/test_read_file_image.py | 56 +++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 18 deletions(-) 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/segments/render.py b/raven/context_engine/segments/render.py index 71ac70fc..6edba944 100644 --- a/raven/context_engine/segments/render.py +++ b/raven/context_engine/segments/render.py @@ -277,12 +277,35 @@ def build_user_content( if not is_image: # No fallback hint when there is no description tool. The # obvious candidate, read_file, decodes text and images and - # fails on a PDF or an audio file, so naming it here would - # just be a different instruction the model cannot follow. + # 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: - notes.append(f"[Image: {p.name} (path: {p}) — too large to read into this message{hint}]") + # 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: @@ -291,17 +314,6 @@ def build_user_content( # 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 - if not can_see_images: - notes.append(f"[Image: {p.name} (path: {p}) — you cannot see images directly{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 block = _inline_image(raw, mime, p, notes) if block is not None: images.append(block) 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/tests/test_read_file_image.py b/tests/test_read_file_image.py index f416e7e7..dd1749c6 100644 --- a/tests/test_read_file_image.py +++ b/tests/test_read_file_image.py @@ -1900,3 +1900,59 @@ def test_turn_send_refuses_an_unbounded_attachment_list() -> None: 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