Fix: Handle missing last_changed timestamps in ha_get_history (#447) (#497)

* chore: Add local/ to gitignore

Files in local/ directory should not be tracked by git.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: handle missing last_changed timestamps in ha_get_history

Fixes #447 where ha_get_history returned null last_changed and missing last_updated.

Root cause: Home Assistant WebSocket API omits the 'lc' (last_changed) field
when it equals 'lu' (last_updated) as an optimization. Our code didn't handle
this case, resulting in null timestamps.

Changes:
- Updated timestamp extraction logic to use last_updated value when last_changed is missing
- Added comprehensive unit tests for _convert_timestamp function
- Added regression tests for the timestamp handling logic
- Added E2E test to verify timestamps are present and valid in responses

The fix ensures both last_changed and last_updated are always populated with
valid ISO 8601 timestamps in the response.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: use nested .get() calls to handle falsy values correctly

Addresses Gemini Code Assist review feedback:
- Changed `or` fallbacks to nested `.get()` calls to avoid bugs with falsy values
- Empty dict `{}` for attributes is valid and should not fall back
- Empty string `''` for state is valid and should not fall back

The nested `.get(key1, .get(key2, default))` pattern only falls back when
the key is missing, not when the value is falsy, which is the correct behavior.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: julienld

src/ha_mcp/tools/tools_history.py

@@ -327,18 +327,21 @@ async def ha_get_history(
                     for state in limited_states:
                         # Get timestamps - WebSocket returns short-form (lc/lu) as Unix epoch floats
                         # or long-form (last_changed/last_updated) as strings
-                        last_changed_raw = state.get("lc", state.get("last_changed"))
+                        # Note: HA WebSocket API omits 'lc' when it equals 'lu' (optimization)
                         last_updated_raw = state.get("lu", state.get("last_updated"))
+                        last_changed_raw = state.get("lc", state.get("last_changed"))
+
+                        # If last_changed is missing, it means it equals last_updated
+                        if last_changed_raw is None and last_updated_raw is not None:
+                            last_changed_raw = last_updated_raw
 
                         state_entry = {
                             "state": state.get("s", state.get("state")),
                             "last_changed": _convert_timestamp(last_changed_raw),
                             "last_updated": _convert_timestamp(last_updated_raw),
                         }
                         if not minimal_response:
-                            state_entry["attributes"] = state.get(
-                                "a", state.get("attributes", {})
-                            )
+                            state_entry["attributes"] = state.get("a", state.get("attributes", {}))
                         formatted_states.append(state_entry)
 
                     entities_history.append(

tests/src/e2e/workflows/core/test_history.py

@@ -270,6 +270,74 @@ async def test_get_history_entity_ids_as_comma_string(self, mcp_client):
         else:
             logger.info("Comma-separated format may not be supported")
 
+    async def test_get_history_timestamps_present(self, mcp_client):
+        """Test that history returns valid timestamps for last_changed and last_updated.
+
+        This is a regression test for issue #447 where timestamps were null/missing.
+        """
+        logger.info("Testing ha_get_history includes valid timestamps")
+
+        result = await mcp_client.call_tool(
+            "ha_get_history",
+            {
+                "entity_ids": "sun.sun",
+                "start_time": "24h",
+                "minimal_response": False,
+                "significant_changes_only": False,
+                "limit": 10,
+            },
+        )
+
+        data = assert_mcp_success(result, "Get history with timestamps")
+
+        # History data is nested in 'data' key
+        inner_data = data.get("data", data)
+        assert "entities" in inner_data, f"Missing 'entities' in response: {data}"
+        assert len(inner_data["entities"]) > 0, "No entities in response"
+
+        entity_history = inner_data["entities"][0]
+        assert "states" in entity_history, f"Missing states: {entity_history}"
+        states = entity_history["states"]
+
+        if len(states) > 0:
+            logger.info(f"Checking {len(states)} state entries for valid timestamps")
+
+            for idx, state in enumerate(states):
+                # Verify both timestamp fields are present
+                assert "last_changed" in state, f"State {idx} missing 'last_changed': {state}"
+                assert "last_updated" in state, f"State {idx} missing 'last_updated': {state}"
+
+                # Verify timestamps are not null
+                last_changed = state["last_changed"]
+                last_updated = state["last_updated"]
+
+                assert last_changed is not None, f"State {idx} has null last_changed: {state}"
+                assert last_updated is not None, f"State {idx} has null last_updated: {state}"
+
+                # Verify timestamps are valid ISO 8601 strings
+                assert isinstance(last_changed, str), (
+                    f"State {idx} last_changed not a string: {type(last_changed)}"
+                )
+                assert isinstance(last_updated, str), (
+                    f"State {idx} last_updated not a string: {type(last_updated)}"
+                )
+
+                # Verify timestamps can be parsed as ISO datetime
+                try:
+                    datetime.fromisoformat(last_changed.replace("Z", "+00:00"))
+                except ValueError as e:
+                    pytest.fail(f"State {idx} last_changed not valid ISO format: {last_changed}: {e}")
+
+                try:
+                    datetime.fromisoformat(last_updated.replace("Z", "+00:00"))
+                except ValueError as e:
+                    pytest.fail(f"State {idx} last_updated not valid ISO format: {last_updated}: {e}")
+
+            logger.info("✓ All state entries have valid last_changed and last_updated timestamps")
+            logger.info(f"Sample: last_changed={states[0]['last_changed']}, last_updated={states[0]['last_updated']}")
+        else:
+            logger.warning("No state history available for test (may be normal for short periods)")
+
 
 @pytest.mark.asyncio
 @pytest.mark.core

tests/src/unit/test_history_helpers.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from ha_mcp.tools.tools_history import parse_relative_time
+from ha_mcp.tools.tools_history import _convert_timestamp, parse_relative_time
 
 
 class TestParseRelativeTime:
@@ -115,3 +115,183 @@ def test_large_hours_value(self):
         result = parse_relative_time("168h")
         expected = datetime.now(UTC) - timedelta(hours=168)
         assert abs((result - expected).total_seconds()) < 1
+
+
+class TestConvertTimestamp:
+    """Test _convert_timestamp function for issue #447 regression."""
+
+    def test_none_returns_none(self):
+        """None input returns None."""
+        assert _convert_timestamp(None) is None
+
+    def test_unix_epoch_int_converted_to_iso(self):
+        """Unix epoch integer converted to ISO format string."""
+        # 1700000000 = 2023-11-14T22:13:20+00:00 UTC
+        timestamp = 1700000000
+        result = _convert_timestamp(timestamp)
+        assert result == "2023-11-14T22:13:20+00:00"
+
+    def test_unix_epoch_float_converted_to_iso(self):
+        """Unix epoch float with microseconds converted to ISO format."""
+        # 1700000000.123456 = 2023-11-14T22:13:20.123456+00:00 UTC
+        timestamp = 1700000000.123456
+        result = _convert_timestamp(timestamp)
+        # Should preserve microseconds
+        assert result.startswith("2023-11-14T22:13:20.123456")
+        assert result.endswith("+00:00")
+
+    def test_iso_string_passed_through(self):
+        """ISO format string passed through unchanged."""
+        iso_string = "2026-01-17T12:00:00+00:00"
+        result = _convert_timestamp(iso_string)
+        assert result == iso_string
+
+    def test_iso_string_with_z_passed_through(self):
+        """ISO format string with Z suffix passed through."""
+        iso_string = "2026-01-17T12:00:00Z"
+        result = _convert_timestamp(iso_string)
+        assert result == iso_string
+
+    def test_invalid_type_returns_none(self):
+        """Invalid type (not int/float/str/None) returns None."""
+        assert _convert_timestamp([]) is None
+        assert _convert_timestamp({}) is None
+        assert _convert_timestamp(object()) is None
+
+    def test_zero_timestamp(self):
+        """Zero timestamp (epoch start) converts correctly."""
+        result = _convert_timestamp(0)
+        assert result == "1970-01-01T00:00:00+00:00"
+
+    def test_negative_timestamp(self):
+        """Negative timestamp (before epoch) converts correctly."""
+        # 1969-12-31T23:00:00+00:00
+        result = _convert_timestamp(-3600)
+        assert result.startswith("1969-12-31T23:00:00")
+
+
+class TestTimestampHandling:
+    """Test timestamp handling logic for issue #447.
+
+    Issue #447: ha_get_history returned null last_changed and missing last_updated.
+    Root cause: HA WebSocket API omits 'lc' when it equals 'lu' (optimization).
+    Fix: When 'lc' is missing, use 'lu' value for both timestamps.
+    """
+
+    def test_both_timestamps_present(self):
+        """When both lc and lu present, use their respective values."""
+        state = {
+            "s": "on",
+            "lc": 1700000000.0,  # Different from lu
+            "lu": 1700000100.0,
+            "a": {},
+        }
+
+        # Simulate the formatting logic from tools_history.py
+        last_updated_raw = state.get("lu") or state.get("last_updated")
+        last_changed_raw = state.get("lc") or state.get("last_changed")
+
+        if last_changed_raw is None and last_updated_raw is not None:
+            last_changed_raw = last_updated_raw
+
+        last_changed = _convert_timestamp(last_changed_raw)
+        last_updated = _convert_timestamp(last_updated_raw)
+
+        assert last_changed is not None
+        assert last_updated is not None
+        assert last_changed == "2023-11-14T22:13:20+00:00"
+        assert last_updated == "2023-11-14T22:15:00+00:00"
+
+    def test_lc_omitted_when_equals_lu(self):
+        """When lc is omitted (equals lu), last_changed should use lu value.
+
+        This is the regression test for issue #447.
+        HA WebSocket API omits 'lc' when state and timestamps are identical.
+        """
+        state = {
+            "s": "on",
+            # 'lc' is omitted when it equals 'lu'
+            "lu": 1700000000.0,
+            "a": {},
+        }
+
+        # Simulate the formatting logic from tools_history.py
+        last_updated_raw = state.get("lu") or state.get("last_updated")
+        last_changed_raw = state.get("lc") or state.get("last_changed")
+
+        # Critical fix: when lc is missing, use lu
+        if last_changed_raw is None and last_updated_raw is not None:
+            last_changed_raw = last_updated_raw
+
+        last_changed = _convert_timestamp(last_changed_raw)
+        last_updated = _convert_timestamp(last_updated_raw)
+
+        # Both should have the same value
+        assert last_changed is not None, "last_changed should not be None (issue #447)"
+        assert last_updated is not None
+        assert last_changed == "2023-11-14T22:13:20+00:00"
+        assert last_updated == "2023-11-14T22:13:20+00:00"
+        assert last_changed == last_updated
+
+    def test_long_form_timestamps(self):
+        """Test long-form timestamp keys (last_changed/last_updated) work."""
+        state = {
+            "state": "on",
+            "last_changed": "2026-01-17T12:00:00+00:00",
+            "last_updated": "2026-01-17T12:00:00+00:00",
+            "attributes": {},
+        }
+
+        last_updated_raw = state.get("lu") or state.get("last_updated")
+        last_changed_raw = state.get("lc") or state.get("last_changed")
+
+        if last_changed_raw is None and last_updated_raw is not None:
+            last_changed_raw = last_updated_raw
+
+        last_changed = _convert_timestamp(last_changed_raw)
+        last_updated = _convert_timestamp(last_updated_raw)
+
+        assert last_changed == "2026-01-17T12:00:00+00:00"
+        assert last_updated == "2026-01-17T12:00:00+00:00"
+
+    def test_long_form_lc_omitted(self):
+        """Test long-form with last_changed omitted."""
+        state = {
+            "state": "on",
+            # 'last_changed' omitted
+            "last_updated": "2026-01-17T12:00:00+00:00",
+            "attributes": {},
+        }
+
+        last_updated_raw = state.get("lu") or state.get("last_updated")
+        last_changed_raw = state.get("lc") or state.get("last_changed")
+
+        if last_changed_raw is None and last_updated_raw is not None:
+            last_changed_raw = last_updated_raw
+
+        last_changed = _convert_timestamp(last_changed_raw)
+        last_updated = _convert_timestamp(last_updated_raw)
+
+        assert last_changed is not None
+        assert last_updated is not None
+        assert last_changed == last_updated
+
+    def test_no_timestamps_at_all(self):
+        """Test edge case where no timestamps present (shouldn't happen in practice)."""
+        state = {
+            "s": "on",
+            "a": {},
+        }
+
+        last_updated_raw = state.get("lu") or state.get("last_updated")
+        last_changed_raw = state.get("lc") or state.get("last_changed")
+
+        if last_changed_raw is None and last_updated_raw is not None:
+            last_changed_raw = last_updated_raw
+
+        last_changed = _convert_timestamp(last_changed_raw)
+        last_updated = _convert_timestamp(last_updated_raw)
+
+        # Both should be None in this edge case
+        assert last_changed is None
+        assert last_updated is None