feat: add /code-review skill with bundled lenses
This commit is contained in:
dan
parent
2103e0994d
commit
5ebae8f86e
|
|
@ -52,14 +52,14 @@
|
|||
{"id":"skills-ebl","title":"Benchmark vision model UI understanding","description":"## Goal\nMeasure how well vision models can answer UI questions from screenshots.\n\n## Test cases\n1. **Element location**: \"Where is the Save button?\" → coordinates\n2. **Element identification**: \"What buttons are visible?\" → list\n3. **State detection**: \"Is the checkbox checked?\" → boolean\n4. **Text extraction**: \"What does the error message say?\" → text\n5. **Layout understanding**: \"What's in the sidebar?\" → structure\n\n## Metrics\n- Accuracy: Does the answer match ground truth?\n- Precision: How close are coordinates to actual element centers?\n- Latency: Time from query to response\n- Cost: Tokens consumed per query\n\n## Prompt engineering questions\n- Does adding a grid overlay help coordinate precision?\n- What prompt format gives most actionable coordinates?\n- Can we get bounding boxes vs point coordinates?\n\n## Comparison baseline\n- Manual annotation of test screenshots\n- AT-SPI data (once enabled) as ground truth\n\n## Depends on\n- Test screenshots from real apps\n- Ground truth annotations","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-17T14:13:10.038933798-08:00","updated_at":"2025-12-17T14:13:10.038933798-08:00"}
|
||||
{"id":"skills-f2p","title":"Skills + Molecules Integration","description":"Integrate skills with beads molecules system.\n\nDesign work tracked in dotfiles (dotfiles-jjb).\n\nComponents:\n- Checklist support (lightweight skills)\n- Audit integration (bd audit for skill execution)\n- Skill frontmatter for triggers/tracking\n- Proto packaging alongside skills\n\nSee: ~/proj/dotfiles ADR work","status":"closed","priority":2,"issue_type":"epic","created_at":"2025-12-23T17:58:55.999438985-05:00","updated_at":"2025-12-23T19:22:38.577280129-05:00","closed_at":"2025-12-23T19:22:38.577280129-05:00","close_reason":"Superseded by skills-4u0 (migrated from dotfiles)","dependencies":[{"issue_id":"skills-f2p","depends_on_id":"skills-vpy","type":"blocks","created_at":"2025-12-23T17:59:17.976956454-05:00","created_by":"daemon"},{"issue_id":"skills-f2p","depends_on_id":"skills-u3d","type":"blocks","created_at":"2025-12-23T17:59:18.015216054-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fo3","title":"Compare WORKFLOWS.md with upstream","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-03T20:15:54.283175561-08:00","updated_at":"2025-12-03T20:19:28.897037199-08:00","closed_at":"2025-12-03T20:19:28.897037199-08:00","dependencies":[{"issue_id":"skills-fo3","depends_on_id":"skills-ebh","type":"discovered-from","created_at":"2025-12-03T20:15:54.286009672-08:00","created_by":"daemon","metadata":"{}"}]}
|
||||
{"id":"skills-fvc","title":"Code Review: {{target}}","description":"Multi-lens code review workflow for {{target}}.\n\n## Philosophy\nThe LLM stays in the loop at every step - this is agent-assisted review, not automated parsing. The agent applies judgment about what's worth filing, how to prioritize, and what context to include.\n\n## Variables\n- target: File or directory to review\n\n## Workflow\n1. Explore codebase to find candidates (if target is directory)\n2. Run lenses via orch consensus for multi-model perspective\n3. Analyze findings - LLM synthesizes across lenses and models\n4. File issues with judgment - group related, set priorities, add context\n5. Summarize for digest\n\n## Lenses Available\n- bloat: size, complexity, SRP violations\n- smells: readability, naming, control flow\n- dead-code: unused, unreachable, obsolete\n- redundancy: duplication, YAGNI, parallel systems","status":"open","priority":2,"issue_type":"epic","created_at":"2025-12-25T10:10:57.652098447-05:00","updated_at":"2025-12-26T01:48:53.382599922-05:00","labels":["template"]}
|
||||
{"id":"skills-fvc.1","title":"Run bloat lens on {{target}}","description":"Run bloat review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/bloat.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: file size, function length, complexity, SRP violations.\nRecord findings for later filing.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:13:59.789715667-05:00","updated_at":"2025-12-26T01:15:59.453348311-05:00","dependencies":[{"issue_id":"skills-fvc.1","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:13:59.80248308-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.2","title":"Run smells lens on {{target}}","description":"Run smells review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/smells.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: naming issues, control flow smells, data smells, structural issues.\nRecord findings for later filing.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:13.977562568-05:00","updated_at":"2025-12-26T01:16:43.064403777-05:00","dependencies":[{"issue_id":"skills-fvc.2","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:13.989662453-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.3","title":"Run dead-code lens on {{target}}","description":"Run dead-code review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/dead-code.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: zombie code, unreachable paths, obsolete shims, import cruft.\nRecord findings for later filing.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:17.522715411-05:00","updated_at":"2025-12-26T01:16:48.452520859-05:00","dependencies":[{"issue_id":"skills-fvc.3","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:17.53423496-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.4","title":"Run redundancy lens on {{target}}","description":"Run redundancy review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/redundancy.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: duplication, parallel systems, YAGNI violations, consolidation opportunities.\nRecord findings for later filing.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:21.94274481-05:00","updated_at":"2025-12-26T01:16:52.579297412-05:00","dependencies":[{"issue_id":"skills-fvc.4","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:21.956965459-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.5","title":"Analyze and file findings as beads","description":"Review all lens findings and file actionable items as beads in the TARGET REPO.\n\n## LLM Judgment Required\nThis is NOT mechanical parsing. The agent should:\n\n1. **Synthesize across lenses** - same issue may appear in bloat + smells\n2. **Prioritize by impact** - P2 for blocking issues, P3 for cleanup\n3. **Group related findings** - one issue for 'split this file' vs 5 separate issues\n4. **Add context** - why it matters, suggested approach, quick-win vs big-refactor\n5. **Skip noise** - LOW severity findings unless pattern emerges\n\n## Filing Pattern\n```bash\ncd \u003ctarget-repo\u003e\nbd create --title=\"refactor: \u003cclear action\u003e\" --type=task --priority=\u003c2|3\u003e --body=\"\u003ccontext\u003e\n\nFound by \u003clens\u003e review.\"\n```\n\n## Key Questions\n- Is this worth someone's time to fix?\n- Can related findings be grouped into one actionable issue?\n- What's the right priority given the codebase context?","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:34.288221353-05:00","updated_at":"2025-12-26T01:49:06.858541026-05:00","dependencies":[{"issue_id":"skills-fvc.5","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:34.303313556-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.6","title":"Summarize review findings","description":"Create summary of code review findings for {{target}}:\n\n- Total findings by severity (HIGH/MED/LOW)\n- Top issues by category\n- Recommendations for immediate action\n- Technical debt assessment\n\nThis summary will become the squash digest.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:34.590409022-05:00","updated_at":"2025-12-25T10:16:34.590409022-05:00","dependencies":[{"issue_id":"skills-fvc.6","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:34.591813242-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.7","title":"Explore {{target}} for review candidates","description":"If {{target}} is a directory, explore to find files worth reviewing.\n\n## For Bloat Lens\n- Find files over 300 lines (warning) or 500 lines (critical)\n- Find functions over 50 lines\n- Identify files with multiple responsibilities\n\n## For Other Lenses\n- Sample representative files from different modules\n- Prioritize high-traffic code over rarely-used\n\n## Output\nList of specific files to run through lenses, prioritized by likely issues.\n\nSkip if {{target}} is already a specific file.","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-26T01:49:18.377427063-05:00","updated_at":"2025-12-26T01:49:18.377427063-05:00","dependencies":[{"issue_id":"skills-fvc.7","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-26T01:49:18.383625871-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc","title":"Code Review: {{target}}","description":"Multi-lens code review workflow for {{target}}.\n\n## Philosophy\nThe LLM stays in the loop at every step - this is agent-assisted review, not automated parsing. The agent applies judgment about what's worth filing, how to prioritize, and what context to include.\n\n## Variables\n- target: File or directory to review\n\n## Workflow\n1. Explore codebase to find candidates (if target is directory)\n2. Run lenses via orch consensus for multi-model perspective\n3. Analyze findings - LLM synthesizes across lenses and models\n4. File issues with judgment - group related, set priorities, add context\n5. Summarize for digest\n\n## Lenses Available\n- bloat: size, complexity, SRP violations\n- smells: readability, naming, control flow\n- dead-code: unused, unreachable, obsolete\n- redundancy: duplication, YAGNI, parallel systems","status":"closed","priority":2,"issue_type":"epic","created_at":"2025-12-25T10:10:57.652098447-05:00","updated_at":"2025-12-26T23:22:41.408582818-05:00","closed_at":"2025-12-26T23:22:41.408582818-05:00","close_reason":"Replaced by /code-review skill","labels":["template"]}
|
||||
{"id":"skills-fvc.1","title":"Run bloat lens on {{target}}","description":"Run bloat review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/bloat.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: file size, function length, complexity, SRP violations.\nRecord findings for later filing.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:13:59.789715667-05:00","updated_at":"2025-12-26T23:22:41.416754154-05:00","closed_at":"2025-12-26T23:22:41.416754154-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.1","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:13:59.80248308-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.2","title":"Run smells lens on {{target}}","description":"Run smells review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/smells.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: naming issues, control flow smells, data smells, structural issues.\nRecord findings for later filing.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:13.977562568-05:00","updated_at":"2025-12-26T23:22:41.423564011-05:00","closed_at":"2025-12-26T23:22:41.423564011-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.2","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:13.989662453-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.3","title":"Run dead-code lens on {{target}}","description":"Run dead-code review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/dead-code.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: zombie code, unreachable paths, obsolete shims, import cruft.\nRecord findings for later filing.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:17.522715411-05:00","updated_at":"2025-12-26T23:22:41.432104796-05:00","closed_at":"2025-12-26T23:22:41.432104796-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.3","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:17.53423496-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.4","title":"Run redundancy lens on {{target}}","description":"Run redundancy review lens via orch:\n\n```bash\norch consensus \"$(cat ~/.config/lenses/redundancy.md)\" flash gemini --file {{target}} --mode open\n```\n\nLook for: duplication, parallel systems, YAGNI violations, consolidation opportunities.\nRecord findings for later filing.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:21.94274481-05:00","updated_at":"2025-12-26T23:22:41.439539488-05:00","closed_at":"2025-12-26T23:22:41.439539488-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.4","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:21.956965459-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.5","title":"Analyze and file findings as beads","description":"Review all lens findings and file actionable items as beads in the TARGET REPO.\n\n## LLM Judgment Required\nThis is NOT mechanical parsing. The agent should:\n\n1. **Synthesize across lenses** - same issue may appear in bloat + smells\n2. **Prioritize by impact** - P2 for blocking issues, P3 for cleanup\n3. **Group related findings** - one issue for 'split this file' vs 5 separate issues\n4. **Add context** - why it matters, suggested approach, quick-win vs big-refactor\n5. **Skip noise** - LOW severity findings unless pattern emerges\n\n## Filing Pattern\n```bash\ncd \u003ctarget-repo\u003e\nbd create --title=\"refactor: \u003cclear action\u003e\" --type=task --priority=\u003c2|3\u003e --body=\"\u003ccontext\u003e\n\nFound by \u003clens\u003e review.\"\n```\n\n## Key Questions\n- Is this worth someone's time to fix?\n- Can related findings be grouped into one actionable issue?\n- What's the right priority given the codebase context?","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:34.288221353-05:00","updated_at":"2025-12-26T23:22:41.451111869-05:00","closed_at":"2025-12-26T23:22:41.451111869-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.5","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:34.303313556-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.6","title":"Summarize review findings","description":"Create summary of code review findings for {{target}}:\n\n- Total findings by severity (HIGH/MED/LOW)\n- Top issues by category\n- Recommendations for immediate action\n- Technical debt assessment\n\nThis summary will become the squash digest.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-25T10:16:34.590409022-05:00","updated_at":"2025-12-26T23:22:41.459279583-05:00","closed_at":"2025-12-26T23:22:41.459279583-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.6","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-25T10:16:34.591813242-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvc.7","title":"Explore {{target}} for review candidates","description":"If {{target}} is a directory, explore to find files worth reviewing.\n\n## For Bloat Lens\n- Find files over 300 lines (warning) or 500 lines (critical)\n- Find functions over 50 lines\n- Identify files with multiple responsibilities\n\n## For Other Lenses\n- Sample representative files from different modules\n- Prioritize high-traffic code over rarely-used\n\n## Output\nList of specific files to run through lenses, prioritized by likely issues.\n\nSkip if {{target}} is already a specific file.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-26T01:49:18.377427063-05:00","updated_at":"2025-12-26T23:22:41.465019112-05:00","closed_at":"2025-12-26T23:22:41.465019112-05:00","close_reason":"Replaced by /code-review skill","dependencies":[{"issue_id":"skills-fvc.7","depends_on_id":"skills-fvc","type":"parent-child","created_at":"2025-12-26T01:49:18.383625871-05:00","created_by":"daemon"}]}
|
||||
{"id":"skills-fvx","title":"use-skills.sh: stderr from nix build corrupts symlinks when repo is dirty","description":"In use-skills.sh, the line:\n\n```bash\nout=$(nix build --print-out-paths --no-link \"${SKILLS_REPO}#${skill}\" 2\u003e\u00261) || {\n```\n\nThe `2\u003e\u00261` merges stderr into stdout. When the skills repo is dirty, nix emits a warning to stderr which gets captured into $out and used as the symlink target.\n\nResult: symlinks like:\n```\norch -\u003e warning: Git tree '/home/dan/proj/skills' is dirty\n/nix/store/j952hgxixifscafb42vmw9vgdphi1djs-ai-skill-orch\n```\n\nFix: redirect stderr to /dev/null or filter it out before creating symlink.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-12-14T11:54:03.06502295-08:00","updated_at":"2025-12-14T11:59:25.472044754-08:00","closed_at":"2025-12-14T11:59:25.472044754-08:00"}
|
||||
{"id":"skills-gas","title":"spec-review: File discovery is brittle, can pick wrong file silently","description":"The fallback `find ... | head -1` is non-deterministic and can select wrong spec/plan/tasks file without user noticing. Branch names with `/` also break path construction.\n\nFixes:\n- Fail fast if expected file missing\n- Print chosen file path before proceeding\n- Require explicit confirmation if falling back\n- Handle branch names with slashes","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-12-15T00:23:22.762045913-08:00","updated_at":"2025-12-15T00:46:16.231199231-08:00","closed_at":"2025-12-15T00:46:16.231199231-08:00"}
|
||||
{"id":"skills-gga","title":"Configure Claude Code LSP integration","description":"Claude Code now has LSP (Language Server Protocol) integration that provides semantic code understanding.\n\n## Key Features\n- Go to Definition - Jump to symbol declarations\n- Find References - Locate all usages (safe refactoring) \n- Hover/Type Info - Get signatures, docstrings, types\n- Diagnostics - See compiler errors/warnings directly\n\n## Setup Required\n1. Enable via env var: ENABLE_LSP_TOOL=1\n2. Install language servers on PATH:\n - TypeScript: typescript-language-server\n - Python: pyright or python-lsp-server\n - Go: gopls\n - Rust: rust-analyzer\n - Nix: nil or nixd\n\n3. Optional: MCP adapter like cclsp for robust AI→LSP bridging\n\n## Tasks\n- [ ] Add language servers to Nix packages\n- [ ] Set ENABLE_LSP_TOOL=1 in shell environment\n- [ ] Test with representative projects\n- [ ] Consider creating an LSP skill documenting available servers\n\n## References\n- Discovered via orch consensus with sonar, flash-or, glm\n- Related to skills+molecules integration (semantic code understanding for traces)","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-12-24T02:16:40.510669749-05:00","updated_at":"2025-12-24T23:33:58.635002347-05:00","closed_at":"2025-12-24T23:33:58.635002347-05:00","close_reason":"LSP servers deployed (gopls, pyright, typescript-language-server, rust-analyzer, nil, elixir-ls); ENABLE_LSP_TOOL=1 in sessionVariables; requires new session to test"}
|
||||
|
|
|
|||
54
skills/code-review/README.md
Normal file
54
skills/code-review/README.md
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
# Code Review Skill
|
||||
|
||||
Multi-lens code review with interactive issue filing.
|
||||
|
||||
## Quick Start
|
||||
|
||||
```
|
||||
/code-review # Review uncommitted changes
|
||||
/code-review src/ # Review directory
|
||||
/code-review file.py # Review specific file
|
||||
```
|
||||
|
||||
## Lenses
|
||||
|
||||
Four focused review perspectives:
|
||||
|
||||
- **bloat** - Size, complexity, SRP violations
|
||||
- **smells** - Code smells, naming, readability
|
||||
- **dead-code** - Unused code, zombie functions
|
||||
- **redundancy** - Duplication, YAGNI violations
|
||||
|
||||
## Workflow
|
||||
|
||||
1. Select target files
|
||||
2. Run all lenses
|
||||
3. Synthesize and rank findings
|
||||
4. Present summary to user
|
||||
5. File issues on approval
|
||||
|
||||
## Design Philosophy
|
||||
|
||||
- **Interactive by default** - No automatic issue spam
|
||||
- **LLM-in-the-loop** - Agent applies judgment at every step
|
||||
- **Orch optional** - Multi-model consensus for high-stakes reviews
|
||||
- **Beads integration** - Issues filed with proper context
|
||||
|
||||
## Files
|
||||
|
||||
```
|
||||
code-review/
|
||||
├── SKILL.md # Main skill prompt
|
||||
├── README.md # This file
|
||||
└── lenses/ # Bundled lens prompts
|
||||
├── bloat.md
|
||||
├── smells.md
|
||||
├── dead-code.md
|
||||
└── redundancy.md
|
||||
```
|
||||
|
||||
## See Also
|
||||
|
||||
- `~/.config/lenses/` - System-wide lens deployment
|
||||
- `bd create` - Beads issue creation
|
||||
- `orch consensus` - Multi-model filtering
|
||||
177
skills/code-review/SKILL.md
Normal file
177
skills/code-review/SKILL.md
Normal file
|
|
@ -0,0 +1,177 @@
|
|||
---
|
||||
name: code-review
|
||||
description: Run multi-lens code review on target files. Analyzes for bloat, smells, dead-code, and redundancy. Interactive - asks before filing issues.
|
||||
---
|
||||
|
||||
# Code Review Skill
|
||||
|
||||
Run focused code analysis using multiple review lenses. Findings are synthesized and presented for your approval before any issues are filed.
|
||||
|
||||
## When to Use
|
||||
|
||||
Invoke this skill when:
|
||||
- "Review this code"
|
||||
- "Run code review on src/"
|
||||
- "Check this file for issues"
|
||||
- "Analyze the codebase"
|
||||
- `/code-review`
|
||||
|
||||
## Arguments
|
||||
|
||||
The skill accepts an optional target:
|
||||
- `/code-review` - Reviews recently changed files (git diff)
|
||||
- `/code-review src/` - Reviews specific directory
|
||||
- `/code-review src/main.py` - Reviews specific file
|
||||
- `/code-review --diff HEAD~5` - Reviews changes in last 5 commits
|
||||
|
||||
## Available Lenses
|
||||
|
||||
Lenses are focused review prompts located in `~/.config/lenses/`:
|
||||
|
||||
| Lens | Focus |
|
||||
|------|-------|
|
||||
| `bloat.md` | File size, function length, complexity, SRP violations |
|
||||
| `smells.md` | Code smells, naming, control flow, readability |
|
||||
| `dead-code.md` | Unused exports, zombie code, unreachable paths |
|
||||
| `redundancy.md` | Duplication, parallel systems, YAGNI violations |
|
||||
|
||||
## Workflow
|
||||
|
||||
### Phase 1: Target Selection
|
||||
1. Parse the target argument (default: git diff of uncommitted changes)
|
||||
2. Identify files to review
|
||||
3. Show file list to user for confirmation
|
||||
|
||||
### Phase 2: Lens Execution
|
||||
For each lens, analyze the target files:
|
||||
|
||||
1. Read the lens prompt from `~/.config/lenses/{lens}.md`
|
||||
2. Apply the lens to the target code
|
||||
3. Collect findings in structured format
|
||||
|
||||
**Finding Format:**
|
||||
```
|
||||
[TAG] <severity:HIGH|MED|LOW> <file:line>
|
||||
Issue: <one-line description>
|
||||
Suggest: <actionable fix>
|
||||
Evidence: <why this matters>
|
||||
```
|
||||
|
||||
### Phase 3: Synthesis
|
||||
After all lenses complete:
|
||||
1. Deduplicate overlapping findings
|
||||
2. Group related issues
|
||||
3. Rank by severity and confidence
|
||||
4. Generate summary report
|
||||
|
||||
**Optional:** If user requests consensus (`--orch` or asks for it):
|
||||
```bash
|
||||
orch consensus "<findings summary>" gpt gemini
|
||||
```
|
||||
Use this to filter false positives and prioritize.
|
||||
|
||||
### Phase 4: Interactive Review
|
||||
Present findings to user:
|
||||
1. Show executive summary (counts by severity)
|
||||
2. List top issues with details
|
||||
3. Ask: "Which findings should I file as issues?"
|
||||
|
||||
**User can respond:**
|
||||
- "File all" - creates beads issues for everything
|
||||
- "File HIGH only" - filters by severity
|
||||
- "File 1, 3, 5" - specific findings
|
||||
- "None" - just keep the report
|
||||
- "Let me review first" - show full details
|
||||
|
||||
### Phase 5: Issue Filing (if requested)
|
||||
For approved findings:
|
||||
1. Create beads issues with `bd create`
|
||||
2. Include lens tag, severity, file location
|
||||
3. Link related issues if applicable
|
||||
|
||||
## Output
|
||||
|
||||
The skill produces:
|
||||
1. **Console summary** - immediate feedback
|
||||
2. **Beads issues** - if user approves filing
|
||||
|
||||
## Example Session
|
||||
|
||||
```
|
||||
User: /code-review src/cli.py
|
||||
|
||||
Agent: I'll review src/cli.py with 4 lenses (bloat, smells, dead-code, redundancy).
|
||||
|
||||
[Running bloat lens...]
|
||||
[Running smells lens...]
|
||||
[Running dead-code lens...]
|
||||
[Running redundancy lens...]
|
||||
|
||||
## Review Summary: src/cli.py
|
||||
|
||||
| Severity | Count |
|
||||
|----------|-------|
|
||||
| HIGH | 1 |
|
||||
| MED | 3 |
|
||||
| LOW | 2 |
|
||||
|
||||
### Top Issues
|
||||
|
||||
1. [BLOAT] HIGH src/cli.py:145-280
|
||||
Issue: Function `handle_request` is 135 lines
|
||||
Suggest: Extract into smaller functions by responsibility
|
||||
|
||||
2. [SMELL] MED src/cli.py:89
|
||||
Issue: Magic number 3600 without explanation
|
||||
Suggest: Extract to named constant SECONDS_PER_HOUR
|
||||
|
||||
3. [DEAD] MED src/cli.py:12
|
||||
Issue: Import `unused_module` has no references
|
||||
Suggest: Remove unused import
|
||||
|
||||
Would you like me to file any of these as beads issues?
|
||||
Options: all, HIGH only, specific numbers (1,2,3), or none
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
The skill respects `.code-review.yml` in the repo root if present:
|
||||
|
||||
```yaml
|
||||
# Optional configuration
|
||||
ignore_paths:
|
||||
- vendor/
|
||||
- node_modules/
|
||||
- "*.generated.*"
|
||||
|
||||
severity_defaults:
|
||||
bloat: MED
|
||||
dead-code: LOW
|
||||
|
||||
max_file_size_kb: 500 # Skip files larger than this
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
1. **Be Thorough But Focused** - Each lens checks one concern deeply
|
||||
2. **Evidence Over Opinion** - Cite specific lines and patterns
|
||||
3. **Actionable Suggestions** - Every finding needs a clear fix
|
||||
4. **Respect User Time** - Summarize first, details on request
|
||||
5. **No Spam** - Don't file issues without explicit approval
|
||||
|
||||
## Process Checklist
|
||||
|
||||
1. [ ] Parse target (files/directory/diff)
|
||||
2. [ ] Confirm scope with user if large (>10 files)
|
||||
3. [ ] Run each lens, collecting findings
|
||||
4. [ ] Deduplicate and rank findings
|
||||
5. [ ] Present summary to user
|
||||
6. [ ] Ask which findings to file
|
||||
7. [ ] Create beads issues for approved findings
|
||||
8. [ ] Report issue IDs created
|
||||
|
||||
## Integration
|
||||
|
||||
- **Lenses**: Read from `~/.config/lenses/*.md`
|
||||
- **Issue Tracking**: Uses `bd create` for beads issues
|
||||
- **Orch**: Optional consensus filtering via `orch consensus`
|
||||
55
skills/code-review/lenses/README.md
Normal file
55
skills/code-review/lenses/README.md
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
# Lenses
|
||||
|
||||
Focused prompts for multi-perspective code analysis.
|
||||
|
||||
## Philosophy
|
||||
|
||||
Each lens examines code through one specific concern. Multiply lenses × models for diverse perspectives.
|
||||
|
||||
## Usage
|
||||
|
||||
**Single lens, multiple models:**
|
||||
```bash
|
||||
orch consensus "$(cat ~/.config/lenses/bloat.md)" flash gemini gpt --file src/target.py
|
||||
```
|
||||
|
||||
**Pipe file content:**
|
||||
```bash
|
||||
cat src/target.py | orch consensus "$(cat ~/.config/lenses/smells.md)" flash gemini
|
||||
```
|
||||
|
||||
**Convergent review (multiple passes):**
|
||||
```bash
|
||||
for lens in bloat smells dead-code; do
|
||||
echo "=== $lens ==="
|
||||
orch chat "$(cat ~/.config/lenses/$lens.md)" --model gemini --file src/target.py
|
||||
done
|
||||
```
|
||||
|
||||
## Available Lenses
|
||||
|
||||
| Lens | Focus |
|
||||
|------|-------|
|
||||
| `bloat.md` | File size, function length, complexity metrics |
|
||||
| `smells.md` | Code smells, readability, naming issues |
|
||||
| `dead-code.md` | Unused exports, zombie code, unreachable paths |
|
||||
| `redundancy.md` | Duplicate systems, YAGNI, consolidation opportunities |
|
||||
|
||||
## Planned Lenses
|
||||
|
||||
- `coverage.md` - Test gaps, untested critical paths
|
||||
- `coupling.md` - Tight coupling, leaky abstractions
|
||||
- `security.md` - OWASP, injection, auth
|
||||
- `perf.md` - Bottlenecks, N+1 queries
|
||||
- `architecture.md` - Boundaries, data flow, trade-offs
|
||||
|
||||
## Output Convention
|
||||
|
||||
All lenses output findings in a consistent format:
|
||||
```
|
||||
[TAG] <severity:HIGH|MED|LOW> <file:line>
|
||||
Issue: <description>
|
||||
Suggest: <action>
|
||||
```
|
||||
|
||||
This enables parsing and automated beads creation.
|
||||
37
skills/code-review/lenses/bloat.md
Normal file
37
skills/code-review/lenses/bloat.md
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
# Bloat Review Lens
|
||||
|
||||
Review the provided code for **size and complexity bloat**.
|
||||
|
||||
## What to Look For
|
||||
|
||||
### File-Level
|
||||
- Files over 300 lines (warning) or 500 lines (critical)
|
||||
- Files with multiple unrelated responsibilities (SRP violations)
|
||||
- God objects or god modules that do too much
|
||||
|
||||
### Function-Level
|
||||
- Functions over 50 lines
|
||||
- Functions with more than 5 parameters
|
||||
- Functions that could be split into smaller, named steps
|
||||
|
||||
### Complexity
|
||||
- Cyclomatic complexity hotspots (many branches/loops)
|
||||
- Deeply nested logic (3+ levels of indentation)
|
||||
- Long method chains that obscure intent
|
||||
|
||||
## Output Format
|
||||
|
||||
For each finding:
|
||||
```
|
||||
[BLOAT] <severity:HIGH|MED|LOW> <file:line>
|
||||
Issue: <what's bloated>
|
||||
Why: <why this is a problem>
|
||||
Suggest: <concrete action - split into X, extract Y, etc.>
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
- Be specific about line counts and locations
|
||||
- Prioritize by impact (high-traffic code > rarely-used)
|
||||
- Don't flag intentionally comprehensive files (test fixtures, generated code)
|
||||
- Focus on actionable splits, not just complaints
|
||||
49
skills/code-review/lenses/dead-code.md
Normal file
49
skills/code-review/lenses/dead-code.md
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
# Dead Code Review Lens
|
||||
|
||||
Review the provided code for **unused, unreachable, or obsolete code**.
|
||||
|
||||
## What to Look For
|
||||
|
||||
### Zombie Code
|
||||
- Commented-out code blocks (should be deleted, git has history)
|
||||
- Functions/methods that are defined but never called
|
||||
- Exported symbols that are never imported elsewhere
|
||||
- Variables assigned but never read
|
||||
- Parameters passed but never used
|
||||
|
||||
### Unreachable Code
|
||||
- Code after unconditional return/throw/break
|
||||
- Branches that can never execute (dead conditions)
|
||||
- Error handlers for errors that can't occur
|
||||
- Feature flags that are always on/off
|
||||
|
||||
### Obsolete Code
|
||||
- Compatibility shims for deprecated dependencies
|
||||
- Polyfills for features now universally supported
|
||||
- Migration code that has already run
|
||||
- TODO comments older than 6 months with no activity
|
||||
- Version checks for versions no longer supported
|
||||
|
||||
### Import/Dependency Cruft
|
||||
- Imported modules that are never used
|
||||
- Dependencies in package manifest but unused in code
|
||||
- Conditional imports that never trigger
|
||||
|
||||
## Output Format
|
||||
|
||||
For each finding:
|
||||
```
|
||||
[DEAD] <severity:HIGH|MED|LOW> <file:line>
|
||||
Type: <zombie|unreachable|obsolete|cruft>
|
||||
Issue: <what's dead>
|
||||
Evidence: <how you know it's unused>
|
||||
Suggest: <delete|archive|verify-then-delete>
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
- HIGH = definitely dead, safe to delete
|
||||
- MED = likely dead, needs verification (grep for dynamic usage)
|
||||
- LOW = possibly dead, context-dependent
|
||||
- Be careful with: reflection, dynamic imports, CLI entrypoints, test fixtures
|
||||
- When uncertain, suggest verification steps rather than immediate deletion
|
||||
53
skills/code-review/lenses/redundancy.md
Normal file
53
skills/code-review/lenses/redundancy.md
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
# Redundancy Review Lens
|
||||
|
||||
Review the provided code for **duplication, parallel systems, and YAGNI violations**.
|
||||
|
||||
## What to Look For
|
||||
|
||||
### Logic Duplication
|
||||
- Near-identical functions with minor variations
|
||||
- Copy-pasted code blocks across files
|
||||
- Same validation logic repeated in multiple places
|
||||
- Parallel implementations that should be unified
|
||||
|
||||
### System Redundancy
|
||||
- Multiple logging systems (pick one)
|
||||
- Multiple HTTP clients or request wrappers
|
||||
- Multiple config/env loading mechanisms
|
||||
- Multiple error handling patterns
|
||||
- Multiple state management approaches
|
||||
- Parallel database abstractions
|
||||
|
||||
### YAGNI (You Ain't Gonna Need It)
|
||||
- Abstractions with only one implementation
|
||||
- Interfaces extracted prematurely
|
||||
- Generic solutions for specific problems
|
||||
- Configuration for things that never change
|
||||
- Plugin systems with no plugins
|
||||
- Extensibility hooks never used
|
||||
|
||||
### Consolidation Opportunities
|
||||
- Utility functions that could be a shared library
|
||||
- Constants defined in multiple places
|
||||
- Type definitions duplicated across modules
|
||||
- Similar data transformations in different contexts
|
||||
|
||||
## Output Format
|
||||
|
||||
For each finding:
|
||||
```
|
||||
[REDUNDANT] <severity:HIGH|MED|LOW> <file:line>
|
||||
Type: <duplication|parallel-system|yagni|consolidation>
|
||||
Issue: <what's redundant>
|
||||
Related: <other locations with same/similar code>
|
||||
Suggest: <consolidate into X|delete Y and use X|simplify to Z>
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
- HIGH = active maintenance burden, divergence risk
|
||||
- MED = wasted complexity, cognitive overhead
|
||||
- LOW = minor duplication, tolerable
|
||||
- Consider: is the duplication intentional? (sometimes 2 similar things should stay separate)
|
||||
- For system redundancy: recommend which one to standardize on and why
|
||||
- For YAGNI: distinguish "not yet needed" from "will never need"
|
||||
46
skills/code-review/lenses/smells.md
Normal file
46
skills/code-review/lenses/smells.md
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
# Code Smells Review Lens
|
||||
|
||||
Review the provided code for **code smells and readability issues**.
|
||||
|
||||
## What to Look For
|
||||
|
||||
### Control Flow Smells
|
||||
- Deep nesting (if inside if inside if...)
|
||||
- Complex boolean expressions without named variables
|
||||
- Early returns that could simplify flow vs. arrow code
|
||||
- Switch/match statements that should be polymorphism or lookup tables
|
||||
|
||||
### Data Smells
|
||||
- Primitive obsession (passing 5 strings instead of an object)
|
||||
- Data clumps (same group of variables passed together repeatedly)
|
||||
- Feature envy (function uses another module's data more than its own)
|
||||
- Mutable state where immutable would work
|
||||
|
||||
### Naming & Clarity
|
||||
- Names that lie (function does more/less than name suggests)
|
||||
- Abbreviated names that obscure meaning
|
||||
- Generic names (data, info, result, tmp, val)
|
||||
- Inconsistent naming conventions within the file
|
||||
|
||||
### Structure Smells
|
||||
- Comments that explain "what" instead of "why" (code should be self-documenting)
|
||||
- Dead parameters (passed but never used)
|
||||
- Flag arguments (boolean that changes function behavior)
|
||||
- Speculative generality (abstraction without current need)
|
||||
|
||||
## Output Format
|
||||
|
||||
For each finding:
|
||||
```
|
||||
[SMELL] <severity:HIGH|MED|LOW> <file:line>
|
||||
Smell: <name of the smell>
|
||||
Issue: <what's wrong>
|
||||
Suggest: <refactoring technique - extract method, rename, introduce parameter object, etc.>
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
- Reference established refactoring patterns (Fowler's catalog) when applicable
|
||||
- Distinguish style preferences from genuine smells
|
||||
- Consider the language idioms (what's a smell in Java may be fine in Python)
|
||||
- HIGH = actively misleading or bug-prone, MED = maintainability drag, LOW = polish
|
||||
Loading…
Reference in a new issue