diff --git a/dev-reports/design/issue-165-has-progress-design-policy.md b/dev-reports/design/issue-165-has-progress-design-policy.md new file mode 100644 index 0000000..8812544 --- /dev/null +++ b/dev-reports/design/issue-165-has-progress-design-policy.md @@ -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` | 差分なし | diff --git a/dev-reports/issue/165/issue-review/hypothesis-verification.md b/dev-reports/issue/165/issue-review/hypothesis-verification.md new file mode 100644 index 0000000..f2b0b30 --- /dev/null +++ b/dev-reports/issue/165/issue-review/hypothesis-verification.md @@ -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 で使用しているケースが複数あり更新が必要 diff --git a/dev-reports/issue/165/issue-review/original-issue.json b/dev-reports/issue/165/issue-review/original-issue.json new file mode 100644 index 0000000..eaa8c6f --- /dev/null +++ b/dev-reports/issue/165/issue-review/original-issue.json @@ -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に変更"} diff --git a/dev-reports/issue/165/issue-review/stage1-review-context.json b/dev-reports/issue/165/issue-review/stage1-review-context.json new file mode 100644 index 0000000..876583c --- /dev/null +++ b/dev-reports/issue/165/issue-review/stage1-review-context.json @@ -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は変更しないとエラーになる。" +} diff --git a/dev-reports/issue/165/issue-review/stage2-apply-result.json b/dev-reports/issue/165/issue-review/stage2-apply-result.json new file mode 100644 index 0000000..fa6fe35 --- /dev/null +++ b/dev-reports/issue/165/issue-review/stage2-apply-result.json @@ -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リファクタリングはスコープ外" + ] +} diff --git a/dev-reports/issue/165/issue-review/stage3-review-context.json b/dev-reports/issue/165/issue-review/stage3-review-context.json new file mode 100644 index 0000000..95143e3 --- /dev/null +++ b/dev-reports/issue/165/issue-review/stage3-review-context.json @@ -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が残るため問題なし。" +} diff --git a/dev-reports/issue/165/issue-review/stage4-apply-result.json b/dev-reports/issue/165/issue-review/stage4-apply-result.json new file mode 100644 index 0000000..8ec8efe --- /dev/null +++ b/dev-reports/issue/165/issue-review/stage4-apply-result.json @@ -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)はスキップ。" +} diff --git a/dev-reports/issue/165/issue-review/summary-report.md b/dev-reports/issue/165/issue-review/summary-report.md new file mode 100644 index 0000000..8a764de --- /dev/null +++ b/dev-reports/issue/165/issue-review/summary-report.md @@ -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は実装に十分な品質。影響範囲が明確で、受け入れ基準も網羅的。 diff --git a/dev-reports/issue/165/multi-stage-design-review/stage1-apply-result.json b/dev-reports/issue/165/multi-stage-design-review/stage1-apply-result.json new file mode 100644 index 0000000..bf1af92 --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage1-apply-result.json @@ -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": [] +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage1-review-context.json b/dev-reports/issue/165/multi-stage-design-review/stage1-review-context.json new file mode 100644 index 0000000..c5272ee --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage1-review-context.json @@ -0,0 +1,35 @@ +{ + "stage": 1, + "type": "design_principles", + "must_fix": [ + { + "id": 1, + "description": "DRY違反: symbol_store.rs find_documents_by_issue()がKnowledgeRelation::parse()を再利用せずハードコードmatchを使用。3箇所で同じマッピングが重複。", + "suggestion": "find_documents_by_issue()をKnowledgeRelation::parse()に統一し、None→エラー変換で対応。" + }, + { + "id": 2, + "description": "sort_order不整合: issue.rs(Reviewprogressフォールバック追加が必要。設計書の簡略化コードが実際の関数シグネチャと乖離。", + "suggestion": "section 4.5を実際のmatchブロックに合わせて更新。" + }, + { + "id": 2, + "description": "Open/Closed原則: 将来のバリアント追加時に4+ファイル変更が必要。sort_order/relation_priorityのマジックナンバー集中定義の検討。", + "suggestion": "技術的負債として記録。本Issue範囲外。" + } + ], + "nice_to_have": [ + { + "id": 1, + "description": "sort_order()のHasProgressテストケース追加をテスト方針に明記。" + } + ], + "summary": "設計は基本的に健全。主要課題はDRY違反(symbol_store.rsのrelationパース重複)とsort_order不整合の文書化。" +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage2-apply-result.json b/dev-reports/issue/165/multi-stage-design-review/stage2-apply-result.json new file mode 100644 index 0000000..638e344 --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage2-apply-result.json @@ -0,0 +1,8 @@ +{ + "applied": [ + "must_fix M2: DRYリファクタリングのmodifies振る舞い変更の注意事項を設計書に追記" + ], + "skipped": [ + "should_fix S3: test_groupedのテスト更新は実装時にテスト全体を更新するため、テスト変更方針の追記は不要" + ] +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage2-review-context.json b/dev-reports/issue/165/multi-stage-design-review/stage2-review-context.json new file mode 100644 index 0000000..a7c576a --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage2-review-context.json @@ -0,0 +1,12 @@ +{ + "stage": 2, + "type": "consistency", + "must_fix": [ + {"id": "M2", "description": "DRYリファクタリングでmodifies関係が成功パスに変わる(現状はエラー)。意図的か要確認。SQLクエリでtype='document'フィルタがあるため実質問題ないが設計書に記載すべき。"} + ], + "should_fix": [ + {"id": "S3", "description": "tests/e2e_issue.rsのtest_groupedテスト(line 285)もHasReview→HasProgress変更が必要だがテスト変更方針に未記載。"} + ], + "nice_to_have": [], + "summary": "設計はコードベースと整合的。DRYリファクタリングのmodifies振る舞い変更を設計書に記載すべき。" +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage3-apply-result.json b/dev-reports/issue/165/multi-stage-design-review/stage3-apply-result.json new file mode 100644 index 0000000..550e07c --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage3-apply-result.json @@ -0,0 +1,9 @@ +{ + "applied": [ + "IMPACT-1とIMPACT-2は設計書に既に記載済み。DRYリファクタリングが必須であることを再確認。" + ], + "skipped": [ + "IMPACT-3: Serialize PascalCase問題は本Issue範囲外の既存不整合", + "IMPACT-4, IMPACT-5: 設計書に既に記載済み" + ] +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage3-review-context.json b/dev-reports/issue/165/multi-stage-design-review/stage3-review-context.json new file mode 100644 index 0000000..6c1b383 --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage3-review-context.json @@ -0,0 +1,14 @@ +{ + "stage": 3, + "type": "impact_analysis", + "must_fix": [ + {"id": "IMPACT-1", "description": "DRYリファクタリングは必須。未対応だとissueコマンドがhas_progress値でランタイムエラー。"}, + {"id": "IMPACT-2", "description": "sort_order()のexhaustive matchは追加必須(コンパイルエラー)。"} + ], + "should_fix": [ + {"id": "IMPACT-3", "description": "Serialize deriveがPascalCase出力(HasProgress)。既存の不整合だが本Issue範囲外。"}, + {"id": "IMPACT-4", "description": "DRYリファクタリングでModifies受け入れの振る舞い変更 — 設計書に記載済み。"}, + {"id": "IMPACT-5", "description": "e2e_issue.rsテストデータ更新必須 — 設計書に記載済み。"} + ], + "summary": "DRYリファクタリングは必須。セキュリティ懸念なし。後方互換性問題なし。" +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage4-apply-result.json b/dev-reports/issue/165/multi-stage-design-review/stage4-apply-result.json new file mode 100644 index 0000000..c063a90 --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage4-apply-result.json @@ -0,0 +1,4 @@ +{ + "applied": [], + "skipped_reason": "セキュリティレビューでmust_fix/should_fix 0件。変更不要。" +} diff --git a/dev-reports/issue/165/multi-stage-design-review/stage4-review-context.json b/dev-reports/issue/165/multi-stage-design-review/stage4-review-context.json new file mode 100644 index 0000000..5edabe3 --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/stage4-review-context.json @@ -0,0 +1,10 @@ +{ + "stage": 4, + "type": "security", + "must_fix": [], + "should_fix": [], + "nice_to_have": [ + {"id": "SEC-1", "description": "relation_strのformat!()エラーメッセージ — ローカルDB由来で低リスク"} + ], + "summary": "セキュリティ懸念なし。enum追加とmatch拡張のみ。新規入力面なし、パス構築なし、SQL変更なし、unsafeなし。" +} diff --git a/dev-reports/issue/165/multi-stage-design-review/summary-report.md b/dev-reports/issue/165/multi-stage-design-review/summary-report.md new file mode 100644 index 0000000..6ffd56a --- /dev/null +++ b/dev-reports/issue/165/multi-stage-design-review/summary-report.md @@ -0,0 +1,26 @@ +# Issue #165 マルチステージ設計レビュー サマリー + +## 実施ステージ + +| Stage | 種別 | Must Fix | Should Fix | Nice to Have | +|-------|------|----------|-----------|--------------| +| 1 | 設計原則 (SOLID/KISS/YAGNI/DRY) | 2 | 3 | 1 | +| 2 | 整合性 | 1 | 1 | 0 | +| 3 | 影響分析 | 0 (既出の再確認) | 2 (既出) | 2 | +| 4 | セキュリティ | 0 | 0 | 1 | +| 5-8 | 2回目レビュー | スキップ(Must Fix残件0) | - | - | + +## 主な改善点 + +1. **DRY改善**: `find_documents_by_issue()` を `KnowledgeRelation::parse()` に統一。将来のバリアント追加時にsymbol_store.rsの変更が不要に +2. **sort_order不整合の文書化**: issue.rs と before_change.rs の順序差異が意図的であることを明記 +3. **human.rs コードスニペット修正**: 実際の関数シグネチャに合わせた正確な変更設計 +4. **Modifies振る舞い変更の注意**: DRYリファクタリングによる微妙な振る舞い変更を文書化 + +## セキュリティ評価 + +セキュリティ懸念なし。enum追加とmatch拡張のみで、新規入力面・パス構築・SQL変更・unsafeコードなし。 + +## 設計品質評価 + +設計は健全で実装可能。影響範囲は明確に特定済み。DRYリファクタリングにより本来のIssue範囲を超えた改善も含まれている。 diff --git a/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-context.json b/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-context.json new file mode 100644 index 0000000..8f9f5a9 --- /dev/null +++ b/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-context.json @@ -0,0 +1,15 @@ +{ + "issue_number": 165, + "title": "ナレッジグラフ: progress-reportのrelationをhas_progressに変更", + "design_policy": "dev-reports/design/issue-165-has-progress-design-policy.md", + "work_plan": "dev-reports/issue/165/work-plan.md", + "tasks": [ + "KnowledgeRelation enumにHasProgressバリアント追加(as_str, parse, Display)", + "build_pattern_rules()のprogress-reportルールをHasProgressに変更", + "symbol_store.rs find_documents_by_issue()をKnowledgeRelation::parse()に統一(DRY改善)", + "issue.rs sort_order()にHasProgress追加", + "before_change.rs relation_priority()にhas_progress追加", + "human.rs relation_display_label()にhas_progress=>progress追加", + "全テスト更新" + ] +} diff --git a/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-result.json b/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-result.json new file mode 100644 index 0000000..625ed2f --- /dev/null +++ b/dev-reports/issue/165/pm-auto-dev/iteration-1/tdd-result.json @@ -0,0 +1,20 @@ +{ + "status": "success", + "tests_passed": true, + "clippy_clean": true, + "changes_summary": "Added HasProgress variant to KnowledgeRelation enum (between HasWorkplan and Modifies). Updated as_str(), parse(), and Display impl. Changed progress-report PatternRule from HasReview to HasProgress. DRY-refactored symbol_store.rs find_documents_by_issue() to use KnowledgeRelation::parse() instead of hardcoded match. Updated sort_order in issue.rs (HasProgress=4, Modifies=5). Updated relation_priority in before_change.rs (has_progress=3, modifies=4, fallback=5). Added 'has_progress' => 'progress' mapping in human.rs relation_display_label. Updated all test data across knowledge.rs, symbol_store.rs, issue.rs, before_change.rs, human.rs, and e2e_issue.rs.", + "files_modified": [ + "src/indexer/knowledge.rs", + "src/indexer/symbol_store.rs", + "src/cli/issue.rs", + "src/cli/before_change.rs", + "src/output/human.rs", + "tests/e2e_issue.rs" + ], + "quality_checks": { + "cargo_build": "pass", + "cargo_clippy": "pass", + "cargo_fmt": "pass", + "cargo_test": "pass (1 pre-existing failure in test_embed_without_ollama_fails unrelated to changes)" + } +} diff --git a/dev-reports/issue/165/work-plan.md b/dev-reports/issue/165/work-plan.md new file mode 100644 index 0000000..b481be1 --- /dev/null +++ b/dev-reports/issue/165/work-plan.md @@ -0,0 +1,103 @@ +# 作業計画: Issue #165 — progress-reportのrelationをhas_progressに変更 + +## Issue: ナレッジグラフ: progress-reportのrelationをhas_progressに変更 +**Issue番号**: #165 +**サイズ**: S +**優先度**: Medium +**依存Issue**: #160(完了済み) + +## 詳細タスク分解 + +### Phase 1: コア変更(knowledge.rs) + +- [ ] **Task 1.1**: KnowledgeRelation enum に HasProgress バリアント追加 + - 成果物: `src/indexer/knowledge.rs` + - 変更箇所: enum定義(L82-87)、as_str()(L90-96)、parse()(L100-107) + - 依存: なし + +- [ ] **Task 1.2**: build_pattern_rules() の progress-report ルール変更 + - 成果物: `src/indexer/knowledge.rs` + - 変更箇所: PatternRule(L394-401) の relation を HasReview → HasProgress + - 依存: Task 1.1 + +### Phase 2: 依存モジュール更新 + +- [ ] **Task 2.1**: symbol_store.rs DRY リファクタリング + - 成果物: `src/indexer/symbol_store.rs` + - 変更箇所: find_documents_by_issue()(L880-889) のハードコードmatchをKnowledgeRelation::parse()に統一 + - 依存: Task 1.1 + +- [ ] **Task 2.2**: issue.rs sort_order() 更新 + - 成果物: `src/cli/issue.rs` + - 変更箇所: sort_order()(L98-103) に HasProgress => 4 追加、Modifies => 5 に変更 + - 依存: Task 1.1 + +- [ ] **Task 2.3**: before_change.rs relation_priority() 更新 + - 成果物: `src/cli/before_change.rs` + - 変更箇所: relation_priority()(L331-338) に "has_progress" => 3 追加、modifies => 4、_ => 5 に変更 + - 依存: Task 1.1 + +- [ ] **Task 2.4**: human.rs relation_display_label() 更新 + - 成果物: `src/output/human.rs` + - 変更箇所: relation_display_label()(L252-257) に "has_progress" => "progress" 追加 + - 依存: Task 1.1 + +### Phase 3: テスト更新 + +- [ ] **Task 3.1**: knowledge.rs テスト更新 + - test_parse_progress_report: HasReview → HasProgress + - test_knowledge_relation_as_str: HasProgress アサーション追加 + - test_knowledge_relation_parse: has_progress パーステスト追加 + - test_knowledge_relation_display: HasProgress 表示テスト追加 + - 依存: Task 1.1, 1.2 + +- [ ] **Task 3.2**: symbol_store.rs テスト更新 + - test_find_documents_by_issue_metadata_parsed: progress-report を HasProgress に変更 + - 依存: Task 2.1 + +- [ ] **Task 3.3**: issue.rs テスト更新 + - テストデータの progress-report relation を HasProgress に変更 + - 依存: Task 2.2 + +- [ ] **Task 3.4**: before_change.rs テスト更新 + - test_relation_priority_order: has_progress 優先度アサーション追加 + - 依存: Task 2.3 + +- [ ] **Task 3.5**: human.rs テスト更新 + - relation_display_label テストに has_progress ケース追加・更新 + - 依存: Task 2.4 + +- [ ] **Task 3.6**: e2e_issue.rs テスト更新 + - progress-report テストデータを HasProgress に変更 + - 依存: Task 1.1 + +### Phase 4: 品質検証 + +- [ ] **Task 4.1**: 全品質チェック実行 + - `cargo build` / `cargo clippy --all-targets -- -D warnings` / `cargo test --all` / `cargo fmt --all -- --check` + +## TDD実装順序 + +1. まず失敗テストを書く(HasProgress のアサーション) +2. knowledge.rs のenum/parse/as_str を実装(テストパス) +3. PatternRule 変更 + テスト更新 +4. 依存モジュール順次更新 + テスト更新 +5. 全品質チェック + +## 品質チェック項目 + +| チェック項目 | コマンド | 基準 | +|-------------|----------|------| +| ビルド | `cargo build` | エラー0件 | +| Clippy | `cargo clippy --all-targets -- -D warnings` | 警告0件 | +| テスト | `cargo test --all` | 全テストパス | +| フォーマット | `cargo fmt --all -- --check` | 差分なし | + +## Definition of Done + +- [ ] `KnowledgeRelation::HasProgress` が追加されている +- [ ] 再インデックス後、progress-report が `has_progress` で登録される +- [ ] human/LLM出力で引き続き `[progress]` が表示される +- [ ] `before-change` コマンドで progress-report が適切な優先度でソートされる +- [ ] 既存テストが全Pass +- [ ] `cargo clippy --all-targets -- -D warnings` 警告0件 diff --git a/src/cli/before_change.rs b/src/cli/before_change.rs index 51df37e..e9b579a 100644 --- a/src/cli/before_change.rs +++ b/src/cli/before_change.rs @@ -333,8 +333,9 @@ fn relation_priority(relation: &str) -> u8 { "has_design" => 0, "has_workplan" => 1, "has_review" => 2, - "modifies" => 3, - _ => 4, + "has_progress" => 3, + "modifies" => 4, + _ => 5, } } @@ -635,7 +636,8 @@ mod tests { fn test_relation_priority_order() { assert!(relation_priority("has_design") < relation_priority("has_workplan")); assert!(relation_priority("has_workplan") < relation_priority("has_review")); - assert!(relation_priority("has_review") < relation_priority("modifies")); + assert!(relation_priority("has_review") < relation_priority("has_progress")); + assert!(relation_priority("has_progress") < relation_priority("modifies")); assert!(relation_priority("modifies") < relation_priority("unknown")); } diff --git a/src/cli/issue.rs b/src/cli/issue.rs index 5a27cdd..4312375 100644 --- a/src/cli/issue.rs +++ b/src/cli/issue.rs @@ -99,7 +99,8 @@ fn sort_order(entry: &IssueDocumentEntry) -> (u8, u8) { KnowledgeRelation::HasDesign => 1, KnowledgeRelation::HasReview => 2, KnowledgeRelation::HasWorkplan => 3, - KnowledgeRelation::Modifies => 4, + KnowledgeRelation::HasProgress => 4, + KnowledgeRelation::Modifies => 5, }; let subtype_order = match entry.doc_subtype { DocSubtype::DesignPolicy => 1, @@ -298,7 +299,7 @@ mod tests { }, IssueDocumentEntry { file_path: "progress.md".to_string(), - relation: KnowledgeRelation::HasReview, + relation: KnowledgeRelation::HasProgress, doc_subtype: DocSubtype::ProgressReport, }, IssueDocumentEntry { diff --git a/src/indexer/knowledge.rs b/src/indexer/knowledge.rs index d9e7aa2..4c4f22c 100644 --- a/src/indexer/knowledge.rs +++ b/src/indexer/knowledge.rs @@ -83,6 +83,7 @@ pub enum KnowledgeRelation { HasDesign, HasReview, HasWorkplan, + HasProgress, Modifies, } @@ -92,6 +93,7 @@ impl KnowledgeRelation { Self::HasDesign => "has_design", Self::HasReview => "has_review", Self::HasWorkplan => "has_workplan", + Self::HasProgress => "has_progress", Self::Modifies => "modifies", } } @@ -102,6 +104,7 @@ impl KnowledgeRelation { "has_design" => Some(Self::HasDesign), "has_review" => Some(Self::HasReview), "has_workplan" => Some(Self::HasWorkplan), + "has_progress" => Some(Self::HasProgress), "modifies" => Some(Self::Modifies), _ => None, } @@ -397,7 +400,7 @@ fn build_pattern_rules() -> Vec { ) .expect("invalid regex"), doc_subtype: DocSubtype::ProgressReport, - relation: KnowledgeRelation::HasReview, + relation: KnowledgeRelation::HasProgress, }, // Note: issue{N} uses no hyphen separator, matching the review tool's output naming convention PatternRule { @@ -552,7 +555,7 @@ mod tests { assert!(result.is_some()); let entry = result.unwrap(); assert_eq!(entry.issue_number, "55"); - assert_eq!(entry.relation, KnowledgeRelation::HasReview); + assert_eq!(entry.relation, KnowledgeRelation::HasProgress); assert_eq!(entry.doc_subtype, DocSubtype::ProgressReport); } @@ -695,6 +698,7 @@ mod tests { assert_eq!(KnowledgeRelation::HasDesign.as_str(), "has_design"); assert_eq!(KnowledgeRelation::HasReview.as_str(), "has_review"); assert_eq!(KnowledgeRelation::HasWorkplan.as_str(), "has_workplan"); + assert_eq!(KnowledgeRelation::HasProgress.as_str(), "has_progress"); assert_eq!(KnowledgeRelation::Modifies.as_str(), "modifies"); } @@ -712,6 +716,10 @@ mod tests { KnowledgeRelation::parse("has_workplan"), Some(KnowledgeRelation::HasWorkplan) ); + assert_eq!( + KnowledgeRelation::parse("has_progress"), + Some(KnowledgeRelation::HasProgress) + ); assert_eq!( KnowledgeRelation::parse("modifies"), Some(KnowledgeRelation::Modifies) @@ -736,6 +744,10 @@ mod tests { format!("{}", KnowledgeRelation::HasWorkplan), "has_workplan" ); + assert_eq!( + format!("{}", KnowledgeRelation::HasProgress), + "has_progress" + ); assert_eq!(format!("{}", KnowledgeRelation::Modifies), "modifies"); } diff --git a/src/indexer/symbol_store.rs b/src/indexer/symbol_store.rs index 2bde871..f344031 100644 --- a/src/indexer/symbol_store.rs +++ b/src/indexer/symbol_store.rs @@ -877,16 +877,10 @@ impl SymbolStore { for row in rows { let (file_path, relation_str, metadata_opt) = row?; - let relation = match relation_str.as_str() { - "has_design" => crate::indexer::knowledge::KnowledgeRelation::HasDesign, - "has_review" => crate::indexer::knowledge::KnowledgeRelation::HasReview, - "has_workplan" => crate::indexer::knowledge::KnowledgeRelation::HasWorkplan, - other => { - return Err(SymbolStoreError::InvalidEmbedding { - reason: format!("Unknown relation type: {other}"), - }); - } - }; + let relation = crate::indexer::knowledge::KnowledgeRelation::parse(&relation_str) + .ok_or_else(|| SymbolStoreError::InvalidEmbedding { + reason: format!("Unknown relation type: {relation_str}"), + })?; let metadata_str = metadata_opt.unwrap_or_default(); let doc_subtype = if metadata_str.is_empty() { @@ -2178,7 +2172,7 @@ mod tests { issue_number: "42".to_string(), file_path: "dev-reports/issue/42/pm-auto-dev/iteration-1/progress-report.md" .to_string(), - relation: KnowledgeRelation::HasReview, + relation: KnowledgeRelation::HasProgress, doc_subtype: DocSubtype::ProgressReport, }]; store.insert_knowledge_entries(&entries).unwrap(); @@ -2186,7 +2180,7 @@ mod tests { let docs = store.find_documents_by_issue("42").unwrap(); assert_eq!(docs.len(), 1); assert_eq!(docs[0].doc_subtype, DocSubtype::ProgressReport); - assert_eq!(docs[0].relation, KnowledgeRelation::HasReview); + assert_eq!(docs[0].relation, KnowledgeRelation::HasProgress); } // --- insert_file_modifies_entries tests --- diff --git a/src/output/human.rs b/src/output/human.rs index bc7938b..a1c136c 100644 --- a/src/output/human.rs +++ b/src/output/human.rs @@ -253,6 +253,7 @@ pub(crate) fn relation_display_label<'a>( "has_design" => "design", "has_review" => "review", "has_workplan" => "workplan", + "has_progress" => "progress", other => other, } } @@ -427,7 +428,7 @@ mod tests { #[test] fn test_relation_display_label_with_doc_subtype() { assert_eq!( - relation_display_label("has_review", Some(&DocSubtype::ProgressReport)), + relation_display_label("has_progress", Some(&DocSubtype::ProgressReport)), "progress" ); assert_eq!( @@ -457,6 +458,7 @@ mod tests { assert_eq!(relation_display_label("has_design", None), "design"); assert_eq!(relation_display_label("has_review", None), "review"); assert_eq!(relation_display_label("has_workplan", None), "workplan"); + assert_eq!(relation_display_label("has_progress", None), "progress"); assert_eq!(relation_display_label("modifies", None), "modifies"); } } diff --git a/tests/e2e_issue.rs b/tests/e2e_issue.rs index 391a65d..35bae51 100644 --- a/tests/e2e_issue.rs +++ b/tests/e2e_issue.rs @@ -43,7 +43,7 @@ fn setup_issue_test_data(tmp: &std::path::Path) -> std::path::PathBuf { issue_number: "140".to_string(), file_path: "dev-reports/issue/140/pm-auto-dev/iteration-1/progress-report.md" .to_string(), - relation: KnowledgeRelation::HasReview, + relation: KnowledgeRelation::HasProgress, doc_subtype: DocSubtype::ProgressReport, }, KnowledgeEntry {