Skip to content

Commit 7ffdd9c

Browse files
IzumiSyIzumiSy
authored
chore: add local api-design-review skill (#339)
* chore: add api design review skill * fix: quote api design review skill description * fix: avoid duplicated gh aw api review imports
1 parent 1297b13 commit 7ffdd9c

5 files changed

Lines changed: 87 additions & 57 deletions

File tree

‎.agents/skills/api-design-review/SKILL.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
name: api-design-review
3+
description: "Review local packages/ changes for API design consistency, public surface risks, and TypeScript/React footguns in the AppShell core package. Use when: reviewing local packages/ changes before push or validating public API changes."
4+
---
5+
6+
# API Design Review
7+
8+
Review local code changes in `packages/` for API consistency and potential present/future footguns.
9+
10+
Read the shared review rubric first: [instruction.md](instruction.md)
11+
12+
## Local Scope
13+
14+
Focus only on changed files matching `packages/**/*.ts`, `packages/**/*.tsx`, and `packages/**/package.json`.
15+
16+
## Before Reviewing
17+
18+
1. Get the local diff against the base branch:
19+
- Prefer `git diff main -- 'packages/**'`
20+
- If that is empty or `main` is unavailable, fall back to `git diff HEAD~1 -- 'packages/**'`
21+
2. Build the public API map from `packages/core/src/index.ts`.
22+
3. Skip internal files entirely — files not exported from `packages/core/src/index.ts`.
23+
4. For each changed public symbol, trace usages across the codebase with the available search tools.
24+
5. Prioritize symbols with many usages or likely breaking changes.
25+
6. Follow the shared review rubric and output format in [instruction.md](instruction.md).
26+
27+
If no matching files changed, state that there are no API-relevant changes to review.

‎.agents/skills/api-design-review/instruction.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
description: Shared API design review rubric for GitHub Agentic Workflows and pi skills.
3+
---
4+
5+
## Review Scope
6+
7+
Only review code that was **changed in this PR or local diff**. Do not comment on unchanged code.
8+
9+
### Breaking Change Baseline
10+
11+
When evaluating whether a change is a "breaking change," **always compare against the base branch (e.g., `main`)**, not against earlier commits within the same PR or branch. If a component, hook, type, or function was **introduced in the current branch** and does not exist on the base branch, modifications to its API within the same branch are NOT breaking changes — they are simply iterating on unreleased code.
12+
13+
### Review Depth
14+
15+
- Use export/usage impact analysis as the basis for tracing affected code paths. Do not redundantly search for usages that you have already identified.
16+
- Evaluate proposed fixes holistically: When suggesting a fix, also analyze what new edge cases or failure modes that fix would introduce.
17+
- Verify documentation-implementation consistency: When a change modifies type signatures (for example, making a field optional), check that JSDoc comments and default behaviors reflect the same semantics.
18+
- Assess test quality, not just coverage: Check that tests verify **runtime behavior**, not just data structure.
19+
20+
### What to Flag
21+
22+
Report **only issues that are genuinely impactful**. Aim for quality over volume. Report up to **10 issues** maximum, sorted by severity (High → Medium → Low). Use this format:
23+
24+
```
25+
[N/total — Severity] Brief title
26+
```
27+
28+
#### Severity Levels
29+
30+
| Severity | Description |
31+
| ---------- | ----------------------------------------------------------------------------------------------------- |
32+
| **High** | Breaking API changes, API inconsistency, critical runtime errors, significant performance degradation |
33+
| **Medium** | Memory leaks, insufficient error handling, behavior not matching expectations |
34+
| **Low** | Insufficient tests, missing documentation, unnecessary complexity, JSDoc-implementation mismatch |
35+
36+
### What NOT to Flag (Out of Scope)
37+
38+
- **Internal (non-exported) components** — Do not flag them as breaking API changes or public API concerns.
39+
- **Detectable by linter/type-check** — Code style, hooks rule violations, explicit `any`, import ordering.
40+
- **Subjective** — Naming preferences, minor refactoring suggestions without clear justification.
41+
42+
## API Design Principles
43+
44+
Follow the API Design Principles defined in the [Add Component Skill](/.agents/skills/add-component/SKILL.md). Key rules:
45+
46+
- Minimal public API surface — only export the component and its primary props type.
47+
- Leverage TypeScript type inference over exporting internal types.
48+
- Follow the pattern: `export { ComponentName, type ComponentNameProps }`.

‎.github/agents/api-design-reviewer.md‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,48 +20,3 @@ Use the Impact Analyzer results to:
2020

2121
- **Skip internal files entirely** — do not review files marked as `internal`.
2222
- **Focus review on risk areas** — prioritize symbols with high usage count or flagged risks.
23-
24-
## Review Scope
25-
26-
Only review code that was **changed in this PR**. Do not comment on unchanged code.
27-
28-
### Breaking Change Baseline
29-
30-
When evaluating whether a change is a "breaking change," **always compare against the base branch (e.g., `main`)**, not against earlier commits within the same PR. If a component, hook, type, or function was **introduced in this PR** and does not exist on the base branch, modifications to its API within the same PR are NOT breaking changes — they are simply iterating on unreleased code.
31-
32-
### Review Depth
33-
34-
- **Use Impact Analyzer results** as the basis for tracing affected code paths. Do not redundantly search for usages that the Impact Analyzer has already reported.
35-
- **Evaluate proposed fixes holistically**: When suggesting a fix, also analyze what new edge cases or failure modes that fix would introduce.
36-
- **Verify documentation-implementation consistency**: When a PR changes type signatures (e.g., making a field optional), check that JSDoc comments and default behaviors reflect the same semantics.
37-
- **Assess test quality, not just coverage**: Check that tests verify **runtime behavior**, not just data structure.
38-
39-
### What to Flag
40-
41-
Report **only issues that are genuinely impactful**. Aim for quality over volume. Report up to **10 issues** maximum, sorted by severity (High → Medium → Low). Use this format:
42-
43-
```
44-
[N/total — Severity] Brief title
45-
```
46-
47-
#### Severity Levels
48-
49-
| Severity | Description |
50-
| ---------- | ----------------------------------------------------------------------------------------------------- |
51-
| **High** | Breaking API changes, API inconsistency, critical runtime errors, significant performance degradation |
52-
| **Medium** | Memory leaks, insufficient error handling, behavior not matching expectations |
53-
| **Low** | Insufficient tests, missing documentation, unnecessary complexity, JSDoc-implementation mismatch |
54-
55-
### What NOT to Flag (Out of Scope)
56-
57-
- **Internal (non-exported) components** — The Impact Analyzer identifies these. Do not flag them as breaking API changes or public API concerns.
58-
- **Detectable by linter/type-check** — Code style, hooks rule violations, explicit `any`, import ordering.
59-
- **Subjective** — Naming preferences, minor refactoring suggestions without clear justification.
60-
61-
## API Design Principles
62-
63-
Follow the API Design Principles defined in the [Add Component Skill](/.agents/skills/add-component/SKILL.md). Key rules:
64-
65-
- Minimal public API surface — only export the component and its primary props type.
66-
- Leverage TypeScript type inference over exporting internal types.
67-
- Follow the pattern: `export { ComponentName, type ComponentNameProps }`.

‎.github/workflows/api-design-review.lock.yml‎

Lines changed: 11 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/api-design-review.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ on:
88
events: [pull_request_comment]
99
imports:
1010
- ../agents/api-design-reviewer.md
11+
- /.agents/skills/api-design-review/instruction.md
1112
permissions:
1213
contents: read
1314
pull-requests: read
@@ -24,8 +25,6 @@ safe-outputs:
2425

2526
# API Design Review
2627

27-
{{#runtime-import .github/agents/api-design-reviewer.md}}
28-
2928
## Review Round Awareness
3029

3130
This review may be one of multiple rounds on the same PR. Before starting your review:

0 commit comments

Comments
 (0)