From 00f4cebc1df6fe4f9ee9068e82eaffbb9aafafcc Mon Sep 17 00:00:00 2001 From: ichwars Date: Tue, 8 Sep 2026 10:12:11 +0200 Subject: [PATCH 1/2] fix(ha): bound persisted sensor states to column width --- .github/workflows/ci.yml | 3 + backend/app/models/printer_ha_sensor.py | 4 +- backend/app/services/ha_sensor_manager.py | 17 ++- .../test_issue_143_ha_sensor_persistence.py | 140 ++++++++++++++++++ .../tests/unit/test_ha_sensor_manager_1148.py | 40 +++++ 5 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 backend/tests/integration/test_issue_143_ha_sensor_persistence.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24aa2078a0..c7c97ae23a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,6 +190,9 @@ jobs: - name: Run live PostgreSQL migration tests run: python -m pytest backend/tests/postgres/test_issue_142_postgres_runtime.py -q + - name: Run live PostgreSQL sensor persistence tests + run: python -m pytest backend/tests/integration/test_issue_143_ha_sensor_persistence.py -q + # ============================================================================ # Frontend Checks # ============================================================================ diff --git a/backend/app/models/printer_ha_sensor.py b/backend/app/models/printer_ha_sensor.py index 5743cdda49..cb53933aa0 100644 --- a/backend/app/models/printer_ha_sensor.py +++ b/backend/app/models/printer_ha_sensor.py @@ -5,6 +5,8 @@ from backend.app.core.database import Base +LAST_STATE_MAX_LENGTH = 64 + class PrinterHASensor(Base): """A read-only Home Assistant entity bound to a printer (#1148, #448). @@ -62,7 +64,7 @@ class PrinterHASensor(Base): # Last poll result. Persisted so a restart doesn't blank the card until the # first poll lands, and so notifications only fire on a real transition. - last_state: Mapped[str | None] = mapped_column(String(64), nullable=True) + last_state: Mapped[str | None] = mapped_column(String(LAST_STATE_MAX_LENGTH), nullable=True) last_changed: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) last_checked: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) diff --git a/backend/app/services/ha_sensor_manager.py b/backend/app/services/ha_sensor_manager.py index b21aec2c3e..7a25a114ca 100644 --- a/backend/app/services/ha_sensor_manager.py +++ b/backend/app/services/ha_sensor_manager.py @@ -24,7 +24,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from backend.app.models.printer import Printer -from backend.app.models.printer_ha_sensor import PrinterHASensor +from backend.app.models.printer_ha_sensor import LAST_STATE_MAX_LENGTH, PrinterHASensor from backend.app.services.homeassistant import as_float, homeassistant_service from backend.app.utils.local_time import utcnow_naive @@ -47,6 +47,11 @@ class SensorReading: reachable: bool +def persistable_state(state: str | None, max_length: int) -> str | None: + """Fit a raw HA state into a bounded ``last_state`` column.""" + return state[:max_length] if state is not None else None + + @dataclass(frozen=True) class InterlockOverride: username: str @@ -224,8 +229,9 @@ async def refresh_one(self, db: AsyncSession, sensor: PrinterHASensor): self._last_alerting[sensor.id] = reading.alerting sensor.last_checked = utcnow_naive() - if reading.reachable and sensor.last_state != reading.state: - sensor.last_state = reading.state + persisted = persistable_state(reading.state, LAST_STATE_MAX_LENGTH) + if reading.reachable and sensor.last_state != persisted: + sensor.last_state = persisted sensor.last_changed = sensor.last_checked await db.commit() await db.refresh(sensor) @@ -258,8 +264,9 @@ async def _apply(self, db: AsyncSession, sensors: list[PrinterHASensor], states: sensor.last_checked = now if reading.reachable: - if sensor.last_state != reading.state: - sensor.last_state = reading.state + persisted = persistable_state(reading.state, LAST_STATE_MAX_LENGTH) + if sensor.last_state != persisted: + sensor.last_state = persisted sensor.last_changed = now # Notify on the edge into alerting only. `was_alerting is None` is diff --git a/backend/tests/integration/test_issue_143_ha_sensor_persistence.py b/backend/tests/integration/test_issue_143_ha_sensor_persistence.py new file mode 100644 index 0000000000..7453f659f3 --- /dev/null +++ b/backend/tests/integration/test_issue_143_ha_sensor_persistence.py @@ -0,0 +1,140 @@ +"""Database coverage for bounded Home Assistant sensor state persistence (#143).""" + +from __future__ import annotations + +import os +from uuid import uuid4 + +import pytest +from sqlalchemy import delete, select + +from backend.app.models.printer import Printer +from backend.app.models.printer_ha_sensor import PrinterHASensor +from backend.app.services.ha_sensor_manager import HASensorManager + +POSTGRES_TEST_URL = os.environ.get("PRINTOPS_POSTGRES_TEST_URL") +LAST_STATE_WIDTH = 64 + + +def _sensors(printer_id: int) -> list[PrinterHASensor]: + return [ + PrinterHASensor( + printer_id=printer_id, + name="Long text", + entity_id="binary_sensor.issue_143_long", + kind="binary", + device_class="door", + alert_state="on", + ), + PrinterHASensor( + printer_id=printer_id, + name="Temperature", + entity_id="sensor.issue_143_temperature", + kind="numeric", + device_class="temperature", + alert_above=35, + block_print=True, + failure_strategy="fail_closed", + last_state="21.5", + ), + PrinterHASensor( + printer_id=printer_id, + name="Peer door", + entity_id="binary_sensor.issue_143_peer", + kind="binary", + device_class="door", + alert_state="on", + last_state="off", + ), + PrinterHASensor( + printer_id=printer_id, + name="Long number", + entity_id="sensor.issue_143_long_number", + kind="numeric", + device_class="temperature", + alert_above=35, + ), + ] + + +async def _exercise_sensor_batch(db, printer_id: int) -> None: + sensors = _sensors(printer_id) + db.add_all(sensors) + await db.commit() + + long_state = "x" * 500 + invalid_numeric_state = "calibrating-" + "z" * 200 + long_numeric_state = "0" * 100 + "42" + manager = HASensorManager() + await manager._apply( + db, + sensors, + { + sensors[0].entity_id: {"state": long_state}, + sensors[1].entity_id: {"state": invalid_numeric_state}, + sensors[2].entity_id: {"state": "on"}, + sensors[3].entity_id: {"state": long_numeric_state}, + }, + ) + + db.expunge_all() + stored = { + sensor.name: sensor + for sensor in (await db.execute(select(PrinterHASensor).where(PrinterHASensor.printer_id == printer_id))) + .scalars() + .all() + } + + assert stored["Long text"].last_state == "x" * LAST_STATE_WIDTH + assert manager.get_reading(sensors[0].id).state == long_state + assert stored["Temperature"].last_state == "21.5" + invalid_reading = manager.get_reading(sensors[1].id) + assert invalid_reading.state == invalid_numeric_state + assert invalid_reading.reachable is False + assert stored["Peer door"].last_state == "on" + assert stored["Peer door"].last_checked is not None + assert stored["Long number"].last_state == "0" * LAST_STATE_WIDTH + numeric_reading = manager.get_reading(sensors[3].id) + assert numeric_reading.state == long_numeric_state + assert numeric_reading.value == 42 + assert numeric_reading.alerting is True + assert await manager.blocked_printers(db) == {printer_id: "Temperature (unavailable)"} + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_sensor_batch_persists_safely_on_sqlite(db_session, printer_factory): + printer = await printer_factory(serial_number="ISSUE143SQLITE") + + await _exercise_sensor_batch(db_session, printer.id) + + +@pytest.mark.asyncio +@pytest.mark.integration +@pytest.mark.skipif(not POSTGRES_TEST_URL, reason="live PostgreSQL test URL not configured") +async def test_sensor_batch_persists_safely_on_postgresql(): + from backend.app.core import database + + assert database.settings.database_url == POSTGRES_TEST_URL + await database.init_db() + + async with database.async_session() as db: + printer = Printer( + name="Issue 143 PostgreSQL", + serial_number=f"ISSUE143-{uuid4().hex}", + ip_address="192.0.2.143", + access_code="issue143", + model="X1C", + ) + db.add(printer) + await db.commit() + await db.refresh(printer) + printer_id = printer.id + + try: + await _exercise_sensor_batch(db, printer_id) + finally: + await db.rollback() + await db.execute(delete(PrinterHASensor).where(PrinterHASensor.printer_id == printer_id)) + await db.execute(delete(Printer).where(Printer.id == printer_id)) + await db.commit() diff --git a/backend/tests/unit/test_ha_sensor_manager_1148.py b/backend/tests/unit/test_ha_sensor_manager_1148.py index 1cb59eac3e..1afdaa2749 100644 --- a/backend/tests/unit/test_ha_sensor_manager_1148.py +++ b/backend/tests/unit/test_ha_sensor_manager_1148.py @@ -306,3 +306,43 @@ async def test_a_dropout_does_not_count_as_the_alert_clearing(self): await self._apply(manager, sensor, {sensor.entity_id: {"state": "on"}}, notify) assert notify.on_ha_sensor_alert.await_count == 1 + + +class TestLastStatePersistence: + """Persisted states fit the column without changing cache semantics (#143).""" + + @pytest.mark.asyncio + async def test_unchanged_long_state_does_not_churn_last_changed(self): + manager = HASensorManager() + unchanged_at = object() + sensor = _sensor( + last_state="x" * 64, + last_changed=unchanged_at, + last_checked=None, + ) + db = AsyncMock() + + with patch("backend.app.services.notification_service.notification_service", AsyncMock()): + await manager._apply(db, [sensor], {sensor.entity_id: {"state": "x" * 500}}) + + assert sensor.last_state == "x" * 64 + assert sensor.last_changed is unchanged_at + assert manager.get_reading(sensor.id).state == "x" * 500 + + @pytest.mark.asyncio + async def test_refresh_one_limits_only_the_persisted_state(self): + manager = HASensorManager() + sensor = _sensor(last_changed=None, last_checked=None) + db = AsyncMock() + + with ( + patch.object(manager, "_configure", AsyncMock(return_value=True)), + patch( + "backend.app.services.ha_sensor_manager.homeassistant_service.fetch_states", + AsyncMock(return_value={sensor.entity_id: {"state": "y" * 300}}), + ), + ): + await manager.refresh_one(db, sensor) + + assert sensor.last_state == "y" * 64 + assert manager.get_reading(sensor.id).state == "y" * 300 From 2a447f954c083317da76d0243832fdb56c77662d Mon Sep 17 00:00:00 2001 From: ichwars Date: Tue, 8 Sep 2026 10:22:06 +0200 Subject: [PATCH 2/2] fix(ha): retain raw sensor transition timestamps --- backend/app/services/ha_sensor_manager.py | 27 +++++++++-------- .../tests/unit/test_ha_sensor_manager_1148.py | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/backend/app/services/ha_sensor_manager.py b/backend/app/services/ha_sensor_manager.py index 7a25a114ca..7bc2076d44 100644 --- a/backend/app/services/ha_sensor_manager.py +++ b/backend/app/services/ha_sensor_manager.py @@ -47,9 +47,17 @@ class SensorReading: reachable: bool -def persistable_state(state: str | None, max_length: int) -> str | None: - """Fit a raw HA state into a bounded ``last_state`` column.""" - return state[:max_length] if state is not None else None +def persist_sensor_reading(sensor: PrinterHASensor, reading: SensorReading, previous: SensorReading | None) -> None: + """Bound storage while preserving observable raw-state transitions.""" + if not reading.reachable: + return + persisted = reading.state[:LAST_STATE_MAX_LENGTH] if reading.state is not None else None + # After a restart/dropout only the bounded DB value can be compared. + # With a valid cached reading, changes beyond the column width count too. + raw_changed = previous is not None and previous.reachable and previous.state != reading.state + if sensor.last_state != persisted or raw_changed: + sensor.last_changed = sensor.last_checked + sensor.last_state = persisted @dataclass(frozen=True) @@ -217,6 +225,7 @@ async def refresh_one(self, db: AsyncSession, sensor: PrinterHASensor): entity, and must not fire another user's notification as a side effect of this one saving a form. """ + previous = self._readings.get(sensor.id) self.forget(sensor.id) if not await self._configure(db): self._readings[sensor.id] = SensorReading(None, None, False, False) @@ -229,10 +238,7 @@ async def refresh_one(self, db: AsyncSession, sensor: PrinterHASensor): self._last_alerting[sensor.id] = reading.alerting sensor.last_checked = utcnow_naive() - persisted = persistable_state(reading.state, LAST_STATE_MAX_LENGTH) - if reading.reachable and sensor.last_state != persisted: - sensor.last_state = persisted - sensor.last_changed = sensor.last_checked + persist_sensor_reading(sensor, reading, previous) await db.commit() await db.refresh(sensor) @@ -260,14 +266,11 @@ async def _apply(self, db: AsyncSession, sensors: list[PrinterHASensor], states: payload = states.get(sensor.entity_id) reading = evaluate(sensor, payload) was_alerting = self._last_alerting.get(sensor.id) + previous = self._readings.get(sensor.id) self._readings[sensor.id] = reading sensor.last_checked = now - if reading.reachable: - persisted = persistable_state(reading.state, LAST_STATE_MAX_LENGTH) - if sensor.last_state != persisted: - sensor.last_state = persisted - sensor.last_changed = now + persist_sensor_reading(sensor, reading, previous) # Notify on the edge into alerting only. `was_alerting is None` is # a cold cache (first poll after a restart) — a door that was diff --git a/backend/tests/unit/test_ha_sensor_manager_1148.py b/backend/tests/unit/test_ha_sensor_manager_1148.py index 1afdaa2749..f31f0325fa 100644 --- a/backend/tests/unit/test_ha_sensor_manager_1148.py +++ b/backend/tests/unit/test_ha_sensor_manager_1148.py @@ -311,6 +311,36 @@ async def test_a_dropout_does_not_count_as_the_alert_clearing(self): class TestLastStatePersistence: """Persisted states fit the column without changing cache semantics (#143).""" + @pytest.mark.parametrize("refresh_single", [False, True]) + @pytest.mark.parametrize("suffix,changed", [("before", False), ("after", True)]) + @pytest.mark.asyncio + async def test_raw_state_transitions_beyond_column_width(self, refresh_single, suffix, changed): + manager = HASensorManager() + previous_changed = object() + sensor = _sensor(last_state="x" * 64, last_changed=previous_changed, last_checked=None) + manager._readings[sensor.id] = evaluate(sensor, {"state": "x" * 64 + "before"}) + states = {sensor.entity_id: {"state": "x" * 64 + suffix}} + db = AsyncMock() + + with ( + patch.object(manager, "_configure", AsyncMock(return_value=True)), + patch( + "backend.app.services.ha_sensor_manager.homeassistant_service.fetch_states", + AsyncMock(return_value=states), + ), + ): + if refresh_single: + await manager.refresh_one(db, sensor) + else: + await manager._apply(db, [sensor], states) + + assert sensor.last_state == "x" * 64 + assert manager.get_reading(sensor.id).state == "x" * 64 + suffix + if changed: + assert sensor.last_changed == sensor.last_checked + else: + assert sensor.last_changed is previous_changed + @pytest.mark.asyncio async def test_unchanged_long_state_does_not_churn_last_changed(self): manager = HASensorManager()