branch-code-reviewer

ブランチコードレビュー

dxs CLI を使用して、Datex Studio のブランチを AI 支援でコードレビューします。ブランチの変更を読み取り、影響を受ける設定の依存関係を追跡し、統合差分を読み取り、明示的な重大度タグと判定を含む構造化されたレビュー概要を作成します。

参考資料

• ../datex-studio-shared/branch-setup.md — ブランチ ID が指定されていない場合のブランチ選択

依存関係

• impact-analysis スキル — 変更された設定が delete の場合、ステップ 4（評価項目 F）で呼び出されます。dxs source explore reverse-trace を実行して呼び出し元を分類します。削除が安全かどうかを判断するための、正式な手段です。前方トレースを手作業で行って代用しないでください。前方トレースでは「削除対象の名前をまだ参照しているものがあるか？」という問いには答えられません（ステップ 2 の制限事項を参照）。

前提条件

• Datex Studio の機能ブランチのブランチ ID。

ワークフロー

[Step 1: Gather Context]
Parallel:
  dxs source changes --branch <ID>
  dxs source workitems --branch <ID> --description --comments
        |
[Step 2: Trace Dependencies of Changed Configs]
For each significantly-changed or deleted config:
  dxs source explore trace <config_name> --branch <ID>
        |
[Step 3: Review Diffs]
dxs source changes --branch <ID> --with-diffs
        |
[Step 4: Analyze Each Change]
For each changed config, evaluate against six dimensions:
  A. Bugs
  B. Code Quality
  C. Security
  D. Performance
  E. Simplification Opportunities
  F. Alignment & Risk
        |
[Step 5: Deeper Dive (as needed)]
If diffs are ambiguous:
  dxs source explore config <config_name> --branch <ID>
        |
[Step 6: Produce Review Summary]
Branch Overview → Changes Table → Detailed Findings (severity-tagged) →
Questions for Developer → Verdict

ステップの詳細

ステップ 1：コンテキストを収集する

次の 2 つのコマンドを並行して実行します。これらは互いに独立しています。

dxs source changes --branch <ID>
dxs source workitems --branch <ID> --description --comments

1 つ目のコマンドでは、変更された設定の概要をすばやく確認できます。2 つ目のコマンドでは、変更の意図（バグ／機能）、担当者、スプリントの背景、および（--comments によって）レビュー担当者の変更内容の読み取り方に影響しそうなスレッド内の議論を確認できます。

ステップ 2：変更された設定の依存関係を追跡する

dxs source explore trace <config_name> --branch <ID>

トレースでは、設定の前方依存関係、つまりその設定が参照するもの（データソース、フロー、グリッド、ダイアログ、バックエンドフロー）と、それぞれの参照元ライブラリが示されます。

トレースするタイミング：

• 削除された設定 — 何に依存していたかを確認するためにトレースします。その後、ほかの設定のトレースと照らし合わせ、削除された設定が名前でまだ参照されていないか確認します。
• 大幅に変更された設定 — 差分を読む前に、構造（このエディターが呼び出すフローはどれか、このグリッドが使用するデータソースはどれか）を把握するためにトレースします。依存関係の範囲を把握してから差分を読むほうが、はるかに効率的です。
• ライブラリをまたぐ参照 — トレースを見ると、設定が共有プラットフォームのフロー（例：Utilities の crud_create_flow）や、ほかのモジュールのダイアログに依存していることがわかります。こうした境界部分は、問題が最も発生しやすい箇所です。

変更された設定をトレースする際は、この同じブランチで削除される設定への参照に注意してください。変更された設定のトレースに、削除済みのデータソースやフローがまだ含まれている場合、クリーンアップが不完全であることを示しています。削除の安全性自体はステップ 4 で impact-analysis が扱いますが、この場合も指摘してください。

制限事項： トレースでわかるのは前方依存関係だけで、逆方向の依存関係は確認できません。削除する設定が安全かどうかを確認するには、その設定を参照している可能性のある設定をトレースし、依存関係の一覧に削除対象の名前がないか調べます。

ステップ 3：差分をレビューする

dxs source changes --branch <ID> --with-diffs

変更されたすべての設定の完全な統合差分です。これはステップ 4 の主な入力情報です。

ステップ 4：各変更を分析する

変更された設定ごとに、次の 6 つの評価項目すべてに照らして評価します。すべての評価項目がすべての設定に当てはまるわけではありません。指摘事項がない場合はその評価項目を省略できますが、省略する前に必ず検討してください。

A. バグ

• ロジックエラー、境界値の誤り、条件の逆転
• 処理されていないエッジケース（null のエンティティ、空のコレクション）
• エラー処理の不足（catch ブロックの欠如、フロー結果の未確認）
• 新たに導入されたデッドコードや到達不能な経路

B. コード品質

• コードを読めば変更の意図が明確にわかりますか？
• 周辺のコードで使われているパターンに合っていますか？
• 不要な重複はありませんか？
• 変数名は意味がわかるものですか？
• プラットフォーム API（Datex Studio ヘルパー、$utils、$flows）を適切に使用していますか？

C. セキュリティ

• 式やテンプレート内の、サニタイズされていないユーザー入力
• 動的なフィルター式におけるインジェクションのリスク
• パラメーター、ログ、返却ペイロードを通じた機密データの漏えい
• 新たに公開された操作における認可チェックの欠如

D. パフォーマンス

• 不要な API 呼び出しや重複したデータ取得
• データソースにおける N+1 クエリのパターン
• プロジェクション（$select／$select を指定した $expand）で済む場合に、ペイロードが過大になっていないか
• 非効率なループや DOM の繰り返し操作

E. 簡素化の機会

• より明確にできる過度に複雑な式
• 冗長な null チェックやオプショナルチェーン
• 共有できる設定間の重複ロジック
• 未使用のパラメーターやデッドな設定プロパティ

F. 整合性とリスク

• 作業項目との整合性 — 変更は、記載されたバグや機能に実際に対処していますか？

• 修正の適切な階層 — 根本原因を修正していますか、それとも不適切な階層で回避策を講じていますか？UI レベルでの再マッピング（たとえば、バインディング内で不正な TypeId を書き換えること）は、エンティティが読み取られる他のすべての箇所で再発するデータレイヤーのバグを覆い隠す可能性があります。どちらが意図されたものか判断できない場合は、重大度を推測するのではなく、「開発者への質問」で指摘してください。

• スコープクリープ — 無関係な変更が含まれていませんか？

• リグレッションのリスク — 既存の動作を壊す可能性はありますか？

• 削除された設定の安全性 — 削除された設定は、他の場所で本当に使われていませんか？変更一覧で modification が delete になっている設定については、その参照名とブランチ ID を指定して impact-analysis スキルを実行してください。逆トレースを実行し、残っている呼び出し元の一覧を返します。結果をレビューに反映してください。

  • 呼び出し元なし → [OK] 削除しても安全です。
  • 呼び出し元があり、この同じブランチで更新されている → 参照用に、更新された呼び出し元の一覧とともに [INFO] を付けます。
  • 呼び出し元があり、このブランチで更新されていない → [ISSUE] — 削除すると、実行時にそれらの呼び出し元が動作しなくなります。

  フォワードトレースでこれを再現しようとしないでください。フォワードトレースでは「この削除された名前を今も参照しているものは何か？」という問いには答えられません（ステップ 2 の制限に関する注記を参照）。

ステップ 5：さらに詳しく確認する（必要に応じて）

差分だけで明確に判断するのに十分なコンテキストが得られるかを評価します。不十分な場合は、影響を受ける設定のより広い連続した範囲を読み、周辺のロジックを把握してください。

dxs source explore config <config_name> --branch <ID>

これは特に、1 行の変更が修正に見えても、周辺コードを見て初めて分かる副作用を引き起こす可能性がある場合に重要です。コンテキストのない差分では、「修正」によって別の問題が生じる事実が隠れてしまうことがあります。

ステップ 6：レビューの要約を作成する

以下の出力形式を正確に使用してください。重大度タグは、レビュー担当者が対応すべき内容を示す主要なシグナルです。

出力形式

# Review — Branch <ID>

## Branch Overview
- **Branch:** <ID> (<status>)
- **Work Item:** <type> <NNNNNN> — <title>
- **Assigned to:** <Author>
- **Sprint:** <sprint name>

## Changes Table

| Config | Type | Action | Summary |
|---|---|---|---|
| `<name>` | <type> | <add/update/delete> | <one-line summary> |
| …      | …    | …                 | …                     |

## Detailed Findings

### Bugs
**`[ISSUE]` <headline>** (`<config>`)
<explanation, with inline code where helpful>

**`[WARNING]` <headline>** (`<config>`)
<explanation>

### Code Quality
…

### Security Concerns
…

### Performance
…

### Simplification Opportunities
…

### Work Item Alignment
…

### Scope
…

## Questions for Developer
1. <question>
2. <question>

## Verdict
**<Approve | Request Changes | Needs Discussion>** — <one-line rationale>.

重大度タグ

タグ	意味
[ISSUE]	マージ前に必ず修正する必要があります
[WARNING]	修正すべきです。問題の可能性があります
[INFO]	所見です。対応は不要です
[OK]	レビュー済みで、懸念はありません

セキュリティや整合性など、重要な観点では [OK] を含める価値があります。これにより、レビュー担当者が実際に確認したことが読み手に伝わります。記載されていない観点は「確認を省略した」と受け取られ、これは別の意味を持ちます。

例（抜粋）

## Detailed Findings

### Bugs

**`[ISSUE]` Duplicate error check in save flow (`custom_field_editor`)**
The new create path checks `result.reason` twice:

    if (result.reason) { /* show error */ } else { /* save option */ }
    if (result.reason) { /* show error AGAIN */ } else { ... }

If `result.reason` is truthy, the error dialog shows twice. Looks like a
refactor artifact — the option-saving logic was inserted in the middle,
duplicating the error branch.

**`[WARNING]` `save_result` is unused (`custom_field_editor`)**
`const save_result = await $flows.Utilities.crud_create_flow({...});` — the
result is captured but never checked. If the create fails silently, the UDF
is created but its initial option value is lost with no error shown.

### Security Concerns
**`[OK]`** No new injection vectors. The datasource filter uses a template
literal with `$datasource.inParams.custom_field_id`, which is a typed number
param — low risk.

## Questions for Developer
1. The TypeId 5→1 remap is applied at the binding level — is this a display
   fix, or does the API return the wrong TypeId and the data layer needs the
   fix instead?
2. `save_result` from creating the initial option value is never checked —
   is silent failure acceptable here?

## Verdict
**Request Changes** — the duplicate error handling will show two dialogs on
failure. The unchecked `save_result` is a secondary concern worth addressing.

ヒント

1. 差分を見る前にトレースしましょう。 設定が何を参照しているかを把握していると、差分の読み方が変わります。explore trace に5分かければ、文脈を無視して差分を誤読するのに費やす1時間を節約できます。
2. 深刻度を適切に判断しましょう。 [ISSUE] は「必ず修正が必要」という意味です。本当のブロッカーにのみ使ってください。多用すると、レビュー担当者はこのタグを無視するようになります。
3. 差分の内容を言い直さないでください。 差分はすでに確認できます。コードを言い換えるのではなく、問題（または問題がない理由）を明確にするのが役割です。
4. 最も価値の高い観点は整合性です。 「変更内容は作業項目の要求どおりか」は最も重要な問いです。他のすべての観点をクリアしていても、誤ったものをリリースしてしまうことは簡単に起こります。
5. 削除した設定は参照を一通り調べましょう。 参照を1つ見落とすだけで、本番環境でランタイムエラーが発生することがあります。迷ったら、削除した名前を参照している可能性のある上位N件の設定をトレースしてください。
6. 空白のみの差分はノイズです。 [INFO] として指摘し、指摘事項のセクションを埋もれさせないでください。
7. 意図的な変更かどうか判断できないときは、質問しましょう。 差分だけでは分からない意図に左右される指摘もあります。未チェックのフロー結果は許容されるのでしょうか？ UI レベルのマッピング変更は意図的なものでしょうか、それとも暫定対応でしょうか？こうした内容は、深刻度を推測して指摘事項に含めるのではなく、開発者への質問に記載してください。誤った [ISSUE] は信頼を損ないますが、的確な質問なら回答を得られます。

よくある間違い

間違い	修正方法
影響を受ける設定をトレースせずに差分を読む	まずステップ2を実行します。内容より先に文脈を把握します
すべてに [ISSUE] を付ける	[ISSUE] は本当のブロッカーにのみ使い、それ以外には [WARNING]／[INFO] を使います
作業項目との整合性チェックを省略する	整合性は最も重要な観点です。省略しないでください
逆参照を確認せずに設定を削除する	削除した参照名に対して impact-analysis を実行します。forward-trace で代用しないでください
変更が微妙な場合に差分だけに頼る	ステップ5：dxs source explore config で設定全体を取得し、周辺のロジックを確認します
空白やフォーマットのノイズを指摘事項として列挙する	1回だけ [INFO] として記載する（または抑制する）ことで、レポートを水増ししないでください
確認できない意図に左右される内容に深刻度を割り当てる	推測せず、開発者への質問に移します