Fix reversed parameter value history intervals

changelog.d/499.fixed.md

@@ -0,0 +1,3 @@
+Fixed `Parameter.parameter_values` to return oldest-first, non-overlapping
+histories with inclusive end dates instead of reversed intervals derived from
+PolicyEngine Core's newest-first storage.

docs/reference/index.md

@@ -4,6 +4,14 @@ title: "Reference"
 
 Reference pages are generated from the installed country-model packages. Authored methodology pages explain why the model is structured the way it is; generated reference pages expose the exact release contents.
 
+## Parameter history contract
+
+`model.get_parameter(name).parameter_values` exposes effective values from
+oldest to newest. Each bounded value has an inclusive `end_date` equal to the
+day before the next value starts, while the newest value has `end_date=None`.
+If the underlying Core history contains duplicate effective starts, the public
+history contains the first value Core would select for that date.
+
 ## What generated reference should include
 
 The variable reference generator already reads the installed country model and can emit:

src/policyengine/core/parameter.py

@@ -1,3 +1,4 @@
+from datetime import datetime, timedelta
 from typing import TYPE_CHECKING, Any, Optional
 from uuid import uuid4
 
@@ -32,25 +33,39 @@ def __init__(self, _core_param: Any = None, **data):
 
     @property
     def parameter_values(self) -> list["ParameterValue"]:
-        """Lazily build parameter values on first access."""
+        """Lazily build the effective parameter history on first access.
+
+        Values are ordered from oldest to newest. ``end_date`` is inclusive,
+        so each bounded value ends one day before the next value starts and the
+        newest value remains open-ended. When Core exposes duplicate effective
+        starts, retain the first entry because Core's lookup uses the first
+        matching entry as the effective value.
+        """
         if self._parameter_values is None:
             self._parameter_values = []
             if self._core_param is not None:
                 from policyengine.utils import parse_safe_date
 
-                for i in range(len(self._core_param.values_list)):
-                    param_at_instant = self._core_param.values_list[i]
-                    if i + 1 < len(self._core_param.values_list):
-                        next_instant = self._core_param.values_list[i + 1]
-                    else:
-                        next_instant = None
+                effective_values: dict[datetime, Any] = {}
+                for value_at_instant in self._core_param.values_list:
+                    start_date = parse_safe_date(value_at_instant.instant_str)
+                    effective_values.setdefault(start_date, value_at_instant)
+
+                chronological_values = sorted(effective_values.items())
+                for index, (start_date, value_at_instant) in enumerate(
+                    chronological_values
+                ):
+                    next_index = index + 1
+                    end_date = (
+                        chronological_values[next_index][0] - timedelta(days=1)
+                        if next_index < len(chronological_values)
+                        else None
+                    )
                     pv = ParameterValue(
                         parameter=self,
-                        start_date=parse_safe_date(param_at_instant.instant_str),
-                        end_date=parse_safe_date(next_instant.instant_str)
-                        if next_instant
-                        else None,
-                        value=param_at_instant.value,
+                        start_date=start_date,
+                        end_date=end_date,
+                        value=value_at_instant.value,
                     )
                     self._parameter_values.append(pv)
         return self._parameter_values

tests/test_models.py

@@ -1,11 +1,25 @@
 """Tests for UK and US tax-benefit model versions and core models."""
 
 import re
+from datetime import timedelta
 
 from policyengine.tax_benefit_models.uk import uk_latest
 from policyengine.tax_benefit_models.us import us_latest
 
 
+def _assert_parameter_value_interval_contract(model, parameter_name):
+    values = model.get_parameter(parameter_name).parameter_values
+
+    assert len(values) > 1
+    assert [value.start_date for value in values] == sorted(
+        value.start_date for value in values
+    )
+    for current, following in zip(values, values[1:]):
+        assert current.end_date == following.start_date - timedelta(days=1)
+        assert current.start_date <= current.end_date
+    assert values[-1].end_date is None
+
+
 class TestUKModel:
     """Tests for PolicyEngine UK model."""
 
@@ -70,6 +84,13 @@ def test_model_version_parameter_values_aggregates_all(self):
         all_values = uk_latest.parameter_values
         assert len(all_values) >= 100
 
+    def test_parameter_value_intervals_are_chronological_and_inclusive(self):
+        """UK catalog histories should expose valid inclusive intervals."""
+        _assert_parameter_value_interval_contract(
+            uk_latest,
+            "gov.hmrc.income_tax.rates.uk[0].rate",
+        )
+
     def test__given_bracket_parameter__then_has_generated_label(self):
         """Bracket parameters should have auto-generated labels."""
         bracket_params_with_labels = [
@@ -155,6 +176,13 @@ def test_model_version_parameter_values_aggregates_all(self):
         all_values = us_latest.parameter_values
         assert len(all_values) >= 100
 
+    def test_parameter_value_intervals_are_chronological_and_inclusive(self):
+        """US catalog histories should expose valid inclusive intervals."""
+        _assert_parameter_value_interval_contract(
+            us_latest,
+            "gov.irs.credits.ctc.amount.base[0].amount",
+        )
+
     def test__given_breakdown_parameter__then_has_generated_label(self):
         """Breakdown parameters (e.g., filing status) should have auto-generated labels."""
         breakdown_params_with_labels = [

tests/test_parameter_values.py

@@ -0,0 +1,138 @@
+"""Tests for parameter history conversion from PolicyEngine Core."""
+
+from datetime import datetime
+from types import SimpleNamespace
+
+from policyengine.core.parameter import Parameter
+from policyengine.core.parameter_value import ParameterValue
+
+
+def _parameter_with_core_values(*values: tuple[str, object]) -> Parameter:
+    parameter = Parameter.model_construct(
+        name="gov.test.value",
+        tax_benefit_model_version=None,
+    )
+    parameter._core_param = SimpleNamespace(
+        values_list=[
+            SimpleNamespace(instant_str=start_date, value=value)
+            for start_date, value in values
+        ]
+    )
+    parameter._parameter_values = None
+    return parameter
+
+
+def test__newest_first_core_values__then_history_is_chronological_and_inclusive():
+    parameter = _parameter_with_core_values(
+        ("2024-07-15", 30),
+        ("2022-03-10", 20),
+        ("2020-01-01", 10),
+    )
+
+    values = parameter.parameter_values
+
+    assert [value.start_date for value in values] == [
+        datetime(2020, 1, 1),
+        datetime(2022, 3, 10),
+        datetime(2024, 7, 15),
+    ]
+    assert [value.end_date for value in values] == [
+        datetime(2022, 3, 9),
+        datetime(2024, 7, 14),
+        None,
+    ]
+    assert [value.value for value in values] == [10, 20, 30]
+    assert all(value.parameter is parameter for value in values)
+
+
+def test__single_core_value__then_history_is_open_ended():
+    parameter = _parameter_with_core_values(("2024-01-01", 10))
+
+    (value,) = parameter.parameter_values
+
+    assert value.start_date == datetime(2024, 1, 1)
+    assert value.end_date is None
+
+
+def test__consecutive_day_updates__then_earlier_value_covers_one_day():
+    parameter = _parameter_with_core_values(
+        ("2024-01-02", 20),
+        ("2024-01-01", 10),
+    )
+
+    values = parameter.parameter_values
+
+    assert values[0].start_date == datetime(2024, 1, 1)
+    assert values[0].end_date == datetime(2024, 1, 1)
+    assert values[0].value == 10
+    assert values[1].start_date == datetime(2024, 1, 2)
+    assert values[1].end_date is None
+    assert values[1].value == 20
+
+
+def test__duplicate_effective_starts__then_first_core_value_wins():
+    parameter = _parameter_with_core_values(
+        ("2024-01-01", "effective"),
+        ("2024-01-01", "shadowed"),
+        ("2020-01-01", "older"),
+    )
+
+    values = parameter.parameter_values
+
+    assert len(values) == 2
+    assert values[0].value == "older"
+    assert values[0].end_date == datetime(2023, 12, 31)
+    assert values[1].value == "effective"
+    assert values[1].end_date is None
+
+
+def test__year_zero_core_value__then_existing_safe_date_normalization_is_preserved():
+    parameter = _parameter_with_core_values(
+        ("2020-01-01", 20),
+        ("0000-01-01", 10),
+    )
+
+    values = parameter.parameter_values
+
+    assert values[0].start_date == datetime(1, 1, 1)
+    assert values[0].end_date == datetime(2019, 12, 31)
+    assert values[1].start_date == datetime(2020, 1, 1)
+    assert values[1].end_date is None
+
+
+def test__repeated_access__then_parameter_history_is_cached():
+    parameter = _parameter_with_core_values(("2024-01-01", 10))
+
+    first = parameter.parameter_values
+
+    assert parameter.parameter_values is first
+
+
+def test__explicit_parameter_values__then_setter_override_is_preserved():
+    parameter = _parameter_with_core_values(("2024-01-01", 10))
+    override = [
+        ParameterValue(
+            parameter=parameter,
+            value=99,
+            start_date=datetime(2030, 1, 1),
+            end_date=None,
+        )
+    ]
+
+    parameter.parameter_values = override
+
+    assert parameter.parameter_values is override
+
+
+def test__parameter_without_core_reference__then_history_is_empty():
+    parameter = Parameter.model_construct(
+        name="gov.test.value",
+        tax_benefit_model_version=None,
+    )
+    parameter._core_param = None
+    parameter._parameter_values = None
+
+    values = parameter.parameter_values
+
+    assert values == []
+    assert parameter.parameter_values is values

tests/test_parametric_reforms.py

@@ -1,7 +1,9 @@
 """Tests for parametric reforms utility functions."""
 
 from datetime import date
+from types import SimpleNamespace
 
+from policyengine.core.parameter import Parameter
 from policyengine.utils.parametric_reforms import (
     reform_dict_from_parameter_values,
     simulation_modifier_from_parameter_values,
@@ -226,6 +228,29 @@ def test__given_open_ended_and_explicit_end_values__then_open_ended_clips_at_nex
             "2026-01-01.2026-12-31": 200,
         }
 
+    def test__given_catalog_history__then_emits_valid_inclusive_ranges(self):
+        """Catalog intervals should preserve inclusive reform boundaries."""
+        parameter = Parameter.model_construct(
+            name="gov.test.catalog_value",
+            tax_benefit_model_version=None,
+        )
+        parameter._core_param = SimpleNamespace(
+            values_list=[
+                SimpleNamespace(instant_str="2022-07-01", value=20),
+                SimpleNamespace(instant_str="2020-01-01", value=10),
+            ]
+        )
+        parameter._parameter_values = None
+
+        result = reform_dict_from_parameter_values(parameter.parameter_values)
+
+        assert result == {
+            "gov.test.catalog_value": {
+                "2020-01-01.2022-06-30": 10,
+                "2022-07-01.2100-12-31": 20,
+            }
+        }
+
 
 class TestSimulationModifierFromParameterValues:
     """Tests for the simulation_modifier_from_parameter_values function."""