Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 1, 2025

Fixes #6549

Summary

This PR restores the functionality where comments typed before clicking the Save button are properly consumed and sent to the LLM. This was a regression that prevented user comments from being included when approving file changes.

Changes

  • Modified handlePrimaryButtonClick in ChatView.tsx to always include text and images in the askResponse message
  • Changed the logic to send undefined for empty values instead of conditionally excluding them
  • This ensures the extension properly handles any comments the user typed before clicking Save

Testing

  • All existing tests pass
  • Manually tested that comments are now properly consumed when clicking Save during file approval operations

Impact

Users can once again add comments when approving file changes, which will be sent to the LLM for processing.


Important

Fixes regression in ChatView.tsx to ensure comments are sent to LLM when Save is clicked, by always including text and images in askResponse.

  • Behavior:
    • Fixes regression in handlePrimaryButtonClick in ChatView.tsx to ensure comments are sent to LLM when Save is clicked.
    • Always includes text and images in askResponse message, sending undefined for empty values.
  • Testing:
    • All existing tests pass.
    • Manually verified comments are consumed when clicking Save during file approval.

This description was created by Ellipsis for 0d0cab8. You can customize this summary. It will automatically update as commits are pushed.

- Modified handlePrimaryButtonClick to always include text/images in askResponse
- Ensures comments typed before clicking Save are properly consumed and sent to LLM
- Fixes regression where comments were being ignored during file approval

Fixes #6549
@roomote roomote bot requested review from cte, jr and mrubens as code owners August 1, 2025 14:09
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Aug 1, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed my own code. Found it suspiciously free of my usual chaos. I've reviewed the changes and have some suggestions for improvement.

} else {
vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" })
}
// Always send the message with text/images if they exist
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case to prevent future regressions. Specifically, a test that verifies comments are included in the askResponse when the Save button is clicked would help ensure this functionality doesn't break again.

vscode.postMessage({
type: "askResponse",
askResponse: "yesButtonClicked",
text: trimmedInput || undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, consider using the same validation pattern for both text and images. Currently checking but only without a length check. Could we use:

} else {
vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" })
}
// Always send the message with text/images if they exist
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment could be more specific about why we're always sending the message. Consider:

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 1, 2025
@daniel-lxs
Copy link
Member

The check shouldn't interfere with sending actual string and images, closing for now

@daniel-lxs daniel-lxs closed this Aug 2, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Aug 2, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Regression: Does not apply comments when saving

4 participants