From 079e51093c4d4133aebf6cfbf27646c3d49313ce Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 9 Mar 2026 22:28:47 +0530 Subject: [PATCH 1/2] Fix inline multimodal read_file attachments --- run_agent.py | 225 +++++++++++++++++++++++++--- tests/test_provider_parity.py | 63 ++++++++ tests/test_run_agent.py | 31 ++++ tests/tools/test_file_tools_live.py | 29 ++++ tools/file_operations.py | 88 +++++------ tools/file_tools.py | 2 +- 6 files changed, 376 insertions(+), 62 deletions(-) diff --git a/run_agent.py b/run_agent.py index 89e1ad00ea02..3056c7866dbf 100644 --- a/run_agent.py +++ b/run_agent.py @@ -791,7 +791,7 @@ def _log_msg_to_db(self, msg: Dict): return try: role = msg.get("role", "unknown") - content = msg.get("content") + content = self._content_for_storage(msg.get("content")) tool_calls_data = None if hasattr(msg, "tool_calls") and msg.tool_calls: tool_calls_data = [ @@ -825,7 +825,7 @@ def _flush_messages_to_session_db(self, messages: List[Dict], conversation_histo start_idx = len(conversation_history) if conversation_history else 0 for msg in messages[start_idx:]: role = msg.get("role", "unknown") - content = msg.get("content") + content = self._content_for_storage(msg.get("content")) tool_calls_data = None if hasattr(msg, "tool_calls") and msg.tool_calls: tool_calls_data = [ @@ -846,6 +846,138 @@ def _flush_messages_to_session_db(self, messages: List[Dict], conversation_histo except Exception as e: logger.debug("Session DB append_message failed: %s", e) + @staticmethod + def _content_for_storage(content: Any) -> Any: + """Serialize non-string message content for session storage.""" + if isinstance(content, (dict, list)): + return json.dumps(content, ensure_ascii=False) + return content + + @staticmethod + def _multimodal_user_parts_to_responses(content: Any) -> Optional[List[Dict[str, Any]]]: + """Convert chat-style multimodal user content to Responses API parts.""" + if not isinstance(content, list): + return None + + parts: List[Dict[str, Any]] = [] + for part in content: + if not isinstance(part, dict): + continue + + part_type = part.get("type") + if part_type == "text": + text = part.get("text") + if isinstance(text, str) and text: + parts.append({"type": "input_text", "text": text}) + continue + + if part_type == "image_url": + image = part.get("image_url") + if isinstance(image, dict): + image_url = image.get("url") + if isinstance(image_url, str) and image_url: + item = {"type": "input_image", "image_url": image_url} + detail = image.get("detail") + if isinstance(detail, str) and detail: + item["detail"] = detail + parts.append(item) + continue + + if part_type == "file": + file_obj = part.get("file") + if isinstance(file_obj, dict): + item = {"type": "input_file"} + filename = file_obj.get("filename") + if isinstance(filename, str) and filename: + item["filename"] = filename + for key in ("file_data", "file_url", "file_id"): + value = file_obj.get(key) + if isinstance(value, str) and value: + item[key] = value + break + if any(key in item for key in ("file_data", "file_url", "file_id")): + parts.append(item) + + return parts or None + + def _supports_inline_read_file_attachment(self, attachment_kind: str) -> bool: + """Return True when the active provider path supports injected attachments.""" + if attachment_kind == "image": + if self.api_mode == "codex_responses": + return self.provider == "openai-codex" + return self.provider == "openrouter" and "openrouter.ai" in (self.base_url or "").lower() + if attachment_kind == "pdf": + return self.api_mode != "codex_responses" and "openrouter.ai" in (self.base_url or "").lower() + return False + + def _build_tool_result_messages( + self, + function_name: str, + function_args: Dict[str, Any], + function_result: str, + tool_call_id: str, + ) -> List[Dict[str, Any]]: + """Build conversation messages for a completed tool call.""" + tool_content = function_result + inline_message = None + + if function_name == "read_file": + try: + parsed = json.loads(function_result) + except Exception: + parsed = None + + if isinstance(parsed, dict) and isinstance(parsed.get("base64_content"), str): + attachment_kind = "image" if parsed.get("is_image") else "pdf" if parsed.get("is_pdf") else None + mime_type = parsed.get("mime_type") + base64_content = parsed.get("base64_content") + path = str(function_args.get("path") or "") + + if attachment_kind and isinstance(mime_type, str) and mime_type and base64_content: + if self._supports_inline_read_file_attachment(attachment_kind): + intro = ( + f"Hermes inline attachment from read_file('{path}'). " + "Treat this as tool output context, not a new user request." + ) + data_url = f"data:{mime_type};base64,{base64_content}" + parts: List[Dict[str, Any]] = [{"type": "text", "text": intro}] + if attachment_kind == "image": + parts.append({"type": "image_url", "image_url": {"url": data_url}}) + else: + parts.append( + { + "type": "file", + "file": { + "filename": os.path.basename(path) or "document.pdf", + "file_data": data_url, + }, + } + ) + inline_message = {"role": "user", "content": parts} + + parsed = parsed.copy() + parsed.pop("base64_content", None) + inline_hint = ( + "Inline attachment added to conversation context." + if inline_message + else ( + "Attachment metadata retained; inline attachment not supported for the current provider/API mode." + if attachment_kind != "pdf" + else "Attachment metadata retained; inline PDF attachment is currently limited to supported OpenRouter chat-completions paths." + ) + ) + existing_hint = parsed.get("hint") + if isinstance(existing_hint, str) and existing_hint.strip(): + parsed["hint"] = f"{existing_hint} {inline_hint}" + else: + parsed["hint"] = inline_hint + tool_content = json.dumps(parsed, ensure_ascii=False) + + messages = [{"role": "tool", "content": tool_content, "tool_call_id": tool_call_id}] + if inline_message is not None: + messages.append(inline_message) + return messages + def _get_messages_up_to_last_assistant(self, messages: List[Dict]) -> List[Dict]: """ Get messages up to (but not including) the last assistant turn. @@ -1516,9 +1648,9 @@ def _chat_messages_to_responses_input(self, messages: List[Dict[str, Any]]) -> L if role in {"user", "assistant"}: content = msg.get("content", "") - content_text = str(content) if content is not None else "" if role == "assistant": + content_text = str(content) if content is not None else "" # Replay encrypted reasoning items from previous turns # so the API can maintain coherent reasoning chains. codex_reasoning = msg.get("codex_reasoning_items") @@ -1572,7 +1704,12 @@ def _chat_messages_to_responses_input(self, messages: List[Dict[str, Any]]) -> L }) continue - items.append({"role": role, "content": content_text}) + responses_parts = self._multimodal_user_parts_to_responses(content) + if responses_parts is not None: + items.append({"role": role, "content": responses_parts}) + else: + content_text = str(content) if content is not None else "" + items.append({"role": role, "content": content_text}) continue if role == "tool": @@ -1663,6 +1800,53 @@ def _preflight_codex_input_items(self, raw_items: Any) -> List[Dict[str, Any]]: role = item.get("role") if role in {"user", "assistant"}: content = item.get("content", "") + if role == "user" and isinstance(content, list): + normalized_parts: List[Dict[str, Any]] = [] + for part in content: + if not isinstance(part, dict): + raise ValueError(f"Codex Responses input[{idx}] user content parts must be objects.") + + part_type = part.get("type") + if part_type == "input_text": + text = part.get("text", "") + if not isinstance(text, str): + text = str(text) + normalized_parts.append({"type": "input_text", "text": text}) + continue + + if part_type == "input_image": + image_url = part.get("image_url") + if not isinstance(image_url, str) or not image_url.strip(): + raise ValueError(f"Codex Responses input[{idx}] input_image is missing image_url.") + normalized_part = {"type": "input_image", "image_url": image_url.strip()} + detail = part.get("detail") + if isinstance(detail, str) and detail.strip(): + normalized_part["detail"] = detail.strip() + normalized_parts.append(normalized_part) + continue + + if part_type == "input_file": + normalized_part = {"type": "input_file"} + filename = part.get("filename") + if isinstance(filename, str) and filename.strip(): + normalized_part["filename"] = filename.strip() + for key in ("file_data", "file_url", "file_id"): + value = part.get(key) + if isinstance(value, str) and value.strip(): + normalized_part[key] = value.strip() + break + if not any(key in normalized_part for key in ("file_data", "file_url", "file_id")): + raise ValueError(f"Codex Responses input[{idx}] input_file is missing file data.") + normalized_parts.append(normalized_part) + continue + + raise ValueError( + f"Codex Responses input[{idx}] has unsupported user content part type {part_type!r}." + ) + + normalized.append({"role": role, "content": normalized_parts}) + continue + if content is None: content = "" if not isinstance(content, str): @@ -2692,29 +2876,36 @@ def _execute_tool_calls(self, assistant_message, messages: list, effective_task_ logging.debug(f"Tool {function_name} completed in {tool_duration:.2f}s") logging.debug(f"Tool result preview: {result_preview}...") + tool_messages = self._build_tool_result_messages( + function_name=function_name, + function_args=function_args, + function_result=function_result, + tool_call_id=tool_call.id, + ) + # Guard against tools returning absurdly large content that would # blow up the context window. 100K chars ≈ 25K tokens — generous # enough for any reasonable tool output but prevents catastrophic - # context explosions (e.g. accidental base64 image dumps). + # context explosions. MAX_TOOL_RESULT_CHARS = 100_000 - if len(function_result) > MAX_TOOL_RESULT_CHARS: - original_len = len(function_result) - function_result = ( - function_result[:MAX_TOOL_RESULT_CHARS] + primary_content = tool_messages[0]["content"] + if isinstance(primary_content, str) and len(primary_content) > MAX_TOOL_RESULT_CHARS: + original_len = len(primary_content) + tool_messages[0]["content"] = ( + primary_content[:MAX_TOOL_RESULT_CHARS] + f"\n\n[Truncated: tool response was {original_len:,} chars, " f"exceeding the {MAX_TOOL_RESULT_CHARS:,} char limit]" ) - tool_msg = { - "role": "tool", - "content": function_result, - "tool_call_id": tool_call.id - } - messages.append(tool_msg) - self._log_msg_to_db(tool_msg) + for tool_msg in tool_messages: + messages.append(tool_msg) + self._log_msg_to_db(tool_msg) if not self.quiet_mode: - response_preview = function_result[:self.log_prefix_chars] + "..." if len(function_result) > self.log_prefix_chars else function_result + log_content = tool_messages[0]["content"] + if not isinstance(log_content, str): + log_content = json.dumps(log_content, ensure_ascii=False) + response_preview = log_content[:self.log_prefix_chars] + "..." if len(log_content) > self.log_prefix_chars else log_content print(f" ✅ Tool {i} completed in {tool_duration:.2f}s - {response_preview}") if self._interrupt_requested and i < len(assistant_message.tool_calls): diff --git a/tests/test_provider_parity.py b/tests/test_provider_parity.py index 2ee3131449dc..ddac86751111 100644 --- a/tests/test_provider_parity.py +++ b/tests/test_provider_parity.py @@ -216,6 +216,69 @@ def test_tool_results_become_function_call_output(self, monkeypatch): assert items[0]["call_id"] == "call_abc" assert items[0]["output"] == "result here" + def test_user_multimodal_message_maps_to_codex_input_parts(self, monkeypatch): + agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses", + base_url="https://chatgpt.com/backend-api/codex") + messages = [{ + "role": "user", + "content": [ + {"type": "text", "text": "Inspect these"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + { + "type": "file", + "file": { + "filename": "report.pdf", + "file_data": "data:application/pdf;base64,JVBERi0xLjQK", + }, + }, + ], + }] + + items = agent._chat_messages_to_responses_input(messages) + + assert items == [{ + "role": "user", + "content": [ + {"type": "input_text", "text": "Inspect these"}, + {"type": "input_image", "image_url": "data:image/png;base64,AAAA"}, + { + "type": "input_file", + "filename": "report.pdf", + "file_data": "data:application/pdf;base64,JVBERi0xLjQK", + }, + ], + }] + + +class TestInlineReadFileAttachmentGating: + def test_custom_chat_completions_does_not_inject_inline_image(self, monkeypatch): + agent = _make_agent(monkeypatch, "custom", api_mode="chat_completions", base_url="http://localhost:1234/v1") + + messages = agent._build_tool_result_messages( + function_name="read_file", + function_args={"path": "assets/pixel.png"}, + function_result='{"is_image":true,"is_binary":true,"mime_type":"image/png","base64_content":"AAAA"}', + tool_call_id="call_img", + ) + + assert len(messages) == 1 + assert messages[0]["role"] == "tool" + assert "not supported for the current provider/API mode" in messages[0]["content"] + + def test_codex_does_not_inject_inline_pdf_and_explains_limitation(self, monkeypatch): + agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses", + base_url="https://chatgpt.com/backend-api/codex") + + messages = agent._build_tool_result_messages( + function_name="read_file", + function_args={"path": "docs/report.pdf"}, + function_result='{"is_pdf":true,"is_binary":true,"mime_type":"application/pdf","base64_content":"JVBERi0xLjQK"}', + tool_call_id="call_pdf", + ) + + assert len(messages) == 1 + assert "supported OpenRouter chat-completions paths" in messages[0]["content"] + def test_encrypted_reasoning_replayed(self, monkeypatch): """Encrypted reasoning items from previous turns must be included in input.""" agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses", diff --git a/tests/test_run_agent.py b/tests/test_run_agent.py index 226b29a6d645..d6c547f69d33 100644 --- a/tests/test_run_agent.py +++ b/tests/test_run_agent.py @@ -217,6 +217,37 @@ def test_extra_newlines_cleaned(self): assert "after" in result +class TestInlineReadFileAttachments: + def test_build_tool_result_messages_injects_image_context(self, agent): + tool_result = json.dumps( + { + "is_image": True, + "is_binary": True, + "file_size": 68, + "mime_type": "image/png", + "base64_content": "AAAA", + } + ) + + messages = agent._build_tool_result_messages( + function_name="read_file", + function_args={"path": "assets/pixel.png"}, + function_result=tool_result, + tool_call_id="call_img", + ) + + assert messages[0]["role"] == "tool" + assert messages[0]["tool_call_id"] == "call_img" + assert "base64_content" not in messages[0]["content"] + assert messages[1]["role"] == "user" + assert messages[1]["content"][0]["type"] == "text" + assert "read_file('assets/pixel.png')" in messages[1]["content"][0]["text"] + assert messages[1]["content"][1] == { + "type": "image_url", + "image_url": {"url": "data:image/png;base64,AAAA"}, + } + + class TestGetMessagesUpToLastAssistant: def test_empty_list(self, agent): assert agent._get_messages_up_to_last_assistant([]) == [] diff --git a/tests/tools/test_file_tools_live.py b/tests/tools/test_file_tools_live.py index 426b3543bbb5..e09ac7a055be 100644 --- a/tests/tools/test_file_tools_live.py +++ b/tests/tools/test_file_tools_live.py @@ -8,6 +8,7 @@ asserts zero contamination from shell noise via _assert_clean(). """ +import base64 import json import os import sys @@ -59,6 +60,10 @@ def _assert_clean(text: str, context: str = "output"): MULTIFILE_A = "def func_alpha():\n return 42\n" MULTIFILE_B = "def func_bravo():\n return 99\n" MULTIFILE_C = "nothing relevant here\n" +TINY_PNG = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Wn9l1cAAAAASUVORK5CYII=" +) +MINIMAL_PDF = b"%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\ntrailer\n<<>>\n%%EOF\n" @pytest.fixture @@ -354,6 +359,30 @@ def test_no_noise_in_content(self, ops, tmp_path): assert result.error is None _assert_clean(result.content) + def test_small_png_returns_inline_attachment_data(self, ops, tmp_path): + f = tmp_path / "pixel.png" + f.write_bytes(TINY_PNG) + + result = ops.read_file(str(f)) + + assert result.error is None + assert result.is_image is True + assert result.is_binary is True + assert result.mime_type == "image/png" + assert base64.b64decode(result.base64_content) == TINY_PNG + + def test_small_pdf_returns_inline_attachment_data(self, ops, tmp_path): + f = tmp_path / "report.pdf" + f.write_bytes(MINIMAL_PDF) + + result = ops.read_file(str(f)) + + assert result.error is None + assert result.is_pdf is True + assert result.is_binary is True + assert result.mime_type == "application/pdf" + assert base64.b64decode(result.base64_content) == MINIMAL_PDF + # ── write_file ─────────────────────────────────────────────────────────── diff --git a/tools/file_operations.py b/tools/file_operations.py index 182d35f5f213..61fbb9dd5270 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -100,6 +100,7 @@ class ReadResult: hint: Optional[str] = None is_binary: bool = False is_image: bool = False + is_pdf: bool = False base64_content: Optional[str] = None mime_type: Optional[str] = None dimensions: Optional[str] = None # For images: "WIDTHxHEIGHT" @@ -273,6 +274,7 @@ def search(self, pattern: str, path: str = ".", target: str = "content", # Image extensions (subset of binary that we can return as base64) IMAGE_EXTENSIONS = {'.png', '.jpg', '.jpeg', '.gif', '.webp', '.bmp', '.ico'} +INLINE_FILE_EXTENSIONS = IMAGE_EXTENSIONS | {'.pdf'} # Linters by file extension LINTERS = { @@ -368,6 +370,15 @@ def _is_image(self, path: str) -> bool: """Check if file is an image we can return as base64.""" ext = os.path.splitext(path)[1].lower() return ext in IMAGE_EXTENSIONS + + def _is_pdf(self, path: str) -> bool: + """Check if file is a PDF we can return as an attachment.""" + return os.path.splitext(path)[1].lower() == '.pdf' + + def _is_inline_attachment(self, path: str) -> bool: + """Check if file is a binary attachment we can inline for multimodal models.""" + ext = os.path.splitext(path)[1].lower() + return ext in INLINE_FILE_EXTENSIONS def _add_line_numbers(self, content: str, start_line: int = 1) -> str: """Add line numbers to content in LINE_NUM|CONTENT format.""" @@ -463,17 +474,8 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: # Still try to read, but warn pass - # Images are never inlined — redirect to the vision tool - if self._is_image(path): - return ReadResult( - is_image=True, - is_binary=True, - file_size=file_size, - hint=( - "Image file detected. Automatically redirected to vision_analyze tool. " - "Use vision_analyze with this file path to inspect the image contents." - ), - ) + if self._is_inline_attachment(path): + return self._read_inline_attachment(path, file_size) # Read a sample to check for binary content sample_cmd = f"head -c 1000 {self._escape_shell_arg(path)} 2>/dev/null" @@ -516,52 +518,48 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: hint=hint ) - # Images larger than this are too expensive to inline as base64 in the - # conversation context. Return metadata only and suggest vision_analyze. - MAX_IMAGE_BYTES = 512 * 1024 # 512 KB + MAX_INLINE_ATTACHMENT_BYTES = 512 * 1024 # 512 KB - def _read_image(self, path: str) -> ReadResult: - """Read an image file, returning base64 content.""" - # Get file size (wc -c is POSIX, works on Linux + macOS) - stat_cmd = f"wc -c < {self._escape_shell_arg(path)} 2>/dev/null" - stat_result = self._exec(stat_cmd) - try: - file_size = int(stat_result.stdout.strip()) - except ValueError: - file_size = 0 - - if file_size > self.MAX_IMAGE_BYTES: + def _read_inline_attachment(self, path: str, file_size: int) -> ReadResult: + """Read an image or PDF file, returning base64 content when small enough.""" + is_image = self._is_image(path) + is_pdf = self._is_pdf(path) + + if file_size > self.MAX_INLINE_ATTACHMENT_BYTES: + noun = "Image" if is_image else "PDF" + guidance = ( + "Use vision_analyze to inspect the image, or reference it by path." + if is_image + else "Reference it by path or use terminal tools to inspect it." + ) return ReadResult( - is_image=True, + is_image=is_image, + is_pdf=is_pdf, is_binary=True, file_size=file_size, - hint=( - f"Image is too large to inline ({file_size:,} bytes). " - "Use vision_analyze to inspect the image, or reference it by path." - ), + hint=f"{noun} is too large to inline ({file_size:,} bytes). {guidance}", ) - - # Get base64 content - b64_cmd = f"base64 -w 0 {self._escape_shell_arg(path)} 2>/dev/null" + + b64_cmd = f"base64 < {self._escape_shell_arg(path)} | tr -d '\\n'" b64_result = self._exec(b64_cmd, timeout=30) - + if b64_result.exit_code != 0: + noun = "image" if is_image else "PDF" return ReadResult( - is_image=True, + is_image=is_image, + is_pdf=is_pdf, is_binary=True, file_size=file_size, - error=f"Failed to read image: {b64_result.stdout}" + error=f"Failed to read {noun}: {b64_result.stdout}", ) - - # Try to get dimensions (requires ImageMagick) + dimensions = None - if self._has_command('identify'): + if is_image and self._has_command('identify'): dim_cmd = f"identify -format '%wx%h' {self._escape_shell_arg(path)} 2>/dev/null" dim_result = self._exec(dim_cmd) if dim_result.exit_code == 0: dimensions = dim_result.stdout.strip() - - # Determine MIME type from extension + ext = os.path.splitext(path)[1].lower() mime_types = { '.png': 'image/png', @@ -571,16 +569,18 @@ def _read_image(self, path: str) -> ReadResult: '.webp': 'image/webp', '.bmp': 'image/bmp', '.ico': 'image/x-icon', + '.pdf': 'application/pdf', } mime_type = mime_types.get(ext, 'application/octet-stream') - + return ReadResult( - is_image=True, + is_image=is_image, + is_pdf=is_pdf, is_binary=True, file_size=file_size, base64_content=b64_result.stdout, mime_type=mime_type, - dimensions=dimensions + dimensions=dimensions, ) def _suggest_similar_files(self, path: str) -> ReadResult: diff --git a/tools/file_tools.py b/tools/file_tools.py index b29d2d274c28..780e60b9b97c 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -211,7 +211,7 @@ def _check_file_reqs(): READ_FILE_SCHEMA = { "name": "read_file", - "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. NOTE: Cannot read images or binary files — use vision_analyze for images.", + "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. Small images may be attached inline for supported multimodal providers. Small PDFs are currently only attached inline on supported OpenRouter chat-completions paths; other providers return metadata only.", "parameters": { "type": "object", "properties": { From 9984383cf37dc4906a1fb05da48a61959ce19449 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 9 Mar 2026 22:56:32 +0530 Subject: [PATCH 2/2] fix: simplify inline attachment code and close truncation guard gap - Extract _classify_attachment() replacing _is_image/_is_pdf/_is_inline_attachment to avoid computing os.path.splitext 4 times per read - Extract module-level MIME_TYPES constant from inline dict - Add substring guard before json.loads in _build_tool_result_messages to skip parsing for ~95% of text-file read_file results - Fix silent base64 stripping when attachment_kind is None by scoping hint/strip logic to known attachment types only - Extend MAX_TOOL_RESULT_CHARS truncation guard to all tool_messages, closing gap where inline user message with ~700KB data URL bypassed it - Replace nested ternary hint construction with explicit if/elif/else - Extract _is_openrouter property to deduplicate base_url checks - Add .strip() to multimodal part values for whitespace safety --- run_agent.py | 89 +++++++++++++++-------------- tests/tools/test_file_operations.py | 12 ++-- tools/file_operations.py | 55 +++++++++--------- 3 files changed, 77 insertions(+), 79 deletions(-) diff --git a/run_agent.py b/run_agent.py index 3056c7866dbf..057dd4dd8ec2 100644 --- a/run_agent.py +++ b/run_agent.py @@ -293,9 +293,8 @@ def __init__( # Anthropic prompt caching: auto-enabled for Claude models via OpenRouter. # Reduces input costs by ~75% on multi-turn conversations by caching the # conversation prefix. Uses system_and_3 strategy (4 breakpoints). - is_openrouter = "openrouter" in self.base_url.lower() is_claude = "claude" in self.model.lower() - self._use_prompt_caching = is_openrouter and is_claude + self._use_prompt_caching = self._is_openrouter and is_claude self._cache_ttl = "5m" # Default 5-minute TTL (1.25x write cost) # Persistent error log -- always writes WARNING+ to ~/.hermes/logs/errors.log @@ -605,6 +604,11 @@ def __init__( else: print(f"📊 Context limit: {self.context_compressor.context_length:,} tokens (auto-compression disabled)") + @property + def _is_openrouter(self) -> bool: + """Whether the active base URL points to OpenRouter.""" + return "openrouter" in self.base_url.lower() + def _max_tokens_param(self, value: int) -> dict: """Return the correct max tokens kwarg for the current provider. @@ -614,7 +618,7 @@ def _max_tokens_param(self, value: int) -> dict: """ _is_direct_openai = ( "api.openai.com" in self.base_url.lower() - and "openrouter" not in self.base_url.lower() + and not self._is_openrouter ) if _is_direct_openai: return {"max_completion_tokens": value} @@ -875,11 +879,11 @@ def _multimodal_user_parts_to_responses(content: Any) -> Optional[List[Dict[str, image = part.get("image_url") if isinstance(image, dict): image_url = image.get("url") - if isinstance(image_url, str) and image_url: - item = {"type": "input_image", "image_url": image_url} + if isinstance(image_url, str) and image_url.strip(): + item = {"type": "input_image", "image_url": image_url.strip()} detail = image.get("detail") - if isinstance(detail, str) and detail: - item["detail"] = detail + if isinstance(detail, str) and detail.strip(): + item["detail"] = detail.strip() parts.append(item) continue @@ -888,12 +892,12 @@ def _multimodal_user_parts_to_responses(content: Any) -> Optional[List[Dict[str, if isinstance(file_obj, dict): item = {"type": "input_file"} filename = file_obj.get("filename") - if isinstance(filename, str) and filename: - item["filename"] = filename + if isinstance(filename, str) and filename.strip(): + item["filename"] = filename.strip() for key in ("file_data", "file_url", "file_id"): value = file_obj.get(key) - if isinstance(value, str) and value: - item[key] = value + if isinstance(value, str) and value.strip(): + item[key] = value.strip() break if any(key in item for key in ("file_data", "file_url", "file_id")): parts.append(item) @@ -905,9 +909,9 @@ def _supports_inline_read_file_attachment(self, attachment_kind: str) -> bool: if attachment_kind == "image": if self.api_mode == "codex_responses": return self.provider == "openai-codex" - return self.provider == "openrouter" and "openrouter.ai" in (self.base_url or "").lower() + return self._is_openrouter if attachment_kind == "pdf": - return self.api_mode != "codex_responses" and "openrouter.ai" in (self.base_url or "").lower() + return self.api_mode != "codex_responses" and self._is_openrouter return False def _build_tool_result_messages( @@ -921,7 +925,7 @@ def _build_tool_result_messages( tool_content = function_result inline_message = None - if function_name == "read_file": + if function_name == "read_file" and '"base64_content"' in function_result: try: parsed = json.loads(function_result) except Exception: @@ -933,7 +937,7 @@ def _build_tool_result_messages( base64_content = parsed.get("base64_content") path = str(function_args.get("path") or "") - if attachment_kind and isinstance(mime_type, str) and mime_type and base64_content: + if attachment_kind is not None and isinstance(mime_type, str) and mime_type and base64_content: if self._supports_inline_read_file_attachment(attachment_kind): intro = ( f"Hermes inline attachment from read_file('{path}'). " @@ -955,23 +959,21 @@ def _build_tool_result_messages( ) inline_message = {"role": "user", "content": parts} - parsed = parsed.copy() - parsed.pop("base64_content", None) - inline_hint = ( - "Inline attachment added to conversation context." - if inline_message - else ( - "Attachment metadata retained; inline attachment not supported for the current provider/API mode." - if attachment_kind != "pdf" - else "Attachment metadata retained; inline PDF attachment is currently limited to supported OpenRouter chat-completions paths." - ) - ) - existing_hint = parsed.get("hint") - if isinstance(existing_hint, str) and existing_hint.strip(): - parsed["hint"] = f"{existing_hint} {inline_hint}" - else: - parsed["hint"] = inline_hint - tool_content = json.dumps(parsed, ensure_ascii=False) + # Build hint based on what happened + if inline_message: + inline_hint = "Inline attachment added to conversation context." + elif attachment_kind == "pdf": + inline_hint = "Attachment metadata retained; inline PDF attachment is currently limited to supported OpenRouter chat-completions paths." + else: + inline_hint = "Attachment metadata retained; inline attachment not supported for the current provider/API mode." + + parsed = {k: v for k, v in parsed.items() if k != "base64_content"} + existing_hint = parsed.get("hint") + if isinstance(existing_hint, str) and existing_hint.strip(): + parsed["hint"] = f"{existing_hint} {inline_hint}" + else: + parsed["hint"] = inline_hint + tool_content = json.dumps(parsed, ensure_ascii=False) messages = [{"role": "tool", "content": tool_content, "tool_call_id": tool_call_id}] if inline_message is not None: @@ -2404,11 +2406,10 @@ def _build_api_kwargs(self, api_messages: list) -> dict: if provider_preferences: extra_body["provider"] = provider_preferences - _is_openrouter = "openrouter" in self.base_url.lower() _is_nous = "nousresearch" in self.base_url.lower() _is_mistral = "api.mistral.ai" in self.base_url.lower() - if (_is_openrouter or _is_nous) and not _is_mistral: + if (self._is_openrouter or _is_nous) and not _is_mistral: if self.reasoning_config is not None: extra_body["reasoning"] = self.reasoning_config else: @@ -2888,14 +2889,15 @@ def _execute_tool_calls(self, assistant_message, messages: list, effective_task_ # enough for any reasonable tool output but prevents catastrophic # context explosions. MAX_TOOL_RESULT_CHARS = 100_000 - primary_content = tool_messages[0]["content"] - if isinstance(primary_content, str) and len(primary_content) > MAX_TOOL_RESULT_CHARS: - original_len = len(primary_content) - tool_messages[0]["content"] = ( - primary_content[:MAX_TOOL_RESULT_CHARS] - + f"\n\n[Truncated: tool response was {original_len:,} chars, " - f"exceeding the {MAX_TOOL_RESULT_CHARS:,} char limit]" - ) + for tool_msg in tool_messages: + msg_content = tool_msg["content"] + if isinstance(msg_content, str) and len(msg_content) > MAX_TOOL_RESULT_CHARS: + original_len = len(msg_content) + tool_msg["content"] = ( + msg_content[:MAX_TOOL_RESULT_CHARS] + + f"\n\n[Truncated: tool response was {original_len:,} chars, " + f"exceeding the {MAX_TOOL_RESULT_CHARS:,} char limit]" + ) for tool_msg in tool_messages: messages.append(tool_msg) @@ -2956,9 +2958,8 @@ def _handle_max_iterations(self, messages: list, api_call_count: int) -> str: api_messages.insert(sys_offset + idx, pfm.copy()) summary_extra_body = {} - _is_openrouter = "openrouter" in self.base_url.lower() _is_nous = "nousresearch" in self.base_url.lower() - if _is_openrouter or _is_nous: + if self._is_openrouter or _is_nous: if self.reasoning_config is not None: summary_extra_body["reasoning"] = self.reasoning_config else: diff --git a/tests/tools/test_file_operations.py b/tests/tools/test_file_operations.py index b427826e5e7b..86a418b9193a 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -215,12 +215,12 @@ def test_is_likely_binary_by_content(self, file_ops): # Normal text -> not binary assert file_ops._is_likely_binary("unknown", "Hello world\nLine 2\n") is False - def test_is_image(self, file_ops): - assert file_ops._is_image("photo.png") is True - assert file_ops._is_image("pic.jpg") is True - assert file_ops._is_image("icon.ico") is True - assert file_ops._is_image("data.pdf") is False - assert file_ops._is_image("code.py") is False + def test_classify_attachment(self, file_ops): + assert file_ops._classify_attachment("photo.png") == "image" + assert file_ops._classify_attachment("pic.jpg") == "image" + assert file_ops._classify_attachment("icon.ico") == "image" + assert file_ops._classify_attachment("data.pdf") == "pdf" + assert file_ops._classify_attachment("code.py") is None def test_add_line_numbers(self, file_ops): content = "line one\nline two\nline three" diff --git a/tools/file_operations.py b/tools/file_operations.py index 61fbb9dd5270..ff3063398368 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -274,7 +274,18 @@ def search(self, pattern: str, path: str = ".", target: str = "content", # Image extensions (subset of binary that we can return as base64) IMAGE_EXTENSIONS = {'.png', '.jpg', '.jpeg', '.gif', '.webp', '.bmp', '.ico'} -INLINE_FILE_EXTENSIONS = IMAGE_EXTENSIONS | {'.pdf'} + +# MIME types for inline attachment extensions +MIME_TYPES = { + '.png': 'image/png', + '.jpg': 'image/jpeg', + '.jpeg': 'image/jpeg', + '.gif': 'image/gif', + '.webp': 'image/webp', + '.bmp': 'image/bmp', + '.ico': 'image/x-icon', + '.pdf': 'application/pdf', +} # Linters by file extension LINTERS = { @@ -366,19 +377,14 @@ def _is_likely_binary(self, path: str, content_sample: str = None) -> bool: return False - def _is_image(self, path: str) -> bool: - """Check if file is an image we can return as base64.""" - ext = os.path.splitext(path)[1].lower() - return ext in IMAGE_EXTENSIONS - - def _is_pdf(self, path: str) -> bool: - """Check if file is a PDF we can return as an attachment.""" - return os.path.splitext(path)[1].lower() == '.pdf' - - def _is_inline_attachment(self, path: str) -> bool: - """Check if file is a binary attachment we can inline for multimodal models.""" + def _classify_attachment(self, path: str) -> Optional[str]: + """Return 'image', 'pdf', or None based on file extension.""" ext = os.path.splitext(path)[1].lower() - return ext in INLINE_FILE_EXTENSIONS + if ext in IMAGE_EXTENSIONS: + return "image" + if ext == ".pdf": + return "pdf" + return None def _add_line_numbers(self, content: str, start_line: int = 1) -> str: """Add line numbers to content in LINE_NUM|CONTENT format.""" @@ -474,8 +480,9 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: # Still try to read, but warn pass - if self._is_inline_attachment(path): - return self._read_inline_attachment(path, file_size) + attachment_kind = self._classify_attachment(path) + if attachment_kind is not None: + return self._read_inline_attachment(path, file_size, attachment_kind) # Read a sample to check for binary content sample_cmd = f"head -c 1000 {self._escape_shell_arg(path)} 2>/dev/null" @@ -520,10 +527,10 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: MAX_INLINE_ATTACHMENT_BYTES = 512 * 1024 # 512 KB - def _read_inline_attachment(self, path: str, file_size: int) -> ReadResult: + def _read_inline_attachment(self, path: str, file_size: int, attachment_kind: str) -> ReadResult: """Read an image or PDF file, returning base64 content when small enough.""" - is_image = self._is_image(path) - is_pdf = self._is_pdf(path) + is_image = attachment_kind == "image" + is_pdf = attachment_kind == "pdf" if file_size > self.MAX_INLINE_ATTACHMENT_BYTES: noun = "Image" if is_image else "PDF" @@ -561,17 +568,7 @@ def _read_inline_attachment(self, path: str, file_size: int) -> ReadResult: dimensions = dim_result.stdout.strip() ext = os.path.splitext(path)[1].lower() - mime_types = { - '.png': 'image/png', - '.jpg': 'image/jpeg', - '.jpeg': 'image/jpeg', - '.gif': 'image/gif', - '.webp': 'image/webp', - '.bmp': 'image/bmp', - '.ico': 'image/x-icon', - '.pdf': 'application/pdf', - } - mime_type = mime_types.get(ext, 'application/octet-stream') + mime_type = MIME_TYPES.get(ext, 'application/octet-stream') return ReadResult( is_image=is_image,