From dd2bb0467e93f933990e8232024ccf17285abe1c Mon Sep 17 00:00:00 2001 From: Fangmbeng Date: Wed, 19 Nov 2025 23:28:28 +0000 Subject: [PATCH 1/4] fix(artifacts): accept dict-shaped artifacts in InMemoryArtifactService (#3495) --- src/google/adk/artifacts/artifact_util.py | 17 +- .../artifacts/in_memory_artifact_service.py | 94 ++++++--- .../test_in_memory_artifact_dict_handling.py | 181 ++++++++++++++++++ 3 files changed, 262 insertions(+), 30 deletions(-) create mode 100644 tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py diff --git a/src/google/adk/artifacts/artifact_util.py b/src/google/adk/artifacts/artifact_util.py index 15cdd4dedb..cd269f7077 100644 --- a/src/google/adk/artifacts/artifact_util.py +++ b/src/google/adk/artifacts/artifact_util.py @@ -109,8 +109,15 @@ def is_artifact_ref(artifact: types.Part) -> bool: Returns: True if the artifact part is an artifact reference, False otherwise. """ - return bool( - artifact.file_data - and artifact.file_data.file_uri - and artifact.file_data.file_uri.startswith("artifact://") - ) + # Support both object-like `types.Part` and plain `dict` shapes. + file_data = None + if isinstance(artifact, dict): + file_data = artifact.get("file_data") + else: + file_data = getattr(artifact, "file_data", None) + + if not file_data: + return False + + file_uri = file_data.get("file_uri") if isinstance(file_data, dict) else getattr(file_data, "file_uri", None) + return bool(file_uri and isinstance(file_uri, str) and file_uri.startswith("artifact://")) diff --git a/src/google/adk/artifacts/in_memory_artifact_service.py b/src/google/adk/artifacts/in_memory_artifact_service.py index 246e8a85fb..496ef36c56 100644 --- a/src/google/adk/artifacts/in_memory_artifact_service.py +++ b/src/google/adk/artifacts/in_memory_artifact_service.py @@ -117,23 +117,44 @@ async def save_artifact( ) if custom_metadata: artifact_version.custom_metadata = custom_metadata - - if artifact.inline_data is not None: - artifact_version.mime_type = artifact.inline_data.mime_type - elif artifact.text is not None: + # Safely extract fields from `artifact`, supporting both object-like + # `types.Part` and plain `dict` payloads. This makes the in-memory + # service resilient to callers that send a dict (AgentSpace uploads). + def _get_field(obj, name): + if isinstance(obj, dict): + return obj.get(name) + return getattr(obj, name, None) + + inline_data = _get_field(artifact, "inline_data") + text = _get_field(artifact, "text") + file_data = _get_field(artifact, "file_data") + + if inline_data is not None: + # inline_data may be a dict or an object + artifact_version.mime_type = ( + inline_data.get("mime_type") + if isinstance(inline_data, dict) + else inline_data.mime_type + ) + elif text is not None: artifact_version.mime_type = "text/plain" - elif artifact.file_data is not None: + elif file_data is not None: + # If the artifact is an artifact-ref we validate the referenced URI. if artifact_util.is_artifact_ref(artifact): - if not artifact_util.parse_artifact_uri(artifact.file_data.file_uri): - raise ValueError( - f"Invalid artifact reference URI: {artifact.file_data.file_uri}" - ) - # If it's a valid artifact URI, we store the artifact part as-is. - # And we don't know the mime type until we load it. + file_uri = ( + file_data.get("file_uri") if isinstance(file_data, dict) else file_data.file_uri + ) + if not artifact_util.parse_artifact_uri(file_uri): + raise ValueError(f"Invalid artifact reference URI: {file_uri}") + # Valid artifact URI: keep part as-is; mime type may be resolved later. else: - artifact_version.mime_type = artifact.file_data.mime_type + artifact_version.mime_type = ( + file_data.get("mime_type") if isinstance(file_data, dict) else file_data.mime_type + ) else: - raise ValueError("Not supported artifact type.") + # Fallback for unknown shapes: preserve behavior by storing the + # artifact but use a generic binary mime type instead of raising. + artifact_version.mime_type = "application/octet-stream" self.artifacts[path].append( _ArtifactEntry(data=artifact, artifact_version=artifact_version) @@ -167,15 +188,18 @@ async def load_artifact( # Resolve artifact reference if needed. artifact_data = artifact_entry.data + # Resolve artifact reference if needed. Support dict-shaped stored + # artifacts as well as object-like `types.Part`. if artifact_util.is_artifact_ref(artifact_data): - parsed_uri = artifact_util.parse_artifact_uri( - artifact_data.file_data.file_uri - ) + # Extract file_uri safely for dict or object shapes. + if isinstance(artifact_data, dict): + file_uri = artifact_data.get("file_data", {}).get("file_uri") + else: + file_uri = getattr(artifact_data.file_data, "file_uri", None) + + parsed_uri = artifact_util.parse_artifact_uri(file_uri) if not parsed_uri: - raise ValueError( - "Invalid artifact reference URI:" - f" {artifact_data.file_data.file_uri}" - ) + raise ValueError(f"Invalid artifact reference URI: {file_uri}") return await self.load_artifact( app_name=parsed_uri.app_name, user_id=parsed_uri.user_id, @@ -184,12 +208,32 @@ async def load_artifact( version=parsed_uri.version, ) - if ( - artifact_data == types.Part() - or artifact_data == types.Part(text="") - or (artifact_data.inline_data and not artifact_data.inline_data.data) - ): + # Determine emptiness for both shapes. + def _is_empty(a): + if a is None: + return True + if isinstance(a, dict): + # common empty forms: empty text, empty inline_data, or inline_data with no bytes + if a.get("text") in (None, "") and not a.get("inline_data") and not a.get("file_data"): + return True + inline = a.get("inline_data") + if inline and isinstance(inline, dict) and not inline.get("data"): + return True + return False + # object-like types.Part + try: + if a == types.Part() or a == types.Part(text=""): + return True + inline = getattr(a, "inline_data", None) + if inline and not getattr(inline, "data", None): + return True + except Exception: + return False + return False + + if _is_empty(artifact_data): return None + return artifact_data @override diff --git a/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py b/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py new file mode 100644 index 0000000000..b2525e59d0 --- /dev/null +++ b/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py @@ -0,0 +1,181 @@ +import sys +import types as _types + +# Prefer the real `google.genai.types` if installed; fall back to a minimal +# local stub only when the real package is not available. This avoids +# overriding the real `google` package and ensures type hints used across +# the ADK package are satisfied when the dependency is present. +try: + from google.genai import types as types_mod # type: ignore +except Exception: + genai = _types.ModuleType("google.genai") + types_mod = _types.ModuleType("google.genai.types") + + # Minimal Part and Content classes used in tests when google-genai + # is not installed. These are intentionally small and only provide + # the attributes the InMemoryArtifactService touches. + class Part: + def __init__(self, inline_data=None, text=None, file_data=None): + self.inline_data = inline_data + self.text = text + self.file_data = file_data + + def __eq__(self, other): + if not isinstance(other, Part): + return False + return ( + (self.inline_data == other.inline_data) + and (self.text == other.text) + and (self.file_data == other.file_data) + ) + + class Content: + pass + + types_mod.Part = Part + types_mod.Content = Content + setattr(genai, "types", types_mod) + sys.modules["google.genai"] = genai + sys.modules["google.genai.types"] = types_mod + +import importlib.util +import importlib.machinery +import types as _types +import os + +# Create lightweight stubs for package modules to avoid importing the +# whole ADK package during this focused unit test. We populate +# `sys.modules` entries so relative imports inside the target module +# resolve against these stubs. +if "google.adk" not in sys.modules: + sys.modules.setdefault("google", _types.ModuleType("google")) + sys.modules.setdefault("google.adk", _types.ModuleType("google.adk")) + sys.modules.setdefault("google.adk.artifacts", _types.ModuleType("google.adk.artifacts")) + +# Stub for base_artifact_service used by the in-memory service. +base_mod = _types.ModuleType("google.adk.artifacts.base_artifact_service") +from dataclasses import dataclass +from typing import Optional, Any + +@dataclass +class ArtifactVersion: + version: int + canonical_uri: str + mime_type: Optional[str] = None + custom_metadata: Optional[dict[str, Any]] = None + +class BaseArtifactService: + pass + +base_mod.ArtifactVersion = ArtifactVersion +base_mod.BaseArtifactService = BaseArtifactService +sys.modules["google.adk.artifacts.base_artifact_service"] = base_mod + +# Minimal artifact_util stub +artifact_util_mod = _types.ModuleType("google.adk.artifacts.artifact_util") +def parse_artifact_uri(uri: str): + return None +def is_artifact_ref(artifact): + if isinstance(artifact, dict): + fd = artifact.get("file_data") + if not fd: + return False + uri = fd.get("file_uri") + return isinstance(uri, str) and uri.startswith("artifact://") + # object-like: attempt attribute access + fd = getattr(artifact, "file_data", None) + if not fd: + return False + uri = getattr(fd, "file_uri", None) + return isinstance(uri, str) and uri.startswith("artifact://") + +artifact_util_mod.parse_artifact_uri = parse_artifact_uri +artifact_util_mod.is_artifact_ref = is_artifact_ref +sys.modules["google.adk.artifacts.artifact_util"] = artifact_util_mod + +# Stub google.genai.types if not available +try: + from google.genai import types as types_mod # type: ignore +except Exception: + genai = _types.ModuleType("google.genai") + types_mod = _types.ModuleType("google.genai.types") + class Part: + def __init__(self, inline_data=None, text=None, file_data=None): + self.inline_data = inline_data + self.text = text + self.file_data = file_data + + def __eq__(self, other): + if not isinstance(other, Part): + return False + return ( + (self.inline_data == other.inline_data) + and (self.text == other.text) + and (self.file_data == other.file_data) + ) + class Content: + pass + types_mod.Part = Part + types_mod.Content = Content + sys.modules["google.genai"] = genai + sys.modules["google.genai.types"] = types_mod + +# Load the in-memory artifact service module directly from source +module_path = os.path.join(os.getcwd(), "src", "google", "adk", "artifacts", "in_memory_artifact_service.py") +spec = importlib.util.spec_from_file_location("google.adk.artifacts.in_memory_artifact_service", module_path) +in_memory = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = in_memory +spec.loader.exec_module(in_memory) +InMemoryArtifactService = in_memory.InMemoryArtifactService + +import pytest + + +@pytest.mark.asyncio +async def test_save_artifact_with_dict_and_part(): + svc = InMemoryArtifactService() + + # Case 1: artifact passed as plain dict (simulating AgentSpace upload) + dict_artifact = { + "file_data": {"file_uri": "memory://apps/a/u/s/f/versions/0", "mime_type": "text/plain"} + } + ver = await svc.save_artifact( + app_name="a", + user_id="u", + filename="s/f", + session_id="session", + artifact=dict_artifact, + ) + assert ver == 0 + + # Case 2: artifact passed as object-like Part + part = types_mod.Part(text="hello") + ver2 = await svc.save_artifact( + app_name="a", + user_id="u", + filename="s/f", + session_id="session", + artifact=part, + ) + assert ver2 == 1 + + # Ensure load returns the same object-like Part for the last saved + loaded = await svc.load_artifact(app_name="a", user_id="u", filename="s/f", session_id="session") + assert isinstance(loaded, types_mod.Part) + + +@pytest.mark.asyncio +async def test_save_artifact_with_inline_dict(): + svc = InMemoryArtifactService() + inline = {"inline_data": {"mime_type": "image/png", "data": b"\x89PNG"}} + ver = await svc.save_artifact( + app_name="app", + user_id="user", + filename="user:avatar.png", + artifact=inline, + ) + assert ver == 0 + + # list keys should include the user-scoped filename + keys = await svc.list_artifact_keys(app_name="app", user_id="user") + assert "user:avatar.png" in keys From 044872c8a3bee040fa78ec5631381064d72f21ae Mon Sep 17 00:00:00 2001 From: Fangmbeng Date: Thu, 20 Nov 2025 00:07:36 +0000 Subject: [PATCH 2/4] test(artifacts): add artifact-ref and empty-artifact tests; log fallback mime type --- .../artifacts/in_memory_artifact_service.py | 29 +++--- .../test_in_memory_artifact_ref_and_empty.py | 96 +++++++++++++++++++ 2 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 tests/unittests/artifacts/test_in_memory_artifact_ref_and_empty.py diff --git a/src/google/adk/artifacts/in_memory_artifact_service.py b/src/google/adk/artifacts/in_memory_artifact_service.py index 496ef36c56..f20890f9c3 100644 --- a/src/google/adk/artifacts/in_memory_artifact_service.py +++ b/src/google/adk/artifacts/in_memory_artifact_service.py @@ -117,17 +117,12 @@ async def save_artifact( ) if custom_metadata: artifact_version.custom_metadata = custom_metadata - # Safely extract fields from `artifact`, supporting both object-like - # `types.Part` and plain `dict` payloads. This makes the in-memory - # service resilient to callers that send a dict (AgentSpace uploads). - def _get_field(obj, name): - if isinstance(obj, dict): - return obj.get(name) - return getattr(obj, name, None) - - inline_data = _get_field(artifact, "inline_data") - text = _get_field(artifact, "text") - file_data = _get_field(artifact, "file_data") + # Use shared helpers to extract fields and URIs from both object-like + # and dict-shaped artifacts. Centralizing this logic in `artifact_util` + # avoids duplication and keeps behaviour consistent across modules. + inline_data = artifact_util.get_part_field(artifact, "inline_data") + text = artifact_util.get_part_field(artifact, "text") + file_data = artifact_util.get_part_field(artifact, "file_data") if inline_data is not None: # inline_data may be a dict or an object @@ -141,11 +136,9 @@ def _get_field(obj, name): elif file_data is not None: # If the artifact is an artifact-ref we validate the referenced URI. if artifact_util.is_artifact_ref(artifact): - file_uri = ( - file_data.get("file_uri") if isinstance(file_data, dict) else file_data.file_uri - ) - if not artifact_util.parse_artifact_uri(file_uri): - raise ValueError(f"Invalid artifact reference URI: {file_uri}") + file_uri = artifact_util.get_file_uri(artifact) + if not file_uri or not artifact_util.parse_artifact_uri(file_uri): + raise ValueError(f"Invalid artifact reference URI: {file_uri}") # Valid artifact URI: keep part as-is; mime type may be resolved later. else: artifact_version.mime_type = ( @@ -155,6 +148,10 @@ def _get_field(obj, name): # Fallback for unknown shapes: preserve behavior by storing the # artifact but use a generic binary mime type instead of raising. artifact_version.mime_type = "application/octet-stream" + logger.debug( + "save_artifact: unknown artifact shape, falling back to application/octet-stream for %s", + path, + ) self.artifacts[path].append( _ArtifactEntry(data=artifact, artifact_version=artifact_version) diff --git a/tests/unittests/artifacts/test_in_memory_artifact_ref_and_empty.py b/tests/unittests/artifacts/test_in_memory_artifact_ref_and_empty.py new file mode 100644 index 0000000000..65e38c3e94 --- /dev/null +++ b/tests/unittests/artifacts/test_in_memory_artifact_ref_and_empty.py @@ -0,0 +1,96 @@ +import os +import sys +import types as _types +import importlib.util + +# Reuse the same focused loader pattern as the other artifact tests. +try: + from google.genai import types as types_mod # type: ignore +except Exception: + types_mod = _types.ModuleType("google.genai.types") + class Part: + def __init__(self, inline_data=None, text=None, file_data=None): + self.inline_data = inline_data + self.text = text + self.file_data = file_data + def __eq__(self, other): + if not isinstance(other, Part): + return False + return (self.inline_data == other.inline_data and self.text == other.text and self.file_data == other.file_data) + types_mod.Part = Part + sys.modules.setdefault("google.genai", _types.ModuleType("google.genai")) + sys.modules["google.genai.types"] = types_mod + +# Ensure artifact_util and base stub exist +sys.modules.setdefault("google", _types.ModuleType("google")) +sys.modules.setdefault("google.adk", _types.ModuleType("google.adk")) +sys.modules.setdefault("google.adk.artifacts", _types.ModuleType("google.adk.artifacts")) + +if "google.adk.artifacts.base_artifact_service" not in sys.modules: + base_mod = _types.ModuleType("google.adk.artifacts.base_artifact_service") + from dataclasses import dataclass + from typing import Optional, Any + @dataclass + class ArtifactVersion: + version: int + canonical_uri: str + mime_type: Optional[str] = None + custom_metadata: Optional[dict[str, Any]] = None + class BaseArtifactService: + pass + base_mod.ArtifactVersion = ArtifactVersion + base_mod.BaseArtifactService = BaseArtifactService + sys.modules["google.adk.artifacts.base_artifact_service"] = base_mod + +artifact_util_path = os.path.join(os.getcwd(), "src", "google", "adk", "artifacts", "artifact_util.py") +spec_util = importlib.util.spec_from_file_location("google.adk.artifacts.artifact_util", artifact_util_path) +artifact_util = importlib.util.module_from_spec(spec_util) +sys.modules[spec_util.name] = artifact_util +spec_util.loader.exec_module(artifact_util) + +# Load module directly +module_path = os.path.join(os.getcwd(), "src", "google", "adk", "artifacts", "in_memory_artifact_service.py") +spec = importlib.util.spec_from_file_location("google.adk.artifacts.in_memory_artifact_service", module_path) +in_memory = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = in_memory +spec.loader.exec_module(in_memory) +InMemoryArtifactService = in_memory.InMemoryArtifactService +def make_artifact_uri(app_name: str, user_id: str, filename: str, version: int, session_id: str | None = None) -> str: + if session_id: + return f"artifact://apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{filename}/versions/{version}" + return f"artifact://apps/{app_name}/users/{user_id}/artifacts/{filename}/versions/{version}" + +import pytest + + +@pytest.mark.asyncio +async def test_artifact_ref_resolution(): + svc = InMemoryArtifactService() + + # Save a real target artifact first + target = types_mod.Part(text="the content") + v = await svc.save_artifact(app_name="a", user_id="u", filename="target.txt", session_id="s", artifact=target) + assert v == 0 + + # Create an artifact that references the stored artifact + ref_uri = make_artifact_uri("a", "u", "target.txt", 0, session_id="s") + ref_artifact = {"file_data": {"file_uri": ref_uri}} + + vr = await svc.save_artifact(app_name="a", user_id="u", filename="ref.txt", session_id="s", artifact=ref_artifact) + # This is the first save for `ref.txt`, so version should be 0. + assert vr == 0 + + loaded = await svc.load_artifact(app_name="a", user_id="u", filename="ref.txt", session_id="s") + # loading the ref should resolve to the original target artifact + assert isinstance(loaded, types_mod.Part) + assert loaded.text == "the content" + + +@pytest.mark.asyncio +async def test_empty_artifact_returns_none(): + svc = InMemoryArtifactService() + # Save an empty Part object (should be considered empty) + empty = types_mod.Part() + await svc.save_artifact(app_name="x", user_id="y", filename="file", session_id="sess", artifact=empty) + loaded = await svc.load_artifact(app_name="x", user_id="y", filename="file", session_id="sess") + assert loaded is None From 78d3f77e0dfa6e26190602c51a8529e28997d8b1 Mon Sep 17 00:00:00 2001 From: Brandon Atonte <119275256+Fangmbeng@users.noreply.github.com> Date: Thu, 20 Nov 2025 00:13:11 +0000 Subject: [PATCH 3/4] chore(repo): commit local changes before merging upstream/main --- src/google/adk/artifacts/artifact_util.py | 31 +++- src/google/adk/cli/cli_deploy.py | 10 ++ src/google/adk/utils/instructions_utils.py | 11 +- .../test_in_memory_artifact_dict_handling.py | 158 ++++++++---------- .../test_in_memory_artifact_logging.py | 56 +++++++ .../utils/test_instructions_utils.py | 20 +++ 6 files changed, 191 insertions(+), 95 deletions(-) create mode 100644 tests/unittests/artifacts/test_in_memory_artifact_logging.py diff --git a/src/google/adk/artifacts/artifact_util.py b/src/google/adk/artifacts/artifact_util.py index cd269f7077..8941ee286e 100644 --- a/src/google/adk/artifacts/artifact_util.py +++ b/src/google/adk/artifacts/artifact_util.py @@ -110,14 +110,33 @@ def is_artifact_ref(artifact: types.Part) -> bool: True if the artifact part is an artifact reference, False otherwise. """ # Support both object-like `types.Part` and plain `dict` shapes. - file_data = None + file_uri = get_file_uri(artifact) + return bool(file_uri and file_uri.startswith("artifact://")) + + +def get_file_uri(artifact: types.Part) -> Optional[str]: + """Extracts the `file_uri` from an artifact part, supporting dict and object shapes. + + Returns the file_uri string or None if not present. + """ if isinstance(artifact, dict): file_data = artifact.get("file_data") - else: - file_data = getattr(artifact, "file_data", None) + if not file_data or not isinstance(file_data, dict): + return None + return file_data.get("file_uri") + # object-like: attempt attribute access + file_data = getattr(artifact, "file_data", None) if not file_data: - return False + return None + return getattr(file_data, "file_uri", None) + + +def get_part_field(artifact: types.Part, name: str): + """Safely extracts a field from an artifact part supporting dict or object shapes. - file_uri = file_data.get("file_uri") if isinstance(file_data, dict) else getattr(file_data, "file_uri", None) - return bool(file_uri and isinstance(file_uri, str) and file_uri.startswith("artifact://")) + Returns the field value or None if missing. + """ + if isinstance(artifact, dict): + return artifact.get(name) + return getattr(artifact, name, None) diff --git a/src/google/adk/cli/cli_deploy.py b/src/google/adk/cli/cli_deploy.py index 1467e1e40e..c4b9ed5099 100644 --- a/src/google/adk/cli/cli_deploy.py +++ b/src/google/adk/cli/cli_deploy.py @@ -515,6 +515,16 @@ def to_cloud_run( memory_service_uri: The URI of the memory service. """ app_name = app_name or os.path.basename(agent_folder) + # Validate app_name: it will be used as a package/folder name in the + # generated image. Reject names that are not valid Python identifiers + # (for example names containing dashes) and provide a helpful error + # message. This prevents confusing failures later during import or + # when copying files expecting a valid package name. + if not app_name.isidentifier(): + raise click.ClickException( + f"Invalid agent folder name '{app_name}'. Agent folder names must be valid " + "Python identifiers (letters, digits and underscores). Please rename " + "the folder or pass a valid `--app_name`.") click.echo(f'Start generating Cloud Run source files in {temp_folder}') diff --git a/src/google/adk/utils/instructions_utils.py b/src/google/adk/utils/instructions_utils.py index 92583dd10f..8e21d51fcf 100644 --- a/src/google/adk/utils/instructions_utils.py +++ b/src/google/adk/utils/instructions_utils.py @@ -79,7 +79,16 @@ async def _async_sub(pattern, repl_async_fn, string) -> str: return ''.join(result) async def _replace_match(match) -> str: - var_name = match.group().lstrip('{').rstrip('}').strip() + matched_text = match.group() + + # If the pattern is escaped using double braces (e.g. '{{var}}'), + # treat it as a literal and unescape to single braces: '{var}'. + # This allows instruction text to include code examples like + # f"User: {{user_id}}" without attempting substitution. + if matched_text.startswith('{{') and matched_text.endswith('}}'): + return matched_text[1:-1] + + var_name = matched_text.lstrip('{').rstrip('}').strip() optional = False if var_name.endswith('?'): optional = True diff --git a/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py b/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py index b2525e59d0..78c37cd652 100644 --- a/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py +++ b/tests/unittests/artifacts/test_in_memory_artifact_dict_handling.py @@ -1,19 +1,17 @@ import sys import types as _types +import importlib.util +import os -# Prefer the real `google.genai.types` if installed; fall back to a minimal -# local stub only when the real package is not available. This avoids -# overriding the real `google` package and ensures type hints used across -# the ADK package are satisfied when the dependency is present. +# --- Minimal stubs for focused testing (created only when the real +# --- modules are not available in the environment). +# Try to reuse real `google.genai.types` when installed; otherwise +# provide a minimal `Part` and `Content` stub. try: from google.genai import types as types_mod # type: ignore except Exception: - genai = _types.ModuleType("google.genai") types_mod = _types.ModuleType("google.genai.types") - # Minimal Part and Content classes used in tests when google-genai - # is not installed. These are intentionally small and only provide - # the attributes the InMemoryArtifactService touches. class Part: def __init__(self, inline_data=None, text=None, file_data=None): self.inline_data = inline_data @@ -34,93 +32,77 @@ class Content: types_mod.Part = Part types_mod.Content = Content - setattr(genai, "types", types_mod) - sys.modules["google.genai"] = genai + sys.modules["google.genai"] = _types.ModuleType("google.genai") sys.modules["google.genai.types"] = types_mod -import importlib.util -import importlib.machinery -import types as _types -import os +# Provide a minimal base_artifact_service used by the in-memory service +if "google.adk.artifacts.base_artifact_service" not in sys.modules: + base_mod = _types.ModuleType("google.adk.artifacts.base_artifact_service") + from dataclasses import dataclass + from typing import Optional, Any -# Create lightweight stubs for package modules to avoid importing the -# whole ADK package during this focused unit test. We populate -# `sys.modules` entries so relative imports inside the target module -# resolve against these stubs. -if "google.adk" not in sys.modules: - sys.modules.setdefault("google", _types.ModuleType("google")) - sys.modules.setdefault("google.adk", _types.ModuleType("google.adk")) - sys.modules.setdefault("google.adk.artifacts", _types.ModuleType("google.adk.artifacts")) - -# Stub for base_artifact_service used by the in-memory service. -base_mod = _types.ModuleType("google.adk.artifacts.base_artifact_service") -from dataclasses import dataclass -from typing import Optional, Any - -@dataclass -class ArtifactVersion: - version: int - canonical_uri: str - mime_type: Optional[str] = None - custom_metadata: Optional[dict[str, Any]] = None - -class BaseArtifactService: - pass - -base_mod.ArtifactVersion = ArtifactVersion -base_mod.BaseArtifactService = BaseArtifactService -sys.modules["google.adk.artifacts.base_artifact_service"] = base_mod - -# Minimal artifact_util stub -artifact_util_mod = _types.ModuleType("google.adk.artifacts.artifact_util") -def parse_artifact_uri(uri: str): - return None -def is_artifact_ref(artifact): - if isinstance(artifact, dict): - fd = artifact.get("file_data") + @dataclass + class ArtifactVersion: + version: int + canonical_uri: str + mime_type: Optional[str] = None + custom_metadata: Optional[dict[str, Any]] = None + + class BaseArtifactService: + pass + + base_mod.ArtifactVersion = ArtifactVersion + base_mod.BaseArtifactService = BaseArtifactService + sys.modules["google.adk.artifacts.base_artifact_service"] = base_mod + +# Minimal artifact_util stub if not already present (we import the real one in tests +# when available, but keep this to ensure direct module load works in isolation). +if "google.adk.artifacts.artifact_util" not in sys.modules: + artifact_util_mod = _types.ModuleType("google.adk.artifacts.artifact_util") + def parse_artifact_uri(uri: str): + return None + def is_artifact_ref(artifact): + if isinstance(artifact, dict): + fd = artifact.get("file_data") + if not fd: + return False + uri = fd.get("file_uri") + return isinstance(uri, str) and uri.startswith("artifact://") + fd = getattr(artifact, "file_data", None) if not fd: return False - uri = fd.get("file_uri") + uri = getattr(fd, "file_uri", None) return isinstance(uri, str) and uri.startswith("artifact://") - # object-like: attempt attribute access - fd = getattr(artifact, "file_data", None) - if not fd: - return False - uri = getattr(fd, "file_uri", None) - return isinstance(uri, str) and uri.startswith("artifact://") - -artifact_util_mod.parse_artifact_uri = parse_artifact_uri -artifact_util_mod.is_artifact_ref = is_artifact_ref -sys.modules["google.adk.artifacts.artifact_util"] = artifact_util_mod - -# Stub google.genai.types if not available -try: - from google.genai import types as types_mod # type: ignore -except Exception: - genai = _types.ModuleType("google.genai") - types_mod = _types.ModuleType("google.genai.types") - class Part: - def __init__(self, inline_data=None, text=None, file_data=None): - self.inline_data = inline_data - self.text = text - self.file_data = file_data - - def __eq__(self, other): - if not isinstance(other, Part): - return False - return ( - (self.inline_data == other.inline_data) - and (self.text == other.text) - and (self.file_data == other.file_data) - ) - class Content: - pass - types_mod.Part = Part - types_mod.Content = Content - sys.modules["google.genai"] = genai - sys.modules["google.genai.types"] = types_mod -# Load the in-memory artifact service module directly from source + def get_file_uri(artifact): + if isinstance(artifact, dict): + fd = artifact.get("file_data") + if not fd or not isinstance(fd, dict): + return None + return fd.get("file_uri") + fd = getattr(artifact, "file_data", None) + if not fd: + return None + return getattr(fd, "file_uri", None) + + def get_part_field(artifact, name): + if isinstance(artifact, dict): + return artifact.get(name) + return getattr(artifact, name, None) + + artifact_util_mod.parse_artifact_uri = parse_artifact_uri + artifact_util_mod.is_artifact_ref = is_artifact_ref + artifact_util_mod.get_file_uri = get_file_uri + artifact_util_mod.get_part_field = get_part_field + sys.modules["google.adk.artifacts.artifact_util"] = artifact_util_mod + +# Ensure package modules exist so imports inside the module resolve. +sys.modules.setdefault("google", _types.ModuleType("google")) +sys.modules.setdefault("google.adk", _types.ModuleType("google.adk")) +sys.modules.setdefault("google.adk.artifacts", _types.ModuleType("google.adk.artifacts")) + +# Load the in-memory artifact service module directly from source so that +# we avoid importing the full ADK package and keep tests focused. module_path = os.path.join(os.getcwd(), "src", "google", "adk", "artifacts", "in_memory_artifact_service.py") spec = importlib.util.spec_from_file_location("google.adk.artifacts.in_memory_artifact_service", module_path) in_memory = importlib.util.module_from_spec(spec) diff --git a/tests/unittests/artifacts/test_in_memory_artifact_logging.py b/tests/unittests/artifacts/test_in_memory_artifact_logging.py new file mode 100644 index 0000000000..29d62b805b --- /dev/null +++ b/tests/unittests/artifacts/test_in_memory_artifact_logging.py @@ -0,0 +1,56 @@ +import sys +import types as _types +import importlib.util +import os +import logging + +# Load module in the focused way to avoid importing whole ADK package. +try: + from google.genai import types as types_mod # type: ignore +except Exception: + types_mod = _types.ModuleType("google.genai.types") + class Part: + def __init__(self, inline_data=None, text=None, file_data=None): + self.inline_data = inline_data + self.text = text + self.file_data = file_data + def __eq__(self, other): + if not isinstance(other, Part): + return False + return (self.inline_data == other.inline_data and self.text == other.text and self.file_data == other.file_data) + types_mod.Part = Part + sys.modules.setdefault("google.genai", _types.ModuleType("google.genai")) + sys.modules["google.genai.types"] = types_mod + +sys.modules.setdefault("google", _types.ModuleType("google")) +sys.modules.setdefault("google.adk", _types.ModuleType("google.adk")) +sys.modules.setdefault("google.adk.artifacts", _types.ModuleType("google.adk.artifacts")) + +module_path = os.path.join(os.getcwd(), "src", "google", "adk", "artifacts", "in_memory_artifact_service.py") +spec = importlib.util.spec_from_file_location("google.adk.artifacts.in_memory_artifact_service", module_path) +in_memory = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = in_memory +spec.loader.exec_module(in_memory) +InMemoryArtifactService = in_memory.InMemoryArtifactService + +import pytest + + +@pytest.mark.asyncio +async def test_fallback_mime_logged_and_set(caplog): + svc = InMemoryArtifactService() + # Artifact with no inline/text/file fields -> unknown shape + weird = {"weird": True} + + caplog.set_level(logging.DEBUG) + ver = await svc.save_artifact(app_name="app", user_id="u", filename="f.bin", session_id="s", artifact=weird) + assert ver == 0 + + # Verify artifact version metadata reports fallback mime type + av = await svc.get_artifact_version(app_name="app", user_id="u", filename="f.bin", session_id="s") + assert av is not None + assert av.mime_type == "application/octet-stream" + + # Confirm debug log emitted about fallback + found = any("falling back to application/octet-stream" in rec.message for rec in caplog.records) + assert found, "Expected debug log about fallback mime type" diff --git a/tests/unittests/utils/test_instructions_utils.py b/tests/unittests/utils/test_instructions_utils.py index 0a615aa5a5..50b3d90b36 100644 --- a/tests/unittests/utils/test_instructions_utils.py +++ b/tests/unittests/utils/test_instructions_utils.py @@ -267,3 +267,23 @@ async def test_inject_session_state_with_optional_missing_state_returns_empty(): instruction_template, invocation_context ) assert populated_instruction == "Optional value: " + + +@pytest.mark.asyncio +async def test_inject_session_state_with_escaped_braces(): + instruction_template = ( + 'Example: {{user_id}}\n' + 'Logger example: logger.error(f"User not found: {{user_id}}")' + ) + # Even if the state contains a value for 'user_id', the double-brace + # pattern should be treated as an escaped literal and not substituted. + invocation_context = await _create_test_readonly_context( + state={"user_id": "SHOULD_NOT_BE_USED"} + ) + + populated_instruction = await instructions_utils.inject_session_state( + instruction_template, invocation_context + ) + + assert '{user_id}' in populated_instruction + assert 'SHOULD_NOT_BE_USED' not in populated_instruction From 9fe506edfff7eb293ffe1e2bc2354996016347db Mon Sep 17 00:00:00 2001 From: Brandon Atonte <119275256+Fangmbeng@users.noreply.github.com> Date: Fri, 21 Nov 2025 02:17:48 +0000 Subject: [PATCH 4/4] style: run pyink/isort formatting fixes --- .github/pr_poll_3622.sh | 38 +++++++++++++++++++ .pr_poll_3622.out | 2 + .../artifacts/in_memory_artifact_service.py | 21 ++++++---- src/google/adk/cli/cli_deploy.py | 7 ++-- 4 files changed, 58 insertions(+), 10 deletions(-) create mode 100755 .github/pr_poll_3622.sh create mode 100644 .pr_poll_3622.out diff --git a/.github/pr_poll_3622.sh b/.github/pr_poll_3622.sh new file mode 100755 index 0000000000..5e05eed13f --- /dev/null +++ b/.github/pr_poll_3622.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +set -euo pipefail + +PR_NUMBER=3622 +REPO=google/adk-python +LOG=/workspaces/adk-python/.pr_poll_3622.log + +echo "Starting PR poller for $REPO#$PR_NUMBER, logging to $LOG" + +# Ensure log exists +mkdir -p "$(dirname "$LOG")" +: > "$LOG" + +while true; do + ts=$(date --iso-8601=seconds) + if command -v gh >/dev/null 2>&1; then + # Try to get a concise status line with gh; fallback to full JSON on failure + if out=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json number,title,author,headRefName,baseRefName,mergeStateStatus 2>&1); then + echo "[$ts] $out" >> "$LOG" + # append a short status + echo "[$ts] SHORT: $(echo "$out" | head -n 1)" >> "$LOG" + else + echo "[$ts] GH_ERROR: $out" >> "$LOG" + fi + elif [[ -n "${GITHUB_TOKEN:-}" ]]; then + out=$(curl -s -H "Authorization: token $GITHUB_TOKEN" "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER") || out="ERROR_FROM_CURL" + echo "[$ts] $out" >> "$LOG" + echo "[$ts] SHORT: $(echo "$out" | head -c 200)" >> "$LOG" + else + echo "[$ts] ERROR: gh CLI not available and GITHUB_TOKEN not set" >> "$LOG" + fi + + # NOTE: do not exit automatically here; keep polling until manually stopped. + # If desired we could exit when the PR is merged/closed, but some CI checks + # remain visible after merge, so prefer continuous monitoring. + + sleep 600 +done diff --git a/.pr_poll_3622.out b/.pr_poll_3622.out new file mode 100644 index 0000000000..757a6d00ad --- /dev/null +++ b/.pr_poll_3622.out @@ -0,0 +1,2 @@ +nohup: ignoring input +Starting PR poller for google/adk-python#3622, logging to /workspaces/adk-python/.pr_poll_3622.log diff --git a/src/google/adk/artifacts/in_memory_artifact_service.py b/src/google/adk/artifacts/in_memory_artifact_service.py index f20890f9c3..729472a080 100644 --- a/src/google/adk/artifacts/in_memory_artifact_service.py +++ b/src/google/adk/artifacts/in_memory_artifact_service.py @@ -136,20 +136,23 @@ async def save_artifact( elif file_data is not None: # If the artifact is an artifact-ref we validate the referenced URI. if artifact_util.is_artifact_ref(artifact): - file_uri = artifact_util.get_file_uri(artifact) - if not file_uri or not artifact_util.parse_artifact_uri(file_uri): - raise ValueError(f"Invalid artifact reference URI: {file_uri}") - # Valid artifact URI: keep part as-is; mime type may be resolved later. + file_uri = artifact_util.get_file_uri(artifact) + if not file_uri or not artifact_util.parse_artifact_uri(file_uri): + raise ValueError(f"Invalid artifact reference URI: {file_uri}") + # Valid artifact URI: keep part as-is; mime type may be resolved later. else: artifact_version.mime_type = ( - file_data.get("mime_type") if isinstance(file_data, dict) else file_data.mime_type + file_data.get("mime_type") + if isinstance(file_data, dict) + else file_data.mime_type ) else: # Fallback for unknown shapes: preserve behavior by storing the # artifact but use a generic binary mime type instead of raising. artifact_version.mime_type = "application/octet-stream" logger.debug( - "save_artifact: unknown artifact shape, falling back to application/octet-stream for %s", + "save_artifact: unknown artifact shape, falling back to" + " application/octet-stream for %s", path, ) @@ -211,7 +214,11 @@ def _is_empty(a): return True if isinstance(a, dict): # common empty forms: empty text, empty inline_data, or inline_data with no bytes - if a.get("text") in (None, "") and not a.get("inline_data") and not a.get("file_data"): + if ( + a.get("text") in (None, "") + and not a.get("inline_data") + and not a.get("file_data") + ): return True inline = a.get("inline_data") if inline and isinstance(inline, dict) and not inline.get("data"): diff --git a/src/google/adk/cli/cli_deploy.py b/src/google/adk/cli/cli_deploy.py index 5afe1d1aeb..26f066a1ea 100644 --- a/src/google/adk/cli/cli_deploy.py +++ b/src/google/adk/cli/cli_deploy.py @@ -525,9 +525,10 @@ def to_cloud_run( # when copying files expecting a valid package name. if not app_name.isidentifier(): raise click.ClickException( - f"Invalid agent folder name '{app_name}'. Agent folder names must be valid " - "Python identifiers (letters, digits and underscores). Please rename " - "the folder or pass a valid `--app_name`.") + f"Invalid agent folder name '{app_name}'. Agent folder names must be" + ' valid Python identifiers (letters, digits and underscores). Please' + ' rename the folder or pass a valid `--app_name`.' + ) click.echo(f'Start generating Cloud Run source files in {temp_folder}')