refactor(internal): extract _fetch_dashboards_list helper (#1193)

[SealKan]

Closes #1193

Three call sites in tools_config_dashboards.py duplicated the same
lovelace/dashboards/list fetch + shape-normalisation + warning logic.
This PR extracts a shared _fetch_dashboards_list(client) helper and
routes all three sites through it.

What changed

New private helper _fetch_dashboards_list(client):

• fetches via send_websocket_message({"type": "lovelace/dashboards/list"})
• normalises dict-with-result (guarded with isinstance(..., list)) vs bare-list response shapes
• emits one logger.warning on unexpected shapes (module-level logger; no extra parameter)
• returns list[dict[str, Any]] | None — None on shape failure

Refactored call sites (all three preserve existing caller semantics):

• _resolve_dashboard: None → return None, None
• ha_config_get_dashboard (list_only=True): None → fallback [];
  now emits the shape-change warning that was previously missing at this site
• ha_config_set_dashboard existence-check: None → fallback [];
  removes the inline warning duplication added in ac52f74

Behaviour deltas (alongside the refactor)

Two small behavioural changes came in with the extraction — both intentional and strictly safer:

1. Guard tightened at the dict branch — from "result" in result to
   isinstance(result.get("result"), list). Previously a response like
   {"result": "string"} or {"result": None} would take the happy path
   and blow up downstream on iteration; now it falls through to the warning path.

2. ha_config_get_dashboard(list_only=True) now emits the unexpected-shape warning.
   Before this PR, an unexpected HA response at the list_only path silently
   returned [] with no operator signal. After: same [] fallback, but the
   warning is emitted — consistent with the other two sites.

Testing

• All existing dashboard unit tests pass (34 tests)
• New unit test TestGetDashboardListOnlyUnexpectedShape added to
  test_config_get_dashboard_search_error.py: pins the new warning
  behaviour at the ha_config_get_dashboard list_only=True path,
  which had no unexpected-shape coverage before this PR
• ruff and mypy both pass on the changed files

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 refactors the dashboard list fetching logic within the configuration tools. By centralizing the WebSocket communication and response normalization into a single helper function, the codebase becomes more maintainable and consistent in its error reporting. The changes preserve existing functionality while ensuring that future API response format changes are handled uniformly across the application.

Highlights

• Code Consolidation: Extracted the repetitive logic for fetching and normalizing dashboard lists into a new private helper function, _fetch_dashboards_list.
• Improved Error Handling: Centralized warning logging for unexpected API response shapes, ensuring consistent behavior across all call sites.
• Refactored Call Sites: Updated three existing locations in tools_config_dashboards.py to utilize the new helper, simplifying the codebase and reducing duplication.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request centralizes the logic for fetching and normalizing Home Assistant dashboard lists by introducing a new private helper function, _fetch_dashboards_list, and refactoring several tools to use it. Feedback focuses on improving the helper function's robustness by ensuring the API response is explicitly checked for the expected list type and simplifying the function signature by removing the redundant logger parameter in favor of the module-level logger. Additionally, suggestions were made to align the docstring and parameter naming with the repository's style guidelines.

[SealKan]

Addressed Gemini's review feedback in a228b8c:

• Dropped the log: logging.Logger parameter from _fetch_dashboards_list; the helper now uses the module-level logger directly (removes the leading-underscore-on-used-param issue at all three call sites).
• Tightened the dict-branch guard to isinstance(result.get("result"), list) so a non-list value in result["result"] no longer slips through.
• Added TestGetDashboardListOnlyUnexpectedShape unit test (e0443ed) to cover the new warning behavior at the list_only=True path (Site 2), which previously had no test coverage for unexpected shapes.

[kingpanther13]

Thanks for the cleanup — the helper shape, per-site None-vs-[] semantics, and
the new Site 2 test all match what #1193 asked for. Two things to address
before this can land, plus a couple of nits, and two side questions that
don't gate the merge.

Requested changes (blocking)

1. PR description undersells what changed. The body frames this as a pure
   extraction, but two behavioural deltas slipped in alongside the refactor:

   • The dict-branch guard tightened from "result" in result to
     isinstance(result.get("result"), list). Pre-PR, a response like
     {"result": "string"} or {"result": None} took the happy path and
     blew up downstream on iteration; post-PR it falls through to the
     warning. Strictly safer, but it is a behaviour change.
   • ha_config_get_dashboard(list_only=True) (Site 2) now emits the
     unexpected-shape warning that was previously missing there.

   Both shifts are exactly what #1193 wanted. Please call them out in
   the PR description so a future spelunker reading "extract helper"
   doesn't miss them.

While you're in there (blocking)

• tests/src/unit/test_config_get_dashboard_search_error.py:117 — the new
  test docstring uses internal "Site 2" shorthand. Inline the meaning
  ("at the ha_config_get_dashboard list path") so pytest -v output
  stands on its own.
• Same file, the warning assertion checks only "unexpected shape" in rec.message. Mirror the precision of
  test_canonical_url_path_branch_warns_on_unexpected_shape in
  test_set_dashboard_metadata.py:289 and also assert "type=str" —
  catches placeholder-formatting regressions, not just message rot.

Side questions (non-blocking, won't hold up merge)

Two things stood out while reviewing. Answers help me understand who I'm
working with on future PRs — they don't gate this one.

1. The PR is opened from `SealKan` (account created today) but the commits
   are authored under `Kan-nguyen` (`nguyenanhkhoa301199@gmail.com`, a much
   older account). Are both accounts yours? Happy to take a yes here, just
   want it on the record.

2. Your `SealKan/bounty-workspace` repo describes itself as an "Autonomous
   OSS bounty contributor workspace," and the commit log there reads like a
   Claude-Code agent loop. Is this PR generated/driven by that agent,
   hand-authored, or some mix? No judgment either way — I want to know
   what the iteration loop will look like if I leave review feedback
   (you, the agent, both?).

Once the PR body is updated and the two test nits are in, I'm happy to
merge.

[SealKan]

Thanks — happy to answer both directly.

1. Accounts: Yes. Kan-nguyen (nguyenanhkhoa301199@gmail.com) is my personal account. SealKan is dedicated to AI-assisted work to keep the separation explicit. Should have said this upfront.

2. Workflow: I identify the problem, decide the approach, and judge whether feedback makes sense to accept. Claude Code handles the implementation. I review diffs before pushing — nothing goes out unread.

For your review: I read your comments, agreed the two blocking items are right, and the nits are reasonable. That call is mine. Claude Code executes the fix. If I disagreed with something, I'd say so here directly.

The iteration loop: you leave feedback → I decide what to do with it → implementation follows. Faster than solo, same judgment on every decision.

---

Addressing the blocking items in the next commit.

[kingpanther13]

All blocking items addressed:

• PR description now calls out both behaviour deltas (`## Behaviour deltas` section).
• Test docstring inlines the meaning ("at the `ha_config_get_dashboard` list path (`list_only=True` branch)").
• Warning assertion now checks `"unexpected shape"` AND `"type=str"`.

Thanks for the straight answers on the side questions — useful context for future PRs. Approving.

[github-actions]

🧪 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):
📖 Dev Channel Documentation

Quick start

# Run dev version
uvx ha-mcp-dev

# Check version
uvx ha-mcp-dev --version

Docker:

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:dev

---

Found an issue? Please open a new bug report and mention this PR for context.

[SealKan]

Yes, both accounts are mine. Kan-nguyen is my personal account; SealKan is dedicated to AI-assisted contributions to keep things separated.

I've now fixed the git config so all future commits (and all existing commits in open PRs) are authored under SealKan. The force-pushed commits in this PR should now show SealKan as author.

[kingpanther13]

Ok, sounds good to me! It's not a big deal, it just seemed weird especially considering SealKan was just created today, so I appreciate the explanation.