-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add review command for pre-PR implementation validation #7864
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
- Modified handleSendMessage to not clear input value when queueing - Only clear selected images when queueing a message - Input will be cleared when message is actually sent, not when queued - Fixes #7861 where unsent text was lost when queued messages were processed
- Add comprehensive review command to built-in commands - Supports reviewing against GitHub issues, Slack comments, and GitHub comments - Includes workflow for analyzing code changes against requirements - Provides confidence scoring and actionable recommendations - Add comprehensive test coverage for the new command
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.
Reviewing my own code feels like debugging in a mirror - everything's backwards and I still can't find the problem.
| Create a todo list to track the review workflow: | ||
| <!-- TOOL CALL --> | ||
| \`\`\` |
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.
The review command content uses escaped backticks within template literals, which could be confusing to maintain. Have you considered using a different delimiter or storing the command content in a separate file for better readability?
| name: "review", | ||
| description: "Review implementation changes against original requirements before creating a pull request", | ||
| argumentHint: "[context]", | ||
| content: `<task> |
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.
This command definition is ~500 lines embedded directly in the object. Would it make sense to extract this to a separate file or constant? It would make the main file easier to navigate and the command content easier to maintain.
| console.log("queueMessage", text, images) | ||
| vscode.postMessage({ type: "queueMessage", text, images }) | ||
| setInputValue("") | ||
| // Don't clear the input when queueing - user may be typing more |
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.
Is this change intentional for the review command feature? The modification to not clear input when queueing messages seems unrelated to adding the review command. Could this be part of a separate PR, or should it be documented in the PR description why it's needed?
| expect(content).toContain("rules-architect") | ||
| }) | ||
|
|
||
| it("review command should have comprehensive content", async () => { |
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.
The test coverage looks good for verifying the command exists and has expected properties. Have you considered adding tests for the actual command execution workflow or edge cases (like malformed input handling)?
|
Closing for now |
This PR adds a comprehensive review command to the built-in commands that helps validate implementation changes against original requirements before creating a pull request.
Changes
reviewcommand to built-in commandsgithub issue owner/repo #123)slack comment: message)github comment: message)Testing
Type of Change
Checklist
Fixes the issue mentioned in the GitHub comment requesting the review command implementation.
Important
Adds
reviewcommand for pre-PR validation of implementation changes against requirements, with comprehensive testing and minor UI adjustments.reviewcommand to validate implementation changes against requirements before PR creation.built-in-commands.spec.tsto include tests forreviewcommand.ChatView.tsxto not clear input when queueing messages.This description was created by
for 0bd6bd3. You can customize this summary. It will automatically update as commits are pushed.