From c8d0af6095b9c8fa964c7a1afbe95cff3a91e259 Mon Sep 17 00:00:00 2001 From: Ravi Suhag Date: Sat, 8 Aug 2026 11:58:44 +0530 Subject: [PATCH 1/2] feat: add design-review skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Claude Code skill for reviewing designs rather than code quality: API surface, maintenance contracts, whether the machinery fits the size of the problem. Works in three modes — reviewing a PR, self- reviewing a branch before opening a PR, and auditing an existing module. --- .claude/skills/design-review/SKILL.md | 134 ++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 .claude/skills/design-review/SKILL.md diff --git a/.claude/skills/design-review/SKILL.md b/.claude/skills/design-review/SKILL.md new file mode 100644 index 000000000..b451ad04d --- /dev/null +++ b/.claude/skills/design-review/SKILL.md @@ -0,0 +1,134 @@ +--- +name: design-review +description: Review the design of a change or a codebase — architecture, API surface, and whether the machinery fits the problem — rather than code quality. Use to analyse a PR's design, self-review your own branch before opening a PR, or audit an existing module or package. For bug-hunting line review use /review or /code-review instead. +--- + +# Design Review + +Review a design: what it exposes, what it costs to maintain, and +whether the machinery fits the size of the problem. The same questions +apply whether you are reviewing someone else's PR, your own work before +you open one, or a module that already shipped. Only the input and the +output change. + +## Phase 0 — Pick the mode + +- **Reviewer mode**: the target is a PR. Output is a proposal comment + on the PR. +- **Author mode**: the target is your own branch or working tree, + before the PR exists. Output is fixes applied now, plus material for + the PR description. Cheapest time to run this review — every cliff + found here is one no reviewer has to argue about. +- **Audit mode**: the target is an existing module, package, or public + API. Output is a written report with ranked proposals. + +## Phase 1 — Understand, then explain simply + +Get the real code, not a description of it: + +- Reviewer: `gh pr view --json title,body,files,additions,deletions` + and `git fetch origin pull//head:pr-`. +- Author: diff against the merge base + (`git diff $(git merge-base HEAD main)...HEAD`). +- Audit: start from the entry points — `package.json` `exports`, the + barrel files, the README. + +Before any judgment, explain in plain terms: what problem does this +solve, and what is the approach? If you cannot explain it simply, you +do not understand it yet. In author mode, write this explanation down — +it becomes the PR description's opening. + +## Phase 2 — Verify every quantitative claim + +Claims come from the PR description, the design doc, the README — or, +in author mode, from your own head. All get checked against the code +(`git show`, `git grep` on the target ref). + +The key split: what does the code use internally vs what does it +export for consumers? The gap between those two numbers is usually +where the design question lives. + +## Phase 3 — Interrogate the API surface + +For each public export ask: + +- Who needs this, and what breaks for them if it goes away? +- What is the maintenance contract? Every exported name is API to keep + stable forever. +- **Find the cliff**: what happens to the user who needs one more thing + than the curated set provides? Check the escape hatch actually exists + — is the factory/helper exported? Are the types open or a closed + union? Read the entry-point file, not the docs. + +## Phase 4 — The value question + +What does this layer add over the underlying dependency? If a consumer +bypassed it and used the dependency directly, what would they lose? +If the answer is "consistency and a stable contract", say exactly that +— do not inflate it into capability the layer doesn't have. + +## Phase 5 — Right-size the machinery + +Does implementation complexity match problem size? Any supporting +machinery — extra build steps, generated files, custom tooling or +checks — earns its keep at scale N; ask what N actually is here. A +plain, committed artifact that existing tooling (compiler, tests) can +check often replaces all of it. Rule of thumb: machinery that exists +to avoid writing K lines by hand is only justified when K is large. + +## Phase 6 — Stress-test the alternative against the roadmap + +Before proposing something simpler, check it still supports the known +future directions — ask the user what those are; in author mode, list +them yourself. A simplification that blocks the roadmap is not a +simplification. Prefer designs where the future feature "falls out for +free" from the same mechanism. Typing rule: narrow types can widen +later without breaking; wide types can never narrow back. When unsure, +ship narrow. + +## Phase 7 — Ripple effects + +If the proposal changes the design, follow it through docs, migration +guides, and tests. Ask: does the docs structure still match the new +center of gravity? (A smaller surface usually means less +documentation to maintain, not just less code.) + +## Phase 8 — Deliver + +**Reviewer mode** — a proposal comment on the PR. Draft in the +scratchpad and keep the file; the comment will be updated several +times via +`gh api repos///issues/comments/ -X PATCH -F body=@file`. + +Structure: + +1. Lead with the question ("Do we need to export all of this?") — no + praise preamble, no throat-clearing. +2. The proposal, numbered, with a short code sample showing the + user-facing result. +3. Implementation consequences (what machinery gets deleted). +4. Where this goes later (roadmap fit). +5. An honest "What we give up" close — every proposal costs something; + name it yourself before a reviewer does. + +Show the draft to the user before posting; post only on approval. As +discussion continues, PATCH the same comment rather than posting new +ones — one coherent proposal, not a thread of fragments. Keep new +sections in reading order (insert before the trade-offs close, not +appended after it). + +**Author mode** — apply what you agree with now, while it's cheap. +What you decide *not* to change goes into the PR description with its +reasoning: state the cliff and its escape hatch, the machinery +trade-off, and the "what we give up" yourself. A PR that answers the +design questions before they're asked reviews faster. + +**Audit mode** — a report (markdown file or artifact) with the same +structure as the reviewer comment, but with proposals ranked by +value-for-effort, since nothing is gated on a merge. + +## Phase 9 — Record + +Save a memory note: the target, the proposal summary, links (comment +URL or report path), and status (awaiting author / accepted / +rejected / applied). Update it as things evolve. From c3467190227707a24be957517a4bee88b0fef6f5 Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Fri, 25 Sep 2026 12:55:06 +0530 Subject: [PATCH 2/2] chore: mark design-review skill internal and opt-in Move the skill to .agents/skills (exposed through the .claude/skills symlink), add internal metadata so it stays out of skills.sh, and disable implicit invocation for Claude Code and Codex. List it in AGENTS.md. --- .agents/skills/design-review/SKILL.md | 7 ++++++- .agents/skills/design-review/agents/openai.yaml | 6 ++++++ AGENTS.md | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 .agents/skills/design-review/agents/openai.yaml diff --git a/.agents/skills/design-review/SKILL.md b/.agents/skills/design-review/SKILL.md index b451ad04d..3d021efc0 100644 --- a/.agents/skills/design-review/SKILL.md +++ b/.agents/skills/design-review/SKILL.md @@ -1,6 +1,11 @@ --- name: design-review -description: Review the design of a change or a codebase — architecture, API surface, and whether the machinery fits the problem — rather than code quality. Use to analyse a PR's design, self-review your own branch before opening a PR, or audit an existing module or package. For bug-hunting line review use /review or /code-review instead. +description: 'Review the design of a change or a codebase — architecture, API surface, and whether the machinery fits the problem — rather than code quality. Analyses a PR''s design, self-reviews your own branch before opening a PR, or audits an existing module or package. Use ONLY when the user asks for it by name: /design-review or $design-review. For bug-hunting line review use /apsara-review instead.' +disable-model-invocation: true +metadata: + author: raystack + version: "1.0" + internal: true --- # Design Review diff --git a/.agents/skills/design-review/agents/openai.yaml b/.agents/skills/design-review/agents/openai.yaml new file mode 100644 index 000000000..2d5f8d6a4 --- /dev/null +++ b/.agents/skills/design-review/agents/openai.yaml @@ -0,0 +1,6 @@ +interface: + display_name: 'Design Review' + short_description: 'Review a PR, branch, or module for API and design fit' + default_prompt: 'Use $design-review to review the design of my current branch.' +policy: + allow_implicit_invocation: false diff --git a/AGENTS.md b/AGENTS.md index d8e881fe7..84a7afb58 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,7 @@ Skills are in `.agents/skills/`. `.claude/skills` is a symlink to that folder, s - `add-new-component`: every step to add a component, from source to docs. - `apsara-review`: reviews a diff for bugs, tests, simplifications, and docs. Run it only when someone asks for it by name (`/apsara-review` or `$apsara-review`). Do not run it for a general review request or after you finish a change. - `apsara`: for apps that use the library. It is not for work in this repo. +- `design-review`: reviews the design of a PR, a branch, or an existing module: API surface, maintenance cost, and whether the machinery fits the problem. Run it only when someone asks for it by name (`/design-review` or `$design-review`). ## Code