Refactor vector search example to use sync API #14
Conversation
Summary of ChangesHello @MauricioPerera, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the vector search functionality to transition from an asynchronous to a synchronous programming model. The core changes involve updating the example script to remove Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Este pull request refactoriza el ejemplo de búsqueda vectorial y las pruebas asociadas para utilizar una nueva API síncrona, lo que simplifica el código al eliminar el uso de async/await y asyncio. Los cambios están bien implementados, incluyendo una capa de retrocompatibilidad en ATDFToolbox para soportar tanto el nombre del método de búsqueda antiguo como el nuevo. Las pruebas se han actualizado en consecuencia, aunque he identificado un par de áreas donde la cobertura de las pruebas podría mejorarse para evitar regresiones futuras.
| not VECTOR_DEPENDENCIES_AVAILABLE, | ||
| "Dependencias de búsqueda vectorial no instaladas", | ||
| ) | ||
| class TestVectorSearchIntegration(unittest.TestCase): |
There was a problem hiding this comment.
La prueba test_find_tools_without_scores_for_legacy_consumers ha sido eliminada en esta refactorización. Esta prueba verificaba una funcionalidad importante de find_tools_by_text: la opción return_scores=False, que es útil para consumidores de la API que no esperan puntuaciones. Dado que esta característica todavía existe en ATDFToolbox, sería bueno mantener la prueba para asegurar su correcto funcionamiento y evitar regresiones. Por favor, considera volver a añadirla.
| self.assertTrue(len(results) > 0) | ||
| # Verificar que al menos uno de los resultados es sobre correo | ||
| has_email_tool = any(tool.tool_id == "test_tool_1" for tool, _ in results) | ||
| self.assertTrue(all(isinstance(score, float) for _, score in results)) | ||
| self.assertTrue(has_email_tool) | ||
| mock_search_tools.assert_called_once() | ||
|
|
||
| def test_find_tools_without_scores_for_legacy_consumers(self): | ||
| """Permite recuperar solo herramientas para código legado.""" | ||
|
|
||
| results = self.toolbox.find_tools_by_text( | ||
| "correo", | ||
| use_vector_search=False, | ||
| return_scores=False, | ||
| ) | ||
|
|
||
| self.assertTrue(results) | ||
| self.assertTrue(all(isinstance(tool, ATDFTool) for tool in results)) | ||
| mock_search.assert_called_once() |
There was a problem hiding this comment.
Las aserciones en esta prueba se han debilitado en comparación con la versión anterior. Ahora solo se comprueba que la búsqueda de fallback devuelva algún resultado, pero no se verifica que el resultado sea el correcto. La implementación anterior validaba que se encontrara la herramienta esperada (test_tool_1). Sería beneficioso restaurar las aserciones más específicas para garantizar que la lógica de fallback funcione correctamente y evitar regresiones.
| self.assertTrue(len(results) > 0) | |
| # Verificar que al menos uno de los resultados es sobre correo | |
| has_email_tool = any(tool.tool_id == "test_tool_1" for tool, _ in results) | |
| self.assertTrue(all(isinstance(score, float) for _, score in results)) | |
| self.assertTrue(has_email_tool) | |
| mock_search_tools.assert_called_once() | |
| def test_find_tools_without_scores_for_legacy_consumers(self): | |
| """Permite recuperar solo herramientas para código legado.""" | |
| results = self.toolbox.find_tools_by_text( | |
| "correo", | |
| use_vector_search=False, | |
| return_scores=False, | |
| ) | |
| self.assertTrue(results) | |
| self.assertTrue(all(isinstance(tool, ATDFTool) for tool in results)) | |
| mock_search.assert_called_once() | |
| self.assertTrue(len(results) > 0) | |
| # Verificar que al menos uno de los resultados es sobre correo | |
| has_email_tool = any(tool.tool_id == "test_tool_1" for tool, _ in results) | |
| self.assertTrue(has_email_tool) | |
| mock_search.assert_called_once() |
1 participant
Summary
searchhelper and update ATDFToolbox to consume itTesting
https://chatgpt.com/codex/tasks/task_e_68e032d06894832d8d6d5dd8a1d431f9