diff --git a/global/tools/code-modernization/SKILL.md b/global/tools/code-modernization/SKILL.md index ace1213..7deed54 100644 --- a/global/tools/code-modernization/SKILL.md +++ b/global/tools/code-modernization/SKILL.md @@ -1,15 +1,173 @@ --- -name: code-modernization -description: Modernize or refactor a legacy codebase incrementally while preserving business logic. Use for migrations (framework/language/version upgrades), killing tech debt, refactoring an old project, or "bring this up to date". Mirrors Anthropic's code-modernization methodology. +name: code-modernizer +description: >- + Modernize or refactor a legacy codebase incrementally while preserving business + logic. Replaces hand-rolled logic with the standard library, a native feature, + an already-installed dependency, or (only after live research) a well-vetted new + package, then applies minimal design patterns to whatever custom code remains. + Confirms scope first (full-codebase, one module/partial, or only the exact lines + named) and flags file count, line count, and breaking-change risk before + generating a large diff, asking instead of assuming when scope, a library choice, + or a behavior change is unclear. + Use for migrations (framework/language/version upgrades), killing tech debt, + refactoring an old project, or "bring this up to date". Use whenever the user + asks to refactor, modernize, de-bloat, clean up, simplify, upgrade, or audit + code; asks "is there a library for this", "don't reinvent the wheel", "replace + with a framework", "reduce boilerplate", or "apply proper design patterns"; wants + a root-cause fix over a quick patch; says "full upgrade" vs "partial upgrade" vs + "just fix this one thing"; or wants a legacy-code/tech-debt audit or a + dependency-choice recommendation. +license: MIT +attribution: >- + This skill's ladder is adapted from DietrichGebert/ponytail (MIT licensed). + See "Relationship to ponytail" below. --- -# Code modernization +# Code Modernizer -For legacy/dated codebases (several of yours qualify). Modernize in safe, -verifiable increments — never a big-bang rewrite. Keep a human in the loop at each -gate. +## Philosophy -## 1. Assess (read-only first) +A senior engineer isn't measured by how much code they wrote today, but by how +little the system needs. Before touching anything, decide whether the code should +be replaced with something already established. If custom code is genuinely +necessary, write the least of it that solves the real problem without dropping +behavior anyone relies on. + +This skill's core decision procedure is adapted from Dietrich Gebert's `ponytail` +project (MIT licensed, github.com/DietrichGebert/ponytail). It extends ponytail +two ways: it applies the same ladder retroactively to audit code that already +exists, not just to gate new code being written, and it adds scope control, a +pre-flight cost/impact gate, and a mandatory live-research step for whenever a +genuinely new dependency is on the table. See "Relationship to ponytail" at the +end. + +## The ladder + +Before writing, keeping, or replacing any piece of logic, stop at the first rung +that holds: + +1. **Does this need to exist at all?** (YAGNI). If a feature isn't used, delete it + instead of modernizing it. +2. **Already solved elsewhere in this codebase?** Reuse it, don't rewrite it. +3. **Standard library does it?** Use it. +4. **Native platform feature does it?** Use it. The browser already has + ``, so don't reach for a date-picker library. +5. **Already-installed dependency does it?** Use it. Zero new install cost. +6. **Is a new dependency genuinely justified?** Only after the research pass in + Step 3 below. Never skip straight here. +7. **Can what's left be one line?** Make it one line. +8. **Only then:** write the minimum code that works, shaped by whichever design + pattern (if any) removes real duplication. See + `references/design-patterns-cheatsheet.md`. + +The ladder runs after understanding the problem, not instead of it. Read the code +and its tests before picking a rung (Step 2). + +## Two modes, same ladder + +- **Forward** (writing new code): apply the ladder before writing anything. +- **Audit** (existing code): apply the same ladder retroactively. For every + hand-rolled block in scope, ask: "would rung 3, 4, 5, or a researched rung 6 + now produce less code, or safer code, than what's already here?" If yes, and the + check in Step 4 clears it, propose the swap. This audit mode is the part + `ponytail` doesn't cover. It governs new code; this skill governs the code + that's already sitting in the repo. + +## Step 0: Scope handshake (every time, before any real work) + +Refactoring has a blast radius. Before generating anything beyond a one-file, +one-function fix, pin down which of these three the user wants: + +- **Full**: a comprehensive pass across the named codebase or repo. Expect + multiple files and possibly breaking changes; produce a migration note. +- **Partial**: bounded to one named module, directory, or feature area. No + drive-by edits outside that boundary, even if something nearby looks equally + outdated. +- **Targeted**: only the exact file, function, or lines named. Nothing else + changes. + +If the request already states scope clearly ("just fix this function", "modernize +the whole `utils/` folder"), proceed without asking. If it's ambiguous ("clean up +this codebase", "make this better"), ask one direct question offering these three +options before reading any more files than needed to ask it well. Don't default to +Full because it seems more thorough. Full is the most expensive and highest-risk +option, not the safe default. + +## Step 1: Pre-flight scan and cost/impact flag (Full/Partial only, skip for Targeted) + +Ground the estimate in real numbers instead of a guess. Run: + +``` +python3 scripts/preflight_scan.py +``` + +It's stdlib-only Python, nothing to install. Report back to the user in a short +block before generating anything: + +``` +Scope: on +~ files, ~ lines in range +Dependencies already available: +Flagged high-cost: +Estimated: , breaking-change risk +Proceed as scoped, narrow it, or switch to Targeted? +``` + +Treat anything the scan flags as high-cost as its own decision point. Offer full +detail there or a summarized diff, rather than silently generating all of it. Full +rubric, including the flexibility trade-off, in `references/cost-impact-preflight.md`. + +## Step 2: Read before you touch (root cause, not a patch) + +Before proposing any swap, read what's actually there: the function, its tests, +nearby comments, and the commit that introduced the "ugly" version if it's +available. Hand-rolled code is sometimes just unmaintained. Sometimes it's +load-bearing for an edge case the clean replacement doesn't handle. Confirm which +one is in front of you before replacing it. If the tests don't cover a case the +old code was clearly written for, say so instead of silently dropping it. + +## Step 3: Research before recommending a new dependency (mandatory, live) + +Rungs 1 through 5 need no research; they're free and already known to be safe. +Rung 6, a genuinely new dependency, is the one place memory goes stale fast: a +library that was the obvious choice eighteen months ago can be unmaintained, +superseded, or carrying an open CVE today. Never recommend a package from memory +alone. Check its current state first, live, with search. Checklist, +current-as-of-lookup sources, and red/green flags are in +`references/research-protocol.md`. Summarize the choice in one or two sentences +per swap in the final answer; this step is not the place to write an essay. + +## Step 4: Impact vs. flexibility, before writing the change + +Before writing the replacement, work out: + +- who else calls this (the blast radius may be bigger than the one call site that + prompted the change), +- what flexibility the current code buys and whether anyone actually uses it (grep + before removing), and +- whether the migration cost (call-site updates, type changes, test churn) is + worth what's gained. + +If the trade isn't clearly worth it, say what was found and ask. The user knows +release timing and risk appetite that the code alone doesn't reveal. Full rubric +in `references/cost-impact-preflight.md`. + +## Step 5: Output discipline + +- Show the diff or the changed function, not the whole file, unless the whole file + is asked for. +- One consolidated summary per batch of related changes, not a running commentary + per file. +- Batch logically related edits together instead of narrating each one separately. +- Don't restate the ladder or the research trail in the final answer. The user + needs the change and a one-line reason, not the process that produced it. + +## Incremental modernization methodology (Full/Partial scope) + +For legacy/dated codebases, modernize in safe, verifiable increments — never a +big-bang rewrite. Keep a human in the loop at each gate. + +### Assess (read-only first) - Map dependencies and the module graph; identify dead code and the highest-churn, highest-complexity, highest-business-value hotspots. - Use the `serena` LSP + `scout` subagent so this exploration doesn't fill main @@ -17,29 +175,101 @@ gate. - Verify target frameworks/versions with `tech-selector` (current best, not training-data defaults). -## 2. Safety net before changing anything +### Safety net before changing anything - Characterize current behavior with tests. If coverage is thin on the code you'll touch, add regression tests that pin the *existing* output first — so a refactor that changes behavior fails loudly. -## 3. Transform incrementally +### Transform incrementally - One bounded slice at a time (a module, a route, a component). Preserve business logic exactly; modernize the form around it. - After each slice: run tests + build + lint, and for UI use `ui-workflow`'s screenshot check. Commit per slice with a conventional message so each step is revertible. -## 4. Verify each slice +### Verify each slice - Tests green, build passes, behavior unchanged (diff against the pinned regression output). Use the `verifier` subagent on non-trivial slices. - Patch security issues surfaced along the way (semgrep is installed) but keep them as separate commits. -## 5. Document +### Document - Update `MODERNIZATION.md` with what changed and why; capture any institutional knowledge recovered from the old code before it's lost. -## Guardrails +### Guardrails - Business continuity first: if a slice can't be proven behavior-preserving, stop and surface it rather than guessing. - No scope creep — modernize what the slice covers, not everything you notice. + +## Never lazy about (unchanged from ponytail) + +Input validation at trust boundaries, error handling that prevents data loss, +security, accessibility, and anything the user explicitly asked for. These are +never traded away for a smaller diff, no matter which rung of the ladder applies. + +## Marking deferred opportunities + +When a swap is correct in principle but out of scope for a Targeted request, don't +make it. Leave a one-line marker instead, mirroring ponytail's `ponytail:` comment: + +```js +// modernize: could use structuredClone() (Node 17+) instead of this deep-clone helper +``` + +At the end of a session, list any `modernize:` markers left behind, so "later" +leaves a paper trail instead of quietly becoming "never." + +## Ask, don't assume + +Stop and ask one direct question, don't proceed on a guess, when: + +- Scope isn't stated and can't be inferred confidently (Step 0). +- Two well-maintained libraries are both reasonable and the right one depends on a + constraint the user hasn't stated (bundle size vs. features, already using a + sibling package from one ecosystem, etc.). +- The swap is a breaking change to a public API or an exported function. +- The "clean" replacement doesn't obviously cover an edge case the current code + handles. + +## Example + +**Request:** "This `debounce` function in `hooks/useDebounce.ts` looks +hand-rolled, can you clean it up?" + +This names an exact file and function, so it's **Targeted**; skip Step 1. Step 2: +read it and its tests; it correctly cancels on unmount, which a naive replacement +might miss. Step 3: `lodash` is already an installed dependency (rung 5) with a +well-tested `debounce`, so no new dependency and no research pass are needed. +Step 4: one call site, low risk, no flexibility lost. Result: a 3-line wrapper +around `lodash.debounce` that preserves the unmount-cancel behavior, with a +one-line note on why. Not a rewrite, not an essay, not a new package. + +## Relationship to ponytail + +This skill deliberately does not re-implement ponytail's own commands +(`/ponytail-review`, `/ponytail-audit`, `/ponytail-debt`, its lite/full/ultra +intensity dial). That would be exactly the kind of reinventing this skill exists +to prevent. For governing new code as it's written, install ponytail itself in +Claude Code: it's free, MIT-licensed, actively maintained, and benchmarked at +roughly half the code and 20% lower cost against a no-skill baseline. + +``` +/plugin marketplace add DietrichGebert/ponytail +/plugin install ponytail@ponytail +``` + +The two compose cleanly. Ponytail keeps new code minimal as it's written; this +skill audits and modernizes what's already in the repo, with the scope control, +cost/impact gate, and live research discipline the ladder alone doesn't cover. + +## Reference files (load only when the step above points here) + +- `references/research-protocol.md`: vetting a candidate library, current sources + to check, red/green flags, decision template. +- `references/design-patterns-cheatsheet.md`: smell → pattern → when to skip it, + compact table. +- `references/cost-impact-preflight.md`: full pre-flight output template and the + impact/flexibility rubric. +- `scripts/preflight_scan.py`: stdlib-only scanner for file count, LOC, and + dependency manifests. diff --git a/global/tools/code-modernization/references/cost-impact-preflight.md b/global/tools/code-modernization/references/cost-impact-preflight.md new file mode 100644 index 0000000..d5c663a --- /dev/null +++ b/global/tools/code-modernization/references/cost-impact-preflight.md @@ -0,0 +1,54 @@ +# Cost / impact pre-flight and flexibility rubric + +## Pre-flight output template (Step 1) + +Run `python3 scripts/preflight_scan.py ` and report: + +``` +Scope: on +~ files, ~ lines in range +Dependencies already available: +Flagged high-cost: +Estimated: , breaking-change risk +Proceed as scoped, narrow it, or switch to Targeted? +``` + +### Breaking-change risk levels + +| Level | Criteria | +|---|---| +| **Low** | Internal/private functions only; no exported API changes; no type signature changes | +| **Medium** | Exported functions change signature but callers are all in-repo; or a dependency is swapped that has a slightly different API | +| **High** | Public API / SDK changes; cross-repo consumers possible; type changes that propagate through generics; removal of a feature flag or config option | + +### High-cost file handling + +When a file exceeds ~800 lines or the total pass touches 15–20+ files: + +1. Name the files explicitly in the pre-flight report. +2. Offer the user a choice: full detailed diff, summarized diff (key changes + only), or skip that file for now. +3. Don't silently generate a massive diff — the user may want to split the work + across PRs. + +## Impact vs. flexibility rubric (Step 4) + +Before writing each swap, evaluate: + +| Question | If yes → | If no → | +|---|---|---| +| Does anyone else call this? | Check all call sites; mention them in the summary | Safe to change in isolation | +| Does the current code handle edge cases the replacement doesn't? | Keep the edge-case handling; wrap the replacement if needed | Straight swap | +| Does the current API surface offer flexibility someone might rely on? | Grep for usage of that flexibility; if unused, remove | Keep the simpler replacement | +| Is the migration cost (call-site updates, type changes, test churn) < the maintenance cost of keeping the old code? | Proceed | Defer — leave a `// modernize:` marker | +| Does the swap change observable behavior (return types, error messages, side effects)? | Treat as a breaking change; flag in the pre-flight report | Transparent swap | + +### When to defer instead of swap + +- The swap is correct but the module is in active development (merge conflicts). +- The user said "Targeted" and the swap is outside the named scope. +- The migration cost is high and the old code works fine — it's debt, not a bug. +- Two equally good replacements exist and the choice depends on constraints the + user hasn't stated. + +In all these cases, leave a `// modernize:` marker and list it at session end. diff --git a/global/tools/code-modernization/references/design-patterns-cheatsheet.md b/global/tools/code-modernization/references/design-patterns-cheatsheet.md new file mode 100644 index 0000000..58f42f9 --- /dev/null +++ b/global/tools/code-modernization/references/design-patterns-cheatsheet.md @@ -0,0 +1,24 @@ +# Design patterns cheatsheet + +Apply a pattern only when it removes real duplication or coupling. If the code is +already clear and short, a pattern adds ceremony for no gain — skip it. + +| Smell | Pattern | When to skip | +|---|---|---| +| Copy-pasted logic with minor variations | **Strategy** — extract the varying part into a function/object, pass it in | Fewer than 3 copies, or the variations are trivial one-liners | +| Long `if/else` or `switch` choosing behavior by type/string | **Strategy** or **Map lookup** — a plain `Record` is usually enough | Only 2–3 branches and unlikely to grow | +| Object construction with many optional params | **Builder** or plain options object | A single config object with defaults already reads well | +| Multiple callers need the same setup/teardown around a core operation | **Template Method** (or just a higher-order function) | Only one caller — inline the setup | +| Need to react to state changes across unrelated modules | **Observer / Event emitter** | Two modules — a direct callback is simpler | +| Expensive object creation, same inputs → same output | **Flyweight / Cache / Memoize** | Object is cheap, or inputs rarely repeat | +| Access to a resource that needs lifecycle management | **Dispose / using / context manager** | Resource is process-scoped (no cleanup needed) | +| External API that doesn't match your domain model | **Adapter** | You control both sides — just change the source | +| Deep nesting of decorators or wrappers | **Middleware / Pipeline** — compose a flat list | Two wrappers max — nesting is still readable | +| Repeated null-checks or fallback chains | **Null Object** or optional chaining (`?.`) | Language already has `??` / `?.` — use it (rung 3/4) | + +## Anti-patterns to avoid + +- **Singleton for testability**: makes mocking hard. Prefer dependency injection. +- **Factory for one type**: a constructor call is simpler. +- **Abstract base class with one subclass**: remove the abstraction. +- **Pattern just to match a textbook**: the code is the product, not the diagram. diff --git a/global/tools/code-modernization/references/research-protocol.md b/global/tools/code-modernization/references/research-protocol.md new file mode 100644 index 0000000..41a61c4 --- /dev/null +++ b/global/tools/code-modernization/references/research-protocol.md @@ -0,0 +1,42 @@ +# Research protocol — vetting a candidate library (rung 6) + +Only reach this file when rungs 1–5 failed and a new dependency is genuinely on +the table. Never skip straight here. + +## Mandatory live checks + +Run these before recommending. Stale memory is the failure mode this protocol +exists to prevent. + +| Check | Source | Red flag | +|---|---|---| +| Last commit / release | GitHub releases or npm/PyPI page | > 12 months with open issues | +| Open CVEs | `npm audit` / `pip-audit` / Snyk DB / GitHub advisories | Any unpatched critical/high | +| Maintenance signal | Issue tracker, PR merge cadence | Dozens of unanswered issues, no maintainer response in months | +| Download trend | npm trends, PyPI stats | Steep decline or flatline vs. competitors | +| License compatibility | `package.json` / `setup.cfg` license field | Copyleft (GPL) in an MIT project, or no license | +| Bundle size (frontend) | bundlephobia.com or `import-cost` | > 50 kB gzipped for a single utility | +| Peer / transitive deps | `npm ls` / `pip show` | Pulls in a heavy tree for a small task | + +## Green flags (not required, but strengthen the case) + +- Active maintainer with a history of responding to security reports. +- TypeScript types shipped (not DefinitelyTyped-only). +- Used by well-known projects (check dependents on GitHub). +- Clear migration path if abandoned (small API surface, stdlib fallback exists). + +## Decision template (one per candidate) + +``` +Library: +Rung: 6 (new dependency) +Replaces: +Last release: | Weekly downloads: | License: +CVEs: +Bundle impact: <+N kB gzipped> (frontend only) +Verdict: adopt / skip / ask user +Reason (1–2 sentences): ... +``` + +Include this block in the final answer when recommending a new dependency. Keep it +short — the user needs the decision, not the research log. diff --git a/global/tools/code-modernization/scripts/preflight_scan.py b/global/tools/code-modernization/scripts/preflight_scan.py new file mode 100755 index 0000000..1499037 --- /dev/null +++ b/global/tools/code-modernization/scripts/preflight_scan.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +"""Pre-flight scanner for code-modernizer skill. + +Stdlib-only — nothing to install. + +Usage: + python3 scripts/preflight_scan.py [--json] + +Reports file count, total lines of code, detected dependency manifests, +and flags high-cost files (> 800 lines). +""" + +import json +import os +import sys +from pathlib import Path + +SKIP_DIRS = { + "node_modules", ".git", "__pycache__", ".venv", "venv", + "dist", "build", ".next", ".nuxt", "coverage", ".tox", + "vendor", "target", "out", ".cache", ".turbo", +} + +CODE_EXTENSIONS = { + ".js", ".jsx", ".ts", ".tsx", ".mjs", ".cjs", + ".py", ".pyx", + ".rs", ".go", ".java", ".kt", ".kts", + ".c", ".cpp", ".cc", ".h", ".hpp", + ".rb", ".php", ".swift", ".scala", + ".vue", ".svelte", ".astro", + ".css", ".scss", ".less", + ".sql", ".sh", ".bash", ".zsh", + ".lua", ".zig", ".nim", ".ex", ".exs", + ".cs", ".fs", +} + +MANIFEST_FILES = { + "package.json", "package-lock.json", "yarn.lock", "pnpm-lock.yaml", + "requirements.txt", "pyproject.toml", "setup.py", "setup.cfg", "Pipfile", + "Cargo.toml", "go.mod", "Gemfile", "composer.json", + "build.gradle", "build.gradle.kts", "pom.xml", + "pubspec.yaml", "mix.exs", +} + +HIGH_COST_THRESHOLD = 800 + + +def count_lines(path: Path) -> int: + try: + with open(path, "r", encoding="utf-8", errors="replace") as f: + return sum(1 for _ in f) + except (OSError, UnicodeDecodeError): + return 0 + + +def parse_dependencies(root: Path) -> list[str]: + deps = [] + pkg = root / "package.json" + if pkg.is_file(): + try: + data = json.loads(pkg.read_text(encoding="utf-8")) + for key in ("dependencies", "devDependencies"): + if key in data: + deps.extend(data[key].keys()) + except (json.JSONDecodeError, OSError): + pass + + req = root / "requirements.txt" + if req.is_file(): + try: + for line in req.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#") and not line.startswith("-"): + name = line.split("==")[0].split(">=")[0].split("<=")[0].split("[")[0].strip() + if name: + deps.append(name) + except OSError: + pass + + pyproject = root / "pyproject.toml" + if pyproject.is_file(): + try: + content = pyproject.read_text(encoding="utf-8") + in_deps = False + for line in content.splitlines(): + if line.strip().startswith("dependencies"): + in_deps = True + continue + if in_deps: + if line.strip() == "]": + in_deps = False + continue + dep = line.strip().strip('",').split(">=")[0].split("==")[0].split("<")[0].strip() + if dep: + deps.append(dep) + except OSError: + pass + + return sorted(set(deps)) + + +def scan(root: Path) -> dict: + file_count = 0 + total_lines = 0 + high_cost_files = [] + manifests_found = [] + + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS] + + for fname in filenames: + fpath = Path(dirpath) / fname + + if fname in MANIFEST_FILES: + manifests_found.append(str(fpath.relative_to(root))) + + if fpath.suffix.lower() in CODE_EXTENSIONS: + file_count += 1 + lines = count_lines(fpath) + total_lines += lines + if lines > HIGH_COST_THRESHOLD: + high_cost_files.append({ + "file": str(fpath.relative_to(root)), + "lines": lines, + }) + + deps = parse_dependencies(root) + + return { + "root": str(root), + "file_count": file_count, + "total_lines": total_lines, + "manifests": manifests_found, + "dependencies": deps, + "high_cost_files": sorted(high_cost_files, key=lambda x: -x["lines"]), + } + + +def format_report(result: dict) -> str: + lines = [ + f"Scope: scan on {result['root']}", + f"~{result['file_count']} files, ~{result['total_lines']} lines of code", + ] + + if result["dependencies"]: + dep_preview = result["dependencies"][:20] + dep_str = ", ".join(dep_preview) + if len(result["dependencies"]) > 20: + dep_str += f" (+{len(result['dependencies']) - 20} more)" + lines.append(f"Dependencies already available: {dep_str}") + else: + lines.append("Dependencies already available: (none detected)") + + if result["high_cost_files"]: + hc = "; ".join(f"{f['file']} ({f['lines']} lines)" for f in result["high_cost_files"][:10]) + lines.append(f"Flagged high-cost (>{HIGH_COST_THRESHOLD} lines): {hc}") + else: + lines.append(f"Flagged high-cost: none (all files <{HIGH_COST_THRESHOLD} lines)") + + risk = "low" + if result["file_count"] > 20: + risk = "medium" + if result["file_count"] > 50 or result["total_lines"] > 20000: + risk = "high" + + lines.append(f"Estimated breaking-change risk: {risk}") + + return "\n".join(lines) + + +def main(): + if len(sys.argv) < 2: + print(f"Usage: {sys.argv[0]} [--json]", file=sys.stderr) + sys.exit(1) + + target = Path(sys.argv[1]).resolve() + if not target.is_dir(): + print(f"Error: {target} is not a directory", file=sys.stderr) + sys.exit(1) + + result = scan(target) + + if "--json" in sys.argv: + print(json.dumps(result, indent=2)) + else: + print(format_report(result)) + + +if __name__ == "__main__": + main()