From 5ebae8f86ed3da2a38f5fb420e6041651609ba0c Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 26 Dec 2025 23:22:51 -0500 Subject: [PATCH] feat: add /code-review skill with bundled lenses --- .beads/issues.jsonl | 16 +-- skills/code-review/README.md | 54 ++++++++ skills/code-review/SKILL.md | 177 ++++++++++++++++++++++++ skills/code-review/lenses/README.md | 55 ++++++++ skills/code-review/lenses/bloat.md | 37 +++++ skills/code-review/lenses/dead-code.md | 49 +++++++ skills/code-review/lenses/redundancy.md | 53 +++++++ skills/code-review/lenses/smells.md | 46 ++++++ 8 files changed, 479 insertions(+), 8 deletions(-) create mode 100644 skills/code-review/README.md create mode 100644 skills/code-review/SKILL.md create mode 100644 skills/code-review/lenses/README.md create mode 100644 skills/code-review/lenses/bloat.md create mode 100644 skills/code-review/lenses/dead-code.md create mode 100644 skills/code-review/lenses/redundancy.md create mode 100644 skills/code-review/lenses/smells.md diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 9947cf5..988dded 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -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"} diff --git a/skills/code-review/README.md b/skills/code-review/README.md new file mode 100644 index 0000000..9a56e6e --- /dev/null +++ b/skills/code-review/README.md @@ -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 diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md new file mode 100644 index 0000000..af57574 --- /dev/null +++ b/skills/code-review/SKILL.md @@ -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] +Issue: +Suggest: +Evidence: +``` + +### 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 "" 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` diff --git a/skills/code-review/lenses/README.md b/skills/code-review/lenses/README.md new file mode 100644 index 0000000..8595da2 --- /dev/null +++ b/skills/code-review/lenses/README.md @@ -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] +Issue: +Suggest: +``` + +This enables parsing and automated beads creation. diff --git a/skills/code-review/lenses/bloat.md b/skills/code-review/lenses/bloat.md new file mode 100644 index 0000000..73b4d58 --- /dev/null +++ b/skills/code-review/lenses/bloat.md @@ -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] +Issue: +Why: +Suggest: +``` + +## 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 diff --git a/skills/code-review/lenses/dead-code.md b/skills/code-review/lenses/dead-code.md new file mode 100644 index 0000000..9aa09ff --- /dev/null +++ b/skills/code-review/lenses/dead-code.md @@ -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] +Type: +Issue: +Evidence: +Suggest: +``` + +## 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 diff --git a/skills/code-review/lenses/redundancy.md b/skills/code-review/lenses/redundancy.md new file mode 100644 index 0000000..1e6fe5e --- /dev/null +++ b/skills/code-review/lenses/redundancy.md @@ -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] +Type: +Issue: +Related: +Suggest: +``` + +## 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" diff --git a/skills/code-review/lenses/smells.md b/skills/code-review/lenses/smells.md new file mode 100644 index 0000000..0764803 --- /dev/null +++ b/skills/code-review/lenses/smells.md @@ -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] +Smell: +Issue: +Suggest: +``` + +## 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