From 2cf737dc0508a7753d067ed8425cfc0ef657b29f Mon Sep 17 00:00:00 2001 From: majdyz Date: Mon, 13 Apr 2026 06:43:57 +0000 Subject: [PATCH] fix(backend): address review comments on cross-user prompt caching PR - Add TODO(#12747) to _SystemPromptPreset for cleanup tracking - Update docstring to note SDK version and migration path - Add debug logging in _build_system_prompt_value for observability - Document empty-string edge case in docstring - Trim redundant block comment at call site to single line - Add test for empty-string system_prompt with cache enabled - Add test for CHAT_CLAUDE_AGENT_CROSS_USER_PROMPT_CACHE=false env var --- .../backend/backend/copilot/sdk/service.py | 28 ++++++++++--------- .../backend/copilot/sdk/service_test.py | 21 ++++++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/autogpt_platform/backend/backend/copilot/sdk/service.py b/autogpt_platform/backend/backend/copilot/sdk/service.py index 3db3bd4104..c09b2d5230 100644 --- a/autogpt_platform/backend/backend/copilot/sdk/service.py +++ b/autogpt_platform/backend/backend/copilot/sdk/service.py @@ -18,19 +18,20 @@ from typing import TYPE_CHECKING, Any, Literal, NamedTuple, TypedDict, cast from typing_extensions import NotRequired +# TODO(#12747): Remove this local TypedDict and import SystemPromptPreset +# from claude_agent_sdk.types once SDK >=0.1.58 is the minimum pin. class _SystemPromptPreset(TypedDict): - """Local stand-in for the SDK's SystemPromptPreset. + """Local stand-in for the SDK's ``SystemPromptPreset`` (mirrors SDK >=0.1.58). - The production SDK (>=0.1.58) defines this type natively. We keep a - local copy so that: - 1. The code type-checks on older SDK pins (e.g. 0.1.45 on dev). - 2. Tests can import and verify the shape without an SDK upgrade. + Kept only for backwards compat with older SDK pins (e.g. 0.1.45 on dev). + Once the minimum SDK version is pinned to >=0.1.58, replace this with a + direct import from ``claude_agent_sdk.types``. """ type: Literal["preset"] preset: Literal["claude_code"] append: NotRequired[str] - exclude_dynamic_sections: NotRequired[bool] # SDK >=0.1.58 + exclude_dynamic_sections: NotRequired[bool] if TYPE_CHECKING: @@ -727,15 +728,21 @@ def _build_system_prompt_value( dict so the Claude Code default prompt becomes a cacheable prefix shared across all users; our custom *system_prompt* is appended after it. - When disabled, the raw *system_prompt* string is returned unchanged. + When disabled (or if the SDK is too old to support ``SystemPromptPreset``), + the raw *system_prompt* string is returned unchanged. + + An empty *system_prompt* is accepted: the preset dict will have + ``append: ""`` which the SDK treats as no custom suffix. """ if cross_user_cache: + logger.debug("Using SystemPromptPreset for cross-user prompt cache") return _SystemPromptPreset( type="preset", preset="claude_code", append=system_prompt, exclude_dynamic_sections=True, ) + logger.debug("Cross-user prompt cache disabled, using raw string") return system_prompt @@ -2260,12 +2267,7 @@ async def stream_chat_completion_sdk( sid, ) - # When cross-user prompt caching is enabled, use SystemPromptPreset - # so the Claude Code default prompt is a cacheable prefix shared - # across all users. Our custom prompt is appended after it and - # dynamic sections (working dir, git status, auto-memory) are - # excluded from the prefix — giving us cross-user cache hits that - # reduce input token cost by ~90%. + # Use SystemPromptPreset for cross-user prompt caching (see _build_system_prompt_value). system_prompt_value = _build_system_prompt_value( system_prompt, cross_user_cache=config.claude_agent_cross_user_prompt_cache, diff --git a/autogpt_platform/backend/backend/copilot/sdk/service_test.py b/autogpt_platform/backend/backend/copilot/sdk/service_test.py index b3e27ed4db..caa3d1b597 100644 --- a/autogpt_platform/backend/backend/copilot/sdk/service_test.py +++ b/autogpt_platform/backend/backend/copilot/sdk/service_test.py @@ -689,6 +689,16 @@ class TestSystemPromptPreset: assert isinstance(result, str) assert result == custom_prompt + def test_empty_string_with_cache_enabled(self): + """Empty system_prompt with cross_user_cache=True produces append=''.""" + result = _build_system_prompt_value("", cross_user_cache=True) + + assert isinstance(result, dict) + assert result["type"] == "preset" + assert result["preset"] == "claude_code" + assert result["append"] == "" + assert result["exclude_dynamic_sections"] is True + def test_default_config_is_enabled(self, _clean_config_env): """The default value for claude_agent_cross_user_prompt_cache is True.""" cfg = cfg_mod.ChatConfig( @@ -698,3 +708,14 @@ class TestSystemPromptPreset: use_claude_code_subscription=False, ) assert cfg.claude_agent_cross_user_prompt_cache is True + + def test_env_var_disables_cache(self, _clean_config_env, monkeypatch): + """CHAT_CLAUDE_AGENT_CROSS_USER_PROMPT_CACHE=false disables caching.""" + monkeypatch.setenv("CHAT_CLAUDE_AGENT_CROSS_USER_PROMPT_CACHE", "false") + cfg = cfg_mod.ChatConfig( + use_openrouter=False, + api_key=None, + base_url=None, + use_claude_code_subscription=False, + ) + assert cfg.claude_agent_cross_user_prompt_cache is False