## 概要
`resources/` ディレクトリを `builtins/` にリネームし、用途を明確化。同時に export-cc コマンドを拡張して全リソースをコピーするように修正する。
---
## タスク一覧
### 1. ディレクトリリネーム(優先度: 高)
| 変更前 | 変更後 |
|--------|--------|
| `resources/` | `builtins/` |
| `resources/global/{lang}/` | `builtins/{lang}/`(global/ 階層を除去) |
| `resources/project/` | `builtins/project/` |
| `resources/skill/` | `builtins/skill/` |
### 2. 不要ファイル削除(優先度: 高)
- `builtins/{lang}/prompts/` を削除
- 対象: `interactive-system.md`, `interactive-summary.md`
- 理由: コードから未参照、実体は `src/shared/prompts/`
### 3. コード修正 — パス参照(優先度: 高)
`resources` → `builtins`、`global/{lang}` → `{lang}` に更新:
| ファイル | 修正内容 |
|----------|----------|
| `src/infra/resources/index.ts` | `getResourcesDir()`, `getGlobalResourcesDir()`, `getLanguageResourcesDir()` 等のパス |
| `src/infra/config/paths.ts` | `getBuiltinPiecesDir()`, `getBuiltinPersonasDir()` |
| `src/infra/config/global/initialization.ts` | `copyLanguageConfigYaml()` |
| `src/infra/config/loaders/pieceCategories.ts` | `getLanguageResourcesDir()` 参照 |
| `src/features/config/ejectBuiltin.ts` | `getLanguageResourcesDir()` 参照 |
| `src/features/config/deploySkill.ts` | `getResourcesDir()` 参照 |
### 4. export-cc 修正(優先度: 高)
ファイル: `src/features/config/deploySkill.ts`
**現状**: pieces/ と personas/ のみコピー
**修正後**:
- `builtins/{lang}/` 全体を `~/.claude/skills/takt/` にコピー
- `skill/` のファイル(SKILL.md, references/, takt-command.md)は従来通り
- サマリー表示を新リソースタイプ(stances, instructions, knowledge 等)に対応
- confirm メッセージ修正:
- 現状: `'上書きしますか?'`
- 修正後: `'既存のスキルファイルをすべて削除し、最新版に置き換えます。続行しますか?'`
### 5. テスト修正(優先度: 中)
| ファイル | 修正内容 |
|----------|----------|
| `src/__tests__/initialization.test.ts` | `getLanguageResourcesDir` のパス期待値 |
| `src/__tests__/piece-category-config.test.ts` | mock パス |
| その他 `resources` パスを参照しているテスト | パス更新 |
### 6. ビルド・パッケージ設定(優先度: 中)
| ファイル | 修正内容 |
|----------|----------|
| `package.json` | `files` フィールドで `resources/` → `builtins/` |
| `tsconfig.json` | `resources/` への参照があれば更新 |
| `.gitignore` | 必要に応じて更新 |
### 7. ドキュメント(優先度: 低)
- `CLAUDE.md` の Directory Structure セクションを更新
- JSDoc コメントから `prompts/` 記述を削除
---
## 制約
- `builtins/{lang}/` のフラット構造は変更不可(ピースYAML内の相対パス依存)
- eject のセーフティ(skip-if-exists)は変更不要
- export-cc のセーフティ(SKILL.md 存在チェック + confirm)は維持
---
## 確認方法
- `npm run build` が成功すること
- `npm test` が全てパスすること
- `takt init` / `takt eject` / `takt export-cc` が正常動作すること
14 KiB
Architecture Knowledge
Structure & Design
File Organization:
| Criteria | Judgment |
|---|---|
| Single file > 200 lines | Consider splitting |
| Single file > 300 lines | REJECT |
| Single file with multiple responsibilities | REJECT |
| Unrelated code coexisting | REJECT |
Module Structure:
- High cohesion: Related functionality grouped together
- Low coupling: Minimal inter-module dependencies
- No circular dependencies
- Appropriate directory hierarchy
Function Design:
- One responsibility per function
- Consider splitting functions over 30 lines
- Side effects clearly defined
Layer Design:
- Dependency direction: Upper layers -> Lower layers (reverse prohibited)
- Controller -> Service -> Repository flow maintained
- 1 interface = 1 responsibility (no giant Service classes)
Directory Structure:
Structure pattern selection:
| Pattern | Use Case | Example |
|---|---|---|
| Layered | Small scale, CRUD-centric | controllers/, services/, repositories/ |
| Vertical Slice | Medium-large scale, high feature independence | features/auth/, features/order/ |
| Hybrid | Common foundation + feature modules | core/ + features/ |
Vertical Slice Architecture (organizing code by feature):
src/
├── features/
│ ├── auth/
│ │ ├── LoginCommand.ts
│ │ ├── LoginHandler.ts
│ │ ├── AuthRepository.ts
│ │ └── auth.test.ts
│ └── order/
│ ├── CreateOrderCommand.ts
│ ├── CreateOrderHandler.ts
│ └── ...
└── shared/ # Shared across features
├── database/
└── middleware/
Vertical Slice criteria:
| Criteria | Judgment |
|---|---|
| Single feature spans 3+ layers | Consider slicing |
| Minimal inter-feature dependencies | Recommend slicing |
| Over 50% shared processing | Keep layered |
| Team organized by features | Slicing required |
Prohibited patterns:
| Pattern | Problem |
|---|---|
Bloated utils/ |
Becomes graveyard of unclear responsibilities |
Lazy placement in common/ |
Dependencies become unclear |
| Excessive nesting (4+ levels) | Navigation difficulty |
| Mixed features and layers | features/services/ prohibited |
Separation of Concerns:
- Read and write responsibilities separated
- Data fetching at root (View/Controller), passed to children
- Error handling centralized (no try-catch scattered everywhere)
- Business logic not leaking into Controller/View
Code Quality Detection
Explanatory Comment (What/How) Detection Criteria:
Detect comments that simply restate code behavior in natural language.
| Judgment | Criteria |
|---|---|
| REJECT | Restates code behavior in natural language |
| REJECT | Repeats what is already obvious from function/variable names |
| REJECT | JSDoc that only paraphrases the function name without adding information |
| OK | Explains why a particular implementation was chosen |
| OK | Explains the reason behind seemingly unusual behavior |
| Best | No comment needed — the code itself communicates intent |
// REJECT - Restates code (What)
// If interrupted, abort immediately
if (status === 'interrupted') {
return ABORT_STEP;
}
// REJECT - Restates the loop
// Check transitions in order
for (const transition of step.transitions) {
// REJECT - Repeats the function name
/** Check if status matches transition condition. */
export function matchesCondition(status: Status, condition: TransitionCondition): boolean {
// OK - Design decision (Why)
// User interruption takes priority over piece-defined transitions
if (status === 'interrupted') {
return ABORT_STEP;
}
// OK - Reason behind seemingly odd behavior
// stay can cause loops, but is only used when explicitly specified by the user
return step.name;
Direct State Mutation Detection Criteria:
Detect direct mutation of arrays or objects.
// REJECT - Direct array mutation
const steps: Step[] = getSteps();
steps.push(newStep); // Mutates original array
steps.splice(index, 1); // Mutates original array
steps[0].status = 'done'; // Nested object also mutated directly
// OK - Immutable operations
const withNew = [...steps, newStep];
const without = steps.filter((_, i) => i !== index);
const updated = steps.map((s, i) =>
i === 0 ? { ...s, status: 'done' } : s
);
// REJECT - Direct object mutation
function updateConfig(config: Config) {
config.logLevel = 'debug'; // Mutates argument directly
config.steps.push(newStep); // Nested mutation too
return config;
}
// OK - Returns new object
function updateConfig(config: Config): Config {
return {
...config,
logLevel: 'debug',
steps: [...config.steps, newStep],
};
}
Security (Basic Checks)
- Injection prevention (SQL, Command, XSS)
- User input validation
- Hardcoded sensitive information
Testability
- Dependency injection enabled
- Mockable design
- Tests are written
Anti-Pattern Detection
REJECT when these patterns are found:
| Anti-Pattern | Problem |
|---|---|
| God Class/Component | Single class with too many responsibilities |
| Feature Envy | Frequently accessing other modules' data |
| Shotgun Surgery | Single change ripples across multiple files |
| Over-generalization | Variants and extension points not currently needed |
| Hidden Dependencies | Child components implicitly calling APIs etc. |
| Non-idiomatic | Custom implementation ignoring language/FW conventions |
Abstraction Level Evaluation
Conditional Branch Proliferation Detection:
| Pattern | Judgment |
|---|---|
| Same if-else pattern in 3+ places | Abstract with polymorphism → REJECT |
| switch/case with 5+ branches | Consider Strategy/Map pattern |
| Flag arguments changing behavior | Split into separate functions → REJECT |
| Type-based branching (instanceof/typeof) | Replace with polymorphism → REJECT |
| Nested conditionals (3+ levels) | Early return or extract → REJECT |
Abstraction Level Mismatch Detection:
| Pattern | Problem | Fix |
|---|---|---|
| Low-level details in high-level processing | Hard to read | Extract details to functions |
| Mixed abstraction levels in one function | Cognitive load | Align to same granularity |
| DB operations mixed with business logic | Responsibility violation | Separate to Repository layer |
| Config values mixed with processing logic | Hard to change | Externalize configuration |
Good Abstraction Examples:
// Proliferating conditionals
function process(type: string) {
if (type === 'A') { /* process A */ }
else if (type === 'B') { /* process B */ }
else if (type === 'C') { /* process C */ }
// ...continues
}
// Abstract with Map pattern
const processors: Record<string, () => void> = {
A: processA,
B: processB,
C: processC,
};
function process(type: string) {
processors[type]?.();
}
// Mixed abstraction levels
function createUser(data: UserData) {
// High level: business logic
validateUser(data);
// Low level: DB operation details
const conn = await pool.getConnection();
await conn.query('INSERT INTO users...');
conn.release();
}
// Aligned abstraction levels
function createUser(data: UserData) {
validateUser(data);
await userRepository.save(data); // Details hidden
}
Workaround Detection
Don't overlook compromises made to "just make it work."
| Pattern | Example |
|---|---|
| Unnecessary package additions | Mystery libraries added just to make things work |
| Test deletion/skipping | @Disabled, .skip(), commented out |
| Empty implementations/stubs | return null, // TODO: implement, pass |
| Mock data in production | Hardcoded dummy data |
| Swallowed errors | Empty catch {}, rescue nil |
| Magic numbers | Unexplained if (status == 3) |
Strict TODO Comment Prohibition
"We'll do it later" never gets done. What's not done now is never done.
TODO comments are immediate REJECT.
// REJECT - Future-looking TODO
// TODO: Add authorization check by facility ID
fun deleteCustomHoliday(@PathVariable id: String) {
deleteCustomHolidayInputPort.execute(input)
}
// APPROVE - Implement now
fun deleteCustomHoliday(@PathVariable id: String) {
val currentUserFacilityId = getCurrentUserFacilityId()
val holiday = findHolidayById(id)
require(holiday.facilityId == currentUserFacilityId) {
"Cannot delete holiday from another facility"
}
deleteCustomHolidayInputPort.execute(input)
}
Only acceptable TODO cases:
| Condition | Example | Judgment |
|---|---|---|
| External dependency prevents implementation + Issued | // TODO(#123): Implement after API key obtained |
Acceptable |
| Technical constraint prevents + Issued | // TODO(#456): Waiting for library bug fix |
Acceptable |
| "Future implementation", "add later" | // TODO: Add validation |
REJECT |
| "No time for now" | // TODO: Refactor |
REJECT |
Correct handling:
- Needed now → Implement now
- Not needed now → Delete the code
- External blocker → Create issue and include ticket number in comment
DRY Violation Detection
Detect duplicate code.
| Pattern | Judgment |
|---|---|
| Same logic in 3+ places | Immediate REJECT - Extract to function/method |
| Same validation in 2+ places | Immediate REJECT - Extract to validator function |
| Similar components 3+ | Immediate REJECT - Create shared component |
| Copy-paste derived code | Immediate REJECT - Parameterize or abstract |
AHA principle (Avoid Hasty Abstractions) balance:
- 2 duplications → Wait and see
- 3 duplications → Extract immediately
- Different domain duplications → Don't abstract (e.g., customer validation vs admin validation are different)
Spec Compliance Verification
Verify that changes comply with the project's documented specifications.
Verification targets:
| Target | What to Check |
|---|---|
| CLAUDE.md / README.md | Conforms to schema definitions, design principles, constraints |
| Type definitions / Zod schemas | New fields reflected in schemas |
| YAML/JSON config files | Follows documented format |
Specific checks:
-
When config files (YAML, etc.) are modified or added:
- Cross-reference with schema definitions in CLAUDE.md, etc.
- No ignored or invalid fields present
- No required fields missing
-
When type definitions or interfaces are modified:
- Documentation schema descriptions are updated
- Existing config files are compatible with new schema
REJECT when these patterns are found:
| Pattern | Problem |
|---|---|
| Fields not in the spec | Ignored or unexpected behavior |
| Invalid values per spec | Runtime error or silently ignored |
| Violation of documented constraints | Against design intent |
Call Chain Verification
When new parameters/fields are added, verify not just the changed file but also callers.
Verification steps:
- When finding new optional parameters or interface fields,
Grepall callers - Check if all callers pass the new parameter
- If fallback value (
?? default) exists, verify if fallback is used as intended
Danger patterns:
| Pattern | Problem | Detection |
|---|---|---|
options.xxx ?? fallback where all callers omit xxx |
Feature implemented but always falls back | grep callers |
| Tests set values directly with mocks | Don't go through actual call chain | Check test construction |
executeXxx() doesn't receive options it uses internally |
No route to pass value from above | Check function signature |
// Missing wiring: No route to receive projectCwd
export async function executePiece(config, cwd, task) {
const engine = new PieceEngine(config, cwd, task); // No options
}
// Wired: Can pass projectCwd
export async function executePiece(config, cwd, task, options?) {
const engine = new PieceEngine(config, cwd, task, options);
}
Logically dead code due to caller constraints:
Call chain verification applies not only to "missing wiring" but also to the reverse — unnecessary guards for conditions that callers already guarantee.
| Pattern | Problem | Detection |
|---|---|---|
| TTY check when all callers require TTY | Unreachable branch remains | grep all callers' preconditions |
| Null guard when callers already check null | Redundant defense | Trace caller constraints |
| Runtime type check when TypeScript types constrain | Not trusting type safety | Check TypeScript type constraints |
Verification steps:
- When finding defensive branches (TTY check, null guard, etc.), grep all callers
- If all callers already guarantee the condition, guard is unnecessary → REJECT
- If some callers don't guarantee it, keep the guard
Quality Attributes
| Attribute | Review Point |
|---|---|
| Scalability | Design handles increased load |
| Maintainability | Easy to modify and fix |
| Observability | Logging and monitoring enabled |
Big Picture
Don't get lost in minor "clean code" nitpicks.
Verify:
- How will this code evolve in the future
- Is scaling considered
- Is technical debt being created
- Does it align with business requirements
- Is naming consistent with the domain
Change Scope Assessment
Check change scope and include in report (non-blocking).
| Scope Size | Lines Changed | Action |
|---|---|---|
| Small | ~200 lines | Review as-is |
| Medium | 200-500 lines | Review as-is |
| Large | 500+ lines | Continue review. Suggest splitting if possible |
Note: Some tasks require large changes. Don't REJECT based on line count alone.
Verify:
- Changes are logically cohesive (no unrelated changes mixed in)
- Coder's scope declaration matches actual changes
Include as suggestions (non-blocking):
- If splittable, present splitting proposal