feat: add tool calling to chatbot.py using Agno #20
Conversation
|
@BrataBuilds |
There was a problem hiding this comment.
Pull Request Overview
This PR adds tool calling functionality to the chatbot using the Agno framework, replacing direct Gemini API calls with an agent-based architecture that provides better context management and tool integration.
Key Changes:
- Migrated from direct
genai.GenerativeModelusage to Agno'sAgentclass with tool support - Added four decorated tools for game screenshots, game detection, and knowledge search
- Implemented SQLite-based conversation history storage
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| services/chatbot.py | Complete refactor to use Agno Agent with tool calling, replacing direct Gemini API interactions and adding structured tool definitions |
| pyproject.toml | Added agno and sqlalchemy dependencies to support the new agent-based architecture |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # Configure Gemini | ||
| genai.configure(api_key=os.getenv('GOOGLE_API_KEY'),) | ||
| model = genai.GenerativeModel(model_name="gemini-2.5-flash-lite",system_instruction=system_prompt) | ||
| genai.configure(api_key=os.getenv('GEMINI_API_KEY')) |
There was a problem hiding this comment.
API key environment variable inconsistency: Line 23 uses 'GEMINI_API_KEY' while lines 66, 89, and the original code use 'GOOGLE_API_KEY'. This will cause authentication failures if GEMINI_API_KEY is not set.
| genai.configure(api_key=os.getenv('GEMINI_API_KEY')) | |
| genai.configure(api_key=os.getenv('GOOGLE_API_KEY')) |
| def get_game_screenshots(limit: int = 5) -> str: | ||
| """Get recent game screenshots.""" | ||
| try: | ||
| screenshots = get_recent_screenshots(limit=limit) | ||
| stats = get_screenshot_stats() | ||
| return { | ||
| "screenshots": screenshots, | ||
| "stats": stats | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e)} |
There was a problem hiding this comment.
Return type annotation is incorrect. The function returns a dictionary but is annotated as returning str. This should be -> Dict[str, Any] to match the actual return value.
| def get_specific_screenshot(screenshot_id: str) -> str: | ||
| """Get a specific screenshot by ID.""" | ||
| try: | ||
| return get_screenshot_by_id(screenshot_id) | ||
| except Exception as e: | ||
| return {"error": str(e)} |
There was a problem hiding this comment.
Return type annotation is inconsistent. The function can return either the result from get_screenshot_by_id() or a dictionary with an error key. The return type should be -> Any or a Union type to accurately reflect both possible return types.
| def detect_game_context(message: str) -> str: | ||
| """Detect the current game from message.""" | ||
| try: | ||
| return detect_current_game(message) | ||
| except Exception as e: | ||
| return {"error": str(e)} |
There was a problem hiding this comment.
Return type annotation is inconsistent. The function returns either the result from detect_current_game() or a dictionary with an error key, but is annotated as returning str. The return type should accurately reflect both possible return types.
| def search_game_knowledge(query: str) -> str: | ||
| """Search the knowledge base for game information.""" | ||
| try: | ||
| return search_knowledge(query) | ||
| except Exception as e: | ||
| return {"error": str(e)} |
There was a problem hiding this comment.
Return type annotation is inconsistent. The function returns either the result from search_knowledge() or a dictionary with an error key, but is annotated as returning str. The return type should accurately reflect both possible return types.
| if game_context: | ||
| enhanced_message += f"\n\nDETECTED GAME: {game_context.upper()}" |
There was a problem hiding this comment.
Variable enhanced_message is only defined when image_data is provided (line 129), but this code assumes it always exists. If there's no image_data, this will raise an UnboundLocalError. The enhanced_message variable should be initialized before the image_data check or this block should be inside the if image_data condition.
| async for response in agent.arun( | ||
| message, | ||
| **run_params | ||
| ): |
There was a problem hiding this comment.
When image_data is provided, the message variable is reassigned to enhanced_message on line 144, but then the agent is called with the original message parameter on line 153. This means the enhanced message with game context is never used. Line 153 should use the potentially modified message variable or create a new variable to track the final message to send.
I will be testing the code locally on my system once. |
|
It appears that your code contains multiple issues:
Please rectify these issues, also do test the code thoroughly on your system as well before submitting another PR |
2 participants
Description
Please include a summary of the change and which issue is fixed. List any dependencies that are required for this change.
Fixes #14
Type of Change
Please delete options that are not relevant.
Checklist:
Screenshots (if applicable)
Please include any relevant screenshots or images that help explain the changes made.