-
Notifications
You must be signed in to change notification settings - Fork 132
Block making AI chat calls for pending statuses #877
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
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.
Pull Request Overview
This PR prevents new AI chat requests from being submitted while a previous request is still pending or streaming.
- Adds a
status
prop to chat containers to be forwarded to the input component - Introduces a shared
ChatStatus
constant inai-chat-input.tsx
and uses it to disable the send button - Removes duplicate
ChatStatus
definitions in React UI and imports the shared constant
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/ui-components/src/components/ai-chat-container/step-settings-ai-chat-container.tsx | Pass status down to the input component |
packages/ui-components/src/components/ai-chat-container/ai-chat-input.tsx | Export ChatStatus , add status prop, disable send button on pending statuses |
packages/ui-components/src/components/ai-chat-container/ai-assistant-chat-container.tsx | Pass status down to the input component |
packages/react-ui/src/app/features/builder/ai-chat/step-settings-ai-conversation.tsx | Remove local ChatStatus , import from shared UI package |
packages/react-ui/src/app/features/ai/ai-assistant-conversation.tsx | Remove local ChatStatus , import from shared UI package |
Comments suppressed due to low confidence (3)
packages/ui-components/src/components/ai-chat-container/ai-chat-input.tsx:17
- [nitpick] The prop name
status
is quite generic; consider renaming it tochatStatus
for clarity and to avoid naming collisions in parent components.
status?: string;
packages/ui-components/src/components/ai-chat-container/ai-chat-input.tsx:60
- Add unit or integration tests to verify that the send button is properly disabled when
status
isSTREAMING
orSUBMITTED
, and enabled otherwise.
disabled={
packages/ui-components/src/components/ai-chat-container/step-settings-ai-chat-container.tsx:288
- Ensure the
StepSettingsAiChatContainer
props interface includesstatus
, otherwise TypeScript will report a missing prop error when forwarding it toAiChatInput
.
status={status}
packages/ui-components/src/components/ai-chat-container/ai-chat-input.tsx
Outdated
Show resolved
Hide resolved
export type AiChatInputProps = { | ||
className?: string; | ||
placeholder?: string; | ||
status?: 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.
I think Copilot's remark is legit, I think it should not be string, but a type based on ChatStatus
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.
Done
|
Fixes OPS-1740.
https://www.loom.com/share/a4ec27b03b8e4a91ad6330322fb9ed4d?sid=29b0ad50-7e01-4a41-8277-4f01e18bc1a3