-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix: Enhance prompt overwrites both edit and new message boxes #8554
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
base: main
Are you sure you want to change the base?
Conversation
…e boxes - Add context parameter to enhance prompt messages to distinguish between main and edit mode - Update ChatTextArea to only respond to enhance prompt messages matching its context - Add comprehensive test coverage for context-specific behavior Fixes #8553
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.
Self-review: reviewing my own code like a hall of mirrors for robots—still cheaper than therapy.
| // Only handle the enhanced prompt if it's for this specific textarea instance | ||
| // Edit mode textareas have context "edit", main textarea has context "main" | ||
| const expectedContext = isEditMode ? "edit" : "main" | ||
| if (message.context === expectedContext) { |
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.
[P2] Backward-compatibility edge case: if older backends emit 'enhancedPrompt' without a context field, this instance will ignore the event and the spinner may never clear. Consider defaulting undefined context to 'main' so the main textarea still handles legacy messages.
Example:
const expectedContext = isEditMode ? "edit" : "main"
const incomingContext = message.context ?? "main"
if (incomingContext === expectedContext) {
// ...
}| }) | ||
| }) | ||
|
|
||
| describe("enhanced prompt response", () => { |
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.
[P2] Missing error-path coverage: when an enhancedPrompt arrives with matching context but no text (error response), the component should still stop the loading spinner. Add a test asserting setIsEnhancingPrompt(false) is called (or the UI state reflects it) when message.text is undefined.
| window.addEventListener("message", messageHandler) | ||
| return () => window.removeEventListener("message", messageHandler) | ||
| }, [setInputValue, searchRequestId, inputValue]) | ||
| }, [setInputValue, searchRequestId, inputValue, isEditMode]) |
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.
[P3] Performance: the message event listener re-subscribes on every inputValue change due to being in a useEffect with inputValue in deps. This can be avoided by using a ref for latest inputValue or the existing useEvent hook (already imported) so the handler stays stable while reading current state via refs, reducing listener churn.
|
Fix tested and works, did not review code |
This PR attempts to address Issue #8553 where the "enhance prompt" feature incorrectly overwrites both the edit message box and the new message box when editing a message.
Problem
When a user is editing a message and uses the enhance prompt feature, the enhanced text appears in both:
Solution
Added a context parameter to distinguish between different ChatTextArea instances:
contextfield in enhanced prompt response messagesChanges
webviewMessageHandler.tsto include context from request in responseChatTextArea.tsxto send context and filter incoming messagesTesting
Fixes #8553
Feedback and guidance are welcome!
Important
Fixes 'enhance prompt' feature to correctly target only the intended message box by using context parameters.
contextparameter.ChatTextAreainstances now send their context ('main' or 'edit') when requesting enhancement.webviewMessageHandler.tsto include context in enhanced prompt responses.ChatTextArea.tsxto send context and filter messages based on context.ChatTextArea.spec.tsxto verify context-specific behavior.This description was created by
for 994e2a2. You can customize this summary. It will automatically update as commits are pushed.