From 2598c3063628f3f3386afe19fbe6eadf961775f5 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 12 Sep 2026 00:14:16 +0000 Subject: [PATCH 1/3] fix(sdk): dispatch titles through Responses and subscription streaming openhands+astra requested by @neubig Created by an AI agent (OpenHands) on behalf of @neubig. Co-authored-by: openhands --- .../openhands/sdk/conversation/title_utils.py | 10 +- tests/sdk/conversation/test_generate_title.py | 141 ++++++++++++++++++ 2 files changed, 147 insertions(+), 4 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/title_utils.py b/openhands-sdk/openhands/sdk/conversation/title_utils.py index 4e98dd015e..1182ec21c9 100644 --- a/openhands-sdk/openhands/sdk/conversation/title_utils.py +++ b/openhands-sdk/openhands/sdk/conversation/title_utils.py @@ -126,12 +126,14 @@ def generate_title_with_llm( ), ] - # Force non-streaming: the title is consumed whole with no on_token - # callback, which a streaming LLM requires. - if llm.stream: + if llm.stream and not llm.requires_streaming: llm = llm.model_copy(update={"stream": False}) - response = llm.completion(messages) + response = ( + llm.responses(messages, store=False) + if llm.uses_responses_api() + else llm.completion(messages) + ) # Extract the title from the response if response.message.content and isinstance( diff --git a/tests/sdk/conversation/test_generate_title.py b/tests/sdk/conversation/test_generate_title.py index 858404dd40..13b2ee8726 100644 --- a/tests/sdk/conversation/test_generate_title.py +++ b/tests/sdk/conversation/test_generate_title.py @@ -1,5 +1,9 @@ """Tests for the generate_title method in Conversation class.""" +import json +import threading +import time +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from unittest.mock import MagicMock, patch import pytest @@ -13,6 +17,8 @@ from openhands.sdk.conversation.title_utils import generate_title_with_llm from openhands.sdk.event.llm_convertible import MessageEvent from openhands.sdk.llm import LLM, LLMResponse, Message, MetricsSnapshot, TextContent +from openhands.sdk.llm.auth.credentials import CredentialStore, OAuthCredentials +from openhands.sdk.llm.auth.openai import OpenAISubscriptionAuth def create_test_agent() -> Agent: @@ -287,3 +293,138 @@ def test_generate_title_disables_streaming_when_llm_streams(mock_transport): assert mock_transport.call_args.kwargs["enable_streaming"] is False assert mock_transport.call_args.kwargs["on_token"] is None assert streaming_llm.stream is True + + +@pytest.fixture +def title_http_server(): + requests = [] + title = "Fix title transport" + + class Handler(BaseHTTPRequestHandler): + def do_POST(self): + body = json.loads(self.rfile.read(int(self.headers["Content-Length"]))) + requests.append((self.path, body)) + if self.path == "/v1/responses": + response = { + "id": "resp_title", + "object": "response", + "created_at": 1, + "status": "completed", + "model": "gpt-5.6-luna", + "output": [ + { + "id": "msg_title", + "type": "message", + "role": "assistant", + "status": "completed", + "content": [ + { + "type": "output_text", + "text": title, + "annotations": [], + } + ], + } + ], + "usage": { + "input_tokens": 10, + "output_tokens": 4, + "total_tokens": 14, + }, + } + if body.get("stream"): + payload = ( + "event: response.completed\ndata: " + + json.dumps( + { + "type": "response.completed", + "sequence_number": 1, + "response": response, + } + ) + + "\n\n" + ).encode() + content_type = "text/event-stream" + else: + payload = json.dumps(response).encode() + content_type = "application/json" + else: + payload = json.dumps( + { + "id": "chat_title", + "object": "chat.completion", + "created": 1, + "model": "gpt-4o-mini", + "choices": [ + { + "index": 0, + "finish_reason": "stop", + "message": {"role": "assistant", "content": title}, + } + ], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 4, + "total_tokens": 14, + }, + } + ).encode() + content_type = "application/json" + self.send_response(200) + self.send_header("Content-Type", content_type) + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, format, *args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}/v1", requests + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +@pytest.mark.parametrize("mode", ["chat", "responses", "subscription"]) +def test_title_uses_real_http_transport(title_http_server, tmp_path, mode): + base_url, requests = title_http_server + if mode == "subscription": + auth = OpenAISubscriptionAuth( + credential_store=CredentialStore(tmp_path / "auth") + ) + llm = auth.create_llm( + model="gpt-5.6-luna", + credentials=OAuthCredentials( + vendor="openai", + access_token="local-test-token", + refresh_token="unused-test-token", + expires_at=int(time.time() * 1000) + 3600000, + ), + ) + llm = llm.model_copy(update={"base_url": base_url, "num_retries": 0}) + else: + llm = LLM( + model="openai/gpt-4o-mini", + api_key=SecretStr("local-test-key"), + base_url=base_url, + api_mode=mode, + stream=True, + num_retries=0, + ) + errors = [] + assert ( + generate_title_with_llm("Fix the title", llm, on_error=errors.append) + == "Fix title transport" + ) + assert not errors + assert len(requests) == 1 + path, body = requests[0] + assert path == ("/v1/chat/completions" if mode == "chat" else "/v1/responses") + assert bool(body.get("stream")) == (mode == "subscription") + if mode != "chat": + assert body["store"] is False From bbe497d63f126f495c7dc80747270badd108a8d2 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 12 Sep 2026 12:05:06 +0000 Subject: [PATCH 2/3] refactor(sdk): centralize LLM API dispatch Created by an AI agent (OpenHands) on behalf of @neubig. Co-authored-by: openhands --- openhands-sdk/openhands/sdk/agent/utils.py | 52 ++--- .../openhands/sdk/conversation/title_utils.py | 9 +- openhands-sdk/openhands/sdk/llm/llm.py | 64 ++++++ tests/sdk/agent/test_agent_utils.py | 208 ++---------------- tests/sdk/llm/test_llm.py | 73 +++++- 5 files changed, 175 insertions(+), 231 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/utils.py b/openhands-sdk/openhands/sdk/agent/utils.py index 60e5acb315..62fefe6cfd 100644 --- a/openhands-sdk/openhands/sdk/agent/utils.py +++ b/openhands-sdk/openhands/sdk/agent/utils.py @@ -672,24 +672,14 @@ def make_llm_completion( Summary field is always added to tool schemas for transparency and explainability of agent actions. """ - if llm.uses_responses_api(): - return llm.responses( - messages=messages, - tools=tools or [], - include=None, - store=False, - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) - else: - return llm.completion( - messages=messages, - tools=tools or [], - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) + return llm.generate( + messages=messages, + tools=tools or [], + store=False, + add_security_risk_prediction=True, + on_token=on_token, + call_context=call_context, + ) # --------------------------------------------------------------------------- @@ -735,21 +725,11 @@ async def amake_llm_completion( call_context: LLMCallContext | None = None, ) -> LLMResponse: """Async variant of :func:`make_llm_completion`.""" - if llm.uses_responses_api(): - return await llm.aresponses( - messages=messages, - tools=tools or [], - include=None, - store=False, - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) - else: - return await llm.acompletion( - messages=messages, - tools=tools or [], - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) + return await llm.agenerate( + messages=messages, + tools=tools or [], + store=False, + add_security_risk_prediction=True, + on_token=on_token, + call_context=call_context, + ) diff --git a/openhands-sdk/openhands/sdk/conversation/title_utils.py b/openhands-sdk/openhands/sdk/conversation/title_utils.py index 1182ec21c9..7861400e54 100644 --- a/openhands-sdk/openhands/sdk/conversation/title_utils.py +++ b/openhands-sdk/openhands/sdk/conversation/title_utils.py @@ -126,14 +126,7 @@ def generate_title_with_llm( ), ] - if llm.stream and not llm.requires_streaming: - llm = llm.model_copy(update={"stream": False}) - - response = ( - llm.responses(messages, store=False) - if llm.uses_responses_api() - else llm.completion(messages) - ) + response = llm.generate(messages, store=False) # Extract the title from the response if response.message.content and isinstance( diff --git a/openhands-sdk/openhands/sdk/llm/llm.py b/openhands-sdk/openhands/sdk/llm/llm.py index 25bb92726e..3af7122e82 100644 --- a/openhands-sdk/openhands/sdk/llm/llm.py +++ b/openhands-sdk/openhands/sdk/llm/llm.py @@ -1494,6 +1494,70 @@ def _validate_chat_response( ) return resp + def generate( + self, + messages: list[Message], + tools: Sequence[ToolDefinition] | None = None, + include: list[str] | None = None, + store: bool | None = None, + add_security_risk_prediction: bool = False, + on_token: TokenCallbackType | None = None, + call_context: LLMCallContext | None = None, + **kwargs, + ) -> LLMResponse: + """Generate a response using the configured API mode.""" + if self.uses_responses_api(): + return self.responses( + messages=messages, + tools=tools, + include=include, + store=store, + add_security_risk_prediction=add_security_risk_prediction, + on_token=on_token, + call_context=call_context, + **kwargs, + ) + return self.completion( + messages=messages, + tools=tools, + add_security_risk_prediction=add_security_risk_prediction, + on_token=on_token, + call_context=call_context, + **kwargs, + ) + + async def agenerate( + self, + messages: list[Message], + tools: Sequence[ToolDefinition] | None = None, + include: list[str] | None = None, + store: bool | None = None, + add_security_risk_prediction: bool = False, + on_token: AnyTokenCallbackType | None = None, + call_context: LLMCallContext | None = None, + **kwargs, + ) -> LLMResponse: + """Async variant of :meth:`generate`.""" + if self.uses_responses_api(): + return await self.aresponses( + messages=messages, + tools=tools, + include=include, + store=store, + add_security_risk_prediction=add_security_risk_prediction, + on_token=on_token, + call_context=call_context, + **kwargs, + ) + return await self.acompletion( + messages=messages, + tools=tools, + add_security_risk_prediction=add_security_risk_prediction, + on_token=on_token, + call_context=call_context, + **kwargs, + ) + # ========================================================================= # Chat Completion API # ========================================================================= diff --git a/tests/sdk/agent/test_agent_utils.py b/tests/sdk/agent/test_agent_utils.py index a91c07ba90..b957f30c85 100644 --- a/tests/sdk/agent/test_agent_utils.py +++ b/tests/sdk/agent/test_agent_utils.py @@ -9,7 +9,11 @@ import pytest from pydantic import Field -from openhands.sdk.agent.utils import make_llm_completion, prepare_llm_messages +from openhands.sdk.agent.utils import ( + amake_llm_completion, + make_llm_completion, + prepare_llm_messages, +) from openhands.sdk.context.condenser.base import CondenserBase from openhands.sdk.context.view import View from openhands.sdk.event import Condensation, MessageEvent @@ -257,97 +261,18 @@ def counting_enforce(self, all_events): # --------------------------------------------------------------------------- -def test_make_llm_completion_with_completion_api(mock_llm, sample_messages): - """Test make_llm_completion using completion API.""" - # Setup mock - mock_llm.uses_responses_api.return_value = False - mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response - - # Call function - result = make_llm_completion(mock_llm, sample_messages) - - # Verify results - assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.completion.assert_called_once_with( - messages=sample_messages, - tools=[], - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - mock_llm.responses.assert_not_called() - - -def test_make_llm_completion_with_responses_api(mock_llm, sample_messages): - """Test make_llm_completion using responses API.""" - # Setup mock - mock_llm.uses_responses_api.return_value = True - mock_response = Mock(spec=LLMResponse) - mock_llm.responses.return_value = mock_response - - # Call function - result = make_llm_completion(mock_llm, sample_messages) - - # Verify results - assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.responses.assert_called_once_with( - messages=sample_messages, - tools=[], - include=None, - store=False, - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - mock_llm.completion.assert_not_called() - - -def test_make_llm_completion_with_tools_completion_api( +def test_make_llm_completion_applies_agent_policy( mock_llm, sample_messages, sample_tools ): - """Test make_llm_completion with tools using completion API.""" - # Setup mock - mock_llm.uses_responses_api.return_value = False mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response + mock_llm.generate.return_value = mock_response - # Call function result = make_llm_completion(mock_llm, sample_messages, tools=sample_tools) - # Verify results assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.completion.assert_called_once_with( + mock_llm.generate.assert_called_once_with( messages=sample_messages, tools=sample_tools, - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - - -def test_make_llm_completion_with_tools_responses_api( - mock_llm, sample_messages, sample_tools -): - """Test make_llm_completion with tools using responses API.""" - # Setup mock - mock_llm.uses_responses_api.return_value = True - mock_response = Mock(spec=LLMResponse) - mock_llm.responses.return_value = mock_response - - # Call function - result = make_llm_completion(mock_llm, sample_messages, tools=sample_tools) - - # Verify results - assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.responses.assert_called_once_with( - messages=sample_messages, - tools=sample_tools, - include=None, store=False, add_security_risk_prediction=True, on_token=None, @@ -355,63 +280,26 @@ def test_make_llm_completion_with_tools_responses_api( ) -def test_make_llm_completion_with_none_tools(mock_llm, sample_messages): - """Test make_llm_completion with None tools parameter.""" - # Setup mock - mock_llm.uses_responses_api.return_value = False - mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response +def test_make_llm_completion_normalizes_missing_tools(mock_llm, sample_messages): + make_llm_completion(mock_llm, sample_messages) - # Call function - result = make_llm_completion(mock_llm, sample_messages, tools=None) - - # Verify results - assert result == mock_response - mock_llm.completion.assert_called_once_with( - messages=sample_messages, - tools=[], - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) + assert mock_llm.generate.call_args.kwargs["tools"] == [] -def test_make_llm_completion_with_empty_tools_list(mock_llm, sample_messages): - """Test make_llm_completion with empty tools list.""" - # Setup mock - mock_llm.uses_responses_api.return_value = False +@pytest.mark.asyncio +async def test_amake_llm_completion_applies_agent_policy( + mock_llm, sample_messages, sample_tools +): mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response + mock_llm.agenerate.return_value = mock_response - # Call function - result = make_llm_completion(mock_llm, sample_messages, tools=[]) + result = await amake_llm_completion(mock_llm, sample_messages, tools=sample_tools) - # Verify results assert result == mock_response - mock_llm.completion.assert_called_once_with( + mock_llm.agenerate.assert_awaited_once_with( messages=sample_messages, - tools=[], - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - - -def test_make_llm_completion_empty_messages(mock_llm): - """Test make_llm_completion with empty messages list.""" - # Setup mock - mock_llm.uses_responses_api.return_value = False - mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response - - # Call function - result = make_llm_completion(mock_llm, []) - - # Verify results - assert result == mock_response - mock_llm.completion.assert_called_once_with( - messages=[], - tools=[], + tools=sample_tools, + store=False, add_security_risk_prediction=True, on_token=None, call_context=None, @@ -431,71 +319,19 @@ def test_prepare_llm_messages_and_make_llm_completion_integration( mock_events_to_messages.return_value = sample_messages view = View(events=sample_events) - # Setup mocks for make_llm_completion - mock_llm.uses_responses_api.return_value = False mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response + mock_llm.generate.return_value = mock_response - # Call functions in sequence (simulating real usage) messages = prepare_llm_messages(view) result = make_llm_completion(mock_llm, messages) - # Verify results assert messages == sample_messages assert result == mock_response - mock_llm.completion.assert_called_once_with( + mock_llm.generate.assert_called_once_with( messages=sample_messages, tools=[], - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - - -def test_make_llm_completion_api_selection(): - """Test that make_llm_completion correctly selects between completion and responses APIs.""" # noqa: E501 - # Test completion API selection - mock_llm = Mock(spec=LLM) - mock_llm.uses_responses_api.return_value = False - mock_response = Mock(spec=LLMResponse) - mock_llm.completion.return_value = mock_response - - messages = [ - Message( - role="user", - content=[TextContent(text="Hello, test message")], - ) - ] - - result = make_llm_completion(mock_llm, messages) - - assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.completion.assert_called_once_with( - messages=messages, - tools=[], - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - mock_llm.responses.assert_not_called() - - # Reset mocks and test responses API selection - mock_llm.reset_mock() - mock_llm.uses_responses_api.return_value = True - mock_llm.responses.return_value = mock_response - - result = make_llm_completion(mock_llm, messages) - - assert result == mock_response - mock_llm.uses_responses_api.assert_called_once() - mock_llm.responses.assert_called_once_with( - messages=messages, - tools=[], - include=None, store=False, add_security_risk_prediction=True, on_token=None, call_context=None, ) - mock_llm.completion.assert_not_called() diff --git a/tests/sdk/llm/test_llm.py b/tests/sdk/llm/test_llm.py index f0c805ed21..fba46fb80d 100644 --- a/tests/sdk/llm/test_llm.py +++ b/tests/sdk/llm/test_llm.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest from litellm.exceptions import ( @@ -44,6 +44,77 @@ def test_llm_init_with_default_config(default_llm): assert default_llm.metrics.model_name == "gpt-4o" +@pytest.mark.parametrize("api_mode", ["chat", "responses"]) +def test_generate_dispatches_to_configured_api(default_llm, api_mode): + llm = default_llm.model_copy(update={"api_mode": api_mode}) + messages = [Message(role="user", content=[TextContent(text="Hello")])] + response = Mock(spec=LLMResponse) + + with ( + patch.object(LLM, "completion", return_value=response) as completion, + patch.object(LLM, "responses", return_value=response) as responses, + ): + assert llm.generate(messages, store=False) is response + + if api_mode == "responses": + responses.assert_called_once_with( + messages=messages, + tools=None, + include=None, + store=False, + add_security_risk_prediction=False, + on_token=None, + call_context=None, + ) + completion.assert_not_called() + else: + completion.assert_called_once_with( + messages=messages, + tools=None, + add_security_risk_prediction=False, + on_token=None, + call_context=None, + ) + responses.assert_not_called() + + +@pytest.mark.parametrize("api_mode", ["chat", "responses"]) +@pytest.mark.asyncio +async def test_agenerate_dispatches_to_configured_api(default_llm, api_mode): + llm = default_llm.model_copy(update={"api_mode": api_mode}) + messages = [Message(role="user", content=[TextContent(text="Hello")])] + response = Mock(spec=LLMResponse) + + with ( + patch.object( + LLM, "acompletion", AsyncMock(return_value=response) + ) as completion, + patch.object(LLM, "aresponses", AsyncMock(return_value=response)) as responses, + ): + assert await llm.agenerate(messages, store=False) is response + + if api_mode == "responses": + responses.assert_awaited_once_with( + messages=messages, + tools=None, + include=None, + store=False, + add_security_risk_prediction=False, + on_token=None, + call_context=None, + ) + completion.assert_not_awaited() + else: + completion.assert_awaited_once_with( + messages=messages, + tools=None, + add_security_risk_prediction=False, + on_token=None, + call_context=None, + ) + responses.assert_not_awaited() + + @patch("openhands.sdk.llm.utils.model_info.httpx.get") def test_base_url_for_openhands_provider(mock_get): """Test that openhands/ remains public while transport uses the proxy.""" From 5bb59ccb3aaf33aa24d78f94bbab396119ca1c5b Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 12 Sep 2026 14:30:19 +0000 Subject: [PATCH 3/3] refactor(sdk): use generic LLM dispatch directly Remove agent completion wrappers, migrate auxiliary and agent callers to LLM.generate/agenerate, and preserve agent-only security policy at Agent.step/astep. Created by an AI agent (OpenHands) on behalf of @neubig. Co-authored-by: openhands --- .github/workflows/precommit.yml | 7 + .pre-commit-config.yaml | 2 +- .../openhands/agent_server/profiles_router.py | 2 +- openhands-sdk/openhands/sdk/agent/agent.py | 20 +-- .../openhands/sdk/agent/stream_context.py | 8 +- openhands-sdk/openhands/sdk/agent/utils.py | 69 +-------- .../condenser/llm_summarizing_condenser.py | 13 +- .../conversation/impl/local_conversation.py | 11 +- openhands-sdk/openhands/sdk/hooks/executor.py | 3 +- .../openhands/sdk/llm/cleanup_profile.py | 14 +- openhands-sdk/openhands/sdk/llm/llm.py | 2 +- .../openhands/sdk/llm/utils/telemetry.py | 34 ++--- .../sdk/tool/builtins/vision_inspect.py | 4 +- .../openhands/tools/ask_oracle/impl.py | 3 +- scripts/check_forbidden_dynamic_attributes.py | 81 ++++++++-- ...forbidden_dynamic_attributes_baseline.json | 25 --- .../agent_server/test_conversation_service.py | 14 +- tests/agent_server/test_profiles_router.py | 2 +- ...test_check_forbidden_dynamic_attributes.py | 79 ++++++++++ .../test_remote_conversation_live_server.py | 33 +++- .../agent/test_agent_step_responses_gating.py | 11 ++ tests/sdk/agent/test_agent_utils.py | 143 +----------------- .../test_message_during_streaming_arun.py | 2 +- tests/sdk/agent/test_stream_context.py | 4 +- .../test_llm_summarizing_condenser.py | 57 +++---- tests/sdk/llm/test_cleanup_profile.py | 2 +- tests/sdk/llm/test_llm_span_cost.py | 7 +- tests/sdk/llm/test_llm_telemetry.py | 6 +- tests/tools/ask_oracle/test_ask_oracle.py | 2 +- 29 files changed, 283 insertions(+), 377 deletions(-) diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml index fb39c4d52c..d15cd80eac 100644 --- a/.github/workflows/precommit.yml +++ b/.github/workflows/precommit.yml @@ -15,6 +15,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v7 + with: + fetch-depth: 0 - name: Set up Python uses: actions/setup-python@v6 @@ -27,5 +29,10 @@ jobs: - name: Install dependencies run: uv sync --frozen --group dev + - name: Ensure dynamic attribute baseline only shrinks + run: >- + uv run python scripts/check_forbidden_dynamic_attributes.py + --baseline-ref ${{ github.event.pull_request.base.sha || github.event.before }} + - name: Run pre-commit (all files) run: uv run pre-commit run --all-files --show-diff-on-failure diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fd0fd25264..ae455c5e56 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -39,7 +39,7 @@ repos: pass_filenames: true always_run: false - id: check-forbidden-dynamic-attributes - name: Forbid getattr and setattr in SDK + name: Forbid dynamic attribute access in SDK entry: uv run python scripts/check_forbidden_dynamic_attributes.py language: system files: ^openhands-sdk/.*\.py$ diff --git a/openhands-agent-server/openhands/agent_server/profiles_router.py b/openhands-agent-server/openhands/agent_server/profiles_router.py index 578fe0a044..3f90316bef 100644 --- a/openhands-agent-server/openhands/agent_server/profiles_router.py +++ b/openhands-agent-server/openhands/agent_server/profiles_router.py @@ -322,7 +322,7 @@ async def validate_profile( llm = await asyncio.to_thread(create_subscription_llm_from_config, llm) - # Mirror the runtime dispatch (see ``amake_llm_completion``) and stay + # Mirror the runtime dispatch (see ``LLM.agenerate``) and stay # async so provider I/O doesn't pin the FastAPI event loop. if llm.uses_responses_api(): await llm.aresponses(messages=messages, max_tokens=1) diff --git a/openhands-sdk/openhands/sdk/agent/agent.py b/openhands-sdk/openhands/sdk/agent/agent.py index d63e801cd0..2d4054faf4 100644 --- a/openhands-sdk/openhands/sdk/agent/agent.py +++ b/openhands-sdk/openhands/sdk/agent/agent.py @@ -20,10 +20,8 @@ ) from openhands.sdk.agent.stream_context import StreamContext from openhands.sdk.agent.utils import ( - amake_llm_completion, aprepare_llm_messages, fix_malformed_tool_arguments, - make_llm_completion, normalize_tool_call, parse_tool_call_arguments, prepare_llm_messages, @@ -536,7 +534,7 @@ def init_state( system_prompt=TextContent(text=self.static_system_message), # Tools are stored as ToolDefinition objects and converted to # OpenAI format with security_risk parameter during LLM completion. - # See make_llm_completion() in agent/utils.py for details. + # Agent calls always expose security risk prediction in tool schemas. tools=list(self.tools_map.values()), dynamic_context=TextContent(text=dynamic_context) if dynamic_context @@ -727,10 +725,11 @@ def _step( ) try: - llm_response = make_llm_completion( - self.llm, - _messages, + llm_response = self.llm.generate( + messages=_messages, tools=list(self.tools_map.values()), + store=False, + add_security_risk_prediction=True, on_token=stream.token_callback, call_context=call_context, ) @@ -844,7 +843,7 @@ async def astep( """Async variant of :meth:`step`. The LLM completion is performed asynchronously via - :func:`amake_llm_completion`. Tool dispatch uses + :meth:`LLM.agenerate`. Tool dispatch uses :meth:`_aexecute_actions` which runs each tool call in its own thread via :func:`asyncio.loop.run_in_executor` and schedules parallel calls with :func:`asyncio.gather`, keeping the event @@ -934,10 +933,11 @@ async def _astep( # and state snapshots aren't blocked for the whole response. No-op # unless the run loop holds the lock (e.g. direct astep() in tests). async with conversation._released_state_lock_during_io(): - llm_response = await amake_llm_completion( - self.llm, - _messages, + llm_response = await self.llm.agenerate( + messages=_messages, tools=list(self.tools_map.values()), + store=False, + add_security_risk_prediction=True, on_token=stream.token_callback, call_context=call_context, ) diff --git a/openhands-sdk/openhands/sdk/agent/stream_context.py b/openhands-sdk/openhands/sdk/agent/stream_context.py index 438a76d755..20f8b3669e 100644 --- a/openhands-sdk/openhands/sdk/agent/stream_context.py +++ b/openhands-sdk/openhands/sdk/agent/stream_context.py @@ -282,9 +282,11 @@ def _split_chunk( delta = choice.delta if delta is None: continue - # getattr, not attribute access: litellm *deletes* reasoning_content - # when the provider omits it, declared field or not. - reasoning = getattr(delta, "reasoning_content", None) + reasoning = ( + delta.reasoning_content + if "reasoning_content" in delta.model_fields_set + else None + ) if isinstance(reasoning, str) and reasoning: out.append(("reasoning", reasoning, chunk.id, choice.index)) if isinstance(delta.content, str) and delta.content: diff --git a/openhands-sdk/openhands/sdk/agent/utils.py b/openhands-sdk/openhands/sdk/agent/utils.py index 62fefe6cfd..c25ea9639b 100644 --- a/openhands-sdk/openhands/sdk/agent/utils.py +++ b/openhands-sdk/openhands/sdk/agent/utils.py @@ -12,7 +12,6 @@ import types from collections.abc import Collection from typing import ( - TYPE_CHECKING, Annotated, Any, Union, @@ -25,16 +24,9 @@ from openhands.sdk.context.condenser.base import CondenserBase from openhands.sdk.context.view import View -from openhands.sdk.conversation.types import ConversationTokenCallbackType from openhands.sdk.event.base import LLMConvertibleEvent from openhands.sdk.event.condenser import Condensation -from openhands.sdk.llm import LLM, LLMResponse, Message -from openhands.sdk.tool import ToolDefinition - - -if TYPE_CHECKING: - from openhands.sdk.llm.llm import LLMCallContext - from openhands.sdk.llm.streaming import AnyTokenCallbackType +from openhands.sdk.llm import LLM, Message # Regex matching raw ASCII control characters (U+0000–U+001F) that are @@ -641,47 +633,6 @@ def prepare_llm_messages( return messages -def make_llm_completion( - llm: LLM, - messages: list[Message], - tools: list[ToolDefinition] | None = None, - on_token: ConversationTokenCallbackType | None = None, - call_context: LLMCallContext | None = None, -) -> LLMResponse: - """Make an LLM completion call with the provided messages and tools. - - Args: - llm: The LLM instance to use for completion - messages: The messages to send to the LLM - tools: Optional list of tools to provide to the LLM - on_token: Optional callback for streaming token updates - call_context: Per-conversation context for cache/session affinity. - - Returns: - LLMResponse from the LLM completion call - - Note: - Always exposes a 'security_risk' parameter in tool schemas via - add_security_risk_prediction=True. This ensures the schema remains - consistent, even if the security analyzer is disabled. Validation of - this field happens dynamically at runtime depending on the analyzer - configured. This allows weaker models to omit risk field and bypass - validation requirements when analyzer is disabled. For detailed logic, - see `_extract_security_risk` method in agent.py. - - Summary field is always added to tool schemas for transparency and - explainability of agent actions. - """ - return llm.generate( - messages=messages, - tools=tools or [], - store=False, - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) - - # --------------------------------------------------------------------------- # Async variants # --------------------------------------------------------------------------- @@ -715,21 +666,3 @@ async def aprepare_llm_messages( messages.extend(additional_messages) return messages - - -async def amake_llm_completion( - llm: LLM, - messages: list[Message], - tools: list[ToolDefinition] | None = None, - on_token: AnyTokenCallbackType | None = None, - call_context: LLMCallContext | None = None, -) -> LLMResponse: - """Async variant of :func:`make_llm_completion`.""" - return await llm.agenerate( - messages=messages, - tools=tools or [], - store=False, - add_security_risk_prediction=True, - on_token=on_token, - call_context=call_context, - ) diff --git a/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py b/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py index d6d9ae62b3..9f9133b872 100644 --- a/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py +++ b/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py @@ -225,13 +225,8 @@ def _generate_condensation( # Do not pass extra_body explicitly. The LLM handles forwarding # litellm_extra_body only when it is non-empty. - from openhands.sdk.agent.utils import make_llm_completion - try: - llm_response = make_llm_completion( - llm=self.llm, - messages=messages, - ) + llm_response = self.llm.generate(messages=messages, store=False) except Exception as e: raise NoCondensationAvailableException( f"Summarization LLM call failed: {e}" @@ -423,13 +418,9 @@ async def _agenerate_condensation( ) messages = [Message(role="user", content=[TextContent(text=prompt)])] - from openhands.sdk.agent.utils import amake_llm_completion try: - llm_response = await amake_llm_completion( - llm=self.llm, - messages=messages, - ) + llm_response = await self.llm.agenerate(messages=messages, store=False) except Exception as e: raise NoCondensationAvailableException( f"Summarization LLM call failed: {e}" diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index a8cf8bccf5..aabb7d6f86 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -1626,8 +1626,7 @@ def _bind_conversation_context(self, llm: LLM) -> None: thread an explicit ``call_context`` through the completion call (e.g. the condenser's dedicated LLM) still get correct per- conversation state. The primary agent completion path threads - context explicitly via ``Agent.step()`` → ``make_llm_completion()`` - → ``llm.completion(call_context=...)``. + context explicitly via ``Agent.step()`` → ``llm.generate(call_context=...)``. See #3443 for background. """ @@ -2859,7 +2858,7 @@ def ask_agent(self, question: str) -> str: return agent_response # Import here to avoid circular imports - from openhands.sdk.agent.utils import make_llm_completion, prepare_llm_messages + from openhands.sdk.agent.utils import prepare_llm_messages template_dir = ( Path(__file__).parent.parent.parent / "context" / "prompts" / "templates" @@ -2895,8 +2894,10 @@ def ask_agent(self, question: str) -> str: self.llm_registry.add(question_llm) # Pass agent tools so LLM can understand tool_calls in conversation history - response = make_llm_completion( - question_llm, messages, tools=list(self.agent.tools_map.values()) + response = question_llm.generate( + messages=messages, + tools=list(self.agent.tools_map.values()), + store=False, ) message = response.message diff --git a/openhands-sdk/openhands/sdk/hooks/executor.py b/openhands-sdk/openhands/sdk/hooks/executor.py index d76ab2d7de..9a95ca1712 100644 --- a/openhands-sdk/openhands/sdk/hooks/executor.py +++ b/openhands-sdk/openhands/sdk/hooks/executor.py @@ -12,7 +12,6 @@ from pydantic import BaseModel -from openhands.sdk.agent.utils import make_llm_completion from openhands.sdk.conversation.visualizer import ConversationVisualizerBase from openhands.sdk.hooks.config import HookDefinition, HookType from openhands.sdk.hooks.types import HookDecision, HookEvent @@ -359,7 +358,7 @@ def _execute_prompt_hook( ] try: - response = make_llm_completion(hook_llm, messages) + response = hook_llm.generate(messages=messages, store=False) raw = "\n".join(content_to_str(response.message.content)) except Exception as e: logger.warning( diff --git a/openhands-sdk/openhands/sdk/llm/cleanup_profile.py b/openhands-sdk/openhands/sdk/llm/cleanup_profile.py index 5323040f1a..e0b2ef968a 100644 --- a/openhands-sdk/openhands/sdk/llm/cleanup_profile.py +++ b/openhands-sdk/openhands/sdk/llm/cleanup_profile.py @@ -127,12 +127,8 @@ def clean_outward_text(text: str, *, cipher: Cipher | None = None) -> str: if cleanup_llm is None: return text - # Imported lazily: ``agent.utils`` imports from ``openhands.sdk.llm``, so a - # module-level import here would create a circular import at package init. - from openhands.sdk.agent.utils import make_llm_completion - try: - response = make_llm_completion(cleanup_llm, _cleanup_messages(text)) + response = cleanup_llm.generate(messages=_cleanup_messages(text), store=False) except Exception as exc: logger.warning("Cleanup profile call failed; sending original text: %s", exc) return text @@ -153,12 +149,10 @@ async def aclean_outward_text(text: str, *, cipher: Cipher | None = None) -> str if cleanup_llm is None: return text - # Imported lazily: ``agent.utils`` imports from ``openhands.sdk.llm``, so a - # module-level import here would create a circular import at package init. - from openhands.sdk.agent.utils import amake_llm_completion - try: - response = await amake_llm_completion(cleanup_llm, _cleanup_messages(text)) + response = await cleanup_llm.agenerate( + messages=_cleanup_messages(text), store=False + ) except Exception as exc: logger.warning("Cleanup profile call failed; sending original text: %s", exc) return text diff --git a/openhands-sdk/openhands/sdk/llm/llm.py b/openhands-sdk/openhands/sdk/llm/llm.py index 3af7122e82..7bf9f3d543 100644 --- a/openhands-sdk/openhands/sdk/llm/llm.py +++ b/openhands-sdk/openhands/sdk/llm/llm.py @@ -202,7 +202,7 @@ class LLMCallContext: """Per-conversation state threaded through the completion call chain. The primary path threads this explicitly: - ``Agent.step()`` → ``make_llm_completion()`` → ``llm.completion(call_context=...)`` + ``Agent.step()`` → ``llm.generate(call_context=...)`` → ``select_chat_options(call_context=...)``. A fallback copy is also stored as a ``PrivateAttr`` on :class:`LLM` diff --git a/openhands-sdk/openhands/sdk/llm/utils/telemetry.py b/openhands-sdk/openhands/sdk/llm/utils/telemetry.py index 4586308acc..b9a39059fe 100644 --- a/openhands-sdk/openhands/sdk/llm/utils/telemetry.py +++ b/openhands-sdk/openhands/sdk/llm/utils/telemetry.py @@ -251,26 +251,20 @@ def _cache_buckets(usage: Usage | ResponseAPIUsage) -> tuple[int, int]: Single source of truth for both ``metrics`` and the span, so the trace and the app's cost cannot disagree about the buckets. """ - cache_read = 0 - p_details = getattr(usage, "prompt_tokens_details", None) or getattr( - usage, "input_tokens_details", None - ) - if p_details is not None: - cache_read = int(getattr(p_details, "cached_tokens", 0) or 0) - # Kimi-K2-thinking populates usage.cached_tokens instead. - if not cache_read: - cache_read = int(getattr(usage, "cached_tokens", 0) or 0) - if not cache_read: - cache_read = int(getattr(usage, "cache_read_input_tokens", 0) or 0) - - # litellm mirrors this onto a private attr; the public one and the - # details dict are both populated on some provider shapes only. - cache_write = int(getattr(usage, "_cache_creation_input_tokens", 0) or 0) - if not cache_write: - cache_write = int(getattr(usage, "cache_creation_input_tokens", 0) or 0) - if not cache_write and p_details is not None: - cache_write = int(getattr(p_details, "cache_creation_tokens", 0) or 0) - return cache_read, cache_write + if isinstance(usage, Usage): + details = usage.prompt_tokens_details + if details is None: + return 0, 0 + cache_write = ( + details.cache_creation_tokens + if "cache_creation_tokens" in details.model_fields_set + else 0 + ) + return int(details.cached_tokens or 0), int(cache_write or 0) + + details = usage.input_tokens_details + cache_read = details.cached_tokens if details is not None else 0 + return int(cache_read or 0), 0 # ---------- Observability span ---------- # These bracket one LLM call: ``on_request`` -> transport -> ``on_response`` diff --git a/openhands-sdk/openhands/sdk/tool/builtins/vision_inspect.py b/openhands-sdk/openhands/sdk/tool/builtins/vision_inspect.py index 09b5c00225..3b59bfe3b3 100644 --- a/openhands-sdk/openhands/sdk/tool/builtins/vision_inspect.py +++ b/openhands-sdk/openhands/sdk/tool/builtins/vision_inspect.py @@ -246,9 +246,7 @@ def __call__( ], ), ] - from openhands.sdk.agent.utils import make_llm_completion - - response = make_llm_completion(vision_llm, messages, tools=[]) + response = vision_llm.generate(messages=messages, store=False) answer = next( ( content.text diff --git a/openhands-tools/openhands/tools/ask_oracle/impl.py b/openhands-tools/openhands/tools/ask_oracle/impl.py index 83817541f5..39922cd292 100644 --- a/openhands-tools/openhands/tools/ask_oracle/impl.py +++ b/openhands-tools/openhands/tools/ask_oracle/impl.py @@ -2,7 +2,6 @@ from typing import TYPE_CHECKING -from openhands.sdk.agent.utils import make_llm_completion from openhands.sdk.llm import Message, TextContent from openhands.sdk.tool.tool import ToolExecutor from openhands.tools.ask_oracle.definition import ( @@ -96,7 +95,7 @@ def __call__( ] try: - llm_response = make_llm_completion(oracle_llm, messages) + llm_response = oracle_llm.generate(messages=messages, store=False) except Exception as exc: return AskOracleObservation.from_text( text=( diff --git a/scripts/check_forbidden_dynamic_attributes.py b/scripts/check_forbidden_dynamic_attributes.py index 4d2f3da426..8d22d0adea 100644 --- a/scripts/check_forbidden_dynamic_attributes.py +++ b/scripts/check_forbidden_dynamic_attributes.py @@ -1,8 +1,9 @@ """Reject new dynamic attribute access in SDK source files. -Existing ``getattr``/``setattr`` calls are recorded in a committed baseline so -the hook can be introduced before the cleanup work (tracked in #4903, #4904, -#4905) is finished. Only calls *not* present in the baseline are reported. +Existing ``getattr``/``setattr`` and ``obj.__dict__.get`` calls are recorded +in a committed baseline so the hook can be introduced before the cleanup work +(tracked in #4903, #4904, #4905) is finished. Only calls *not* present in the +baseline are reported. A violation is identified by its file path (relative to the repo root), the call name, and a hash of the full call source segment (not just the first @@ -25,15 +26,30 @@ import ast import hashlib import json +import subprocess import sys from collections import Counter from pathlib import Path FORBIDDEN = {"getattr", "setattr"} +DICT_GET = "__dict__.get" BASELINE_FILE = Path(__file__).with_name("forbidden_dynamic_attributes_baseline.json") +def _forbidden_call_name(node: ast.Call) -> str | None: + if isinstance(node.func, ast.Name) and node.func.id in FORBIDDEN: + return node.func.id + if ( + isinstance(node.func, ast.Attribute) + and node.func.attr == "get" + and isinstance(node.func.value, ast.Attribute) + and node.func.value.attr == "__dict__" + ): + return DICT_GET + return None + + def _segment_hash(source: str, node: ast.Call) -> str: """Hash the full source segment of *node*, not just its first line.""" segment = ast.get_source_segment(source, node) @@ -53,20 +69,43 @@ def violations(path: Path) -> list[tuple[int, str, str]]: tree = ast.parse(source, filename=str(path)) result: list[tuple[int, str, str]] = [] for node in ast.walk(tree): - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Name) - and node.func.id in FORBIDDEN - ): - result.append((node.lineno, node.func.id, _segment_hash(source, node))) + if not isinstance(node, ast.Call): + continue + name = _forbidden_call_name(node) + if name is not None: + result.append((node.lineno, name, _segment_hash(source, node))) return result -def _load_baseline() -> Counter[tuple[str, str, str]]: +Baseline = Counter[tuple[str, str, str]] + + +def _parse_baseline(content: str) -> Baseline: + data = json.loads(content) + return Counter((entry["file"], entry["name"], entry["hash"]) for entry in data) + + +def _load_baseline() -> Baseline: if not BASELINE_FILE.exists(): return Counter() - data = json.loads(BASELINE_FILE.read_text()) - return Counter((entry["file"], entry["name"], entry["hash"]) for entry in data) + return _parse_baseline(BASELINE_FILE.read_text()) + + +def _load_baseline_from_git(ref: str) -> Baseline: + root = Path(__file__).resolve().parent.parent + relative_path = BASELINE_FILE.resolve().relative_to(root) + result = subprocess.run( + ["git", "show", f"{ref}:{relative_path.as_posix()}"], + cwd=root, + check=True, + capture_output=True, + text=True, + ) + return _parse_baseline(result.stdout) + + +def _baseline_additions(reference: Baseline, current: Baseline) -> Baseline: + return current - reference def _write_baseline(entries: list[tuple[str, str, str]]) -> None: @@ -105,8 +144,26 @@ def main(argv: list[str]) -> int: action="store_true", help="Rewrite the baseline from the current violations and exit 0.", ) + parser.add_argument( + "--baseline-ref", + help="Reject baseline entries not present at this Git reference.", + ) args = parser.parse_args(argv) + if args.update_baseline and args.baseline_ref: + parser.error("--update-baseline cannot be combined with --baseline-ref") + + if args.baseline_ref: + baseline = _load_baseline() + additions = _baseline_additions( + _load_baseline_from_git(args.baseline_ref), baseline + ) + if additions: + for (file, name, _digest), count in sorted(additions.items()): + print(f"{file}: baseline adds {count} forbidden {name} allowance(s)") + print("error: the forbidden dynamic attributes baseline may only shrink") + return 1 + # When no paths are given (e.g. via pre-commit with pass_filenames: false), # auto-discover all SDK Python files so deletions are caught. paths = args.paths if args.paths else _discover_sdk_files() diff --git a/scripts/forbidden_dynamic_attributes_baseline.json b/scripts/forbidden_dynamic_attributes_baseline.json index 35f94dc7ec..a10ad3a8d0 100644 --- a/scripts/forbidden_dynamic_attributes_baseline.json +++ b/scripts/forbidden_dynamic_attributes_baseline.json @@ -384,11 +384,6 @@ "name": "getattr", "hash": "4195e43bdf920781d93f8b42a955b0be81191b07b749a8397be797185c9bd3d4" }, - { - "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", - "name": "getattr", - "hash": "47404691ad754f6d634e7663cdf0779890f4564efb5d7f832347c4761b5e5a11" - }, { "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", "name": "getattr", @@ -399,11 +394,6 @@ "name": "getattr", "hash": "69e5e28543d27d551b8fa6322d51917f54fed9aa61f6f2ee608476736ad6c027" }, - { - "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", - "name": "getattr", - "hash": "69e5e28543d27d551b8fa6322d51917f54fed9aa61f6f2ee608476736ad6c027" - }, { "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", "name": "getattr", @@ -434,31 +424,16 @@ "name": "getattr", "hash": "7e0a34e78ce4877def53523401778467dba57bac511e33b2618e7da6d422ea57" }, - { - "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", - "name": "getattr", - "hash": "87bd2727ce5eef3fdf2a169d38b23a4615bed1edfacccf917f4e60d92e7dc6f0" - }, { "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", "name": "getattr", "hash": "a2f271f61f01e70e183a017e7b67de2171beff8bfefb09e604baaf48bb1f480f" }, - { - "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", - "name": "getattr", - "hash": "b6ac688d986e964a9a8cee6d47617e207a02740d7ae0648fb966eab33a048dc8" - }, { "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", "name": "getattr", "hash": "cb8eb6e82d059fb861a0bcc3820067d85b5dabb9ab199cf81186ec57d4d14f19" }, - { - "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", - "name": "getattr", - "hash": "cce9dbfcf2e3c10a5b17bbe043532e50bf7cbb4e0f6372fe808869daed684e77" - }, { "file": "openhands-sdk/openhands/sdk/llm/utils/telemetry.py", "name": "getattr", diff --git a/tests/agent_server/test_conversation_service.py b/tests/agent_server/test_conversation_service.py index 95f027d714..bf1c29ef85 100644 --- a/tests/agent_server/test_conversation_service.py +++ b/tests/agent_server/test_conversation_service.py @@ -3426,7 +3426,7 @@ async def test_autotitle_integration_routes_through_profile_store( """End-to-end: profile on disk → LLMProfileStore.load → title LLM call. Exercises the real wiring from AutoTitleSubscriber through LLMProfileStore - to LLM.completion. Only the network boundary (LLM.completion) is mocked, + to LLM.generate. Only the generic dispatch boundary (LLM.generate) is mocked, so this catches regressions in profile loading, LLM passthrough, and the agent-server → SDK integration — the unit tests above only exercise AutoTitleSubscriber in isolation. @@ -3457,7 +3457,7 @@ async def test_autotitle_integration_routes_through_profile_store( calls: list[str] = [] - def fake_completion(self_llm, _messages, **_kwargs): + def fake_generate(self_llm, _messages, **_kwargs): calls.append(self_llm.usage_id) msg = LiteLLMMessage(content="✨ Generated", role="assistant") choice = Choices(finish_reason="stop", index=0, message=msg) @@ -3492,9 +3492,9 @@ def fake_completion(self_llm, _messages, **_kwargs): request.addfinalizer(reset_stores) with patch( - "openhands.sdk.llm.llm.LLM.completion", + "openhands.sdk.llm.llm.LLM.generate", autospec=True, - side_effect=fake_completion, + side_effect=fake_generate, ): subscriber = AutoTitleSubscriber(service=service) await subscriber(self._user_message_event("Fix the login bug")) @@ -3552,7 +3552,7 @@ async def test_autotitle_decrypts_cipher_encrypted_title_profile( seen_keys: list[str] = [] - def fake_completion(self_llm, _messages, **_kwargs): + def fake_generate(self_llm, _messages, **_kwargs): seen_keys.append( self_llm.api_key.get_secret_value() if self_llm.api_key else "" ) @@ -3586,9 +3586,9 @@ def fake_completion(self_llm, _messages, **_kwargs): request.addfinalizer(reset_stores) with patch( - "openhands.sdk.llm.llm.LLM.completion", + "openhands.sdk.llm.llm.LLM.generate", autospec=True, - side_effect=fake_completion, + side_effect=fake_generate, ): subscriber = AutoTitleSubscriber(service=service) await subscriber(self._user_message_event("Fix the login bug")) diff --git a/tests/agent_server/test_profiles_router.py b/tests/agent_server/test_profiles_router.py index 29ae961a89..9670fb3bd9 100644 --- a/tests/agent_server/test_profiles_router.py +++ b/tests/agent_server/test_profiles_router.py @@ -1770,7 +1770,7 @@ def test_validate_profile_responses_api(client): Regression: the endpoint must route through ``aresponses`` for profiles where ``uses_responses_api()`` is true, matching the runtime dispatch used - by real conversations (`amake_llm_completion`) — otherwise preflight would + by real conversations (`LLM.agenerate`) — otherwise preflight would validate a different path than the server actually calls. """ from unittest.mock import MagicMock diff --git a/tests/cross/test_check_forbidden_dynamic_attributes.py b/tests/cross/test_check_forbidden_dynamic_attributes.py index 2a382a2e44..5645f4e737 100644 --- a/tests/cross/test_check_forbidden_dynamic_attributes.py +++ b/tests/cross/test_check_forbidden_dynamic_attributes.py @@ -124,6 +124,20 @@ def test_setattr_also_detected(checker, tmp_path: Path): assert checker.main([str(f)]) == 1 +def test_dunder_dict_get_also_detected(checker, tmp_path: Path): + """Direct instance dictionary lookup is also forbidden.""" + f = _write_py(tmp_path / "dict_get.py", 'value = obj.__dict__.get("attr")\n') + + assert checker.main([str(f)]) == 1 + + +def test_regular_dict_get_is_allowed(checker, tmp_path: Path): + """Ordinary mapping access is not dynamic attribute access.""" + f = _write_py(tmp_path / "dict_get.py", 'value = data.get("key")\n') + + assert checker.main([str(f)]) == 0 + + def test_deleted_baselined_file_is_stale(checker, tmp_path: Path): """Deleting a baselined file must be caught even when checking other files. @@ -154,3 +168,68 @@ def test_multiline_arg_change_is_caught(checker, tmp_path: Path): # Change the argument on a subsequent line — same first line, different call. _write_py(f, "v = getattr(\n obj,\n 'other',\n)\n") assert checker.main([str(f)]) == 1 + + +def test_baseline_addition_is_rejected(checker): + """A baseline may never gain a new allowance.""" + reference = checker.Counter({("a.py", "getattr", "old"): 1}) + current = checker.Counter( + { + ("a.py", "getattr", "old"): 1, + ("b.py", "setattr", "new"): 1, + } + ) + + assert checker._baseline_additions(reference, current) == checker.Counter( + {("b.py", "setattr", "new"): 1} + ) + + +def test_baseline_removal_is_allowed(checker): + """Deleting an existing allowance preserves baseline monotonicity.""" + reference = checker.Counter( + { + ("a.py", "getattr", "old"): 1, + ("b.py", "setattr", "removed"): 1, + } + ) + current = checker.Counter({("a.py", "getattr", "old"): 1}) + + assert not checker._baseline_additions(reference, current) + + +def test_duplicate_baseline_addition_is_rejected(checker): + """Increasing an existing allowance's multiplicity is also an addition.""" + key = ("a.py", "getattr", "same") + + assert checker._baseline_additions( + checker.Counter({key: 1}), checker.Counter({key: 2}) + ) == checker.Counter({key: 1}) + + +def test_baseline_ref_option_rejects_addition(checker, tmp_path: Path, monkeypatch): + """The CLI guard fails before accepting an expanded baseline.""" + key = ("added.py", "getattr", "new") + checker.BASELINE_FILE.write_text( + json.dumps([{"file": key[0], "name": key[1], "hash": key[2]}]) + ) + monkeypatch.setattr( + checker, "_load_baseline_from_git", lambda _ref: checker.Counter() + ) + clean = _write_py(tmp_path / "clean.py", "x = 1\n") + + assert checker.main([str(clean), "--baseline-ref", "base-sha"]) == 1 + + +def test_baseline_ref_option_allows_removal(checker, tmp_path: Path, monkeypatch): + """The CLI guard permits a baseline that is a strict subset.""" + removed = ("removed.py", "getattr", "old") + checker.BASELINE_FILE.write_text("[]\n") + monkeypatch.setattr( + checker, + "_load_baseline_from_git", + lambda _ref: checker.Counter({removed: 1}), + ) + clean = _write_py(tmp_path / "clean.py", "x = 1\n") + + assert checker.main([str(clean), "--baseline-ref", "base-sha"]) == 0 diff --git a/tests/cross/test_remote_conversation_live_server.py b/tests/cross/test_remote_conversation_live_server.py index dc544100a8..f0b87be8af 100644 --- a/tests/cross/test_remote_conversation_live_server.py +++ b/tests/cross/test_remote_conversation_live_server.py @@ -1842,10 +1842,16 @@ def fake_completion_with_finish( from openhands.sdk.llm.message import Message from openhands.sdk.llm.utils.metrics import MetricsSnapshot - call_count["count"] += 1 + is_title_call = not tools + if not is_title_call: + call_count["count"] += 1 - # First call: return finish tool call (triggers PostToolUse and Stop hooks) - if call_count["count"] == 1: + if is_title_call: + litellm_msg = LiteLLMMessage.model_validate( + {"role": "assistant", "content": "Generated title"} + ) + # First agent call triggers PostToolUse and Stop hooks. + elif call_count["count"] == 1: litellm_msg = LiteLLMMessage.model_validate( { "role": "assistant", @@ -2060,9 +2066,15 @@ def fake_completion_with_finish( from openhands.sdk.llm.message import Message from openhands.sdk.llm.utils.metrics import MetricsSnapshot - call_count["count"] += 1 + is_title_call = not tools + if not is_title_call: + call_count["count"] += 1 - if call_count["count"] == 1: + if is_title_call: + litellm_msg = LiteLLMMessage.model_validate( + {"role": "assistant", "content": "Generated title"} + ) + elif call_count["count"] == 1: litellm_msg = LiteLLMMessage.model_validate( { "role": "assistant", @@ -2197,8 +2209,15 @@ def fake_completion( from openhands.sdk.llm.message import Message from openhands.sdk.llm.utils.metrics import MetricsSnapshot - call_count["count"] += 1 - if call_count["count"] == 1: + is_title_call = not tools + if not is_title_call: + call_count["count"] += 1 + + if is_title_call: + litellm_msg = LiteLLMMessage.model_validate( + {"role": "assistant", "content": "Generated title"} + ) + elif call_count["count"] == 1: litellm_msg = LiteLLMMessage.model_validate( { "role": "assistant", diff --git a/tests/sdk/agent/test_agent_step_responses_gating.py b/tests/sdk/agent/test_agent_step_responses_gating.py index 9aad10b353..fbe9d07eb0 100644 --- a/tests/sdk/agent/test_agent_step_responses_gating.py +++ b/tests/sdk/agent/test_agent_step_responses_gating.py @@ -13,6 +13,7 @@ class DummyLLM(LLM): _calls: list[str] = PrivateAttr(default_factory=list) + _call_kwargs: list[dict] = PrivateAttr(default_factory=list) _force_responses: bool = PrivateAttr(default=False) def __init__(self, *, model: str, force_responses: bool): @@ -25,6 +26,7 @@ def uses_responses_api(self) -> bool: # override gating # Minimal stubs; not actually invoking providers def completion(self, *, messages, tools=None, **kwargs) -> LLMResponse: # type: ignore[override] self._calls.append("completion") + self._call_kwargs.append(kwargs) # Return an assistant message with no tool calls to end the step return LLMResponse( message=Message(role="assistant", content=[]), @@ -39,6 +41,7 @@ def completion(self, *, messages, tools=None, **kwargs) -> LLMResponse: # type: def responses(self, *, messages, tools=None, **kwargs) -> LLMResponse: # type: ignore[override] self._calls.append("responses") + self._call_kwargs.append(kwargs) return LLMResponse( message=Message(role="assistant", content=[]), metrics=MetricsSnapshot( @@ -76,6 +79,14 @@ def on_event(e): agent.step(convo, on_event=on_event) assert llm._calls == [expected] + assert llm._call_kwargs == [ + { + "add_security_risk_prediction": True, + "on_token": None, + "call_context": convo.get_llm_call_context(), + **({"include": None, "store": False} if force_responses else {}), + } + ] assert any(isinstance(e, MessageEvent) for e in events) diff --git a/tests/sdk/agent/test_agent_utils.py b/tests/sdk/agent/test_agent_utils.py index b957f30c85..411213d453 100644 --- a/tests/sdk/agent/test_agent_utils.py +++ b/tests/sdk/agent/test_agent_utils.py @@ -1,24 +1,14 @@ -"""Tests for agent utility functions. - -This module tests the prepare_llm_messages and make_llm_completion utility -functions that are used by the agent for message preparation and LLM calls. -""" +"""Tests for agent message preparation utilities.""" from unittest.mock import Mock, patch import pytest -from pydantic import Field -from openhands.sdk.agent.utils import ( - amake_llm_completion, - make_llm_completion, - prepare_llm_messages, -) +from openhands.sdk.agent.utils import prepare_llm_messages from openhands.sdk.context.condenser.base import CondenserBase from openhands.sdk.context.view import View from openhands.sdk.event import Condensation, MessageEvent -from openhands.sdk.llm import LLM, LLMResponse, Message, TextContent -from openhands.sdk.tool import Action, Observation, ToolDefinition +from openhands.sdk.llm import Message, TextContent # --------------------------------------------------------------------------- @@ -26,14 +16,6 @@ # --------------------------------------------------------------------------- -@pytest.fixture -def mock_llm(): - """Create a mock LLM for testing.""" - llm = Mock(spec=LLM) - llm.uses_responses_api.return_value = False - return llm - - @pytest.fixture def sample_events(): """Create sample events for testing.""" @@ -87,44 +69,6 @@ def mock_condenser(): return Mock(spec=CondenserBase) -class MockAgentUtilsAction(Action): - """Mock action for agent utils testing.""" - - param1: str = Field(description="First parameter") - - -class MockAgentUtilsObservation(Observation): - """Mock observation for agent utils testing.""" - - result: str = Field(description="Result of the action") - - @property - def to_llm_content(self): - return [TextContent(text=self.result)] - - -class MockAgentUtilsTool( - ToolDefinition[MockAgentUtilsAction, MockAgentUtilsObservation] -): - """Mock tool definition for agent utils testing.""" - - @classmethod - def create(cls, conv_state=None, **params): - return [cls(**params)] - - -@pytest.fixture -def sample_tools(): - """Create sample tool definitions for testing.""" - return [ - MockAgentUtilsTool( - description="A test tool for agent utils", - action_type=MockAgentUtilsAction, - observation_type=MockAgentUtilsObservation, - ) - ] - - # --------------------------------------------------------------------------- # Tests for prepare_llm_messages # --------------------------------------------------------------------------- @@ -254,84 +198,3 @@ def counting_enforce(self, all_events): assert enforce_calls == 0, ( "prepare_llm_messages must not call enforce_properties on the hot path" ) - - -# --------------------------------------------------------------------------- -# Tests for make_llm_completion -# --------------------------------------------------------------------------- - - -def test_make_llm_completion_applies_agent_policy( - mock_llm, sample_messages, sample_tools -): - mock_response = Mock(spec=LLMResponse) - mock_llm.generate.return_value = mock_response - - result = make_llm_completion(mock_llm, sample_messages, tools=sample_tools) - - assert result == mock_response - mock_llm.generate.assert_called_once_with( - messages=sample_messages, - tools=sample_tools, - store=False, - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - - -def test_make_llm_completion_normalizes_missing_tools(mock_llm, sample_messages): - make_llm_completion(mock_llm, sample_messages) - - assert mock_llm.generate.call_args.kwargs["tools"] == [] - - -@pytest.mark.asyncio -async def test_amake_llm_completion_applies_agent_policy( - mock_llm, sample_messages, sample_tools -): - mock_response = Mock(spec=LLMResponse) - mock_llm.agenerate.return_value = mock_response - - result = await amake_llm_completion(mock_llm, sample_messages, tools=sample_tools) - - assert result == mock_response - mock_llm.agenerate.assert_awaited_once_with( - messages=sample_messages, - tools=sample_tools, - store=False, - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) - - -# --------------------------------------------------------------------------- -# Integration tests -# --------------------------------------------------------------------------- - - -@patch("openhands.sdk.event.base.LLMConvertibleEvent.events_to_messages") -def test_prepare_llm_messages_and_make_llm_completion_integration( - mock_events_to_messages, sample_events, sample_messages, mock_llm -): - """Test integration between prepare_llm_messages and make_llm_completion.""" - mock_events_to_messages.return_value = sample_messages - view = View(events=sample_events) - - mock_response = Mock(spec=LLMResponse) - mock_llm.generate.return_value = mock_response - - messages = prepare_llm_messages(view) - result = make_llm_completion(mock_llm, messages) - - assert messages == sample_messages - assert result == mock_response - mock_llm.generate.assert_called_once_with( - messages=sample_messages, - tools=[], - store=False, - add_security_risk_prediction=True, - on_token=None, - call_context=None, - ) diff --git a/tests/sdk/agent/test_message_during_streaming_arun.py b/tests/sdk/agent/test_message_during_streaming_arun.py index 58ee24585e..ac6039ae75 100644 --- a/tests/sdk/agent/test_message_during_streaming_arun.py +++ b/tests/sdk/agent/test_message_during_streaming_arun.py @@ -67,7 +67,7 @@ class _InjectingAsyncLLM(LLM): def __init__(self): super().__init__(model=MODEL, usage_id="test-llm") - def uses_responses_api(self) -> bool: # keep amake_llm_completion on acompletion + def uses_responses_api(self) -> bool: # keep agenerate on acompletion return False async def acompletion(self, *, messages, tools=None, **kwargs): # type: ignore[override] diff --git a/tests/sdk/agent/test_stream_context.py b/tests/sdk/agent/test_stream_context.py index 774e814521..318a4fe8d7 100644 --- a/tests/sdk/agent/test_stream_context.py +++ b/tests/sdk/agent/test_stream_context.py @@ -39,9 +39,9 @@ def _chunk( delta_kwargs: dict = {"role": "assistant"} if content is not None: delta_kwargs["content"] = content - delta = Delta(**delta_kwargs) if reasoning_content is not None: - object.__setattr__(delta, "reasoning_content", reasoning_content) + delta_kwargs["reasoning_content"] = reasoning_content + delta = Delta(**delta_kwargs) choice = StreamingChoices(delta=delta, index=index, finish_reason=None) return ModelResponseStream(id=chunk_id, choices=[choice], model="test-model") diff --git a/tests/sdk/context/condenser/test_llm_summarizing_condenser.py b/tests/sdk/context/condenser/test_llm_summarizing_condenser.py index 35f7e0fac5..d9c467873e 100644 --- a/tests/sdk/context/condenser/test_llm_summarizing_condenser.py +++ b/tests/sdk/context/condenser/test_llm_summarizing_condenser.py @@ -57,10 +57,10 @@ def create_completion_result(content: str) -> LLMResponse: raw_response.id = "mock-llm-response-id" return LLMResponse(message=message, metrics=metrics, raw_response=raw_response) - mock_llm.completion.return_value = create_completion_result( + mock_llm.generate.return_value = create_completion_result( "Summary of forgotten events" ) - mock_llm.acompletion = AsyncMock(return_value=mock_llm.completion.return_value) + mock_llm.agenerate = AsyncMock(return_value=mock_llm.generate.return_value) mock_llm.uses_responses_api = lambda: False mock_llm.requires_streaming = False mock_llm.format_messages_for_llm = lambda messages: messages @@ -99,8 +99,8 @@ def create_completion_result(content: str) -> LLMResponse: # Helper method to set mock response content def set_mock_response_content(content: str): result = create_completion_result(content) - mock_llm.completion.return_value = result - mock_llm.acompletion = AsyncMock(return_value=result) + mock_llm.generate.return_value = result + mock_llm.agenerate = AsyncMock(return_value=result) mock_llm.set_mock_response_content = set_mock_response_content @@ -156,7 +156,7 @@ def test_condense_returns_view_when_no_condensation_needed(mock_llm: LLM) -> Non assert isinstance(result, View) assert result == view # LLM should not be called - cast(MagicMock, mock_llm.completion).assert_not_called() + cast(MagicMock, mock_llm.generate).assert_not_called() def test_condense_returns_condensation_when_needed(mock_llm: LLM) -> None: @@ -185,24 +185,7 @@ def test_condense_returns_condensation_when_needed(mock_llm: LLM) -> None: assert len(result.forgotten_event_ids) > 0 # LLM should be called once - cast(MagicMock, mock_llm.completion).assert_called_once() - - -def test_condense_uses_responses_api_when_required(mock_llm: LLM) -> None: - condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=10, keep_first=3) - cast(Any, mock_llm).set_mock_response_content("Summary from responses") - mock_llm.uses_responses_api = lambda: True - cast(Any, mock_llm.responses).return_value = cast( - Any, mock_llm.completion - ).return_value - - view = View.from_events([message_event(f"Event {i}") for i in range(11)]) - result = condenser.condense(view) - - assert isinstance(result, Condensation) - assert result.summary == "Summary from responses" - cast(MagicMock, mock_llm.responses).assert_called_once() - cast(MagicMock, mock_llm.completion).assert_not_called() + cast(MagicMock, mock_llm.generate).assert_called_once() def test_get_condensation_with_previous_summary(mock_llm: LLM) -> None: @@ -243,7 +226,7 @@ def test_get_condensation_with_previous_summary(mock_llm: LLM) -> None: assert result.summary == "Updated summary" # Verify that the LLM was called with the previous summary - completion_mock = cast(MagicMock, mock_llm.completion) + completion_mock = cast(MagicMock, mock_llm.generate) completion_mock.assert_called_once() call_args = completion_mock.call_args messages = call_args[1]["messages"] # Get keyword arguments @@ -273,7 +256,7 @@ def test_invalid_config(mock_llm: LLM) -> None: def test_get_condensation_does_not_pass_extra_body(mock_llm: LLM) -> None: - """Condenser should not pass extra_body to llm.completion. + """Condenser should not pass extra_body to llm.generate. This prevents providers like 1p Anthropic from rejecting the request with "extra_body: Extra inputs are not permitted". @@ -288,7 +271,7 @@ def test_get_condensation_does_not_pass_extra_body(mock_llm: LLM) -> None: assert isinstance(result, Condensation) # Ensure completion was called without an explicit extra_body kwarg - completion_mock = cast(MagicMock, mock_llm.completion) + completion_mock = cast(MagicMock, mock_llm.generate) assert completion_mock.call_count == 1 @@ -310,11 +293,11 @@ def test_condense_with_agent_llm(mock_llm: LLM) -> None: assert isinstance(result, Condensation) # Verify the condenser still uses its own LLM for summarization - completion_mock = cast(MagicMock, mock_llm.completion) + completion_mock = cast(MagicMock, mock_llm.generate) assert completion_mock.call_count == 1 # Agent LLM should not be called for completion (condenser uses its own LLM) - assert not agent_llm.completion.called + assert not agent_llm.generate.called _, kwargs = completion_mock.call_args assert "extra_body" not in kwargs @@ -362,7 +345,7 @@ def mock_token_count(messages, **_kwargs): assert isinstance(result, Condensation) # Verify the condenser used its own LLM for summarization - completion_mock = cast(MagicMock, mock_llm.completion) + completion_mock = cast(MagicMock, mock_llm.generate) assert completion_mock.call_count == 1 # Verify forgotten events were calculated based on token reduction @@ -614,7 +597,7 @@ def mock_token_count(messages, **_kwargs): assert len(result.forgotten_event_ids) > 0 # Verify the condenser used its own LLM for summarization - completion_mock = cast(MagicMock, mock_llm.completion) + completion_mock = cast(MagicMock, mock_llm.generate) assert completion_mock.call_count == 1 @@ -673,7 +656,7 @@ def test_generate_condensation_raises_on_zero_events(mock_llm: LLM) -> None: ) # Verify the LLM was never called - cast(MagicMock, mock_llm.completion).assert_not_called() + cast(MagicMock, mock_llm.generate).assert_not_called() @pytest.mark.parametrize( @@ -825,7 +808,7 @@ def test_condense_with_soft_requirement_and_no_condensation_available( assert isinstance(result, View) assert result == view # LLM should not be called - cast(MagicMock, mock_llm.completion).assert_not_called() + cast(MagicMock, mock_llm.generate).assert_not_called() def test_minimum_progress_default_value(mock_llm: LLM) -> None: @@ -917,7 +900,7 @@ def test_generate_condensation_wraps_llm_errors(mock_llm: LLM) -> None: """LLM failures in _generate_condensation raise NoCondensationAvailableException.""" # noqa: E501 condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=10, keep_first=2) - cast(MagicMock, mock_llm.completion).side_effect = RuntimeError("boom") + cast(MagicMock, mock_llm.generate).side_effect = RuntimeError("boom") events: list[Event] = [message_event(f"Event {i}") for i in range(12)] view = View.from_events(events) @@ -931,7 +914,7 @@ async def test_agenerate_condensation_wraps_llm_errors(mock_llm: LLM) -> None: """Async variant: LLM failures surface as NoCondensationAvailableException.""" condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=10, keep_first=2) - cast(MagicMock, mock_llm.acompletion).side_effect = RuntimeError("boom") + cast(MagicMock, mock_llm.agenerate).side_effect = RuntimeError("boom") events: list[Event] = [message_event(f"Event {i}") for i in range(12)] view = View.from_events(events) @@ -951,8 +934,8 @@ def test_llm_error_triggers_hard_context_reset(mock_llm: LLM) -> None: # First call (get_condensation path) fails; second call # (hard_context_reset path) succeeds. - success_response = cast(Any, mock_llm).completion.return_value - cast(MagicMock, mock_llm.completion).side_effect = [ + success_response = cast(Any, mock_llm).generate.return_value + cast(MagicMock, mock_llm.generate).side_effect = [ RuntimeError("context window exceeded"), success_response, ] @@ -961,7 +944,7 @@ def test_llm_error_triggers_hard_context_reset(mock_llm: LLM) -> None: assert isinstance(result, Condensation) assert result.summary == "Summary of forgotten events" - assert cast(MagicMock, mock_llm.completion).call_count == 2 + assert cast(MagicMock, mock_llm.generate).call_count == 2 def _streaming_llm() -> LLM: diff --git a/tests/sdk/llm/test_cleanup_profile.py b/tests/sdk/llm/test_cleanup_profile.py index 1480c13566..3f09a55144 100644 --- a/tests/sdk/llm/test_cleanup_profile.py +++ b/tests/sdk/llm/test_cleanup_profile.py @@ -103,7 +103,7 @@ def load_profile(self: LLMProfileStore, name: str, *, cipher: Any = None) -> LLM assert result == "Done! I appreciate the nudge." # Stateless call: only a system + user message, no tools, no history. assert [message.role for message in cleanup_llm.last_messages] == ["system", "user"] - assert cleanup_llm.last_tools == [] + assert cleanup_llm.last_tools is None assert "repair" in _message_text(cleanup_llm.last_messages[0]).lower() assert original in _message_text(cleanup_llm.last_messages[1]) diff --git a/tests/sdk/llm/test_llm_span_cost.py b/tests/sdk/llm/test_llm_span_cost.py index 34e4dd2165..e5775137cc 100644 --- a/tests/sdk/llm/test_llm_span_cost.py +++ b/tests/sdk/llm/test_llm_span_cost.py @@ -89,8 +89,11 @@ def test_span_cost_agrees_with_metrics(exporter): def test_cache_buckets_survive_absent_prompt_tokens_details(): - usage = Usage(prompt_tokens=100, completion_tokens=5) - object.__setattr__(usage, "cache_creation_input_tokens", 42) + usage = Usage( + prompt_tokens=100, + completion_tokens=5, + cache_creation_input_tokens=42, + ) assert Telemetry._cache_buckets(usage)[1] == 42 diff --git a/tests/sdk/llm/test_llm_telemetry.py b/tests/sdk/llm/test_llm_telemetry.py index 4bead882c0..84de822c20 100644 --- a/tests/sdk/llm/test_llm_telemetry.py +++ b/tests/sdk/llm/test_llm_telemetry.py @@ -183,14 +183,12 @@ def test_record_usage_with_cache_write(self, basic_telemetry): """Test token usage recording with cache write tokens.""" from litellm import Usage - usage = Usage.model_construct( + usage = Usage( prompt_tokens=100, completion_tokens=50, total_tokens=150, - model_extra={"cache_creation_input_tokens": 30}, + cache_creation_input_tokens=30, ) - # Set the attribute that telemetry code expects - usage._cache_creation_input_tokens = 30 basic_telemetry._record_usage(usage, "test-id", 4096) diff --git a/tests/tools/ask_oracle/test_ask_oracle.py b/tests/tools/ask_oracle/test_ask_oracle.py index 9857282562..0aae674c7a 100644 --- a/tests/tools/ask_oracle/test_ask_oracle.py +++ b/tests/tools/ask_oracle/test_ask_oracle.py @@ -164,7 +164,7 @@ def load_profile( assert "The tool needs an Oracle profile name." in _message_text( oracle_llm.last_messages[1] ) - assert oracle_llm.last_tools == [] + assert oracle_llm.last_tools is None assert conversation.agent.llm.model == "default-model" assert conversation.state.agent.llm.model == "default-model"