Feature/native tool calling #791

collindutter · 2024-05-17T20:22:15Z

This PR adds the ability for prompt drivers to use native function calling over the Griptape provided approach (regex).

To do this, the following things have been changed:

Added PrompStack.TOOL_CALL_ROLE, and PromptStack.TOOL_RESULT_ROLE. Depending on the the LLM provider, these may get translated into user or assistant roles.
Added ActionsArtifact for tracking function calls made by the LLM.
Added ActionChunkArtifact for tracking partial function calls made by the LLM when streaming.
Moved ActionsSubtask.Action to ActionsArtifact.Action so that Prompt Drivers could more easily interact with Actions.
Changed PromptStack.content: str to PromptStack.content: str | BaseArtifact. Right now the type of content depends on the role (Tool roles use ActionsArtifact), but this will be changed in the next PR that breaks up PromptStack.Input into distinct classes (PromptStackTextInput, PromptStackActionsInput, PromptStackImageInput etc.)
Changed ActionsSubtask.input: str to ActionsSubtask.input: str | ActionsArtifact.Action. If a string is provided, the Task will parse it into an ActionsSubtask.Action using regex (current behavior). If an ActionsSubtask.Action is provided, it will use this (new behavior).
Added ActionsSubtask.single_action flag for use with ToolTask to denote that the incoming action string is not a list. The parsing logic changes slightly when this flag is set to true.
Added BasePromptDriver.use_native_tools to toggle between old regex tool usage and native tool usage.
Updated OpenAiChatPromptDriver to use new Prompt Stack tool inputs
Updated AnthropicChatPromptDriver to use new Prompt Stack tool inputs.
Updated toolkit_task/system.j2 prompt to conditionally render CoT wording based on use_native_tools.
Updated tool_task/system.j2 prompt to conditionally render action schema instructions based on use_native_tools.

codecov · 2024-05-21T17:05:39Z

Codecov Report

Attention: Patch coverage is 87.50000% with 51 lines in your changes are missing coverage. Please review.

Files	Patch %	Lines
griptape/drivers/prompt/anthropic_prompt_driver.py	83.33%	4 Missing and 7 partials ⚠️
...iptape/drivers/prompt/openai_chat_prompt_driver.py	85.71%	5 Missing and 5 partials ⚠️
griptape/tasks/actions_subtask.py	87.01%	8 Missing and 2 partials ⚠️
griptape/drivers/prompt/base_prompt_driver.py	83.33%	4 Missing and 5 partials ⚠️
griptape/tasks/tool_task.py	78.26%	3 Missing and 2 partials ⚠️
griptape/artifacts/actions_artifact.py	89.65%	1 Missing and 2 partials ⚠️
griptape/utils/stream.py	66.66%	0 Missing and 2 partials ⚠️
griptape/utils/prompt_stack.py	95.83%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

collindutter · 2024-05-22T21:38:53Z

griptape/artifacts/action_chunk_artifact.py

+
+
+@define
+class ActionChunkArtifact(TextArtifact, SerializableMixin):


Really don't like this class, but it's required for when we're streaming tool usage. Should we change ActionsArtifact.Action into ActionArtifact? Then we can yield ActionArtifacts much like we yield TextArtifacts during streaming.

Are you saying that you would yield an ActionArtifact after getting all the ActionChunkArtifact for that action, instead of yielding the ActionChunkArtifacts themselves? (Or would you yield both?)

Regardless, I think moving ActionsArtifact.Action into a non-nested ActionArtifact is a good idea.

dylanholmes · 2024-05-23T17:57:02Z

griptape/drivers/prompt/anthropic_prompt_driver.py

+        text = response.content[0].text if response.content and response.content[0].type == "text" else ""
+        tool_uses = [content for content in response.content if content.type == "tool_use"]


Is it guaranteed that the text always appears as the first element?

dylanholmes · 2024-05-23T18:04:27Z

griptape/artifacts/action_chunk_artifact.py

+
+
+@define
+class ActionChunkArtifact(TextArtifact, SerializableMixin):


Are you saying that you would yield an ActionArtifact after getting all the ActionChunkArtifact for that action, instead of yielding the ActionChunkArtifacts themselves? (Or would you yield both?)

vachillo · 2024-05-23T23:59:52Z

griptape/templates/tasks/tool_task/system.j2


+If you choose to execute an action, your response should be a plain JSON object that successfully validates against the schema. The schema is provided below.


not sure if the LLM cares, but the correct syntax would be JSON string not JSON object

dylanholmes

Almost got my head around everything. I think I'll need one more review though

dylanholmes · 2024-05-24T13:17:41Z

tests/unit/artifacts/test_action_chunk_artifact.py

+    def test___add__(self):
+        artifact_1 = ActionChunkArtifact("foo", tag="bar", name="baz", path="qux", partial_input="quux", index=1)
+        artifact_2 = ActionChunkArtifact("bar", tag="baz", name="qux", path="quux", partial_input="quuz", index=2)
+        added_artifact = artifact_1 + artifact_2
+        assert added_artifact.value == "foobar"
+        assert added_artifact.tag == "barbaz"
+        assert added_artifact.name == "bazqux"
+        assert added_artifact.path == "quxquux"
+        assert added_artifact.partial_input == "quuxquuz"
+        assert added_artifact.index == 1


I have a hard time understanding why this behavior is desirable... is the add operator even used anywhere?

dylanholmes · 2024-05-24T13:21:31Z

tests/unit/config/test_amazon_bedrock_structure_config.py

@@ -42,6 +42,7 @@ def test_to_dict(self, config):
                "stream": False,
                "temperature": 0.1,
                "type": "AmazonBedrockPromptDriver",
+                "use_native_tools": False,


Do we (or should we) throw an error if True is passed to a prompt driver that doesn't support native tools? (Seems like it'd be an easy change, just have that be the default behavior in base prompt driver)

dylanholmes · 2024-05-24T13:22:47Z

tests/unit/drivers/file_manager/test_amazon_s3_file_manager_driver.py

-        response = s3_client.list_objects_v2(Bucket=bucket)
-        for obj in response.get("Contents", []):
-            print(obj.get("Key"))
+        s3_client.list_objects_v2(Bucket=bucket)


dylanholmes · 2024-05-24T13:32:31Z

griptape/artifacts/action_chunk_artifact.py

+        else:
+            return ActionChunkArtifact(
+                value=self.value + other.value,
+                tag=self.tag,
+                name=self.name,
+                path=self.path,
+                partial_input=self.partial_input,
+                index=self.index,
+            )


imo action_chunk_artifact.value += "some text" is more readable than action_chunk_artifact += "some text"

dylanholmes · 2024-05-24T13:35:20Z

griptape/artifacts/action_chunk_artifact.py

+
+
+@define
+class ActionChunkArtifact(TextArtifact, SerializableMixin):


Regardless, I think moving ActionsArtifact.Action into a non-nested ActionArtifact is a good idea.

dylanholmes · 2024-05-24T14:08:57Z

griptape/drivers/prompt/openai_chat_prompt_driver.py

+                    for tool_call in tool_calls
+                ]
+
+                return ActionsArtifact(actions=actions)


Should we be passing the message like in the anthropic prompt driver (value=message.content.strip())?

dylanholmes · 2024-05-24T14:17:09Z

griptape/drivers/prompt/openai_chat_prompt_driver.py

+                        if tool_call_delta.id is not None:
+                            name, path = tool_call_delta.function.name.split("-")


Did you mean to check tool_call_delta.function.name (rather than tool_call_delta.id)?

dylanholmes · 2024-05-24T14:20:47Z

griptape/drivers/prompt/openai_chat_prompt_driver.py

+                else:
+                    content_delta = delta.content or ""


Is it ever possible for both delta.content and delta.tool_calls to be present / non-None?

dylanholmes · 2024-05-24T14:23:34Z

griptape/drivers/prompt/anthropic_prompt_driver.py

+                elif content_block_type == "tool_use":
+                    name, path = content_block.name.split("-")
+                    yield ActionChunkArtifact(
+                        value=f"{name}-{path}", index=chunk.index, tag=content_block.id, name=name, path=path


Its a little confusing to sometimes set value to the "function name" (e.g. name-path) and sometimes set it to partial_input. It feels like we want two different types of chunks.

Or maybe just add some note in the doc string about how to interpret value in the context of ActionChunkArtifact.

dylanholmes · 2024-05-24T14:32:32Z

griptape/drivers/prompt/anthropic_prompt_driver.py

+    tool_choice: dict = field(default=Factory(lambda: {"type": "auto"}), kw_only=True, metadata={"serializable": False})
+    use_native_tools: bool = field(default=True, kw_only=True, metadata={"serializable": True})


doc strings for these would be nice

collindutter · 2024-06-14T22:52:24Z

Closing in favor of #867

collindutter force-pushed the feature/native-tool-calling branch 7 times, most recently from 5205b8f to 32cd862 Compare May 21, 2024 15:47

collindutter closed this May 21, 2024

collindutter reopened this May 21, 2024

collindutter force-pushed the feature/native-tool-calling branch 5 times, most recently from e758ebe to fc63600 Compare May 22, 2024 21:29

collindutter commented May 22, 2024

View reviewed changes

collindutter force-pushed the feature/native-tool-calling branch from 72e890c to 99d2253 Compare May 22, 2024 23:05

collindutter marked this pull request as ready for review May 22, 2024 23:07

collindutter requested review from vasinov, andrewfrench and dylanholmes May 22, 2024 23:10

collindutter force-pushed the feature/native-tool-calling branch from 8fb4345 to 4766361 Compare May 23, 2024 15:43

dylanholmes reviewed May 23, 2024

View reviewed changes

collindutter force-pushed the feature/native-tool-calling branch from 278044a to d0e0bf9 Compare May 23, 2024 21:38

collindutter added 6 commits May 23, 2024 15:06

Made some progress refactoring prompt stack

3dc0deb

More work, add some tests

ea516a3

It's working

b0c60ac

Fix tests

59454ef

Remove log

effbfd5

Fix lint

ef8e067

collindutter added 16 commits May 23, 2024 15:06

Fix test

8ae0356

More changes

7adafd9

Update docstring

28e3369

Revert tmp

514f62e

Negate conditions

e9473fc

Improve test coverage

92faad1

Update docs

9fae37b

Update changelog

f1759b7

Tool task fixes

610f4b3

Fix docs

f5c0463

Clean up drivers, improve coverage

dbfc683

Add missing test

547c8dc

Handle None value

af97c43

Fix CoT cycle

0a7a87e

Fix subtask error handling

ba0d1a4

Simplify system prompt

e634b86

collindutter force-pushed the feature/native-tool-calling branch from d0e0bf9 to e634b86 Compare May 23, 2024 22:53

vachillo reviewed May 24, 2024

View reviewed changes

dylanholmes reviewed May 24, 2024

View reviewed changes

collindutter marked this pull request as draft May 24, 2024 16:41

Very broken artifact refactors

6f95ba1

collindutter mentioned this pull request May 24, 2024

Refactor/artifact chunks #806

Closed

Merge branch 'refactor/artifact-chunks' into feature/native-tool-calling

a33b782

collindutter changed the base branch from dev to refactor/artifact-chunks May 24, 2024 19:50

Fix tests

637f617

griptape-ai deleted a comment from dylanholmes Jun 13, 2024

collindutter closed this Jun 14, 2024

collindutter deleted the feature/native-tool-calling branch August 21, 2024 17:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/native tool calling #791

Feature/native tool calling #791

collindutter commented May 17, 2024 •

edited

Loading

codecov bot commented May 21, 2024 •

edited

Loading

collindutter May 22, 2024

dylanholmes May 23, 2024

dylanholmes May 24, 2024

dylanholmes May 23, 2024

dylanholmes May 23, 2024

vachillo May 23, 2024

dylanholmes left a comment

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

dylanholmes May 24, 2024

collindutter commented Jun 14, 2024



		@define
		class ActionChunkArtifact(TextArtifact, SerializableMixin):

		text = response.content[0].text if response.content and response.content[0].type == "text" else ""
		tool_uses = [content for content in response.content if content.type == "tool_use"]


		If you choose to execute an action, your response should be a plain JSON object that successfully validates against the schema. The schema is provided below.

		if tool_call_delta.id is not None:
		name, path = tool_call_delta.function.name.split("-")

		tool_choice: dict = field(default=Factory(lambda: {"type": "auto"}), kw_only=True, metadata={"serializable": False})
		use_native_tools: bool = field(default=True, kw_only=True, metadata={"serializable": True})

Feature/native tool calling #791

Feature/native tool calling #791

Conversation

collindutter commented May 17, 2024 • edited Loading

codecov bot commented May 21, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylanholmes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collindutter commented Jun 14, 2024

collindutter commented May 17, 2024 •

edited

Loading

codecov bot commented May 21, 2024 •

edited

Loading