Skip to content
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

feat(playground): add tool role messages to ui #5103

Merged
merged 12 commits into from
Oct 22, 2024

Conversation

Parker-Stafford
Copy link
Contributor

@Parker-Stafford Parker-Stafford commented Oct 18, 2024

resolves #5041

  • Adds server and ui support for tool role messages
  • adds server and ui support for ai messages with tool_calls attached
Screen.Recording.2024-10-18.at.5.25.58.PM.mov

@@ -101,9 +81,9 @@ export type ModelConfig = {
/**
* The type of a tool in the playground
*/
export type Tool = {
export type OpenAITool = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just being more honest in preparation for the conversion from one tool schema to another

* allow for extra keys when the zod schema is used for parsing. This is to allow more flexibility for users
* to define their own tool calls according
*/
export const openAIToolCallSchema = z.object({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment below, trying to nme things more honestly in prep for tool conversion between providers @cephalization

@Parker-Stafford Parker-Stafford marked this pull request as ready for review October 19, 2024 00:45
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 19, 2024
@mikeldking
Copy link
Contributor

@Parker-Stafford pushed some minor padding in 12dfaba

Looks great. Just need to work on that rebase with @anticorrelator and @axiomofjoy

Copy link
Contributor

@cephalization cephalization left a comment

Choose a reason for hiding this comment

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

I hit this error when attempting to change message role to AI

For some reason our build process is heavily over-caching the @arizeai/components package.. Had to nuke node modules and reinstall for the new components to appear

@cephalization cephalization dismissed their stale review October 21, 2024 15:23

there was an unrelated issue

@cephalization
Copy link
Contributor

When you change message role, tools are not reset

2024-10-21 11 54 19

@Parker-Stafford
Copy link
Contributor Author

I hit this error when attempting to change message role to AI

For some reason our build process is heavily over-caching the @arizeai/components package.. Had to nuke node modules and reinstall for the new components to appear

I noticed this as well, i hado nuke node modules and not frozen install... then it worked

@cephalization
Copy link
Contributor

I hit this error when attempting to change message role to AI
For some reason our build process is heavily over-caching the @arizeai/components package.. Had to nuke node modules and reinstall for the new components to appear

I noticed this as well, i hado nuke node modules and not frozen install... then it worked

pnpm claims to be installing new versions, and if I look at my node_modules folder the new components are in src, so I have to wonder if its an issue with our vite dev setup.

@Parker-Stafford
Copy link
Contributor Author

I hit this error when attempting to change message role to AI
For some reason our build process is heavily over-caching the @arizeai/components package.. Had to nuke node modules and reinstall for the new components to appear

I noticed this as well, i hado nuke node modules and not frozen install... then it worked

pnpm claims to be installing new versions, and if I look at my node_modules folder the new components are in src, so I have to wonder if its an issue with our vite dev setup.

ahh could be i did custom chunking for some of our deps including ui components, i wonder if those are getting cached somehow

@cephalization
Copy link
Contributor

I hit this error when attempting to change message role to AI
For some reason our build process is heavily over-caching the @arizeai/components package.. Had to nuke node modules and reinstall for the new components to appear

I noticed this as well, i hado nuke node modules and not frozen install... then it worked

pnpm claims to be installing new versions, and if I look at my node_modules folder the new components are in src, so I have to wonder if its an issue with our vite dev setup.

ahh could be i did custom chunking for some of our deps including ui components, i wonder if those are getting cached somehow

That's some good intuition, sounds like a possible candidate. Will experiment with it this week

tool_calls: Optional[List[JSON]] = UNSET
"""The tool calls that were made in the message"""
tool_call_id: Optional[str] = UNSET
"""The ID of the tool call that was made in a prior message"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""The ID of the tool call that was made in a prior message"""
"""The ID of the tool call described in the current message"""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this has to match the id of a tool call in a prior message, the wording might be a little confusing right now, but it needs to match a pre-existing tool call id, which would be message.tool_calls.id in a previous message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try to update a bit

Comment on lines 283 to 301
def to_openai_tool_call_param(
self,
tool_calls: List[JSONScalarType],
) -> List["ChatCompletionMessageToolCallParam"]:
from openai.types.chat import ChatCompletionMessageToolCallParam

return [
ChatCompletionMessageToolCallParam(
id=tool_call.get("id", ""),
function={
"name": tool_call.get("function", {}).get("name", ""),
"arguments": safe_json_dumps(
tool_call.get("function", {}).get("arguments", "")
),
},
type="function",
)
for tool_call in tool_calls
]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this input a single tool call and return a single tool call param for the sake of granularity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh sounds good

@Parker-Stafford Parker-Stafford merged commit 083ef42 into main Oct 22, 2024
33 checks passed
@Parker-Stafford Parker-Stafford deleted the parker/5041-tool-messages-ui branch October 22, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[playground][ui] support function / tool call messages
4 participants