State machine for Assistant's execution phases #677

dghirardo · 2024-06-24T14:34:52Z

Issue #566

This pull request introduces a simplified state machine for managing the Assistant's execution phases. Inspired by the OpenAI Assistants model, this leaner approach enhances code readability and simplifies debugging.

Changes:

Introduced state management within the run method with possible states: in_progress, running_tools, completed, failed, and expired.
Refactored code into distinct methods following the Single Responsibility Principle (SRP), enhancing readability and ease of debugging.
Used case statements for state management to improve code clarity and avoid nested if statements.
Created a standard_role method for consistent message type handling across different LLMs, enabling case statements in process_latest_message to manage the execution flow based on the role of the last message.
Incorporated an expired status similar to OpenAI, transitioning to expired if tool output is not provided within 10 minutes to prevent application hang.
Fixed an issue where the content of a message was not included if the LLM responded with one or more tool calls.

About implementation:

After analyzing OpenAI's approach, I believe a specialized state machine gem is over-engineered for our needs. OpenAI initializes a thread and creates a job for each run, allowing state access during execution, which is ideal for asynchronous processing. In our synchronous context, a simpler state machine within the run method is sufficient, reducing complexity while maintaining clear state management.

Notes:

I have left several TODO notes in the code for further improvements and would appreciate your feedback on these points. I am open to discussing the implementation and any potential improvements you might suggest. Thank you!

andreibondarev · 2024-06-25T15:30:13Z

lib/langchain/assistants/assistant.rb


+      # TODO: Should we return the final state along with the messages?


I think it's okay to skip for now. But I think the developer should be able to access it with:

assistant.state #=> :in_progress

What do you think?

Yes, adding a state attribute to the assistant could be a valid solution without modifying the return value of the method.

A note: In synchronous execution, assistant.state will always return the final state (:completed, :failed, or :expired) because during the run method, you cannot access the attribute. Therefore, you will never see :in_progress or :running_tools. Correct?

Wouldn't you see the :running_tools (:requires_action)? For example:

irb(main):019> a.add_message content:"Latests news in politics?" => [#<Langchain::Messages::OpenAIMessage:0x000000011f1de708 @content="You are a news reporter", @role="system", @tool_call_id=nil, @tool_calls=[]>, #<Langchain::Messages::OpenAIMessage:0x00000001202b2ae8 @content="Latests news in politics?", @role="user", @tool_call_id=nil, @tool_calls=[]>] irb(main):020> a.run auto_tool_execution: false I, [2024-06-25T13:10:51.775951 #77146] INFO -- : [Langchain.rb] [Langchain::Assistant]: Sending a call to Langchain::LLM::OpenAI => [#<Langchain::Messages::OpenAIMessage:0x000000011f1de708 @content="You are a news reporter", @role="system", @tool_call_id=nil, @tool_calls=[]>, #<Langchain::Messages::OpenAIMessage:0x00000001202b2ae8 @content="Latests news in politics?", @role="user", @tool_call_id=nil, @tool_calls=[]>, #<Langchain::Messages::OpenAIMessage:0x000000012011a370 @content="", @role="assistant", @tool_call_id=nil, @tool_calls= [{"id"=>"call_MnJgY7DHwfPT1yPXRM1KRF1H", "type"=>"function", "function"=>{"name"=>"news_retriever__get_top_headlines", "arguments"=>"{\"category\":\"general\",\"q\":\"politics\",\"page_size\":5}"}}]>] irb(main):021>

At this point the state would be :running_tools (or :requires_action), correct?

No, as you can see in handle_llm_message, if auto tool execution is disabled, the return value is currently :completed. This is necessary to exit the loop (see run_finished?).

if last_message.tool_calls.any? auto_tool_execution ? :running_tools : :completed else

As written in the TODO notes, I was thinking to implement a new state (like :requires_manual_action). Or we should find a way to exit while keeping the state requires_action. What do you think?

@dghirardo My gut feeling is to simplify, so less states. Do you agree? It's much easier to add new states than to try to simplify it later.

One last question. When the assistant is initialized for the first time, what will state be set to? Nil?

Good question. Either "new" or "initialized" or "ready". What do you think?

andreibondarev · 2024-06-25T15:33:21Z

lib/langchain/assistants/assistant.rb

+      case state
+      when :in_progress
+        process_latest_message(auto_tool_execution)
+      when :running_tools


I'm wondering if we should just copy how OpenAI calls it and name it :requires_action to be more aligned with that interface?

Yes, sure, I agree!

andreibondarev · 2024-06-25T15:37:33Z

lib/langchain/assistants/assistant.rb

+      when :running_tools
+        execute_tools
+      else
+        Langchain.logger.error("Unexpected state encountered: #{state}")


What are some examples of when the execution would go here?

I removed it. There can't be any other state different from the ones we have decided.

andreibondarev · 2024-06-25T15:48:13Z

lib/langchain/assistants/assistant.rb

+    # @return [Symbol] The next state
+    def execute_tools
+      # TODO: Should we create a method parameter to let the user change the value of the tool timeout?
+      Timeout.timeout(600) { run_tools(thread.messages.last.tool_calls) }


This is 600 seconds so 10 minutes, right? I was thinking of starting very lean and not putting these kinds of guardrails just yet.

Yes, it's 10 minutes. I took inspiration from OpenAI's assistant, which does the same. If you confirm, I can remove this check and the :expired state as well.

Yep, let's get rid of it!

andreibondarev · 2024-06-25T16:40:10Z

@dghirardo First off -- thank you for this PR! 🎉❤️

I left you a few comments but I had pulled it out, tested it out, and it was looking pretty good!

andreibondarev · 2024-06-25T17:24:31Z

@dghirardo I think I just realized our slight misunderstanding here. The way I was thinking about this is:

if (LLM returns tool_calls)
  state = :requires_action
  if auto_tool_execution == true
    # We automatically execute the tools
  else
    # Return the control back to the developer

The developer has an option to execute the tools manually and submit the output if they want to, e.g.: assistant.submit_tool_output(tool_call_id:, output:)

dghirardo · 2024-06-27T15:21:52Z

Hi @andreibondarev, I just made a commit with the latest changes we discussed in the past few days. We should be good now!

andreibondarev · 2024-06-28T06:25:46Z

@dghirardo Wow, amazing work!

dghirardo added 3 commits June 24, 2024 00:36

Implemented message standard_role

122ffe8

Refactored assistant to use state machine

7ae138b

Fixed standardrb linting error

61d14cb

andreibondarev reviewed Jun 25, 2024

View reviewed changes

Updated assistant based on PR feedback

c36d1e7

andreibondarev merged commit 75b57f4 into patterns-ai-core:main Jun 28, 2024
5 checks passed

dghirardo deleted the feature/state-machine-assistant branch June 28, 2024 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

State machine for Assistant's execution phases #677

State machine for Assistant's execution phases #677

dghirardo commented Jun 24, 2024

andreibondarev Jun 25, 2024 •

edited

Loading

dghirardo Jun 25, 2024

andreibondarev Jun 25, 2024

dghirardo Jun 25, 2024

andreibondarev Jun 25, 2024

dghirardo Jun 26, 2024

andreibondarev Jun 27, 2024

andreibondarev Jun 25, 2024

dghirardo Jun 25, 2024

andreibondarev Jun 25, 2024

dghirardo Jun 26, 2024

andreibondarev Jun 25, 2024

dghirardo Jun 25, 2024

andreibondarev Jun 25, 2024

andreibondarev commented Jun 25, 2024

andreibondarev commented Jun 25, 2024 •

edited

Loading

dghirardo commented Jun 27, 2024

andreibondarev commented Jun 28, 2024


		# TODO: Should we return the final state along with the messages?

State machine for Assistant's execution phases #677

State machine for Assistant's execution phases #677

Conversation

dghirardo commented Jun 24, 2024

Issue #566

andreibondarev Jun 25, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreibondarev commented Jun 25, 2024

andreibondarev commented Jun 25, 2024 • edited Loading

dghirardo commented Jun 27, 2024

andreibondarev commented Jun 28, 2024

andreibondarev Jun 25, 2024 •

edited

Loading

andreibondarev commented Jun 25, 2024 •

edited

Loading