fix(bidi): respect role from BidiTextInputEvent instead of hardcoding user#2083
fix(bidi): respect role from BidiTextInputEvent instead of hardcoding user#2083btdeviant wants to merge 4 commits intostrands-agents:mainfrom
Conversation
… user BidiTextInputEvent carries a role field but the bidi loop hardcoded "user" when appending to message history. This prevented injecting context/system messages with role="assistant" via GatewayBidiInput.
Changed versioning to dynamic based on git tags.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
|
||
| if isinstance(event, BidiTextInputEvent): | ||
| message: Message = {"role": "user", "content": [{"text": event.text}]} | ||
| message: Message = {"role": event.role, "content": [{"text": event.text}]} |
There was a problem hiding this comment.
Can you elaborate a bit more on the use case here? Note that we do allow you to prefill the initial message history that is sent to the model with assistant messages (Agent(messages=...)). They are sent separately in start(). Curious though what it would mean for the user to send an assistant message to the model in the middle of a conversation.
There was a problem hiding this comment.
I also ran some tests against 2 models (1 from google and 1 from openai). Google rejected the assistant message with an error response and openai seemed to ignore it.
Note, there is also some hardcoding of roles in the model providers themselves that I had to update for the tests.
There was a problem hiding this comment.
@pgrayy good catch on the Google test. Admittedly I'm using a custom adapter for an openai api compatible server (sglang / llama.cpp), and at the risk of being a bit presumptuous, I take your point that this PR may be premature given the official supported clients for BidiAgent are limited.
The use case it addresses is pushing context from async process as an input source, which through some other magic elicits unprompted engagement from the users perspective. A bit atypical I suppose. I can monkey patch around it, not a big deal, just took a bit of debugging to find out why assistant wasn't making it to the model - was a bit of a bear trap :)
Motivation
The bidi agent loop hardcodes
"role": "user"when appending messages from aBidiTextInputEvent, ignoring the event'sroleproperty entirely. This means callers who construct aBidiTextInputEventwithrole="assistant"(e.g., for injecting assistant context into the conversation history) have that role silently overwritten. Theroleparameter onBidiTextInputEventexists specifically to support this — it should be used.Related Issues
None (no existing upstream issue for this bug).
Documentation PR
N/A — no documentation changes needed.
Type of Change
Bug fix
Public API Changes
No public API changes.
BidiTextInputEventalready accepts aroleparameter — this fix ensures the agent loop honors it.Testing
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.