BRIC-1: error handling - add timeout - add ut

[Guemri-Jawher]

PR review comments fix!

[Kaiohz]

Code Review — BRIC-1: Error handling + timeout + UT

✅ Points positifs

• Validation input solide : MessageRole enum, regex sur identifier, field_validator et model_validator Pydantic — c'est du bon travail
• Tenacity pour retry/timeout — nécessaire pour les calls Phoenix
• Tests ajoutés : 198 lignes de tests sur les use cases prompt
• Phoenix adapter refactoré (+94/-38) — plus propre

⚠️ Points bloquants

1. .gitignore doublon — .vscode/ est listé 2 fois + opencode.json aussi. À nettoyer avant merge.

2. Breaking change sur API request format : le format de content passe de list[dict[str, str]] à list[PromptMessage]. Tout client existant qui envoie {"content": [{"role": "user", "content": "..."}]} doit adapter.

   • Soit documenter le breaking change
   • Soit ajouter un field_validator qui accepte l'ancien format dict et le convertit en PromptMessage

⚠️ Points d'attention

3. Pas de test sur les routes modifiées — les routes prompt sont changées mais pas de test d'intégration sur l'API. Ajouter au minimum un test qui valide le schema de request.

4. Tenacity config — vérifier que les defaults (max_retries, wait策略) sont adaptés au contexte Phoenix. Pas de config visible dans le diff.

Score : 7/10

Bon travail sur la validation et le retry. Corriger le doublon .gitignore et clarifier le breaking change avant merge.

[Kaiohz]

Code Review — BRIC-1: Error handling, timeout, unit tests

+379 / -60 — 8 fichiers modifiés

✅ Points positifs

Architecture hexagonale respectée :

• PhoenixUnavailableError est une erreur domaine propre, pas une leak httpx dans le domaine ✅
• _wrap_phoenix_error() mappe les exceptions infrastructure (httpx) → erreurs domaine (ValueError, PhoenixUnavailableError) ✅
• _handle_http_error() dans les routes mappe les erreurs domaine → HTTP status codes ✅
• Le flow est clair : infrastructure → domaine → routes, chaque couche convertit

Error handling solide :

• Retry avec tenacity : 3 tentatives, exponential backoff (1s-10s), retry sur TimeoutException + ConnectError ✅
• HTTP status codes corrects : 201 pour create, 404 pour not found, 409 pour duplicate, 400 pour validation ✅
• except (ValueError, PhoenixUnavailableError): raise — re-raise les erreurs déjà wrappées sans double-wrap ✅
• Logging amélioré : f-strings → lazy %s, identifiers dans les messages ✅

Validation des inputs :

• PromptMessage avec enum MessageRole (system/user/assistant) ✅ — empêche les rôles invalides
• identifier avec regex ^[a-zA-Z0-9_-]+$ ✅ — empêche les injections
• content min_length=1 + model_validator (au moins un message user ou system) ✅
• UpdatePromptRequest.content : empty list → erreur ✅

Timeout configurable :

• httpx.Timeout(connect=10.0, read=timeout, write=timeout, pool=10.0) ✅
• Default 10s, configurable via constructeur ✅

Tests unitaires :

• 198 lignes de tests dans test_prompt_use_cases.py ✅
• CreatePromptUseCase : 3 tests (success, exception, optional fields)
• GetPromptUseCase : 4 tests (success, with version_id, with tag, not found)
• UpdatePromptUseCase : 4 tests (success, partial update, not found, exception)

Autres :

• @cached(cache=TTLCache) sur get_prompt_content — cache avec TTL 5min ✅
• Phoenix API prompts.tag → prompts.tags.create (API mise à jour) ✅
• Warning si description update non supporté par Phoenix ✅
• .gitignore nettoyé (.vscode, opencode.json)

⚠️ Points d'attention (non-bloquants)

1. update_prompt ne passe pas metadata au client Phoenix : le paramètre metadata est reçu dans la signature mais jamais transmis dans self._client.prompts.create(). C'est un bug — metadata est ignoré silencieusement sur les updates.

2. _phoenix_retry sur des méthodes async : tenacity fonctionne avec async, mais le décorateur est défini au niveau module avec retry=retry_if_exception_type((httpx.TimeoutException, httpx.ConnectError)). Si le client Phoenix lève une autre exception (ex: ValueError), pas de retry — c'est correct, mais à documenter.

3. get_prompt_content retourne dict[str, str] — ne retourne que le premier message. Le type de retour est limitant si on veut supporter les conversations multi-turn. Pas bloquant maintenant mais à garder en tête.

4. Pas de test d'intégration : les tests unitaires mockent le PromptManager. Pas de test contre une vraie instance Phoenix. Normal pour des UT, mais un test d'intégration serait un plus.

5. test_add_tag mock : manager._client.prompts.tag.create = MagicMock() — le mock est sur tags.create (pluriel) mais l'assertion est aussi sur tags.create. Cohérent, mais vérifier que l'API Phoenix utilise bien tags.create et pas tag.create.

6. functools.wraps importé mais non utilisé dans phoenix_prompt_adapter.py.

🔴 Bug potentiel

• update_prompt : metadata ignoré — le paramètre est dans la signature de la méthode mais n'est pas passé à self._client.prompts.create(). Si le front-end envoie du metadata à mettre à jour, il sera silencieusement perdu.

Score : 7/10

Bonne PR dans l'ensemble. L'error handling et la validation sont sérieux. Le retry avec tenacity est un vrai plus. Le bug sur metadata dans update_prompt est le point à corriger avant merge. Le reste est du cosmétique/amélioration future.

[Kaiohz]

Test from SoluBot - ignore this