From b8f7928db595afb1c534dff040576adf73f9dee3 Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 31 Dec 2025 22:34:28 -0500 Subject: [PATCH] refactor: single source of truth for lenses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove top-level lenses/ dir (was stale with only 4 lenses) - Update ai-skills module to source from skills/code-review/lenses/ - Fix visit.py: extract DEFAULT_WAIT_MS, use specific exception handlers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- lenses/README.md | 55 ------------------------ lenses/bloat.md | 37 ---------------- lenses/dead-code.md | 49 --------------------- lenses/redundancy.md | 53 ----------------------- lenses/smells.md | 46 -------------------- modules/ai-skills.nix | 4 +- skills/playwright-visit/scripts/visit.py | 18 +++++--- 7 files changed, 13 insertions(+), 249 deletions(-) delete mode 100644 lenses/README.md delete mode 100644 lenses/bloat.md delete mode 100644 lenses/dead-code.md delete mode 100644 lenses/redundancy.md delete mode 100644 lenses/smells.md diff --git a/lenses/README.md b/lenses/README.md deleted file mode 100644 index 8595da2..0000000 --- a/lenses/README.md +++ /dev/null @@ -1,55 +0,0 @@ -# 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/lenses/bloat.md b/lenses/bloat.md deleted file mode 100644 index 73b4d58..0000000 --- a/lenses/bloat.md +++ /dev/null @@ -1,37 +0,0 @@ -# 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/lenses/dead-code.md b/lenses/dead-code.md deleted file mode 100644 index 9aa09ff..0000000 --- a/lenses/dead-code.md +++ /dev/null @@ -1,49 +0,0 @@ -# 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/lenses/redundancy.md b/lenses/redundancy.md deleted file mode 100644 index 1e6fe5e..0000000 --- a/lenses/redundancy.md +++ /dev/null @@ -1,53 +0,0 @@ -# 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/lenses/smells.md b/lenses/smells.md deleted file mode 100644 index 0764803..0000000 --- a/lenses/smells.md +++ /dev/null @@ -1,46 +0,0 @@ -# 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 diff --git a/modules/ai-skills.nix b/modules/ai-skills.nix index e6cd7ab..5a12250 100644 --- a/modules/ai-skills.nix +++ b/modules/ai-skills.nix @@ -88,10 +88,10 @@ in { ) )) - # Lenses for orch + # Lenses for orch (sourced from code-review skill) (mkIf cfg.enableLenses { ".config/lenses" = { - source = "${repoRoot}/lenses"; + source = "${cfg.skillsPath}/code-review/lenses"; recursive = true; }; }) diff --git a/skills/playwright-visit/scripts/visit.py b/skills/playwright-visit/scripts/visit.py index 17ad5ee..7f70611 100755 --- a/skills/playwright-visit/scripts/visit.py +++ b/skills/playwright-visit/scripts/visit.py @@ -5,9 +5,10 @@ import argparse import sys from pathlib import Path -from playwright.sync_api import sync_playwright +from playwright.sync_api import sync_playwright, Error as PlaywrightError CHROMIUM_PATH = "/run/current-system/sw/bin/chromium" +DEFAULT_WAIT_MS = 1000 def get_browser(playwright): @@ -77,7 +78,7 @@ def main(): p_screenshot.add_argument("url", help="URL to visit") p_screenshot.add_argument("output", help="Output PNG path") p_screenshot.add_argument( - "--wait", type=int, default=1000, help="Wait after load (ms)" + "--wait", type=int, default=DEFAULT_WAIT_MS, help="Wait after load (ms)" ) p_screenshot.add_argument( "--full-page", action="store_true", help="Capture full scrollable page" @@ -87,20 +88,20 @@ def main(): # Text command p_text = subparsers.add_parser("text", help="Extract visible text content") p_text.add_argument("url", help="URL to visit") - p_text.add_argument("--wait", type=int, default=1000, help="Wait after load (ms)") + p_text.add_argument("--wait", type=int, default=DEFAULT_WAIT_MS, help="Wait after load (ms)") p_text.set_defaults(func=cmd_text) # HTML command p_html = subparsers.add_parser("html", help="Get rendered HTML") p_html.add_argument("url", help="URL to visit") - p_html.add_argument("--wait", type=int, default=1000, help="Wait after load (ms)") + p_html.add_argument("--wait", type=int, default=DEFAULT_WAIT_MS, help="Wait after load (ms)") p_html.set_defaults(func=cmd_html) # PDF command p_pdf = subparsers.add_parser("pdf", help="Save page as PDF") p_pdf.add_argument("url", help="URL to visit") p_pdf.add_argument("output", help="Output PDF path") - p_pdf.add_argument("--wait", type=int, default=1000, help="Wait after load (ms)") + p_pdf.add_argument("--wait", type=int, default=DEFAULT_WAIT_MS, help="Wait after load (ms)") p_pdf.set_defaults(func=cmd_pdf) args = parser.parse_args() @@ -112,8 +113,11 @@ def main(): try: args.func(args) - except Exception as e: - print(f"Error: {e}", file=sys.stderr) + except PlaywrightError as e: + print(f"Browser error: {e}", file=sys.stderr) + sys.exit(1) + except OSError as e: + print(f"File error: {e}", file=sys.stderr) sys.exit(1)