-
Notifications
You must be signed in to change notification settings - Fork 3
feat: allow conversational agents to optionally end exchange [JAR-9933] #127
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
Changes from 1 commit
f94398c
fb37dc9
a9e4c35
5af927e
c44218b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,16 +35,20 @@ def __init__( | |
| self, | ||
| delegate: UiPathRuntimeProtocol, | ||
| chat_bridge: UiPathChatProtocol, | ||
| end_exchange: bool = True, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't add this here - the event needs to be emitted, whether you honor it or not, that should be in the bridge implementation
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andrewwan-uipath uipath-runtime is a very low-level abstraction. CAS/Maestro-specific logic belongs in the implementation layer - it shouldn't be pushed down into the underlying abstractions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, I will move this to the implementation layer
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cristipufu [Not-blocking] But wanted to ask: This new feature is to ensure the agent-runtime doesn't always end the exchange (so downstream agents/messages that have more messages to add can end the exchange themselves). This means that for Flow, we can call a conversational agent without having it emit end-exchange event, so that the chat-UI still shows "responding..." rather than that the turn was over. so my intuition is saying to use the original approach where the runtime-layer doesn't call the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the runtime's event contract changes based on a constructor arg, sometimes it ends the exchange, sometimes it doesn't, so you can't reason about what events come out of the runtime without knowing how it was built. The "don't end the turn so the UI keeps showing responding…" logic is pure CAS/Flow orchestration concern, and we're pushing it down into a low-level abstraction that shouldn't know Flow exists.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, thank you @cristipufu ! I guess its more of a naming thing then - I'm good to get this method (extracted to bridge layer) merged.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the other PR: UiPath/uipath-python#1720 |
||
| ): | ||
| """Initialize the UiPathChatRuntime. | ||
|
|
||
| Args: | ||
| delegate: The underlying runtime to wrap | ||
| chat_bridge: Bridge for chat event communication | ||
| end_exchange: Whether to emit the exchange end event. When False, the exchange is left open so a | ||
| downstream consumer can continue it and end it later. | ||
| """ | ||
| super().__init__() | ||
| self.delegate = delegate | ||
| self.chat_bridge = chat_bridge | ||
| self.end_exchange = end_exchange | ||
|
|
||
| async def execute( | ||
| self, | ||
|
|
@@ -167,7 +171,8 @@ async def stream( | |
| else: | ||
| yield event | ||
| execution_completed = True | ||
| await self.chat_bridge.emit_exchange_end_event() | ||
| if self.end_exchange: | ||
| await self.chat_bridge.emit_exchange_end_event() | ||
| else: | ||
| yield event | ||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I think we can get away with a patch increase to avoid incrementing the ranges in all the repos