diff --git a/builtins/en/facets/personas/architecture-reviewer.md b/builtins/en/facets/personas/architecture-reviewer.md index 83fef9e..732e71c 100644 --- a/builtins/en/facets/personas/architecture-reviewer.md +++ b/builtins/en/facets/personas/architecture-reviewer.md @@ -17,6 +17,7 @@ Code is read far more often than it is written. Poorly structured code destroys - No "conditional approval". If there are issues, reject - If you find in-scope fixable issues, flag them without exception - Existing issues (unrelated to current change) are non-blocking, but issues introduced or fixable in this change must be flagged +- Do not overlook branches that operate below a function's responsibility level ## Areas of Expertise diff --git a/builtins/en/facets/policies/coding.md b/builtins/en/facets/policies/coding.md index 188fdac..630352d 100644 --- a/builtins/en/facets/policies/coding.md +++ b/builtins/en/facets/policies/coding.md @@ -115,6 +115,36 @@ function processOrder(order) { } ``` +In orchestration functions (Step 1 → Step 2 → Step 3), pay special attention. If an individual step's internals expand with conditional branches, extract that step into a function. The criterion is not the number of branches, but **whether the branch belongs at the function's abstraction level**. + +```typescript +// ❌ Low-level branching exposed in orchestration function +async function executePipeline(options) { + const task = resolveTask(options); // Step 1: high level ✅ + + // Step 2: low-level details exposed ❌ + let execCwd = cwd; + if (options.createWorktree) { + const result = await confirmAndCreateWorktree(cwd, task, true); + execCwd = result.execCwd; + branch = result.branch; + } else if (!options.skipGit) { + baseBranch = getCurrentBranch(cwd); + branch = generateBranchName(config, options.issueNumber); + createBranch(cwd, branch); + } + + await executeTask({ cwd: execCwd, ... }); // Step 3: high level ✅ +} + +// ✅ Extract details, keep abstraction levels consistent +async function executePipeline(options) { + const task = resolveTask(options); + const ctx = await resolveExecutionContext(options); + await executeTask({ cwd: ctx.execCwd, ... }); +} +``` + ### Follow Language and Framework Conventions - Write Pythonic Python, idiomatic Kotlin, etc. diff --git a/builtins/ja/facets/personas/architecture-reviewer.md b/builtins/ja/facets/personas/architecture-reviewer.md index d02e534..2d4e853 100644 --- a/builtins/ja/facets/personas/architecture-reviewer.md +++ b/builtins/ja/facets/personas/architecture-reviewer.md @@ -24,3 +24,4 @@ - 軽微な問題でも後に持ち越さない。今修正できる問題は今修正させる - 「条件付き承認」はしない。問題があれば差し戻す - 既存コードの踏襲を理由にした問題の放置は認めない +- 関数の責務より低い粒度の分岐が混入していたら見逃さない diff --git a/builtins/ja/facets/policies/coding.md b/builtins/ja/facets/policies/coding.md index eabef2f..2e08de6 100644 --- a/builtins/ja/facets/policies/coding.md +++ b/builtins/ja/facets/policies/coding.md @@ -115,6 +115,36 @@ function processOrder(order) { } ``` +オーケストレーション関数(Step 1 → Step 2 → Step 3 と処理を並べる関数)では特に注意する。あるStepの内部に条件分岐が膨らんでいたら、そのStepを関数に抽出する。判定基準は分岐の数ではなく、**その分岐がその関数の抽象レベルに合っているか**。 + +```typescript +// ❌ オーケストレーション関数に詳細な分岐が露出 +async function executePipeline(options) { + const task = resolveTask(options); // Step 1: 高レベル ✅ + + // Step 2: 低レベル詳細が露出 ❌ + let execCwd = cwd; + if (options.createWorktree) { + const result = await confirmAndCreateWorktree(cwd, task, true); + execCwd = result.execCwd; + branch = result.branch; + } else if (!options.skipGit) { + baseBranch = getCurrentBranch(cwd); + branch = generateBranchName(config, options.issueNumber); + createBranch(cwd, branch); + } + + await executeTask({ cwd: execCwd, ... }); // Step 3: 高レベル ✅ +} + +// ✅ 詳細を関数に抽出し、抽象度を揃える +async function executePipeline(options) { + const task = resolveTask(options); + const ctx = await resolveExecutionContext(options); + await executeTask({ cwd: ctx.execCwd, ... }); +} +``` + ### 言語・フレームワークの作法に従う - Pythonなら Pythonic に、KotlinならKotlinらしく