Help us improve
Share bugs, ideas, or general feedback.
From smg
Enforces structured PR reviews in SMG Rust repository: fetch diff, map files to subsystem checklists (layering, config, workers, routing, parsers, storage), check anti-patterns, summarize findings.
npx claudepluginhub lightseekorg/smg-dev-guide --plugin smgHow this skill is triggered — by the user, by Claude, or both
Slash command
/smg:review-prThe summary Claude sees in its skill listing — used to decide when to auto-load this skill
```
Conducts systematic code reviews of GitHub pull requests, auditing CLAUDE.md compliance, bugs, git history, prior PR comments, and code comments for actionable feedback.
Reviews GitHub pull requests end-to-end using gh CLI: analyzes diffs, commits, CI/CD checks; provides blocking/suggestion/nit/praise feedback and submits review. Use for assigned PRs, self-reviews, or post-merge audits.
Performs thorough pull request reviews with parallel agents for bugs, security issues, guideline compliance, and error handling. Provides confidence-scored feedback and batched GitHub comments.
Share bugs, ideas, or general feedback.
NO REVIEW APPROVAL WITHOUT CHECKING ALL TOUCHED SUBSYSTEMS
If you haven't mapped the changed files to checklist sections, you cannot start reviewing.
1. FETCH: Get the PR diff (gh pr diff <number>)
2. MAP: List changed files → match to sections using the file-to-section table
3. TASK: Create one review task per matched section
4. CHECK: Work through each task, flag issues as blocker/suggestion/nit
5. ANTI-PATTERNS: Read @anti-patterns.md for the touched subsystems
6. SUMMARIZE: List all findings with severity and file:line citations
| Files Changed | Review Sections |
|---|---|
crates/protocols/src/ | 1 (Layering), 3 (Worker Lifecycle) |
model_gateway/src/config/ | 2 (Config Plumbing) |
model_gateway/src/main.rs | 2 (Config Plumbing) |
model_gateway/src/service_discovery.rs | 3 (Worker Lifecycle) |
model_gateway/src/core/steps/worker/ | 3 (Worker Lifecycle) |
model_gateway/src/core/routing/ | 4 (Routing Policy) |
crates/tool_parser/src/ | 5 (Parser Changes) |
crates/reasoning_parser/src/ | 5 (Parser Changes) |
crates/data_connector/src/ | 6 (Storage) |
bindings/ | 2 (Config Plumbing) |
| Any file | 7 (Error Handling), 8 (Testing), 9 (Code Quality) |
Sections 7, 8, 9 always apply. Section 10 applies to PRs touching 3+ files or adding new types.
crates/protocols/ types that only one crate setsconfig/types.rs, runtime → module-specificto_router_config() AND to_server_config() in main.rs updatedvalue_parser validationDefault impl includes new fields#[serde(default, skip_serializing_if)] for backward compat_override fields)build_model_cardSelectWorkerInfo)Send + Sync (DashMap, Arc — no bare Mutex on hot paths).unwrap() on worker slices — handle empty listw.is_healthy() && w.circuit_breaker().can_execute()reset)ParserFactory with model pattern mappingunwrap() in production codeanyhow::Contextthiserror for domain errors, anyhow for wrappinge2e_test/ — tests run sequentially with class-scoped backends)@pytest.mark.engine(...), @pytest.mark.gpu(count), @pytest.mark.model(...) as neededcargo +nightly fmt --all cleancargo clippy --all-targets --all-features -- -D warnings clean#[expect] with reason, not #[allow]tracing for logging, not println!crates/protocols/See @anti-patterns.md for subsystem-specific anti-patterns.
| Excuse | Reality |
|---|---|
| "This change is small, I can eyeball it" | Small changes cause the biggest bugs — the two-path config rule is a one-line omission. |
| "I know this code well" | Familiarity breeds blindness. Use the checklist. |
| "It's just a config change" | Config changes touch the most layers (CLI → types → main.rs → bindings). Check section 2. |
| "Tests pass so it's fine" | Tests don't catch layering violations, missing bindings, or one-path config bugs. |