fix(react): Use full attributes instead of changed ones for agent#1307
fix(react): Use full attributes instead of changed ones for agent#1307chenghao-mou wants to merge 4 commits intomainfrom
Conversation
it can drop agent state and cause chat panel disappear in the playground. This fixes that.
🦋 Changeset detectedLatest commit: 33de417 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
size-limit report 📦
|
1egoman
left a comment
There was a problem hiding this comment.
Left a quick note - if this is urgent / you need to get it out to fix a live issue, I think I'd be ok merging it in its current state. But I also don't want to let potential issues like this go uninvestigated.
packages/react/src/hooks/useAgent.ts
Outdated
There was a problem hiding this comment.
FWIW, I think there might be a simpler way to make this change, swap the whole useEffect part for something like this:
const agentParticipantAttributes = agentParticipant.attributes;But also, I just want to make sure I understand the reason behind this change - can you outline a sequence where agentParticipant.attributes changes but the value passed as part of the ParticipantEvent.AttributesChanged event is out of date?
For context, the reason why I'm asking is agentParticipant comes from here, which is a filtered version of the return value of the useRemoteParticipants hook, which (via connectedParticipantsObserver) is emitting state updates when any of these events emit, not just ParticipantEvent.AttributesChanged.
So I'm a little worried this fix might be covering up an issue deeper in the stack where participant attributes are updated the ParticipantEvent.AttributesChanged isn't being emitted.
There was a problem hiding this comment.
the ParticipantEvent.AttributesChanged event contains the latest changed attributes for a participant, not the full list of attributes. e.g. when the Python agent code runs this following code:
ctx.room.local_participant.set_attributes(
{"metrics": json.dumps(asdict(stats))}
)the received input is
{"setStateTo":{"metrics":"{\"stt_latency\": 338, \"llm_ttft\": 466, \"tts_ttfb\": 257, \"eot_latency\": 352, \"total_latency\": 1255, \"user_speaking_timestamp\": 1773316125.461067, \"false_interruption_count\": 0, \"interruption_count\": 0, \"vad_interruption_count\": 0}"}}
with no lk.agent.state or any other previously already set attributes. setAgentParticipantAttributes is then overriding the entire participant attributes with only the changed values. The ParticipantEvent.AttributesChanged event is working as intended with the latest changes. It is more about how we handle the "delta" with setAgentParticipantAttributes.
In our playground chat panel, its mounting is tied to the lk.agent.state attribute, so it will disappear whenever the change event is related to something other that the speaking/listening state change (like the above mentioned metrics update).
There was a problem hiding this comment.
Got it! Ok, so then if what you are reporting is correct, this is a bug in the original implementation.
If it wouldn't be much trouble, do you think you'd be able to test my proposed alternate fix (ie, get rid of the useEffect outright and just use the already reactive agentParticipant.attributes value)? If that works I think given what you are reporting it would be a little cleaner / match a bit closer to I think the original intended logic.
Anyway. given this new information, I think what you have here makes sense. I will add my approval! ✅
1egoman
left a comment
There was a problem hiding this comment.
Prior to merging if you could add a changeset that would be appreciated!
It can drop agent state and cause chat panel disappear in the playground. This fixes that by always using the latest participant attributes.