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

Add initial user msg to /new_conversation route #6314

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Conversation

malhotra5
Copy link
Contributor

@malhotra5 malhotra5 commented Jan 16, 2025

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions

Adding an initial user_msg field to /new_conversation route; enables new_conversation to be called elsewhere and send an initial user msg without waiting for the agent status to be INITIALIZED


Link of any specific issues this addresses


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:eaefc33-nikolaik   --name openhands-app-eaefc33   docker.all-hands.dev/all-hands-ai/openhands:eaefc33

@malhotra5 malhotra5 marked this pull request as draft January 16, 2025 18:13
@malhotra5 malhotra5 changed the title Feat: Allow user to queue user messages before agent is initialized Add initial user msg to /new_conversation route Jan 16, 2025
@malhotra5 malhotra5 marked this pull request as ready for review January 17, 2025 03:03
if initial_user_msg:
self.event_stream.add_event(
MessageAction(content=initial_user_msg), EventSource.USER
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assumed the purpose of this PR is to move towards sending a user message before the runtime is initialized, but here it isn't. The PR description also said that it doesn't wait for init, but the change action to INIT did happen just before this.

Can you please explain a bit what does the PR aim to achieve, what is before and after?

Copy link
Contributor Author

@malhotra5 malhotra5 Jan 17, 2025

Choose a reason for hiding this comment

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

Ah apologies for the description. I want to make it so that when requesting a new conversation, I also have the ability to specify an initial message the agent should run as soon as it is initialized

Expected behavior

  1. Request a new convo with initial message
  2. Wait to the agent to initialize
  3. Drop the message in the event stream right after initialization is done

Areas to be used

Case 1

image

In the image above, "build the app to view pull requests" is an initial user instruction. However, currently the frontend waits for the agent to be initialized before sending the instruction to the event stream. This change would allow the frontend to request a new conversation and include the initial user instruction together.

  1. Frontend doesn't have to explicitly wait for agent to be initialized
  2. Backend makes sure the agent is initialized when its first instruction is received

Case 2

When creating the Github app for the resolver, I want to be able to spin up a new conversation with initial instructions

Note: this is emulating the same behavior of initial_user_action when running the controller

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! I see, that makes sense. Actually, I'm also doing a similar thing for cli.py here 😅

On a side note, I'm particularly looking forward to Case 2! I wonder how that will look like, if it will head towards making the new conversation backend more backend-y.

Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

The code LGTM. It seems doing this requires a lot of passing the little string around until it gets into a MessageAction, but I guess it's not avoidable.

Just a thought: the frontend sometimes "eats" user messages, somehow it doesn't show them later. I wonder if it handles this one well, IMHO it's worth testing with a restored session, to see if it does get it and display it.

@malhotra5 malhotra5 enabled auto-merge (squash) January 17, 2025 14:20
@malhotra5 malhotra5 merged commit 000055b into main Jan 17, 2025
13 checks passed
@malhotra5 malhotra5 deleted the fix/pull-queued-msgs branch January 17, 2025 14:43
csmith49 pushed a commit to csmith49/OpenHands that referenced this pull request Jan 19, 2025
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.

4 participants