You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Scope :src/infrastructure/mcp/adapter.py — suppression des constantes _STREAMABLE_HTTP_TIMEOUT et _STREAMABLE_HTTP_SSE_READ_TIMEOUT, ainsi que des clés timeout / sse_read_timeout passées au server_config du transport streamable_http.
Analyse des changements
Le diff est minimal (1 fichier, -10 lignes) et cohérent dans son intention : revenir aux valeurs par défaut de la lib mcp / langchain-mcp-adapters au lieu de forcer des timeouts explicites.
👍 Points positifs
Diff chirurgical : un seul fichier touché, pas de changement de comportement collatéral.
Suppression de configuration implicite : on s'aligne sur la lib, donc moins de surface à maintenir quand la lib évolue.
Constantes devenues orphelines proprement retirées (pas de dead code laissé).
⚠️ Points d'attention
Pas de description de PR — le diff supprime un commentaire qui justifiait explicitement les valeurs choisies :
"Explicit values make client/server behaviour deterministic instead of relying on library defaults, and reduce the reconnect storms seen when the SSE GET stream drops between tool calls."
"Keep aligned with the server (mcp-raganything / fastmcp) SDK version."
Si on enlève les timeouts explicites, on perd cette protection. Il manque dans la PR :
la raison du retour aux défauts (bump de lib ? bug ? ),
la confirmation que les défauts actuels sont alignés avec mcp-raganything / fastmcp côté serveur.
Risque de régression silencieuse — le commentaire original mentionne des "reconnect storms" observés quand le stream SSE GET lâchait entre deux tool calls. Sans timeouts explicites, on dépend des défauts de la lib :
httpx default timeout = 5s (très court pour des tool calls MCP un peu lents).
sse_read_timeout default = 30s (vs 10 min avant, donc plus restrictif).
Si des tool calls RAG dépassent 30s côté mcp-raganything (multimodal, indexing), on risque des déconnexions en chaîne.
Cohérence client/serveur non vérifiée — la version de langchain-mcp-adapters et de mcp n'est pas bumpée dans cette PR. Si on s'appuie sur les défauts, encore faut-il qu'ils matchent la version SDK côté serveur.
Pas de test ajouté/modifié — même si la logique est triviale, un test qui vérifie l'absence des clés timeout / sse_read_timeout (ou la résilience à un tool call long) aurait valeur de garde-fou.
Score qualité du code
7 / 10
Le code en lui-même est propre et la suppression est cohérente. On perd 3 points sur :
absence de justification documentée dans la PR,
risque non discuté de régression sur les tool calls longs (RAG multimodal),
pas de test accompagnant le changement.
Suggestions d'amélioration
Compléter la description de la PR avec : lib bumpée ? problème résolu ? valeur des défauts actuels ?
Vérifier et documenter les défauts actuels de mcp SDK pour timeout et sse_read_timeout, et confirmer qu'ils matchent mcp-raganything / fastmcp.
Ajouter un test d'intégration : appel MCP sur un tool dont la réponse met >30s, vérifier qu'on n'a pas de coupure.
Si le but est juste de tester un bump de lib, envisager une PR séparée qui bump + ajuste les timeouts de manière ciblée plutôt que de tout supprimer.
Si le problème venait d'un conflit avec la lib, garder les constantes mais les passer via un wrapper qui catch l'erreur "unknown kwarg" — au lieu de tout retirer.
/cc @SoluDevTech/maintainers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.