-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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: prompt instructions for runtimes #6943
base: main
Are you sure you want to change the base?
Conversation
openhands/utils/prompt.py
Outdated
@@ -19,6 +19,7 @@ | |||
@dataclass | |||
class RuntimeInfo: | |||
available_hosts: dict[str, int] | |||
deduce_all_web_hosts: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being dense, what is the use case for deduce_all_web_hosts=False
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defaults to False because other runtimes might not allow exposing any port for serving applications and might only allow a fixed number of them which they defined in web_hosts
.
For Daytona, we want to instruct the agent to change any occurance of "sending" the user to a port to send the user to our preview URL instead.
This was the way I tried interfacing the runtimes, but I since got a suggestion to just add a runtime_instructions
property allowing runtimes to define any custom instructions they want. I'll update the PR soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other runtimes might not allow exposing any port for serving applications
Then they can set the list of ports to an empty list, and the template will not display anything, right? I'm sorry, I still don't see why that needs an additional boolean, but I'm looking forward to the updates, don't worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, but if a runtime were to set web_hosts
3000 and 3001 -> localhost, and those were the only endpoints open for access, I believe asking the LLM to use port 6000 would still return "localhost:6000" if deduce_all_web_hosts
were true, while in reality the runtime might not want the LLM to assume those ports as well
I pushed the new additional_instructions
property - let me know whether you think I used proper naming
Signed-off-by: Ivan Dagelic <[email protected]>
71a3334
to
d245561
Compare
End-user friendly description of the problem this fixes or functionality that this introduces.
Tells the agent which URLs to use when responding with links for reaching running apps on Daytona.
Give a summary of what the PR does, explaining any non-trivial design decisions.
This PR adds an
additional_instructions
property to runtimes which then gets forwarded to thePromptManager
.For Daytona, it is used to pass this message:
which shows the user a correct link to access the preview URL for a running application.
Link of any specific issues this addresses.
Discussed briefly in #6863