From 48166e25216f86e59aff383f19226b6fc05c2f3e Mon Sep 17 00:00:00 2001 From: martin Date: Sat, 21 Feb 2026 11:47:10 +0100 Subject: [PATCH] added core-review todo --- skills/code-review-todo/SKILL.md | 205 +++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 skills/code-review-todo/SKILL.md diff --git a/skills/code-review-todo/SKILL.md b/skills/code-review-todo/SKILL.md new file mode 100644 index 0000000..3883230 --- /dev/null +++ b/skills/code-review-todo/SKILL.md @@ -0,0 +1,205 @@ +--- +name: code-review-todo +description: Creates detailed code-review todo files with problem analysis, impact assessment, best-practice recommendations, and fix examples. Use when documenting code-review findings, performance issues, security concerns, or architectural problems. +argument-hint: "[problem description or file path]" +allowed-tools: Read, Glob, Grep, Write, Edit, Bash, WebSearch +--- + +# Code Review Todo Creator + +Creates structured code-review finding files with comprehensive problem descriptions accessible to non-experts, concrete improvement recommendations, and best-practice references. + +## When to Use + +- Documenting a code-review finding (bug, anti-pattern, security issue, performance problem) +- Recording an architectural concern with explanation of why it matters +- Creating actionable improvement tickets from review observations +- Turning vague "this feels wrong" into a clear, teachable todo + +## Workflow + +### Step 1: Gather Context + +1. Read the project's `CLAUDE.md` to understand tech stack, architecture, and conventions +2. Parse the user's finding - identify the affected area, files, and nature of the problem +3. Use Glob/Grep to locate the actual code exhibiting the problem +4. Read the affected files to confirm the issue and understand surrounding context +5. If the issue involves a framework feature (Next.js, React, Django, etc.), verify behavior against the project's framework version in `package.json` or `requirements.txt` + +### Step 2: Classify Priority + +Assign a priority level based on impact and urgency: + +| Priority | Tag | Criteria | Examples | +|----------|-----|----------|----------| +| **Critical** | `P0` | Security vulnerability, data loss risk, broken auth, exposed secrets | XSS, SQL injection, leaked tokens, missing auth guards | +| **High** | `P1` | Silent data corruption, unhandled errors causing bad UX, major performance bottleneck | Missing response.ok checks, N+1 queries on hot paths, O(n^2) in render loops | +| **Important** | `P2` | Performance degradation, inconsistent patterns, maintainability risk, DRY violations | Missing code splitting, stale cache config, duplicate code paths, inline magic values | +| **Medium** | `P3` | Code quality issues with limited blast radius, dead code, debug leftovers | console.log in production, unused components, debug output in DOM | +| **Low** | `P4` | Stylistic issues, optional improvements, documentation gaps, nice-to-haves | Missing type annotations, optional stricter patterns, cosmetic inconsistencies | + +### Step 3: Research Best Practice + +Before writing the todo, verify the recommended fix: + +1. **Check project conventions**: Does CLAUDE.md or existing code show the correct pattern? Reference it. +2. **Check framework docs**: Is there an official recommendation? Note the framework version. +3. **Find reference implementations**: Is the correct pattern already used elsewhere in the codebase? Point to it as a model. +4. **Web search if needed**: For framework-specific behavior, verify against current docs (especially for fast-moving frameworks like Next.js, React). + +### Step 4: Choose Filename + +Format: `P[0-4]-[kebab-case-description].md` in the `todos/code-review/` directory. + +Create `todos/code-review/` if it doesn't exist. + +Rules: +- Prefix with priority tag: `P0-`, `P1-`, `P2-`, `P3-`, or `P4-` +- Use kebab-case after the priority prefix +- Be specific: `P1-missing-response-ok-checks` not `P1-error-handling` +- Keep filenames under 60 characters (including prefix) + +### Step 5: Write Todo + +Use this template. Write in the project's language (check CLAUDE.md and existing todos for language preference - German or English). + +```markdown +# [Concise title describing the problem] + +**Status:** Open +**Priority:** [Critical | High | Important | Medium | Low] +**Area:** [Module name] - [Sub-area] +**Tags:** code-review, [category1], [category2] + +## Beschreibung + +### Was ist das Problem? + +[Explain the problem in plain language. Assume the reader is a competent developer but NOT an expert in this specific framework or pattern. Explain framework-specific concepts when they're central to understanding the issue.] + +[Include a concrete code snippet showing the problematic code - use actual code from the codebase, not pseudocode:] + +```typescript +// Aktueller Code in path/to/file.ts (Zeile XX) +const data = await fetch(url).then(res => res.json()); +// ^ Problem: Wenn der Server einen 404 oder 500 zurueckgibt, +// wird trotzdem .json() aufgerufen +``` + +### Warum ist das problematisch? + +[Explain the real-world consequences. Be concrete:] +- [What happens to the user?] +- [What happens to the data?] +- [When does this trigger? How likely is it?] +- [What makes debugging difficult?] + +### Wie sollte es richtig gemacht werden? + +[Show the recommended fix with a complete, copy-pasteable code example:] + +```typescript +// Empfohlenes Pattern +const res = await fetch(url); +if (!res.ok) { + throw new Error(`API error: ${res.status} ${res.statusText}`); +} +const data = await res.json(); +``` + +[Explain WHY this is better - connect back to the problem:] +- [Benefit 1] +- [Benefit 2] + +### Referenz im Projekt + +[Point to existing code in the project that already does it correctly, if available:] + +> Korrekt umgesetzt in `path/to/good-example.ts` (Zeile XX): +> ```typescript +> // This file already follows the recommended pattern +> ``` + +[If no project reference exists, link to official documentation or well-known best practices.] + +## Requirements + +1. [Specific, actionable requirement] +2. [Specific, actionable requirement] + +## Affected Files + +- `path/to/file.ts` - Zeile XX: [What specifically needs to change] +- `path/to/other-file.ts` - Zeile XX: [What specifically needs to change] + +## Best Practice + +[A brief, reusable lesson. Frame it as a general principle that applies beyond this specific finding:] + +> **Regel:** [One-sentence principle, e.g. "Jede fetch()-Antwort muss vor .json() auf res.ok geprueft werden."] + +[Optional: Link to official docs, blog posts, or RFC that support this practice.] + +## Acceptance Criteria + +- [ ] [Verifiable criterion] +- [ ] [Verifiable criterion] +- [ ] `npm run check` laeuft fehlerfrei +``` + +## Writing Guidelines + +### Problem Description Quality + +The problem description is the most important section. It must: + +1. **Start with the observable symptom**, not the technical root cause + - Good: "Wenn ein Nutzer zwischen Seiten navigiert, wird dieselbe API zweimal aufgerufen" + - Bad: "QueryClient wird pro Page neu instanziiert" + +2. **Explain framework concepts inline** when they're needed to understand the issue + - Good: "React Query cached Daten in einem QueryClient. Wenn jede Page ihren eigenen QueryClient erstellt, kann kein Cache geteilt werden." + - Bad: "Redundante QueryClient-Instanzen brechen die Cache-Kohaerenz" + +3. **Show real code, not abstractions** - copy actual lines from the codebase with file paths and line numbers + +4. **Quantify impact where possible** + - "~500KB zusaetzlich im initialen Bundle" + - "~180 React-Subscriptions statt einer einzigen" + - "Bei 30 Spieltagen: 30 x .find() = O(n^2)" + +### Fix Recommendation Quality + +The recommended fix must be: + +1. **Copy-pasteable** - a developer should be able to apply it directly +2. **Minimal** - only change what's necessary, don't refactor surroundings +3. **Explained** - connect back to why this solves the described problem +4. **Verified** - if referencing a framework API, confirm it exists in the project's version + +### Tag Categories + +Use these tag categories after `code-review`: + +| Tag | Use for | +|-----|---------| +| `security` | Auth, XSS, CSRF, secrets, cookies | +| `performance` | Bundle size, render cycles, queries, caching | +| `bug` | Incorrect behavior, data corruption, crashes | +| `bug-risk` | Code that works now but is fragile/will break | +| `architecture` | Structural concerns, patterns, module boundaries | +| `cleanup` | Dead code, debug leftovers, naming | +| `caching` | React Query, HTTP cache, stale data | +| `auth` | Authentication, authorization, tokens, sessions | +| `i18n` | Hardcoded strings, locale handling | +| `integration` | API layer, Django communication, endpoints | + +## Rules + +1. **Verify before writing** - always read the actual source file. Never create a finding based on assumptions. +2. **One finding per file** - if a single review observation reveals multiple independent issues, create separate todos. +3. **Reference project code** - always point to the exact file, line number, and code snippet. +4. **Find the positive example** - if the correct pattern exists elsewhere in the project, reference it. This makes the fix concrete and shows it's achievable within the project's conventions. +5. **Version-aware** - check `package.json` / framework version before claiming something is an anti-pattern. Framework behavior changes between versions (e.g., Next.js 16 renamed middleware.ts to proxy.ts). +6. **No false alarms** - if you're not sure the issue is real, investigate further or note the uncertainty explicitly. +7. **Match project language** - write in the same language as CLAUDE.md and existing todos (German/English).