feat: detect last_changed/last_updated duration math and suggest for: field (#1157)

src/ha_mcp/tools/best_practice_checker.py

@@ -77,6 +77,13 @@
 _RE_STATE_IN = re.compile(r"states\s*\([^)]+\)\s+in\s+[\[(]")
 # Unsafe direct state access: states.sensor.x.state
 _RE_DIRECT_STATE = re.compile(r"\bstates\.\w+\.\w+\.state\b")
+# Duration/recency checks via last_changed or last_updated arithmetic.
+# Both alternations require at least one dotted qualifier (e.g. ``states.sensor.x.``)
+# so bare Jinja variables named ``last_changed`` are not falsely flagged.
+_RE_DURATION_MATH = re.compile(
+    r"\bnow\(\)\s*-\s*(?:\w+\.)+last_(?:changed|updated)\b"
+    r"|\b(?:\w+\.)+last_(?:changed|updated)\s*<\s*now\(\)"
+)
 # Motion entity pattern
 _RE_MOTION = re.compile(r"binary_sensor\.\w*motion", re.IGNORECASE)
 # Any Jinja template marker — catch-all and target-field scan.
@@ -273,16 +280,23 @@ def _check_template_string(
             "function instead (returns 'unknown' if missing rather than raising)."
             + _ref(skill_prefix, "template-guidelines.md#common-patterns")
         )
+    if _RE_DURATION_MATH.search(template):
+        warnings.append(
+            f"{label} uses template for duration/recency check "
+            "(`now() - X.last_changed/last_updated`) — use the native `for:` field "
+            "on a `state` trigger or condition instead "
+            "(e.g., `platform: state, entity_id: binary_sensor.motion, to: 'off', "
+            "for: {minutes: 5}`). Native `for:` is event-driven and avoids repeated "
+            "template evaluation on every state change."
+            + _ref(skill_prefix, "automation-patterns.md#native-conditions")
+        )
 
     # Generic fallback: any Jinja in this logic position that didn't match
     # a specific detector. Catches new anti-patterns (issue #1011) and
     # 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 +318,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(
@@ -439,15 +451,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"
+                    )
                 )
 
 
@@ -500,6 +516,19 @@ def _check_triggers(
                             "automation-patterns.md#trigger-types",
                         )
                     )
+                if _RE_DURATION_MATH.search(vt):
+                    warnings.append(
+                        "Trigger uses template for duration/recency check "
+                        "(`now() - X.last_changed/last_updated`) — use the native "
+                        "`for:` field on a `state` trigger instead "
+                        "(e.g., `platform: state, entity_id: binary_sensor.motion, "
+                        "to: 'off', for: {minutes: 5}`). Native `for:` is event-driven "
+                        "and doesn't re-evaluate on every state change."
+                        + _ref(
+                            skill_prefix,
+                            "automation-patterns.md#trigger-types",
+                        )
+                    )
                 # Generic fallback for unmatched template triggers.
                 if len(warnings) == initial and _RE_ANY_TEMPLATE.search(vt):
                     warnings.append(
@@ -513,6 +542,21 @@ def _check_triggers(
                         )
                     )
 
+        # numeric_state trigger: value_template can also contain duration math
+        # (e.g. transforming last_changed into a seconds value for the threshold).
+        if platform == "numeric_state":
+            vt = trigger.get("value_template", "")
+            if isinstance(vt, str) and _RE_DURATION_MATH.search(vt):
+                warnings.append(
+                    "`numeric_state` trigger uses `value_template` for duration/recency "
+                    "check (`now() - X.last_changed/last_updated`) — use the native "
+                    "`for:` field on a `state` trigger instead "
+                    "(e.g., `platform: state, entity_id: binary_sensor.motion, "
+                    "to: 'off', for: {minutes: 5}`). Native `for:` is event-driven "
+                    "and doesn't re-evaluate on every state change."
+                    + _ref(skill_prefix, "automation-patterns.md#trigger-types")
+                )
+
 
 # ---------------------------------------------------------------------------
 # Mode + motion check