refactor(complexity): reduce C901 violations in tools/ — batch 4 (#1408)

* refactor(complexity): reduce C901 violations in tools/ batch 4 (#925)

- Extract _check_control_flow_actions() in best_practice_checker.py
- Extract _parse_event_data() in tools_service.py
- Extract _try_raw_cdn() in tools_updates.py
- Extract _process_menu_flow_result(), _build_flow_error_context(),
  _submit_step() in tools_config_entry_flow.py
- Migrate tools_yaml_config.py to class-based pattern (YamlConfigTools)
- Migrate tools_bug_report.py to class-based pattern (BugReportTools),
  extracting _build_formatted_report()
- Add defensive guard in _process_menu_flow_result for empty intro_flow_id
- Add tests for _try_raw_cdn loop behavior and _submit_step error propagation
- Update test fixtures in test_tools_bug_report and test_yaml_config_tool
  to use mcp.add_tool() (class-based registration pattern)

* fixup: ruff format + address Gemini review comments

- ruff format all changed files (CI enforces ruff format --check)
- _process_menu_flow_result: str | None type hint for intro_flow_id
- _process_menu_flow_result: narrow except to (HomeAssistantAPIError, TimeoutError)
- fetch_helper_flow_info: remove `and intro_flow_id` guard from MENU branch
  (restores menu_options surfacing when intro_flow_id is absent)
- _try_raw_cdn: narrow except to httpx.RequestError
- test_tools_updates: use httpx.ConnectError in exception-continue test

* chore(hooks): add ruff-format to pre-commit hook

Mirrors the CI ruff format --check gate locally so formatting
issues are caught and auto-fixed before push.

* Potential fix for pull request finding 'Implicit string concatenation in a list'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: join implicit string concatenation in suggestions list

* refactor(complexity): fix C901 violations in tools_config_automations and util_helpers

- tools_config_automations: extract _scene_create_in_choose, _check_scene_create_misroute,
  _validate_condition_platform, _run_python_transform, _run_config_update helpers
- util_helpers: extract _parse_json_to_str_list, _sample_state_change,
  _discover_automation_sample, _automation_event_filter, _fetch_raw_diagnostics,
  _apply_data_path_resolution, _ws_subscribe_all, _ws_post_subscribe_check,
  _ws_run_wait_loop, _ws_cleanup, _apply_truncation_cap helpers
- pyproject.toml: remove util_helpers.py from C901 per-file-ignores

* refactor(service): extract _parse_json_dict_param to deduplicate JSON dict validation

_parse_event_data and _parse_service_data shared identical parse-validate-raise
logic differing only in the type error message. Extract into a single helper.

Author: sergeykad

lefthook.yml

@@ -7,6 +7,10 @@ pre-commit:
             glob: "**/*.py"
             run: uv run ruff check {staged_files} --fix
             stage_fixed: true
+          - name: ruff-format
+            glob: "**/*.py"
+            run: uv run ruff format {staged_files}
+            stage_fixed: true
           - name: ast-grep
             glob: "src/ha_mcp/tools/**/*.py"
             run: uv run ast-grep scan

pyproject.toml

@@ -147,7 +147,6 @@ ignore = [
 "src/ha_mcp/tools/tools_search.py" = ["C901"]
 "src/ha_mcp/tools/tools_utility.py" = ["C901"]
 "src/ha_mcp/tools/smart_search.py" = ["C901"]
-"src/ha_mcp/tools/util_helpers.py" = ["C901"]
 
 [tool.pytest.ini_options]
 testpaths = ["tests"]

src/ha_mcp/tools/best_practice_checker.py

@@ -279,10 +279,7 @@ def _check_template_string(
     # reframes #695 from "enumerate bad shapes" to "surface every template
     # in a logic position". Specific detectors above keep their tailored
     # messages.
-    if (
-        len(warnings) == initial_count
-        and _RE_ANY_TEMPLATE.search(template)
-    ):
+    if len(warnings) == initial_count and _RE_ANY_TEMPLATE.search(template):
         warnings.append(
             f"Template detected in {position} — if this maps to a native option "
             "(`numeric_state`, `state`, `time`, `sun`, `zone`, `device`), use that "
@@ -304,9 +301,7 @@ def _check_choose_actions(
             _check_condition_templates(
                 option.get("conditions", []), warnings, skill_prefix
             )
-            _check_action_tree(
-                option.get("sequence", []), warnings, skill_prefix
-            )
+            _check_action_tree(option.get("sequence", []), warnings, skill_prefix)
 
 
 def _check_repeat_actions(
@@ -317,6 +312,31 @@ def _check_repeat_actions(
     _check_action_tree(repeat.get("sequence", []), warnings, skill_prefix)
 
 
+def _check_control_flow_actions(
+    action: dict[str, Any], warnings: list[str], skill_prefix: str | None
+) -> None:
+    """Check choose/if/then/else/repeat/parallel sub-trees in a single action."""
+    if "choose" in action:
+        _check_choose_actions(action["choose"], warnings, skill_prefix)
+
+    if "if" in action:
+        _check_condition_templates(action["if"], warnings, skill_prefix)
+
+    for key in ("then", "else", "default"):
+        nested = action.get(key)
+        if isinstance(nested, list):
+            _check_action_tree(nested, warnings, skill_prefix)
+
+    if "repeat" in action and isinstance(action["repeat"], dict):
+        _check_repeat_actions(action["repeat"], warnings, skill_prefix)
+
+    # `parallel:` runs sub-actions concurrently — same shape as `sequence`,
+    # different semantics. Recurse so templates inside parallel branches
+    # are inspected the same as templates inside choose/repeat sequences.
+    if "parallel" in action and isinstance(action["parallel"], list):
+        _check_action_tree(action["parallel"], warnings, skill_prefix)
+
+
 def _check_action_tree(
     actions: Any, warnings: list[str], skill_prefix: str | None
 ) -> None:
@@ -359,26 +379,7 @@ def _check_action_tree(
         if isinstance(target, dict):
             _check_target_dict(target, warnings, skill_prefix)
 
-        # Nested conditions in choose/if/repeat
-        if "choose" in action:
-            _check_choose_actions(action["choose"], warnings, skill_prefix)
-
-        if "if" in action:
-            _check_condition_templates(action["if"], warnings, skill_prefix)
-
-        for key in ("then", "else", "default"):
-            nested = action.get(key)
-            if isinstance(nested, list):
-                _check_action_tree(nested, warnings, skill_prefix)
-
-        if "repeat" in action and isinstance(action["repeat"], dict):
-            _check_repeat_actions(action["repeat"], warnings, skill_prefix)
-
-        # `parallel:` runs sub-actions concurrently — same shape as `sequence`,
-        # different semantics. Recurse so templates inside parallel branches
-        # are inspected the same as templates inside choose/repeat sequences.
-        if "parallel" in action and isinstance(action["parallel"], list):
-            _check_action_tree(action["parallel"], warnings, skill_prefix)
+        _check_control_flow_actions(action, warnings, skill_prefix)
 
 
 def _check_service_template(
@@ -439,15 +440,19 @@ def _check_target_dict(
                     f"hardcode the literal value instead. The self-reference is always "
                     f"resolvable at write time, so the template adds runtime cost without "
                     f"any flexibility."
-                    + _ref(skill_prefix, "template-guidelines.md#when-to-avoid-templates")
+                    + _ref(
+                        skill_prefix, "template-guidelines.md#when-to-avoid-templates"
+                    )
                 )
             else:
                 warnings.append(
                     f"Action `target.{field}` uses a template — prefer a hardcoded literal, "
                     f"or use a `choose` action with native conditions to dispatch to different "
                     f"hardcoded targets. Templates in target fields fail silently if they "
                     f"resolve to a non-existent entity."
-                    + _ref(skill_prefix, "template-guidelines.md#when-to-avoid-templates")
+                    + _ref(
+                        skill_prefix, "template-guidelines.md#when-to-avoid-templates"
+                    )
                 )
 
 

src/ha_mcp/tools/tools_bug_report.py

@@ -16,6 +16,7 @@
 
 import httpx
 from fastmcp import Context
+from fastmcp.tools import tool
 from pydantic import Field
 
 from ha_mcp import __version__
@@ -28,7 +29,7 @@
     get_recent_logs,
     get_startup_logs,
 )
-from .helpers import log_tool_usage
+from .helpers import log_tool_usage, register_tool_methods
 from .util_helpers import ANSI_ESCAPE_RE
 
 logger = logging.getLogger(__name__)
@@ -388,10 +389,66 @@ async def _fetch_addon_logs() -> str:
     return ""
 
 
-def register_bug_report_tools(mcp: Any, client: Any, **kwargs: Any) -> None:
-    """Register bug report tools with the MCP server."""
+def _build_formatted_report(
+    diagnostic_info: dict[str, Any],
+    mcp_transport: str,
+    client_info: dict[str, str],
+    platform_info: dict[str, str],
+    config_toggles: dict[str, Any],
+    startup_logs: list[dict[str, Any]],
+    startup_log_summary: str,
+    recent_logs: list[dict[str, Any]],
+    log_summary: str,
+    addon_logs: str,
+) -> str:
+    report_lines = [
+        "=== ha-mcp Bug Report Info ===",
+        "",
+        f"ha-mcp Version: {diagnostic_info['ha_mcp_version']}",
+        f"Installation Method: {diagnostic_info['installation_method']}",
+        f"MCP Transport: {mcp_transport}",
+        f"MCP Client: {_format_client_info_for_template(client_info)}",
+        f"Operating System: {platform_info['os']} {platform_info['os_release']} ({platform_info['architecture']})",
+        f"Python Version: {platform_info['python_version']}",
+        f"Home Assistant Version: {diagnostic_info['home_assistant_version']}",
+        f"Connection Status: {diagnostic_info['connection_status']}",
+        f"Entity Count: {diagnostic_info['entity_count']}",
+    ]
+    if "location_name" in diagnostic_info:
+        report_lines.append(f"Location Name: {diagnostic_info['location_name']}")
+    if "time_zone" in diagnostic_info:
+        report_lines.append(f"Time Zone: {diagnostic_info['time_zone']}")
+    if config_toggles:
+        report_lines.extend(["", "=== ha-mcp Config Toggles ==="])
+        for key, value in config_toggles.items():
+            report_lines.append(f"  {key}: {value}")
+    if startup_logs:
+        report_lines.extend(
+            [
+                "",
+                f"=== Startup Logs ({len(startup_logs)} entries) ===",
+                startup_log_summary,
+            ]
+        )
+    if recent_logs:
+        report_lines.extend(
+            [
+                "",
+                f"=== Recent Tool Calls ({len(recent_logs)} entries) ===",
+                log_summary,
+            ]
+        )
+    if addon_logs:
+        report_lines.extend(["", "=== Add-on Container Logs ===", addon_logs])
+    return "\n".join(report_lines)
+
 
-    @mcp.tool(
+class BugReportTools:
+    def __init__(self, client: Any) -> None:
+        self._client = client
+
+    @tool(
+        name="ha_report_issue",
         tags={"Utilities"},
         annotations={
             "idempotentHint": True,
@@ -401,6 +458,7 @@ def register_bug_report_tools(mcp: Any, client: Any, **kwargs: Any) -> None:
     )
     @log_tool_usage
     async def ha_report_issue(
+        self,
         tool_call_count: Annotated[
             int,
             Field(
@@ -476,7 +534,7 @@ async def ha_report_issue(
 
         # Try to get Home Assistant config and connection status
         try:
-            config = await client.get_config()
+            config = await self._client.get_config()
             diagnostic_info["connection_status"] = "Connected"
             diagnostic_info["home_assistant_version"] = config.get("version", "Unknown")
             diagnostic_info["location_name"] = config.get("location_name", "Unknown")
@@ -487,7 +545,7 @@ async def ha_report_issue(
 
         # Try to get entity count
         try:
-            states = await client.get_states()
+            states = await self._client.get_states()
             if states:
                 diagnostic_info["entity_count"] = len(states)
         except Exception as e:
@@ -511,59 +569,18 @@ async def ha_report_issue(
         startup_log_summary = _format_startup_logs(startup_logs)
 
         # Build the formatted report
-        report_lines = [
-            "=== ha-mcp Bug Report Info ===",
-            "",
-            f"ha-mcp Version: {diagnostic_info['ha_mcp_version']}",
-            f"Installation Method: {diagnostic_info['installation_method']}",
-            f"MCP Transport: {mcp_transport}",
-            f"MCP Client: {_format_client_info_for_template(client_info)}",
-            f"Operating System: {platform_info['os']} {platform_info['os_release']} ({platform_info['architecture']})",
-            f"Python Version: {platform_info['python_version']}",
-            f"Home Assistant Version: {diagnostic_info['home_assistant_version']}",
-            f"Connection Status: {diagnostic_info['connection_status']}",
-            f"Entity Count: {diagnostic_info['entity_count']}",
-        ]
-
-        # Add optional fields if available
-        if "location_name" in diagnostic_info:
-            report_lines.append(f"Location Name: {diagnostic_info['location_name']}")
-        if "time_zone" in diagnostic_info:
-            report_lines.append(f"Time Zone: {diagnostic_info['time_zone']}")
-
-        if config_toggles:
-            report_lines.extend(["", "=== ha-mcp Config Toggles ==="])
-            for key, value in config_toggles.items():
-                report_lines.append(f"  {key}: {value}")
-
-        if startup_logs:
-            report_lines.extend(
-                [
-                    "",
-                    f"=== Startup Logs ({len(startup_logs)} entries) ===",
-                    startup_log_summary,
-                ]
-            )
-
-        if recent_logs:
-            report_lines.extend(
-                [
-                    "",
-                    f"=== Recent Tool Calls ({len(recent_logs)} entries) ===",
-                    log_summary,
-                ]
-            )
-
-        if addon_logs:
-            report_lines.extend(
-                [
-                    "",
-                    "=== Add-on Container Logs ===",
-                    addon_logs,
-                ]
-            )
-
-        formatted_report = "\n".join(report_lines)
+        formatted_report = _build_formatted_report(
+            diagnostic_info,
+            mcp_transport,
+            client_info,
+            platform_info,
+            config_toggles,
+            startup_logs,
+            startup_log_summary,
+            recent_logs,
+            log_summary,
+            addon_logs,
+        )
 
         # Generate suggested title up-front so it can be folded into the
         # submission URLs as a `&title=` query param. This auto-fills the
@@ -673,6 +690,11 @@ async def ha_report_issue(
         }
 
 
+def register_bug_report_tools(mcp: Any, client: Any, **kwargs: Any) -> None:
+    """Register bug report tools with the MCP server."""
+    register_tool_methods(mcp, BugReportTools(client))
+
+
 def _format_config_toggles_for_template(toggles: dict[str, Any]) -> str:
     """Render config toggle snapshot as a markdown bullet list.