Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 166 additions & 0 deletions dev-reports/design/issue-165-has-progress-design-policy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
# 設計方針書: Issue #165 — progress-reportのrelationをhas_progressに変更

## 1. 概要

`KnowledgeRelation` enumに `HasProgress` バリアントを追加し、progress-report.md のrelationを `has_review` から `has_progress` に変更する。

## 2. レイヤー構成と影響範囲

| レイヤー | モジュール | 変更内容 | 影響度 |
|---------|-----------|---------|--------|
| **Indexer** | `src/indexer/knowledge.rs` | enum定義・PatternRule変更(コア変更) | 高 |
| **Indexer** | `src/indexer/symbol_store.rs` | DBパースmatch追加 | 高 |
| **CLI** | `src/cli/issue.rs` | sort_order exhaustive match追加 | 中 |
| **CLI** | `src/cli/before_change.rs` | relation_priority追加 | 中 |
| **Output** | `src/output/human.rs` | display labelフォールバック追加 | 低 |

## 3. 設計判断とトレードオフ

### 判断1: HasProgress を独立バリアントとして追加

- **選択肢A**: HasProgress バリアント追加(採用)
- **選択肢B**: HasReview のままdoc_subtypeで区別(現状維持)
- **判断理由**: relation フィールドだけで意味を判別可能にすることで、JSONコンシューマやDB直接クエリの正確性が向上する。doc_subtype に依存しない設計がクリーン。

### 判断2: sort_order での HasProgress の位置(issue.rs)

```rust
// issue.rs sort_order() — ドキュメント一覧の表示順序(開発フロー順)
KnowledgeRelation::HasDesign => 1,
KnowledgeRelation::HasReview => 2,
KnowledgeRelation::HasWorkplan => 3,
KnowledgeRelation::HasProgress => 4,
KnowledgeRelation::Modifies => 5,
```

- **判断理由**: `issue` コマンドの表示順は開発フロー順(設計→レビュー→作業計画→進捗→変更ファイル)。ユーザーが開発の流れを追えるよう時系列に沿った配置。

### 判断3: before_change.rs の relation_priority での位置

```rust
// before_change.rs relation_priority() — 変更前確認の重要度順(低い値 = 高優先度)
"has_design" => 0,
"has_workplan" => 1,
"has_review" => 2,
"has_progress" => 3,
"modifies" => 4,
```

- **判断理由**: `before-change` コマンドの優先度は「変更前に確認すべき重要度」順。設計→作業計画→レビュー→進捗の順で、進捗レポートは参考情報としてレビューより低い優先度。

> **注意**: issue.rs と before_change.rs で HasReview/HasWorkplan の相対順序が異なるのは意図的。
> - issue.rs: 開発フロー順(Review→Workplan)= ドキュメントの時系列表示
> - before_change.rs: 重要度順(Workplan→Review)= 変更前に参照すべき優先度

### 判断4: マイグレーション戦略

- 既存DBの `has_review` レコード(progress-report)は再インデックス(`ci index`)で自動的に `has_progress` に更新される
- `HasReview` バリアント自体は残るため、IssueReview/DesignReview/StageReview は影響なし
- 明示的なDBマイグレーションスクリプトは不要

## 4. 具体的な変更設計

### 4.1 src/indexer/knowledge.rs

```rust
// enum定義
pub enum KnowledgeRelation {
HasDesign,
HasReview,
HasWorkplan,
HasProgress, // 追加
Modifies,
}

// as_str()
Self::HasProgress => "has_progress",

// parse()
"has_progress" => Some(Self::HasProgress),

// build_pattern_rules() — progress-reportルール
relation: KnowledgeRelation::HasProgress, // HasReview → HasProgress
```

### 4.2 src/indexer/symbol_store.rs

```rust
// find_documents_by_issue() の relation パース — KnowledgeRelation::parse() に統一(DRY改善)
// Before: ハードコードmatch(has_design/has_review/has_workplan の3パターン)
// After: KnowledgeRelation::parse() を再利用し、None→エラー変換
let relation = crate::indexer::knowledge::KnowledgeRelation::parse(&relation_str)
.ok_or_else(|| SymbolStoreError::InvalidEmbedding {
reason: format!("Unknown relation type: {relation_str}"),
})?;
```

> **DRY改善**: find_knowledge_by_issue() (line 974) は既に parse() を使用。find_documents_by_issue() も統一することで、将来のバリアント追加時に symbol_store.rs の変更が不要になる。
>
> **振る舞い変更の注意**: parse() は `Modifies` も成功として返すが、現在のハードコード match は `modifies` をエラーとする。SQLクエリが `kn_doc.type = 'document'` でフィルタしているため、Modifies ノードは返されず実質的な影響はない。ただし防御的に parse 後に Modifies を除外するフィルタを検討しても良い。

### 4.3 src/cli/issue.rs

```rust
// sort_order()
KnowledgeRelation::HasDesign => 1,
KnowledgeRelation::HasReview => 2,
KnowledgeRelation::HasWorkplan => 3,
KnowledgeRelation::HasProgress => 4, // 追加
KnowledgeRelation::Modifies => 5, // 4 → 5
```

### 4.4 src/cli/before_change.rs

```rust
// relation_priority()
"has_design" => 0,
"has_workplan" => 1,
"has_review" => 2,
"has_progress" => 3, // 追加
"modifies" => 4, // 3 → 4
_ => 5, // 4 → 5
```

### 4.5 src/output/human.rs

```rust
// relation_display_label() のフォールバック match ブロック (line 252)
// doc_subtype が Some の場合は subtype.display_label_en() が優先されるが、
// None の場合のフォールバックとして追加
match relation {
"has_design" => "design",
"has_review" => "review",
"has_workplan" => "workplan",
"has_progress" => "progress", // 追加
other => other,
}
```

## 5. テスト変更方針

| ファイル | テスト | 変更内容 |
|---------|-------|---------|
| `src/indexer/knowledge.rs` | `test_parse_progress_report` | `HasReview` → `HasProgress` |
| `src/indexer/knowledge.rs` | `test_knowledge_relation_as_str` | `HasProgress` アサーション追加 |
| `src/indexer/knowledge.rs` | `test_knowledge_relation_parse` | `has_progress` パーステスト追加 |
| `src/indexer/knowledge.rs` | `test_knowledge_relation_display` | `HasProgress` 表示テスト追加 |
| `src/indexer/symbol_store.rs` | `test_find_documents_by_issue_metadata_parsed` | progress-report を `HasProgress` に変更 |
| `src/output/human.rs` | `test_relation_display_label_*` | `has_progress` テストケース追加・更新 |
| `src/cli/before_change.rs` | `test_relation_priority_order` | `has_progress` 優先度アサーション追加 |
| `src/cli/issue.rs` | テストデータ | progress-report の relation を `HasProgress` に変更 |
| `tests/e2e_issue.rs` | テストデータ | progress-report の relation を `HasProgress` に変更 |

## 6. セキュリティ設計

- 本変更はenum値の追加とパターンマッチの拡張のみ
- パストラバーサル、入力検証等のセキュリティリスクなし
- unsafe コード不使用

## 7. 品質基準

| チェック項目 | コマンド | 基準 |
|-------------|----------|------|
| ビルド | `cargo build` | エラー0件 |
| Clippy | `cargo clippy --all-targets -- -D warnings` | 警告0件 |
| テスト | `cargo test --all` | 全テストパス |
| フォーマット | `cargo fmt --all -- --check` | 差分なし |
22 changes: 22 additions & 0 deletions dev-reports/issue/165/issue-review/hypothesis-verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# 仮説検証レポート: Issue #165

## 仮説1: progress-report.mdがhas_reviewとして登録されている
- **判定**: Confirmed
- **根拠**: `src/indexer/knowledge.rs:400` の `build_pattern_rules()` で progress-report パターンの relation が `KnowledgeRelation::HasReview` に設定されている

## 仮説2: 影響ファイルは knowledge.rs と human.rs
- **判定**: Partially Confirmed
- **根拠**: Issue記載の2ファイルに加え、以下のファイルにも影響がある
- `src/indexer/symbol_store.rs:880-888` — DB読み取り時の relation パース(`has_progress`追加必要)
- `src/cli/before_change.rs:331-338` — relation_priority に `has_progress` 追加必要
- `src/cli/issue.rs:98-103` — sort_order の match に `HasProgress` 追加必要
- `src/output/human.rs:252-257` — relation_display_label に `has_progress` 追加必要

## 仮説3: KnowledgeRelation enumにHasProgressバリアント追加が必要
- **判定**: Confirmed
- **根拠**: 現在のenum(knowledge.rs:82-87)は HasDesign, HasReview, HasWorkplan, Modifies の4バリアント。HasProgress追加が必要

## 追加発見
- `src/indexer/symbol_store.rs:880-888` の DB relation パースは `parse()` メソッドではなく直接 match しているため、`has_progress` の追加が必要
- `src/cli/issue.rs:98-103` の `sort_order` は exhaustive match のため、新バリアント追加でコンパイルエラーになる(対応必須)
- テストファイル内にも `HasReview` を progress-report で使用しているケースが複数あり更新が必要
1 change: 1 addition & 0 deletions dev-reports/issue/165/issue-review/original-issue.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"body":"## 概要\n\n`knowledge_edges`テーブルで`progress-report.md`が`has_review`として登録されているのを、専用の`has_progress` relationに変更する。\n\n## 背景\n\n#160 でhuman/LLM出力の表示は`[progress]`に修正済みだが、DB層の`relation`は`has_review`のまま。\n\n### 現状\n\n```\nDB: knowledge_edges.relation = \"has_review\" (26件のprogress-reportが対象)\nJSON: relation = \"has_review\", doc_subtype = \"ProgressReport\"\nHuman: [progress] (doc_subtypeから変換)\n```\n\n### 問題\n\n- JSONで`relation`フィールドのみ参照するコンシューマが`has_review`と誤認する可能性\n- DB直接クエリで`has_review`にprogress-reportが混在し、正確なレビュー件数が取得できない\n\n## 対応内容\n\n1. `KnowledgeRelation` enumに`HasProgress`バリアント追加\n2. `build_pattern_rules()`のprogress-reportルールのrelationを`HasProgress`に変更\n3. `KnowledgeRelation::parse()`に`\"has_progress\"`を追加\n4. 既存DBのマイグレーション(`has_review` → `has_progress`)は再インデックスで対応\n\n## 影響ファイル\n\n| ファイル | 変更内容 |\n|---------|---------|\n| `src/indexer/knowledge.rs` | `HasProgress`バリアント追加、PatternRule修正 |\n| `src/output/human.rs` | `relation_display_label`に`has_progress`ケース追加 |\n\n## 受け入れ基準\n\n- [ ] `KnowledgeRelation::HasProgress`が追加されている\n- [ ] 再インデックス後、progress-reportが`has_progress`で登録される\n- [ ] `why --format json`で`relation: \"has_progress\"`が返る\n- [ ] human/LLM出力で引き続き`[progress]`が表示される\n- [ ] 既存テストが全Pass\n\n## 関連\n\n- #160 (表示層での修正)","title":"ナレッジグラフ: progress-reportのrelationをhas_progressに変更"}
49 changes: 49 additions & 0 deletions dev-reports/issue/165/issue-review/stage1-review-context.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"must_fix": [
{
"id": 1,
"description": "影響ファイルの一覧が不完全。src/indexer/symbol_store.rs(DBパースmatch)、src/cli/issue.rs(exhaustive match)、src/cli/before_change.rs(relation_priority)の追加が必要。",
"suggestion": "影響ファイルテーブルにこれら3ファイルを追加し、各変更内容を明記する。"
},
{
"id": 2,
"description": "テストファイルの変更が影響ファイルに含まれていない。knowledge.rs:555, symbol_store.rs:2181,2189, issue.rs:301, tests/e2e_issue.rsのテストで進捗レポートにHasReviewが使われている。",
"suggestion": "影響ファイルにtests/e2e_issue.rsを追加し、各ソースファイル内テストモジュールの変更も明記する。"
}
],
"should_fix": [
{
"id": 1,
"description": "relation_display_labelのフォールバックケース仕様が未定義。doc_subtypeがNoneでhas_progressが渡された場合の表示が未定義。",
"suggestion": "has_progress => progressのフォールバック追加を対応内容に明記する。"
},
{
"id": 2,
"description": "src/cli/why.rsのテストデータにhas_reviewが使われており、影響分析に含めるべき。",
"suggestion": "受け入れ基準にwhyコマンドのテスト全Passを明記するか影響範囲分析にwhy.rsを含める。"
},
{
"id": 3,
"description": "before-changeコマンドでの動作確認が受け入れ基準に含まれていない。",
"suggestion": "受け入れ基準にbefore-changeコマンドでのソート優先度検証を追加する。"
},
{
"id": 4,
"description": "issueコマンドのsort_order()でHasProgressの順序値が未定義。",
"suggestion": "対応内容にHasProgressの順序値を明記する。"
}
],
"nice_to_have": [
{
"id": 1,
"description": "背景セクションの26件という数値は開発環境固有のデータ。",
"suggestion": "具体的件数を削除または一般的表現に変更する。"
},
{
"id": 2,
"description": "symbol_store.rsのDB parse matchとKnowledgeRelation::parse()が重複ロジック。DRY原則の機会。",
"suggestion": "スコープ外だが将来的なリファクタリングIssue作成を検討。"
}
],
"summary": "影響ファイルが大幅に不足。symbol_store.rs, issue.rs, before_change.rs, tests/e2e_issue.rsで追加変更が必要。特にsymbol_store.rsのDBパースとissue.rsのexhaustive matchは変更しないとエラーになる。"
}
14 changes: 14 additions & 0 deletions dev-reports/issue/165/issue-review/stage2-apply-result.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"applied": [
"must_fix#1: 影響ファイルテーブルにsymbol_store.rs, issue.rs, before_change.rsを追加",
"must_fix#2: tests/e2e_issue.rsを影響ファイルに追加",
"should_fix#1: relation_display_labelのhas_progress=>progressフォールバックを対応内容に追加",
"should_fix#3: before-changeコマンドのソート優先度を受け入れ基準に追加",
"should_fix#4: sort_order()のHasProgress順序値(5)を対応内容に明記",
"nice_to_have#1: 26件の具体的件数を一般的表現に変更"
],
"skipped": [
"should_fix#2: why.rsのテストデータはprogress-reportではなく一般的レビューのため、影響なし。既存テスト全Pass基準でカバー",
"nice_to_have#2: DRYリファクタリングはスコープ外"
]
}
23 changes: 23 additions & 0 deletions dev-reports/issue/165/issue-review/stage3-review-context.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"must_fix": [
{"file": "src/indexer/knowledge.rs", "description": "HasProgressバリアント追加、as_str/parse/Display/PatternRule修正"},
{"file": "src/indexer/symbol_store.rs", "description": "find_documents_by_issue()のrelationパースにhas_progress追加(未対応だとエラー)"},
{"file": "src/cli/issue.rs", "description": "sort_order() exhaustive matchにHasProgress追加(コンパイルエラー)"},
{"file": "src/cli/before_change.rs", "description": "relation_priority()にhas_progress追加(未対応だとソート不正)"},
{"file": "src/output/human.rs", "description": "relation_display_labelにhas_progress=>progress追加(未対応だとhas_progressが直接表示)"}
],
"should_fix": [
{"file": "tests/e2e_issue.rs", "description": "progress-reportテストデータをHasProgressに変更"},
{"file": "src/indexer/knowledge.rs", "description": "test_parse_progress_reportのアサーション更新、HasProgressのas_str/parse/Displayテスト追加"},
{"file": "src/indexer/symbol_store.rs", "description": "test_find_documents_by_issue_metadata_parsedのHasReview→HasProgress更新"},
{"file": "src/cli/issue.rs", "description": "test_sort_orderとtest_groupedにHasProgressテストケース追加"},
{"file": "src/output/human.rs", "description": "relation_display_labelテストにhas_progressケース追加"},
{"file": "src/cli/before_change.rs", "description": "test_relation_priority_orderにhas_progressの優先度アサーション追加"}
],
"nice_to_have": [
{"description": "src/cli/why.rsのテストデータは一般reviewなので変更不要"},
{"description": "src/cli/suggest.rsもHasReview不使用で影響なし"},
{"description": "混在状態(旧DB has_review + 新インデックス has_progress)は再インデックスで解決"}
],
"summary": "コンパイルに影響する変更: knowledge.rs(enum/match), issue.rs(exhaustive match), symbol_store.rs(エラーパス)。動作に影響する変更: before_change.rs(ソート), human.rs(表示)。パフォーマンス影響なし。後方互換性はHasReviewが残るため問題なし。"
}
8 changes: 8 additions & 0 deletions dev-reports/issue/165/issue-review/stage4-apply-result.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"applied": [],
"skipped_reason": "Stage 3の影響範囲レビュー結果はStage 1-2で既に特定・反映済みの内容を確認。追加のmust_fixなし。テスト更新のshould_fixは受け入れ基準の既存テスト全Passでカバー済み。",
"must_fix_count": 0,
"should_fix_count": 0,
"skip_stage_5_8": true,
"skip_reason": "1回目レビュー(Stage 1-4)でMust Fix合計0件(Stage 3)。Stage 1のMust Fix 2件は既にStage 2で反映済み。2回目レビュー(Stage 5-8)はスキップ。"
}
22 changes: 22 additions & 0 deletions dev-reports/issue/165/issue-review/summary-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Issue #165 マルチステージレビュー サマリー

## 実施ステージ

| Stage | 種別 | 結果 |
|-------|------|------|
| 0.5 | 仮説検証 | 完了(3仮説中2 Confirmed, 1 Partially Confirmed) |
| 1 | 通常レビュー | Must Fix 2件, Should Fix 4件, Nice to Have 2件 |
| 2 | 指摘反映 | Must Fix 2件, Should Fix 3件反映。影響ファイル4件追加、受け入れ基準3件追加 |
| 3 | 影響範囲レビュー | 新規Must Fix 0件(Stage 1-2で特定済みの内容を再確認) |
| 4 | 指摘反映 | 追加反映なし |
| 5-8 | 2回目レビュー | スキップ(Must Fix残件0のため) |

## 主な改善点

1. **影響ファイル拡充**: 2ファイル → 6ファイル(symbol_store.rs, issue.rs, before_change.rs, tests/e2e_issue.rs 追加)
2. **対応内容具体化**: 4項目 → 9項目(display_label, DB parse, priority, sort_order 追加)
3. **受け入れ基準強化**: 5項目 → 8項目(before-change, issue コマンド, clippy 追加)

## Issue品質評価

レビュー後のIssueは実装に十分な品質。影響範囲が明確で、受け入れ基準も網羅的。
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"applied": [
"must_fix#1: symbol_store.rsのrelationパースをKnowledgeRelation::parse()に統一(DRY改善)",
"must_fix#2: issue.rsとbefore_change.rsのsort_order不整合の理由を明記",
"should_fix#1: human.rsのrelation_display_labelコードスニペットを実際の関数に合わせて更新",
"should_fix#2: Open/Closed原則の技術的負債を認識(本Issue範囲外として記録)"
],
"skipped": []
}
Loading
Loading