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

Toolkit Task Improvements #1268

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Toolkit Task Improvements #1268

wants to merge 6 commits into from

Conversation

collindutter
Copy link
Member

@collindutter collindutter commented Oct 17, 2024

Describe your changes

Changed

  • Updated ToolkitTask system prompt to no longer mention memory_name and artifact_namespace.
  • Models in ToolkitTask with native tool calling no longer need to provide their final answer as Answer:.

Fixed

  • Models occasionally hallucinating memory_name and artifact_namespace into Tool schemas when using ToolkitTask.
  • Models occasionally providing overly succinct final answers when using ToolkitTask.

Issue ticket number and link

https://discord.com/channels/1096466116672487547/1105169591619027055/1296270653238022226


📚 Documentation preview 📚: https://griptape--1268.org.readthedocs.build//1268/

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
griptape/tasks/actions_subtask.py 94.11% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@collindutter collindutter force-pushed the fix/toolkit branch 2 times, most recently from 89c688c to c057ceb Compare October 17, 2024 23:39
Comment on lines -183 to -185
elif not subtask.actions:
# handle case when the LLM failed to follow the ReAct prompt and didn't return a proper action
subtask.output = subtask.input
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to handle this slightly differently depending on whether we're using native tool calling or not. Moved this logic to ActionsSubtask.

Comment on lines -250 to +273
def __parse_actions(self, actions_matches: list[str]) -> None:
def __parse_actions(self, actions_matches: list[str]) -> list[ToolAction]:
if len(actions_matches) == 0:
return
return []
try:
data = actions_matches[-1]
actions_list: list[dict] = json.loads(data, strict=False)

self.actions = [self.__process_action_object(action_object) for action_object in actions_list]
return [self.__process_action_object(action_object) for action_object in actions_list]
except json.JSONDecodeError as e:
logger.exception("Subtask %s\nInvalid actions JSON: %s", self.origin_task.id, e)

self.output = ErrorArtifact(f"Actions JSON decoding error: {e}", exception=e)

return []

Copy link
Member Author

Choose a reason for hiding this comment

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

Fewer side effects, more consistent with native tool calling action parsing.

Comment on lines -32 to -34
!!! warning
`JsonSchemaRule` may break [ToolkitTask](../structures/tasks.md#toolkit-task) which relies on a specific [output token](https://github.com/griptape-ai/griptape/blob/e6a04c7b88cf9fa5d6bcf4c833ffebfab89a3258/griptape/tasks/toolkit_task.py#L28).

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically this is still true for non native tool calling, but that feels edge case enough where it's not worth mentioning.

Comment on lines -19 to -20

Actions might store their output in memory as artifacts (with `memory_name` and `artifact_namespace`). If action output is stored in memory, ALWAYS try to pass it to another action. NEVER make up memory names or artifact namespaces.
Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing the LLM to hallucinate these keys into the Tool schema. The Tool schema is descriptive enough, we don't need to mention them here.

@collindutter collindutter changed the title Fix/toolkit Toolkit Task Improvements Oct 17, 2024
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.

1 participant