feat: add ha_remove_entity tool (closes #874) - #876
Conversation
Summary of ChangesHello, 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 introduces a new tool, 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. 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 introduces the ha_remove_entity tool, which allows for the permanent removal of entities from the Home Assistant entity registry. The implementation includes the necessary tool metadata, WebSocket API integration, and both unit and E2E tests. Feedback focuses on aligning the error handling with the repository's style guide by providing actionable suggestions in the error responses and improving the robustness of error message parsing. Additionally, there are recommendations to include more context in error logs and to add a success-path scenario to the E2E test suite.
sergeykad
left a comment
sergeykad
left a comment
There was a problem hiding this comment.
Two issues found — both on missing suggestions in error responses.
|
Hi @kingpanther13 — closing this PR. After a more thorough review |
|
@Patch76 this is actually not that bad! Just ask Claude code to look at all of the inline reviews and resolve them, it'll take care of it for you. This is a normal part of the PR process, you should see some of my PRs, it can take a lot of reviews to get it right sometimes. |
|
@Patch76 I'm going to reopen it because this is a legit tool we should be adding. If you don't want to continue working on it that's fine we can take over from here, but I have confidence that you're capable of handling it. This is a very good first PR to learn the process on. |
- Add actionable suggestions to ENTITY_NOT_FOUND and SERVICE_CALL_FAILED error responses (addresses sergeykad + Gemini Code Assist feedback) - Include entity_id in logger.error message for better debuggability - Add happy-path E2E test: create helper → remove → verify gone
|
/gemini review |
|
@Patch76 take as long as you need to resolve all of the review comments and get it how you like it. Once you're ready just mark it ready for review and one of us will approve the CI tests to run... At that point it takes like 5-6 minutes for them to finish, then you'll ask Claude to fix whatever tests may have failed. After that Usually Sergey or me will do our review and request further changes if necessary. Then you just kinda repeat the process till we post an approving review and it merges. Usually we won't review till it is taken out of draft state but since it started as open then converted to draft it still automatically requested reviews from us (no big deal). Just so you know for the future :). |
There was a problem hiding this comment.
Code Review
This pull request introduces the ha_remove_entity tool to the Entity Registry, allowing for the permanent removal of entities from Home Assistant. The implementation includes the tool definition, updated documentation in the README, and comprehensive E2E and unit tests. The review feedback suggests aligning the tool's safety annotations with repository standards by enabling idempotentHint and destructiveHint. Additionally, there is a recommendation to improve the robustness of error message extraction from WebSocket responses to provide better debugging context.
- idempotentHint: true — consistent with ha_remove_device and ha_remove_zone; safe to retry since repeated calls on a missing entity cause no additional harm - Add isinstance check for error field to handle both dict and string payloads - Regenerate tools.json
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ha_remove_entity tool, enabling the permanent removal of entities from the Home Assistant entity registry. The implementation includes the tool logic in tools_entities.py, updated documentation in README.md, and metadata in tools.json, supported by new unit and E2E tests. Review feedback correctly identifies that the idempotentHint annotation is set to True in both the source code and metadata, which contradicts the tool's behavior of raising an error on subsequent calls for the same entity; this should be updated to False to align with the repository style guide.
…ot_found)
Live verified: config/entity_registry/remove on non-existent entity returns
{success: false, error: {code: not_found}} — rest_client raises Exception,
tool raises ToolError. Behavior differs between calls 1 and 2.
PR description and E2E test confirm this. Consistent annotation: False.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ha_remove_entity tool to the Home Assistant MCP server, allowing for the permanent removal of entities from the registry. The changes include updates to the README, tool metadata, and the addition of comprehensive E2E and unit tests. The review feedback correctly identifies an inconsistency where ha_remove_entity is marked as idempotent despite subsequent calls failing when an entity is missing; this should be corrected to False in both the implementation and metadata. Additionally, the ha_set_entity docstring should be updated to explicitly reflect its dual capability for both creating and updating entities.
…ntity - ha_remove_entity: idempotentHint True → False (second call raises ToolError) - ha_set_entity: idempotentHint False → True (restore upstream value, accidentally changed in previous commit via non-unique str.replace) - Regenerate tools.json
|
/gemini review |
|
@Patch76 Heads up — the Gemini reviews on this PR are contradicting themselves on Timeline:
This is a great example of why we can't always trust automated reviews — sometimes you just need to dismiss them. You're allowed to push back on any reviewer's requested changes — bot or human — as long as you have a reason for it. If a suggestion doesn't make sense or contradicts itself, resolve it with a brief note explaining why you're dismissing it. That's a normal part of the review process. What About |
There was a problem hiding this comment.
Code Review
This pull request adds the ha_remove_entity tool to the Entity Registry domain, allowing for the permanent removal of entities from Home Assistant. The implementation includes structured error handling, documentation updates in the README and site data, and comprehensive unit and E2E tests. Feedback was provided to ensure the tool's safety annotations are consistent with repository standards by setting both idempotentHint and destructiveHint to True.
…tion All existing removal tools (ha_remove_device, ha_remove_zone, ha_remove_area) use idempotentHint: True + destructiveHint: True regardless of double-removal behavior. Following established convention per maintainer guidance.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ha_remove_entity tool to the Home Assistant MCP server, allowing for the permanent removal of entities from the entity registry. The changes include the tool implementation in tools_entities.py, updates to the README.md and tools.json documentation, and the addition of both E2E and unit tests. One issue was identified regarding the idempotentHint annotation, which should be set to False because the operation fails on subsequent calls if the entity has already been removed.
🧪 Your changes are now in the dev channel!Your PR has been merged to master and is available for testing in the dev channel. Test your changes before the next stable release (biweekly Wednesday): Quick start# Run dev version
uvx ha-mcp-dev
# Check version
uvx ha-mcp-dev --versionDocker: docker pull ghcr.io/homeassistant-ai/ha-mcp:dev
docker run --rm -i \
-e HOMEASSISTANT_URL=http://your-ha:8123 \
-e HOMEASSISTANT_TOKEN=your_token \
ghcr.io/homeassistant-ai/ha-mcp:devFound an issue? Please open a new bug report and mention this PR for context. |
Picks up ha_remove_entity from PR homeassistant-ai#876 merged into master. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
What does this PR do?
Adds
ha_remove_entitytool that removes an entity from the Home Assistant entity registry via WebSocket (config/entity_registry/remove). Closes #874.Modelled on
ha_remove_device(same WARNING/EXAMPLES/NOTE pattern). The tool:Type of change
Testing
uv run pytest)uv run ruff check)Checklist
Note on
idempotentHint: The automated triage bot suggestedidempotentHint: True, but this PR usesFalse. A second call with the sameentity_idreturns anot_founderror rather than succeeding silently — so the operation is not idempotent per the MCP spec definition. Happy to discuss if you see it differently.Note on commit history: This branch has 3 commits (feat + chore + fix). Squash on merge is fine.