Feat/remove builtin agents

[Kaiohz]

No description provided.

[Kaiohz]

📊 Code Confidence Score: 7/10

✅ Points positifs

Objectif clair et bien exécuté : Suppression du concept de "built-in agents" — les agents sont désormais tous créés dynamiquement via l'API, plus de seeding au démarrage. C'est la bonne direction pour un produit SaaS.

Architecture hexagonale respectée :

• is_builtin retiré proprement de l'entité domaine (AgentConfigMetadata), du modèle DB (AgentConfigModel), du repository adapter
• SeedAgentsUseCase et DeepAgentRegistry supprimés (code mort)
• Use cases create/update/delete nettoyés des guards is_builtin
• Migration Alembic 003_drop_is_builtin_column.py propre avec downgrade

Améliorations annexes utiles :

• chat.py : ajout d'un event done en fin de stream + error au lieu de raise — meilleure gestion SSE côté client
• stream_message.py : try/catch sur add_message après stream — évite de crasher si la persistance échoue après un stream réussi
• main.py : logging configuré avant les imports FastAPI (ordre correct), __main__ utilise settings.host/port
• Dockerfile : CMD passe de uvicorn direct à python -m src.main (cohérent avec __main__)
• pyproject.toml : dépendances Phoenix intégrées en dur (plus d'optional-dependencies fragmentées)

Tests mis à jour :

• test_agent_crud.py : suppression des tests builtin_protected (update/delete) — cohérent
• test_registry.py et test_seed_agents.py supprimés (code mort)
• test_routes.py, test_thread_management.py adaptés

55 fichiers changés, 203 additions, 4077 deletions — c'est un nettoyage massif, la majorité est la suppression des 29 fichiers YAML d'agents.

⚠️ Points d'attention

• main.py : logging configuré au niveau module (lignes 1-30) avant les imports FastAPI. Ça fonctionne mais c'est inhabituel — la config logging dans une factory function ou un module dédié serait plus propre. Le logging.basicConfig d'origine était aussi à ce niveau, donc c'est un déplacement, pas une régression.

• .gitignore : ajout de agents — ce dossier est maintenant ignoré. Mais si un développeur met des YAML dans agents/ localement, ils seront ignorés par git. OK puisque les agents sont censés venir de MinIO.

• pyproject.toml : Phoenix en dépendances obligatoires — arize-phoenix-otel==0.15.0 est maintenant obligatoire. Si quelqu'un n'utilise pas Phoenix, il installe quand même ces packages. Les optional-dependencies étaient plus flexibles.

• Dockerfile : plus de COPY agents/ — OK puisque les agents viennent de MinIO, mais vérifier que le déploiement K3S (flux repo) est cohérent.

• chat.py : raise remplacé par yield {"event": "error"} — l'erreur est absorbée silencieusement côté serveur. Le client reçoit un event error mais pas de log d'erreur à l'extérieur du logger.exception. Ça pourrait rendre le debugging plus difficile en production.

🔴 Risques identifiés

• Pas de migration de données : la migration 003 drop la colonne is_builtin mais ne fait pas de cleanup des données existantes. Si des agents is_builtin=True existent en DB, ils perdront leur flag mais resteront en base. Ce n'est probablement pas un problème (le flag n'existait que pour bloquer update/delete), mais ça mérite d'être documenté.

• main.py : deux instances de Settings() sont créées — une au niveau module (_settings = Settings()) et une dans le bloc __main__ (settings = Settings()). Si les vars d'env changent entre le startup et le run, ça peut causer une incohérence. Mieux vaut utiliser une seule instance.

Recommandation

1. Factoriser le logging setup dans un module dédié (ex: src/logging_config.py) plutôt qu'au niveau de main.py
2. Utiliser une seule instance de Settings() (pattern singleton ou dependency injection)
3. Ajouter un commentaire dans la migration 003 sur le comportement avec les données existantes is_builtin=True
4. Revenir sur les dépendances Phoenix obligatoires si certains déploiements n'en ont pas besoin

Bonne PR dans l'ensemble — le nettoyage est massif mais cohérent. Le concept de built-in agents était un couplage inutile pour un SaaS.

---

Review automatique par SoluBot 🤖

[Kaiohz]

🔴 Code Review — Remove builtin agents

⚠️ Points critiques

1. Où les agents seront-ils chargés au runtime ? Le COPY agents/ est retiré du Dockerfile mais il n'y a pas de documentation sur comment les agents YAML seront montés au runtime (volume? configmap? env var?).

2. real-estate-extractor (408 lignes) supprimé sans migration — C'est un agent complet avec des prompts métier. Un rollback ou une migration path devrait être documentée.

3. CI : --extra phoenix retiré dans ci.yaml → les tests Phoenix ne tourneront plus. Si c'est intentionnel, le mentionner dans la PR.

4. .gitignore ajoute agents/ → les agents ne sont plus versionnés dans le repo. Changement architectural majeur qui mérite explication.

💡 Suggestions

• Ajouter un AGENTS_DIR configurable (env var) pour le runtime mount
• Documenter le workflow de déploiement des agents YAML
• Considérer un dossier agents/examples/ versionné pour les exemples
• Mettre à jour le README

Score de confiance : 5/10

[Kaiohz]

🔴 Review: Remove builtin agents — Score 5/10

⚠️ Points critiques

1. 7 agents YAML supprimés (dont real-estate-extractor 408 lignes) sans documentation sur le plan de migration
2. COPY agents/ retiré du Dockerfile mais aucune doc sur le runtime mount (volume K8s? ConfigMap? env var?)
3. .gitignore ajoute agents/ → les agents ne sont plus versionnés dans le repo
4. CI : --extra phoenix retiré sans explication → les tests Phoenix ne tourneront plus
5. Migration Alembic 003 : drop is_builtin_column — OK mais le code qui lit is_builtin doit aussi être nettoyé
6. pyproject.toml : optional-dependencies phoenix/langfuse retirés → breaking change pour les users qui installent via pip

💡 Suggestions

• Documenter le workflow de déploiement des agents : Comment les agents YAML seront-ils montés au runtime ? (K8s ConfigMap, volume NFS, AGENTS_DIR configurable...)
• Conserver un dossier agents/examples/ versionné avec 1-2 exemples minimaux pour l'onboarding
• Mettre à jour le README : la section agents est probablement obsolète
• Séparer en 2 PRs : suppression des agents + refactor pyproject/CI — ce sont des changements orthogonaux

🔴 Blocking

Sans documentation du runtime mounting, cette PR casse le déploiement existant.

---

Review by SoluBot 🤖