-
Notifications
You must be signed in to change notification settings - Fork 0
More Human Like Reviews #29
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
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.
❇️ CodePress Review Summary
👋 Hey team,
Overall the changes look solid, but I spotted 1 must-fix issue and left 1 helpful note inline.
Here's the quick rundown:
❌ Decision: REQUEST CHANGES
Undefined references and stub implementations must be addressed, and performance/test coverage improvements are required before merging.
🚧 Needs a bit of love
The required finding highlights a critical issue where unescaped XML tags in instruction strings can corrupt the reviewRequest payload. Embedding raw <resolvedComments> tags without proper escaping breaks the expected XML format, potentially causing downstream processing failures. Ensuring that XML special characters are escaped or avoiding direct inclusion of tag syntax in instruction text will maintain payload integrity and prevent parsing errors.
ℹ️ Other thoughts
The nit-level observation points to readability improvements in the illustrative JSON examples, noting that escaped quotes detract from clarity. Since these blocks serve as documentation rather than executable code, presenting them as raw, well-formatted JSON would be more user-friendly. Adopting this change enhances the maintainability and comprehension of the guidelines without impacting functionality.
| Group similar issues into comprehensive feedback. | ||
| </step5> | ||
| **Tool Usage Guidelines**: |
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.
🔵 NIT: The example JSON in the Tool Usage Guidelines still escapes quotes (e.g., \"domain\"). Consider using raw JSON formatting (without backslashes) for readability, since this block is illustrative rather than code that’s parsed literally.
| 5. **Prioritize Impact**: Focus on issues that would genuinely help the developer improve the codebase. | ||
| ${existingComments.length > 0 ? "Special attention to existing comments:\n - Avoid creating duplicate or similar comments unless you have significantly different insights\n - Analyze whether any existing comments have been addressed by the changes\n - If existing comments are resolved by the code changes, include them in the <resolvedComments> section" : ""} |
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.
🔴 REQUIRED: The literal <resolvedComments> in this instruction string will be injected unescaped into the XML payload, breaking the reviewRequest format. Escape the tag (e.g. <resolvedComments>) or otherwise avoid embedding raw XML tags in instruction text.
| ${existingComments.length > 0 ? "Special attention to existing comments:\n - Avoid creating duplicate or similar comments unless you have significantly different insights\n - Analyze whether any existing comments have been addressed by the changes\n - If existing comments are resolved by the code changes, include them in the <resolvedComments> section" : ""} | |
| Replace the `<resolvedComments>` snippet with an escaped version, for example: |
${existingComments.length > 0 ? "Special attention to existing comments:\n - Avoid creating duplicate or similar comments unless you have significantly different insights\n - Analyze whether any existing comments have been addressed by the changes\n - If existing comments are resolved by the code changes, include them in the <resolvedComments> section" : ""}
Add Contextual Review Tools and ContextGatherer
This PR extends the review agent by adding three new tools (find_patterns, find_utilities, analyze_architecture), updates the system prompt and index.ts to integrate these tools into the review workflow, and introduces a ContextGatherer class for aggregating review context programmatically.
Key Changes:
Review Notes: