Downstream preflight + invalidated_caches guards

.github/workflows/pr.yaml

@@ -87,12 +87,15 @@ jobs:
             uv pip install --system "tables>=3.11.0"
             uv pip install --system policyengine-us --no-deps
             # Install remaining dependencies manually (skip pytest-dependency which has build issues)
-            uv pip install --system click pathlib synthimpute tabulate
+            uv pip install --system click pathlib synthimpute tabulate spm-calculator
             uv pip install --system policyengine-us-data --no-deps
           else
             uv pip install --system policyengine-us
           fi
         shell: bash
+      - name: Install -uk package from PyPI
+        run: uv pip install --system policyengine-uk
+        shell: bash
       - name: Run smoke tests only
         run: python -m pytest -m smoke --reruns 2 --reruns-delay 5 -v -s
         env:

changelog.d/add-downstream-preflight.added.md

@@ -0,0 +1 @@
+Add a downstream preflight smoke test (`tests/smoke/test_country_init.py`) that instantiates `CountryTaxBenefitSystem()` from the currently-published `policyengine-us` and `policyengine-uk` against the core under test. Catches the class of cross-repo regressions that the existing `RUN_SMOKE_TESTS`-gated microsimulation smoke test misses — breakdown-validator errors at parameter load, `_fast_cache`-init assumptions, and any other init-time changes that happen to break downstream country models. Also extends the smoke job in `.github/workflows/pr.yaml` to install `policyengine-uk` alongside `policyengine-us`.

changelog.d/guard-invalidated-caches.fixed.md

@@ -0,0 +1 @@
+Guard `invalidated_caches` mutation sites in `Simulation` against the attribute being missing, matching the pattern used for `_fast_cache` in #474. Country-package subclasses that override `__init__` without calling `super().__init__` can skip initialising this private attribute, so `purge_cache_of_invalid_values` and `invalidate_cache_entry` now fall back gracefully instead of raising `AttributeError`.

policyengine_core/simulations/simulation.py

@@ -853,7 +853,10 @@ def purge_cache_of_invalid_values(self) -> None:
         if self.tracer.stack:
             return
         _fast_cache = getattr(self, "_fast_cache", None)
-        for _name, _period in self.invalidated_caches:
+        invalidated_caches = getattr(self, "invalidated_caches", None)
+        if invalidated_caches is None:
+            return
+        for _name, _period in invalidated_caches:
             holder = self.get_holder(_name)
             holder.delete_arrays(_period)
             if _fast_cache is not None:
@@ -1144,7 +1147,11 @@ def _check_for_cycle(self, variable: str, period: Period) -> None:
             raise SpiralError(message, variable)
 
     def invalidate_cache_entry(self, variable: str, period: Period) -> None:
-        self.invalidated_caches.add((variable, period))
+        invalidated_caches = getattr(self, "invalidated_caches", None)
+        if invalidated_caches is None:
+            self.invalidated_caches = {(variable, period)}
+            return
+        invalidated_caches.add((variable, period))
 
     def invalidate_spiral_variables(self, variable: str) -> None:
         # Visit the stack, from the bottom (most recent) up; we know that we'll find

tests/core/test_fast_cache_guards.py

@@ -1,19 +1,22 @@
 """Regression tests: cache-manipulation methods on ``Simulation`` must not
-crash when ``_fast_cache`` is missing.
-
-``Simulation.__init__`` sets ``self._fast_cache = {}`` as the first step of
-initialisation. Country-package subclasses (e.g. ``policyengine_uk.Simulation``)
-can legitimately override ``__init__`` without calling ``super().__init__``
-— instead they set the handful of attributes they need directly. In that
-case ``_fast_cache`` never gets initialised, so any cache-mutation path
-that assumes the attribute exists raised ``AttributeError`` during
-``build_from_single_year_dataset`` / ``set_input`` / ``delete_arrays``.
-
-The defensive fix is to guard the bare ``.pop`` / ``.items`` / re-assign
-sites the same way the read-side fast path in ``calculate()`` already does
-— ``getattr(self, "_fast_cache", None)`` and skip the cache write when
-it's ``None``. Core owns this protection so every downstream subclass
-doesn't have to mirror the attribute.
+crash when private cache attributes are missing.
+
+``Simulation.__init__`` sets ``self._fast_cache = {}`` and
+``self.invalidated_caches = set()`` as part of initialisation.
+Country-package subclasses (e.g. ``policyengine_uk.Simulation``) can
+legitimately override ``__init__`` without calling ``super().__init__``
+— instead they set the handful of attributes they need directly. In
+that case the skipped attribute never gets initialised, so any
+cache-mutation path that assumes the attribute exists raised
+``AttributeError`` during ``build_from_single_year_dataset`` /
+``set_input`` / ``delete_arrays`` / ``purge_cache_of_invalid_values`` /
+``invalidate_cache_entry``.
+
+The defensive fix is to guard the bare ``.pop`` / ``.items`` / ``.add``
+sites the same way the read-side fast path in ``calculate()`` already
+does — ``getattr(self, "<attr>", None)`` and skip or lazily initialise
+when it's missing. Core owns this protection so every downstream
+subclass doesn't have to mirror the attributes.
 """
 
 from __future__ import annotations
@@ -74,3 +77,27 @@ def test_purge_cache_of_invalid_values_without_fast_cache_attribute():
 
     sim.purge_cache_of_invalid_values()
     assert sim.invalidated_caches == set()
+
+
+def test_purge_cache_of_invalid_values_without_invalidated_caches_attribute():
+    sim = _bare_simulation()
+    sim.tracer = types.SimpleNamespace(stack=[])
+    # No invalidated_caches, no _fast_cache — must not crash
+    sim.get_holder = lambda name: types.SimpleNamespace(
+        delete_arrays=lambda period: None
+    )
+
+    sim.purge_cache_of_invalid_values()
+
+
+def test_invalidate_cache_entry_without_invalidated_caches_attribute():
+    sim = _bare_simulation()
+    # No invalidated_caches — method should lazily initialise it
+    sim.invalidate_cache_entry("variable_name", "2024")
+    assert sim.invalidated_caches == {("variable_name", "2024")}
+    # Subsequent invalidations should accumulate
+    sim.invalidate_cache_entry("other_variable", "2025")
+    assert sim.invalidated_caches == {
+        ("variable_name", "2024"),
+        ("other_variable", "2025"),
+    }

tests/smoke/test_country_init.py

@@ -0,0 +1,53 @@
+"""Preflight: instantiating the country tax-benefit systems must not
+raise under the current `policyengine-core` under test.
+
+This catches a class of cross-repo regressions that full microsim
+smoke tests don't: breakdown-validator errors at parameter load,
+``_fast_cache``-attribute assumptions, and any other
+system-initialisation changes core makes that happen to break
+downstream country models.
+
+Unlike ``tests/smoke/test_us.py`` (which is gated by
+``RUN_SMOKE_TESTS=1`` and needs real microdata credentials), these
+tests only exercise ``CountryTaxBenefitSystem()`` — they don't need
+a dataset, don't need HF tokens, and complete in seconds. They run
+on every PR against the policyengine-core master branch via
+``SmokeTestForMultipleVersions``.
+
+Each test is a soft-fail via ``skip`` if the corresponding country
+package isn't installed — the workflow installs them explicitly, so
+in CI this should always find the package. The skip just keeps
+local test runs frictionless if you happen not to have both
+policyengine-us and policyengine-uk on your machine.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+
+import pytest
+
+
+@pytest.mark.smoke
+def test_policyengine_us_tax_benefit_system_instantiates():
+    if importlib.util.find_spec("policyengine_us") is None:
+        pytest.skip("policyengine-us is not installed")
+    # Importing this triggers the country package to load all
+    # parameters and all Variables, which is where breakdown-validator
+    # errors / _fast_cache-init bugs / etc. surface.
+    from policyengine_us import CountryTaxBenefitSystem
+
+    system = CountryTaxBenefitSystem()
+    assert len(system.variables) > 0
+    assert len(list(system.parameters.get_descendants())) > 0
+
+
+@pytest.mark.smoke
+def test_policyengine_uk_tax_benefit_system_instantiates():
+    if importlib.util.find_spec("policyengine_uk") is None:
+        pytest.skip("policyengine-uk is not installed")
+    from policyengine_uk import CountryTaxBenefitSystem
+
+    system = CountryTaxBenefitSystem()
+    assert len(system.variables) > 0
+    assert len(list(system.parameters.get_descendants())) > 0