(refactor|test)(frontend): chat #1488
Conversation
new store, and utilize new reducers
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1488 +/- ##
=======================================
Coverage ? 60.74%
=======================================
Files ? 87
Lines ? 3709
Branches ? 0
=======================================
Hits ? 2253
Misses ? 1456
Partials ? 0 ☔ View full report in Codecov by Sentry. |
| function ActionBanner() { | ||
| return ( | ||
| <div | ||
| data-testid="typing" | ||
| className="flex items-center justify-center gap-2 bg-neutral-700 border-y border-neutral-500 py-1.5 px-4" | ||
| > | ||
| <div className="flex h-5 w-5 items-center justify-center" /> | ||
| <p className="text-sm text-gray-200 dark:text-gray-200">Working...</p> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
@openartist can you take a look at this? I like the typing animation better. Now we have the messages popping in with this banner that seems a bit out of place.
There was a problem hiding this comment.
There's a design in the figma for a three dot animation appearing in the bubble before text appears. @Sparkier let me know if you have time to review some options. I agree that even if we aren't streaming the AI response the typewriter effect gives a greater sense of responsiveness—perhaps we can just refine it a bit?
| @@ -1,79 +0,0 @@ | |||
| import { useEffect, useState } from "react"; | |||
There was a problem hiding this comment.
As mentioned, think this was a better indicator of work. Could even do something like ... while the agent is thinking.
There was a problem hiding this comment.
Yes, my alternative thought to the action banner was the .... The banner looked better in my head because I was also imagining a similar banner could be used to indicate what tools the agent is using or process (such as install status) indicators.
I'm not fully convinced of my current implementation either, but it does make up for it in fast responses, which is why I was searching for an alternative to the typing effect in the first place
|
Until there is any other better suggestion, I'll go ahead and test/implement typing effect as previous (albeit slower 😔) |
Also other people might have different opinions, please chime in! (@rbren @yimothysu @openartist) |
|
New changes feature both, typing effect + banner (until relevant party members share opinion). Whatever decision, we'll go for it.
|
I personally prefer to omit the typing effect because it gives the impression that we're streaming when we're not. IMO a loading animation (such as the one used by Gemini) is sufficient, but either way this looks great! |
Summary
Test and rebuild the chat UI and create a new chat slice to accompany it.
Motivation
The
<ChatInterface>component looks to be one of the oldest components that has been built since the initial versions of the UI and business logic. The components and children use and reuse some Redux state values, confusing CSS, and is generally difficult to read through.Additionally, the chat slice has methods and properties that are not being used at all, such as the separation of
assistantMessagesanduserMessages(which anyways could be obtained frommessages). It also mixes simple chat logic with UI-dependent logic (typing effect).Todo
useTypingEffecthookThoughts
It may also be worth considering to remove the typing effect altogether. In existing applications built around LLMs, they "stream" the results generating by the LLM. This allows a relatively fast and responsive-feeling compared to outputting the entire generated response, which takes longer.
The way OpenDevin is currently setup, we are giving the illusion of streaming, where we first obtain the fully generated response, then forcefully type it out via the
useTypingEffecthook.!!This actually ends up taking the longest compared to steaming OR outputting full generated response!!
Take this opportunity to switch to this PR's branch, and see for yourself how fast OpenDevin feels without the constraint on typing.
I can't say the extent of work required to allow for streaming from the backend, also given that not all models support it. But I suppose it is worth the discussion.
(We can also drop it entirely for the time being unless we really want to look like we are streaming)
By default, I will complete this draft with the current behavior of the chat and typing effect so it would be better tested, simplified, and readable. If there are other suggestions, I'd be glad to pursue them.Related issues
#1399 - Markdown is not applied to messages being typed, this is because the chat interface has two different components for rendering messages, one for already existing, and one for messages still being typed. The
<Markdown>component was only applied to the former (easy to miss). This PR will attempt to resolve this by only serving one component, typing or not, to render messages.