From 10c13d30770f8edc4ef25c8a0a2abeb876e99ddb Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:18:22 -0400
Subject: [PATCH 1/9] fix(38-03): stale-check error handler crashed with
 UnboundLocalError

The lazy `from ... import async_create as pn_create` inside
_async_check_stale_and_rebuild's try block makes `pn_create` a
function-local name for the whole method scope. Any exception raised
before that import ran left the name unbound, so the `except` handler
died with UnboundLocalError instead of posting its error notification.

- Re-import pn_create inside the except handler (additive; keeps the
  sys.modules patchability the tests rely on)
- Add regression test reproducing the path via a tz-naive _last_rebuilt

Verified RED before the fix (UnboundLocalError at coordinator.py:1107),
GREEN after. Full offline suite 818 passed.
---
 custom_components/asp_parking/coordinator.py | 10 +++++
 tests/test_coordinator_stale.py              | 42 ++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/custom_components/asp_parking/coordinator.py b/custom_components/asp_parking/coordinator.py
index 0835c30..0d2ada8 100644
--- a/custom_components/asp_parking/coordinator.py
+++ b/custom_components/asp_parking/coordinator.py
@@ -1098,6 +1098,16 @@ async def _async_check_stale_and_rebuild(self, now: datetime | None = None) -> N
                 "ASP Parking: stale-check/rebuild encountered unexpected error",
                 exc_info=True,
             )
+            # Re-import locally: the lazy import above lives inside the try
+            # block, so `pn_create` is a function-local name that is still
+            # UNBOUND whenever the exception was raised before that import ran
+            # (e.g. a TypeError from the `_last_rebuilt` subtraction).  Without
+            # this, the handler itself dies with UnboundLocalError and the
+            # error notification is never posted.
+            from homeassistant.components.persistent_notification import (
+                async_create as pn_create,
+            )
+
             pn_create(
                 self.hass,
                 "The automatic stale-index check failed unexpectedly. "
diff --git a/tests/test_coordinator_stale.py b/tests/test_coordinator_stale.py
index 02131fd..38d913f 100644
--- a/tests/test_coordinator_stale.py
+++ b/tests/test_coordinator_stale.py
@@ -633,3 +633,45 @@ async def async_save(self, payload):
         f"Corrupt payload should emit a WARNING about index_stale store; "
         f"got {[r.getMessage() for r in caplog.records]!r}"
     )
+
+
+async def test_pre_import_exception_posts_error_notification_not_unbound_local(
+    pn_module: SimpleNamespace, caplog: pytest.LogCaptureFixture
+):
+    """Regression: an exception raised BEFORE the lazy pn_create import must
+    still reach the error-notification path.
+
+    The lazy ``from ... import async_create as pn_create`` lives inside the
+    ``try`` block, which makes ``pn_create`` a function-local name for the
+    WHOLE method scope.  When the exception fires before that import runs, the
+    ``except`` handler used to die with ``UnboundLocalError`` instead of
+    posting its notification.
+
+    A tz-naive ``_last_rebuilt`` is the trigger: ``dt_util.utcnow() - naive``
+    raises TypeError at the subtraction, which precedes the lazy import.
+    ``index_io._sync_read_build_timestamp`` normalises to tz-aware so this is
+    not reachable via ``async_start`` today, but the handler must not depend on
+    that upstream guarantee.
+    """
+    stub = _make_coord_stub_stale(last_rebuilt=datetime(2020, 1, 1, 0, 0, 0))
+    check = _bind(stub, "_async_check_stale_and_rebuild")
+    caplog.set_level(logging.ERROR, logger="custom_components.asp_parking.coordinator")
+
+    # Must not propagate (UnboundLocalError previously escaped the handler).
+    await check()
+
+    # The error notification actually fired, with its own distinct id.
+    error_calls = [
+        c
+        for c in pn_module.async_create.call_args_list
+        if c.kwargs.get("notification_id") == "asp_parking_index_stale_check_error"
+    ]
+    assert len(error_calls) == 1, (
+        f"Expected exactly one stale-check-error notification; got "
+        f"{pn_module.async_create.call_args_list!r}"
+    )
+
+    # No rebuild triggered, but last_stale_check still persisted (finally block).
+    assert stub.async_request_rebuild.await_count == 0
+    assert stub._index_stale_store.async_save.await_count == 1
+    assert caplog.records, "Unexpected-error path should log at ERROR"

From 7a242383f0527861bdd7c7128d6c80ef3e14544a Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:41:24 -0400
Subject: [PATCH 2/9] fix(38): CR-01 fix button-triggered rebuild always
 routing to FROM_SOURCE

async_request_rebuild() overwrote self._last_button_press with "now"
before _async_decide_rebuild_path() read it, making the 24h double-press
check a self-comparison that was always true. Every button press
(including the first) was misrouted to the slow FROM_SOURCE path,
defeating the dual-path routing feature. Fix threads the previous press
snapshot through _async_do_rebuild -> _async_decide_rebuild_path as an
explicit argument instead of re-reading the mutated instance attribute.
Adds a real async_request_rebuild() end-to-end regression test (fired
twice) that the prior isolated-stub tests could not catch.
---
 custom_components/asp_parking/coordinator.py  |  37 +++-
 ...est_coordinator_button_double_press_e2e.py | 164 ++++++++++++++++++
 tests/test_coordinator_path_selection.py      |  17 +-
 3 files changed, 208 insertions(+), 10 deletions(-)
 create mode 100644 tests/test_coordinator_button_double_press_e2e.py

diff --git a/custom_components/asp_parking/coordinator.py b/custom_components/asp_parking/coordinator.py
index 0d2ada8..0dc3130 100644
--- a/custom_components/asp_parking/coordinator.py
+++ b/custom_components/asp_parking/coordinator.py
@@ -685,7 +685,15 @@ async def async_request_rebuild(
         # write cannot bypass the IDX-02 concurrent-press guard (CR-01).
         self._is_rebuilding = True
 
+        # CR-01: snapshot the PREVIOUS press before overwriting so the
+        # 24h double-press check in ``_async_decide_rebuild_path`` compares
+        # "now" against the prior press, not against itself. Threading this
+        # through as an argument (rather than re-reading the mutated
+        # ``self._last_button_press`` instance attribute later) is what
+        # makes the check correct -- see 38-REVIEW.md CR-01.
+        previous_button_press: datetime | None = None
         if triggered_by == "button":
+            previous_button_press = self._last_button_press
             self._last_button_press = dt_util.utcnow()
             if self._index_stale_store is not None:
                 try:
@@ -714,12 +722,19 @@ async def async_request_rebuild(
         # self._async_do_rebuild() since `self` is an ASPParkingCoordinator.
         self._rebuild_task = self.entry.async_create_background_task(
             self.hass,
-            ASPParkingCoordinator._async_do_rebuild(self, triggered_by=triggered_by),
+            ASPParkingCoordinator._async_do_rebuild(
+                self,
+                triggered_by=triggered_by,
+                previous_button_press=previous_button_press,
+            ),
             name="asp_parking_index_rebuild",
         )
 
     async def _async_do_rebuild(
-        self, *, triggered_by: Literal["button", "stale_check"] = "button"
+        self,
+        *,
+        triggered_by: Literal["button", "stale_check"] = "button",
+        previous_button_press: datetime | None = None,
     ) -> None:
         """Background task body — performs the full rebuild lifecycle.
 
@@ -760,7 +775,9 @@ async def _async_do_rebuild(
                 # Phase 38 (IDX-05): decide which executor strategy to run
                 # BEFORE doing any work so the INFO log records intent even
                 # if the chosen path fails.
-                path, reason = await self._async_decide_rebuild_path(triggered_by)
+                path, reason = await self._async_decide_rebuild_path(
+                    triggered_by, previous_button_press
+                )
                 logger.info(
                     "asp_parking: index rebuild path=%s reason=%s",
                     path.value,
@@ -864,7 +881,7 @@ async def _async_do_rebuild(
     # ------------------------------------------------------------------
 
     async def _async_decide_rebuild_path(
-        self, triggered_by: str
+        self, triggered_by: str, previous_button_press: datetime | None = None
     ) -> tuple[RebuildPath, str]:
         """Return ``(path, reason_log_tag)`` for the rebuild router.
 
@@ -876,9 +893,17 @@ async def _async_decide_rebuild_path(
 
         D-03: ``triggered_by="stale_check"`` SKIPS the 24h double-press
         override entirely — that rule is button-only.
+
+        CR-01: ``previous_button_press`` is the value of
+        ``self._last_button_press`` from BEFORE the current press
+        overwrote it (snapshotted by the caller in
+        ``async_request_rebuild``). Re-reading the mutated instance
+        attribute here would always compare "now" against itself, making
+        every button press look like a double-press — see 38-REVIEW.md
+        CR-01.
         """
-        if triggered_by == "button" and self._last_button_press is not None:
-            window = dt_util.utcnow() - self._last_button_press
+        if triggered_by == "button" and previous_button_press is not None:
+            window = dt_util.utcnow() - previous_button_press
             if window < timedelta(hours=BUTTON_DOUBLE_PRESS_WINDOW_HOURS):
                 return RebuildPath.FROM_SOURCE, "double_press"
 
diff --git a/tests/test_coordinator_button_double_press_e2e.py b/tests/test_coordinator_button_double_press_e2e.py
new file mode 100644
index 0000000..6a0dea1
--- /dev/null
+++ b/tests/test_coordinator_button_double_press_e2e.py
@@ -0,0 +1,164 @@
+"""CR-01 regression test (38-REVIEW.md): real end-to-end button-press routing.
+
+Prior to the CR-01 fix, ``async_request_rebuild()`` overwrote
+``self._last_button_press`` with "now" *before* spawning
+``_async_do_rebuild()``, which in turn called
+``_async_decide_rebuild_path()``. That method re-read the (already
+mutated) ``self._last_button_press`` attribute, so the "was there a press
+within the last 24h?" check always compared "now" against itself --
+misrouting *every* button press (including the very first one ever) to
+the slow FROM_SOURCE path.
+
+None of the existing tests in ``test_coordinator_path_selection.py``
+caught this: they either (a) call ``_async_decide_rebuild_path`` directly
+against a stub with a hand-set ``_last_button_press``/``previous_button_press``
+(never going through ``async_request_rebuild``), or (b) call
+``_async_do_rebuild`` directly with ``_async_decide_rebuild_path`` replaced
+by an ``AsyncMock``. This test drives the REAL production call chain
+``async_request_rebuild()`` -> ``_async_do_rebuild()`` ->
+``_async_decide_rebuild_path()`` twice in a row to prove the fix.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import sys
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from custom_components.asp_parking.coordinator import (
+    ASPParkingCoordinator,
+    RebuildPath,
+)
+
+
+def _make_stub() -> SimpleNamespace:
+    """Build a bare coordinator stub with the REAL decision method bound.
+
+    Unlike ``tests/test_coordinator_path_selection.py``'s
+    ``_make_coord_stub`` (which leaves ``_async_decide_rebuild_path``
+    unbound so individual tests can stub it), this stub binds the real
+    method -- that binding is the entire point of this regression test.
+    """
+    entry = SimpleNamespace(
+        entry_id="test_entry_38_cr01_e2e",
+        async_create_background_task=MagicMock(),
+    )
+    # Plain AsyncMock (no side_effect): executor jobs are recorded but
+    # never actually run, so this test does not depend on real on-disk
+    # index files / network access.
+    hass = SimpleNamespace(async_add_executor_job=AsyncMock())
+    index_stale_store = SimpleNamespace(
+        async_load=AsyncMock(return_value=None),
+        async_save=AsyncMock(),
+    )
+    stub = SimpleNamespace(
+        entry=entry,
+        hass=hass,
+        _is_rebuilding=False,
+        _rebuild_task=None,
+        _rebuild_lock=asyncio.Lock(),
+        _last_rebuilt=None,
+        _sign_cache={},
+        _async_notify_entities=MagicMock(),
+        _index_stale_store=index_stale_store,
+        _last_button_press=None,
+        _last_stale_check=None,
+        _remote_age_cache=None,
+        # Mocked-fresh GitHub remote (< REMOTE_FRESH_DAYS=30): absent the
+        # CR-01 fix, EVERY press would still be misrouted to FROM_SOURCE
+        # via the double-press self-comparison, so this alone would not
+        # prove the fix -- the double_press check must run first and
+        # correctly distinguish "no prior press" from "prior press".
+        _fetch_remote_asset_age_days=AsyncMock(return_value=5.0),
+    )
+    stub._async_decide_rebuild_path = (
+        ASPParkingCoordinator._async_decide_rebuild_path.__get__(
+            stub, ASPParkingCoordinator
+        )
+    )
+    return stub
+
+
+def _install_spies(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Mirror test_coordinator_path_selection.py's _install_path_spies.
+
+    Only patches what runs OUTSIDE the executor (SpatialIndex.reset,
+    persistent_notification) since ``hass.async_add_executor_job`` is a
+    plain (non-dispatching) AsyncMock here.
+    """
+    monkeypatch.setattr(
+        "custom_components.asp_parking.coordinator.SpatialIndex.reset",
+        MagicMock(name="SpatialIndex.reset"),
+        raising=False,
+    )
+    monkeypatch.setitem(
+        sys.modules,
+        "homeassistant.components.persistent_notification",
+        SimpleNamespace(
+            async_create=MagicMock(name="pn_create"),
+            async_dismiss=MagicMock(name="pn_dismiss"),
+        ),
+    )
+
+
+async def test_two_button_presses_first_download_second_double_press(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """1st button press -> DOWNLOAD; 2nd press within 24h -> FROM_SOURCE/double_press.
+
+    Before the CR-01 fix this test failed on the FIRST assertion: the
+    first-ever button press was already misrouted to FROM_SOURCE because
+    ``_last_button_press`` had just been set to "now" by
+    ``async_request_rebuild`` before ``_async_decide_rebuild_path`` read it.
+    """
+    _install_spies(monkeypatch)
+    stub = _make_stub()
+
+    decisions: list[tuple[RebuildPath, str]] = []
+    real_decide = stub._async_decide_rebuild_path
+
+    async def _spy_decide(triggered_by, previous_button_press=None):
+        result = await real_decide(triggered_by, previous_button_press)
+        decisions.append(result)
+        return result
+
+    stub._async_decide_rebuild_path = _spy_decide
+
+    captured_coros: list = []
+
+    def _capture_spawn(hass, coro, *, name=None):
+        captured_coros.append(coro)
+
+    stub.entry.async_create_background_task.side_effect = _capture_spawn
+
+    request_rebuild = ASPParkingCoordinator.async_request_rebuild.__get__(
+        stub, ASPParkingCoordinator
+    )
+
+    # -- First press: no prior press exists ------------------------------
+    await request_rebuild(triggered_by="button")
+    assert len(captured_coros) == 1
+    await captured_coros[0]  # run _async_do_rebuild -> _async_decide_rebuild_path
+    assert stub._is_rebuilding is False, (
+        "finally block must reset _is_rebuilding so the button is usable again"
+    )
+
+    assert decisions[0] == (RebuildPath.DOWNLOAD, "remote_fresh"), (
+        "CR-01 regression: the first-ever button press with a fresh remote "
+        f"must resolve to DOWNLOAD, got {decisions[0]!r} -- if this is "
+        "FROM_SOURCE/double_press, self-comparison bug is back"
+    )
+
+    # -- Second press: within the 24h double-press window ----------------
+    await request_rebuild(triggered_by="button")
+    assert len(captured_coros) == 2
+    await captured_coros[1]
+    assert stub._is_rebuilding is False
+
+    assert decisions[1] == (RebuildPath.FROM_SOURCE, "double_press"), (
+        f"Second button press within 24h must be FROM_SOURCE/double_press, "
+        f"got {decisions[1]!r}"
+    )
diff --git a/tests/test_coordinator_path_selection.py b/tests/test_coordinator_path_selection.py
index 8d6c2c9..7a7b051 100644
--- a/tests/test_coordinator_path_selection.py
+++ b/tests/test_coordinator_path_selection.py
@@ -205,11 +205,20 @@ async def test_press_remote_exactly_30_days_uses_from_source():
 
 
 async def test_double_press_within_24h_uses_from_source():
-    """SPEC AC: second press within 24h -> FROM_SOURCE regardless of remote age."""
+    """SPEC AC: second press within 24h -> FROM_SOURCE regardless of remote age.
+
+    CR-01: ``previous_button_press`` is now an explicit argument (the
+    caller's snapshot of the PRIOR press) rather than being re-read from
+    ``self._last_button_press`` — see 38-REVIEW.md CR-01. This isolated
+    unit test exercises the decision function's contract directly; the
+    end-to-end double-press behavior through the real
+    ``async_request_rebuild`` call chain is covered by
+    ``test_coordinator_button_double_press_e2e.py``.
+    """
     recent = datetime.now(timezone.utc) - timedelta(hours=2)
     stub = _make_coord_stub(remote_age_days=5.0, last_button_press=recent)
     decide = _bind(stub, "_async_decide_rebuild_path")
-    path, reason = await decide("button")
+    path, reason = await decide("button", recent)
     assert path == RebuildPath.FROM_SOURCE
     assert reason == "double_press"
 
@@ -219,7 +228,7 @@ async def test_press_after_24h_window_uses_download():
     past = datetime.now(timezone.utc) - timedelta(hours=25)
     stub = _make_coord_stub(remote_age_days=5.0, last_button_press=past)
     decide = _bind(stub, "_async_decide_rebuild_path")
-    path, reason = await decide("button")
+    path, reason = await decide("button", past)
     assert path == RebuildPath.DOWNLOAD
     assert reason == "remote_fresh"
 
@@ -229,7 +238,7 @@ async def test_stale_check_triggered_by_skips_24h_override():
     recent = datetime.now(timezone.utc) - timedelta(hours=2)
     stub = _make_coord_stub(remote_age_days=5.0, last_button_press=recent)
     decide = _bind(stub, "_async_decide_rebuild_path")
-    path, reason = await decide("stale_check")
+    path, reason = await decide("stale_check", recent)
     assert path == RebuildPath.DOWNLOAD
     assert reason == "remote_fresh"
 

From fd3db64439cab7b5c1fdb818adf0e7a84e6aeac4 Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:43:44 -0400
Subject: [PATCH 3/9] fix(38): WR-01 verify newly-swapped-in index before
 trusting it

_async_do_rebuild went straight from _sync_atomic_swap to posting a
success notification without ever calling the existing
_sync_verify_index() / IndexIntegrityError integrity check (already used
by the first-time-setup path). A partial/corrupt write (disk full
mid-write, SD-card corruption, truncated extraction) would be silently
promoted to the live index with a false "success" message. Adds the
verify call right after the swap so a raised IndexIntegrityError is
caught by the existing except block and reported as a normal rebuild
failure instead of an opaque rtree crash later. Updates test spy helpers
in the affected suites to stub the new call.
---
 custom_components/asp_parking/coordinator.py | 10 +++
 tests/test_coordinator_path_selection.py     | 84 ++++++++++++++++++++
 tests/test_coordinator_rebuild.py            |  5 ++
 3 files changed, 99 insertions(+)

diff --git a/custom_components/asp_parking/coordinator.py b/custom_components/asp_parking/coordinator.py
index 0dc3130..8ed363a 100644
--- a/custom_components/asp_parking/coordinator.py
+++ b/custom_components/asp_parking/coordinator.py
@@ -120,6 +120,7 @@
     _sync_cleanup_stale,
     _sync_download_and_extract,
     _sync_read_build_timestamp,
+    _sync_verify_index,
 )
 
 if TYPE_CHECKING:
@@ -800,6 +801,15 @@ async def _async_do_rebuild(
 
                 await self.hass.async_add_executor_job(_sync_atomic_swap, INDEX_DIR)
 
+                # WR-01: verify the newly-swapped-in index BEFORE trusting it
+                # and posting a success notification. A partial/corrupt write
+                # (disk full mid-write, SD-card corruption, truncated
+                # extraction) must be caught here -- not later as an opaque
+                # IndexIntegrityError/rtree crash from the resolve pipeline.
+                # A raised IndexIntegrityError is caught by the ``except``
+                # block below and reported as a normal rebuild failure.
+                await self.hass.async_add_executor_job(_sync_verify_index, INDEX_DIR)
+
                 # RESEARCH Pitfall 2: reset MUST happen AFTER atomic_swap so the
                 # next SpatialIndex.get() re-opens the new files. reset() just
                 # closes the rtree handle and nulls the singleton — safe to
diff --git a/tests/test_coordinator_path_selection.py b/tests/test_coordinator_path_selection.py
index 7a7b051..f33f1db 100644
--- a/tests/test_coordinator_path_selection.py
+++ b/tests/test_coordinator_path_selection.py
@@ -122,6 +122,7 @@ def _install_path_spies(monkeypatch: pytest.MonkeyPatch) -> dict:
     download_and_extract = MagicMock(name="_sync_download_and_extract")
     build_from_source = MagicMock(name="_sync_build_from_source")
     atomic_swap = MagicMock(name="_sync_atomic_swap")
+    verify_index = MagicMock(name="_sync_verify_index")
     read_build_timestamp = MagicMock(
         name="_sync_read_build_timestamp", return_value=None
     )
@@ -134,6 +135,9 @@ def _install_path_spies(monkeypatch: pytest.MonkeyPatch) -> dict:
         coord_mod, "_sync_build_from_source", build_from_source, raising=False
     )
     monkeypatch.setattr(coord_mod, "_sync_atomic_swap", atomic_swap, raising=False)
+    # WR-01: coordinator now verifies the swapped-in index before trusting
+    # it; stub this out so tests don't depend on real on-disk index files.
+    monkeypatch.setattr(coord_mod, "_sync_verify_index", verify_index, raising=False)
     monkeypatch.setattr(
         coord_mod, "_sync_read_build_timestamp", read_build_timestamp, raising=False
     )
@@ -161,6 +165,7 @@ def _install_path_spies(monkeypatch: pytest.MonkeyPatch) -> dict:
         "download_and_extract": download_and_extract,
         "build_from_source": build_from_source,
         "atomic_swap": atomic_swap,
+        "verify_index": verify_index,
         "read_build_timestamp": read_build_timestamp,
         "spatial_index_reset": spatial_index_reset,
         "pn_create": pn_create,
@@ -560,3 +565,82 @@ async def _executor_dispatch(fn, *args, **kwargs):
         f"Expected INFO log matching {pattern.pattern!r}; got "
         f"{[r.getMessage() for r in caplog.records]!r}"
     )
+
+
+# ---------------------------------------------------------------------------
+# WR-01: newly-swapped-in index must be integrity-checked before trust
+# ---------------------------------------------------------------------------
+
+
+async def test_do_rebuild_verifies_index_after_atomic_swap(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """_sync_verify_index MUST run after _sync_atomic_swap, before success."""
+    stub = _make_coord_stub(is_rebuilding=True)
+    spies = _install_path_spies(monkeypatch)
+
+    async def _executor_dispatch(fn, *args, **kwargs):
+        return fn(*args, **kwargs)
+
+    stub.hass.async_add_executor_job.side_effect = _executor_dispatch
+    stub._async_decide_rebuild_path = AsyncMock(
+        return_value=(RebuildPath.DOWNLOAD, "remote_fresh")
+    )
+
+    call_order: list[str] = []
+    spies["atomic_swap"].side_effect = lambda *a, **k: call_order.append(
+        "atomic_swap"
+    )
+    spies["verify_index"].side_effect = lambda *a, **k: call_order.append(
+        "verify_index"
+    )
+
+    do_rebuild = _bind(stub, "_async_do_rebuild")
+    await do_rebuild(triggered_by="button")
+
+    assert spies["verify_index"].call_count == 1, (
+        "WR-01: _sync_verify_index MUST be called once after the atomic swap"
+    )
+    assert call_order == ["atomic_swap", "verify_index"], (
+        f"verify_index MUST run AFTER atomic_swap, got order {call_order!r}"
+    )
+    assert spies["pn_create"].call_args_list[-1].kwargs.get(
+        "notification_id"
+    ) == "asp_parking_index_rebuild_success", (
+        "A valid index MUST still post the success notification"
+    )
+
+
+async def test_do_rebuild_treats_integrity_failure_as_rebuild_failure(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """WR-01: a raised IndexIntegrityError is reported as a rebuild failure,
+    not silently promoted to a success notification.
+    """
+    from custom_components.asp_parking.index_io import IndexIntegrityError
+
+    stub = _make_coord_stub(is_rebuilding=True)
+    spies = _install_path_spies(monkeypatch)
+
+    async def _executor_dispatch(fn, *args, **kwargs):
+        return fn(*args, **kwargs)
+
+    stub.hass.async_add_executor_job.side_effect = _executor_dispatch
+    stub._async_decide_rebuild_path = AsyncMock(
+        return_value=(RebuildPath.DOWNLOAD, "remote_fresh")
+    )
+    spies["verify_index"].side_effect = IndexIntegrityError("corrupt rtree")
+
+    do_rebuild = _bind(stub, "_async_do_rebuild")
+    await do_rebuild(triggered_by="button")
+
+    notification_ids = [
+        call.kwargs.get("notification_id")
+        for call in spies["pn_create"].call_args_list
+    ]
+    assert "asp_parking_index_rebuild_error" in notification_ids, (
+        "A corrupt post-swap index MUST post the rebuild-error notification"
+    )
+    assert "asp_parking_index_rebuild_success" not in notification_ids, (
+        "A corrupt post-swap index MUST NOT post the success notification"
+    )
diff --git a/tests/test_coordinator_rebuild.py b/tests/test_coordinator_rebuild.py
index 181c8d7..2543f29 100644
--- a/tests/test_coordinator_rebuild.py
+++ b/tests/test_coordinator_rebuild.py
@@ -191,6 +191,7 @@ def _install_executor_spies(
     cleanup_stale = MagicMock(name="_sync_cleanup_stale")
     download_and_extract = MagicMock(name="_sync_download_and_extract")
     atomic_swap = MagicMock(name="_sync_atomic_swap")
+    verify_index = MagicMock(name="_sync_verify_index")
     read_build_timestamp = MagicMock(
         name="_sync_read_build_timestamp", return_value=build_timestamp_return
     )
@@ -203,6 +204,9 @@ def _install_executor_spies(
         coord_mod, "_sync_download_and_extract", download_and_extract, raising=False
     )
     monkeypatch.setattr(coord_mod, "_sync_atomic_swap", atomic_swap, raising=False)
+    # WR-01: coordinator now verifies the swapped-in index before trusting
+    # it; stub this out so tests don't depend on real on-disk index files.
+    monkeypatch.setattr(coord_mod, "_sync_verify_index", verify_index, raising=False)
     monkeypatch.setattr(
         coord_mod, "_sync_read_build_timestamp", read_build_timestamp, raising=False
     )
@@ -232,6 +236,7 @@ def _install_executor_spies(
         "cleanup_stale": cleanup_stale,
         "download_and_extract": download_and_extract,
         "atomic_swap": atomic_swap,
+        "verify_index": verify_index,
         "read_build_timestamp": read_build_timestamp,
         "spatial_index_reset": spatial_index_reset,
         "pn_create": pn_create,

From 56536dfa526b3af1ef9910da8def7cbc90f75329 Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:45:27 -0400
Subject: [PATCH 4/9] fix(38): WR-04 dismiss stale-index notification on
 rebuild failure

_async_check_stale_and_rebuild posts the "index is stale, auto-rebuilding"
notification then awaits async_request_rebuild(triggered_by="stale_check").
If the rebuild failed, _async_do_rebuild's except block dismissed only
"asp_parking_index_rebuild" and posted "asp_parking_index_rebuild_error" --
it never dismissed "asp_parking_index_stale". Users ended up with a stale,
now-inaccurate "auto-rebuilding" banner sitting alongside the new
"Rebuild Failed" notification until the next 24h stale-check cycle.
Dismissing it is idempotent (safe even when the notification was never
posted, e.g. button-triggered rebuilds).
---
 custom_components/asp_parking/coordinator.py |  7 ++++
 tests/test_coordinator_rebuild.py            | 44 ++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/custom_components/asp_parking/coordinator.py b/custom_components/asp_parking/coordinator.py
index 8ed363a..4f367c5 100644
--- a/custom_components/asp_parking/coordinator.py
+++ b/custom_components/asp_parking/coordinator.py
@@ -845,6 +845,13 @@ async def _async_do_rebuild(
 
             except Exception as err:  # noqa: BLE001
                 pn_dismiss(self.hass, "asp_parking_index_rebuild")
+                # WR-04: dismiss the "index is stale, auto-rebuilding" banner
+                # on failure too -- otherwise it lingers alongside the new
+                # "Rebuild Failed" notification until the next stale-check
+                # cycle happens to re-post it. Idempotent to dismiss a
+                # notification that was never posted (button-triggered
+                # rebuilds never create this one).
+                pn_dismiss(self.hass, "asp_parking_index_stale")
                 if isinstance(err, OSError):
                     if err.strerror and err.filename:
                         _err_summary = f"{err.strerror} ({err.filename})"
diff --git a/tests/test_coordinator_rebuild.py b/tests/test_coordinator_rebuild.py
index 2543f29..6194900 100644
--- a/tests/test_coordinator_rebuild.py
+++ b/tests/test_coordinator_rebuild.py
@@ -516,6 +516,50 @@ async def _executor_dispatch(fn, *args, **kwargs):
     )
 
 
+async def test_async_do_rebuild_failure_path_dismisses_stale_notification(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """WR-01/WR-04 (38-REVIEW.md): a failed rebuild MUST also dismiss the
+    'asp_parking_index_stale' banner, not just 'asp_parking_index_rebuild'.
+
+    Before the WR-04 fix, only the success path dismissed
+    'asp_parking_index_stale'. A rebuild triggered by the stale-check flow
+    that then FAILED would leave the "auto-rebuilding" banner dangling
+    alongside the new "Rebuild Failed" notification.
+    """
+    stub = _make_coord_stub(is_rebuilding=True)
+    spies = _install_executor_spies(
+        monkeypatch,
+        download_raises=RuntimeError("network down"),
+    )
+
+    async def _executor_dispatch(fn, *args, **kwargs):
+        return fn(*args, **kwargs)
+
+    stub.hass.async_add_executor_job.side_effect = _executor_dispatch
+
+    do_rebuild = _bind(stub, "_async_do_rebuild")
+    await do_rebuild(triggered_by="stale_check")  # MUST NOT raise
+
+    dismiss_ids_positional = {
+        c.args[1] for c in spies["pn_dismiss"].call_args_list if len(c.args) > 1
+    }
+    dismiss_ids_kwarg = {
+        c.kwargs.get("notification_id")
+        for c in spies["pn_dismiss"].call_args_list
+        if "notification_id" in c.kwargs
+    }
+    all_dismiss_ids = dismiss_ids_positional | dismiss_ids_kwarg
+
+    assert "asp_parking_index_stale" in all_dismiss_ids, (
+        "WR-04: a failed rebuild MUST dismiss 'asp_parking_index_stale' too, "
+        f"got: {all_dismiss_ids}"
+    )
+    assert "asp_parking_index_rebuild" in all_dismiss_ids, (
+        f"In-progress notification must still be dismissed; got: {all_dismiss_ids}"
+    )
+
+
 # ---------------------------------------------------------------------------
 # New edge-case tests (appended)
 # ---------------------------------------------------------------------------

From 2044c0f27793ff19ce932cf682de422bf7e0c94c Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:49:47 -0400
Subject: [PATCH 5/9] fix(38): WR-02 add pagination cap to SODA ASP-signs
 fetcher

_sync_fetch_asp_signs paginated the SODA parking-signs endpoint with the
same while-True/offset-increment shape as _sync_fetch_cscl_features but
had no equivalent MAX_CSCL_PAGES-style cap. A misbehaving or compromised
SODA endpoint that kept returning exactly SIGNS_BATCH_SIZE records would
loop indefinitely in the executor thread. Adds MAX_SIGNS_PAGES (mirroring
MAX_CSCL_PAGES) and breaks with a warning + partial results once the cap
is hit, preserving the fetcher's existing fail-soft contract (unlike CSCL's
fail-hard RuntimeError).
---
 custom_components/asp_parking/const.py    |  4 ++
 custom_components/asp_parking/index_io.py | 19 +++++++++
 tests/test_index_io_build_from_source.py  | 51 ++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/custom_components/asp_parking/const.py b/custom_components/asp_parking/const.py
index 45bed46..18e3afe 100644
--- a/custom_components/asp_parking/const.py
+++ b/custom_components/asp_parking/const.py
@@ -88,6 +88,10 @@
 MAX_CSCL_PAGES = 30
 CSCL_BATCH_SIZE = 10000
 SIGNS_BATCH_SIZE = 50000
+# WR-02 (38-REVIEW.md): SODA ASP-signs pagination DoS guard, mirroring
+# MAX_CSCL_PAGES -- without this, a misbehaving/compromised SODA endpoint
+# that keeps returning exactly SIGNS_BATCH_SIZE records loops forever.
+MAX_SIGNS_PAGES = 30
 
 # Vehicular street filter (CSCL RW_TYPE codes)
 VEHICULAR_RW_TYPES = frozenset({1, 2, 3, 4, 5})
diff --git a/custom_components/asp_parking/index_io.py b/custom_components/asp_parking/index_io.py
index 6e3221b..e14be77 100644
--- a/custom_components/asp_parking/index_io.py
+++ b/custom_components/asp_parking/index_io.py
@@ -53,6 +53,7 @@
     CSCL_BATCH_SIZE,
     CSCL_GEOJSON_URL,
     MAX_CSCL_PAGES,
+    MAX_SIGNS_PAGES,
     SIGNS_BATCH_SIZE,
     SODA_PARKING_SIGNS_URL,
     VEHICULAR_RW_TYPES,
@@ -780,12 +781,29 @@ def _sync_fetch_asp_signs(
     """Fetch ASP sign block-faces. Fail-soft on httpx.HTTPError (T-38-01-02)."""
     asp_tuples: set[tuple[str, str, str, str]] = set()
     offset = 0
+    page_count = 0
 
     try:
         with httpx.Client(
             timeout=300, headers=headers, follow_redirects=True
         ) as client:
             while True:
+                # WR-02 (38-REVIEW.md): mirror the CSCL fetcher's
+                # MAX_CSCL_PAGES DoS guard. Without a cap, a misbehaving or
+                # compromised SODA endpoint that keeps returning exactly
+                # SIGNS_BATCH_SIZE records loops forever in the executor
+                # thread -- unlike CSCL (fail-hard RuntimeError), the
+                # signs fetch is fail-soft, so this breaks with partial
+                # results instead of raising.
+                if page_count >= MAX_SIGNS_PAGES:
+                    logger.warning(
+                        "SODA ASP signs pagination exceeded MAX_SIGNS_PAGES=%d "
+                        "(offset=%d); using partial results",
+                        MAX_SIGNS_PAGES,
+                        offset,
+                    )
+                    break
+
                 params = {
                     "$where": (
                         "sign_description LIKE '%SANITATION BROOM%'"
@@ -810,6 +828,7 @@ def _sync_fetch_asp_signs(
                     side = (record.get("side_of_street") or "").upper().strip()
                     if on_street and side:
                         asp_tuples.add((on_street, from_street, to_street, side))
+                page_count += 1
                 if len(records) < SIGNS_BATCH_SIZE:
                     break
                 offset += SIGNS_BATCH_SIZE
diff --git a/tests/test_index_io_build_from_source.py b/tests/test_index_io_build_from_source.py
index f348e7f..b4780d5 100644
--- a/tests/test_index_io_build_from_source.py
+++ b/tests/test_index_io_build_from_source.py
@@ -33,7 +33,10 @@
     MAX_CSCL_PAGES,
     SODA_PARKING_SIGNS_URL,
 )
-from custom_components.asp_parking.index_io import _sync_build_from_source
+from custom_components.asp_parking.index_io import (
+    _sync_build_from_source,
+    _sync_fetch_asp_signs,
+)
 
 FIXTURE_DIR = Path(__file__).parent / "fixtures"
 
@@ -183,6 +186,52 @@ def test_pagination_cap_raises(tmp_path: Path, monkeypatch) -> None:
         _sync_build_from_source(index_dir)
 
 
+@respx.mock
+def test_soda_signs_pagination_cap_stops_instead_of_looping_forever(
+    monkeypatch,
+) -> None:
+    """WR-02 (38-REVIEW.md): a misbehaving SODA endpoint that keeps returning
+    full-size batches must stop at the pagination cap instead of looping
+    forever.
+
+    Unlike the CSCL fetcher (fail-hard RuntimeError), the SODA signs fetch is
+    fail-soft (T-38-01-02) -- it must return partial results, not raise.
+
+    Batch size / page cap are monkeypatched small (5 records / 3 pages) so
+    this test runs fast while still exercising the real cap-check codepath
+    in ``_sync_fetch_asp_signs`` (which reads the module-level constants at
+    call time).
+    """
+    monkeypatch.delenv("NYC_OPEN_DATA_APP_TOKEN", raising=False)
+    small_batch_size = 5
+    small_page_cap = 3
+    monkeypatch.setattr(
+        "custom_components.asp_parking.index_io.SIGNS_BATCH_SIZE", small_batch_size
+    )
+    monkeypatch.setattr(
+        "custom_components.asp_parking.index_io.MAX_SIGNS_PAGES", small_page_cap
+    )
+
+    base_record = _load_soda_fixture()[0]
+    full_batch = [dict(base_record) for _ in range(small_batch_size)]
+
+    # Mount enough full-batch responses to exceed the cap; if the guard is
+    # missing, respx will exhaust these routes and raise instead of hanging,
+    # which is itself proof the guard is required.
+    responses = [
+        httpx.Response(200, json=full_batch) for _ in range(small_page_cap + 2)
+    ]
+    route = respx.get(SODA_PARKING_SIGNS_URL).mock(side_effect=responses)
+
+    result = _sync_fetch_asp_signs(headers={})
+
+    assert route.call_count == small_page_cap, (
+        f"Expected pagination to stop at exactly the page cap="
+        f"{small_page_cap} requests, got {route.call_count}"
+    )
+    assert isinstance(result, set)
+
+
 @respx.mock
 def test_soda_failure_is_fail_soft(tmp_path: Path, monkeypatch) -> None:
     """SODA HTTP 500 must NOT raise — has_asp lookup is empty but the build completes."""

From e1097e76d120c7c87eeee6f05984d193faa48aa1 Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:51:04 -0400
Subject: [PATCH 6/9] fix(38): WR-03 guard SODA ASP-signs fetcher against
 non-list JSON body

_sync_fetch_cscl_features guards its response shape
(features = body.get(...) if isinstance(body, dict) else []) but
_sync_fetch_asp_signs did not: `records = resp.json()` followed by
`if not records: break` then `for record in records:`. A truthy
non-list JSON body (e.g. {"error": "..."} on an HTTP-200 soft error)
made `not records` False, so the loop iterated the dict's string KEYS
and `record.get(...)` raised AttributeError -- an exception type not in
the caught tuple, turning the documented "SODA outages must NOT block a
CSCL rebuild" fail-soft contract (T-38-01-02) into a hard failure of the
entire from-source rebuild. Adds an isinstance(records, list) guard that
treats a non-list body as "no more data", mirroring the CSCL fetcher.
---
 custom_components/asp_parking/index_io.py | 17 +++++++++++++++++
 tests/test_index_io_build_from_source.py  | 19 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/custom_components/asp_parking/index_io.py b/custom_components/asp_parking/index_io.py
index e14be77..034f908 100644
--- a/custom_components/asp_parking/index_io.py
+++ b/custom_components/asp_parking/index_io.py
@@ -817,6 +817,23 @@ def _sync_fetch_asp_signs(
                 resp = client.get(SODA_PARKING_SIGNS_URL, params=params)
                 resp.raise_for_status()
                 records = resp.json()
+                # WR-03 (38-REVIEW.md): mirror the CSCL fetcher's body-shape
+                # guard. SODA can return a truthy non-list JSON body (e.g.
+                # ``{"error": "..."}`` on an HTTP-200 soft error). Without
+                # this guard, `not records` is False for such a body, so
+                # the loop falls into `for record in records:` -- which
+                # iterates a dict's string KEYS, and `record.get(...)` then
+                # raises AttributeError (not in the caught exception tuple
+                # below), turning a fail-soft SODA outage into a hard
+                # failure of the entire from-source rebuild (T-38-01-02).
+                if not isinstance(records, list):
+                    logger.warning(
+                        "SODA ASP signs response was not a list (offset=%d): "
+                        "%r -- treating as no more data",
+                        offset,
+                        type(records).__name__,
+                    )
+                    break
                 if not records:
                     break
                 for record in records:
diff --git a/tests/test_index_io_build_from_source.py b/tests/test_index_io_build_from_source.py
index b4780d5..c831acf 100644
--- a/tests/test_index_io_build_from_source.py
+++ b/tests/test_index_io_build_from_source.py
@@ -232,6 +232,25 @@ def test_soda_signs_pagination_cap_stops_instead_of_looping_forever(
     assert isinstance(result, set)
 
 
+@respx.mock
+def test_soda_signs_non_list_response_treated_as_no_more_data(
+    monkeypatch,
+) -> None:
+    """WR-03 (38-REVIEW.md): a truthy non-list JSON body (e.g. an error dict on
+    an HTTP-200 soft error) must be treated as "no more data", not iterated
+    as if it were a list of records (which would AttributeError on
+    ``record.get(...)`` when iterating a dict's string keys).
+    """
+    monkeypatch.delenv("NYC_OPEN_DATA_APP_TOKEN", raising=False)
+    respx.get(SODA_PARKING_SIGNS_URL).mock(
+        return_value=httpx.Response(200, json={"error": "soft failure"})
+    )
+
+    # Must not raise AttributeError/TypeError -- fail-soft contract (T-38-01-02).
+    result = _sync_fetch_asp_signs(headers={})
+    assert result == set()
+
+
 @respx.mock
 def test_soda_failure_is_fail_soft(tmp_path: Path, monkeypatch) -> None:
     """SODA HTTP 500 must NOT raise — has_asp lookup is empty but the build completes."""

From 9daf1e4d069f1bd6f3ab0a5a800e85464b123ff5 Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 17:52:40 -0400
Subject: [PATCH 7/9] fix(38): WR-05 attempt copy-based recovery before
 discarding last index backup

_sync_cleanup_stale's crash-recovery path treats _bak as the LAST viable
copy of the index when index_dir is absent. If os.replace(bak, index_dir)
raised OSError (e.g. EXDEV when _bak and the parent dir unexpectedly end
up on different filesystems, or a transient EBUSY), the handler
unconditionally rmtree'd _bak -- permanently discarding a state that was
often still recoverable via a plain copy. Adds a shutil.copytree fallback
before giving up; _bak is wiped either way once the fallback is resolved
(redundant on copy success, unusable on copy failure). Splits the old
combined regression test into a copy-succeeds case (index recovered) and
a both-fallbacks-fail case (bak wiped, index absent, matching the
previous behavior only as a true last resort).
---
 custom_components/asp_parking/index_io.py | 24 +++++++++-
 tests/test_index_io.py                    | 58 +++++++++++++++++++----
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/custom_components/asp_parking/index_io.py b/custom_components/asp_parking/index_io.py
index 034f908..b4d78ee 100644
--- a/custom_components/asp_parking/index_io.py
+++ b/custom_components/asp_parking/index_io.py
@@ -183,13 +183,33 @@ def _sync_cleanup_stale(index_dir: Path) -> None:
             except OSError as exc:
                 logger.error(
                     "cleanup_stale: could not restore backup index from %s to %s (%s) — "
-                    "destroying backup; the index will need to be rebuilt",
+                    "attempting copy-based fallback recovery",
                     bak,
                     index_dir,
                     exc,
                     exc_info=True,
                 )
-                shutil.rmtree(bak, ignore_errors=True)
+                # WR-05 (38-REVIEW.md): os.replace can fail for reasons that
+                # don't affect a plain copy (e.g. EXDEV when _bak and the
+                # parent dir unexpectedly end up on different filesystems,
+                # or a transient EBUSY). Before permanently discarding the
+                # LAST viable copy of the index, try shutil.copytree as a
+                # fallback — it trades a recoverable state (a valid index,
+                # copied instead of renamed) for an avoidable one (forcing
+                # a full rebuild). _bak is wiped either way once we're done
+                # with it: on copy success it is now redundant; on copy
+                # failure it is unusable and there is nothing left to keep.
+                try:
+                    shutil.copytree(bak, index_dir)
+                except OSError as copy_exc:
+                    logger.error(
+                        "cleanup_stale: copytree fallback also failed (%s) — "
+                        "destroying backup; the index will need to be rebuilt",
+                        copy_exc,
+                        exc_info=True,
+                    )
+                finally:
+                    shutil.rmtree(bak, ignore_errors=True)
 
     try:
         download_zip.unlink(missing_ok=True)
diff --git a/tests/test_index_io.py b/tests/test_index_io.py
index 23efd27..dc8844c 100644
--- a/tests/test_index_io.py
+++ b/tests/test_index_io.py
@@ -583,12 +583,19 @@ def test_cleanup_stale_restores_empty_bak_when_index_absent(tmp_path: Path) -> N
     assert not bak.exists(), "_bak must be gone after restore"
 
 
-def test_cleanup_stale_wipes_bak_when_os_replace_raises(tmp_path: Path) -> None:
-    """When os.replace raises OSError during _bak restore, _bak is wiped.
-
-    Edge-case 5: _sync_cleanup_stale catches the OSError and falls back to
-    shutil.rmtree(_bak), so _bak disappears and index_dir remains absent.
-    The function must not re-raise.
+def test_cleanup_stale_copytree_fallback_recovers_index_when_os_replace_raises(
+    tmp_path: Path,
+) -> None:
+    """WR-05 (38-REVIEW.md): when os.replace raises OSError during _bak
+    restore, _sync_cleanup_stale falls back to shutil.copytree instead of
+    immediately discarding the LAST viable copy of the index. If the copy
+    succeeds, index_dir ends up populated and _bak is wiped (now redundant).
+
+    Supersedes the old "always wipe _bak on os.replace failure" contract —
+    the review flagged that behavior as trading a recoverable state (rebuild
+    forced) for an avoidable one, since a same-filesystem `rename` failure
+    (e.g. transient EBUSY) does not necessarily mean a `copytree` would also
+    fail.
     """
     from unittest.mock import patch
 
@@ -603,10 +610,45 @@ def test_cleanup_stale_wipes_bak_when_os_replace_raises(tmp_path: Path) -> None:
         "custom_components.asp_parking.index_io.os.replace",
         side_effect=OSError("EBUSY"),
     ):
-        # Must not raise.
+        # Must not raise. Only os.replace is patched to fail — the
+        # shutil.copytree fallback runs for real against tmp_path.
         _sync_cleanup_stale(index_dir)
 
-    assert not bak.exists(), "_bak must be wiped after os.replace failure"
+    assert index_dir.exists(), (
+        "index_dir must be recovered via the copytree fallback"
+    )
+    assert (index_dir / "segments.idx").read_text() == "data"
+    assert not bak.exists(), "_bak must be wiped once the copy fallback succeeds"
+
+
+def test_cleanup_stale_wipes_bak_when_both_replace_and_copytree_raise(
+    tmp_path: Path,
+) -> None:
+    """WR-05 (38-REVIEW.md): when BOTH os.replace and the copytree fallback
+    raise OSError, _bak is wiped as a last resort (unusable either way) and
+    index_dir remains absent. The function must not re-raise.
+    """
+    from unittest.mock import patch
+
+    index_dir = tmp_path / "index"
+    bak = tmp_path / "index_bak"
+    bak.mkdir()
+    (bak / "segments.idx").write_text("data")
+
+    assert not index_dir.exists()
+
+    with patch(
+        "custom_components.asp_parking.index_io.os.replace",
+        side_effect=OSError("EBUSY"),
+    ):
+        with patch(
+            "custom_components.asp_parking.index_io.shutil.copytree",
+            side_effect=OSError("disk full"),
+        ):
+            # Must not raise.
+            _sync_cleanup_stale(index_dir)
+
+    assert not bak.exists(), "_bak must be wiped after both fallbacks fail"
     assert not index_dir.exists(), "index_dir must remain absent"
 
 

From 0a2e55795afa2290f2474ef37a17cf5a52a98414 Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 22:08:58 -0400
Subject: [PATCH 8/9] test(phase-38): add Nyquist validation tests for
 from-source index build invariants

Retroactive Nyquist audit found the existing tests only checked file
existence, not the invariants the threat model actually cares about:
R-tree population (guards rtree bug #159's silent-empty-index failure
mode), graph.json.zst round-tripping through StreetGraph, and manifest.json
never picking up a heavy GIS dependency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 tests/test_index_io_build_from_source.py | 93 ++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/tests/test_index_io_build_from_source.py b/tests/test_index_io_build_from_source.py
index c831acf..50018b0 100644
--- a/tests/test_index_io_build_from_source.py
+++ b/tests/test_index_io_build_from_source.py
@@ -24,6 +24,7 @@
 import httpx
 import pytest
 import respx
+from rtree import index as rtree_index
 
 # Imports targeted at the future symbol Plan 38-01 Task 2 must implement.
 # Collection MUST fail with ImportError until Task 2 lands the function.
@@ -304,3 +305,95 @@ def test_trafdir_nv_excluded(tmp_path: Path, monkeypatch) -> None:
 
     segments = json.loads((tmp_path / "idx_tmp" / "segments.json").read_text())
     assert "10006" not in segments, "TRAFDIR=NV segment must be filtered out"
+
+
+@respx.mock
+def test_rtree_non_empty(tmp_path: Path, monkeypatch) -> None:
+    """Nyquist gap 38-01-03: segments.idx/.dat must form a POPULATED R-tree.
+
+    Guards against rtree bug #159 -- using the generator-constructor form
+    (instead of an insert loop inside try/finally) silently writes an empty
+    index while the .idx/.dat files still exist on disk, so a mere
+    ``.exists()`` check on the files is not sufficient coverage.
+    """
+    monkeypatch.delenv("NYC_OPEN_DATA_APP_TOKEN", raising=False)
+    _route_cscl_two_pages(_load_cscl_fixture())
+    _route_soda_ok()
+
+    index_dir = tmp_path / "idx"
+    _sync_build_from_source(index_dir)
+
+    tmp = tmp_path / "idx_tmp"
+    assert (tmp / "segments.idx").exists()
+    assert (tmp / "segments.dat").exists()
+
+    idx = rtree_index.Index(str(tmp / "segments"))
+    try:
+        assert idx.count(idx.bounds) > 0, (
+            "R-tree at "
+            f"{tmp / 'segments'} is empty despite .idx/.dat files existing "
+            "on disk (rtree bug #159 -- generator constructor instead of "
+            "insert loop)"
+        )
+        hits = list(idx.intersection(idx.bounds))
+        assert len(hits) > 0
+    finally:
+        idx.close()
+
+
+@respx.mock
+def test_graph_json_zst_round_trip(tmp_path: Path, monkeypatch) -> None:
+    """Nyquist gap 38-01-04: graph.json.zst must be parseable by StreetGraph.load
+    and contain at least one segment/adjacency entry -- not merely exist on disk.
+    """
+    from custom_components.asp_parking.gps2asp.signs.graph import StreetGraph
+
+    monkeypatch.delenv("NYC_OPEN_DATA_APP_TOKEN", raising=False)
+    _route_cscl_two_pages(_load_cscl_fixture())
+    _route_soda_ok()
+
+    index_dir = tmp_path / "idx"
+    _sync_build_from_source(index_dir)
+
+    tmp = tmp_path / "idx_tmp"
+    assert (tmp / "graph.json.zst").exists()
+
+    graph = StreetGraph.load(tmp)
+    assert graph is not None, (
+        "StreetGraph.load returned None for a freshly-built graph.json.zst "
+        "-- either the file is corrupt/unreadable or empty"
+    )
+    assert len(graph.segment_streets) > 0, "graph has zero segment_streets entries"
+    assert len(graph.adjacency) > 0, "graph has zero adjacency entries"
+
+
+def test_manifest_no_heavy_gis_dependency() -> None:
+    """Nyquist gap 38-01-06: manifest.json requirements must never gain a heavy
+    GIS/GDAL dependency (geopandas, GDAL, fiona, pyogrio).
+
+    Evergreen regression test -- not a git-diff/byte-identical check, which
+    has no meaning outside the PR that originally introduced this plan.
+    Per 38-01-SUMMARY.md: "geopandas pulls GDAL (~500MB) into the HA Python
+    environment, violating the manifest.json 'no new external deps'
+    constraint."
+    """
+    manifest_path = (
+        Path(__file__).parent.parent
+        / "custom_components"
+        / "asp_parking"
+        / "manifest.json"
+    )
+    manifest = json.loads(manifest_path.read_text())
+    requirements = manifest.get("requirements", [])
+    assert isinstance(requirements, list) and requirements, (
+        "manifest.json requirements array is missing or empty"
+    )
+
+    banned_substrings = ("geopandas", "gdal", "fiona", "pyogrio")
+    for req in requirements:
+        req_lower = str(req).lower()
+        for banned in banned_substrings:
+            assert banned not in req_lower, (
+                f"manifest.json requirements contains a banned heavy GIS "
+                f"dependency: {req!r} (matched {banned!r})"
+            )

From 8030eba7fb73444db2b6bec8d01e5ca17a23326e Mon Sep 17 00:00:00 2001
From: Pascal <pascalschaefer2011@gmail.com>
Date: Thu, 13 Aug 2026 22:56:34 -0400
Subject: [PATCH 9/9] style(38): apply ruff format to
 test_coordinator_path_selection.py, test_index_io.py

CI's ruff format check flagged these two pre-existing files as unformatted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 tests/test_coordinator_path_selection.py | 16 ++++++----------
 tests/test_index_io.py                   |  4 +---
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/tests/test_coordinator_path_selection.py b/tests/test_coordinator_path_selection.py
index f33f1db..710f756 100644
--- a/tests/test_coordinator_path_selection.py
+++ b/tests/test_coordinator_path_selection.py
@@ -588,9 +588,7 @@ async def _executor_dispatch(fn, *args, **kwargs):
     )
 
     call_order: list[str] = []
-    spies["atomic_swap"].side_effect = lambda *a, **k: call_order.append(
-        "atomic_swap"
-    )
+    spies["atomic_swap"].side_effect = lambda *a, **k: call_order.append("atomic_swap")
     spies["verify_index"].side_effect = lambda *a, **k: call_order.append(
         "verify_index"
     )
@@ -604,11 +602,10 @@ async def _executor_dispatch(fn, *args, **kwargs):
     assert call_order == ["atomic_swap", "verify_index"], (
         f"verify_index MUST run AFTER atomic_swap, got order {call_order!r}"
     )
-    assert spies["pn_create"].call_args_list[-1].kwargs.get(
-        "notification_id"
-    ) == "asp_parking_index_rebuild_success", (
-        "A valid index MUST still post the success notification"
-    )
+    assert (
+        spies["pn_create"].call_args_list[-1].kwargs.get("notification_id")
+        == "asp_parking_index_rebuild_success"
+    ), "A valid index MUST still post the success notification"
 
 
 async def test_do_rebuild_treats_integrity_failure_as_rebuild_failure(
@@ -635,8 +632,7 @@ async def _executor_dispatch(fn, *args, **kwargs):
     await do_rebuild(triggered_by="button")
 
     notification_ids = [
-        call.kwargs.get("notification_id")
-        for call in spies["pn_create"].call_args_list
+        call.kwargs.get("notification_id") for call in spies["pn_create"].call_args_list
     ]
     assert "asp_parking_index_rebuild_error" in notification_ids, (
         "A corrupt post-swap index MUST post the rebuild-error notification"
diff --git a/tests/test_index_io.py b/tests/test_index_io.py
index dc8844c..1a61f33 100644
--- a/tests/test_index_io.py
+++ b/tests/test_index_io.py
@@ -614,9 +614,7 @@ def test_cleanup_stale_copytree_fallback_recovers_index_when_os_replace_raises(
         # shutil.copytree fallback runs for real against tmp_path.
         _sync_cleanup_stale(index_dir)
 
-    assert index_dir.exists(), (
-        "index_dir must be recovered via the copytree fallback"
-    )
+    assert index_dir.exists(), "index_dir must be recovered via the copytree fallback"
     assert (index_dir / "segments.idx").read_text() == "data"
     assert not bak.exists(), "_bak must be wiped once the copy fallback succeeds"