fix(EVO-2181): let images reach the model (not only audio)

src/services/adk/runners/runner_utils.py

@@ -274,6 +274,7 @@ async def process_files(
         transcribed_texts = []
 
         if files and len(files) > 0:
+            inlined_bytes = 0
             for file_data in files:
                 try:
                     # Check if file is audio
@@ -293,6 +294,32 @@ async def process_files(
                         )
                     )
 
+                    # EVO-2181: add the file to the content parts so the model
+                    # actually receives it. This append used to be gated behind
+                    # `if is_audio`, so an image was blobbed and saved to the
+                    # artifact store but never handed to the LLM -> the agent
+                    # replied "No content to process".
+                    #
+                    # It runs before save_artifact on purpose: the bytes are
+                    # already in hand, and a failure while storing them must not
+                    # cost the model its copy of the file and bring this very bug
+                    # back.
+                    skip_reason = self._inline_skip_reason(
+                        file_data.content_type, len(file_bytes), inlined_bytes
+                    )
+                    if skip_reason:
+                        logger.warning(
+                            f"File {file_data.filename} ({file_data.content_type}) not sent"
+                            f" to the model: {skip_reason}. Still saved as an artifact."
+                        )
+                    else:
+                        inlined_bytes += len(file_bytes)
+                        file_parts.append(file_part)
+                        logger.info(
+                            f"Added {'audio' if is_audio else 'file'} {file_data.filename}"
+                            f" ({file_data.content_type}) to content parts for LLM processing"
+                        )
+
                     # Always save to artifacts for reference
                     await artifacts_service.save_artifact(
                         app_name=agent_id,
@@ -301,12 +328,6 @@ async def process_files(
                         filename=file_data.filename,
                         artifact=file_part,
                     )
-                    if is_audio:
-                        # Audio file - add to content parts for LLM processing
-                        file_parts.append(file_part)
-                        logger.info(
-                            f"Added audio file {file_data.filename} to content parts for LLM processing"
-                        )
 
                 except Exception as e:
                     logger.error(
@@ -315,6 +336,55 @@ async def process_files(
 
         return file_parts, transcribed_texts
 
+    # What google-adk can actually turn into a model content part. Its LiteLlm
+    # path (`_get_content`) decodes text/* inline, maps image//audio//video/ to
+    # the matching data-uri part and application/pdf + application/json to a file
+    # part, then raises ValueError on anything else -- and the runner turns that
+    # ValueError into a 500, losing the whole turn, the user's text included.
+    # Mirrored here because it is a private ADK detail; test_media_file_parts
+    # pins the two together against the installed version.
+    _MODEL_READABLE_MIME_PREFIXES = ("text/", "image/", "audio/", "video/")
+    _MODEL_READABLE_MIME_TYPES = frozenset({"application/pdf", "application/json"})
+
+    # Ceilings on what we inline, mirroring the bounds the bot-runtime already
+    # applies when downloading the attachment (ai_adapter.go: maxAttachmentBytes
+    # / maxAttachmentsTotalBytes). Providers reject an oversized inline request,
+    # and that rejection is a 500 here too.
+    MAX_INLINE_FILE_BYTES = 15 * 1024 * 1024
+    MAX_INLINE_REQUEST_BYTES = 20 * 1024 * 1024
+
+    def _inline_skip_reason(
+        self, content_type: str, size_bytes: int, already_inlined: int
+    ) -> Optional[str]:
+        """Why this file must not travel to the model as inline data, or None.
+
+        Skipping is the graceful path: the file is still saved as an artifact and
+        the rest of the message -- text and readable media -- still gets an
+        answer. Forwarding it instead costs the whole turn.
+        """
+        # "audio/webm;codecs=opus" -> "audio/webm". The Blob keeps the content
+        # type verbatim; only the check normalizes.
+        mime_type = (content_type or "").split(";")[0].strip().lower()
+        if not (
+            mime_type.startswith(self._MODEL_READABLE_MIME_PREFIXES)
+            or mime_type in self._MODEL_READABLE_MIME_TYPES
+        ):
+            return "the model layer cannot carry this content type as inline data"
+
+        if size_bytes > self.MAX_INLINE_FILE_BYTES:
+            return (
+                f"{size_bytes} bytes is over the {self.MAX_INLINE_FILE_BYTES} byte"
+                " per-file inline limit"
+            )
+
+        if already_inlined + size_bytes > self.MAX_INLINE_REQUEST_BYTES:
+            return (
+                "it would push this request past the"
+                f" {self.MAX_INLINE_REQUEST_BYTES} byte inline budget"
+            )
+
+        return None
+
     def _is_audio_file(self, content_type: str, filename: str) -> bool:
         """Check if file is an audio file based on content type and extension."""
         if not content_type and not filename:

tests/unit/test_media_file_parts.py

@@ -0,0 +1,189 @@
+"""EVO-2181: every incoming file the model can read must reach the model.
+
+process_files used to append the file part to `file_parts` only `if is_audio`, so
+images were blobbed + saved to artifacts but never sent to the LLM, and
+create_content("", file_parts) returned None -> "No content to process".
+
+The other half of the contract: what the model layer *cannot* carry must stay out
+of the content parts. google-adk's LiteLlm raises ValueError on a mime type it
+does not know, the runner turns that into a 500, and the user loses the whole
+turn -- their text included. A plain WhatsApp document (docx/zip) takes exactly
+that path, and a caller that omits `mimeType` gets application/octet-stream from
+a2a_routes.extract_files_from_message.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import base64
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+from google.adk.models.lite_llm import _get_content
+
+from src.schemas.chat import FileData
+from src.services.adk.runners.runner_utils import RunnerUtils
+
+DOCX = "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
+
+
+def _utils():
+    # Bypass __init__ (which builds AgentBuilder(db)); the methods under test use
+    # neither self.db nor self.agent_builder.
+    return RunnerUtils.__new__(RunnerUtils)
+
+
+def _artifacts():
+    a = MagicMock()
+    a.save_artifact = AsyncMock()
+    return a
+
+
+def _file(name, ctype):
+    return FileData(
+        filename=name, content_type=ctype, data=base64.b64encode(b"bytes-" + name.encode()).decode()
+    )
+
+
+def _run(coro):
+    return asyncio.run(coro)
+
+
+def test_image_is_appended_to_file_parts():
+    parts, transcribed = _run(
+        _utils().process_files([_file("photo.png", "image/png")], _artifacts(), "agent", "ext", "sess")
+    )
+    assert len(parts) == 1
+    assert parts[0].inline_data.mime_type == "image/png"
+    assert transcribed == []
+
+
+def test_audio_still_appended():
+    parts, _ = _run(
+        _utils().process_files([_file("voice.ogg", "audio/ogg")], _artifacts(), "a", "e", "s")
+    )
+    assert len(parts) == 1
+    assert parts[0].inline_data.mime_type == "audio/ogg"
+
+
+def test_image_and_audio_both_appended():
+    parts, _ = _run(
+        _utils().process_files(
+            [_file("p.png", "image/png"), _file("v.ogg", "audio/ogg")], _artifacts(), "a", "e", "s"
+        )
+    )
+    assert len(parts) == 2
+
+
+def test_create_content_with_image_only_is_not_none():
+    utils = _utils()
+    parts, _ = _run(utils.process_files([_file("photo.png", "image/png")], _artifacts(), "a", "e", "s"))
+    content = utils.create_content("", parts)
+    assert content is not None  # regression guard for "No content to process"
+    assert content.role == "user"
+    assert len(content.parts) == 2  # empty text part + the image file part
+
+
+def test_create_content_empty_is_none():
+    assert _utils().create_content("", []) is None
+
+
+@pytest.mark.parametrize("content_type", [DOCX, "application/zip", "application/octet-stream", ""])
+def test_unreadable_file_stays_out_of_the_content_parts(content_type):
+    artifacts = _artifacts()
+    parts, _ = _run(
+        _utils().process_files([_file("file.bin", content_type)], artifacts, "a", "e", "s")
+    )
+    assert parts == []
+    artifacts.save_artifact.assert_awaited_once()  # still kept for reference
+
+
+def test_unreadable_file_never_costs_the_text_reply():
+    # The regression this guards: forwarding the docx raises ValueError inside
+    # LiteLlm, the runner answers 500 and the caption goes unanswered.
+    utils = _utils()
+    parts, _ = _run(utils.process_files([_file("planilha.docx", DOCX)], _artifacts(), "a", "e", "s"))
+    content = utils.create_content("segue o documento", parts)
+    assert content is not None
+    assert content.parts[0].text == "segue o documento"
+    assert all(p.inline_data is None for p in content.parts)
+
+
+def test_unreadable_file_does_not_drop_the_image_next_to_it():
+    parts, _ = _run(
+        _utils().process_files(
+            [_file("a.zip", "application/zip"), _file("p.png", "image/png")], _artifacts(), "a", "e", "s"
+        )
+    )
+    assert [p.inline_data.mime_type for p in parts] == ["image/png"]
+
+
+def test_pdf_and_text_still_reach_the_model():
+    parts, _ = _run(
+        _utils().process_files(
+            [_file("r.pdf", "application/pdf"), _file("n.txt", "text/plain")], _artifacts(), "a", "e", "s"
+        )
+    )
+    assert [p.inline_data.mime_type for p in parts] == ["application/pdf", "text/plain"]
+
+
+def test_mime_parameters_do_not_break_the_check():
+    parts, _ = _run(
+        _utils().process_files([_file("v.webm", "audio/webm;codecs=opus")], _artifacts(), "a", "e", "s")
+    )
+    # Normalized for the check, verbatim on the Blob.
+    assert [p.inline_data.mime_type for p in parts] == ["audio/webm;codecs=opus"]
+
+
+def test_image_survives_an_artifact_store_failure():
+    artifacts = MagicMock()
+    artifacts.save_artifact = AsyncMock(side_effect=RuntimeError("artifact store down"))
+    parts, _ = _run(_utils().process_files([_file("p.png", "image/png")], artifacts, "a", "e", "s"))
+    assert [p.inline_data.mime_type for p in parts] == ["image/png"]
+
+
+def test_oversized_file_is_not_inlined(monkeypatch):
+    monkeypatch.setattr(RunnerUtils, "MAX_INLINE_FILE_BYTES", 4)
+    artifacts = _artifacts()
+    parts, _ = _run(_utils().process_files([_file("big.png", "image/png")], artifacts, "a", "e", "s"))
+    assert parts == []
+    artifacts.save_artifact.assert_awaited_once()
+
+
+def test_inline_budget_is_per_request(monkeypatch):
+    # first.png decodes to 15 bytes, second.png to 16 -> only the first fits.
+    monkeypatch.setattr(RunnerUtils, "MAX_INLINE_REQUEST_BYTES", 20)
+    parts, _ = _run(
+        _utils().process_files(
+            [_file("first.png", "image/png"), _file("second.png", "image/png")], _artifacts(), "a", "e", "s"
+        )
+    )
+    assert len(parts) == 1
+
+
+def test_every_forwarded_part_is_accepted_by_the_installed_adk():
+    """Pins the allowlist to what google-adk actually converts.
+
+    `_get_content` is what raises the ValueError the runner turns into a 500. If
+    an ADK bump narrows the accepted set, this fails here instead of in front of
+    a customer.
+    """
+    utils = _utils()
+    samples = [
+        "image/png",
+        "image/jpeg",
+        "image/webp",
+        "audio/ogg",
+        "audio/mpeg",
+        "video/mp4",
+        "text/plain",
+        "application/pdf",
+        "application/json",
+    ]
+    parts, _ = _run(
+        utils.process_files(
+            [_file(f"f{i}.bin", ctype) for i, ctype in enumerate(samples)], _artifacts(), "a", "e", "s"
+        )
+    )
+    assert len(parts) == len(samples)
+    _get_content(utils.create_content("", parts).parts)  # must not raise