-
Notifications
You must be signed in to change notification settings - Fork 237
システムコンテキストの保存機能 #416
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
システムコンテキストの保存機能 #416
Conversation
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.
- sendIconEnabled のバグの影響だと思いますが、現状は「システムコンテキストを保存せずに変更する」ということができませんよね?これはデグレです。
- 「プロンプト例」の中に「システムコンテキスト」があるのが不自然という指摘が直っていないです!「プロンプト例」と同じように「システムコンテキスト」を扱ってください!また「システムコンテキスト」だけだと意味が伝わらないと思うので、「保存したシステムコンテキスト」等にしてください!
28ac1d3
to
f054390
Compare
<SystemContextItem | ||
systemContextTitle={item.systemContextTitle} | ||
systemContext={item.systemContext} | ||
key={`${i}`} |
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.
これ key がないとエラーになりますか?(なくても問題ないように見えました)
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.
ありがとうございます!
不要でした 🙏
packages/web/src/prompts/index.ts
Outdated
systemContextTitle: string; | ||
systemContext: string; | ||
systemContextId: string; | ||
}; |
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.
PrimaryKey があっても特に困らないのであれば types の SystemContext を使いまわした方がシンプルで良いと思いました!
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.
レビューありがとうございます!
使い回す形で修正しました!
return null; | ||
} | ||
return _systemContextId.split('#')[1]; | ||
}; |
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.
実装が chatId と全く同じだと思うので、decomposeChatId を decomposeId とかに rename して使いまわした方が良いと思います!
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.
レビューありがとうございます!使い回すように変更しました!
|
||
type Props = BaseProps & { | ||
onClick: (params: ChatPageQueryParams) => void; | ||
systemContextListItem: SystemContextListItem[]; | ||
onClickDeleteSystemContext: (params: string, index: number) => void; |
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.
ここは引数の名前が params ではないですよね?
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.
修正しました!
packages/cdk/cdk.json
Outdated
@@ -41,7 +41,7 @@ | |||
"allowedIpV4AddressRanges": null, | |||
"allowedIpV6AddressRanges": null, | |||
"allowedCountryCodes": null, | |||
"dashboard": false, | |||
"dashboard": true, |
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.
cdk.json がコミットされています!
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.
ご指摘ありがとうございます 🙏
修正しました!
d99d745
to
3f8edd2
Compare
Issue #281 , if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.