Improvement ideas regarding your review request

[rabinah1]

Hello, I'm writing here some comments after receiving a review request from you via email.

Regarding the code:

I have practically zero experience in TypeScript or JavaScript (or web development in general), so properly reviewing the code is a bit difficult for me. However, one thing that I noticed was that in "speechToText/route.ts" and "AssistantButton.tsx", raw numbers are used as the response status values, e.g.

return NextResponse.json({ result: text }, { status: 200 });

I'd like to generally avoid using such "magic numbers". Maybe some descriptive variables could be defined for these, so that e.g. the above line could look something like

return NextResponse.json({ result: text }, { status: HttpOk });

Regarding the demo app:

Overall, I think the app works quite well. I experienced some problems that are already described in comments by others (e.g. regarding multilingual support), so I'll not go into those. However, I have two comments for which I didn't see existing comments:

1. When the assistant starts to process the response, there is the "thinking" notification visible, which I think is good. But if I ask a question that takes a bit longer time to process, the notification can go away clearly before the assistant starts speaking. I think it would be better if the notification stays visible until the assistant starts to speak.

2. Is there some way to "force stop" the answer while the assistant is speaking? For example, when occasionally the assistant heard my question incorrectly and it started to give an answer to something that I didn't ask, I would have wanted to just terminate the answer and try again. But now I had to wait until the assistant finishes (unless I just closed and reopened the whole page) before I could try again.

[lambrugeorge]

I will review tommorow the code ok and solving Yahoo Mail: Căutare, organizare, reușită Pe dum., dec. 1, 2024 la 23:03, Henry ***@***.***> a scris: Hello, I'm writing here some comments after receiving a review request from you via email. Regarding the code: I have practically zero experience in TypeScript or JavaScript (or web development in general), so properly reviewing the code is a bit difficult for me. However, one thing that I noticed was that in "speechToText/route.ts" and "AssistantButton.tsx", raw numbers are used as the response status values, e.g. return NextResponse.json({ result: text }, { status: 200 }); I'd like to generally avoid using such "magic numbers". Maybe some descriptive variables could be defined for these, so that e.g. the above line could look something like return NextResponse.json({ result: text }, { status: HttpOk }); Regarding the demo app: Overall, I think the app works quite well. I experienced some problems that are already described in comments by others (e.g. regarding multilingual support), so I'll not go into those. However, I have two comments for which I didn't see existing comments: - When the assistant starts to process the response, there is the "thinking" notification visible, which I think is good. But if I ask a question that takes a bit longer time to process, the notification can go away clearly before the assistant starts speaking. I think it would be better if the notification stays visible until the assistant starts to speak. - Is there some way to "force stop" the answer while the assistant is speaking? For example, when occasionally the assistant heard my question incorrectly and it started to give an answer to something that I didn't ask, I would have wanted to just terminate the answer and try again. But now I had to wait until the assistant finishes (unless I just closed and reopened the whole page) before I could try again. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[AlbertArakelyan]

Hello, while I was setting up eslint I also read the code briefly, so I will also post here comments if you don't mind @ntegrals, my whole experience is with Typescript mostly 😉