484 lines
15 KiB
Markdown
484 lines
15 KiB
Markdown
# Code-Prüfung WP25b: Lazy-Prompt-Orchestration
|
||
|
||
**Datum:** 2026-01-02
|
||
**Version:** WP25b (Lazy Prompt Integration)
|
||
**Prüfer:** Auto (AI Code Review)
|
||
|
||
---
|
||
|
||
## 📋 Übersicht
|
||
|
||
Diese Prüfung analysiert die WP25b-Änderungen auf:
|
||
- **Schwachstellen** (Security, Error Handling, Robustness)
|
||
- **Inkonsistenzen** (Naming, Patterns, Architecture)
|
||
- **Verbesserungspotenzial** (Performance, Maintainability, Code Quality)
|
||
|
||
---
|
||
|
||
## 🔴 KRITISCHE SCHWACHSTELLEN
|
||
|
||
### 1. **Fehlende Validierung von `prompt_key` in `llm_service.py`**
|
||
|
||
**Datei:** `app/services/llm_service.py:169-176`
|
||
|
||
**Problem:**
|
||
```python
|
||
if prompt_key:
|
||
template = self.get_prompt(prompt_key, model_id=target_model, provider=target_provider)
|
||
try:
|
||
current_prompt = template.format(**(variables or {}))
|
||
except Exception as e:
|
||
logger.error(f"❌ Prompt formatting failed for key '{prompt_key}': {e}")
|
||
current_prompt = template # Sicherheits-Fallback
|
||
```
|
||
|
||
**Risiko:**
|
||
- Wenn `prompt_key` nicht existiert, gibt `get_prompt()` einen leeren String zurück
|
||
- Leerer Prompt wird an LLM gesendet → unerwartetes Verhalten
|
||
- Keine Warnung/Fehlerbehandlung für fehlende Keys
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
if prompt_key:
|
||
template = self.get_prompt(prompt_key, model_id=target_model, provider=target_provider)
|
||
if not template or not template.strip():
|
||
logger.error(f"❌ Prompt key '{prompt_key}' not found or empty. Available keys: {list(self.prompts.keys())}")
|
||
raise ValueError(f"Invalid prompt_key: '{prompt_key}'")
|
||
# ... rest of code
|
||
```
|
||
|
||
**Schweregrad:** 🔴 Hoch (kann zu stillem Fehlverhalten führen)
|
||
|
||
---
|
||
|
||
### 2. **Inkonsistenter Fallback in `decision_engine.py`**
|
||
|
||
**Datei:** `app/core/retrieval/decision_engine.py:258-261`
|
||
|
||
**Problem:**
|
||
```python
|
||
return await self.llm_service.generate_raw_response(
|
||
prompt=f"Beantworte: {query}\n\nKontext:\n{fallback_context}",
|
||
system=system_prompt, priority="realtime", profile_name=profile
|
||
)
|
||
```
|
||
|
||
**Risiko:**
|
||
- Fallback verwendet `prompt=` statt `prompt_key=` → inkonsistent mit WP25b-Architektur
|
||
- Keine Lazy-Loading-Vorteile (modell-spezifische Prompts werden ignoriert)
|
||
- Hardcodierter Prompt-String statt konfigurierbarer Template
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
return await self.llm_service.generate_raw_response(
|
||
prompt_key="fallback_synthesis", # In prompts.yaml definieren
|
||
variables={"query": query, "context": fallback_context},
|
||
system=system_prompt, priority="realtime", profile_name=profile
|
||
)
|
||
```
|
||
|
||
**Schweregrad:** 🟡 Mittel (funktional, aber architektonisch inkonsistent)
|
||
|
||
---
|
||
|
||
### 3. **Zu permissives Error-Handling in `ingestion_validation.py`**
|
||
|
||
**Datei:** `app/core/ingestion/ingestion_validation.py:77-80`
|
||
|
||
**Problem:**
|
||
```python
|
||
except Exception as e:
|
||
logger.warning(f"⚠️ Validation error for {target_id} using {profile_name}: {e}")
|
||
# Im Zweifel (Timeout/Fehler) erlauben wir die Kante, um Datenverlust zu vermeiden
|
||
return True
|
||
```
|
||
|
||
**Risiko:**
|
||
- **Alle** Fehler führen zu `return True` → ungültige Kanten werden akzeptiert
|
||
- Keine Unterscheidung zwischen transienten Fehlern (Timeout) und permanenten Fehlern (Invalid Config)
|
||
- Kann zu Graph-Verschmutzung führen
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
except Exception as e:
|
||
error_type = type(e).__name__
|
||
# Transiente Fehler (Timeout, Network) → erlauben
|
||
if any(x in str(e).lower() for x in ["timeout", "connection", "network"]):
|
||
logger.warning(f"⚠️ Transient error for {target_id}: {e}. Allowing edge.")
|
||
return True
|
||
# Permanente Fehler (Config, Validation) → ablehnen
|
||
logger.error(f"❌ Permanent validation error for {target_id}: {e}")
|
||
return False
|
||
```
|
||
|
||
**Schweregrad:** 🟡 Mittel (kann Graph-Qualität beeinträchtigen)
|
||
|
||
---
|
||
|
||
### 4. **Fehlende Validierung von YAML-Konfigurationen**
|
||
|
||
**Datei:** `app/core/retrieval/decision_engine.py:38-51`
|
||
|
||
**Problem:**
|
||
```python
|
||
def _load_engine_config(self) -> Dict[str, Any]:
|
||
path = os.getenv("MINDNET_DECISION_CONFIG", "config/decision_engine.yaml")
|
||
if not os.path.exists(path):
|
||
logger.error(f"❌ Decision Engine Config not found at {path}")
|
||
return {"strategies": {}}
|
||
try:
|
||
with open(path, "r", encoding="utf-8") as f:
|
||
config = yaml.safe_load(f) or {}
|
||
logger.info(f"⚙️ Decision Engine Config loaded (v{config.get('version', 'unknown')})")
|
||
return config
|
||
except Exception as e:
|
||
logger.error(f"❌ Failed to load decision_engine.yaml: {e}")
|
||
return {"strategies": {}}
|
||
```
|
||
|
||
**Risiko:**
|
||
- Keine Schema-Validierung → fehlerhafte YAML wird stillschweigend akzeptiert
|
||
- Fehlende Pflichtfelder (z.B. `strategies`, `streams_library`) führen zu Runtime-Fehlern
|
||
- Keine Warnung bei unbekannten Keys
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
def _load_engine_config(self) -> Dict[str, Any]:
|
||
# ... existing code ...
|
||
try:
|
||
with open(path, "r", encoding="utf-8") as f:
|
||
config = yaml.safe_load(f) or {}
|
||
|
||
# Schema-Validierung
|
||
required_keys = ["strategies", "streams_library"]
|
||
missing = [k for k in required_keys if k not in config]
|
||
if missing:
|
||
logger.error(f"❌ Missing required keys in config: {missing}")
|
||
return {"strategies": {}, "streams_library": {}}
|
||
|
||
# Warnung bei unbekannten Top-Level-Keys
|
||
known_keys = {"version", "settings", "strategies", "streams_library"}
|
||
unknown = set(config.keys()) - known_keys
|
||
if unknown:
|
||
logger.warning(f"⚠️ Unknown keys in config: {unknown}")
|
||
|
||
return config
|
||
except yaml.YAMLError as e:
|
||
logger.error(f"❌ YAML syntax error in {path}: {e}")
|
||
return {"strategies": {}, "streams_library": {}}
|
||
```
|
||
|
||
**Schweregrad:** 🟡 Mittel (kann zu Runtime-Fehlern führen)
|
||
|
||
---
|
||
|
||
## 🟡 INKONSISTENZEN
|
||
|
||
### 5. **Mix aus Deutsch und Englisch in Logs**
|
||
|
||
**Problem:**
|
||
- Logs enthalten sowohl deutsche als auch englische Nachrichten
|
||
- Inkonsistente Emoji-Nutzung (manchmal, manchmal nicht)
|
||
|
||
**Beispiele:**
|
||
- `logger.info(f"🎯 [ROUTING] Parsed Intent: '{intent}'...")` (Englisch)
|
||
- `logger.warning(f"⚠️ Profil '{profile_name}' nicht in llm_profiles.yaml gefunden!")` (Deutsch)
|
||
|
||
**Empfehlung:**
|
||
- Einheitliche Sprache wählen (empfohlen: Deutsch, da User-Regel "Always respond in German")
|
||
- Oder: Englisch für technische Logs, Deutsch für User-sichtbare Nachrichten
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Wartbarkeit)
|
||
|
||
---
|
||
|
||
### 6. **Unterschiedliche Config-Loading-Patterns**
|
||
|
||
**Problem:**
|
||
- `decision_engine.py`: Lädt Config bei jedem Aufruf (kein Cache)
|
||
- `chat.py`: Nutzt globalen Cache (`_DECISION_CONFIG_CACHE`)
|
||
- `llm_service.py`: Lädt Prompts/Profiles einmalig im `__init__`
|
||
|
||
**Empfehlung:**
|
||
- Einheitliches Pattern: Lazy Loading mit Cache
|
||
- Cache-Invalidierung bei Datei-Änderungen (optional, aber wünschenswert)
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Performance-Optimierung)
|
||
|
||
---
|
||
|
||
### 7. **Private Methoden werden extern genutzt**
|
||
|
||
**Datei:** `app/routers/chat.py:138`
|
||
|
||
**Problem:**
|
||
```python
|
||
intent = await llm.decision_engine._determine_strategy(query)
|
||
```
|
||
|
||
**Risiko:**
|
||
- Nutzung von `_determine_strategy()` (private Methode) von außen
|
||
- Verletzt Encapsulation-Prinzip
|
||
- Kann bei Refactoring brechen
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
# In decision_engine.py:
|
||
async def determine_strategy(self, query: str) -> str: # Public
|
||
return await self._determine_strategy(query)
|
||
|
||
# In chat.py:
|
||
intent = await llm.decision_engine.determine_strategy(query)
|
||
```
|
||
|
||
**Schweregrad:** 🟡 Mittel (Wartbarkeit)
|
||
|
||
---
|
||
|
||
### 8. **Inkonsistente Prompt-Formatierung**
|
||
|
||
**Problem:**
|
||
- `decision_engine.py:248-250`: Manuelle `prepend_instruction`-Anhängung
|
||
- Sollte eigentlich über `variables` im Template gehandhabt werden
|
||
|
||
**Empfehlung:**
|
||
- `prepend_instruction` als Variable in `prompts.yaml` Templates integrieren
|
||
- Entfernung der manuellen Anhängung
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Code-Qualität)
|
||
|
||
---
|
||
|
||
## 🟢 VERBESSERUNGSPOTENZIAL
|
||
|
||
### 9. **Fehlende Type Hints**
|
||
|
||
**Problem:**
|
||
- Viele Funktionen haben unvollständige oder fehlende Type Hints
|
||
- `Any` wird zu häufig verwendet
|
||
|
||
**Beispiele:**
|
||
- `app/routers/chat.py:116`: `async def _classify_intent(query: str, llm: LLMService) -> tuple[str, str]:`
|
||
- `app/services/llm_service.py:125`: Viele `Optional[...]` aber auch `Any`
|
||
|
||
**Empfehlung:**
|
||
- Vollständige Type Hints für alle öffentlichen Methoden
|
||
- Verwendung von `TypedDict` für Config-Strukturen
|
||
- `from __future__ import annotations` für Forward References
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Code-Qualität, IDE-Support)
|
||
|
||
---
|
||
|
||
### 10. **Code-Duplikation bei Config-Loading**
|
||
|
||
**Problem:**
|
||
- Ähnliche YAML-Loading-Logik in mehreren Dateien:
|
||
- `llm_service.py:_load_prompts()`
|
||
- `llm_service.py:_load_llm_profiles()`
|
||
- `decision_engine.py:_load_engine_config()`
|
||
- `chat.py:_load_decision_config()`
|
||
- `embeddings_client.py:_load_embedding_profile()`
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
# app/core/config_loader.py
|
||
def load_yaml_config(path: Path, required_keys: List[str] = None) -> Dict[str, Any]:
|
||
"""Zentrale YAML-Loading-Logik mit Validierung."""
|
||
# ... unified implementation ...
|
||
```
|
||
|
||
**Schweregrad:** 🟢 Niedrig (DRY-Prinzip)
|
||
|
||
---
|
||
|
||
### 11. **Fehlende Retry-Logik für Embedding-Requests**
|
||
|
||
**Datei:** `app/services/embeddings_client.py:83-96`
|
||
|
||
**Problem:**
|
||
- Embedding-Requests haben keine Retry-Logik
|
||
- Bei transienten Fehlern (Network, Timeout) wird sofort `[]` zurückgegeben
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
async def _request_embedding_with_client(self, client: httpx.AsyncClient, text: str, max_retries: int = 2) -> List[float]:
|
||
for attempt in range(max_retries + 1):
|
||
try:
|
||
# ... existing code ...
|
||
except (httpx.TimeoutException, httpx.NetworkError) as e:
|
||
if attempt < max_retries:
|
||
await asyncio.sleep(2 ** attempt)
|
||
continue
|
||
raise
|
||
```
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Resilienz)
|
||
|
||
---
|
||
|
||
### 12. **Potenzielle Race Condition bei Background-Semaphore**
|
||
|
||
**Datei:** `app/services/llm_service.py:39-42`
|
||
|
||
**Problem:**
|
||
- `_background_semaphore` wird als Klassen-Variable initialisiert
|
||
- Bei mehreren `LLMService`-Instanzen könnte es zu Race Conditions kommen
|
||
|
||
**Empfehlung:**
|
||
- Thread-safe Initialisierung mit `asyncio.Lock`
|
||
- Oder: Singleton-Pattern für `LLMService`
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Edge Case)
|
||
|
||
---
|
||
|
||
### 13. **Fehlende Validierung von `variables` in Prompt-Formatierung**
|
||
|
||
**Datei:** `app/services/llm_service.py:173`
|
||
|
||
**Problem:**
|
||
```python
|
||
current_prompt = template.format(**(variables or {}))
|
||
```
|
||
|
||
**Risiko:**
|
||
- Wenn Template Variablen erwartet, die nicht in `variables` sind → `KeyError`
|
||
- Keine Warnung über fehlende Variablen
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
if variables:
|
||
# Validierung: Prüfe ob alle benötigten Variablen vorhanden sind
|
||
import string
|
||
required_vars = set(string.Formatter().parse(template))
|
||
required_vars = {v[1] for v in required_vars if v[1] is not None}
|
||
missing = required_vars - set(variables.keys())
|
||
if missing:
|
||
logger.warning(f"⚠️ Missing variables in prompt '{prompt_key}': {missing}")
|
||
current_prompt = template.format(**(variables or {}))
|
||
else:
|
||
current_prompt = template
|
||
```
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Debugging-Hilfe)
|
||
|
||
---
|
||
|
||
### 14. **Ineffiziente String-Operationen**
|
||
|
||
**Datei:** `app/core/retrieval/decision_engine.py:248-250`
|
||
|
||
**Problem:**
|
||
```python
|
||
if prepend and prepend not in response[:len(prepend)+50]:
|
||
```
|
||
|
||
**Risiko:**
|
||
- String-Slicing bei jeder Antwort (auch wenn `prepend` leer ist)
|
||
- Ineffizient für lange Antworten
|
||
|
||
**Empfehlung:**
|
||
```python
|
||
if prepend and prepend.strip():
|
||
# Prüfe nur ersten Teil der Antwort
|
||
check_length = min(len(response), len(prepend) + 100)
|
||
if prepend not in response[:check_length]:
|
||
logger.info("ℹ️ Adding prepend_instruction manually (not found in response).")
|
||
response = f"{prepend}\n\n{response}"
|
||
```
|
||
|
||
**Schweregrad:** 🟢 Niedrig (Performance-Mikrooptimierung)
|
||
|
||
---
|
||
|
||
## 📊 ZUSAMMENFASSUNG
|
||
|
||
### Kritische Schwachstellen: 4
|
||
- 🔴 **Hoch:** Fehlende `prompt_key`-Validierung
|
||
- 🟡 **Mittel:** Inkonsistenter Fallback, Permissives Error-Handling, Fehlende YAML-Validierung
|
||
|
||
### Inkonsistenzen: 4
|
||
- 🟡 **Mittel:** Private Methoden-Nutzung
|
||
- 🟢 **Niedrig:** Sprach-Mix, Config-Patterns, Prompt-Formatierung
|
||
|
||
### Verbesserungspotenzial: 6
|
||
- 🟢 **Niedrig:** Type Hints, Code-Duplikation, Retry-Logik, Race Conditions, Variable-Validierung, String-Optimierung
|
||
|
||
---
|
||
|
||
## ✅ PRIORISIERTE EMPFEHLUNGEN
|
||
|
||
### Sofort (vor Merge):
|
||
1. ✅ **Prompt-Key-Validierung** hinzufügen (`llm_service.py`)
|
||
2. ✅ **Fallback konsistent** machen (`decision_engine.py`)
|
||
3. ✅ **YAML-Schema-Validierung** implementieren
|
||
|
||
### Kurzfristig (nächste Iteration):
|
||
4. ✅ **Error-Handling** in `ingestion_validation.py` differenzieren
|
||
5. ✅ **Public API** für `_determine_strategy()` erstellen
|
||
6. ✅ **Zentrale Config-Loader** implementieren
|
||
|
||
### Langfristig (Refactoring):
|
||
7. ✅ **Type Hints** vervollständigen
|
||
8. ✅ **Code-Duplikation** reduzieren
|
||
9. ✅ **Retry-Logik** für Embeddings
|
||
|
||
---
|
||
|
||
## 🎯 POSITIVE ASPEKTE
|
||
|
||
✅ **Gute Architektur:**
|
||
- Saubere Trennung von Concerns (Lazy Loading, MoE, Fallback-Kaskade)
|
||
- Modulare Struktur mit klaren Verantwortlichkeiten
|
||
|
||
✅ **Robustheit:**
|
||
- Umfassende Fallback-Mechanismen
|
||
- Background-Semaphore für Rate-Limiting
|
||
|
||
✅ **Dokumentation:**
|
||
- Ausführliche Code-Kommentare
|
||
- Versions-Tracking in Datei-Headern
|
||
|
||
✅ **Konsistenz:**
|
||
- Einheitliche Verwendung von `prompt_key` + `variables` (WP25b)
|
||
- Klare Profil-Steuerung über `llm_profiles.yaml`
|
||
|
||
---
|
||
|
||
**Status:** 🟡 **Bedingt genehmigt** - Kritische Fixes sollten vor Merge implementiert werden.
|
||
|
||
---
|
||
|
||
## 🔧 IMPLEMENTIERTE FIXES
|
||
|
||
### ✅ Fix 1: Prompt-Key-Validierung (`llm_service.py`)
|
||
- **Hinzugefügt:** Validierung, ob `prompt_key` existiert und nicht leer ist
|
||
- **Hinzugefügt:** Bessere Fehlermeldung mit verfügbaren Keys
|
||
- **Hinzugefügt:** Spezifische Behandlung von `KeyError` bei fehlenden Variablen
|
||
|
||
### ✅ Fix 2: Konsistenter Fallback (`decision_engine.py`)
|
||
- **Geändert:** Fallback nutzt nun `prompt_key="fallback_synthesis"` statt hardcodiertem Prompt
|
||
- **Hinzugefügt:** Fallback-Template in `prompts.yaml` (Zeile 429-447)
|
||
- **Hinzugefügt:** Graceful Degradation, falls Template nicht existiert
|
||
|
||
### ✅ Fix 3: YAML-Schema-Validierung (`decision_engine.py`)
|
||
- **Hinzugefügt:** Validierung der Pflichtfelder (`strategies`, `streams_library`)
|
||
- **Hinzugefügt:** Warnung bei unbekannten Top-Level-Keys
|
||
- **Hinzugefügt:** Spezifische Behandlung von `yaml.YAMLError`
|
||
|
||
### ✅ Fix 4: Differenziertes Error-Handling (`ingestion_validation.py`)
|
||
- **Geändert:** Unterscheidung zwischen transienten (Timeout, Network) und permanenten Fehlern
|
||
- **Verbessert:** Transiente Fehler → `return True` (Datenverlust vermeiden)
|
||
- **Verbessert:** Permanente Fehler → `return False` (Graph-Qualität schützen)
|
||
|
||
### 📝 TODO (Nicht kritisch, aber empfohlen):
|
||
- [ ] Public API für `_determine_strategy()` erstellen (`decision_engine.py`)
|
||
- [ ] Zentrale Config-Loader-Funktion implementieren
|
||
- [ ] Type Hints vervollständigen
|
||
- [ ] Retry-Logik für Embedding-Requests hinzufügen
|
||
|
||
---
|
||
|
||
**Status nach Fixes:** 🟢 **Genehmigt** - Kritische Probleme behoben, Code ist produktionsreif.
|