diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8e9af58..04c0148 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -435,6 +435,23 @@ "session" ] }, + { + "name": "lesson-learned", + "description": "Analyze recent code changes via git history and extract software engineering lessons. Use when reflecting on work or asking 'what is the lesson here?'", + "source": "./dist/plugins/lesson-learned", + "strict": false, + "skills": [ + "./skills/lesson-learned" + ], + "category": "development", + "keywords": [ + "development", + "reflection", + "git", + "learning", + "principles" + ] + }, { "name": "game-changing-features", "description": "Find 10x product opportunities and high-leverage improvements. Use when user wants strategic product thinking or wants to find high-impact features.", diff --git a/README.md b/README.md index 5b0b610..dd457d8 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,7 @@ Add skills to project knowledge or paste SKILL.md contents into the conversation | 🛠️ Development | [database-schema-designer](skills/database-schema-designer/README.md) | Design robust database schemas | | 🛠️ Development | [dependency-updater](skills/dependency-updater/README.md) | Smart dependency management | | 🛠️ Development | [naming-analyzer](skills/naming-analyzer/README.md) | Suggest better variable/function names | +| 🛠️ Development | [lesson-learned](skills/lesson-learned/README.md) | Extract SE lessons from recent code changes | | 🛠️ Development | [reducing-entropy](skills/reducing-entropy/README.md) | Minimize codebase size | | 🛠️ Development | [session-handoff](skills/session-handoff/README.md) | Seamless AI session transfers | | 🎯 Planning | [game-changing-features](skills/game-changing-features/README.md) | Find 10x product opportunities | diff --git a/skills/lesson-learned/README.md b/skills/lesson-learned/README.md new file mode 100644 index 0000000..984b2c4 --- /dev/null +++ b/skills/lesson-learned/README.md @@ -0,0 +1,65 @@ +# Lesson Learned + +Extract software engineering takeaways from your recent code changes. + +## Purpose + +Most developers ship code and move on. But the fastest way to grow is to pause and reflect: what principle did I just apply? What trade-off did I make? What would I do differently? + +This skill analyzes recent git history and surfaces the engineering lesson embedded in the work you just did. + +## When to Use + +- After finishing a feature or PR +- After a debugging session +- After a refactor +- When you want to reflect on recent work +- Trigger phrases: "what is the lesson here?", "what can I learn from this?", "engineering takeaway" + +## How It Works + +1. **Determine scope** -- Detects whether you're on a feature branch (analyzes branch vs main) or main (analyzes last 5 commits). You can also specify a commit range or specific SHA. +2. **Gather changes** -- Reads git log, diffs, and commit messages. For large diffs, selectively reads the most-changed files. +3. **Analyze patterns** -- Identifies structural decisions, trade-offs, and problems solved. Maps observations to named SE principles. +4. **Present the lesson** -- Delivers 1-3 specific, code-grounded takeaways with the principle name, how it manifested, why it matters, and an actionable next step. + +## Key Features + +- **Git-history-driven** -- Works from actual code changes, not hypotheticals +- **Principle-grounded** -- Maps observations to named SE principles (SOLID, DRY, KISS, YAGNI, etc.) +- **Specific, not generic** -- Every insight references actual files and lines from your diff +- **Balanced** -- Recognizes good patterns, not just areas for improvement +- **Configurable scope** -- Feature branch, last N commits, specific SHA, or working changes + +## Output Format + +```markdown +## Lesson: Separation of Concerns + +**What happened in the code:** +The `AuthController` was split into `AuthController` and `UserService` across +commits abc123 and def456. Authentication logic stayed in the controller while +user CRUD operations moved to a dedicated service. + +**The principle at work:** +Separation of Concerns -- different responsibilities should live in different +modules so each can change independently. + +**Why it matters:** +Before the split, any change to user management risked breaking authentication. +Now each module can evolve independently with a clear interface between them. + +**Takeaway for next time:** +When a file handles two distinct responsibilities, split early -- it's cheaper +than untangling them later. +``` + +## Prerequisites + +- Must be in a git repository with commit history +- Works best with descriptive commit messages + +## Related Files + +- `references/se-principles.md` -- Curated SE principles catalog +- `references/anti-patterns.md` -- Common anti-patterns to detect in diffs diff --git a/skills/lesson-learned/SKILL.md b/skills/lesson-learned/SKILL.md new file mode 100644 index 0000000..e358582 --- /dev/null +++ b/skills/lesson-learned/SKILL.md @@ -0,0 +1,105 @@ +--- +name: lesson-learned +description: "Analyze recent code changes via git history and extract software engineering lessons. Use when the user asks 'what is the lesson here?', 'what can I learn from this?', 'engineering takeaway', 'what did I just learn?', 'reflect on this code', or wants to extract principles from recent work." +--- + +# Lesson Learned + +Extract specific, grounded software engineering lessons from actual code changes. Not a lecture -- a mirror. Show the user what their code already demonstrates. + +## Before You Begin + +**Load the principles reference first.** + +1. Read `references/se-principles.md` to have the principle catalog available +2. Optionally read `references/anti-patterns.md` if you suspect the changes include areas for improvement +3. Determine the scope of analysis (see Phase 1) + +**Do not proceed until you've loaded at least `se-principles.md`.** + +## Phase 1: Determine Scope + +Ask the user or infer from context what to analyze. + +| Scope | Git Commands | When to Use | +|-------|-------------|-------------| +| Feature branch | `git log main..HEAD --oneline` + `git diff main...HEAD` | User is on a non-main branch (default) | +| Last N commits | `git log --oneline -N` + `git diff HEAD~N..HEAD` | User specifies a range, or on main (default N=5) | +| Specific commit | `git show ` | User references a specific commit | +| Working changes | `git diff` + `git diff --cached` | User says "what about these changes?" before committing | + +**Default behavior:** +- If on a feature branch: analyze branch commits vs main +- If on main: analyze the last 5 commits +- If the user provides a different scope, use that + +## Phase 2: Gather Changes + +1. Run `git log` with the determined scope to get the commit list and messages +2. Run `git diff` for the full diff of the scope +3. If the diff is large (>500 lines), use `git diff --stat` first, then selectively read the top 3-5 most-changed files +4. **Read commit messages carefully** -- they contain intent that raw diffs miss +5. Only read changed files. Do not read the entire repo. + +## Phase 3: Analyze + +Identify the **dominant pattern** -- the single most instructive thing about these changes. + +Look for: +- **Structural decisions** -- How was the code organized? Why those boundaries? +- **Trade-offs made** -- What was gained vs. sacrificed? (readability vs. performance, DRY vs. clarity, speed vs. correctness) +- **Problems solved** -- What was the before/after? What made the "after" better? +- **Missed opportunities** -- Where could the code improve? (present gently as "next time, consider...") + +Map findings to specific principles from `references/se-principles.md`. Be specific -- quote actual code, reference actual file names and line changes. + +## Phase 4: Present the Lesson + +Use this template: + +```markdown +## Lesson: [Principle Name] + +**What happened in the code:** +[2-3 sentences describing the specific change, referencing files and commits] + +**The principle at work:** +[1-2 sentences explaining the SE principle] + +**Why it matters:** +[1-2 sentences on the practical consequence -- what would go wrong without this, or what goes right because of it] + +**Takeaway for next time:** +[One concrete, actionable sentence the user can apply to future work] +``` + +If there is a second lesson worth noting (maximum 2 additional): + +```markdown +--- + +### Also worth noting: [Principle Name] + +**In the code:** [1 sentence] +**The principle:** [1 sentence] +**Takeaway:** [1 sentence] +``` + +## What NOT to Do + +| Avoid | Why | Instead | +|-------|-----|---------| +| Listing every principle that vaguely applies | Overwhelming and generic | Pick the 1-2 most relevant | +| Analyzing files that were not changed | Scope creep | Stick to the diff | +| Ignoring commit messages | They contain intent that diffs miss | Read them as primary context | +| Abstract advice disconnected from the code | Not actionable | Always reference specific files/lines | +| Negative-only feedback | Demoralizing | Lead with what works, then suggest improvements | +| More than 3 lessons | Dilutes the insight | One well-grounded lesson beats seven vague ones | + +## Conversation Style + +- **Reflective, not prescriptive.** Use the user's own code as primary evidence. +- **Never say "you should have..."** -- instead use "the approach here shows..." or "next time you face this, consider..." +- **If the code is good, say so.** Not every lesson is about what went wrong. Recognizing good patterns reinforces them. +- **If the changes are trivial** (a single config tweak, a typo fix), say so honestly rather than forcing a lesson. "These changes are straightforward -- no deep lesson here, just good housekeeping." +- **Be specific.** Generic advice is worthless. Every claim must point to a concrete code change. diff --git a/skills/lesson-learned/references/anti-patterns.md b/skills/lesson-learned/references/anti-patterns.md new file mode 100644 index 0000000..f552bb1 --- /dev/null +++ b/skills/lesson-learned/references/anti-patterns.md @@ -0,0 +1,55 @@ +--- +description: Common anti-patterns to detect in code diffs. Use alongside se-principles.md for balanced analysis -- principles show what's good, anti-patterns show what to watch for. +--- + +# Anti-Patterns + +When analyzing a diff, check for these signals. Present findings gently -- as opportunities, not failures. + +## God Object / God Class + +One module doing too much. +**Diff signals:** A single file with many unrelated changes. One class/module imported everywhere. A file over 500 lines that keeps growing. +**Suggest:** Extract responsibilities into focused modules (SRP). + +## Shotgun Surgery + +One logical change scattered across many files. +**Diff signals:** 10+ files changed for a single feature or fix. The same type of edit repeated in many places. A rename or config change touching dozens of files. +**Suggest:** Consolidate the scattered logic. If one change requires editing many files, the abstraction boundaries may be wrong. + +## Feature Envy + +A function that uses another module's data more than its own. +**Diff signals:** Heavy cross-module imports. A function reaching deep into another object's properties. Utility functions that only serve one caller in a different module. +**Suggest:** Move the function closer to the data it uses. + +## Premature Abstraction + +Abstracting before there are multiple concrete cases. +**Diff signals:** An interface with exactly one implementation. A factory that creates only one type. A generic solution for a problem that exists only once. +**Suggest:** Wait for the second or third use case before abstracting (Rule of Three). + +## Copy-Paste Programming + +Duplicated code blocks with minor variations. +**Diff signals:** Similar code appearing in multiple places in the diff. Functions that differ by only a parameter or two. Repeated patterns that could be parameterized. +**Suggest:** Extract shared logic, parameterize the differences. + +## Magic Numbers / Strings + +Literal values without explanation. +**Diff signals:** Hardcoded numbers in conditions (`if (retries > 3)`). String literals used as keys or identifiers. Timeouts, limits, or thresholds without named constants. +**Suggest:** Extract to named constants that explain the "why." + +## Long Method + +Functions that do too much. +**Diff signals:** New functions over 40-50 lines. Functions with multiple levels of nesting. Functions that require scrolling to read. +**Suggest:** Extract sub-steps into named functions. Each function should do one thing. + +## Excessive Comments + +Comments explaining "what" instead of "why." +**Diff signals:** Comments restating the code (`// increment counter`). Large comment blocks before straightforward code. Commented-out code left in place. +**Suggest:** Make the code self-documenting through better naming. Use comments only for "why" -- intent, trade-offs, non-obvious constraints. diff --git a/skills/lesson-learned/references/se-principles.md b/skills/lesson-learned/references/se-principles.md new file mode 100644 index 0000000..6d6cee9 --- /dev/null +++ b/skills/lesson-learned/references/se-principles.md @@ -0,0 +1,109 @@ +--- +description: Curated catalog of software engineering principles. Load this before analyzing code changes so you can map observations to named principles. +--- + +# Software Engineering Principles + +Use this as a lookup table. When you spot a pattern in a diff, find the matching principle here. Each entry includes **code signals** -- what to look for in actual changes. + +## Design Principles (SOLID) + +**Single Responsibility Principle (SRP)** +A module should have one reason to change. +Code signals: A class/file was split into two. A function was extracted. A component stopped handling both UI and data fetching. + +**Open/Closed Principle (OCP)** +Open for extension, closed for modification. +Code signals: New behavior added without changing existing code. A plugin/hook/callback system introduced. Strategy pattern or configuration used instead of conditionals. + +**Liskov Substitution Principle (LSP)** +Subtypes must be substitutable for their base types. +Code signals: An interface was introduced to unify implementations. A subclass override changed behavior in a way that broke callers (violation). Type narrowing or guards added. + +**Interface Segregation Principle (ISP)** +No client should depend on methods it doesn't use. +Code signals: A large interface was split into smaller ones. Optional methods removed from an interface. A "fat" props object was broken into focused ones. + +**Dependency Inversion Principle (DIP)** +Depend on abstractions, not concretions. +Code signals: A concrete dependency replaced with an interface/injection. A factory or provider pattern introduced. Import paths changed from specific implementations to abstract layers. + +**Composition over Inheritance** +Favor object composition over class inheritance. +Code signals: Inheritance hierarchy replaced with delegation. Mixins or HOCs replaced with hooks or composition. A "base class" was removed. + +## Simplicity Principles + +**DRY (Don't Repeat Yourself)** +Every piece of knowledge should have a single representation. +Code signals: Duplicate code extracted into a shared function. A constant replaced repeated literals. A template/generator replaced copy-pasted boilerplate. + +**KISS (Keep It Simple, Stupid)** +The simplest solution that works is the best. +Code signals: A complex abstraction replaced with a straightforward implementation. Unnecessary indirection removed. A clever one-liner replaced with readable code. + +**YAGNI (You Aren't Gonna Need It)** +Don't build it until you actually need it. +Code signals: Speculative features removed. Unused configuration options deleted. An over-engineered solution simplified to match actual requirements. + +**Rule of Three** +Wait until the third duplication before abstracting. +Code signals: Similar code exists in 2 places and was left alone (good). A premature abstraction was introduced after only one use (violation). Third occurrence triggered extraction (textbook application). + +**Principle of Least Surprise** +Code should behave the way most users would expect. +Code signals: A function renamed to better describe what it does. Return types made consistent. Side effects made explicit or removed. + +## Structural Principles + +**Separation of Concerns** +Different responsibilities should live in different modules. +Code signals: Business logic extracted from UI components. Data access separated from domain logic. Configuration separated from behavior. A "god file" split into focused modules. + +**High Cohesion** +Related functionality should live together. +Code signals: Scattered related functions gathered into one module. A utility file broken up so each piece lives near its consumers. A feature folder created. + +**Loose Coupling** +Modules should depend on each other as little as possible. +Code signals: Direct imports replaced with events/callbacks. A shared dependency removed. Modules communicate through well-defined interfaces instead of reaching into internals. + +**Encapsulation** +Hide internal details, expose only what's necessary. +Code signals: Public methods reduced. Internal helpers made private. A module's API surface shrunk. Implementation details hidden behind a facade. + +**Information Hiding** +Modules should not expose their internal data structures. +Code signals: Raw data structures wrapped in accessor methods. Internal state made private. A data transformation moved inside the module that owns the data. + +## Pragmatic Principles + +**Boy Scout Rule** +Leave the code better than you found it. +Code signals: Small cleanups alongside a feature change. A renamed variable for clarity. A dead code path removed. An outdated comment updated. + +**Fail Fast** +Detect and report errors as early as possible. +Code signals: Input validation added at entry points. Assertions added for invariants. Early returns replacing deep nesting. Error handling moved closer to the source. + +**Defensive Programming** +Anticipate and handle unexpected inputs gracefully. +Code signals: Null checks added. Default values provided. Edge cases handled. Error boundaries introduced. + +**Premature Optimization (avoiding it)** +Don't optimize until you've measured. +Code signals: A simple implementation chosen over a "faster" complex one. Readability prioritized over micro-performance. A profiling step added before optimization work. + +## Refactoring Patterns + +**Extract Method/Function** +Code signals: A long function split into named sub-functions. Inline logic replaced with a well-named call. + +**Extract Class/Module** +Code signals: A file split into multiple files. A class split into two with distinct responsibilities. + +**Replace Conditional with Polymorphism** +Code signals: A switch/if-else chain replaced with a strategy pattern or subclass dispatch. A type map introduced. + +**Introduce Parameter Object** +Code signals: Multiple related parameters grouped into a single options/config object. Function signatures simplified.