-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: handle mcp tool calls in previous response correctly #3155
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
Conversation
Could you run and modify the integration tests as appropriate too? These are in |
Two concerns/questions to run by you on this: (1) I see that the current function call integration test is being skipped as "Tool calling is not reliable."[1] [1]
|
@grs if you use library client for testing, then all of this is actually already possible. See https://github.com/llamastack/llama-stack/blob/main/tests/integration/non_ci/responses/test_responses.py#L556 you can execute this test as such: pytest -s -v tests/integration/non_ci/responses/ \
--stack-config=starter \
--text-model openai/gpt-4o \
--embedding-model=sentence-transformers/all-MiniLM-L6-v2 \
-k "multi_turn_tool and streaming and client_with_models" there are a couple of important things in that command line:
Let me know if this helps. |
@ashwinb Thank you that does indeed help! Sorry also that I missed the |
@grs I moved a bunch of things around and refactored. Your changes should be relatively easy to rebase though. |
bceb88f
to
7ad5ef2
Compare
fixes llamastack#3105 Signed-off-by: Gordon Sim <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…#3155) # What does this PR do? Handles MCP tool calls in a previous response Closes llamastack#3105 ## Test Plan Made call to create response with tool call, then made second call with the first linked through previous_response_id. Did not get error. Also added unit test. Signed-off-by: Gordon Sim <[email protected]>
What does this PR do?
Handles MCP tool calls in a previous response
Closes #3105
Test Plan
Made call to create response with tool call, then made second call with the first linked through previous_response_id. Did not get error.
Also added unit test.