Systematic code review for correctness, security, and growth — not just style enforcement
Good reviews catch bugs. Great reviews teach the author something.
| Pass | Focus | What You're Looking For | Time |
|---|---|---|---|
| 1. Orientation | Big picture | Does the approach make sense? Is the scope right? Over-engineered? |
| 2-3 min |
| 2. Logic | Deep read | Edge cases, null handling, error paths, concurrency, off-by-one | 10-15 min |
| 3. Polish | Surface | Naming, duplication, test coverage, docs | 3-5 min |
Pass 1 shortcut: Read the PR description and test names first. They reveal intent faster than code.
| Prefix | Meaning | Author Response |
|---|---|---|
[blocking] | Must fix before merge | Fix it |
[suggestion] | Better approach exists | Consider it, explain if declining |
[question] | I don't understand | Clarify (in code, not just in reply) |
[nit] | Trivial style issue | Fix if easy, skip if not |
[praise] | This is well done | Appreciate it |
| Bad | Why | Good |
|---|---|---|
| "This is confusing" | Vague, unhelpful | "[suggestion] This nested ternary is hard to follow. Consider extracting to a named function like isEligibleForDiscount()." |
| "Fix this" | No context | "[blocking] This accepts user input without sanitization. Use escapeHtml() before rendering." |
| "Why?" | Sounds hostile | "[question] What's the motivation for the custom sort here vs Array.sort()? Is there a performance concern?" |
| "LGTM" (on 500-line PR) | Rubber stamp | "Pass 1: Approach looks right. Pass 2 comments below. Pass 3: naming is clean." |
| Anti-Pattern | What Happens | Instead |
|---|---|---|
| Rubber-stamp | Bugs ship | Actually read Pass 1-3 |
| Bikeshedding | Hours on naming, ignore logic bugs | Spend 80% on Pass 2 |
| Gatekeeping | Reviewees dread PRs | Teach, don't block |
| Week-long queue | PRs go stale, conflicts pile up | Review within 4 hours, merge within 24 |
| Style wars | Team friction | Automate style (ESLint, Prettier, etc.) |
| Everything-is-blocking | Author overwhelmed | Use prefix system honestly |
| PR Size | Expected Review Time | If Larger |
|---|---|---|
| < 100 lines | < 30 min | — |
| 100-400 lines | 30-60 min | Ideal size |
| 400+ lines | 60+ min | Ask author to split |
| 1000+ lines | Don't | Refuse; request breakdown |
When: Before release, after major refactoring, or on quality concerns
Scope: Multi-dimensional code quality analysis beyond standard code review
| Dimension | Focus | Tools/Methods | Output |
|---|---|---|---|
| Debug & Logging | Console statements, debug code | grep -r "console\\.log|console\\.debug" | Categorize: legitimate vs removable |
| Dead Code | Unused imports, orphaned files, broken refs | TypeScript compilation + manual scan | List dead commands, UI, dependencies |
| Performance | Blocking I/O, sync operations, bottlenecks | grep -r "Sync\(" src/, profiling | Async refactoring candidates |
| Menu Validation | All commands/buttons work | Manual testing + error logs | Broken commands, missing handlers |
| Dependencies | Unused packages, leftover references | package.json vs import analysis | Removable dependencies |
## Executive Summary
- Console statements: X remaining (Y legitimate, Z removable)
- Dead code: [commands/UI/dependencies list]
- Performance: [blocking operations count]
- Menu validation: [working/broken ratio]
## Recommendations
1. [Category]: [Issue] → [Action] (Priority: Critical/High/Medium)
2. [Category]: [Issue] → [Action] (Priority: Critical/High/Medium)
| Category | Keep? | Examples |
|---|---|---|
| Enterprise compliance | ✅ | Audit logs, security events, GDPR actions |
| User feedback | ✅ | TTS status, long-running ops, critical errors |
| Debug noise | ❌ | Setup verbosity, migration logs, info messages |
| Development artifacts | ❌ | "Entering function X", temporary debugging |
fs.readFileSync, fs.existsSync, fs.readdirSync
fs-extra async: await fs.readFile, await fs.pathExists, await fs.readdirPromise.all([op1(), op2(), op3()])vscode.commands.registerCommand('command.id', ...)npm run compile → exit 0Pattern applies to: VS Code extensions, Electron apps, Node.js services with UI
See synapses.json for connections.