--- name: RAG Code Review slug: rag-code-review category: AI Engineering description: RAG Code Review consults related ADRs, standards, similar files, and prior incidents before commenting on a code change. Use it for PR reviews or any critique of a diff so feedback is grounded in the team’s own decisions. github: "https://github.com/lyonzin/knowledge-rag/tree/master/skills/workflow/rag-code-review" language: Python stars: 265 forks: 38 install: "npx degit https://github.com/lyonzin/knowledge-rag/tree/master/skills/workflow/rag-code-review ~/.claude/skills/rag-code-review" installs_to: ~/.claude/skills/rag-code-review source_path: skills/workflow/rag-code-review/SKILL.md collection_size: 10 category_size: 2631 collection_url: "https://dirskills.com/collections/lyonzin/knowledge-rag" added: 2026-09-02T05:19:48.525Z last_synced: 2026-09-02T05:19:48.525Z canonical_url: "https://dirskills.com/skills/rag-code-review" --- # RAG Code Review RAG Code Review consults related ADRs, standards, similar files, and prior incidents before commenting on a code change. Use it for PR reviews or any critique of a diff so feedback is grounded in the team’s own decisions. **Install:** ```bash npx degit https://github.com/lyonzin/knowledge-rag/tree/master/skills/workflow/rag-code-review ~/.claude/skills/rag-code-review ``` ## README # rag-code-review — review against team standards, not the internet ## When to use this skill Trigger when the user asks: - "Review this PR / code / diff" - "Any issues with this?" - "Does this look right?" - "Is this idiomatic?" - "Should I merge this?" - Any critique-style prompt on a code artifact --- ## What this skill commits to Before offering ANY review comment, the agent consults the corpus for: 1. **ADRs** governing the area (auth, retries, error handling, naming, dependencies…) 2. **Existing patterns** — similar files that show the "how we do this" convention 3. **Prior incidents** touching the code being changed Review comments then read as *"per ADR-XXXX we do Y here"* rather than *"generally you should Z"*. --- ## Steps 1. **Identify the "area" of the change.** From the diff / snippet, extract: - The file path or module name (e.g. `services/payment/`, `mcp_server/security.py`) - The concern touched (auth, retries, logging, config, ingestion, search, storage…) - Any new dependencies, endpoints, or public API changes 2. **Search for governing ADRs / standards:** ``` search_knowledge(query=" ADR standard", max_results=5) ``` Example: `search_knowledge(query="retry policy ADR")` 3. **Find similar existing files** (patterns to follow): ``` search_similar(filepath="", max_results=5) ``` If the file is new, use `search_knowledge` with the file's purpose instead. 4. **Look for prior incidents / postmortems** on adjacent code: ``` search_knowledge(query=" incident postmortem", max_results=3) ``` 5. **Structure the review comments:** ```markdown ### Findings 1. **[required]** Per ADR-0031 [docs/adr/0031-retries.md] the retry policy is "3 attempts, exponential backoff". This PR uses 5 attempts — either match the ADR or open a follow-up ADR to document the exception. 2. **[suggestion]** The error-handling shape here differs from what `services/payment/refund_service.py` does [search_similar hit]. Consider aligning for consistency. 3. **[nit]** Missing docstring on `_new_helper()`. Team standard is ≥80% docstring coverage (interrogate gate, per CONTRIBUTING.md). ### Not commented on Style / naming / whitespace — those are ruff's job. Reviewed only what ruff cannot catch. ``` 6. **Every finding cites a source** (ADR, similar file, incident, coding standard). If a finding is opinion-only, mark it `[opinion]` explicitly. --- ## Examples **Example 1 — Retry policy diff** User: *"Review this. `for attempt in range(5): try: ... except: sleep(2)` — new call site added to payment service."* Agent: ``` search_knowledge(query="retry policy exponential backoff") → 2 hits → docs/adr/0031-retries.md (0.83), services/common/retry.py (0.71) search_similar(filepath="services/payment/refund_service.py") → 5 hits → All use `services/common/retry.retry_with_backoff` decorator Reply: "3 findings: 1. **[required]** ADR-0031 [docs/adr/0031-retries.md] says '3 attempts, exponential backoff, max 30s'. This code has 5 attempts + fixed 2s sleep — violates both. Fix: use `@retry_with_backoff` decorator from `services/common/retry.py:12`. 2. **[required]** Bare `except:` catches BaseException including SystemExit and KeyboardInterrupt. Team standard is to catch specific exceptions [PEP 8]. Change to `except (ConnectionError, TimeoutError) as e:`. 3. **[suggestion]** Every other payment service call site uses the decorator pattern [services/payment/refund_service.py, capture_service.py]. This ad-hoc loop is the odd one out — align for consistency." ``` **Example 2 — New auth middleware** User: *"Look at this new middleware — it adds bearer auth to a new endpoint."* Agent: ``` search_knowledge(query="bearer auth middleware ADR") → 3 hits → docs/adr/0018-auth.md, mcp_server/security.py (existing BearerAuthMiddleware), CHANGELOG.md#v451 Reply: "The project already has `BearerAuthMiddleware` in `mcp_server/security.py:434` with constant-time comparison, RFC 6750 challenge, exempt paths, etc. This new middleware duplicates that logic — worse, uses `==` (timing side-channel). Fix: import and reuse the existing `BearerAuthMiddleware` instead. If the new endpoint has different auth needs (different token, different exempt paths), compose the existing middleware or open an ADR before duplicating." ``` **Example 3 — Nothing to cite, honest** User: *"Review this small helper for parsing dates."* Agent: ``` search_knowledge(query="date parsing helper standard") → 0 hits search_similar(filepath="") → 2 weak hits (unrelated helpers) Reply: "Nothing in the corpus about date-parsing conventions here (searched 'date parsing helper standard' and adjacent files, 0 strong hits). Reviewing against general Python practice: 1. **[opinion]** `datetime.strptime` is slow — if this is a hot path, consider `dateutil.parser` or pre-compiling. 2. **[nit]** Missing timezone handling — will surprise on DST edges. Flagging both as opinion/nit because there is no ADR to cite. If date parsing becomes a repeated topic, worth opening an ADR." ``` --- ## Edge cases - **Very small diff** (1-2 lines) — still search once for the governing ADR of the area, then a light review. Skip similar-search. - **Very large diff** (whole feature) — do the ADR search for the top 2-3 concerns; do not try to search every file. Focus review energy on the ADR-governed parts. - **Newly-created file** — `search_similar` needs an existing file. Use `search_knowledge` with the file's purpose instead. - **User pasted code without file context** — ask "what area/service is this from?" so search queries can be targeted. --- ## Related skills - **[`rag-check-first`](https://github.com/lyonzin/knowledge-rag/blob/master/skills/foundation/rag-check-first/SKILL.md)** — the base pattern (this is `check-first` specialized for review). - **[`rag-cite-sources`](https://github.com/lyonzin/knowledge-rag/blob/master/skills/foundation/rag-cite-sources/SKILL.md)** — critical here; a review comment without a citation is just opinion. - **[`rag-deep-dive`](https://github.com/lyonzin/knowledge-rag/blob/master/skills/workflow/rag-deep-dive/SKILL.md)** — for very architectural reviews, chain into deep-dive to understand the full context. - **[`rag-index-decisions`](https://github.com/lyonzin/knowledge-rag/blob/master/skills/maintenance/rag-index-decisions/SKILL.md)** — if the review surfaces a new pattern, index the decision.