feat: add /x/acpio package by johnstcn · Pull Request #188 · coder/agentapi
Comment on lines +107 to +114
| Id: len(c.messages), | ||
| Role: st.ConversationRoleUser, | ||
| Message: message, | ||
| Time: c.clock.Now(), | ||
| }) | ||
| // Add placeholder for streaming agent response | ||
| c.messages = append(c.messages, st.ConversationMessage{ | ||
| Id: len(c.messages), |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have logic for removing placeholder messages I'd like to confirm something: Do we ever return placeholders messages to the user? If we do we might end up re-using an ID and that could be a little funky.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC it's used in the UI on initial send.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, if so, we should probably not re-use the ID within a conversation. Our react frontend will likely key={message.id} and this could cause some funky behavior.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we instead mutate the placeholder message content?
The alternative is that we have a gap in the monotonically increasing message IDs, which may not be an issue?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of any issues with having a gap (we probably shouldn't use IDs for indexing anyways)