feat: Support function call requests/responses #56

caffeinism · 2025-07-05T15:22:10Z

I have implemented the issue (#50)

I also changed the OpenAI related messages internally from BaseModel to the actual internal data format of openai python sdk. This shouldn't be an issue in the data format actually used by users, since we only added tools and tool_choice to CreateAgentDto.

In the chat completion unit test, message.role = "ai" was incorrect for the openai message format, so i changed it to "assistant".

Example:

Server

class AgentFactory(BaseAgentFactory):
    async def create_agent(self, dto: CreateAgentDto):
        llm = self.create_llm(
            ...
        )
        return llm.bind_tools(dto.tools)

Client

tools = [{
    "type": "function",
    "function": {
        "name": "get_weather",
        "description": "Get current temperature for a given location.",
        "parameters": {
            "type": "object",
            "properties": {
                "location": {
                    "type": "string",
                    "description": "City and country e.g. Bogotá, Colombia"
                }
            },
            "required": [
                "location"
            ],
            "additionalProperties": False
        },
        "strict": True
    }
}]

chat_completion = await openai_client.chat.completions.create(
    model="gpt-4o-mini",
    messages=[
        {
            "role": "user",
            "content": 'What is the weather like in London today?',
        }
    ],
    tools=tools,
)

# ChatCompletion(id='...', choices=[Choice(finish_reason='tool_calls', index=0, logprobs=None, message=ChatCompletionMessage(content=None, refusal=None, role='assistant', annotations=None, audio=None, function_call=None, tool_calls=[ChatCompletionMessageToolCall(id='...', function=Function(arguments='{"location": "London, UK"}', name='get_weather'), type='function')]))], created=1751731311, model='gpt-4o-mini', object='chat.completion', service_tier=None, system_fingerprint='', usage=CompletionUsage(completion_tokens=-1, prompt_tokens=-1, total_tokens=-1, completion_tokens_details=None, prompt_tokens_details=None))

samuelint

A functional test is missing to verify that the new feature works as intended.
Adding one would help ensure the feature behaves correctly and prevent regressions in future changes.

langchain_openai_api_bridge/chat_completion/chat_completion_chunk_choice_adapter.py

langchain_openai_api_bridge/chat_completion/langchain_stream_adapter.py

langchain_openai_api_bridge/chat_completion/langchain_invoke_adapter.py

samuelint · 2025-07-06T14:27:53Z

For tests to pass in the pipeline, please have secrets set in your GitHub pipeline.

ANTHROPIC_API_KEY
GROQ_API_KEY
OPENAI_API_KEY

caffeinism · 2025-07-06T14:41:19Z

For tests to pass in the pipeline, please have secrets set in your GitHub pipeline.

ANTHROPIC_API_KEY

GROQ_API_KEY

OPENAI_API_KEY

Oh, so that should be added to my repository?

samuelint · 2025-07-06T15:05:46Z

For tests to pass in the pipeline, please have secrets set in your GitHub pipeline.
ANTHROPIC_API_KEY
GROQ_API_KEY
OPENAI_API_KEY

Oh, so that should be added to my repository?

Yes, since the project is forked, it should be set in your repository. This is a known limitation on GitHub, where my secrets cannot be used in forked pull-requests.

caffeinism · 2025-07-06T15:07:13Z

For tests to pass in the pipeline, please have secrets set in your GitHub pipeline.
ANTHROPIC_API_KEY
GROQ_API_KEY
OPENAI_API_KEY

Oh, so that should be added to my repository?

Yes, since the project is forked, it should be set in your repository. This is a known limitation on GitHub, where my secrets cannot be used in forked pull-requests.

For tests, what format should I add tests in? I'm not very familiar with something like this. In functional tests, I'm thinking of doing simple tool calls like addition to verify behavior, but for unit tests, I'm not sure how I should write them down. Should I just add the various tests in the format they're currently added in (I've added one commit as an example)?

For repository secrets, I've added them to my own repository, but it's a bit confusing to me that secrets from my own repository are used for actions that run on PRs here.

…me as OpenAI's server.

caffeinism · 2025-07-07T07:00:01Z

I added a simple unittest and functional test for the function call functionality. I also found where the behavior of the server differs from the actual openai server, so I changed it (only the first chunk should contain role)

samuelint

Great job, one last minor comment and it will be ready to be merged.

samuelint · 2025-07-07T18:57:39Z

tests/test_unit/chat_completion/test_chat_completion_chunk_choice_adapter.py

+        assert result.delta.role == "assistant"

-    def test_message_have_assistant_role_by_default(self):
+    def test_delta_message_have_none_role_by_default(self):


This test was there for a reason. It might break something, I think the behaviour should remains as before except if you have a good reason for it.

samuelint · 2025-07-07T18:58:03Z

tests/test_unit/chat_completion/test_chat_completion_chunk_choice_adapter.py

+        assert result.choices[0].delta.role == "assistant"

-    def test_message_have_assistant_role_by_default(self):
+    def test_delta_message_have_none_role_by_default(self):


This test was there for a reason. It might break something, I think the behaviour should remains as before except if you have a good reason for it.

If so, is it okay to leave “assistant” as the default, but in the first actual behavior, only send “assistant” for the first chunk and None for the rest?

This change is due to:

I also found where the behavior of the server differs from the actual openai server, so I changed it (only the first chunk should contain role)

When returning a chunk, the openai server returns None by default, and only the very first chunk returns role.

It's ok if this is the default openai api behaviour.

Please add a unit test in the langchain_openai_api_bridge/chat_completion/langchain_stream_adapter.py unit test.

Have a test function name that explain that behaviour, "Only the first chunk has role assistant, afterward role is None". That will document the use case.

samuelint · 2025-07-07T18:59:10Z

tests/test_unit/chat_completion/test_chat_completion_chunk_choice_adapter.py

+        result = to_openai_chat_completion_chunk_object(event)
+
+        assert result.choices[0].delta.function_call is not None
+        assert result.choices[0].delta.function_call.name == "my_func"


The test does not validate the args property. I think it should have a test for the name and another for the args

samuelint · 2025-07-07T19:03:49Z

..._functional/fastapi_chat_completion_openai_function_call/test_server_openai_function_call.py

+
+
+def test_chat_completion_function_call_weather_stream(openai_client: OpenAI):
+    tools = [{


Since tools section is the same in each tests, it could be re-used instead of copy/paste

samuelint · 2025-07-07T19:07:04Z

For tests to pass in the pipeline, please have secrets set in your GitHub pipeline.
ANTHROPIC_API_KEY
GROQ_API_KEY
OPENAI_API_KEY

Oh, so that should be added to my repository?

Yes, since the project is forked, it should be set in your repository. This is a known limitation on GitHub, where my secrets cannot be used in forked pull-requests.

For tests, what format should I add tests in? I'm not very familiar with something like this. In functional tests, I'm thinking of doing simple tool calls like addition to verify behavior, but for unit tests, I'm not sure how I should write them down. Should I just add the various tests in the format they're currently added in (I've added one commit as an example)?

For repository secrets, I've added them to my own repository, but it's a bit confusing to me that secrets from my own repository are used for actions that run on PRs here.

What you did is fine, you've respected how other tests are made. The goal of functional tests is to validate the feature work from the API standpoint.

samuelint · 2025-07-09T00:38:44Z

tests/test_unit/chat_completion/test_chat_completion_chunk_choice_adapter.py

+        assert result.choices[0].delta.role == "assistant"

-    def test_message_have_assistant_role_by_default(self):
+    def test_delta_message_have_none_role_by_default(self):


It's ok if this is the default openai api behaviour.

Please add a unit test in the langchain_openai_api_bridge/chat_completion/langchain_stream_adapter.py unit test.

Have a test function name that explain that behaviour, "Only the first chunk has role assistant, afterward role is None". That will document the use case.

caffeinism · 2025-07-09T03:49:24Z

I've add it

caffeinism added 2 commits July 6, 2025 00:15

feat: Support function call requests/responses

cddbc73

fix: Allow finish_reason=tool_call in streaming mode

ef3f35a

samuelint requested changes Jul 6, 2025

View reviewed changes

test: Add chat completion chunk tests

6ecb6e1

caffeinism added 2 commits July 7, 2025 15:57

fix: Change to only include role=assistant in the first chunk, the sa…

d104dd0

…me as OpenAI's server.

test: Add chat completion function call functional test

1cfb88b

caffeinism force-pushed the support-tool-response branch from d115c6e to 1cfb88b Compare July 7, 2025 06:57

samuelint requested changes Jul 7, 2025

View reviewed changes

fix: Change to respond only to the finish_reason of the last stream

219e233

caffeinism force-pushed the support-tool-response branch from 34bf370 to 219e233 Compare July 8, 2025 03:56

caffeinism added 3 commits July 8, 2025 13:00

test: Remove duplicate code

77320d5

test: Add arguments validation

58e9d69

fix: Check attribute for Any type chunk

0abf216

samuelint requested changes Jul 9, 2025

View reviewed changes

test: Add stream validation

4673dc1

samuelint changed the base branch from main to feat-Support-function-call-requests/responses-(from-fork) July 11, 2025 14:35

samuelint merged commit e0b6a06 into samuelint:feat-Support-function-call-requests/responses-(from-fork) Jul 11, 2025
1 check failed



		def test_chat_completion_function_call_weather_stream(openai_client: OpenAI):
		tools = [{

Uh oh!

feat: Support function call requests/responses #56

feat: Support function call requests/responses #56

Uh oh!

Conversation

caffeinism commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example:

Uh oh!

samuelint left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samuelint commented Jul 6, 2025

Uh oh!

caffeinism commented Jul 6, 2025

Uh oh!

samuelint commented Jul 6, 2025

Uh oh!

caffeinism commented Jul 6, 2025

Uh oh!

caffeinism commented Jul 7, 2025

Uh oh!

samuelint left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelint commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caffeinism commented Jul 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

caffeinism commented Jul 5, 2025 •

edited

Loading