| # 設計パターン詳細分析 |
|
|
| ## 概要 |
|
|
| 本ドキュメントでは、現在の `src/` に適用されている設計パターンをコードベースに基づいて詳細に解説し、問題点と改善案を記録する。 |
|
|
| --- |
|
|
| ## 1. Strategy パターン |
|
|
| ### 目的 |
| アルゴリズムをオブジェクトとして差し替え可能にする。 |
|
|
| ### 適用箇所 |
| ``` |
| src/subclaim_processor/strategies/ |
| ├── aggregation/ |
| │ ├── base.py → AggregationStrategy (ABC) |
| │ ├── mean.py → MeanAggregation |
| │ └── max.py → MaxAggregation |
| └── scoring/ |
| ├── base.py → ScoringStrategy (ABC) |
| └── product.py → ProductScoreStrategy |
| ``` |
|
|
| ### 実際の動作（`subclaim_scorer.py:46-68`） |
| ```python |
| AGGREGATION_STRATEGIES: Dict[str, Callable] = { |
| "max": MaxAggregation, |
| "mean": MeanAggregation, |
| } |
| SCORING_STRATEGIES: Dict[str, Callable] = { |
| "product": ProductScoreStrategy, |
| } |
| |
| def score(self, claim, retrieved_docs, aggregation_strategy, scoring_strategy): |
| agg_func = AGGREGATION_STRATEGIES[aggregation_strategy]() # "mean" or "max" |
| scoring_func = SCORING_STRATEGIES[scoring_strategy]() # "product" |
| |
| for doc in retrieved_docs: |
| score = scoring_func.compute_score(...) # 差し替え可能 |
| |
| return agg_func.aggregate(doc_scores) # 差し替え可能 |
| ``` |
| |
| ### 問題点 |
| Strategy の選択が文字列キーの辞書で行われており、型安全性がない。`"product"` を typo しても実行時まで気づけない。 |
|
|
| ### 改善案 |
| ```python |
| # Enum で型安全にする |
| from enum import Enum |
| |
| class AggregationMethod(Enum): |
| MEAN = "mean" |
| MAX = "max" |
| |
| # または Protocol を使う（Python 3.8+） |
| from typing import Protocol |
| |
| class AggregationStrategy(Protocol): |
| def aggregate(self, scores: list[float]) -> float: ... |
| ``` |
|
|
| --- |
|
|
| ## 2. Template Method パターン |
|
|
| ### 目的 |
| 処理の骨格を親クラスで定め、詳細を子クラスに委ねる。 |
|
|
| ### 適用箇所 |
| ``` |
| src/data_processor/raw_data_processor.py → IRawDataProcessor, DatasetProcessor (ABC) |
| src/calibration/base_calibration.py → ICalibration (ABC) |
| src/common/llm/llm_agent.py → LLMAgent (ABC) |
| ``` |
|
|
| ### 現在の実装（`raw_data_processor.py`） |
| ```python |
| class IRawDataProcessor(ABC): |
| @abstractmethod |
| def get_queries(self, input_file, output_file): pass |
| |
| @abstractmethod |
| def get_documents(self, query_file, output_file): pass |
| |
| class DatasetProcessor(ABC): |
| @abstractmethod |
| def process_queries(self, input_file, **kwargs) -> list: pass |
| |
| @abstractmethod |
| def process_documents(self, query_file, db, **kwargs) -> dict: pass |
| ``` |
|
|
| ### 問題点 |
| 抽象クラスが2つある（`IRawDataProcessor` と `DatasetProcessor`）。 |
|
|
| | クラス | 役割 | |
| |--------|------| |
| | `IRawDataProcessor` | ファイルI/O込みのインターフェース | |
| | `DatasetProcessor` | 純粋な処理ロジックのインターフェース | |
|
|
| `QueryProcessor` は `IRawDataProcessor` を継承しつつ、内部で `DatasetProcessor` を利用するというダブル構造になっている。 |
|
|
| 真の Template Method パターンなら、親クラスに「骨格メソッド」が実装されているはずだが、現在の実装では `QueryProcessor.get_queries()` が骨格を担っている。 |
|
|
| --- |
|
|
| ## 3. Factory / Dispatcher パターン |
|
|
| ### 目的 |
| 入力に基づいて適切なオブジェクトを生成・委譲する。 |
|
|
| ### 実際のコード（`query_processor.py:26-31`） |
| ```python |
| self.processors = { |
| "fact_score": FactScoreProcessor(), |
| "hotpot_qa": HotpotQAProcessor(), |
| "pop_qa": PopQAProcessor(), |
| "medlf_qa": MedLFQAProcessor(), |
| } |
| |
| # 使用時 |
| processor = self.processors.get(dataset) |
| ``` |
| |
| ### 問題点 |
| これは「Factory パターン」ではなく「Registry パターン」に近い。 |
| |
| | パターン | 特徴 | |
| |---------|------| |
| | Factory（正） | 必要時にオブジェクトを生成する | |
| | Registry（現在） | 起動時に全オブジェクトを一括生成して辞書に保持 | |
| |
| `QueryProcessor()` を作った時点で4データセット全プロセッサがインスタンス化される。実害は少ないが、設計の意図とずれている。 |
| |
| --- |
| |
| ## 4. Manager パターン |
| |
| ### 目的 |
| リソースのライフサイクル（生成・使用・破棄）を一元管理する。 |
| |
| ### 各 Manager の評価 |
| |
| | Manager | 管理対象 | 実際の責任数 | 評価 | |
| |---------|---------|------------|------| |
| | `ConfigManager` | YAMLファイル | 設定読み込み + ログセットアップ | △ 2責任 | |
| | `FAISSIndexManager` | FAISSインデックス | インデックス管理 + 検索 + 応答生成 | ✗ 3責任 | |
| | `FileManager` | ドキュメント | ファイル読み込み + チャンキング | △ 2責任 | |
| | `OpenAIManager` | Embeddings API | API呼び出しのみ | ✓ 適切 | |
| |
| ### 最大の問題：`FAISSIndexManager`（`faiss_manager.py:245-286`） |
| ```python |
| class FAISSIndexManager: |
| def upsert_file_to_faiss(...) # インデックス管理 ✓ |
| def search_faiss_index(...) # 検索 ✓ |
| def generate_response_from_context(...) # ← LLM応答生成（無関係） |
| def parse_result(...) # ← 文字列パース（雑多） |
| ``` |
| |
| `generate_response_from_context` はインデックス管理とは無関係なのに `FAISSIndexManager` に含まれている。この責任は `OpenAIRAGAgent` に移すべき。 |
|
|
| --- |
|
|
| ## 5. Pipeline アーキテクチャ |
|
|
| ### 目的 |
| 処理を段階的に分けて順次実行する。 |
|
|
| ### 現在のパイプライン |
| ``` |
| main.py |
| 1. DataLoader → 生データ取得 |
| 2. QueryProcessor → データ正規化 |
| 3. FAISSIndexManager → インデックス構築 |
| 4. SubclaimProcessor → 応答生成 → サブクレーム抽出 → スコアリング → アノテーション |
| 5. SplitConformalCalibration → 統計的キャリブレーション |
| ``` |
|
|
| ### 問題点：ステージ間の文字列シリアライズ（`faiss_manager.py:196-216`） |
| ```python |
| # 検索結果を文字列として返す |
| results.append( |
| f"{text} indice={idx} fileposition={relative_idx} score={dist:.4f}" |
| # TODO reformat this ← 本人コメントあり |
| ) |
| ``` |
| |
| その後 `parse_result()` で正規表現パースし直している： |
| ```python |
| pattern = re.compile( |
| r"page_content='(.*?)'\smetadata=(\{.*?\})\sindice=(\d+)\sfileposition=(\d+)\sscore=([\d.]+)", |
| re.DOTALL, |
| ) |
| ``` |
|
|
| ### 改善案 |
| dataclass でステージ間のデータ型を定義すれば `parse_result()` が不要になる： |
| ```python |
| @dataclass |
| class SearchResult: |
| text: str |
| indice: int |
| fileposition: int |
| score: float |
| ``` |
|
|
| --- |
|
|
| ## 6. Dependency Injection（DI） |
|
|
| ### 目的 |
| 依存オブジェクトを外から注入してテスト容易性を高める。 |
|
|
| ### 現在の実装（`subclaim_scorer.py:29-44`） |
| ```python |
| class SubclaimScorer(IDocumentScorer): |
| def __init__(self, index_truncation_config, embedding_model, index_path, ...): |
| self.faiss_manager = FAISSIndexManager(...) # ← 内部で直接生成 |
| self.gen = OpenAIAtomicFactGenerator() # ← 内部で直接生成 |
| self.openai_client = OpenAI() # ← 内部で直接生成 |
| ``` |
| |
| ### 問題点 |
| これは DI ではない。本当の DI は依存オブジェクトを外部から受け取る： |
| ```python |
| # DI（正） |
| class SubclaimScorer: |
| def __init__( |
| self, |
| faiss_manager: FAISSIndexManager, |
| fact_generator: OpenAIAtomicFactGenerator, |
| openai_client: OpenAI, |
| ): |
| self.faiss_manager = faiss_manager |
| ``` |
|
|
| 現在の設計では `SubclaimScorer` の単体テストに本物の OpenAI API と FAISS インデックスが必要になる。 |
|
|
| --- |
|
|
| ## 設計全体の評価サマリー |
|
|
| | パターン | 意図の正確さ | 実装の品質 | 改善優先度 | |
| |---------|------------|----------|----------| |
| | Strategy | ✓ 正しい | △ 文字列キーで型安全でない | 低（機能はしている） | |
| | Template Method | △ 混乱あり | △ 2つの抽象クラスが混在 | 中 | |
| | Factory/Dispatcher | △ Registry に近い | △ 起動時に全数生成 | 低 | |
| | Manager | △ 責任過多 | ✗ FAISSがLLM応答生成を担当 | 高 | |
| | Pipeline | ✓ 正しい | ✗ 文字列シリアライズ＋正規表現パース | 高 | |
| | Dependency Injection | ✗ 名前だけ | ✗ 内部生成で外部注入不可 | 中 | |
|
|
| ### 優先度「高」の改善項目（構造変更不要） |
|
|
| 1. **`FAISSIndexManager.generate_response_from_context` の移動** |
| - 移動先：`OpenAIRAGAgent`（すでに存在する） |
| - 効果：Manager の単一責任原則回復 |
| |
| 2. **パイプライン間データ型の構造化** |
| - `SearchResult` dataclass の導入 |
| - `parse_result()` の正規表現解析の廃止 |
| - 効果：バグ発生箇所の削減、デバッグ容易性の向上 |
| |
