BRIC-18: Add MCP tool docstring warnings and simplify publish_section_version

[Kaiohz]

Jira

BRIC-18

Changes

• Add IMPORTANT docstring warnings to all 3 MCP tools
• Simplify publish_section_version: remove section_key, workflow_id, workflow_name, workflow_metadata from public params
• Reduce LLM hallucination surface area for parameter errors

Related: SoluDevTech/composable-agents#24

[Kaiohz]

Code Review — PR #38

📊 Score : 8/10

---

✅ Points positifs

1. Objectif clair et bien exécuté
La réduction de la surface d'erreur pour les LLMs est un vrai problème. Moins de params = moins d'hallucination sur les arguments. Le passage de 6 → 2 params sur publish_section_version est concret et mesurable.

2. Docstrings IMPORTANT bien pensées
Les avertissements IMPORTANT: This tool ONLY accepts ... Do NOT pass any other parameters sont directs et explicites — exactement ce qu'un LLM a besoin de lire pour éviter les hallucinations de params. C'est un pattern qui mérite d'être standardisé sur tous les MCP tools.

3. Hardcoding au bon endroit
section_key="consolidated_data" et workflow_id/workflow_name="agent-haiku-files-v1" sont des valeurs d'implémentation interne. Les exposer dans la signature du tool MCP n'apportait rien à l'appelant (LLM), ça créait de la confusion.

4. Tests bien mis à jour
Les tests suivent le mouvement proprement — les asserts vérifient bien les valeurs hardcodées et pas juste que ça passe.

---

⚠️ Points à améliorer / Suggestions

1. La valeur section_key="consolidated_data" est hardcodée — est-ce intentionnel ?
C'est la question clé de cette PR. Si section_key doit TOUJOURS être "consolidated_data" pour ce tool MCP, alors le hardcoder est la bonne décision. Mais si un appelant pourrait avoir besoin d'une section différente à l'avenir, il vaut mieux le garder en paramètre (même si c'est rare).

→ Suggestion : Ajouter un commentaire dans le code expliquant pourquoi cette valeur est hardcodée (ex: # Hardcoded: MCP tool only publishes consolidated data sections — see BRIC-18). Ça évitera à un futur dev de se demander s'il faut le remettre en paramètre.

2. Les 3 docstrings IMPORTANT sont identiques
Les 3 tools ont exactement le même format de warning : IMPORTANT: This tool ONLY accepts X and Y. Do NOT pass any other parameters (e.g., limit, offset, etc.) — they are not supported.

→ Suggestion : L'exemple (e.g., limit, offset, etc.) est générique et ne correspond pas aux params retirés. Pour publish_section_version, mentionner les params retirés serait plus précis : Do NOT pass section_key, workflow_id, workflow_name, or workflow_metadata — they are handled internally. Ça rend le warning plus actionnable pour le LLM.

3. Pas de newline à la fin du fichier
Le diff montre No newline at end of file → le fichier mcp_bricks_tools.py ne termine pas par une newline. C'est un détail mineur mais certains linters/CI l'exigent.

4. Test test_forward_all_params_to_use_case — le nom du test est devenu trompeur
Le test s'appelle toujours test_forward_all_params_to_use_case mais il ne forward plus les params — il vérifie que les defaults hardcodés sont passés. Un rename en test_forward_params_with_hardcoded_defaults serait plus descriptif.

---

🏁 Verdict

PR solide, bien ciblée, les tests suivent. Les suggestions ci-dessus sont des améliorations mineures (commentaire sur le hardcoding, docstring plus précise, newline, rename test). Aucun bloqueur.

Recommendation : Approve with minor suggestions 🚀

[Kaiohz]

See inline comments for specific suggestions.

[Kaiohz]

📌 Ce hardcoding est la bonne décision pour réduire la surface d'erreur LLM, mais un commentaire expliquant pourquoi serait utile pour les devs futurs :

# Hardcoded: MCP tool only publishes consolidated data sections.
# These params are internal implementation details, not caller-facing.

[Kaiohz]

💡 L'exemple (e.g., limit, offset, etc.) est générique. Pour ce tool spécifiquement, mentionner les params retirés serait plus précis pour le LLM :

IMPORTANT: This tool ONLY accepts project_unique_id and content.
Do NOT pass section_key, workflow_id, workflow_name, or workflow_metadata — they are handled internally and are not supported as parameters.

[Kaiohz]

Le nom du test test_forward_all_params_to_use_case est devenu trompeur — il ne forward plus "all params" mais vérifie les defaults hardcodés. Suggestion : renommer en test_forward_params_with_hardcoded_defaults.