Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions openhands-sdk/openhands/sdk/llm/llm.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,10 @@ def completion(
f"for model {self.model}"
)
formatted_messages, kwargs = self.pre_request_prompt_mock(
formatted_messages, cc_tools or [], kwargs
formatted_messages,
cc_tools or [],
kwargs,
include_security_params=add_security_risk_prediction,
)

# 3) normalize provider params
Expand Down Expand Up @@ -820,7 +823,10 @@ def _one_attempt(**retry_kwargs) -> ModelResponse:
if use_mock_tools:
raw_resp = copy.deepcopy(resp)
resp = self.post_response_prompt_mock(
resp, nonfncall_msgs=formatted_messages, tools=cc_tools
resp,
nonfncall_msgs=formatted_messages,
tools=cc_tools,
include_security_params=add_security_risk_prediction,
)
# 6) telemetry
self._telemetry.on_response(resp, raw_resp=raw_resp)
Expand Down
27 changes: 20 additions & 7 deletions openhands-sdk/openhands/sdk/llm/mixins/fn_call_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import json
import re
from collections.abc import Iterable
from typing import Any, Literal, NotRequired, TypedDict, cast
from typing import Any, Final, Literal, NotRequired, TypedDict, cast

from litellm import ChatCompletionToolParam, ChatCompletionToolParamFunctionChunk

Expand Down Expand Up @@ -65,6 +65,11 @@ class TextPart(TypedDict):
</IMPORTANT>
""" # noqa: E501

SECURITY_PARAMS_EXAMPLE: Final[str] = """\
<parameter=security_risk>LOW</parameter>
<parameter=summary>Brief description of action</parameter>
"""

STOP_WORDS = ["</function"]

IN_CONTEXT_LEARNING_EXAMPLE_PREFIX = get_example_for_tools
Expand Down Expand Up @@ -316,14 +321,18 @@ def convert_fncall_messages_to_non_fncall_messages(
messages: list[dict],
tools: list[ChatCompletionToolParam],
add_in_context_learning_example: bool = True,
include_security_params: bool = False,
) -> list[dict]:
"""Convert function calling messages to non-function calling messages."""
messages = copy.deepcopy(messages)

formatted_tools = convert_tools_to_description(tools)
system_message_suffix = system_message_suffix_TEMPLATE.format(
description=formatted_tools
)
template = system_message_suffix_TEMPLATE
if include_security_params:
template = template.replace(
"</function>", SECURITY_PARAMS_EXAMPLE + "</function>"
)
system_message_suffix = template.format(description=formatted_tools)

converted_messages = []
first_user_message_encountered = False
Expand Down Expand Up @@ -621,13 +630,17 @@ def _normalize_parameter_tags(fn_body: str) -> str:
def convert_non_fncall_messages_to_fncall_messages(
messages: list[dict],
tools: list[ChatCompletionToolParam],
include_security_params: bool = False,
) -> list[dict]:
"""Convert non-function calling messages back to function calling messages."""
messages = copy.deepcopy(messages)
formatted_tools = convert_tools_to_description(tools)
system_message_suffix = system_message_suffix_TEMPLATE.format(
description=formatted_tools
)
template = system_message_suffix_TEMPLATE
if include_security_params:
template = template.replace(
"</function>", SECURITY_PARAMS_EXAMPLE + "</function>"
)
system_message_suffix = template.format(description=formatted_tools)

converted_messages = []
tool_call_counter = 1 # Counter for tool calls
Expand Down
11 changes: 9 additions & 2 deletions openhands-sdk/openhands/sdk/llm/mixins/non_native_fc.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def pre_request_prompt_mock(
messages: list[dict],
tools: list[ChatCompletionToolParam],
kwargs: dict,
include_security_params: bool = False,
) -> tuple[list[dict], dict]:
"""Convert to non-fncall prompting when native tool-calling is off."""
# Skip in-context learning examples for models that understand the format
Expand All @@ -48,7 +49,10 @@ def pre_request_prompt_mock(
s in self.model for s in ("openhands-lm", "devstral", "nemotron")
)
messages = convert_fncall_messages_to_non_fncall_messages(
messages, tools, add_in_context_learning_example=add_iclex
messages,
tools,
add_in_context_learning_example=add_iclex,
include_security_params=include_security_params,
)
if get_features(self.model).supports_stop_words and not self.disable_stop_word:
kwargs = dict(kwargs)
Expand All @@ -63,6 +67,7 @@ def post_response_prompt_mock(
resp: ModelResponse,
nonfncall_msgs: list[dict],
tools: list[ChatCompletionToolParam],
include_security_params: bool = False,
) -> ModelResponse:
if len(resp.choices) < 1:
raise LLMNoResponseError(
Expand All @@ -84,7 +89,9 @@ def _all_choices(
orig_msg = resp.choices[0].message
non_fn_message: dict = orig_msg.model_dump()
fn_msgs: list[dict] = convert_non_fncall_messages_to_fncall_messages(
nonfncall_msgs + [non_fn_message], tools
nonfncall_msgs + [non_fn_message],
tools,
include_security_params=include_security_params,
)
last: dict = fn_msgs[-1]

Expand Down
41 changes: 41 additions & 0 deletions tests/sdk/llm/test_llm_fncall_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
convert_non_fncall_messages_to_fncall_messages,
convert_tool_call_to_string,
convert_tools_to_description,
system_message_suffix_TEMPLATE,
)


Expand Down Expand Up @@ -919,3 +920,43 @@ def test_convert_tools_to_description_object_details():
)

assert result.strip() == expected.strip()


def test_system_message_suffix_template_excludes_security_risk_by_default():
"""Test that system_message_suffix_TEMPLATE does NOT include security_risk
when the security analyzer is disabled."""
assert "<parameter=security_risk>" not in system_message_suffix_TEMPLATE
assert "<parameter=summary>" not in system_message_suffix_TEMPLATE


def test_security_params_included_when_flag_is_true():
"""Test that security_risk and summary appear in converted messages
when include_security_params=True (i.e., security analyzer is active).

Regression test for issue #2740.
"""
messages = [
{"role": "system", "content": "You are a helpful assistant."},
{"role": "user", "content": "Hello"},
]
result = convert_fncall_messages_to_non_fncall_messages(
messages, FNCALL_TOOLS, include_security_params=True
)
system_content = result[0]["content"]
assert "<parameter=security_risk>" in system_content
assert "<parameter=summary>" in system_content


def test_security_params_excluded_when_flag_is_false():
"""Test that security_risk and summary do NOT appear in converted messages
when include_security_params=False (default)."""
messages = [
{"role": "system", "content": "You are a helpful assistant."},
{"role": "user", "content": "Hello"},
]
result = convert_fncall_messages_to_non_fncall_messages(
messages, FNCALL_TOOLS, include_security_params=False
)
system_content = result[0]["content"]
assert "<parameter=security_risk>" not in system_content
assert "<parameter=summary>" not in system_content
Loading