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

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

Adds _RE_DURATION_MATH to best_practice_checker.py to detect template
conditions and triggers using now() - X.last_changed/last_updated arithmetic.
These patterns should use the native `for:` field on state triggers instead
(event-driven, no template re-evaluation overhead). Fires in both condition
templates (_check_template_string) and template trigger value_templates
(_check_triggers). Includes 6 new tests covering conditions, triggers,
the positive clean case, skill ref presence, and no-generic-double-flag.

* fix: tighten _RE_DURATION_MATH to require dotted qualifier on both alternations

The second alternation previously matched bare Jinja variables named
`last_changed` (e.g. `{{ last_changed < now() }}`). Require at least one
dotted qualifier on both patterns so only entity attribute access is flagged.

Add two new tests:
- test_trigger_warning_uses_trigger_types_anchor: verifies trigger-path warning
  links to #trigger-types (not #native-conditions)
- test_no_false_positive_bare_last_changed_variable: verifies bare Jinja
  variable `last_changed < now()` does not trigger the detector

* feat(internal): extend duration math detector to numeric_state value_template

numeric_state triggers can carry a value_template field; duration/recency
math inside that field was previously not flagged. Add a dedicated check
block for platform: numeric_state so the detector covers both template and
numeric_state trigger platforms.

Add test_numeric_state_trigger_value_template_duration_math to verify.

* style: ruff format best_practice_checker and test file

* test: add reversed-comparison duration-math regression tests (#1157)

Cover the `X.last_changed/last_updated < now()` alternation of
_RE_DURATION_MATH in both condition and trigger value_template contexts.
Each test asserts exactly one warning fires, which also guards against the
two alternations double-flagging the same template.

Addresses review feedback on PR #1264.

---------

Co-authored-by: khoanguyentester-lgtm <nguyenanhkhoa301199@gmail.com>
Co-authored-by: kingpanther13 <25392815+kingpanther13@users.noreply.github.com>

Author: 3 people

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,6 +280,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
@@ -505,6 +522,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(
@@ -518,6 +548,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