fix: standardize empty result feedback across all tools#51
Conversation
Summary of ChangesHello @StatPan, 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 focuses on enhancing the robustness and clarity of API responses within the Multi-Agent Collaboration Platform (MCP) by standardizing how tools handle scenarios where no data is found. The primary goal is to provide more explicit feedback to consuming agents, particularly Language Models, by returning descriptive string messages instead of empty data structures. This improves the interpretability of API outcomes and prevents potential misinterpretations of empty results. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
This pull request effectively standardizes the handling of empty results across various tools by returning informative string messages instead of empty data structures. This is a great improvement for the consuming LLM. The type hints have also been updated accordingly.
My review includes a couple of suggestions to improve robustness:
- In
get_committee_info, I've suggested handling a specific error case from a downstream service to avoid returning a malformed response. - In
get_member_voting_history, I've proposed adding input validation to ensure at least one required parameter is provided, which prevents confusing error messages.
| if not committees and not members: | ||
| return f"위원회 '{committee_name or committee_code}'에 대한 정보를 찾을 수 없습니다." |
There was a problem hiding this comment.
The current logic does not properly handle the case where the get_committee_members service returns a dictionary containing an error. In this scenario, members evaluates to a truthy value, which can lead to an incomplete JSON object containing an error being returned to the user. It would be more robust to explicitly check for the error from the service and return a user-friendly message.
| if not committees and not members: | |
| return f"위원회 '{committee_name or committee_code}'에 대한 정보를 찾을 수 없습니다." | |
| if isinstance(members, dict) and "error" in members: | |
| error_details = members.get("error", {}) | |
| if isinstance(error_details, dict): | |
| return error_details.get("suggestion", "위원회 위원 명단 정보를 찾을 수 없습니다.") | |
| return "위원회 위원 명단 정보를 찾을 수 없습니다." | |
| if not committees and not members: | |
| return f"위원회 '{committee_name or committee_code}'에 대한 정보를 찾을 수 없습니다." |
| records = await service.get_member_voting_history(name=name, bill_id=bill_id, age=age, page=page, limit=limit) | ||
| if not records: | ||
| target = f"의원 '{name}'" if name else f"의안 ID '{bill_id}'" | ||
| return f"{target}에 대한 표결 기록을 찾을 수 없습니다." |
There was a problem hiding this comment.
If both name and bill_id are passed as None, the target variable becomes "의안 ID 'None'", resulting in a confusing error message. Since at least one of these parameters is required for a meaningful search, it's better to add a validation check at the beginning of the function to provide clear guidance to the user. I've also improved the message when both parameters are provided.
if not name and not bill_id:
return "의원명(name) 또는 의안 ID(bill_id) 중 하나는 반드시 입력해야 합니다."
records = await service.get_member_voting_history(name=name, bill_id=bill_id, age=age, page=page, limit=limit)
if not records:
if name and bill_id:
target = f"의원 '{name}'과(와) 의안 ID '{bill_id}'"
elif name:
target = f"의원 '{name}'"
else:
target = f"의안 ID '{bill_id}'"
return f"{target}에 대한 표결 기록을 찾을 수 없습니다."- Handle error responses in committee member lookups with helpful suggestions - Add input validation for member voting history (require name or bill_id) - Refine error messages in voting history for better context
1 participant
Changes
| strfor these tools.