-
Notifications
You must be signed in to change notification settings - Fork 135
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
Refactoring プロンプト周り & 会話履歴クリックの挙動 & クリアボタンの挙動 #119
Conversation
fix: clear old chat state
上記の操作をすると、「Drawerでは会話Aが選択されたまま」「会話BがDrawerに追加される(未選択状態)」「会話Aはデータ上消えておらず再度開くと見ることができる」という状態になり、直感的とは言いづらい挙動かなと思いました。 「最初からやり直す」という文言がわかりづらい要因な気がするので、「新規チャットであること」がわかるような表示名に変えた方がいいかなと思いました。 |
id から匿名で引きたいのは systemContext だけで、generatePrompt の方は匿名である必要はないので generics は不要として削除しました!
端的に言うと、Prompt を作成したい人と、systemContext が欲しい人が違うということですね。 |
packages/web/src/hooks/useChat.ts
Outdated
if (!chatId && systemContext) { | ||
init(id, systemContext); | ||
if (!chatId) { | ||
init(id); | ||
} | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
既存バグな気がしますが、depends に id
と chatId
がないと以下の操作でエラーになります。
(会話履歴 -> 新規チャット の遷移時に初期化が行われない)
会話履歴で任意の会話を開く -> ブラウザを更新(URLリンク等で直接会話履歴を開いたことを想定) -> 「チャット」メニューで新規チャットを開く -> 適当にメッセージを送信 ->
chats
が初期されていないためエラー
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あざます!かなり exhaustive-deps が悪さしているのでどっかのタイミングで全部外した方が良いかもしれないですね。
元々は #90 に commit 被せる予定でしたが、ほぼ原型を留めていないので新規で出します。
関連 Issue
Prompt の形式を統一して、id で PromptGenerator を引けるようにしました。
これにより、init する際に systemContext は必要なくなりました。
また、動作としては、以下の変更点を入れてあります。
動作確認メモ
A ... デモを実行して、別のユースケースをクリックし、戻ってきても画面 & 会話履歴はリセットされていない
B ... デモを実行して、同じ chatId を持つ会話履歴をクリックし、戻ってきて更新すると別の会話履歴として保存される
C ... デモを実行して、クリアボタンを押して更新すると、別の会話履歴に保存される