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: patch missing inner thoughts on new openai models #1562

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

cpacker
Copy link
Collaborator

@cpacker cpacker commented Jul 23, 2024

TODOs:

Refactor code to convert to inner thought kwarg format

  • Add utility method inside of data types to convert Message objects
    • See note*
    • Add utility to convert ToolCalls into versions that have a kwarg included (add_inner_thoughts_to_tool_call)
    • Add flag put_inner_thoughts_in_kwargs to Message.to_openai_dict which will call utility to make put self.text (the content / inner thoughts) into ToolCalls via the utility
  • Add utility method to convert functions list / tool objects (will require dumping and reading json strings of arguments)
    • add_inner_thoughts_to_functions
  • Add utility method to convert response from OpenAI response to strip inner thoughts from kwargs and into the content field for data storage
    • unpack_inner_thoughts_from_kwargs
  • Add flag to CLI to allow turning this on manually
    • I added a --no-content flag (yes = put inner thoughts in kwargs, no = leave as normal (gpt4 style), default = let us decide, eg for gpt4o turn it on, for gpt4 turn it off)
  • TODO future PR - add support for our other API backends

*note: I was going to create a Message -> Message utility function that puts self.text into self.tool_calls (as an alternative to putting the logic for extracting inner thoughts from self.text into the function/tool call in the to_*_dict() methods), however upon further thought I think it's better to leave this logic inside the _to_dict() methods because it might vary significantly from provider to provider. Example of what I was going to do:

# TODO have this use model_copy
def put_inner_thoughts_in_kwargs(message: Message) -> Message:
    """Convert a message that has inner monologue in self.text to one that has it inside a tool call"""
    # message_copy = message.model_copy()
    message_copy = copy.deepcopy(message)

    # TODO
    # step 1: assert the role is assistant and tool_calls > 1
    # step 2: copy self.text into self.tool_calls[0], and make self.text = None
    # step 3: return the message copy

Please describe the purpose of this pull request.

Fixes issues with missing content / inner thoughts on newer OpenAI models like 4o-mini

How to test

memgpt run with gpt-4o-mini

Have you tested this PR?

Yes, gpt4o-mini works fine now:
image

Related issues or PRs

Closes #1555

@cpacker cpacker marked this pull request as ready for review July 24, 2024 00:03
@cpacker cpacker requested a review from sarahwooders July 24, 2024 01:10
Copy link
Collaborator

@sarahwooders sarahwooders left a comment

Choose a reason for hiding this comment

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

lgtm!

memgpt/cli/cli.py Show resolved Hide resolved
@cpacker cpacker merged commit 477867f into main Jul 24, 2024
15 checks passed
@cpacker cpacker deleted the fix-inner-thoughts branch July 24, 2024 02:51
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.

calling the/app/agents/id/messages interface never returns a result, suspected to be stuck in a dead loop
2 participants