From da3d4afc8a4172fa2268b841723c522448a83b18 Mon Sep 17 00:00:00 2001 From: kewton Date: Wed, 25 Mar 2026 17:04:08 +0900 Subject: [PATCH] fix(suggest): limit knowledge graph expansion to representative docs per issue (#167) Add filtering and per-issue limiting to suggest command's knowledge graph expansion, reducing proposals from ~80 to ~15-20 by selecting only representative documents (design policy, work plan, review summaries). - Add KnowledgeRelation::priority() method for shared relation ordering - Add filter_and_limit_kg_docs() with Modifies/HasProgress/StageReview exclusion - Switch from find_knowledge_by_issue() to find_documents_by_issue() for doc_subtype support - Limit to MAX_KG_DOCS_PER_ISSUE=4 documents per issue - Refactor before_change.rs relation_priority() as compatibility wrapper (DRY) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../issue-167-suggest-limit-design-policy.md | 318 ++++++++++++++++++ .../issue-review/hypothesis-verification.md | 35 ++ .../167/issue-review/original-issue.json | 1 + .../issue-review/stage1-review-context.json | 57 ++++ .../167/issue-review/stage2-apply-result.json | 15 + .../issue-review/stage3-review-context.json | 63 ++++ .../167/issue-review/stage4-apply-result.json | 15 + .../issue-review/stage5-review-context.json | 45 +++ .../167/issue-review/stage6-apply-result.json | 14 + .../issue-review/stage7-review-context.json | 51 +++ .../167/issue-review/stage8-apply-result.json | 15 + .../issue/167/issue-review/summary-report.md | 45 +++ .../stage1-apply-result.json | 1 + .../stage1-review-context.json | 18 + .../stage2-review-context.json | 17 + .../stage3-review-context.json | 17 + .../stage4-review-context.json | 14 + .../stage5-review-context.json | 55 +++ .../stage6-apply-result.json | 1 + .../stage7-review-context.json | 13 + .../stage8-apply-result.json | 1 + .../summary-report.md | 42 +++ dev-reports/issue/167/work-plan.md | 106 ++++++ src/cli/before_change.rs | 10 +- src/cli/suggest.rs | 309 +++++++++++++++-- src/indexer/knowledge.rs | 13 + 26 files changed, 1254 insertions(+), 37 deletions(-) create mode 100644 dev-reports/design/issue-167-suggest-limit-design-policy.md create mode 100644 dev-reports/issue/167/issue-review/hypothesis-verification.md create mode 100644 dev-reports/issue/167/issue-review/original-issue.json create mode 100644 dev-reports/issue/167/issue-review/stage1-review-context.json create mode 100644 dev-reports/issue/167/issue-review/stage2-apply-result.json create mode 100644 dev-reports/issue/167/issue-review/stage3-review-context.json create mode 100644 dev-reports/issue/167/issue-review/stage4-apply-result.json create mode 100644 dev-reports/issue/167/issue-review/stage5-review-context.json create mode 100644 dev-reports/issue/167/issue-review/stage6-apply-result.json create mode 100644 dev-reports/issue/167/issue-review/stage7-review-context.json create mode 100644 dev-reports/issue/167/issue-review/stage8-apply-result.json create mode 100644 dev-reports/issue/167/issue-review/summary-report.md create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage1-apply-result.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage1-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage2-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage3-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage4-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage5-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage6-apply-result.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage7-review-context.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/stage8-apply-result.json create mode 100644 dev-reports/issue/167/multi-stage-design-review/summary-report.md create mode 100644 dev-reports/issue/167/work-plan.md diff --git a/dev-reports/design/issue-167-suggest-limit-design-policy.md b/dev-reports/design/issue-167-suggest-limit-design-policy.md new file mode 100644 index 0000000..aaa068c --- /dev/null +++ b/dev-reports/design/issue-167-suggest-limit-design-policy.md @@ -0,0 +1,318 @@ +# 設計方針書: Issue #167 suggestコマンドのナレッジグラフ展開制限 + +## 1. 概要 + +suggestコマンドのナレッジグラフ展開が過剰（80件提案）になる問題を修正する。`prepend_knowledge_steps()` の前段にフィルタリングロジックを追加し、代表文書に絞った提案を生成する。 + +## 2. 変更対象モジュールと責務 + +| レイヤー | モジュール | 変更内容 | +|---------|-----------|---------| +| **CLI** | `src/cli/suggest.rs` | フィルタリング関数追加、KG取得API変更、SuggestKgDoc構造体追加 | +| **CLI** | `src/cli/before_change.rs` | `relation_priority()` を `KnowledgeRelation::priority()` に置き換え（DRY改善） | +| **Indexer** | `src/indexer/knowledge.rs` | `KnowledgeRelation::priority()` メソッド追加 | + +## 3. 設計判断とトレードオフ + +### 判断1: KG文書取得APIの選択 + +| 選択肢 | メリット | デメリット | +|--------|---------|----------| +| **(A) `find_knowledge_by_issue()` + retain()** | 既存コード変更が最小、複数Issue一括取得可能 | `doc_subtype`がなくfile_pathパターンマッチに依存 | +| **(B) `find_documents_by_issue()` を使用** ← 採用 | `doc_subtype`による型安全なフィルタ、file_path非依存 | 単一Issue用APIのため複数Issue時はループ呼び出し（最大3回） | + +**採用理由**: `DocSubtype` enum値で `IssueReview`/`DesignReview`（保持）vs `StageReview`（除外）を直接判定でき、ファイルパス規約変更に対して堅牢。SQLiteアクセス回数増加（最大3回）はローカルDBのため影響軽微。 + +### 判断2: フィルタリングレイヤーの配置 + +**採用**: `prepend_knowledge_steps()` の前段（`suggest.rs`内）で実施。 + +**理由**: `before_change.rs` と同パターン。`find_documents_by_issue()` のSQL変更は不要で、他コマンド（issue, before_change）への影響を回避。 + +### 判断3: MAX_KG_DOCS_PER_ISSUE の値 + +**採用**: `MAX_KG_DOCS_PER_ISSUE = 4` + +**根拠**: 代表文書4種（design-policy, work-plan, issue-review/summary-report.md, design-review/summary-report.md）と整合。`before_change.rs` の `MAX_DOCS_PER_ISSUE = 2` より大きいが、suggestは調査開始ガイドとしてより多くのコンテキストが有用。 + +### 判断4: relation_priorityの実装方針 + +**採用**: `KnowledgeRelation` に `priority()` メソッドを追加し、`suggest.rs` と `before_change.rs` の両方で共通利用する。 + +**理由**: `before_change.rs` の `relation_priority()` は `&str` ベースで同一の優先度値を定義しており、DRY違反となっている。`KnowledgeRelation::priority()` を `src/indexer/knowledge.rs` に追加することで、優先度定義を一元化する。`before_change.rs` の既存 `relation_priority()` 関数は互換ラッパーとして残し、内部実装を `KnowledgeRelation::parse().map_or(5, |r| r.priority())` に委譲する。これにより未知のrelation値に対するフォールバック（優先度5）を維持する。 + +### 判断5: find_documents_by_issue()ループ方式の採用根拠 + +`find_knowledge_by_issue()` + SQLに `metadata`（`doc_subtype`）カラムを追加する方式も検討した。しかし、`find_knowledge_by_issue()` は `issue.rs`・`before_change.rs` でも使用されており、SQL変更やレスポンス型変更がこれらのコマンドに波及する。`find_documents_by_issue()` ループ方式は既存APIを変更せず、`suggest.rs` 内で完結するため、影響範囲を最小化できる。 + +## 4. 詳細設計 + +### 4.1 新規定数 + +```rust +/// ナレッジグラフからのIssue単位最大ドキュメント数 +const MAX_KG_DOCS_PER_ISSUE: usize = 4; +``` + +### 4.2 新規構造体: SuggestKgDoc + +`find_documents_by_issue()` は `IssueDocumentEntry`（issue_numberなし）を返すため、suggest用に `issue_number` を付与したDTOを定義する。 + +```rust +/// suggestコマンド用のKGドキュメントDTO +struct SuggestKgDoc { + issue_number: String, + file_path: String, + relation: KnowledgeRelation, + doc_subtype: DocSubtype, +} +``` + +### 4.3 新規関数: filter_and_limit_kg_docs() + +```rust +/// ナレッジグラフドキュメントをフィルタリング・Issue単位制限する。 +/// +/// 1. modifies / has_progress / has_review(StageReview) を除外 +/// 2. relation_priority でソート +/// 3. Issue単位にグルーピングし MAX_KG_DOCS_PER_ISSUE 件に制限 +/// +/// issue_numbersの順序でIssueをグルーピングすることで、入力順を維持する。 +/// これにより、呼び出し元が指定したIssue優先順位が結果に反映される。 +fn filter_and_limit_kg_docs(docs: Vec, issue_numbers: &[String]) -> Vec { + // Step 1: retain() フィルタリング + let mut filtered: Vec = docs.into_iter() + .filter(|d| { + match d.relation { + KnowledgeRelation::Modifies => false, + KnowledgeRelation::HasProgress => false, + KnowledgeRelation::HasReview => { + // IssueReview, DesignReview のみ保持、StageReview は除外。 + // ProgressReport は has_progress リレーションで管理されるため + // HasReview + ProgressReport の組み合わせは通常発生しないが、 + // 万一存在した場合は DocSubtype の match で暗黙的に除外される。 + // これは意図的な設計判断である。 + matches!(d.doc_subtype, DocSubtype::IssueReview | DocSubtype::DesignReview) + } + KnowledgeRelation::HasDesign | KnowledgeRelation::HasWorkplan => true, + } + }) + .collect(); + + // Step 2: KnowledgeRelation::priority() でソート（sort_by は安定ソート） + filtered.sort_by(|a, b| { + a.relation.priority().cmp(&b.relation.priority()) + }); + + // Step 3: Issue単位グルーピング + 上限制御 + // issue_numbers の順序を維持してグルーピングする（SF-2対応） + let mut issue_groups: HashMap> = HashMap::new(); + for doc in filtered { + issue_groups.entry(doc.issue_number.clone()).or_default().push(doc); + } + + let mut result = Vec::new(); + for issue_num in issue_numbers { + if let Some(docs) = issue_groups.remove(issue_num) { + result.extend(docs.into_iter().take(MAX_KG_DOCS_PER_ISSUE)); + } + } + result +} +``` + +### 4.4 KnowledgeRelation::priority() メソッド追加（src/indexer/knowledge.rs） + +`KnowledgeRelation` に `priority()` メソッドを追加し、`suggest.rs` と `before_change.rs` で共通利用する。 + +```rust +impl KnowledgeRelation { + /// Relation priority for sorting (lower = higher priority). + /// HasProgress / Modifies はフィルタで除外されることが多いが、 + /// 型の網羅性（exhaustive match）を保証するために優先度を定義している。 + pub fn priority(&self) -> u8 { + match self { + Self::HasDesign => 0, + Self::HasWorkplan => 1, + Self::HasReview => 2, + Self::HasProgress => 3, + Self::Modifies => 4, + } + } +} +``` + +**NH-2: priority()をKnowledgeRelationに配置する理由**: リレーションの優先度はドメイン知識（どのリレーションがより重要か）に基づく判断であり、リレーション型自体に帰属させることで、各利用箇所（suggest.rs, before_change.rs）が独自の優先度定義を持つ必要がなくなる。ドメインルールの一元管理先として `KnowledgeRelation` が適切である。 + +**変更対象**: +- `src/indexer/knowledge.rs`: `priority()` メソッド追加 +- `src/cli/before_change.rs`: 既存の `relation_priority(&str) -> u8` 関数を互換ラッパーとして残す（MF-2対応、下記参照） +- `src/cli/suggest.rs`: `kg_relation_priority()` ローカル関数の代わりに `relation.priority()` を使用 + +**MF-2: before_change.rsの `relation_priority()` 互換ラッパー**: `before_change.rs` の `relation_priority()` は `&str` ベースのインターフェースであり、未知のrelation値に対するフォールバック（`unknown → 5`）を提供している。`KnowledgeRelation::parse()` + `priority()` への単純置換ではこのフォールバックが失われる。そのため、`relation_priority()` 関数自体は互換ラッパーとして残し、内部実装のみを `KnowledgeRelation` に委譲する形とする。 + +```rust +// src/cli/before_change.rs — 互換ラッパーとして残す +fn relation_priority(s: &str) -> u8 { + KnowledgeRelation::parse(s).map_or(5, |r| r.priority()) +} +``` + +これにより、既知のrelation値は `KnowledgeRelation::priority()` の一元定義を使用しつつ、未知値に対してはフォールバック優先度5を返す安全な動作を維持する。 + +### 4.5 query_knowledge_graph() の変更 + +```rust +fn query_knowledge_graph(ctx: &SearchContext, issue_numbers: &[String]) -> Vec { + if issue_numbers.is_empty() { + return Vec::new(); + } + + let db_path = ctx.symbol_db_path(); + if !db_path.exists() { + return Vec::new(); + } + + // SymbolStore::open() はループ外で1回だけ実行する（DB接続コスト削減） + let store = match SymbolStore::open(&db_path) { + Ok(s) => s, + Err(e) => { + eprintln!("[suggest] knowledge graph open failed: {e}"); + return Vec::new(); + } + }; + + let mut all_docs = Vec::new(); + for issue_num in issue_numbers { + // find_documents_by_issue() の呼び出し + // 個別Issueのエラー時はそのIssueをスキップし、他のIssueの処理を継続する + match store.find_documents_by_issue(issue_num) { + Ok(entries) => { + // IssueDocumentEntry → SuggestKgDoc への変換 + for entry in entries { + all_docs.push(SuggestKgDoc { + issue_number: issue_num.clone(), + file_path: entry.file_path, + relation: entry.relation, + doc_subtype: entry.doc_subtype, + }); + } + } + Err(e) => { + eprintln!("[suggest] knowledge graph query failed for issue {issue_num}: {e}"); + // エラー時はこのIssueをスキップして次のIssueを処理 + continue; + } + } + } + all_docs +} +``` + +**SF-3: 部分失敗時の方針**: +- **全Issue失敗**: `query_knowledge_graph()` が空の `Vec` を返す。suggestコマンドはKGなし（ナレッジグラフステップを生成せず）で処理を継続する。BM25検索・セマンティック検索の結果のみで提案を生成する。 +- **一部Issue失敗**: 成功したIssueの文書のみを採用し、失敗したIssueはスキップする。失敗したIssueについては `eprintln!` で警告を出力するが、コマンド全体のエラーとはしない。 + +### 4.6 prepend_knowledge_steps() の変更 + +`prepend_knowledge_steps()` の引数型を `&[KnowledgeDocResult]` から `&[SuggestKgDoc]` に変更する。 + +**変更理由（MF-1/SF-1対応）**: `KnowledgeDocResult` には `title: Option` フィールドが必要だが `SuggestKgDoc` はこれを持たない（suggestではtitle不要のため）。`KnowledgeDocResult` への変換時に `title: None` を埋める中間変換コードを挟むよりも、`prepend_knowledge_steps()` が直接 `&[SuggestKgDoc]` を受け取る方がKISSの原則に沿う。 + +```rust +// prepend_knowledge_steps() のシグネチャ変更 +fn prepend_knowledge_steps( + strategy: &mut Vec, + kg_docs: &[SuggestKgDoc], + issue_numbers: &[String], +) { + // SuggestKgDoc のフィールド（issue_number, file_path, relation）を直接参照 + // title は不要（suggestのステップ生成では file_path と relation のみ使用するため） + // ... +} +``` + +**既存テスト3件の修正**: `test_prepend_knowledge_steps_with_docs`、`test_prepend_knowledge_steps_empty`、`test_prepend_knowledge_steps_multiple_issues` のテストデータを `KnowledgeDocResult` から `SuggestKgDoc` に変更する。`SuggestKgDoc` は `suggest.rs` 内で定義されるローカル構造体のため、テストも同モジュール内で完結する。変更コストは小さい。 + +**NH-1: SuggestKgDocにtitleを持たせない理由**: suggestコマンドのステップ生成では、参照すべきファイルパスとリレーション種別のみが必要であり、ドキュメントのタイトルは出力に含めない。titleを保持するとfind_documents_by_issue()の返却値からの追加取得が必要になり、不要な複雑性が生じる。 + +### 4.7 処理フロー + +``` +run_suggest() + → ステップ1-4: 入力バリデーション・インデックス解決・リソースオープン・Issue番号抽出（既存） + → ステップ5: query_knowledge_graph() で各Issueの文書取得 + → SymbolStore::open() （1回のみ） + → find_documents_by_issue(issue) × N回（エラー時はスキップ） + → SuggestKgDoc に変換・結合 + → ステップ5.5（新規挿入）: filter_and_limit_kg_docs(docs, &issue_numbers) でフィルタ・制限 + → Modifies/HasProgress除外、StageReview除外 + → relation.priority() でソート + → issue_numbers順でグルーピング、Issue単位 MAX_KG_DOCS_PER_ISSUE 件に制限 + → ステップ6-9: BM25検索・セマンティック検索・結果統合・戦略生成（既存） + → ステップ10: prepend_knowledge_steps() でステップ生成（引数型を &[SuggestKgDoc] に変更） +``` + +**注記**: `filter_and_limit_kg_docs()` はステップ5（KG参照）とステップ6（BM25検索）の間に挿入する。ステップ10の `prepend_knowledge_steps()` は引数型を `&[SuggestKgDoc]` に変更する（MF-1/SF-1対応）。 + +**MAX_KG_DOCS_PER_ISSUE と同一relation複数文書の扱い**: 同一Issueに同一relationの文書が複数存在する場合（例: HasReview が IssueReview と DesignReview の2文書）、`relation.priority()` でソート後に `take(MAX_KG_DOCS_PER_ISSUE)` で先頭4件を採用する。同一priority内の順序は安定ソートにより `find_documents_by_issue()` の返却順を維持する。 + +## 5. 影響範囲 + +### 影響あり +- `src/cli/suggest.rs`: `query_knowledge_graph()` の変更、新規関数 `filter_and_limit_kg_docs()` 追加、新規構造体 `SuggestKgDoc` 追加 +- `src/indexer/knowledge.rs`: `KnowledgeRelation::priority()` メソッド追加 +- `src/cli/before_change.rs`: 既存の `relation_priority(&str) -> u8` ローカル関数を `KnowledgeRelation::parse().map_or(5, |r| r.priority())` の互換ラッパーに変更（DRY改善、未知値フォールバック維持） + +### 影響なし +- `src/cli/issue.rs`: `find_documents_by_issue()` API自体は変更なし +- `src/indexer/symbol_store.rs`: API変更なし + +## 6. テスト戦略 + +### ユニットテスト（suggest.rs内 #[cfg(test)]） + +#### 新規テスト + +| テスト | 検証内容 | +|--------|---------| +| `test_filter_removes_modifies` | Modifies リレーション除外 | +| `test_filter_removes_has_progress` | HasProgress リレーション除外 | +| `test_filter_keeps_issue_review_removes_stage_review` | IssueReview/DesignReview保持、StageReview除外 | +| `test_filter_keeps_design_and_workplan` | HasDesign/HasWorkplan保持 | +| `test_filter_limits_per_issue` | MAX_KG_DOCS_PER_ISSUE制限 | +| `test_filter_empty_after_all_filtered` | 全件除外時に空Vec | +| `test_kg_relation_priority_order` | 優先度ソート順の検証 | + +#### 既存テスト修正（prepend_knowledge_steps引数型変更対応） + +`prepend_knowledge_steps()` の引数型を `&[KnowledgeDocResult]` から `&[SuggestKgDoc]` に変更するため（MF-1/SF-1対応）、以下の既存テスト3件のテストデータを `SuggestKgDoc` に修正する。変更はテストデータの構造体型のみであり、検証内容（ステップ生成結果の確認）は変更しない。 + +| 既存テスト | 影響 | +|-----------|------| +| `test_prepend_knowledge_steps_with_docs` | テストデータを `SuggestKgDoc` に変更 | +| `test_prepend_knowledge_steps_empty` | テストデータを `SuggestKgDoc` に変更 | +| `test_prepend_knowledge_steps_multiple_issues` | テストデータを `SuggestKgDoc` に変更 | + +### E2Eテスト（tests/e2e_suggest.rs） + +| テスト | 検証内容 | +|--------|---------| +| `test_suggest_kg_limit` | KGステップ数が上限内 | +| `test_suggest_no_modifies_in_output` | modifies文書がcontext出力に含まれない | + +## 7. セキュリティ考慮 + +- パストラバーサル: `file_path` はSQLiteから取得した既存パスを使用。新たなファイルアクセスは発生しない。 +- unsafe: 使用なし。 +- SQLインジェクション: `find_documents_by_issue()` は既存APIでパラメータバインドを使用しており、Issue番号の直接SQL埋め込みは行わない。本変更で新たなSQLクエリは追加しない。 +- リスク認識: SymbolStore（SQLite）のファイルパスはユーザーが `--index-path` で指定可能だが、これは既存の設計であり本Issueのスコープ外。悪意あるDBファイルへの差し替えリスクは全コマンド共通の課題として別途対応を検討する。 + +## 8. 品質基準 + +| チェック | コマンド | 基準 | +|---------|---------|------| +| ビルド | `cargo build` | エラー0件 | +| Clippy | `cargo clippy --all-targets -- -D warnings` | 警告0件 | +| テスト | `cargo test --all` | 全パス | +| フォーマット | `cargo fmt --all -- --check` | 差分なし | diff --git a/dev-reports/issue/167/issue-review/hypothesis-verification.md b/dev-reports/issue/167/issue-review/hypothesis-verification.md new file mode 100644 index 0000000..0a9a2a9 --- /dev/null +++ b/dev-reports/issue/167/issue-review/hypothesis-verification.md @@ -0,0 +1,35 @@ +# 仮説検証レポート: Issue #167 + +## 検証対象の仮説 + +suggestコマンドがナレッジグラフからIssue関連の全ファイルを1件ずつcontextコマンドに展開するため、提案数が80件に膨らむ。 + +## 検証結果: **Confirmed** + +### 根拠 + +1. **フィルタリングなしの展開**: `query_knowledge_graph()` (suggest.rs:221-246) がIssueに紐づく全ドキュメントを返却 +2. **リレーション種別フィルタなし**: has_progress, modifies を含む全リレーションが展開対象 +3. **doc_subtypeフィルタなし**: JSON成果物、stage別レビュー等も個別展開 +4. **件数制限なし**: `prepend_knowledge_steps()` (suggest.rs:249-276) が全ドキュメントを1件1コマンドで展開 + +### 参照: before_changeコマンドの既存実装 + +`before_change.rs` では以下の制御が既に実装済み: +- `relation_priority()` による優先度付け (has_design=0, has_workplan=1, has_review=2, has_progress=3, modifies=4) +- `MAX_DOCS_PER_ISSUE = 2` によるIssue単位の件数制限 +- `modifies` リレーションの除外フィルタ + +### 改善案の妥当性 + +Issue記載の改善案（ドキュメント種別で優先度をつけてフィルタリング）は、既存の `before_change.rs` の実装パターンと整合しており、妥当。 + +### 関連コード + +| コンポーネント | ファイル | 行 | 関数 | +|---|---|---|---| +| KG展開（問題箇所） | suggest.rs | 249-276 | `prepend_knowledge_steps()` | +| KGクエリ | suggest.rs | 221-246 | `query_knowledge_graph()` | +| リレーション定義 | knowledge.rs | 80-88 | `KnowledgeRelation` | +| 参照実装 | before_change.rs | 331-340 | `relation_priority()` | +| 参照実装 | before_change.rs | 349-381 | Issue単位グルーピング | diff --git a/dev-reports/issue/167/issue-review/original-issue.json b/dev-reports/issue/167/issue-review/original-issue.json new file mode 100644 index 0000000..3803f17 --- /dev/null +++ b/dev-reports/issue/167/issue-review/original-issue.json @@ -0,0 +1 @@ +{"body":"## 概要\n\n#157 の修正でsuggestがナレッジグラフを参照するようになったが、Issue関連の全ファイルを1件ずつ`context`コマンドに展開するため、提案数が80件に膨らみ実用的でない。\n\n## 再現手順\n\n```bash\ncommandindexdev suggest --for \"Issue #299のiPadレイアウト修正の設計判断を理解したい\"\n```\n\n### 実際の結果\n\n80件の提案。Issue #299の全関連ファイル（JSON成果物、stage別レビューコンテキスト等を含む）が個別にcontext展開される:\n\n```\n1. commandindexdev issue 299 --format json (OK - これは適切)\n2. commandindexdev context -- 'dev-reports/design/issue-299-...' --max-files 5\n3. commandindexdev context -- 'dev-reports/issue/299/pm-auto-dev/...' --max-files 5\n...\n58. commandindexdev context -- 'tests/unit/config/z-index.test.ts' --max-files 5\n...（計75件のIssue関連context + 5件のBM25ベース提案）\n```\n\n### 期待される結果\n\n5-10件程度の提案。代表文書に絞る:\n\n```\n1. commandindexdev issue 299 --format json\n2. commandindexdev context -- 'dev-reports/design/issue-299-ipad-layout-fix-design-policy.md' --max-files 5\n3. commandindexdev context -- 'dev-reports/issue/299/work-plan.md' --max-files 5\n4. commandindexdev context -- 'dev-reports/issue/299/issue-review/summary-report.md' --max-files 5\n5. commandindexdev context -- 'dev-reports/issue/299/multi-stage-design-review/summary-report.md' --max-files 5\n```\n\n## 改善案\n\nナレッジグラフからの展開時にドキュメント種別で優先度をつけてフィルタリングする:\n\n1. **最優先（常に含める）**: `has_design`（設計ポリシー）、`has_workplan`（作業計画）\n2. **次点（サマリーのみ）**: `has_review`のうちsummary-report.mdのみ。stage別の個別レビューは省略\n3. **省略**: JSON成果物（`*-context.json`, `*-result.json`）、`has_progress`\n4. **省略**: `modifies`のソースコード個別展開\n\nsuggestの目的は「どこから調べ始めるか」のガイドであり、全文書の網羅ではない。\n\n## テスト環境\n\n- commandindex 0.1.0\n- CommandMateリポジトリ（2910ファイル、124690セクション）","title":"suggestコマンドのナレッジグラフ展開が過剰（80件提案）"} diff --git a/dev-reports/issue/167/issue-review/stage1-review-context.json b/dev-reports/issue/167/issue-review/stage1-review-context.json new file mode 100644 index 0000000..8dbe0ac --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage1-review-context.json @@ -0,0 +1,57 @@ +{ + "must_fix": [ + { + "id": "MF-1", + "title": "改善案の「省略: modifiesのソースコード個別展開」はsuggestでは不要な記述", + "description": "Issue本文の改善案4番目に「省略: modifiesのソースコード個別展開」とあるが、suggest.rsのprepend_knowledge_steps()はfind_knowledge_by_issue()の結果をそのまま使っており、find_knowledge_by_issue()はdocumentノードとfileノードの両方を返す（kn_doc.type IN ('document', 'file')）。つまりmodifies関係のfileノード（ソースコード）もcontext展開される。before_change.rsではdocs.retain(|d| d.relation != KnowledgeRelation::Modifies)で明示的に除外しているが、suggest.rsにはこのフィルタがない。Issue本文はこの問題を正しく指摘しているが、「省略」ではなく「フィルタリングで除外」と明示すべき。", + "suggestion": "改善案を以下のように修正: modifiesのfileノードはprepend_knowledge_steps()の前段でretain()により明示的に除外する。before_change.rs L434のパターンを参考にすること。受け入れ基準にも「modifies関係のファイルノードがsuggestの提案に含まれないこと」を明記する。" + }, + { + "id": "MF-2", + "title": "受け入れ基準が未定義", + "description": "Issue本文に受け入れ基準（Acceptance Criteria）が記載されていない。改善案は方向性を示しているが、具体的な合格条件（提案数の上限、フィルタリング対象の網羅的リスト、既存テストへの影響など）が不明確。", + "suggestion": "以下の受け入れ基準を追加すべき: (1) ナレッジグラフから展開されるcontextステップ数がIssueあたり最大N件に制限されること、(2) has_progress関係のドキュメントが除外されること、(3) modifies関係のファイルノードが除外されること、(4) has_reviewのうちsummary-report.md以外のstage別レビューが除外されること、(5) JSON成果物（.json拡張子）が除外されること、(6) 既存テストが更新され新しいフィルタリングロジックをカバーすること。" + } + ], + "should_fix": [ + { + "id": "SF-1", + "title": "before_change.rsのgroup_and_limit_by_issueパターンをsuggestでも採用すべきことをIssueに明記する", + "description": "before_change.rsにはgroup_and_limit_by_issue()という成熟したパターンがあり、Issue単位のグルーピング、relation_priorityによるドキュメント選択、MAX_DOCS_PER_ISSUE制限を実現している。Issue本文の改善案はフィルタリングの方向性を示しているが、この既存パターンの再利用を明示的に推奨していない。", + "suggestion": "改善案に「before_change.rsのgroup_and_limit_by_issue()およびrelation_priority()パターンを参考に、suggest用のフィルタリング関数を実装する」と明記する。" + }, + { + "id": "SF-2", + "title": "フィルタリングのレイヤー設計を明確にする", + "description": "フィルタリングを(A) find_knowledge_by_issue()のSQL側で行うか、(B) prepend_knowledge_steps()の前段で行うか、(C) prepend_knowledge_steps()内部で行うかが不明確。before_change.rsは(B)のパターンを採用している。", + "suggestion": "before_change.rsと一貫性を保つため、(B)のパターンを推奨する。find_knowledge_by_issue()のSQL変更は不要（他のコマンドへの影響を避ける）。" + }, + { + "id": "SF-3", + "title": "KnowledgeDocResultにdoc_subtypeフィールドがない問題", + "description": "改善案ではhas_reviewのうちsummary-report.mdのみを残すフィルタリングを提案しているが、KnowledgeDocResult構造体にはdoc_subtypeフィールドがない。そのためsummary-report.mdの判別はfile_pathの文字列マッチに依存することになるが、Issue本文ではこの制約に言及していない。", + "suggestion": "file_pathの文字列パターンマッチで「summary-report.md」を含むかどうかで判定する簡易アプローチで十分。Issueに選択の根拠を記載すべき。" + }, + { + "id": "SF-4", + "title": "提案数の具体的上限値をIssueに記載する", + "description": "Issue本文で「5-10件程度の提案」と期待されるが、具体的な上限定数が示されていない。", + "suggestion": "MAX_KG_DOCS_PER_ISSUE = 2 を定義。最大ステップ数 = MAX_ISSUE_NUMBERS(3) * (1 + MAX_KG_DOCS_PER_ISSUE(2)) = 9ステップ + 既存戦略ステップ。" + } + ], + "nice_to_have": [ + { + "id": "NH-1", + "title": "issueコマンドステップとcontextステップの重複可能性", + "description": "issueコマンド自体がドキュメント一覧を返すため、contextで個別展開するのは情報の重複がある。", + "suggestion": "フィルタリング実装後、issueコマンドステップを残すかどうかを検討する。" + }, + { + "id": "NH-2", + "title": "JSON成果物の除外ルールをパターンルールと整合させる", + "description": "knowledge.rsのparse_dev_report_path()は既にJSONファイルをパターンマッチ対象外としている。ナレッジグラフに登録されるドキュメントノードにJSONファイルは含まれない。ただしfileノード（modifies関係）経由でJSONファイルが含まれる可能性はある。", + "suggestion": "Issue本文の改善案から「JSON成果物の省略」を削除するか、「modifiesのfileノード除外によりJSON成果物も自動的に除外される」と補足する。" + } + ], + "summary": "Issue #167の問題指摘は正確。before_change.rsに参考にすべき成熟したパターンが存在する。修正必須事項は(1) modifiesのfileノード除外の明示、(2) 受け入れ基準の定義。改善事項として、既存パターン再利用の明示、フィルタリングレイヤーの設計方針、doc_subtypeがない制約への対処方針、具体的な上限定数の定義がある。" +} diff --git a/dev-reports/issue/167/issue-review/stage2-apply-result.json b/dev-reports/issue/167/issue-review/stage2-apply-result.json new file mode 100644 index 0000000..c794092 --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage2-apply-result.json @@ -0,0 +1,15 @@ +{ + "stage": 2, + "action": "apply_review", + "applied_items": ["MF-1", "MF-2", "SF-1", "SF-2", "SF-3", "SF-4", "NH-1", "NH-2"], + "changes": [ + "modifies除外をretain()によるフィルタリングと明示", + "受け入れ基準セクションを新設（7項目）", + "before_change.rsのパターン再利用を明記", + "フィルタリングレイヤーの処理フロー図を追加", + "file_pathパターンマッチによるhas_reviewフィルタを注記", + "MAX_KG_DOCS_PER_ISSUE = 2 を定数として明記", + "補足セクションにNH-1, NH-2を追記" + ], + "issue_updated": true +} diff --git a/dev-reports/issue/167/issue-review/stage3-review-context.json b/dev-reports/issue/167/issue-review/stage3-review-context.json new file mode 100644 index 0000000..faeabc5 --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage3-review-context.json @@ -0,0 +1,63 @@ +{ + "must_fix": [ + { + "id": "MF-1", + "title": "KnowledgeDocResultにdoc_subtypeフィールドがなく、has_reviewのサブタイプ判別不可", + "description": "KnowledgeDocResult (symbol_store.rs:66-71) にdoc_subtypeフィールドがないため、IssueReview/DesignReview(summary)とStageReview(非summary)を区別できない。find_knowledge_by_issue()もmetadata/doc_subtypeを取得していない。", + "suggestion": "file_pathパターンマッチ（summary-report.mdで終わるかどうか）で判定する。doc_subtypeのスキーマ変更は不要。" + }, + { + "id": "MF-2", + "title": "新しいフィルタリングロジックのユニットテストが必要", + "description": "既存のprepend_knowledge_stepsテストはフィルタリング前の挿入ロジックのみテスト。新しいフィルタリングロジック（modifies除外、has_progress除外、has_review非summary除外、Issue単位制限）のテストがない。", + "suggestion": "filter_and_limit_kg_docs()のユニットテストを追加: (1) modifies除外、(2) has_progress除外、(3) has_review非summary除外、(4) has_design/has_workplan保持、(5) MAX_KG_DOCS_PER_ISSUE制限、(6) 全件フィルタ後0件のエッジケース。" + } + ], + "should_fix": [ + { + "id": "SF-1", + "title": "has_reviewフィルタリング基準の明確化", + "description": "has_reviewにはIssueReview(summary-report.md), DesignReview(summary-report.md), StageReview(stage別ファイル)が含まれる。ALLを除外するのかsummary以外を除外するのかを明確にすべき。", + "suggestion": "file_pathがsummary-report.mdで終わるhas_reviewのみ保持、それ以外を除外する方針。" + }, + { + "id": "SF-2", + "title": "Issue単位グルーピングでrelation_priorityソートを実装すべき", + "description": "before_change.rsと同じパターンで、Issue単位グルーピング後にrelation_priorityでソートし上位を選択すべき。", + "suggestion": "KnowledgeRelation enumにpriority()メソッドを追加するか、suggest.rs内でrelation_priority関数を定義する。" + }, + { + "id": "SF-3", + "title": "before_change.rsのgroup_and_limit_by_issueはBeforeChangeFinding型で直接再利用不可", + "description": "before_change.rsの関数はBeforeChangeFinding型を扱う。suggest.rsではKnowledgeDocResultを扱うため、専用のフィルタリング関数が必要。", + "suggestion": "suggest.rs内にfilter_and_limit_kg_docs()関数を実装する。KnowledgeRelation enumで直接パターンマッチ。" + } + ], + "nice_to_have": [ + { + "id": "NH-1", + "title": "relation_priorityを共通モジュールに抽出", + "description": "before_change.rsとsuggest.rsで同じ優先度ロジックが必要。KnowledgeRelation enumにpriority()メソッドを追加すればDRY。", + "suggestion": "KnowledgeRelation::priority() -> u8 メソッド追加。" + }, + { + "id": "NH-2", + "title": "MAX_KG_DOCS_PER_ISSUEをbefore_changeと共有検討", + "description": "before_change.rsのMAX_DOCS_PER_ISSUE=2と同値。将来的に共有定数にする検討。", + "suggestion": "現時点ではsuggest.rs内に定義。将来的に共通化。" + }, + { + "id": "NH-3", + "title": "他コマンドへの影響なし（確認済み）", + "description": "変更はsuggest.rsのquery_knowledge_graph()とprepend_knowledge_steps()の間のみ。before_change, issue等の他コマンドへの副作用なし。", + "suggestion": "対応不要。" + }, + { + "id": "NH-4", + "title": "パフォーマンス影響は無視可能", + "description": "O(n) retain() + O(n) groupingのみ。BM25/semantic検索やSQLiteクエリに比べて無視可能。", + "suggestion": "対応不要。" + } + ], + "summary": "主要リスクはMF-1: KnowledgeDocResultにdoc_subtypeがなく、has_reviewのsummary判別にfile_pathパターンマッチが必要。MF-2: フィルタリングロジックのテスト追加が必要。変更はsuggest.rsに限定され他コマンドへの影響なし。パフォーマンス影響も無視可能。" +} diff --git a/dev-reports/issue/167/issue-review/stage4-apply-result.json b/dev-reports/issue/167/issue-review/stage4-apply-result.json new file mode 100644 index 0000000..f852cae --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage4-apply-result.json @@ -0,0 +1,15 @@ +{ + "stage": 4, + "action": "apply_impact_review", + "applied_items": ["MF-1", "MF-2", "SF-1", "SF-2", "SF-3", "NH-1", "NH-3"], + "changes": [ + "file_path.ends_with(\"summary-report.md\")による判定方針を明記", + "テスト要件セクション新設（5つのユニットテスト項目）", + "has_reviewフィルタリング基準の明確化", + "relation_priorityソート順を明記", + "filter_and_limit_kg_docs()関数新設の方針を追加", + "relation_priority共通化検討を補足セクションに追加", + "他コマンドへの影響なし確認を記載" + ], + "issue_updated": true +} diff --git a/dev-reports/issue/167/issue-review/stage5-review-context.json b/dev-reports/issue/167/issue-review/stage5-review-context.json new file mode 100644 index 0000000..41381c0 --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage5-review-context.json @@ -0,0 +1,45 @@ +{ + "must_fix": [ + { + "id": "MF-1", + "title": "期待結果の代表文書例とMAX_KG_DOCS_PER_ISSUE = 2が両立していない", + "description": "Issue本文の期待される結果では1 Issueにつきissue --format jsonに加えてdesign / work-plan / issue-review summary / multi-stage-design-review summaryの4件を残す例になっている。一方、改善案と受け入れ基準ではKG由来ドキュメントを1 Issueあたり最大2件に制限しており、さらに優先度をhas_design > has_workplan > has_reviewとしているため、designとwork-planが存在するIssueではreview summaryは選ばれない。現状の記述だと、期待結果・優先度・上限制御が相互に矛盾している。", + "suggestion": "1 Issueあたり何を代表文書として残すのかを明確に再定義してください。例えばMAX_KG_DOCS_PER_ISSUEを4にする、あるいは2のままにして期待結果サンプルをdesign + work-planの2件に修正する、またはrelationごとの最低1件保証ルールに変える、のいずれかに揃えるべきです。" + }, + { + "id": "MF-2", + "title": "「5-10件程度の提案」という期待値を現在の実装方針だけでは保証できない", + "description": "suggestはKGステップを先頭に追加した後も、既存のBM25 / related / impact / semantic / additional contextの各ステップをそのまま出す実装である。Issue本文の方針はKG由来ドキュメント数しか制限していないため、複数Issueが抽出された場合はMAX_ISSUE_NUMBERS = 3によりissueコマンドだけで最大3件、KG documentが最大6件、さらに既存戦略が数件追加され、全体で10件を超えうる。問題文の「80件から5-10件へ」という期待に対し、受け入れ基準が全体件数を拘束していない。", + "suggestion": "受け入れ基準に『全提案数』と『KG由来ステップ数』のどちらを制御対象とするかを明記してください。全提案数も抑えたいなら、KG追加後にstrategy全体へ上限をかけるか、複数Issue時のissue/contextの総数上限を別途定義する必要があります。" + } + ], + "should_fix": [ + { + "id": "SF-1", + "title": "既存のfind_documents_by_issue()を踏まえた実装方針の比較が不足している", + "description": "Issue本文ではKnowledgeDocResultにdoc_subtypeがないためfile_path.ends_with(\"summary-report.md\")で判定するとしているが、既存コードベースにはSymbolStore::find_documents_by_issue()があり、こちらはdocumentノード限定でIssueDocumentEntry { relation, doc_subtype }を返す。つまり『現在のfind_knowledge_by_issue()を前提にsuggest.rs内でフィルタする』案は成立する一方で、『Issueごとにfind_documents_by_issue()を使ってfile nodeを最初から含めない』実装も既存パターンとして選べる。", + "suggestion": "Issue本文に、find_documents_by_issue()を使わずfind_knowledge_by_issue() + フィルタを採る理由を明記するか、逆にfind_documents_by_issue()を使う案を第一候補として再検討してください。" + }, + { + "id": "SF-2", + "title": "テスト要件がユニットテスト中心で、E2E観点が不足している", + "description": "今回の不具合はsuggestの出力件数とコマンド列に直接表れるユーザー向け挙動の問題である。Issue本文の受け入れ基準はfilter_and_limit_kg_docs()のユニットテストに寄っているが、prepend_knowledge_steps()と組み合わせた結果としてsuggest出力が本当に縮小され、modifiesやstage別reviewが出てこないことまでは保証していない。", + "suggestion": "受け入れ基準にE2Eテストを追加してください。少なくとも『Issue付きクエリでsuggestを実行したとき、issue --format jsonが出ること』『不要なcontextが含まれないこと』『件数が想定上限内であること』を確認するテストがあると妥当です。" + }, + { + "id": "SF-3", + "title": "受け入れ基準で「優先的に含まれる」の意味が曖昧", + "description": "機能要件にhas_design、has_workplanのドキュメントが優先的に含まれることとあるが、これは『存在すれば必ず採用される』のか、『reviewより前に並ぶ』のか、『上限制御に達した場合にreviewより先に残る』のかが読み手により解釈できる。", + "suggestion": "受け入れ基準を結果ベースで書き換えてください。例: 『1 Issue内でdesignがあれば最優先で採用される』『work-planがあればreviewより先に採用される』『上限制御により除外されうるrelationは何か』を明文化すると実装判断がぶれません。" + } + ], + "nice_to_have": [ + { + "id": "NH-1", + "title": "relation_priorityの扱いをissueコマンドの並び順との差分まで含めて補足すると読みやすい", + "description": "Issue本文はbefore_change.rsの優先度順を踏襲する方針になっているが、既存のissueコマンドはHasDesign > HasReview > HasWorkplan > HasProgressの並びで表示している。Suggestにおいてはbefore-changeの優先度を採ること自体はありうるが、既存コマンド間で並び順が異なる理由が説明されていない。", + "suggestion": "本Issueでは『調査開始地点の提示』を目的にbefore_change側の優先度を採用する、と一言補足しておくと、実装レビュー時の迷いが減ります。" + } + ], + "summary": "前回指摘のmodifies除外明記、受け入れ基準追加、before_change.rsパターン参照、doc_subtype制約の注記は概ね反映されている。一方で、今回のIssue本文にはまだ2つ大きな整合性問題がある。第一に、期待される代表文書の例とMAX_KG_DOCS_PER_ISSUE = 2 / has_design > has_workplan > has_reviewが両立していない。第二に、KG由来文書数だけを制限してもsuggest全体件数は5-10件に収まる保証がない。加えて、既存のfind_documents_by_issue()を使う案との比較がないこと、E2E観点の受け入れ基準が不足していること、優先採用ルールの文言が曖昧なことは再修正した方がよい。" +} diff --git a/dev-reports/issue/167/issue-review/stage6-apply-result.json b/dev-reports/issue/167/issue-review/stage6-apply-result.json new file mode 100644 index 0000000..d497477 --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage6-apply-result.json @@ -0,0 +1,14 @@ +{ + "stage": 6, + "action": "apply_codex_review", + "applied_items": ["MF-1", "MF-2", "SF-1", "SF-2", "SF-3", "NH-1"], + "changes": [ + "MAX_KG_DOCS_PER_ISSUEを2から4に変更", + "KG由来ステップ数の制御対象と上限計算式を明記", + "KG文書取得APIをfind_documents_by_issue()に変更", + "E2Eテスト要件を受け入れ基準に追加", + "「優先的に含まれる」をrelation_priority順で明確化", + "before_change.rs優先度採用理由を補足" + ], + "issue_updated": true +} diff --git a/dev-reports/issue/167/issue-review/stage7-review-context.json b/dev-reports/issue/167/issue-review/stage7-review-context.json new file mode 100644 index 0000000..2936826 --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage7-review-context.json @@ -0,0 +1,51 @@ +{ + "must_fix": [ + { + "id": "MF-1", + "title": "doc_subtype == \"summary\" という判定条件が既存スキーマと整合していない", + "description": "既存コードベースのDocSubtypeはDesignPolicy / WorkPlan / IssueReview / DesignReview / ProgressReport / StageReviewであり、\"summary\"という値は存在しない。find_documents_by_issue()が返すIssueDocumentEntryでもsummary判定はdoc_subtype単独では表現されず、IssueReviewとDesignReviewがsummary-report.md系、StageReviewがstage別レビューを表している。現状のIssue本文の受け入れ基準と改善案のままだと、実装不能または誤実装になる。", + "suggestion": "has_reviewの保持条件はdoc_subtype == IssueReview || doc_subtype == DesignReviewに、除外条件はdoc_subtype == StageReviewと明記してください。受け入れ基準のdoc_subtype == \"summary\"という表現も同様に修正すべきです。" + }, + { + "id": "MF-2", + "title": "find_documents_by_issue()を使う実装フローが複数Issue対応と整合していない", + "description": "suggestは既存実装でクエリから最大MAX_ISSUE_NUMBERS = 3件のIssue番号を抽出する。一方、Issue本文の処理フローはfind_documents_by_issue()を単数APIとして書いており、複数Issueをどう集約するかが記載されていない。find_knowledge_by_issue(&[...])からfind_documents_by_issue(issue)へ切り替えるなら、Issueごとに個別呼び出しして結果へissue_numberを再付与しない限り、現行のprepend_knowledge_steps()相当の処理やIssue単位制限の前提が崩れる。", + "suggestion": "処理フローを『抽出した各Issue番号についてfind_documents_by_issue(issue)を呼び、issue_numberを付与したsuggest用DTOに変換してからfilter_and_limit_kg_docs()に渡す』と具体化してください。もしくは複数Issue対応の新APIを用意する方針に改めてください。" + } + ], + "should_fix": [ + { + "id": "SF-1", + "title": "suggestのKG取得経路変更の回帰確認を明記すべき", + "description": "find_documents_by_issue()へ切り替える場合、影響は単なるsuggest.rs内フィルタ追加に留まらず、KG取得部分のデータ構造変換、複数Issue集約まで及ぶ。『他コマンドへの影響なし（確認済み）』は少し強すぎる。", + "suggestion": "『他コマンドの出力仕様には影響しないが、suggestのKG取得経路は変更されるため、Issue抽出・KG取得・ステップ生成の回帰確認が必要』と修正。" + }, + { + "id": "SF-2", + "title": "DocSubtypeの解釈がissueコマンドと矛盾しないことの確認", + "description": "find_documents_by_issue()はissueコマンドでも使われている既存API。DocSubtypeの解釈をsuggest側で誤ると仕様のズレが生じる。", + "suggestion": "DocSubtype::IssueReview / DesignReview / StageReviewの扱いがissueコマンドの分類と矛盾しないことを確認するテストまたは観点を追記。" + }, + { + "id": "SF-3", + "title": "パフォーマンス影響: SQLiteクエリ回数増加を明記", + "description": "find_knowledge_by_issue()は複数Issueを1クエリで取得できたが、find_documents_by_issue()をIssueごとに呼ぶため最大3回のSQLiteクエリになる。", + "suggestion": "パフォーマンス影響欄に『SQLiteクエリは最大1回→最大3回に増えるが、対象Issue数はMAX_ISSUE_NUMBERSで上限3、取得文書数も小さいため実用上の影響は軽微』と明記。" + } + ], + "nice_to_have": [ + { + "id": "NH-1", + "title": "依存関係への影響を具体化", + "description": "外部依存の追加なし。内部依存としてsuggestがIssueDocumentEntry / DocSubtypeを参照するようになる。", + "suggestion": "依存関係の変更を明記。" + }, + { + "id": "NH-2", + "title": "E2E受け入れ基準の「15-20件程度」は幅が広く不安定", + "description": "既存戦略ステップ数はqueryやembeddingの有無で変動するため、E2Eで全体件数をざっくり15-20と置くと将来壊れやすい。", + "suggestion": "E2Eでは『KG由来ステップ数が15以下』『1 Issueあたりcontextが4以下』を主判定にする。" + } + ], + "summary": "find_documents_by_issue()を使う方針は前進だが、doc_subtype == \"summary\"という判定は既存DocSubtype定義と一致しておらず誤り。単数APIの複数Issue適用方法が未定義。影響範囲はsuggestのKG取得経路に限定、他コマンドの仕様変更なし。テストはsuggestのユニット/E2E追加が必要。パフォーマンスはクエリ回数が最大3回に増えるが軽微。" +} diff --git a/dev-reports/issue/167/issue-review/stage8-apply-result.json b/dev-reports/issue/167/issue-review/stage8-apply-result.json new file mode 100644 index 0000000..c7dbc9d --- /dev/null +++ b/dev-reports/issue/167/issue-review/stage8-apply-result.json @@ -0,0 +1,15 @@ +{ + "stage": 8, + "action": "apply_codex_impact_review", + "applied_items": ["MF-1", "MF-2", "SF-1", "SF-2", "SF-3", "NH-1", "NH-2"], + "changes": [ + "doc_subtype判定をIssueReview/DesignReview(保持) vs StageReview(除外)に修正", + "複数Issue集約フロー具体化（各Issue個別呼び出し→DTO変換→結合→フィルタ）", + "suggestのKG取得経路変更の回帰確認必要と明記", + "DocSubtype解釈の整合確認テスト観点を追加", + "パフォーマンス影響セクション新設（SQLiteクエリ最大3回）", + "内部依存変更（IssueDocumentEntry/DocSubtype参照）を明記", + "E2E基準をKG由来ステップ数主体に" + ], + "issue_updated": true +} diff --git a/dev-reports/issue/167/issue-review/summary-report.md b/dev-reports/issue/167/issue-review/summary-report.md new file mode 100644 index 0000000..f1d63b6 --- /dev/null +++ b/dev-reports/issue/167/issue-review/summary-report.md @@ -0,0 +1,45 @@ +# マルチステージIssueレビュー サマリーレポート + +## Issue #167: suggestコマンドのナレッジグラフ展開が過剰（80件提案） + +### レビュー概要 + +| ステージ | 種別 | 実行エージェント | Must Fix | Should Fix | Nice to Have | +|---|---|---|---|---|---| +| 0.5 | 仮説検証 | Claude | - | - | - | +| 1 | 通常レビュー（1回目） | Claude Opus | 2 | 4 | 2 | +| 2 | 指摘反映（1回目） | Claude Sonnet | - | - | - | +| 3 | 影響範囲レビュー（1回目） | Claude Opus | 2 | 3 | 4 | +| 4 | 指摘反映（1回目） | Claude Sonnet | - | - | - | +| 5 | 通常レビュー（2回目） | Codex (gpt-5.4) | 2 | 3 | 1 | +| 6 | 指摘反映（2回目） | Claude Sonnet | - | - | - | +| 7 | 影響範囲レビュー（2回目） | Codex (gpt-5.4) | 2 | 3 | 2 | +| 8 | 指摘反映（2回目） | Claude Sonnet | - | - | - | + +### 仮説検証結果 + +**Confirmed**: suggestコマンドの`prepend_knowledge_steps()`がフィルタリングなしで全ドキュメントを展開している。 + +### 主要な指摘と反映内容 + +#### 1回目レビュー（Claude Opus） +- **受け入れ基準の追加**: Issue本文に具体的な受け入れ基準セクションを新設 +- **modifies除外の明示**: retain()によるフィルタリング除外を明記 +- **フィルタリングレイヤー設計**: prepend_knowledge_steps()の前段で実施する方針を確定 +- **MAX_KG_DOCS_PER_ISSUE定数の導入**: Issue単位の上限制御を明確化 +- **テスト要件の追加**: filter_and_limit_kg_docs()のユニットテスト項目を追加 + +#### 2回目レビュー（Codex） +- **MAX_KG_DOCS_PER_ISSUE矛盾の解消**: 2→4に変更し、期待結果の4件と整合 +- **doc_subtype判定の修正**: "summary"ではなくIssueReview/DesignReview(保持) vs StageReview(除外)に修正 +- **KG文書取得APIの変更**: find_knowledge_by_issue()→find_documents_by_issue()に変更（doc_subtype取得可能） +- **複数Issue集約フローの具体化**: 各Issue個別呼び出し→DTO変換→結合→フィルタ +- **E2Eテスト要件の追加**: 提案数制御、modifies除外、Issue単位上限の3項目 + +### 最終Issue状態 + +Issue #167 は全8ステージのレビューを経て更新済み。以下が確定: +- フィルタリング戦略: modifies/has_progress/StageReview除外、IssueReview/DesignReview保持 +- 上限制御: MAX_KG_DOCS_PER_ISSUE = 4、relation_priorityによるソート +- KG文書取得: find_documents_by_issue() API使用（doc_subtype対応） +- テスト: ユニットテスト5項目 + E2Eテスト3項目 + DocSubtype整合確認 diff --git a/dev-reports/issue/167/multi-stage-design-review/stage1-apply-result.json b/dev-reports/issue/167/multi-stage-design-review/stage1-apply-result.json new file mode 100644 index 0000000..15514cf --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage1-apply-result.json @@ -0,0 +1 @@ +{"stage": "1-4", "action": "apply_all_reviews", "applied_items": ["S1-MF1","S1-MF2","S2-MF1","S2-MF2","S3-MF1","S1-SF1","S1-SF2","S1-SF3","S2-SF1","S2-SF2","S2-SF3","S3-SF3","S4-SF1"], "design_policy_updated": true} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage1-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage1-review-context.json new file mode 100644 index 0000000..2d3fd1d --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage1-review-context.json @@ -0,0 +1,18 @@ +{ + "stage": 1, + "type": "design_principles", + "must_fix": [ + {"id": "MF-1", "title": "ProgressReport DocSubtypeのフィルタ条件での扱いが未明記"}, + {"id": "MF-2", "title": "relation_priorityの重複実装（DRY違反）"} + ], + "should_fix": [ + {"id": "SF-1", "title": "SuggestKgDocの導入が既存型と責務重複（KISS）"}, + {"id": "SF-2", "title": "find_documents_by_issue()のループ呼び出しがN+1問題（KISS）"}, + {"id": "SF-3", "title": "MAX_KG_DOCS_PER_ISSUE=4の根拠が複数文書ケースで不整合"} + ], + "nice_to_have": [ + {"id": "NH-1", "title": "eprintlnのログレベル分離"}, + {"id": "NH-2", "title": "境界値テストの追加"}, + {"id": "NH-3", "title": "prepend_knowledge_stepsの引数型変更のOCP懸念"} + ] +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage2-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage2-review-context.json new file mode 100644 index 0000000..6bf0ed5 --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage2-review-context.json @@ -0,0 +1,17 @@ +{ + "stage": 2, + "type": "consistency", + "must_fix": [ + {"id": "MF-1", "title": "SuggestKgDocへの変換ロジックが設計書に明示されていない"}, + {"id": "MF-2", "title": "prepend_knowledge_stepsの型変更に伴う既存テスト修正がテスト戦略に未記載"} + ], + "should_fix": [ + {"id": "SF-1", "title": "SymbolStore::open()のライフサイクル管理の明示"}, + {"id": "SF-2", "title": "kg_relation_priorityでフィルタ除外済みの値に優先度定義"}, + {"id": "SF-3", "title": "run_suggest()内のフロー変更箇所の明示化"} + ], + "nice_to_have": [ + {"id": "NH-1", "title": "HashMapのuse宣言追加の注記"}, + {"id": "NH-2", "title": "relation_priority値の整合性コメント"} + ] +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage3-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage3-review-context.json new file mode 100644 index 0000000..7ce088e --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage3-review-context.json @@ -0,0 +1,17 @@ +{ + "stage": 3, + "type": "impact_analysis", + "must_fix": [ + {"id": "MF-1", "title": "既存ユニットテスト3件がSuggestKgDoc型変更でコンパイルエラー"} + ], + "should_fix": [ + {"id": "SF-1", "title": "SuggestKgDocへの変換でissue_number付与漏れリスク"}, + {"id": "SF-2", "title": "kg_relation_priorityのHasProgress/Modifiesが到達不能"}, + {"id": "SF-3", "title": "find_documents_by_issue()のエラーハンドリング方針が未記載"} + ], + "nice_to_have": [ + {"id": "NH-1", "title": "relation_priority重複の技術的負債記録"}, + {"id": "NH-2", "title": "E2Eテストのフィクスチャ追加必要性"}, + {"id": "NH-3", "title": "SymbolStoreの複数回オープン非効率性"} + ] +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage4-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage4-review-context.json new file mode 100644 index 0000000..5412fbb --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage4-review-context.json @@ -0,0 +1,14 @@ +{ + "stage": 4, + "type": "security", + "must_fix": [], + "should_fix": [ + {"id": "SF-1", "title": "shell_quoteにNULバイトガード追加を推奨"}, + {"id": "SF-2", "title": "SQLiteからのfile_pathにパスサニティチェック推奨"} + ], + "nice_to_have": [ + {"id": "NH-1", "title": "MAX_INPUT_LENGTHがバイト数判定（文字数ではない）"}, + {"id": "NH-2", "title": "SQL IN句構築がformat!使用（安全だが注意）"}, + {"id": "NH-3", "title": "shell_quoteの型安全ラッパー検討"} + ] +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage5-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage5-review-context.json new file mode 100644 index 0000000..dea5da1 --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage5-review-context.json @@ -0,0 +1,55 @@ +{ + "must_fix": [ + { + "id": "MF-1", + "title": "SuggestKgDoc -> KnowledgeDocResult 変換例が既存API定義と不整合", + "description": "設計書4.6の変換例ではKnowledgeDocResult { issue_number, file_path, relation: d.relation.as_str().to_string() }となっているが、既存のKnowledgeDocResultはrelation: KnowledgeRelationとtitle: Optionを持つ。変換コードは型が合わず、titleも欠落している。", + "suggestion": "4.6をKnowledgeDocResult { issue_number: d.issue_number.clone(), relation: d.relation.clone(), file_path: d.file_path.clone(), title: None }に修正するか、prepend_knowledge_steps()をSuggestKgDoc受け取りに変更して中間DTO変換をなくす。" + }, + { + "id": "MF-2", + "title": "before_change.rsのrelation_priority()置換方針が未知値フォールバックを欠く", + "description": "既存のrelation_priority(&str)はunknown -> 5を返すフォールバックを持つ。parse()はOptionを返すため、そのまま置換すると未知値の扱いが未定義になる。", + "suggestion": "fn relation_priority_str(s: &str) -> u8 { KnowledgeRelation::parse(s).map_or(5, |r| r.priority()) } のような薄い互換ラッパーを残す方針に修正。" + }, + { + "id": "MF-3", + "title": "ctx.symbol_store_db_path()が存在しないAPI名", + "description": "設計書4.5ではctx.symbol_store_db_path()を使用しているが、既存SearchContextが持つのはsymbol_db_path()。", + "suggestion": "ctx.symbol_db_path()に修正。" + } + ], + "should_fix": [ + { + "id": "SF-1", + "title": "prepend_knowledge_stepsのシグネチャ維持はKISS観点で再検討余地", + "description": "中間DTOとしてSuggestKgDocとKnowledgeDocResultを相互変換する方針は回りくどい。prepend_knowledge_stepsが使うのはissue_numberとfile_pathだけ。", + "suggestion": "prepend_knowledge_stepsをSuggestKgDoc受け取りに変更するか、issue_numberとfile_pathだけの専用DTOに統一。" + }, + { + "id": "SF-2", + "title": "Issue順序がfilter後に不安定", + "description": "filter_and_limit_kg_docs()は全件をrelation.priority()でソート後にissue_orderを確定。Issue順が元のissue_numbersの順ではなく、最も優先度の高い文書の位置に依存。", + "suggestion": "Issue順はrun_suggest()で抽出したissue_numbersの順を維持し、各Issue内だけをpriority()で整列する方針に。" + }, + { + "id": "SF-3", + "title": "部分失敗時の可観測性とテスト観点が不足", + "description": "全Issue失敗時・一部Issue失敗時のユーザー体験、テスト方針が未定義。", + "suggestion": "『全Issue失敗時はKGなしで継続』『一部失敗時は成功分のみ採用』を明文化し、テスト方針を追記。" + } + ], + "nice_to_have": [ + { + "id": "NH-1", + "title": "SuggestKgDocにtitleを持たせない判断を明記", + "suggestion": "SuggestKgDocはステップ生成に必要な最小フィールドのみ保持し、titleはsuggestでは不要のため持たない、と一文補足。" + }, + { + "id": "NH-2", + "title": "priority()の配置理由にドメイン知識の帰属先を補足", + "suggestion": "4.4に『priorityはrelationに付随するドメインルールであり、CLI層ではなくドメイン型へ置く』と補足。" + } + ], + "summary": "find_documents_by_issue()採用、KnowledgeRelation::priority()共通化、部分失敗許容方針は概ね妥当。ただし実装不能レベルのAPI不整合が3点: (1) SuggestKgDoc->KnowledgeDocResult変換例の型不一致、(2) relation_priority置換での未知値フォールバック欠落、(3) ctx.symbol_store_db_path()という存在しないAPI名。" +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage6-apply-result.json b/dev-reports/issue/167/multi-stage-design-review/stage6-apply-result.json new file mode 100644 index 0000000..686c69d --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage6-apply-result.json @@ -0,0 +1 @@ +{"stage": 6, "action": "apply_codex_design_review", "applied_items": ["MF-1","MF-2","MF-3","SF-1","SF-2","SF-3","NH-1","NH-2"], "design_policy_updated": true} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage7-review-context.json b/dev-reports/issue/167/multi-stage-design-review/stage7-review-context.json new file mode 100644 index 0000000..a6e77a1 --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage7-review-context.json @@ -0,0 +1,13 @@ +{ + "stage": 7, + "type": "final_consistency", + "must_fix": [ + {"id": "MF-1", "title": "prepend_knowledge_stepsのstrategy引数型がVecと記載、正しくはVec"} + ], + "should_fix": [ + {"id": "SF-1", "title": "4.6の変更理由のrelation型不一致記述が誤り（両方KnowledgeRelation型）、titleのみが理由"} + ], + "nice_to_have": [ + {"id": "NH-1", "title": "sort_byが安定ソートであることをコメント明示"} + ] +} diff --git a/dev-reports/issue/167/multi-stage-design-review/stage8-apply-result.json b/dev-reports/issue/167/multi-stage-design-review/stage8-apply-result.json new file mode 100644 index 0000000..a869bba --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/stage8-apply-result.json @@ -0,0 +1 @@ +{"stage": 8, "action": "apply_final_review", "applied_items": ["MF-1","SF-1","NH-1"], "changes": ["strategy引数型をVecに修正","変更理由からrelation型不一致の誤記述を削除、titleのみを理由に","sort_byが安定ソートであるコメントを追加"], "design_policy_updated": true} diff --git a/dev-reports/issue/167/multi-stage-design-review/summary-report.md b/dev-reports/issue/167/multi-stage-design-review/summary-report.md new file mode 100644 index 0000000..bc7f1ab --- /dev/null +++ b/dev-reports/issue/167/multi-stage-design-review/summary-report.md @@ -0,0 +1,42 @@ +# マルチステージ設計レビュー サマリーレポート + +## Issue #167: suggestコマンドのナレッジグラフ展開制限 + +### レビュー概要 + +| Stage | 種別 | 実行エージェント | Must Fix | Should Fix | Nice to Have | +|---|---|---|---|---|---| +| 1 | 設計原則 | Claude Opus | 2 | 3 | 3 | +| 2 | 整合性 | Claude Opus | 2 | 3 | 2 | +| 3 | 影響分析 | Claude Opus | 1 | 3 | 3 | +| 4 | セキュリティ | Claude Opus | 0 | 2 | 3 | +| 5 | 設計原則(2回目) | Codex gpt-5.4 | 3 | 3 | 2 | +| 6 | 指摘反映 | Claude Sonnet | - | - | - | +| 7 | 整合性(2回目) | Claude Opus (Codex接続エラー代替) | 1 | 1 | 1 | +| 8 | 指摘反映 | Claude Sonnet | - | - | - | + +### 主要な修正事項 + +#### 1回目レビュー（Stage 1-4） +- `KnowledgeRelation::priority()` メソッド追加によるDRY改善 +- `ProgressReport` DocSubtypeのフィルタ条件の明記 +- `SuggestKgDoc → KnowledgeDocResult` 変換の設計明確化 +- 既存テスト影響の明記 +- エラーハンドリング方針の追記 +- セキュリティリスク認識の記載 + +#### 2回目レビュー（Stage 5-8） +- `prepend_knowledge_steps()` を `&[SuggestKgDoc]` 受け取りに変更（KISS改善） +- `before_change.rs` の `relation_priority()` を互換ラッパーとして残す設計（未知値フォールバック維持） +- `ctx.symbol_store_db_path()` → `ctx.symbol_db_path()` のAPI名修正 +- `strategy` 引数型の `Vec` → `Vec` 修正 +- `filter_and_limit_kg_docs()` にissue_numbers引数追加（Issue順序維持） +- 部分失敗時の方針明文化 + +### 最終設計方針書の状態 + +設計方針書は全8ステージのレビューを経て成熟。以下が確定: +- 変更対象: `suggest.rs`, `knowledge.rs`, `before_change.rs` +- 新規要素: `SuggestKgDoc` 構造体, `filter_and_limit_kg_docs()` 関数, `KnowledgeRelation::priority()` メソッド +- テスト: 新規ユニットテスト7件 + 既存テスト修正3件 + E2Eテスト2件 +- セキュリティ: 問題なし diff --git a/dev-reports/issue/167/work-plan.md b/dev-reports/issue/167/work-plan.md new file mode 100644 index 0000000..6aba996 --- /dev/null +++ b/dev-reports/issue/167/work-plan.md @@ -0,0 +1,106 @@ +# 作業計画: Issue #167 suggestコマンドのナレッジグラフ展開制限 + +## Issue: suggestコマンドのナレッジグラフ展開が過剰（80件提案） +**Issue番号**: #167 +**サイズ**: M +**優先度**: High +**依存Issue**: なし +**ブランチ**: `fix/issue-167-suggest-limit`（既存） + +## 設計方針書 + +`dev-reports/design/issue-167-suggest-limit-design-policy.md` + +## 詳細タスク分解 + +### Phase 1: コア実装 + +#### Task 1.1: KnowledgeRelation::priority() メソッド追加 +- **ファイル**: `src/indexer/knowledge.rs` +- **内容**: `KnowledgeRelation` impl に `pub fn priority(&self) -> u8` メソッドを追加 +- **依存**: なし +- **テスト**: `test_kg_relation_priority_order`（Task 2.1で作成） + +#### Task 1.2: before_change.rs の relation_priority() 互換ラッパー化 +- **ファイル**: `src/cli/before_change.rs` +- **内容**: 既存 `relation_priority(&str) -> u8` 関数の内部実装を `KnowledgeRelation::parse(s).map_or(5, |r| r.priority())` に変更 +- **依存**: Task 1.1 +- **テスト**: 既存テストが通ること（回帰確認） + +#### Task 1.3: SuggestKgDoc 構造体定義 +- **ファイル**: `src/cli/suggest.rs` +- **内容**: `SuggestKgDoc { issue_number, file_path, relation, doc_subtype }` 構造体追加、`MAX_KG_DOCS_PER_ISSUE = 4` 定数追加 +- **依存**: なし + +#### Task 1.4: filter_and_limit_kg_docs() 関数実装 +- **ファイル**: `src/cli/suggest.rs` +- **内容**: フィルタリング（Modifies/HasProgress/StageReview除外）、priority()ソート、Issue単位グルーピング・制限 +- **依存**: Task 1.1, Task 1.3 +- **テスト**: Task 2.1 の新規テスト7件 + +#### Task 1.5: query_knowledge_graph() の変更 +- **ファイル**: `src/cli/suggest.rs` +- **内容**: `find_knowledge_by_issue()` → `find_documents_by_issue()` ループ呼び出しに変更、`IssueDocumentEntry → SuggestKgDoc` 変換 +- **依存**: Task 1.3 +- **use追加**: `IssueDocumentEntry`, `DocSubtype` のインポート + +#### Task 1.6: prepend_knowledge_steps() の引数型変更 +- **ファイル**: `src/cli/suggest.rs` +- **内容**: 第2引数を `&[KnowledgeDocResult]` → `&[SuggestKgDoc]` に変更、内部ロジックは `issue_number` と `file_path` のみ参照するため大きな変更なし +- **依存**: Task 1.3 + +#### Task 1.7: run_suggest() の統合 +- **ファイル**: `src/cli/suggest.rs` +- **内容**: ステップ5とステップ10の間に `filter_and_limit_kg_docs()` 呼び出しを挿入、`KnowledgeDocResult` の use を整理 +- **依存**: Task 1.4, Task 1.5, Task 1.6 + +### Phase 2: テスト + +#### Task 2.1: 新規ユニットテスト（suggest.rs） +- **ファイル**: `src/cli/suggest.rs` (#[cfg(test)]) +- **テスト一覧**: + - `test_filter_removes_modifies` + - `test_filter_removes_has_progress` + - `test_filter_keeps_issue_review_removes_stage_review` + - `test_filter_keeps_design_and_workplan` + - `test_filter_limits_per_issue` + - `test_filter_empty_after_all_filtered` + - `test_kg_relation_priority_order` +- **依存**: Task 1.4 + +#### Task 2.2: 既存テスト修正（suggest.rs） +- **ファイル**: `src/cli/suggest.rs` (#[cfg(test)]) +- **内容**: `test_prepend_knowledge_steps_with_docs`, `_empty`, `_multiple_issues` のテストデータを `SuggestKgDoc` に変更 +- **依存**: Task 1.6 + +#### Task 2.3: 品質チェック +- `cargo build` +- `cargo clippy --all-targets -- -D warnings` +- `cargo test --all` +- `cargo fmt --all -- --check` + +## 実行順序 + +``` +Task 1.1 (knowledge.rs: priority()) + → Task 1.2 (before_change.rs: 互換ラッパー) + → Task 1.3 (suggest.rs: SuggestKgDoc + 定数) + → Task 1.4 (suggest.rs: filter_and_limit_kg_docs) + → Task 1.5 (suggest.rs: query_knowledge_graph変更) + → Task 1.6 (suggest.rs: prepend_knowledge_steps型変更) + → Task 1.7 (suggest.rs: run_suggest統合) + → Task 2.1 (新規テスト) + → Task 2.2 (既存テスト修正) + → Task 2.3 (品質チェック) +``` + +## Definition of Done + +- [ ] `KnowledgeRelation::priority()` メソッドが追加されている +- [ ] `before_change.rs` の `relation_priority()` が互換ラッパー化されている +- [ ] `filter_and_limit_kg_docs()` が実装されている +- [ ] suggestコマンドのKGステップ数が制限されている +- [ ] 新規テスト7件 + 既存テスト修正3件が全パス +- [ ] `cargo clippy --all-targets -- -D warnings` で警告0件 +- [ ] `cargo test --all` で全テストパス +- [ ] `cargo fmt --all -- --check` で差分なし diff --git a/src/cli/before_change.rs b/src/cli/before_change.rs index e9b579a..00e8792 100644 --- a/src/cli/before_change.rs +++ b/src/cli/before_change.rs @@ -328,15 +328,9 @@ fn findings_without_ranking(docs: &[KnowledgeDocResult]) -> Vec u8 { - match relation { - "has_design" => 0, - "has_workplan" => 1, - "has_review" => 2, - "has_progress" => 3, - "modifies" => 4, - _ => 5, - } + KnowledgeRelation::parse(relation).map_or(5, |r| r.priority()) } // --------------------------------------------------------------------------- diff --git a/src/cli/suggest.rs b/src/cli/suggest.rs index c6bb48f..3058ad9 100644 --- a/src/cli/suggest.rs +++ b/src/cli/suggest.rs @@ -8,15 +8,14 @@ Examples: commandindexdev suggest --for \"fix login bug\" --format json commandindexdev suggest --for \"refactor database layer\" --format path"; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::path::Path; -use std::collections::HashSet; - use crate::cli::search::SearchContext; -use crate::indexer::knowledge::extract_issue_numbers; +use crate::indexer::knowledge::{DocSubtype, KnowledgeRelation, extract_issue_numbers}; use crate::indexer::reader::IndexReaderWrapper; -use crate::indexer::symbol_store::{KnowledgeDocResult, SymbolStore}; +use crate::indexer::symbol_store::SymbolStore; use crate::output::{self, OutputFormat, SuggestResult, SuggestStep}; use crate::search::hybrid::rrf_merge_files; use crate::search::ranking; @@ -40,6 +39,17 @@ const DEDUP_FILE_LIMIT: usize = 5; /// ナレッジグラフ参照時の最大Issue番号数 const MAX_ISSUE_NUMBERS: usize = 3; +/// ナレッジグラフからのIssue単位最大ドキュメント数 +const MAX_KG_DOCS_PER_ISSUE: usize = 4; + +/// suggestコマンド用のKGドキュメントDTO +struct SuggestKgDoc { + issue_number: String, + file_path: String, + relation: KnowledgeRelation, + doc_subtype: DocSubtype, +} + // --------------------------------------------------------------------------- // Error type // --------------------------------------------------------------------------- @@ -218,37 +228,100 @@ fn maybe_add_semantic_step( /// ナレッジグラフからIssue関連文書を取得する。 /// symbols.db が存在しない場合や、マッチするIssueがない場合は空のVecを返す。 -fn query_knowledge_graph(ctx: &SearchContext, issue_numbers: &[String]) -> Vec { +fn query_knowledge_graph(ctx: &SearchContext, issue_numbers: &[String]) -> Vec { if issue_numbers.is_empty() { return Vec::new(); } - // symbols.db 存在チェック + let db_path = ctx.symbol_db_path(); if !db_path.exists() { return Vec::new(); } - // SymbolStore オープン（エラー時は空を返す） + + // SymbolStore::open() はループ外で1回だけ実行する（DB接続コスト削減） let store = match SymbolStore::open(&db_path) { Ok(s) => s, Err(e) => { - eprintln!("[suggest] knowledge graph query skipped: {e}"); + eprintln!("[suggest] knowledge graph open failed: {e}"); return Vec::new(); } }; - // クエリ実行（エラー時は空を返す） - match store.find_knowledge_by_issue(issue_numbers) { - Ok(results) => results, - Err(e) => { - eprintln!("[suggest] knowledge graph query failed: {e}"); - Vec::new() + + let mut all_docs = Vec::new(); + for issue_num in issue_numbers { + // 個別Issueのエラー時はそのIssueをスキップし、他のIssueの処理を継続する + match store.find_documents_by_issue(issue_num) { + Ok(entries) => { + for entry in entries { + all_docs.push(SuggestKgDoc { + issue_number: issue_num.clone(), + file_path: entry.file_path, + relation: entry.relation, + doc_subtype: entry.doc_subtype, + }); + } + } + Err(e) => { + eprintln!("[suggest] knowledge graph query failed for issue {issue_num}: {e}"); + continue; + } } } + all_docs +} + +/// ナレッジグラフドキュメントをフィルタリング・Issue単位制限する。 +/// +/// 1. modifies / has_progress / has_review(StageReview) を除外 +/// 2. relation_priority でソート +/// 3. Issue単位にグルーピングし MAX_KG_DOCS_PER_ISSUE 件に制限 +/// +/// issue_numbersの順序でIssueをグルーピングすることで、入力順を維持する。 +fn filter_and_limit_kg_docs( + docs: Vec, + issue_numbers: &[String], +) -> Vec { + // Step 1: フィルタリング + let mut filtered: Vec = docs + .into_iter() + .filter(|d| match d.relation { + KnowledgeRelation::Modifies => false, + KnowledgeRelation::HasProgress => false, + KnowledgeRelation::HasReview => { + matches!( + d.doc_subtype, + DocSubtype::IssueReview | DocSubtype::DesignReview + ) + } + KnowledgeRelation::HasDesign | KnowledgeRelation::HasWorkplan => true, + }) + .collect(); + + // Step 2: KnowledgeRelation::priority() でソート（sort_by は安定ソート） + filtered.sort_by(|a, b| a.relation.priority().cmp(&b.relation.priority())); + + // Step 3: Issue単位グルーピング + 上限制御 + let mut issue_groups: HashMap> = HashMap::new(); + for doc in filtered { + issue_groups + .entry(doc.issue_number.clone()) + .or_default() + .push(doc); + } + + let mut result = Vec::new(); + for issue_num in issue_numbers { + if let Some(docs) = issue_groups.remove(issue_num) { + result.extend(docs.into_iter().take(MAX_KG_DOCS_PER_ISSUE)); + } + } + result } /// ナレッジグラフ結果を戦略ステップとして先頭に挿入する。 fn prepend_knowledge_steps( strategy: &mut Vec, - kg_docs: &[KnowledgeDocResult], + kg_docs: &[SuggestKgDoc], matched_issues: &[String], ) { let mut kg_steps = Vec::new(); @@ -326,6 +399,9 @@ pub fn run_suggest( // 5. ナレッジグラフ参照（Issue番号がある場合のみ） let kg_docs = query_knowledge_graph(&ctx, &issue_numbers); + // 5.5. フィルタリング・Issue単位制限 + let kg_docs = filter_and_limit_kg_docs(kg_docs, &issue_numbers); + // 6. BM25検索 → ファイル単位dedup let bm25_results = reader .search(&query, BM25_SEARCH_LIMIT) @@ -598,17 +674,15 @@ mod tests { #[test] fn test_prepend_knowledge_steps_with_docs() { - use crate::indexer::knowledge::KnowledgeRelation; - let mut strategy = vec![SuggestStep { command: "existing_cmd".to_string(), reason: "existing_reason".to_string(), }]; - let kg_docs = vec![KnowledgeDocResult { + let kg_docs = vec![SuggestKgDoc { issue_number: "42".to_string(), - relation: KnowledgeRelation::HasDesign, file_path: "docs/design.md".to_string(), - title: Some("Design Doc".to_string()), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, }]; let matched_issues = vec!["42".to_string()]; @@ -635,7 +709,7 @@ mod tests { command: "existing_cmd".to_string(), reason: "existing_reason".to_string(), }]; - let kg_docs: Vec = vec![]; + let kg_docs: Vec = vec![]; let matched_issues: Vec = vec![]; prepend_knowledge_steps(&mut strategy, &kg_docs, &matched_issues); @@ -647,24 +721,22 @@ mod tests { #[test] fn test_prepend_knowledge_steps_multiple_issues() { - use crate::indexer::knowledge::KnowledgeRelation; - let mut strategy = vec![SuggestStep { command: "existing_cmd".to_string(), reason: "existing_reason".to_string(), }]; let kg_docs = vec![ - KnowledgeDocResult { + SuggestKgDoc { issue_number: "10".to_string(), - relation: KnowledgeRelation::HasDesign, file_path: "docs/design-10.md".to_string(), - title: Some("Design 10".to_string()), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, }, - KnowledgeDocResult { + SuggestKgDoc { issue_number: "20".to_string(), - relation: KnowledgeRelation::HasWorkplan, file_path: "docs/plan-20.md".to_string(), - title: Some("Plan 20".to_string()), + relation: KnowledgeRelation::HasWorkplan, + doc_subtype: DocSubtype::WorkPlan, }, ]; let matched_issues = vec!["10".to_string(), "20".to_string()]; @@ -759,4 +831,183 @@ mod tests { assert_eq!(arr[0], "42"); assert_eq!(arr[1], "100"); } + + // --- filter_and_limit_kg_docs tests --- + + #[test] + fn test_filter_removes_modifies() { + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "src/main.rs".to_string(), + relation: KnowledgeRelation::Modifies, + doc_subtype: DocSubtype::DesignPolicy, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design.md".to_string(), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert_eq!(result.len(), 1); + assert_eq!(result[0].file_path, "design.md"); + } + + #[test] + fn test_filter_removes_has_progress() { + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "progress.md".to_string(), + relation: KnowledgeRelation::HasProgress, + doc_subtype: DocSubtype::ProgressReport, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design.md".to_string(), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert_eq!(result.len(), 1); + assert_eq!(result[0].file_path, "design.md"); + } + + #[test] + fn test_filter_keeps_issue_review_removes_stage_review() { + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "issue-review.md".to_string(), + relation: KnowledgeRelation::HasReview, + doc_subtype: DocSubtype::IssueReview, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design-review.md".to_string(), + relation: KnowledgeRelation::HasReview, + doc_subtype: DocSubtype::DesignReview, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "stage-review.md".to_string(), + relation: KnowledgeRelation::HasReview, + doc_subtype: DocSubtype::StageReview, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert_eq!(result.len(), 2); + let paths: Vec<&str> = result.iter().map(|d| d.file_path.as_str()).collect(); + assert!(paths.contains(&"issue-review.md")); + assert!(paths.contains(&"design-review.md")); + assert!(!paths.contains(&"stage-review.md")); + } + + #[test] + fn test_filter_keeps_design_and_workplan() { + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design.md".to_string(), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "workplan.md".to_string(), + relation: KnowledgeRelation::HasWorkplan, + doc_subtype: DocSubtype::WorkPlan, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert_eq!(result.len(), 2); + } + + #[test] + fn test_filter_limits_per_issue() { + // Create 6 docs for one issue, should be limited to MAX_KG_DOCS_PER_ISSUE (4) + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design.md".to_string(), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "workplan.md".to_string(), + relation: KnowledgeRelation::HasWorkplan, + doc_subtype: DocSubtype::WorkPlan, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "issue-review.md".to_string(), + relation: KnowledgeRelation::HasReview, + doc_subtype: DocSubtype::IssueReview, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design-review.md".to_string(), + relation: KnowledgeRelation::HasReview, + doc_subtype: DocSubtype::DesignReview, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "design2.md".to_string(), + relation: KnowledgeRelation::HasDesign, + doc_subtype: DocSubtype::DesignPolicy, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "workplan2.md".to_string(), + relation: KnowledgeRelation::HasWorkplan, + doc_subtype: DocSubtype::WorkPlan, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert_eq!(result.len(), MAX_KG_DOCS_PER_ISSUE); + } + + #[test] + fn test_filter_empty_after_all_filtered() { + let docs = vec![ + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "src/main.rs".to_string(), + relation: KnowledgeRelation::Modifies, + doc_subtype: DocSubtype::DesignPolicy, + }, + SuggestKgDoc { + issue_number: "1".to_string(), + file_path: "progress.md".to_string(), + relation: KnowledgeRelation::HasProgress, + doc_subtype: DocSubtype::ProgressReport, + }, + ]; + let issues = vec!["1".to_string()]; + let result = filter_and_limit_kg_docs(docs, &issues); + assert!(result.is_empty()); + } + + #[test] + fn test_kg_relation_priority_order() { + assert!( + KnowledgeRelation::HasDesign.priority() < KnowledgeRelation::HasWorkplan.priority() + ); + assert!( + KnowledgeRelation::HasWorkplan.priority() < KnowledgeRelation::HasReview.priority() + ); + assert!( + KnowledgeRelation::HasReview.priority() < KnowledgeRelation::HasProgress.priority() + ); + assert!(KnowledgeRelation::HasProgress.priority() < KnowledgeRelation::Modifies.priority()); + } } diff --git a/src/indexer/knowledge.rs b/src/indexer/knowledge.rs index 4c4f22c..71e194c 100644 --- a/src/indexer/knowledge.rs +++ b/src/indexer/knowledge.rs @@ -98,6 +98,19 @@ impl KnowledgeRelation { } } + /// Relation priority for sorting (lower = higher priority). + /// HasProgress / Modifies はフィルタで除外されることが多いが、 + /// 型の網羅性（exhaustive match）を保証するために優先度を定義している。 + pub fn priority(&self) -> u8 { + match self { + Self::HasDesign => 0, + Self::HasWorkplan => 1, + Self::HasReview => 2, + Self::HasProgress => 3, + Self::Modifies => 4, + } + } + /// Parse a relation string from the database. Returns `None` for unknown values. pub fn parse(s: &str) -> Option { match s {