Skip to content
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

Refactor messages #13

Merged
merged 6 commits into from
Jul 24, 2024
Merged

Refactor messages #13

merged 6 commits into from
Jul 24, 2024

Conversation

g-francesca
Copy link
Contributor

@g-francesca g-francesca commented Jul 24, 2024

Here are the key points covered in the PR:

  • Ref ORM-1537: The abort button is now visible while the message is streaming.
  • Ref ORM-1536: Quick actions are now visible as soon as the message streaming completes. The regenerate button is visible only for the last item, and it runs the regenerateLast function provided by the orama client. We are currently not getting answers as an array of strings, so we always show the last regenerated answer.
  • Ref ORM-1547: Hardcoded sources have been replaced with the real ones. However, there are still some points to improve/fix:
    • The path is not absolute, so we may want to pass a baseUrl to make the link work.
    • The sources documents have different data structures, so we need to create some mapping (similar to what we did with resultsMapping) to display the correct fields.
    • I noticed that sometimes we get an answer like 'I'm not able to answer', but still get sources. I think we should not get any sources in this case.

I also did some refactoring to the chatContext state, so it now only includes:

const { state: chatContext } = createStore({
  chatService: null as ChatService | null,
  interactions: [] as TChatInteraction[],
})

I removed messages in favor of interactions, and the status is now handled as a single interaction status (not a global one). This way, updating the UI according to the answer status (which. is what we mostly do) becomes simpler. The type of a single interaction is:

export type TChatInteraction = {
  query: string
  response?: string
  sources?: TSource[]
  status: TAnswerStatus
  interactionId?: string
}

Which is closer to the status we get from the server. These types should be imported by the client for consistency though

Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orama-ui-components-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2024 9:43pm

Copy link

linear bot commented Jul 24, 2024

ORM-1537 Stop answer button - fix

Currently, we only allow aborting the answer when the message is loading, not when it's updating. Therefore, if the answer is too long, we can't stop it

ORM-1547 Implement sources

Replace hardcoded sources with the real ones

ORM-1536 Quick actions fixes

  • Quick actions must be displayed only on message complete ✅
  • Add aria-label to buttons to make them accessible ✅
  • implement regenerate logic and make sure to handle error if regenerate ✅
  • Regenerate should be visible only for the last item ✅

Copy link
Collaborator

@rjborba rjborba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🔥🔥🔥

)
}

if (this.interaction.response) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should do a earlier return here so we do not have everything wrapped in a If:

if(!this.interaction.reponse) {
    return;
}

}
})

console.log('latest state', latestState)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we leave it here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for spotting 😺

@rjborba rjborba merged commit 5b09f10 into main Jul 24, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants