From d0f7e4bdb37cfb0f8f0e8b854ed06b49fc3fa841 Mon Sep 17 00:00:00 2001 From: Ether Date: Mon, 4 May 2026 18:14:56 +0900 Subject: [PATCH 1/3] feat: add 4 mattpocock-inspired skills for plan grilling and architecture Adds four new skills inspired by mattpocock/skills (MIT) and boosts developing-test-first with a Horizontal Slicing anti-pattern. New skills: - grilling-plans: adversarial pre-implementation interrogation, decision-tree walk with recommended answers, codebase-exploration-first - building-shared-vocabulary: maintains domain glossary (CONTEXT.md) and ADRs as living artifacts with lazy creation and a 3-criterion ADR gate - zooming-out-on-code: system-level map of unfamiliar code in the project's domain vocabulary (responsibility / callers / dependencies / siblings) - improving-architecture: surfaces deep-module refactor candidates using Ousterhout's depth/seam framing and the deletion test New slash commands: - /skill-set:plan:grill - /skill-set:code:zoom-out Improves developing-test-first with Horizontal Slicing anti-pattern section forbidding bulk-write tests-first then implementations-after. Bumps version 1.11.1 to 1.12.0. Attribution in README and CHANGELOG. --- CHANGELOG.md | 15 ++ README.md | 12 ++ plugins/skill-set/.claude-plugin/plugin.json | 2 +- plugins/skill-set/commands/code/zoom-out.md | 12 ++ plugins/skill-set/commands/plan/grill.md | 15 ++ .../building-shared-vocabulary/SKILL.md | 118 +++++++++++++ .../reference/adr-format.md | 96 ++++++++++ .../reference/context-format.md | 111 ++++++++++++ .../skills/developing-test-first/SKILL.md | 27 +++ .../skill-set/skills/grilling-plans/SKILL.md | 163 +++++++++++++++++ .../reference/codebase-cross-reference.md | 115 ++++++++++++ .../reference/decision-tree-walk.md | 73 ++++++++ .../skills/improving-architecture/SKILL.md | 165 ++++++++++++++++++ .../reference/deep-modules.md | 129 ++++++++++++++ .../reference/deepening.md | 52 ++++++ .../reference/deletion-test.md | 71 ++++++++ .../reference/interface-design.md | 56 ++++++ .../skills/zooming-out-on-code/SKILL.md | 109 ++++++++++++ 18 files changed, 1340 insertions(+), 1 deletion(-) create mode 100644 plugins/skill-set/commands/code/zoom-out.md create mode 100644 plugins/skill-set/commands/plan/grill.md create mode 100644 plugins/skill-set/skills/building-shared-vocabulary/SKILL.md create mode 100644 plugins/skill-set/skills/building-shared-vocabulary/reference/adr-format.md create mode 100644 plugins/skill-set/skills/building-shared-vocabulary/reference/context-format.md create mode 100644 plugins/skill-set/skills/grilling-plans/SKILL.md create mode 100644 plugins/skill-set/skills/grilling-plans/reference/codebase-cross-reference.md create mode 100644 plugins/skill-set/skills/grilling-plans/reference/decision-tree-walk.md create mode 100644 plugins/skill-set/skills/improving-architecture/SKILL.md create mode 100644 plugins/skill-set/skills/improving-architecture/reference/deep-modules.md create mode 100644 plugins/skill-set/skills/improving-architecture/reference/deepening.md create mode 100644 plugins/skill-set/skills/improving-architecture/reference/deletion-test.md create mode 100644 plugins/skill-set/skills/improving-architecture/reference/interface-design.md create mode 100644 plugins/skill-set/skills/zooming-out-on-code/SKILL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 014d683..84e7f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ All notable changes to the skill-set plugin will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.12.0] - 2026-05-04 + +### Added + +- **grilling-plans**: New skill for adversarial pre-implementation interrogation of an existing plan or design — walks the decision tree one question at a time with recommended answers, prefers codebase exploration over questions, and surfaces contradictions between stated intent and actual code. Inspired by [mattpocock/skills](https://github.com/mattpocock/skills) `grill-me` / `grill-with-docs`. +- **building-shared-vocabulary**: New skill that maintains a project's domain glossary in `CONTEXT.md` and architecture decisions in `docs/adr/` as living artifacts — files are created lazily, updated inline as terms resolve and decisions crystallize. ADR creation is gated on three criteria (hard-to-reverse, surprising-without-context, real trade-off). Inspired by [mattpocock/skills](https://github.com/mattpocock/skills) `grill-with-docs` (CONTEXT.md / ADR pattern). +- **zooming-out-on-code**: New skill that draws a higher-level system map of unfamiliar code in the project's domain vocabulary — describes responsibility, callers, dependencies, and sibling modules without diving into implementation. Inspired by [mattpocock/skills](https://github.com/mattpocock/skills) `zoom-out`. +- **improving-architecture**: New skill that surfaces deep-module refactor candidates across a codebase using Ousterhout's depth/seam framing — applies the deletion test, presents candidates with locality and leverage justifications, and hands off to `grilling-plans` for the chosen candidate's design. Inspired by [mattpocock/skills](https://github.com/mattpocock/skills) `improve-codebase-architecture`. +- **/skill-set:plan:grill**: Slash command for `grilling-plans`. +- **/skill-set:code:zoom-out**: Slash command for `zooming-out-on-code`. + +### Improved + +- **developing-test-first**: Added "Anti-Pattern: Horizontal Slicing" section after the Iron Law to forbid the bulk RED→RED→RED→...→GREEN→GREEN→GREEN pattern that produces tests of imagined rather than actual behavior. Inspired by [mattpocock/skills](https://github.com/mattpocock/skills) `tdd`. + ## [1.11.1] - 2026-05-04 ### Fixed diff --git a/README.md b/README.md index a722437..3d23c87 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,8 @@ Skills are automatically available after installing the plugin. Claude will use /skill-set:ralph:execute /skill-set:pr-review:fix /skill-set:consulting:review +/skill-set:plan:grill +/skill-set:code:zoom-out ``` ## Project Structure @@ -164,6 +166,16 @@ Contributions welcome! Please see [AGENTS.md](AGENTS.md) for development guideli MIT +## Acknowledgements + +The following skills were inspired by [mattpocock/skills](https://github.com/mattpocock/skills) (MIT license): + +- `grilling-plans` ← `grill-me` / `grill-with-docs` +- `building-shared-vocabulary` ← `grill-with-docs` (CONTEXT.md / ADR pattern) +- `zooming-out-on-code` ← `zoom-out` +- `improving-architecture` ← `improve-codebase-architecture` +- `developing-test-first` (Horizontal Slicing section) ← `tdd` + ## Changelog See [CHANGELOG.md](CHANGELOG.md) for version history and migration guides. diff --git a/plugins/skill-set/.claude-plugin/plugin.json b/plugins/skill-set/.claude-plugin/plugin.json index 2a67b45..897f1f4 100644 --- a/plugins/skill-set/.claude-plugin/plugin.json +++ b/plugins/skill-set/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "skill-set", "description": "Comprehensive productivity skills and development tools for Claude Code, including git workflow automation, code context understanding, peer LLM consulting, and PR review feedback processing.", - "version": "1.11.1", + "version": "1.12.0", "author": { "name": "ether-moon" }, diff --git a/plugins/skill-set/commands/code/zoom-out.md b/plugins/skill-set/commands/code/zoom-out.md new file mode 100644 index 0000000..64eb203 --- /dev/null +++ b/plugins/skill-set/commands/code/zoom-out.md @@ -0,0 +1,12 @@ +--- +description: Draw a higher-level system map of unfamiliar code in the project's domain vocabulary. Describes the module's responsibility, callers, dependencies, and sibling modules without diving into implementation details. +--- + +Invoke the `zooming-out-on-code` skill to get oriented in an unfamiliar area of the codebase. The skill goes up one level of abstraction from the file or function in question, reads the project's `CONTEXT.md` and relevant ADRs first, and produces a four-part map: responsibility, callers, dependencies, siblings — all in domain vocabulary, not implementation language. + +**When to use this command:** +- Before changing code in an area you don't know +- When a teammate hands off a system you have no prior context on +- When you need orientation before diving deeper + +**Output is intentionally compact.** Deeper detail is a follow-up question, not part of this command's output. diff --git a/plugins/skill-set/commands/plan/grill.md b/plugins/skill-set/commands/plan/grill.md new file mode 100644 index 0000000..16f1c26 --- /dev/null +++ b/plugins/skill-set/commands/plan/grill.md @@ -0,0 +1,15 @@ +--- +description: Adversarially interrogate an existing plan or design before implementation. Walks the decision tree one question at a time with recommended answers, prefers codebase exploration over questions, and surfaces contradictions between stated intent and actual code. +--- + +Invoke the `grilling-plans` skill to stress-test the current plan, design, or proposal. The skill walks the decision tree one branch at a time, provides a recommended answer with each question, reads the codebase rather than asking when possible, and surfaces contradictions between user statements and actual code. + +**When to use this command:** +- Between `superpowers:brainstorming` (creation) and `superpowers:writing-plans` (lock-down) +- Before invoking `superpowers:executing-plans` +- Right before a PR description is finalized +- After picking a candidate from `improving-architecture` + +**Outputs:** +- A shared understanding sharp enough to feed into implementation planning +- Optionally hands off to `building-shared-vocabulary` if a domain term is sharpened or an ADR-worthy decision crystallizes diff --git a/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md b/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md new file mode 100644 index 0000000..238dc20 --- /dev/null +++ b/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md @@ -0,0 +1,118 @@ +--- +name: building-shared-vocabulary +description: Maintains a project's domain glossary in CONTEXT.md and architecture decisions in docs/adr/ as living artifacts — files are created lazily, updated inline as terms resolve and decisions crystallize, and conflicts with existing entries are surfaced immediately. Use this skill whenever a domain term is being pinned down, a decision sounds ADR-worthy, or vocabulary is about to drift — phrases like "domain glossary", "용어집", "context.md", "ADR 만들자", "도메인 용어 정리", "we should write this down", "let's name this", "what do we call this", "is this worth an ADR", "pin down this term" — and as a side-effect of grilling-plans when terms or decisions surface. +--- + +# Building Shared Vocabulary + +## Overview + +A project's **domain glossary** (`CONTEXT.md`) and **architecture decision records** (`docs/adr/`) are not documentation written after the fact — they are operational artifacts that the agent reads to stay aligned with the project, and updates inline as the conversation produces new shared meaning. + +**Core principle:** Vocabulary is built up one resolved term at a time, in the moment the resolution happens. Batched glossary writes go stale. + +> `CONTEXT.md` is a **domain artifact**, not an agent directive. `guarding-agent-directives` does not apply — that skill protects `CLAUDE.md` / `AGENTS.md` and the documents they reference. This skill owns `CONTEXT.md` and `docs/adr/`. + +## When to Use + +- A domain term is being pinned down during conversation (especially during grilling) +- A term in the user's plan conflicts with how the codebase uses it +- A decision that is **hard-to-reverse**, **surprising-without-context**, AND **the result of a real trade-off** has been made — record an ADR +- User explicitly asks to "build a glossary," "set up CONTEXT.md," "add an ADR," "용어집 만들자," "도메인 용어 정리" + +## Do NOT use for + +- Implementation-level documentation → README, code comments +- Process or workflow documentation → AGENTS.md / CLAUDE.md (and check `guarding-agent-directives` first) +- Library / framework reference → `understanding-code-context` +- Generating PRDs or issues from conversation → out of scope here +- Decisions that are easily reversed, obvious in hindsight, or had no real alternative → no ADR + +## Scope + +This skill supports a **single root `CONTEXT.md`** and a **single `docs/adr/` directory**. Multi-context monorepos (per-bounded-context glossaries, `CONTEXT-MAP.md` indexes) are out of scope; if a project genuinely needs them, that's a future enhancement. + +## Process + +### Lazy file creation + +Create files only on first real demand. An empty `CONTEXT.md` signals "this project has no shared vocabulary," not "no vocabulary surfaced yet" — speculative skeletons mislead future readers and the agent itself. + +| Trigger | Create | +|---|---| +| First domain term gets resolved | `CONTEXT.md` at repo root | +| First decision meets the ADR bar | `docs/adr/0001-.md` | + +If the file already exists, append to it; never overwrite or reorder existing entries silently. + +### Updating `CONTEXT.md` inline + +When a term is resolved during conversation, update `CONTEXT.md` **right away** — not at the end of the session. Use the format in `reference/context-format.md`. + +**Rules:** +- Only domain-meaningful terms. No implementation jargon, no tool names, no helper-function names. +- One canonical name per concept. If the project has been calling the same concept by two names, pick one and note the deprecated alternative under "Avoid:". +- Capture relationships between terms (e.g., "An Order has many Line Items"). +- Flag genuine ambiguity explicitly under "Flagged ambiguities". + +### Surface conflicts immediately + +When the user uses a term that conflicts with an existing `CONTEXT.md` entry, stop and surface it before continuing: + +> "`CONTEXT.md` defines **Cancellation** as the user-initiated revocation. You're using it for the system-initiated timeout. Do we update the definition, or do we need a new term?" + +Do not silently let the conflict pass. The conflict is the reason vocabulary exists. + +### Recording ADRs + +Offer an ADR **only when all three are true**: + +1. **Hard-to-reverse** — undoing the decision later costs real engineering effort +2. **Surprising-without-context** — a future reader will wonder why this choice was made +3. **The result of a real trade-off** — there were genuine alternatives, and one was picked for specific reasons + +If even one is missing, do not offer an ADR. "We picked Postgres because we know Postgres" is not an ADR. + +Use the format in `reference/adr-format.md`. Number sequentially (`0001-`, `0002-`, …). Filename slug should be a noun phrase (`0007-event-sourced-orders.md`). + +## Process Flow + +``` +term resolved during conversation + → conflict with existing CONTEXT.md entry? yes → surface, resolve + → update CONTEXT.md inline + +decision made during conversation + → hard-to-reverse? no → skip + → surprising-no-context? no → skip + → real trade-off? no → skip + → all yes → offer ADR; on accept, write docs/adr/NNNN-.md +``` + +## Reading `CONTEXT.md` and ADRs + +When this skill is invoked or when the agent enters a project for code work, read these files first if present: + +1. `CONTEXT.md` at repo root (if exists) +2. `docs/adr/*.md` in the area being touched (don't over-read; pull only ADRs whose titles match the area) + +Use the vocabulary from `CONTEXT.md` in: +- File and module names suggested in plans +- Variable and function names in implementations +- PR titles, commit messages, issue descriptions +- Conversation with the user + +## Reference + +- `reference/context-format.md` — exact format for a `CONTEXT.md` entry, with examples +- `reference/adr-format.md` — exact ADR template and the three-criterion check + +## Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| `CONTEXT.md` is bloated with implementation terms | Adding non-domain terms | Remove anything that wouldn't appear in a conversation with a non-engineer domain expert | +| Many ADRs over a short period | Bar set too low | Re-check each against the three criteria; demote ADRs that fail any | +| Same concept has multiple ADRs | No conflict surfacing | When updating, search ADR titles first; if related, supersede the older one explicitly | +| `CONTEXT.md` and code disagree | Code drifted, glossary didn't | Surface as a contradiction (this is exactly the signal `grilling-plans` Rule 4 catches) | +| Multi-context monorepo wants per-domain glossaries | Out of scope for this skill | Use a single root `CONTEXT.md` with section headers per bounded context as a stopgap; raise as an enhancement | diff --git a/plugins/skill-set/skills/building-shared-vocabulary/reference/adr-format.md b/plugins/skill-set/skills/building-shared-vocabulary/reference/adr-format.md new file mode 100644 index 0000000..0a69290 --- /dev/null +++ b/plugins/skill-set/skills/building-shared-vocabulary/reference/adr-format.md @@ -0,0 +1,96 @@ +# ADR Format + +Exact template for an Architecture Decision Record, plus the three-criterion check that gates whether to write one at all. + +## File location and naming + +Single directory at `/docs/adr/`. Filenames are sequential and slug-suffixed: + +``` +docs/adr/0001-event-sourced-orders.md +docs/adr/0002-postgres-for-write-model.md +docs/adr/0003-cancellation-as-line-item-operation.md +``` + +The slug is a noun phrase describing what the decision is about. Avoid verbs ("use-postgres") in favor of subjects ("postgres-for-write-model"). + +## The three-criterion gate + +**Write an ADR only if all three are true.** + +### 1. Hard-to-reverse + +Undoing this decision later costs real engineering effort. + +| Hard-to-reverse | Easy-to-reverse | +|---|---| +| Database choice | Logger choice | +| Public API shape | Internal helper signature | +| Domain model decomposition | File organization | +| Wire protocol | Local variable naming | +| Library that touches every module | Library used in one place | + +### 2. Surprising-without-context + +A future reader, looking only at the code, would wonder *why* this choice was made. + +If the code makes the reasoning obvious, no ADR is needed. ADRs exist for choices whose rationale is not visible at the call site. + +### 3. The result of a real trade-off + +There were genuine alternatives. They were considered. One was picked for specific reasons. + +"We picked X because it's the standard" is not a real trade-off. "We picked X over Y because Y's eventual consistency would break our cancellation invariant" is. + +## Template + +The minimal template: + +```markdown +# + +<1-3 sentences: what's the context, what we decided, and why.> +``` + +That's it. **An ADR can be a single paragraph.** The value is in recording *that* a decision was made and *why* — not in filling out sections. Resist the urge to bulk it up. + +## Optional sections + +Only include these when they add genuine value. Most ADRs do not need them. + +- **Status** frontmatter (`proposed | accepted | deprecated | superseded by ADR-NNNN`) — useful when decisions are revisited +- **Considered Options** — only when the rejected alternatives are worth remembering +- **Consequences** — only when non-obvious downstream effects need to be called out + +If you reach for a section, ask: would removing this section lose information a future reader would need? If not, drop it. + +## Updates + +ADRs are immutable in principle. To change a decision: + +1. Write a new ADR explaining the new decision and what changed +2. Add Status to the old ADR: `Superseded by ADR-NNNN` +3. Cross-link both directions + +Do not delete superseded ADRs. The history is the value. + +## What qualifies for an ADR + +- **Architectural shape.** "We're using a monorepo." "The write model is event-sourced; the read model is projected into Postgres." +- **Integration patterns between contexts / services.** "Ordering and Billing communicate via domain events, not synchronous HTTP." +- **Technology choices that carry lock-in.** Database, message bus, auth provider, deployment target. Not every library — just the ones that would take a quarter to swap out. +- **Boundary and scope decisions.** "Customer data is owned by the Customer context; other contexts reference it by ID only." The explicit no-s are as valuable as the yes-s. +- **Deliberate deviations from the obvious path.** "We're using manual SQL instead of an ORM because X." Anything where a reasonable reader would assume the opposite — these stop the next engineer from "fixing" something deliberate. +- **Constraints not visible in the code.** "We can't use AWS because of compliance requirements." "Response times must be under 200ms because of the partner API contract." +- **Rejected alternatives when the rejection is non-obvious.** If you considered GraphQL and picked REST for subtle reasons, record it — otherwise someone will suggest GraphQL again in six months. + +## What does NOT qualify + +- Renaming a function or file +- Picking lint / formatter rules +- Choosing a logger library (unless logging is a domain concern) +- Migrating from yarn to pnpm, or similar tool swaps +- Adding a new test framework +- Picking commit message style + +If the decision is reversible by a single PR with no migration cost, it is not ADR-worthy. diff --git a/plugins/skill-set/skills/building-shared-vocabulary/reference/context-format.md b/plugins/skill-set/skills/building-shared-vocabulary/reference/context-format.md new file mode 100644 index 0000000..11f10cc --- /dev/null +++ b/plugins/skill-set/skills/building-shared-vocabulary/reference/context-format.md @@ -0,0 +1,111 @@ +# CONTEXT.md Format + +Exact format for a project's domain glossary, with examples. + +## File location + +A single file at the repository root: `/CONTEXT.md`. + +## Top-level structure + +```markdown +# — Domain Context + +Brief one-paragraph orientation. What domain does this project serve? +What is the primary user / actor? What is the value proposition? + +## Language + +[Term entries — see "Term entry format" below] + +## Relationships + +- An **** has many **** +- A **** belongs to one **** +- [...] + +## Example dialogue + +> **Dev:** "When a **Customer** places an **Order**, do we create the **Invoice** immediately?" +> **Domain expert:** "No — an **Invoice** is only generated once a **Fulfillment** is confirmed." + +## Flagged ambiguities + +- "" — what's still unclear, what we ruled out, what's outstanding +``` + +Keep "Language" flat by default — every term sits at the same level. **Group terms under sub-headings only when natural clusters emerge** (e.g., a project with distinct sub-domains may benefit from `### Ordering` / `### Billing` groupings). If all terms belong to a single cohesive area, leave it flat. + +The **Example dialogue** is a short conversation between a developer and a domain expert that demonstrates how the canonical terms interact in practice. It clarifies boundaries between related concepts (e.g., `Order` vs. `Cart`, `Invoice` vs. `Payment`) better than definitions alone. Write it the way the conversation would actually happen — bold the canonical terms, keep it under a half-dozen exchanges. + +## Term entry format + +Each entry is a level-3 heading with the canonical name (bold), an italicized one-line definition, and an optional "_Avoid:_" line listing deprecated alternatives. + +```markdown +****: +The single-sentence definition that a domain expert would recognize. +_Avoid_: , +``` + +Example: + +```markdown +**Order**: +A confirmed customer purchase intent. Carries one or more **Line Items**, has a single **Customer**, and progresses through a fixed lifecycle. +_Avoid_: cart, basket, transaction (those mean different things — see `Cart` and `Payment Transaction`) + +**Line Item**: +A single product line within an **Order** — quantity, unit price at time of order, product reference. Cancellation operates at this level. +_Avoid_: order line, item + +**Cancellation**: +User-initiated revocation of one or more **Line Items**, possibly the whole **Order**. Distinct from system-initiated **Timeout**. +_Avoid_: refund (refund is a money-flow concept, see `Payment Reversal`) +``` + +`_Avoid_` is **optional**. Include it only when there is a real deprecated alternative the team has used or might mistakenly use. A clean, unambiguous term needs no Avoid line: + +```markdown +**Fulfillment**: +The act of preparing and shipping the goods on a confirmed **Order**. A single **Order** maps to one or more **Fulfillments** (split shipments). +``` + +If you find yourself inventing deprecated alternatives just to fill the line, drop it. + +## Rules + +### What belongs + +- Concepts a domain expert would name in conversation +- Terms that appear in user-facing copy, marketing, support +- Concepts that have a lifecycle, identity, or invariants in the system +- Roles, actors, capabilities + +### What does NOT belong + +- Implementation classes (`OrderRepository`, `OrderDTO`) +- Framework concepts (`Component`, `Reducer`, `Middleware`) +- Helper utilities, parsers, validators +- File paths, table names, environment variables +- Anything that would change if the implementation language changed + +### Naming + +- Canonical term in **bold** at first mention in any entry +- PascalCase for concepts that map to entity-like things ("Order", "Line Item") +- Plain lowercase for verbs and lifecycle states ("cancellation", "fulfilled") unless the domain itself capitalizes them + +### Conflicts + +When a new term clashes with an existing one, update the entry. Mark the resolution under "Flagged ambiguities" with a brief history line: + +```markdown +- "backlog" was previously used to mean both the *tool* hosting issues and the *body of work* inside it — resolved 2026-04: the tool is the **Issue Tracker**; "backlog" is no longer used as a domain term. +``` + +### Updates + +Each update is one entry at a time, in the moment of resolution. Do not batch. + +If you find yourself wanting to "do a vocabulary cleanup pass," that's a sign the conversation isn't surfacing terms naturally — return to grilling instead. diff --git a/plugins/skill-set/skills/developing-test-first/SKILL.md b/plugins/skill-set/skills/developing-test-first/SKILL.md index 59de7f0..8cad331 100644 --- a/plugins/skill-set/skills/developing-test-first/SKILL.md +++ b/plugins/skill-set/skills/developing-test-first/SKILL.md @@ -49,6 +49,33 @@ Write code before the test? Delete it. Start over. Implement fresh from tests. Period. +## Anti-Pattern: Horizontal Slicing + +The Iron Law forbids production code without a failing test, but it does not by itself forbid writing five tests up front and then five implementations. That sequence — **horizontal slicing** — is the most common way TDD silently fails. + +``` +WRONG (horizontal): + RED RED RED RED RED → GREEN GREEN GREEN GREEN GREEN + +RIGHT (vertical / tracer bullet): + RED → GREEN, RED → GREEN, RED → GREEN, ... +``` + +Why horizontal slicing produces bad tests: + +- **Tests written in bulk verify *imagined* behavior, not actual behavior.** The implementation hasn't been built yet, so the tests can only assert against guesses. +- **They test the *shape* of things** — data structures, function signatures, return types — instead of user-facing behavior. +- **They become insensitive to real changes.** They pass when the system breaks in ways the bulk-author didn't anticipate; they fail when behavior is fine but a signature shifted. +- **You out-run your headlights.** Committing to test structure before any implementation feedback locks you into assumptions you haven't validated. + +Why vertical slicing works: + +- Each test responds to what the previous cycle taught you +- You write the test you actually need, because you just felt the implementation +- Tests stay honest because they were written against working code, not imagination + +**Rule:** one test, one implementation, one cycle. Then the next. + ## Red-Green-Refactor ```dot diff --git a/plugins/skill-set/skills/grilling-plans/SKILL.md b/plugins/skill-set/skills/grilling-plans/SKILL.md new file mode 100644 index 0000000..5c19fbe --- /dev/null +++ b/plugins/skill-set/skills/grilling-plans/SKILL.md @@ -0,0 +1,163 @@ +--- +name: grilling-plans +description: Adversarially interrogates an existing plan, design, or proposal before implementation — walks the decision tree one question at a time, provides a recommended answer with each question, prefers codebase exploration over questions, and surfaces contradictions between stated intent and actual code. Use this skill whenever a plan, design doc, RFC, ADR draft, ticket spec, or implementation outline is shared and the user wants review, sanity check, sign-off, or asks "is this ready" — even without the word "grill". Trigger phrases include "grill me", "challenge this plan", "poke holes", "stress test", "내 계획 부숴봐", "구멍 찾아봐". Also use before locking down a spec for implementation. +--- + +# Grilling Plans + +## Overview + +Adversarial validation mode. Take an existing plan, design, or proposal and **walk the decision tree one branch at a time**, surfacing hidden assumptions, fuzzy terminology, and contradictions before any code is written. + +**Core principle:** Misalignment is the #1 failure mode. The cure is not more brainstorming — it is forcing every implicit decision to become explicit. + +This is a discipline skill. Each rule below corresponds to a specific way grilling silently fails when the rule is dropped — keeping all six is what separates grilling from casual conversation. + +## When to Use + +**Use grilling when a plan already exists and is about to be locked:** +- Between `superpowers:brainstorming` (creation) and `superpowers:writing-plans` (lock-down) +- Before invoking `superpowers:executing-plans` +- Right before a PR description is finalized +- User says "challenge this", "poke holes", "stress test", "grill me", "내 계획 부숴봐", "구멍 찾아봐" +- When a candidate is selected from `improving-architecture` + +**Do NOT use for:** +- Blank-slate ideas with no plan yet → `superpowers:brainstorming` +- Locked specs that just need decomposition → `superpowers:writing-plans` +- Bug investigation → `superpowers:systematic-debugging` +- Reviewing already-written code → `simplify` +- External library API questions → `understanding-code-context` + +Thinking "the plan looks good enough, skip grilling"? That is exactly when grilling matters most. + +## Domain Awareness + +Before the first question, absorb whatever domain context already exists: + +- If `CONTEXT.md` and `docs/adr/` exist (per `building-shared-vocabulary`), read them first — `CONTEXT.md` carries canonical vocabulary, ADRs in the area being grilled record decisions you should not re-litigate. If absent, infer domain vocabulary from package/module names, test descriptions, and recent commit messages. +- Walk relevant code with Grep / Glob / Read or an `Explore` subagent — see `superpowers:dispatching-parallel-agents` if multiple independent areas need pre-grilling sweeps. + +This costs a few minutes and saves a session of mis-aimed questions. It also lets Rule 5 (term sharpening) work against an actual baseline rather than guesses. + +## The Grilling Protocol + +Six rules. Each blocks a different failure mode (decision-tree drift, unanchored questions, lazy code-skipping, silent contradictions, vocabulary slop, abstract hand-waving) — weakening any one collapses grilling back into casual conversation. + +### Rule 1 — Walk the Decision Tree, One Question Per Turn + +Plans contain a tree of dependent decisions. Resolve them in dependency order, **one at a time**. Wait for the user's answer before moving to the next branch. + +Why one at a time: batched questions get answered at the same shallow level. The user spreads attention across all of them, the dependency tree collapses into a flat list, and downstream answers stop being informed by upstream ones. Single questions force depth. + +When a question opens up sub-questions, descend into the sub-tree before backing up. + +### Rule 2 — Provide a Recommended Answer With Every Question + +Every question must include the model's own recommendation, with reasoning. This forces commitment instead of polite hedging. + +``` +WRONG: "What should happen when the cache is empty?" +RIGHT: "What should happen when the cache is empty? My recommendation: + fall through to the source of truth and repopulate, because the + alternative (return empty) creates a thundering herd on miss. + Counter-argument: warmup cost. Which way?" +``` + +If you cannot recommend an answer, the question is not yet sharp enough — refine it before asking. + +### Rule 3 — Codebase Exploration Beats Asking + +If a question can be answered by reading code, **read the code instead of asking**. Use Grep, Glob, or Read mid-walk; reserve `superpowers:dispatching-parallel-agents` for the Domain Awareness pre-grilling sweep, not for questions interleaved with the user. + +``` +WRONG: "Does this project already have a retry helper?" +RIGHT: [run Grep for "retry"] → "Found `src/lib/retry.ts` with exponential + backoff. We should reuse it. Confirm?" +``` + +Only ask the user when the answer requires intent, judgment, or knowledge outside the repository. + +### Rule 4 — Surface Contradictions Immediately + +When the user's stated plan conflicts with what the code actually does, surface it the moment you see it. Do not let the contradiction silently become a future bug. + +``` +"You said partial cancellation is supported, but the code in +src/orders/cancel.ts cancels the entire Order in one transaction. +Which is the source of truth right now — the plan or the code?" +``` + +### Rule 5 — Sharpen Fuzzy Terms + +When a term is overloaded, ambiguous, or context-dependent, propose a canonical name and pin down the meaning before continuing. Vague vocabulary now becomes a wrong implementation later. + +``` +"You're saying 'account' — do you mean the Customer record, the +authenticated User, or the Billing entity? Those are three different +things in this codebase. Pick one canonical term and let's use it." +``` + +If the project has a `CONTEXT.md` glossary, check the term against it. Conflicts with the glossary are themselves a signal. + +### Rule 6 — Stress-Test With Concrete Scenarios + +When domain relationships or boundaries are being discussed, invent a specific scenario that probes the edge. Force precision instead of letting abstractions slide by. + +``` +"You said partial cancellation is supported. Concrete scenario: +Order has three Line Items, customer cancels two. The third item +ships, the first two don't. Does the Order itself get a status? +What status? What does the invoice look like?" +``` + +Edge-case scenarios surface invariants the user hadn't thought about. Pick scenarios that lie at the boundaries — empty, full, partial, simultaneous, racing, retried, reordered. The user's answer either commits to a behavior or reveals that the behavior wasn't decided yet. + +## Process Flow + +``` +receive plan + → Domain Awareness (read CONTEXT.md / ADRs / code) + → loop: + pick next undecided branch (dependency order) + → can codebase answer? yes → read, present finding + no → form question + recommendation, ask + → contradiction with code? yes → surface, then continue + → fuzzy term? yes → sharpen (canonical name), then continue + → tree complete? no → next branch + yes → plan validated + (optional: update CONTEXT.md / ADR) +``` + +## Outputs + +**Default output:** a shared understanding between user and agent. The plan is now sharp enough to feed into `superpowers:writing-plans` or `superpowers:executing-plans`. + +**Optional outputs (only if conditions are met):** + +| Trigger | Action | +|---|---| +| A fuzzy term was sharpened during grilling | Hand off to `building-shared-vocabulary` to update `CONTEXT.md` | +| A decision is **hard-to-reverse** AND **surprising-without-context** AND **the result of a real trade-off** | Offer to record an ADR via `building-shared-vocabulary` | +| The plan reveals an architectural friction worth a separate refactor | Hand off to `improving-architecture` | + +Do not produce these artifacts speculatively. Only when the bar is met. + +**ADR yes-example:** "We project the read model into Postgres while the write model is event-sourced." Hard to reverse (touches every read path), surprising (a future reader will ask why the duplication exists), real trade-off (we chose CQRS over a simpler unified model for specific consistency reasons). → ADR. + +**ADR no-example:** "We picked `pino` for logging because the team knows it." Easy to reverse (one PR), not surprising (default-ish choice), no real alternatives weighed. → No ADR, just a code comment if anything. + +## Reference + +- `reference/decision-tree-walk.md` — how to identify dependency order, when to descend vs. back up, what counts as a leaf +- `reference/codebase-cross-reference.md` — which tools to use for which kinds of contradictions, examples of high-value cross-checks + +## Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| User feels grilled past the useful point | Asking questions whose recommendation is obvious | Stop asking and just commit to the recommendation; surface it as a finding instead | +| User keeps saying "I don't know, you decide" | Decisions are too granular for grilling | Back up one level; the user's threshold is the right granularity | +| Questions feel disconnected from each other | Not walking the dependency tree | Re-read the plan, list undecided branches in dependency order, restart from the top | +| Same fuzzy term keeps recurring | Term not pinned down | Stop and sharpen it now; do not continue with the term still ambiguous | +| Codebase contradicts the plan in many places | Plan written without exploring the code first | Pause grilling; have user re-explore code before continuing, or adjust the plan to match code | diff --git a/plugins/skill-set/skills/grilling-plans/reference/codebase-cross-reference.md b/plugins/skill-set/skills/grilling-plans/reference/codebase-cross-reference.md new file mode 100644 index 0000000..74d229c --- /dev/null +++ b/plugins/skill-set/skills/grilling-plans/reference/codebase-cross-reference.md @@ -0,0 +1,115 @@ +# Codebase Cross-Reference + +How to use the codebase to answer questions, surface contradictions, and stress-test claims — instead of pinging the user. + +## Contents + +- [The Core Principle](#the-core-principle) +- [High-Value Cross-Checks](#high-value-cross-checks) +- [Tool Selection](#tool-selection) +- [When to Stop Reading and Start Asking](#when-to-stop-reading-and-start-asking) +- [Reporting What You Read](#reporting-what-you-read) + +## The Core Principle + +Every question to the user has a cost. Most questions about *what currently exists* have zero cost to answer from the code. The asymmetry says: read first, ask second. + +## High-Value Cross-Checks + +These are the cross-checks that catch the most defects. + +### 1. "X already exists" check + +Before proposing to build a helper, retry, parser, validator, or utility, search for it. + +``` +Tools: Grep (for keywords), Glob (for filename patterns) +Example: User plans a new `withRetry` wrapper. + → Grep "retry" in src/ + → Found `src/lib/withRetry.ts`. Reuse, don't rebuild. +``` + +### 2. Stated behavior vs. actual behavior + +When the user says "X currently does Y," check that the code agrees. + +``` +User: "The /orders endpoint returns 404 if the order is cancelled." +Read src/routes/orders.ts → endpoint returns 200 with cancelled=true. +Surface: "Code returns 200 with cancelled=true, not 404. Is the plan +relying on 404, or is the behavior changing as part of this work?" +``` + +### 3. Invariant claims + +When the user states "this never happens" or "this is always true," look for code that assumes the opposite. + +``` +User: "User always has an email at signup." +Grep for "email == null" or "!email" → found 3 sites that handle null email. +Surface: "Three call sites currently handle null email. Either they're dead +code, or your invariant is not enforced. Which?" +``` + +### 4. Dependency direction claims + +When the user describes a dependency direction ("A calls B, never the reverse"), verify with an import scan. + +``` +User: "billing/ never depends on auth/." +Grep "from.*auth" inside src/billing/ → found imports. +Surface contradiction. +``` + +### 5. Naming consistency + +When a new term is introduced, check if a different name for the same concept already exists. + +``` +User plans to add a "RevocationToken". +Grep "Cancel|Revoke|Invalidate.*Token" → found "InvalidationToken" in +src/auth/tokens.ts. Surface: "Same concept already named InvalidationToken. +Reuse the name, or rename the existing one?" +``` + +## Tool Selection + +| Question type | First tool | +|---|---| +| Does symbol X exist? | Grep with the name | +| Where is X defined? | LSP `goToDefinition` if a path is known, else Grep | +| Who calls X? | LSP `findReferences` if precise, else Grep | +| Is there code matching pattern Y? | Grep with regex | +| What files match a structure? | Glob | +| Wide unfamiliar area | Dispatch an Explore agent (`Agent` tool) | + +Use the LSP tool when symbol-precise (avoids false positives from comments, strings, similar names). Use Grep when the question is broader. + +## When to Stop Reading and Start Asking + +Reading the codebase is not free. Stop and ask the user when: + +- You have made 3-5 unsuccessful searches or 2 read-throughs without finding evidence either way +- The question requires intent (why was it built this way?) not facts (what does it do?) +- The question requires future direction not present state +- Reading would require understanding a system far outside the current scope + +The recommended-answer rule (Rule 2) still applies: if you ask after exploration, lead with what you found and what you'd recommend on that basis. + +## Reporting What You Read + +When you read code on the user's behalf, report it concisely: + +``` +WRONG (too verbose): + "I searched the codebase using Grep with the pattern 'retry' and found + several matches in src/lib/retry.ts which is a utility module that + provides exponential backoff functionality with configurable max + attempts and..." + +RIGHT (terse, evidence-first): + "Found `src/lib/withRetry.ts` (exponential backoff, configurable max + attempts). Reuse it. Confirm?" +``` + +Lead with the finding, link the file path, recommend, ask for confirmation. diff --git a/plugins/skill-set/skills/grilling-plans/reference/decision-tree-walk.md b/plugins/skill-set/skills/grilling-plans/reference/decision-tree-walk.md new file mode 100644 index 0000000..4f30577 --- /dev/null +++ b/plugins/skill-set/skills/grilling-plans/reference/decision-tree-walk.md @@ -0,0 +1,73 @@ +# Decision Tree Walk + +How to identify dependency order, when to descend, and what counts as a leaf. + +## Identifying the Tree + +Before asking the first question, read the plan and write down the **undecided branches** — points where the plan is silent, hand-wavy, or admits multiple interpretations. Each branch is a candidate question. + +A useful prompt to surface branches: + +- "What inputs does this assume exist?" +- "What outputs does this commit to?" +- "What error modes are unhandled?" +- "What invariants must hold across boundaries?" +- "What happens at the empty case, the boundary case, the failure case?" +- "What does each domain term in the plan actually mean here?" + +Branches surfaced this way are usually unordered. Order them next. + +## Dependency Order + +Two branches are **dependent** if the answer to one constrains the answer to the other. Resolve the constraining branch first. + +``` +Q: "Which datastore are we using?" ← decide first +Q: "Which transaction isolation?" ← depends on datastore +Q: "How do we test it?" ← depends on both +``` + +If two branches are independent, pick whichever the user can answer faster. Save the high-cognitive-load decisions for when context is rich. + +When you cannot tell whether two branches are dependent, ask the user — but propose your guess with reasoning (Rule 2 still applies). + +## When to Descend vs. Back Up + +After a question is answered, an answer often opens a **sub-tree**: + +``` +Q1: "Which datastore?" → "Postgres" + └─ sub-Q: "Which Postgres version?" + └─ sub-Q: "Connection pooling strategy?" + └─ sub-Q: "Migration tool?" +``` + +**Descend** when the sub-questions are tightly coupled to the answer (changing the parent invalidates them). + +**Back up** when the sub-questions are independent of the answer and would stall progress. + +A useful test: if the user says "let's come back to that," the question belongs higher in the tree, not deeper. Note it on a stack and move on. + +## What Counts as a Leaf + +A branch is a **leaf** (no further questions) when: + +- The answer is captured precisely (specific value, specific term, specific behavior) +- No reasonable reader of the plan would interpret the answer differently +- Implementation can begin from the answer alone + +Watch for **false leaves**: + +- "We'll figure that out in implementation" — not a leaf, just deferred +- "Standard approach" — not a leaf, the standard is the question +- "It depends" — not a leaf, the dependency is the question + +## When to Stop + +Grilling ends when **every branch is a leaf**. Not when the user is tired. Not when 30 minutes have elapsed. Not when the plan "feels good." + +If the user wants to stop early, surface what is still un-leafed: + +> "Three branches remain undecided: cache eviction policy, retry budget, and the meaning of 'partial success' in the response shape. Stopping now means implementation will guess at these. Continue, or accept the guesses?" + +This makes the cost of stopping explicit. The user can still choose to stop — but with eyes open. diff --git a/plugins/skill-set/skills/improving-architecture/SKILL.md b/plugins/skill-set/skills/improving-architecture/SKILL.md new file mode 100644 index 0000000..70de63b --- /dev/null +++ b/plugins/skill-set/skills/improving-architecture/SKILL.md @@ -0,0 +1,165 @@ +--- +name: improving-architecture +description: Surfaces deep-module refactor candidates across a codebase using domain vocabulary and Ousterhout's depth/seam framing — applies the deletion test, presents candidates with locality and leverage justifications, and hands off to the `grilling-plans` skill for the chosen candidate's design. Use this skill whenever the user mentions architecture, refactoring scope, deep/shallow modules, seams, ports/adapters, modularity, or expresses frustration with tangled code — phrases like "improve architecture", "find refactor opportunities", "deep module", "ball-of-mud area", "this code is a mess", "untangle this", "split this module", "make this testable", "extract a seam", "shallow module", "리팩토링 거리 찾아" — even without the word "architecture". Not for reviewing recently changed code (use `simplify`) or designing new features (use `superpowers:brainstorming`). +--- + +# Improving Architecture + +## Overview + +Surface architectural friction in a codebase and propose **deepening opportunities** — refactors that turn shallow modules into deep ones. The aim is testability and AI-navigability, not aesthetic cleanup. + +**Core principle:** A deep module hides a lot of behavior behind a small interface. A shallow module's interface is nearly as complex as its implementation. Find the shallow ones. + +## When to Use + +- The user wants to schedule architectural improvement work +- A bug fix or feature surfaces a tangled area worth improving separately +- Periodic review of an area that has accreted complexity over time +- User says "improve the architecture", "find refactor opportunities", "리팩토링 거리 찾아", "what should we deepen", "ball of mud" + +## Do NOT use for + +- Reviewing recently changed code → `simplify` +- Single-file cleanup (rename, dedupe, format) → just do it inline +- Bug investigation → `superpowers:systematic-debugging` +- New feature design → `superpowers:brainstorming` +- Test strategy → `driving-with-tests` + +## Glossary + +Consistent vocabulary lets two reviewers compare candidates across reviews; drift into "service" / "component" / "boundary" makes it impossible to tell whether two findings are the same or different. That is why the terms below are canonical and used as-is. + +| Term | Meaning | +|---|---| +| **Module** | Anything with an interface and an implementation — function, class, package, slice | +| **Interface** | Everything a caller must know to use the module: types, invariants, error modes, ordering, config — not just the type signature | +| **Implementation** | The code inside the module | +| **Depth** | Leverage at the interface. **Deep** = a lot of behavior behind a small interface. **Shallow** = interface nearly as complex as the implementation. | +| **Seam** | Where an interface lives — a place behavior can be altered without editing in place | +| **Adapter** | A concrete thing satisfying an interface at a seam | +| **Leverage** | What callers gain from depth | +| **Locality** | What maintainers gain from depth — change, bugs, knowledge concentrated in one place | + +Full elaboration in `reference/deep-modules.md`. The deletion test (the most useful single heuristic) lives in `reference/deletion-test.md`. + +## Process + +### 1. Orient + +If `CONTEXT.md` and `docs/adr/` exist (per `building-shared-vocabulary`), read them first — `CONTEXT.md` gives names to good seams, ADRs record decisions to not re-litigate. If absent, infer domain vocabulary from package/module names, test descriptions, and recent commit messages, and proceed. + +### 2. Explore + +Walk the codebase looking for friction. Don't apply rigid heuristics — observe organically and note where understanding is hard: + +- Where does understanding one concept require bouncing between many small files? +- Where is a module's interface nearly as complex as its implementation? (shallow) +- Where have pure functions been extracted just for testability, while the real bugs hide in *how* they're called? (no locality) +- Where do tightly-coupled modules leak across their seams? +- Which parts are untested or hard to test through their current interface? + +For broader sweeps, dispatch an `Explore` agent (`Agent` tool with `subagent_type=Explore`) to walk a directory or feature area in parallel — see `superpowers:dispatching-parallel-agents`. + +Apply the **deletion test** to anything you suspect is shallow: imagine deleting it. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep. Full procedure: `reference/deletion-test.md`. + +### 3. Present candidates + +Number them. For each: + +``` +**N. ** + +- **Files:** +- **Problem:** +- **Solution:** +- **Locality gain:** +- **Leverage gain:** +- **Test impact:** +``` + +**Do not propose specific interfaces yet.** That belongs to the next phase. + +**Vocabulary discipline:** Use `CONTEXT.md` terms for domain concepts ("the Order intake module") and the glossary above for architecture concepts ("a deep seam over the rate limiter"). Do not invent new architectural vocabulary; use the canonical terms. + +**Filled example:** + +``` +**1. Order Intake validation cluster** + +- **Files:** src/orders/intake/promotion-check.ts, inventory-check.ts, + credit-check.ts, route.ts (calls all three) +- **Problem:** Three shallow validators each export a single function; + every caller has to remember the right ordering and aggregate errors + manually. Two of three are also called by the admin Manual Order tool, + which currently re-aggregates errors with a different shape. +- **Solution:** Collapse the three validators behind a single + `validateOrder(order) → ValidationResult` interface that owns the + ordering, the aggregation, and the error shape. +- **Locality gain:** Adding a new check (e.g., fraud scoring) becomes + one edit inside the validation module; today it is three. +- **Leverage gain:** Both the HTTP route and the admin tool stop + re-implementing aggregation; both consume the same `ValidationResult`. +- **Test impact:** Existing per-validator unit tests can stay as + internal helpers; new tests assert against `validateOrder` outcomes — + closer to user-observable behavior. +``` + +**ADR conflicts:** If a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant reopening the decision. Mark it: _"Contradicts ADR-0007 — but worth reopening because…"_. Do not list every theoretical refactor an ADR forbids. + +Ask the user: "Which of these would you like to explore?" + +### 4. Classify dependencies + +Before designing a new interface, classify the candidate's dependencies (see `reference/deepening.md`): + +1. **In-process** — pure computation; merge and test directly +2. **Local-substitutable** — has a local stand-in (PGLite, in-memory FS); use it +3. **Remote but owned** — your services across a network; define a port with 2+ adapters +4. **True external** — third-party (Stripe, Twilio); inject port, mock in tests + +The category determines the seam strategy and what tests look like. + +### 5. (Optional) Design It Twice + +If interface shape is non-obvious, run a parallel sub-agent design generation pass — see `reference/interface-design.md`. Spawn 3+ agents with radically different design constraints (minimal interface / maximum flexibility / optimize-for-common-caller / ports-and-adapters), present the results sequentially, then commit to one. + +Skip this step when the right interface is already obvious. Use it when the user is unsure or when the candidate's interface shape would set a long-term direction. + +### 6. Hand off + +Hand off to `grilling-plans` to interrogate the chosen design. Do not jump to writing implementation plans directly — the candidate is still under-specified, and grilling will surface what is unclear. + +After grilling, ADRs and `CONTEXT.md` updates are owned by `building-shared-vocabulary` — this skill does not write them. + +## Process Flow + +``` +orient (read CONTEXT.md and relevant ADRs) + → explore codebase for friction (optionally via Explore subagents) + → apply deletion test to suspect shallow modules + → present numbered candidates with locality / leverage / test impact + → user picks one? no → end (note for later) + yes → classify dependencies + (in-process / local-sub / remote-owned / true-external) + → interface shape obvious? yes → hand off to grilling-plans + no → Design It Twice + → hand off to grilling-plans +``` + +## Reference + +- `reference/deep-modules.md` — full elaboration of the depth / seam / adapter / locality vocabulary, with examples of deep vs. shallow at function, module, and package scales +- `reference/deletion-test.md` — how to actually run the deletion test, what counts as "complexity reappears", common false-negatives +- `reference/deepening.md` — dependency categorization (in-process / local-substitutable / remote-owned / true-external) and the testing strategy each implies +- `reference/interface-design.md` — the "Design It Twice" parallel sub-agent pattern for generating radically different interface candidates before committing + +## Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| Candidate list grows past 5–7 items | Including every theoretical improvement | Cut to the friction you actually felt during exploration; reject the speculative ones. (Past ~7 dilutes user attention; pick the highest-leverage subset.) | +| User can't choose between candidates | Candidates not differentiated by impact | Rank by locality/leverage gain, not by file count or apparent size | +| Output uses generic terms ("service", "boundary", "component") | Vocabulary discipline broke | Re-edit using only the glossary terms above; the precision is the point | +| Suggesting a candidate that contradicts a recent ADR | Did not read ADRs in step 1 | Read the ADR; either drop the candidate or surface it explicitly as an ADR-reopening proposal | +| Candidates require ground-up rewrites | Bar set too high | Look for *one-step* deepenings — refactors a single PR could deliver — not architectural revolutions | diff --git a/plugins/skill-set/skills/improving-architecture/reference/deep-modules.md b/plugins/skill-set/skills/improving-architecture/reference/deep-modules.md new file mode 100644 index 0000000..8d2b1d5 --- /dev/null +++ b/plugins/skill-set/skills/improving-architecture/reference/deep-modules.md @@ -0,0 +1,129 @@ +# Deep Modules + +Full elaboration of the depth / seam / adapter / locality vocabulary, with examples at function, module, and package scales. + +Source: John Ousterhout, *A Philosophy of Software Design*. The vocabulary here adapts that book's framing to be operational for an agent. + +## Contents + +- [Module / Interface / Implementation](#module) +- [Depth — the central concept](#depth) +- [Examples by scale](#examples-by-scale) +- [Seam / Adapter](#seam) +- [Leverage / Locality](#leverage) +- [How these terms compose](#how-these-terms-compose) +- [Common shallow patterns to look for](#common-shallow-patterns-to-look-for) +- [Things that look shallow but are not](#things-that-look-shallow-but-are-not) + +## Module + +Anything with an interface and an implementation. The *scale* varies — a single function is a module, a class is a module, a package is a module, a microservice is a module. Reasoning about depth applies at every scale. + +## Interface + +Everything a caller must know to use the module. This is *not just the type signature* — it includes: + +- Type signatures (input, output, errors) +- Invariants the caller must preserve (preconditions) +- Invariants the module guarantees (postconditions) +- Ordering constraints (must call X before Y) +- Side effects (writes to disk, network, mutates input) +- Error modes (what can fail, how it surfaces, recovery options) +- Configuration (what the caller has to decide) + +Interface = the surface area of cognitive load on every caller. + +## Implementation + +The code inside the module. Everything callers do *not* need to know. + +## Depth + +The ratio between leverage gained and interface complexity carried. + +``` +deep: [ ============= IMPLEMENTATION ============= ] + [ interface ] + +shallow: [ implementation ] + [ interface ] +``` + +A **deep module** does a lot of work behind a small interface. A caller passes minimal information, gets significant behavior in return. + +A **shallow module** is one whose interface is nearly as complex as its implementation. Callers carry almost as much cognitive load as they would have without the module. + +## Examples by scale + +### Function scale + +**Deep:** `parseISODate(s: string) → Date | ParseError`. One input, one well-typed output, all the date-parsing horror inside. + +**Shallow:** `applyOrderDiscount(order, discount, taxRate, locale, customerTier, productCategory, isHoliday, ...) → Order`. The caller has to assemble all the inputs anyway — the function is barely doing anything the call site wasn't already doing. + +### Module scale + +**Deep:** A `RetryPolicy` module that exposes `withRetry(operation, policy)`. Callers don't think about exponential backoff, jitter, max attempts, error classification — those live behind the interface. + +**Shallow:** A `RetryHelper` that exposes `getBackoffMs`, `shouldRetry`, `incrementAttempt`, `resetAttempts`. Every caller has to compose these correctly. The "module" is just a namespaced bag of helpers. + +### Package scale + +**Deep:** A `billing/` package that exposes `chargeCustomer(customerId, amount, idempotencyKey)`. Inside: payment provider abstraction, retry on transient failures, idempotency checks, audit logging, ledger writes. + +**Shallow:** A `billing/` package that exposes `getPaymentProvider`, `formatAmount`, `validateAmount`, `recordCharge`, `recordRefund`, `auditLog`. Every caller has to thread these together. The package adds no leverage over a folder. + +## Seam + +A seam is a point where the interface lives — a place behavior can be altered without editing the existing code in place. + +A seam is *interesting* when there's a real reason to vary behavior at it: testing (substitute a fake), product needs (swap providers), platform constraints (switch implementations). A seam invented for "potential future flexibility" is just speculative complexity. + +> One adapter = hypothetical seam. Two adapters = real seam. + +If only one adapter exists and is the only one that ever will exist, the seam is fictional and can usually be deleted. + +## Adapter + +A concrete thing that satisfies an interface at a seam. `PostgresUserRepo` and `InMemoryUserRepo` are two adapters at the `UserRepo` seam. + +Adapters justify seams. A seam without at least two real adapters is suspicious. + +## Leverage + +The user-facing benefit of depth. What does the caller stop having to think about? What gets shorter? What can callers compose now that they couldn't before? + +## Locality + +The maintainer-facing benefit of depth. When this concept changes, where do the changes go? + +- **High locality:** changes to one concept stay in one module +- **Low locality:** changes to one concept ripple across N callers + +Locality is the property that lets a small team reason about a big system. Depth is the mechanism that produces locality. + +## How these terms compose + +> A **deep module** sits behind a **seam** with one or more **adapters**. Its **interface** carries low cognitive load relative to its **implementation**, giving callers **leverage** and concentrating maintenance in one place — that's **locality**. + +When proposing a deepening, the proposal should make explicit what changes for **leverage** (callers) and **locality** (maintainers). If both can't be named, the deepening isn't real. + +## Common shallow patterns to look for + +| Pattern | Smell | +|---|---| +| Helper module with N independent functions | Each caller has to compose them — locality wasted | +| Pure function extracted from a stateful flow for "testability" | The bugs are in the stateful flow; pure-function tests pass while the real path breaks | +| Wrapper that just renames calls | Pass-through with vocabulary churn — see deletion test | +| "Service" that exposes its CRUD | Callers know the storage shape; the seam isn't doing anything | +| Configuration object passed through three layers | Each layer is a shallow conduit; consider pushing the consumer up to the source | + +## Things that look shallow but are not + +| Looks shallow | But actually | +|---|---| +| One-line wrapper | If it consolidates an invariant in one place, it's earning its keep | +| Tiny module with one function | If two adapters exist, the seam is real | +| Module with verbose interface | Verbose ≠ shallow — a deep module can have many parameters if each one carries real semantic weight | + +The deletion test is the disambiguator. Run it when in doubt. diff --git a/plugins/skill-set/skills/improving-architecture/reference/deepening.md b/plugins/skill-set/skills/improving-architecture/reference/deepening.md new file mode 100644 index 0000000..053cf0b --- /dev/null +++ b/plugins/skill-set/skills/improving-architecture/reference/deepening.md @@ -0,0 +1,52 @@ +# Deepening + +How to deepen a cluster of shallow modules safely, given its dependencies. Assumes the vocabulary in `deep-modules.md` — **module**, **interface**, **seam**, **adapter**. + +## Dependency categories + +When assessing a candidate for deepening, classify its dependencies. The category determines how the deepened module is tested across its seam. + +### 1. In-process + +Pure computation, in-memory state, no I/O. **Always deepenable** — merge the modules and test through the new interface directly. No adapter needed. + +### 2. Local-substitutable + +Dependencies that have local test stand-ins (PGLite for Postgres, in-memory filesystem, embedded Redis). **Deepenable if the stand-in exists.** The deepened module is tested with the stand-in running in the test suite. The seam is internal; no port at the module's external interface. + +### 3. Remote but owned (Ports & Adapters) + +Your own services across a network boundary — microservices, internal APIs, your own message queues. **Define a port (interface) at the seam.** The deep module owns the logic; the transport is injected as an **adapter**. Tests use an in-memory adapter. Production uses an HTTP / gRPC / queue adapter. + +Recommendation shape: + +> "Define a port at the seam, implement an HTTP adapter for production and an in-memory adapter for testing, so the logic sits in one deep module even though it's deployed across a network." + +### 4. True external (Mock) + +Third-party services you don't control — Stripe, Twilio, OAuth providers. **Take the external dependency as an injected port; tests provide a mock adapter.** Different from category 3 only in that you cannot control the wire-level behavior, so the mock is not a proxy for "real but cheap" — it's an explicit fiction the test owns. + +## Seam discipline + +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a port unless at least two adapters are justified (typically production + test). A single-adapter seam is just indirection. +- **Internal seams vs. external seams.** A deep module can have internal seams (private to its implementation, used by its own tests) as well as the external seam at its interface. **Don't expose internal seams through the interface just because tests use them** — that leaks implementation into the contract. + +## Testing strategy: replace, don't layer + +When deepening shallow modules: + +- **Old unit tests on the shallow modules become candidates for removal** once tests at the deepened module's interface exist — consider deleting the now-redundant ones, but keep any that document a non-obvious invariant a future reader would need. +- **Write new tests at the deepened module's interface.** The interface is the test surface. +- **Tests assert on observable outcomes through the interface, not internal state.** +- **Tests should survive internal refactors** — they describe behavior, not implementation. If a test has to change when the implementation changes, it's testing past the interface. + +## Choosing the strategy in practice + +``` +Pure computation? → Category 1. Just merge. +Has a local stand-in? → Category 2. Use it. +Network-bounded, you own both sides? → Category 3. Port + 2 adapters. +Third-party you don't own? → Category 4. Mock at the seam. +``` + +If a single deepening candidate spans multiple categories (e.g., it computes plus calls Stripe), split the seams: pure computation stays internal, the Stripe call gets its own port + mock. diff --git a/plugins/skill-set/skills/improving-architecture/reference/deletion-test.md b/plugins/skill-set/skills/improving-architecture/reference/deletion-test.md new file mode 100644 index 0000000..da55c77 --- /dev/null +++ b/plugins/skill-set/skills/improving-architecture/reference/deletion-test.md @@ -0,0 +1,71 @@ +# The Deletion Test + +The single most useful heuristic for distinguishing deep from shallow modules. + +## The test + +Imagine deleting the module. Inline its body at every call site. + +- **If complexity vanishes:** the module was a pass-through. It added vocabulary but no leverage. *Shallow.* Delete or merge. +- **If complexity reappears across N callers:** the module was concentrating something. Each caller would now have to reproduce the work. *Deep.* Keep it. +- **If the module mostly disappears but a small invariant has to be re-asserted at each call site:** the module was earning its keep at exactly that invariant. Keep it, possibly trim the surrounding code. + +## How to actually run it + +You don't need to literally delete the file. Run it as a thought experiment in three steps: + +### Step 1 — list callers + +Use `LSP findReferences` on the public API of the module. Get the actual call sites. + +### Step 2 — for each call site, write what would need to change + +For each caller, ask: if the module disappeared, what would I have to put here? + +- Just the function body? → That's a pass-through. +- The function body plus a couple of guards / coercions / wrap-unwraps? → Borderline; think about whether those guards are the same at every site. +- The function body plus state, error handling, retries, ordering? → The module is doing real work. + +### Step 3 — count the duplication + +If the same non-trivial thing would be repeated at 3+ call sites, the module is deep enough to keep. Two callers can be a coincidence; three suggests a pattern worth concentrating in one place. + +If the "complexity" is just type coercion and could be replaced by a one-liner everywhere, it's shallow. + +## What counts as "complexity reappearing" + +| Counts | Doesn't count | +|---|---| +| Stateful logic — counters, retries, caches | Argument forwarding | +| Invariant assertions that callers would otherwise forget | Type narrowing the caller already does | +| Coordinated multi-step work | Renaming a single function call | +| Error classification with non-trivial branching | Error re-throw | +| Locking, ordering, atomicity | Plain dispatch | + +## Common false-negatives (modules that *look* shallow but pass the test) + +- **Validation modules.** Look like one function calling N small checks. But the checks together encode a domain rule that callers would otherwise duplicate poorly. The module is the *single source of truth* for the rule. +- **Idempotency wrappers.** May look like passthroughs. But the idempotency key handling, ledger lookup, and outcome caching would be a nightmare to reproduce per caller. +- **Migration shims.** Often have a tiny implementation but their value is "all the legacy handling lives here, callers stay clean." + +## Common false-positives (modules that *look* deep but fail the test) + +- **Large helper bags.** A module with ten unrelated functions can look "big" but each function is shallow on its own and callers compose them anyway. +- **Wrappers that just rename.** A `BillingService.charge()` that just calls `paymentProvider.charge()` adds vocabulary, no leverage. +- **"Service" objects with leaked internals.** If callers reach inside (`service.repo.findById(...)`), the seam was theatrical. + +## When the test is ambiguous + +Apply the **two-adapter test**: does this module have two real, used adapters? If yes, the seam is real. If no, the seam is hypothetical and can usually be deleted. + +If still ambiguous, leave it alone. Architectural review should not propose changes the team is unsure about. List it under "watch" rather than "propose." + +## What to do with the result + +| Result | Action | +|---|---| +| Clearly shallow, single caller | Inline at the call site, delete the module | +| Clearly shallow, multiple callers | Inline; if duplication appears across callers, propose a different deepening that captures the *real* invariant | +| Clearly deep | Leave alone. Possibly improve naming or trim incidental complexity. | +| Borderline | Note it; come back when you have more callers or more friction | +| Two-adapter test fails (only one adapter, no plan for another) | Propose collapsing the seam — the indirection is unjustified | diff --git a/plugins/skill-set/skills/improving-architecture/reference/interface-design.md b/plugins/skill-set/skills/improving-architecture/reference/interface-design.md new file mode 100644 index 0000000..2b8a81f --- /dev/null +++ b/plugins/skill-set/skills/improving-architecture/reference/interface-design.md @@ -0,0 +1,56 @@ +# Interface Design + +**When to skip:** if the right interface is already obvious for the chosen candidate, skip this entire reference and hand off to `grilling-plans` directly. Use this only when the interface shape would set a long-term direction and the user is unsure between alternatives. + +When the user picks a deepening candidate and you need to explore alternative interfaces for the deepened module, use this **parallel sub-agent pattern**. + +Based on Ousterhout's "Design It Twice": your first interface idea is unlikely to be the best. Force yourself to generate radically different alternatives, then choose. + +Uses the vocabulary in `deep-modules.md` — **module**, **interface**, **seam**, **adapter**, **leverage**. + +## Process + +### 1. Frame the problem space + +Before spawning sub-agents, write a user-facing explanation of the problem space for the chosen candidate: + +- The constraints any new interface would need to satisfy +- The dependencies it would rely on, and which category they fall into (see `deepening.md`) +- A rough illustrative code sketch to ground the constraints — not a proposal, just a way to make the constraints concrete + +Show this to the user, then immediately proceed to step 2. The user reads and thinks while sub-agents work in parallel. + +### 2. Spawn sub-agents + +Use the `Agent` tool with `superpowers:dispatching-parallel-agents` to spawn 3+ agents in parallel. Each must produce a **radically different** interface for the deepened module. (Ousterhout's original argument is "design it twice"; we go to three because the fourth Ports & Adapters constraint only fires for category-3/4 dependencies, so three is the realistic floor when network seams are in play.) + +Prompt each sub-agent with a separate technical brief: file paths, coupling details, dependency category from `deepening.md`, what sits behind the seam. The brief is independent of the user-facing problem-space explanation. Give each agent a different design constraint: + +- **Agent 1: Minimize the interface.** Aim for 1–3 entry points max. Maximize leverage per entry point. +- **Agent 2: Maximize flexibility.** Support many use cases and extension points. +- **Agent 3: Optimize for the most common caller.** Make the default case trivial; rare cases may carry extra ceremony. +- **Agent 4 (when applicable): Ports & Adapters.** Design around the dependency seams from `deepening.md` so adapters can vary without touching the logic. + +Include both architecture vocabulary (`deep-modules.md`) and project domain vocabulary (`CONTEXT.md`) in each brief so each sub-agent names things consistently. + +Each sub-agent outputs: + +1. **Interface** — types, methods, params, plus invariants, ordering, error modes +2. **Usage example** showing how callers use it +3. **What the implementation hides** behind the seam +4. **Dependency strategy** and adapters (per `deepening.md`) +5. **Trade-offs** — where leverage is high, where it's thin + +### 3. Present and compare + +Present designs **sequentially** so the user can absorb each one, then compare them in prose. Contrast by: + +- **Depth** — leverage at the interface +- **Locality** — where change concentrates +- **Seam placement** — what varies, what's fixed + +After comparing, give your own recommendation: which design you think is strongest and why. **Be opinionated — the user wants a strong read, not a menu.** If elements from different designs combine well, propose a hybrid. + +### 4. Hand off to grilling + +Once the user picks a design, hand off to `grilling-plans` to interrogate it before implementation. Designs always look better in the abstract; grilling forces the unspoken assumptions into the open. diff --git a/plugins/skill-set/skills/zooming-out-on-code/SKILL.md b/plugins/skill-set/skills/zooming-out-on-code/SKILL.md new file mode 100644 index 0000000..f25b9de --- /dev/null +++ b/plugins/skill-set/skills/zooming-out-on-code/SKILL.md @@ -0,0 +1,109 @@ +--- +name: zooming-out-on-code +description: Draws a higher-level system map of unfamiliar internal/project code in the project's domain vocabulary — describes the module's responsibility, callers, dependencies, and sibling modules without diving into implementation details. Use this skill proactively whenever a user expresses unfamiliarity with a project code area before changing it — phrases like "zoom out", "give me the big picture", "explain this code area", "new to this code", "where does X fit", "what calls this", "tour the module", "before I change this", "이 코드 큰 그림", "이 모듈 어디 쓰여" — even if they don't say "zoom out" explicitly. +--- + +# Zooming Out On Code + +## Overview + +When dropped into unfamiliar code, the agent's first instinct is often to read the file linearly. That produces local understanding without orientation. **Zooming out** does the opposite: go up one level of abstraction first, so the file in question is read with full system context. + +**Core principle:** Understand a module by understanding its place, not its body. + +## When to Use + +- User opens a file or function they don't recognize and asks for context +- Before proposing changes to an unfamiliar area +- When a teammate hands off a system you have no prior context on +- User says "zoom out", "explain this code area", "give me the big picture", "what does this do in context", "이 코드 큰 그림", "이 모듈 어디 쓰여" + +## Do NOT use for + +- External library / framework documentation → `understanding-code-context` +- Reviewing changed code for quality → `simplify` +- Finding refactor opportunities → `improving-architecture` +- Debugging a specific bug → `superpowers:systematic-debugging` + +## Process + +1. **Read project context first.** If `CONTEXT.md` and `docs/adr/` exist (per `building-shared-vocabulary`), read them first. If absent, infer domain vocabulary from package/module names, public type/class names that mention business nouns, test descriptions, and recent commit messages. + +2. **Go up one level.** + - If the user asks about a function, describe its containing module + - If the user asks about a module, describe its containing package or layer + - If the user asks about a package, describe the system + - If the user asks about a top-level package or whole app, describe its role in the product/business domain — do not recurse further (recursing past the top-level package turns orientation into product strategy, which is out of scope and rarely useful for a code change) + +3. **Map four things.** These four answer "what is it for, who needs it, what does it need, what's adjacent" — together they place the module in the dependency graph without reading its body. + - **Responsibility** — what is this module's single reason to exist, in domain vocabulary? + - **Callers** — who depends on this module? Grep on the public API name; use `LSP findReferences` if an LSP is available for the language. + - **Dependencies** — what does this module depend on? Read first-level imports only; transitive imports belong to a deeper zoom, not this one. + - **Siblings** — what other modules sit at the same level of the system? Glob the parent directory. + +4. **Answer in domain language, not implementation language.** "The Order intake module validates incoming Orders against the active Promotion catalog" beats "OrderHandler calls promotionService.validate()." + +5. **Stop at the right level.** The goal is *orientation*, not *encyclopedia*. If the user wants to go deeper into a specific branch, they'll ask. + +## Output template + +``` +**** + +**Responsibility:** + +**Callers:** +- — +- — + +**Dependencies:** +- — +- — + +**Siblings (same layer):** +- — +- — + +**Where it sits in the system:** + +``` + +## Example + +User asks about `src/orders/intake/validate.ts`. + +``` +**Order Intake — Validation** + +**Responsibility:** Confirms an incoming Order satisfies promotion eligibility, +inventory availability, and customer credit before the Order is accepted. + +**Callers:** +- `src/orders/intake/route.ts` — POST /orders endpoint, calls validation before persisting +- `src/admin/manual-orders.ts` — admin tool that creates Orders on behalf of a Customer + +**Dependencies:** +- `src/promotions/catalog.ts` — looks up active promotions for the Customer +- `src/inventory/availability.ts` — confirms each Line Item is stocked +- `src/billing/credit.ts` — reads Customer credit limit + +**Siblings (same layer — `src/orders/intake/`):** +- `route.ts` — HTTP boundary +- `persist.ts` — writes accepted Orders to the write model +- `events.ts` — emits `OrderPlaced` events on success + +**Where it sits in the system:** +Validation is the gate between an HTTP request and a confirmed Order in the write +model. After it passes, persist + event emission are mechanical; before it passes, +nothing about the Order is durable. +``` + +## Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| Output sounds like the file's docstring | Did not actually go up a level | Re-do step 2 — describe the *containing* unit, not the unit itself | +| Output uses class/function names instead of domain terms | Skipped step 1 (didn't read CONTEXT.md) or no glossary exists | Read CONTEXT.md if present; otherwise note its absence and use the domain terms inferable from the code | +| Caller list is empty but module clearly is in use | API is exported through a barrel/index file | Search for re-exports, then trace from there | +| Output is huge | Stopped at the wrong level | Cut to the four-item template strictly; deeper detail belongs to follow-up questions | From 7f413dcdfb25c3a854e7075674ed59812abdb71b Mon Sep 17 00:00:00 2001 From: Ether Date: Mon, 4 May 2026 18:23:28 +0900 Subject: [PATCH 2/3] docs(improving-architecture): add language tags to fenced code blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `text` language identifier to four fenced code blocks flagged by markdownlint rule MD040 (fenced-code-language): - SKILL.md:70 — candidate template block - SKILL.md:87 — filled candidate example block - SKILL.md:137 — process-flow diagram block - reference/deepening.md:45 — strategy-selection diagram block Addresses CodeRabbit OBVIOUS review comments on PR #21. --- plugins/skill-set/skills/improving-architecture/SKILL.md | 6 +++--- .../skills/improving-architecture/reference/deepening.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/skill-set/skills/improving-architecture/SKILL.md b/plugins/skill-set/skills/improving-architecture/SKILL.md index 70de63b..0f25c6e 100644 --- a/plugins/skill-set/skills/improving-architecture/SKILL.md +++ b/plugins/skill-set/skills/improving-architecture/SKILL.md @@ -67,7 +67,7 @@ Apply the **deletion test** to anything you suspect is shallow: imagine deleting Number them. For each: -``` +```text **N. ** - **Files:** @@ -84,7 +84,7 @@ Number them. For each: **Filled example:** -``` +```text **1. Order Intake validation cluster** - **Files:** src/orders/intake/promotion-check.ts, inventory-check.ts, @@ -134,7 +134,7 @@ After grilling, ADRs and `CONTEXT.md` updates are owned by `building-shared-voca ## Process Flow -``` +```text orient (read CONTEXT.md and relevant ADRs) → explore codebase for friction (optionally via Explore subagents) → apply deletion test to suspect shallow modules diff --git a/plugins/skill-set/skills/improving-architecture/reference/deepening.md b/plugins/skill-set/skills/improving-architecture/reference/deepening.md index 053cf0b..459d3f7 100644 --- a/plugins/skill-set/skills/improving-architecture/reference/deepening.md +++ b/plugins/skill-set/skills/improving-architecture/reference/deepening.md @@ -42,7 +42,7 @@ When deepening shallow modules: ## Choosing the strategy in practice -``` +```text Pure computation? → Category 1. Just merge. Has a local stand-in? → Category 2. Use it. Network-bounded, you own both sides? → Category 3. Port + 2 adapters. From 7c3839862cfe92889714214c589c1e3ce6d739cc Mon Sep 17 00:00:00 2001 From: Ether Date: Mon, 4 May 2026 18:27:47 +0900 Subject: [PATCH 3/3] docs(skills): remove Korean trigger phrases per repo English-only policy Removes Korean-language trigger phrases from descriptions and "When to Use" sections of the four new skills, per AGENTS.md repository language policy ("All repository content MUST be written in English by default"). Korean runtime triggers were CodeRabbit review feedback on PR #21. Affected skills: - grilling-plans - building-shared-vocabulary - zooming-out-on-code - improving-architecture --- plugins/skill-set/skills/building-shared-vocabulary/SKILL.md | 4 ++-- plugins/skill-set/skills/grilling-plans/SKILL.md | 4 ++-- plugins/skill-set/skills/improving-architecture/SKILL.md | 4 ++-- plugins/skill-set/skills/zooming-out-on-code/SKILL.md | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md b/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md index 238dc20..c97e9b8 100644 --- a/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md +++ b/plugins/skill-set/skills/building-shared-vocabulary/SKILL.md @@ -1,6 +1,6 @@ --- name: building-shared-vocabulary -description: Maintains a project's domain glossary in CONTEXT.md and architecture decisions in docs/adr/ as living artifacts — files are created lazily, updated inline as terms resolve and decisions crystallize, and conflicts with existing entries are surfaced immediately. Use this skill whenever a domain term is being pinned down, a decision sounds ADR-worthy, or vocabulary is about to drift — phrases like "domain glossary", "용어집", "context.md", "ADR 만들자", "도메인 용어 정리", "we should write this down", "let's name this", "what do we call this", "is this worth an ADR", "pin down this term" — and as a side-effect of grilling-plans when terms or decisions surface. +description: Maintains a project's domain glossary in CONTEXT.md and architecture decisions in docs/adr/ as living artifacts — files are created lazily, updated inline as terms resolve and decisions crystallize, and conflicts with existing entries are surfaced immediately. Use this skill whenever a domain term is being pinned down, a decision sounds ADR-worthy, or vocabulary is about to drift — phrases like "domain glossary", "context.md", "we should write this down", "let's name this", "what do we call this", "is this worth an ADR", "pin down this term" — and as a side-effect of grilling-plans when terms or decisions surface. --- # Building Shared Vocabulary @@ -18,7 +18,7 @@ A project's **domain glossary** (`CONTEXT.md`) and **architecture decision recor - A domain term is being pinned down during conversation (especially during grilling) - A term in the user's plan conflicts with how the codebase uses it - A decision that is **hard-to-reverse**, **surprising-without-context**, AND **the result of a real trade-off** has been made — record an ADR -- User explicitly asks to "build a glossary," "set up CONTEXT.md," "add an ADR," "용어집 만들자," "도메인 용어 정리" +- User explicitly asks to "build a glossary," "set up CONTEXT.md," "add an ADR" ## Do NOT use for diff --git a/plugins/skill-set/skills/grilling-plans/SKILL.md b/plugins/skill-set/skills/grilling-plans/SKILL.md index 5c19fbe..fae0b45 100644 --- a/plugins/skill-set/skills/grilling-plans/SKILL.md +++ b/plugins/skill-set/skills/grilling-plans/SKILL.md @@ -1,6 +1,6 @@ --- name: grilling-plans -description: Adversarially interrogates an existing plan, design, or proposal before implementation — walks the decision tree one question at a time, provides a recommended answer with each question, prefers codebase exploration over questions, and surfaces contradictions between stated intent and actual code. Use this skill whenever a plan, design doc, RFC, ADR draft, ticket spec, or implementation outline is shared and the user wants review, sanity check, sign-off, or asks "is this ready" — even without the word "grill". Trigger phrases include "grill me", "challenge this plan", "poke holes", "stress test", "내 계획 부숴봐", "구멍 찾아봐". Also use before locking down a spec for implementation. +description: Adversarially interrogates an existing plan, design, or proposal before implementation — walks the decision tree one question at a time, provides a recommended answer with each question, prefers codebase exploration over questions, and surfaces contradictions between stated intent and actual code. Use this skill whenever a plan, design doc, RFC, ADR draft, ticket spec, or implementation outline is shared and the user wants review, sanity check, sign-off, or asks "is this ready" — even without the word "grill". Trigger phrases include "grill me", "challenge this plan", "poke holes", "stress test". Also use before locking down a spec for implementation. --- # Grilling Plans @@ -19,7 +19,7 @@ This is a discipline skill. Each rule below corresponds to a specific way grilli - Between `superpowers:brainstorming` (creation) and `superpowers:writing-plans` (lock-down) - Before invoking `superpowers:executing-plans` - Right before a PR description is finalized -- User says "challenge this", "poke holes", "stress test", "grill me", "내 계획 부숴봐", "구멍 찾아봐" +- User says "challenge this", "poke holes", "stress test", "grill me" - When a candidate is selected from `improving-architecture` **Do NOT use for:** diff --git a/plugins/skill-set/skills/improving-architecture/SKILL.md b/plugins/skill-set/skills/improving-architecture/SKILL.md index 0f25c6e..719c2e0 100644 --- a/plugins/skill-set/skills/improving-architecture/SKILL.md +++ b/plugins/skill-set/skills/improving-architecture/SKILL.md @@ -1,6 +1,6 @@ --- name: improving-architecture -description: Surfaces deep-module refactor candidates across a codebase using domain vocabulary and Ousterhout's depth/seam framing — applies the deletion test, presents candidates with locality and leverage justifications, and hands off to the `grilling-plans` skill for the chosen candidate's design. Use this skill whenever the user mentions architecture, refactoring scope, deep/shallow modules, seams, ports/adapters, modularity, or expresses frustration with tangled code — phrases like "improve architecture", "find refactor opportunities", "deep module", "ball-of-mud area", "this code is a mess", "untangle this", "split this module", "make this testable", "extract a seam", "shallow module", "리팩토링 거리 찾아" — even without the word "architecture". Not for reviewing recently changed code (use `simplify`) or designing new features (use `superpowers:brainstorming`). +description: Surfaces deep-module refactor candidates across a codebase using domain vocabulary and Ousterhout's depth/seam framing — applies the deletion test, presents candidates with locality and leverage justifications, and hands off to the `grilling-plans` skill for the chosen candidate's design. Use this skill whenever the user mentions architecture, refactoring scope, deep/shallow modules, seams, ports/adapters, modularity, or expresses frustration with tangled code — phrases like "improve architecture", "find refactor opportunities", "deep module", "ball-of-mud area", "this code is a mess", "untangle this", "split this module", "make this testable", "extract a seam", "shallow module" — even without the word "architecture". Not for reviewing recently changed code (use `simplify`) or designing new features (use `superpowers:brainstorming`). --- # Improving Architecture @@ -16,7 +16,7 @@ Surface architectural friction in a codebase and propose **deepening opportuniti - The user wants to schedule architectural improvement work - A bug fix or feature surfaces a tangled area worth improving separately - Periodic review of an area that has accreted complexity over time -- User says "improve the architecture", "find refactor opportunities", "리팩토링 거리 찾아", "what should we deepen", "ball of mud" +- User says "improve the architecture", "find refactor opportunities", "what should we deepen", "ball of mud" ## Do NOT use for diff --git a/plugins/skill-set/skills/zooming-out-on-code/SKILL.md b/plugins/skill-set/skills/zooming-out-on-code/SKILL.md index f25b9de..29138eb 100644 --- a/plugins/skill-set/skills/zooming-out-on-code/SKILL.md +++ b/plugins/skill-set/skills/zooming-out-on-code/SKILL.md @@ -1,6 +1,6 @@ --- name: zooming-out-on-code -description: Draws a higher-level system map of unfamiliar internal/project code in the project's domain vocabulary — describes the module's responsibility, callers, dependencies, and sibling modules without diving into implementation details. Use this skill proactively whenever a user expresses unfamiliarity with a project code area before changing it — phrases like "zoom out", "give me the big picture", "explain this code area", "new to this code", "where does X fit", "what calls this", "tour the module", "before I change this", "이 코드 큰 그림", "이 모듈 어디 쓰여" — even if they don't say "zoom out" explicitly. +description: Draws a higher-level system map of unfamiliar internal/project code in the project's domain vocabulary — describes the module's responsibility, callers, dependencies, and sibling modules without diving into implementation details. Use this skill proactively whenever a user expresses unfamiliarity with a project code area before changing it — phrases like "zoom out", "give me the big picture", "explain this code area", "new to this code", "where does X fit", "what calls this", "tour the module", "before I change this" — even if they don't say "zoom out" explicitly. --- # Zooming Out On Code @@ -16,7 +16,7 @@ When dropped into unfamiliar code, the agent's first instinct is often to read t - User opens a file or function they don't recognize and asks for context - Before proposing changes to an unfamiliar area - When a teammate hands off a system you have no prior context on -- User says "zoom out", "explain this code area", "give me the big picture", "what does this do in context", "이 코드 큰 그림", "이 모듈 어디 쓰여" +- User says "zoom out", "explain this code area", "give me the big picture", "what does this do in context" ## Do NOT use for