feat: add best practices from 2000+ PR reviews#49
Conversation
Synthesized lessons learned from 407 PRs across 5 repositories: - ado-git-repo-insights (954 comments) - odd-ai-reviewers (544 comments) - ado-git-repo-seeder (151 comments) - odd-demonstration (128 comments) - repo-standards (124 comments) Key findings: - Security: Path traversal (225), input validation (45), command injection (32) - Reliability: Empty catch blocks (26), error recovery paths (15) - Contracts: Schema versioning, documentation drift - CI/CD: Runtime version alignment, run ID uniqueness Includes real P1/P2 examples from AI-assisted code review.
AI Code Review Summary
Findings by File
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,420 @@ | |||
| # Best Practices: Lessons from 2,000+ PR Reviews | |||
|
|
|||
There was a problem hiding this comment.
Multiple issues found in this area (3):
🟡 Line 2 🐺: The claim of '2,000+ PR reviews' in the title doesn't match the '407 PRs' mentioned in the source line below
💡 Update the title to reflect the actual number analyzed (407 PRs) or clarify if 2,000+ refers to individual review comments rather than PRs
🟡 Line 4 🐺: Inconsistent statistics: '407 PRs' and '1,901 review comments' don't align with the title's '2,000+ PR reviews'
💡 Clarify the distinction between PRs reviewed vs. review comments analyzed, and ensure all numbers are consistent throughout the document
🔵 Line 1 🐺: Excellent comprehensive documentation with real-world examples and specific remediation guidance. The structure and content provide significant value for code review standards.
💡 Consider adding links to external resources (OWASP, CWE references) for readers who want to learn more about specific security topics
|
|
||
| --- | ||
|
|
||
| ### 🔴 Input Validation (45 occurrences) |
There was a problem hiding this comment.
🔵 🐺: The Python example for credential exposure shows a syntax that may not be standard (f-strings with simple variable interpolation)
Rule: pr_agent/docs
💡 Suggestion: Consider showing a more realistic example like logging request details that might accidentally include tokens
| --- | ||
|
|
||
| ## API & Schema Contracts | ||
|
|
There was a problem hiding this comment.
🔵 🐺: The 'Real P2 finding' quote formatting could be more consistent - some use quotes while others use block quotes
Rule: pr_agent/docs
💡 Suggestion: Standardize the formatting of real findings throughout the document for better readability
| 20 javascript.lang.security.detect-child-process | ||
| 20 javascript.lang.security.audit.detect-non-literal-regexp | ||
| 17 security/path-traversal | ||
| 13 reliability/error-handling |
There was a problem hiding this comment.
🔵 🐺: The Semgrep rule counts in the appendix (totaling ~357) don't align with the methodology section's claim of analyzing 1,901 review comments
Rule: pr_agent/docs
💡 Suggestion: Clarify how the 1,901 comments relate to the specific Semgrep rule counts, or provide a breakdown of comment types
|
|
||
| --- | ||
|
|
||
| *This document is living. As new patterns emerge from PR reviews, they should be added here.* |
There was a problem hiding this comment.
🔵 🐺: The document ends abruptly without a clear conclusion or call to action
Rule: pr_agent/docs
💡 Suggestion: Consider adding a brief conclusion summarizing key takeaways or next steps for readers
1 participant
Synthesized lessons learned from 407 PRs across 5 repositories:
Key findings:
Includes real P1/P2 examples from AI-assisted code review.