Skip to content

chat.py fixes #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 24, 2025
Merged

chat.py fixes #84

merged 10 commits into from
Jun 24, 2025

Conversation

gordon-lim
Copy link
Contributor

Added Tool: prefix before <tool_response> tag.

Example test case with this fix:

def test_form_prompt_string_with_tool_calls_chat_completions() -> None:
    """Test formatting with tool calls in chat completions format."""
    messages: list[dict[str, Any]] = [
        {"role": "user", "content": "What's the weather in Paris?"},
        {
            "role": "assistant",
            "content": "",
            "tool_calls": [
                {
                    "type": "function",
                    "id": "call_123",
                    "function": {
                        "name": "get_weather",
                        "arguments": '{"location": "Paris"}',
                    },
                }
            ],
        },
        {
            "role": "tool",
            "name": "get_weather",
            "tool_call_id": "call_123",
            "content": "22.1",
        },
    ]
    expected = (
        "User: What's the weather in Paris?\n\n"
        "Assistant: <tool_call>\n"
        "{\n"
        '  "name": "get_weather",\n'
        '  "arguments": {\n'
        '    "location": "Paris"\n'
        "  },\n"
        '  "call_id": "call_123"\n'
        "}\n"
        "</tool_call>\n\n"
        "Tool: "
        "<tool_response>\n"
        "{\n"
        '  "name": "get_weather",\n'
        '  "call_id": "call_123",\n'
        '  "output": "22.1"\n'
        "}\n"
        "</tool_response>\n\n"
        "Assistant:"
    )
    assert form_prompt_string(messages) == expected

Tool prompt follows from https://github.com/NousResearch/Hermes-Function-Calling including the following lines:

You are a function calling AI model. You are provided with function signatures within XML tags.  
Don't make assumptions about what values to plug into functions.

Are they fine to keep? Relevant test case for tool prompt:

def test_form_prompt_string_with_tools_chat_completions() -> None:
    """Test formatting with tools in chat completions format."""
    messages = [
        {"role": "user", "content": "What can you do?"},
    ]
    tools = [
        {
            "type": "function",
            "function": {
                "name": "search",
                "description": "Search the web for information",
                "parameters": {
                    "type": "object",
                    "properties": {"query": {"type": "string", "description": "The search query"}},
                    "required": ["query"],
                },
            },
        }
    ]
    expected = (
        "System: You are a function calling AI model. You are provided with function signatures within <tools> </tools> XML tags. "
        "You may call one or more functions to assist with the user query. If available tools are not relevant in assisting "
        "with user query, just respond in natural conversational language. Don't make assumptions about what values to plug "
        "into functions. After calling & executing the functions, you will be provided with function results within "
        "<tool_response> </tool_response> XML tags.\n\n"
        "<tools>\n"
        '{"type":"function","function":{"name":"search","description":"Search the web for information","parameters":'
        '{"type":"object","properties":{"query":{"type":"string","description":"The search query"}},"required":["query"]}}}\n'
        "</tools>\n\n"
        "For each function call return a JSON object, with the following pydantic model json schema:\n"
        "{'name': <function-name>, 'arguments': <args-dict>}\n"
        "Each function call should be enclosed within <tool_call> </tool_call> XML tags.\n"
        "Example:\n"
        "<tool_call>\n"
        "{'name': <function-name>, 'arguments': <args-dict>}\n"
        "</tool_call>\n\n"
        "Note: Your past messages will include a call_id in the <tool_call> XML tags. "
        "However, do not generate your own call_id when making a function call.\n\n"
        "User: What can you do?\n\n"
        "Assistant:"
    )
    assert form_prompt_string(messages, tools) == expected

@gordon-lim gordon-lim requested a review from jwmueller June 20, 2025 16:42
@gordon-lim
Copy link
Contributor Author

gordon-lim commented Jun 23, 2025

Updated _TOOL_DEFINITIONS_PREFIX

_TOOL_DEFINITIONS_PREFIX = (
    "You are an AI Assistant that can call provided tools (a.k.a. functions). "
    "The set of available tools is provided to you as function signatures within "
    f"{_TOOLS_TAG_START} {_TOOLS_TAG_END} XML tags. "
    "You may call one or more of these functions to assist with the user query. If the provided functions are not helpful/relevant, "
    "then just respond in natural conversational language. Don't make assumptions about what values to plug "
    "into functions. After you choose to call a function, you will be provided with the function's results within "
    f"{_TOOL_RESPONSE_TAG_START} {_TOOL_RESPONSE_TAG_END} XML tags.\n\n"
    f"{_TOOLS_TAG_START}\n"
)

Updated test cases that used the old prefix accordingly.

@gordon-lim gordon-lim requested review from jwmueller and removed request for jwmueller June 23, 2025 16:26
@gordon-lim gordon-lim requested a review from jwmueller June 23, 2025 18:52
@gordon-lim
Copy link
Contributor Author

gordon-lim commented Jun 23, 2025

I am piggybacking on this PR a bug fix when tools is an empty list and we are still using the tools prompt. I thought it was minor enough.

@gordon-lim gordon-lim changed the title Added Tool: prefix to chat.py chat.py Tool fixes Jun 23, 2025
@gordon-lim gordon-lim changed the title chat.py Tool fixes chat.py fixes + add form_response_string Jun 23, 2025
Copy link
Member

@jwmueller jwmueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont forget to make the formatting/CI pass before merging this

@gordon-lim gordon-lim changed the title chat.py fixes + add form_response_string chat.py fixes Jun 24, 2025
@gordon-lim
Copy link
Contributor Author

Since this is already approved, I will make the form_response_string addition a separate PR.

@gordon-lim gordon-lim merged commit 9c27947 into main Jun 24, 2025
3 checks passed
@gordon-lim gordon-lim deleted the gordonlim-tlm-chat-tools-prompt branch June 24, 2025 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants