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

src/ha_mcp/tools/best_practice_checker.py

@@ -77,6 +77,11 @@
 _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
+_RE_DURATION_MATH = re.compile(
+    r"\bnow\(\)\s*-\s*(?:\w+\.)+last_(?:changed|updated)\b"
+    r"|\blast_(?: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,6 +278,16 @@ 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
@@ -500,6 +515,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(

tests/src/unit/test_best_practice_checker.py

@@ -1311,3 +1311,81 @@ def test_generic_catchall(self):
             "action": [],
         }, skill_prefix=None)
         self._assert_clean(warnings)
+
+
+# ---------------------------------------------------------------------------
+# Duration math / for: field detector
+# ---------------------------------------------------------------------------
+
+
+class TestDurationMathDetector:
+    """Detects now() - X.last_changed patterns and suggests native for:."""
+
+    def test_condition_last_changed_math(self):
+        config = {
+            "condition": [{
+                "condition": "template",
+                "value_template": "{{ now() - states.binary_sensor.motion.last_changed > timedelta(minutes=5) }}",
+            }],
+            "action": [],
+        }
+        warnings = check_automation_config(config)
+        assert _has_warning_containing(warnings, "last_changed/last_updated", "for:")
+
+    def test_condition_last_updated_math(self):
+        config = {
+            "condition": [{
+                "condition": "template",
+                "value_template": "{{ now() - states.sensor.temp.last_updated > timedelta(minutes=10) }}",
+            }],
+            "action": [],
+        }
+        warnings = check_automation_config(config)
+        assert _has_warning_containing(warnings, "last_changed/last_updated", "for:")
+
+    def test_trigger_template_last_changed_math(self):
+        config = {
+            "trigger": [{
+                "platform": "template",
+                "value_template": "{{ now() - trigger.last_changed > timedelta(seconds=30) }}",
+            }],
+            "action": [],
+        }
+        warnings = check_automation_config(config)
+        assert _has_warning_containing(warnings, "last_changed/last_updated", "for:")
+
+    def test_state_trigger_with_for_field_no_warning(self):
+        config = {
+            "trigger": [{
+                "platform": "state",
+                "entity_id": "binary_sensor.motion",
+                "to": "off",
+                "for": {"minutes": 5},
+            }],
+            "action": [{"service": "light.turn_off", "target": {"entity_id": "light.hall"}}],
+        }
+        warnings = check_automation_config(config)
+        assert not _has_warning_containing(warnings, "last_changed", "for:")
+
+    def test_warning_contains_skill_ref(self):
+        config = {
+            "condition": [{
+                "condition": "template",
+                "value_template": "{{ now() - states.sensor.x.last_changed > timedelta(hours=1) }}",
+            }],
+            "action": [],
+        }
+        warnings = check_automation_config(config, skill_prefix=SKILL_PREFIX)
+        assert _has_warning_containing(warnings, "automation-patterns.md#native-conditions")
+
+    def test_no_generic_fallback_when_duration_fires(self):
+        config = {
+            "condition": [{
+                "condition": "template",
+                "value_template": "{{ now() - states.binary_sensor.door.last_changed > timedelta(minutes=1) }}",
+            }],
+            "action": [],
+        }
+        warnings = check_automation_config(config)
+        generic_warnings = [w for w in warnings if "if this maps to a native option" in w]
+        assert not generic_warnings, "Generic fallback should not fire alongside specific detector"