From 8713319f450b7624af5dda99cc76a0bae48628f6 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 7 Oct 2026 11:41:19 -0400 Subject: [PATCH 1/4] Refuse products that are not a usual rate times weeks; tie annual flows to year A product binding now reads exactly two concepts: one usual rate held as a float and one annual flow counted in weeks, in either order. Before, the only check was the count of two, so these validated: - Product(liquid_financial_assets, weeks_worked), although the first is a stock; - usual weekly hours times a year's income. policyengine-uk's hours_worked, usual weekly hours times weeks worked, is the one committed product, and it still validates. A concept declared an annual flow must also have period year. All 14 committed annual flows already do, so concept_schema_sha256 does not move. The tie runs one way: usual weekly hours, a usual rate, keeps period year. Tests: - Hypothesis: either operand of the hours product is swapped for any concept and its basis redeclared; for a numeric quantity that is neither an amount nor a pointer, its dtype and unit are redeclared too. In either order, the binding validates exactly when the rule's text says it should, and a refusal names only operands at fault. - Exhaustive: of all ordered pairs of committed concepts, only usual weekly hours and weeks worked multiply. - Each of 13 ways to break the rule is refused with its whole message, and validates once the field at fault is redeclared. A product of one concept or of three is refused by name. - Exhaustive: redeclaring either operand of the committed product under any other basis refuses it. - The complement test now excludes products and redeclares the reads of every other committed binding under all four bases. - Exhaustive over policyengine-uk's JSON form: any other person concept in either operand's place is refused. - Exhaustive over every committed concept, basis and period: exactly an annual flow outside period year is refused. The _redeclared test helper now sets period year when it redeclares a concept as an annual flow. ADR invariant 11 states both rules. No pinned hash moves. Co-Authored-By: Claude Opus 5.5 --- ...ct-reads-usual-rate-times-weeks.changed.md | 1 + docs/concept-schema-transport-adr.md | 53 ++- .../src/microcosm/frame/concept_mapping.py | 52 ++- .../src/microcosm/frame/concepts.py | 11 +- .../shared/test_concept_mapping.py | 396 +++++++++++++++++- .../tests/engine_free/shared/test_concepts.py | 35 ++ 6 files changed, 520 insertions(+), 28 deletions(-) create mode 100644 changelog.d/product-reads-usual-rate-times-weeks.changed.md diff --git a/changelog.d/product-reads-usual-rate-times-weeks.changed.md b/changelog.d/product-reads-usual-rate-times-weeks.changed.md new file mode 100644 index 0000000000..b55272aa8d --- /dev/null +++ b/changelog.d/product-reads-usual-rate-times-weeks.changed.md @@ -0,0 +1 @@ +Concept mappings now refuse a `product` binding unless it reads exactly one usual rate, held as a float, and one annual flow counted in weeks, in either order. Before, `Product(liquid_financial_assets, weeks_worked)` validated, although the first is a stock; so did a product of usual weekly hours and a year's income. Concepts now also refuse an `annual_flow` temporal basis with any period but `year`. Every committed binding and concept already meets both rules, so no binding, coverage report, golden or pinned hash changes; policyengine-uk's `hours_worked`, usual weekly hours times weeks worked, still validates. diff --git a/docs/concept-schema-transport-adr.md b/docs/concept-schema-transport-adr.md index 11bea0041e..36f60457d6 100644 --- a/docs/concept-schema-transport-adr.md +++ b/docs/concept-schema-transport-adr.md @@ -10,7 +10,9 @@ New Zealand's Accommodation Supplement cash-asset test. Only the Axiom New Zealand mapping binds it, as a deferred `Family` binding; the other three mappings list it as unmapped with a reason. A scale, sum, share or fraction now computes from annual flows only (invariant 11), so none of them can rescale the -stock, add it to a year's income, or split it. +stock, add it to a year's income, or split it. A product now multiplies one +usual rate by one annual flow counted in weeks, and every annual flow has period +`year` (both also invariant 11). ## Context @@ -56,7 +58,8 @@ Each concept declares: person id or unit interval. - **Period semantics.** The kernel's `year`, `month` or `point`, plus what the value measures over it: an annual flow, a state at the reference date, a usual - rate over the weeks worked, or persistent state. + rate over the weeks worked, or persistent state. An annual flow's period is + `year`. - **Currency and price level.** Every amount is nominal, in whole units of the frame's single declared currency (`ConceptFrameDeclaration.currency`, ISO 4217), at the prices of its own period. Each amount names the index family @@ -132,7 +135,8 @@ binds engine inputs to concepts. Each binding states: another input holds whole, a fixed scale (annual to weekly or monthly), a positivity test, sum, product, allocation to the reference person, a predicate, a relationship role, a co-resident child count, or a take-up - threshold. A scale, sum, share or fraction computes from annual flows only; + threshold. A scale, sum, share or fraction computes from annual flows only, + and a product multiplies one usual rate by one annual flow counted in weeks; - the relation, in the same vocabulary; - the evidence, quoting the engine's own definition or how builds populate the input. @@ -278,7 +282,8 @@ These hold for every input and are tested (Hypothesis properties unless noted): round-trips through JSON. Malformed input raises `ValueError` and never another exception, whatever is corrupted; a missing, unexpected or wrongly typed field is named. -11. **Arithmetic reads annual flows.** A scale, sum, share or fraction binding +11. **Arithmetic reads annual flows, and a product multiplies a usual rate by + weeks.** A scale, sum, share or fraction binding computes only from concepts whose declared temporal basis is `annual_flow`: every concept in its `concepts`. A stock such as `liquid_financial_assets` has no weekly value and cannot be added to a @@ -293,12 +298,40 @@ These hold for every input and are tested (Hypothesis properties unless noted): were an annual flow. Two checks are exhaustive rather than sampled. Redeclaring any operand of a committed arithmetic binding under another basis refuses that binding, while redeclaring its placement pointer - leaves it valid. Redeclaring anything any other committed binding reads - leaves that binding valid. Every committed mapping validates. Other - transforms are outside the rule. The New Zealand binding reads the stock - through an identity whose group rule sums it over a family's members, and - policyengine-uk's hours input is a product of usual weekly hours and - weeks worked. + leaves it valid. Redeclaring anything any other committed binding (other + than a product) reads, under any basis, leaves that binding valid. Every + committed mapping validates. + + A product has a rule of its own. It reads exactly two concepts, in either + order: one usual rate held as a float, and one annual flow counted in weeks + (unit `weeks`, an int or a float). A usual rate holds over the reference + year's weeks of activity, so only a number of weeks multiplies it to a + year's total. The one committed product is policyengine-uk's hours input, + usual weekly hours times weeks worked. The rule refuses a product of any + number of concepts but two, and one that reads a stock or a persistent draw, + two usual rates, two annual flows, a usual rate that is not a float, or an + annual flow that is not a number of weeks, such as a year's income. A + refusal of two concepts names only operands at fault. A Hypothesis property + swaps either operand of the committed product for any concept and redeclares + its basis, and, for a numeric quantity that is neither an amount nor a + pointer, its dtype and unit. In either order, the binding validates exactly + when this rule says it should. Each way to break the rule is also refused by + name. Three checks are exhaustive: of all ordered pairs of committed + concepts, only usual weekly hours and weeks worked multiply; redeclaring + either operand of the committed product under any other basis refuses it; + and in policyengine-uk's JSON form, putting any other person concept in + either operand's place is refused. + + Every annual flow has period `year`. A concept declared an annual flow + with another period is refused at construction. The tie runs one way: + other bases take any period, and usual weekly hours, a usual rate, has + period `year`. Every committed annual flow already had period `year`, so + the schema digest does not move. The test is exhaustive over every + committed concept, basis and period. + + Other transforms are outside both rules. The New Zealand binding reads + the stock through an identity whose group rule sums it over a family's + members. ## Consequences diff --git a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py index fd5f5f8a63..2e4a392466 100644 --- a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py +++ b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py @@ -207,7 +207,13 @@ class Sum: @dataclass(frozen=True) class Product: - """The engine input is the product of two concepts (hours x weeks).""" + """A usual rate times the weeks it held over, in either order. + + Reads exactly two concepts: one usual rate, a float, and one annual flow + counted in weeks. Usual weekly hours times weeks worked is the year's + hours. A usual rate holds over the reference year's weeks of activity, so + nothing else multiplies it to a year's total. + """ @dataclass(frozen=True) @@ -749,10 +755,13 @@ def bind( #: that needs one needs a new transform. The rule checks the concepts a #: binding computes from (``concepts``), not the household reference person a #: binding allocated to the reference unit also reads to place its value. A -#: product is outside the rule: it multiplies a usual rate (weekly hours) by -#: weeks. +#: product has its own rule (:data:`_PRODUCT_BASES`). _FLOW_TRANSFORMS = (Scale, Sum, Share, Fraction) +#: A product multiplies one usual rate by one annual flow (the weeks the rate +#: held over), so it reads exactly these two bases, once each. +_PRODUCT_BASES = (TemporalBasis.USUAL_RATE, TemporalBasis.ANNUAL_FLOW) + def _check_transform_arity(binding: InputBinding) -> None: transform = binding.transform @@ -809,8 +818,8 @@ def _check_transform_arity(binding: InputBinding) -> None: f"reads annual flows only, and {item.id} has temporal " f"basis {item.temporal_basis.value}." ) - if isinstance(transform, Product) and len(items) != 2: - raise ValueError(f"Binding for {name!r}: a product reads two concepts.") + if isinstance(transform, Product): + _check_product(name, items) if isinstance(transform, AllocateToReferencePerson) and items[0].entity != ( "household" ): @@ -853,6 +862,39 @@ def _check_transform_arity(binding: InputBinding) -> None: raise ValueError(f"Binding for {name!r}: take-up names its program.") +def _check_product(name: str, items: list[Concept]) -> None: + if len(items) != 2: + raise ValueError(f"Binding for {name!r}: a product reads two concepts.") + for item in items: + if item.temporal_basis not in _PRODUCT_BASES: + raise ValueError( + f"Binding for {name!r}: a product reads a usual rate and an " + f"annual flow, and {item.id} has temporal basis " + f"{item.temporal_basis.value}." + ) + first, second = items + if first.temporal_basis is second.temporal_basis: + raise ValueError( + f"Binding for {name!r}: a product reads one usual rate and one " + f"annual flow, and {first.id} and {second.id} both have temporal " + f"basis {first.temporal_basis.value}." + ) + rate, weeks = sorted( + items, key=lambda item: _PRODUCT_BASES.index(item.temporal_basis) + ) + if rate.dtype != "float": + raise ValueError( + f"Binding for {name!r}: a product's usual rate is a float, and " + f"{rate.id} has dtype {rate.dtype}." + ) + if weeks.unit is not Unit.WEEKS or weeks.dtype not in ("int", "float"): + raise ValueError( + f"Binding for {name!r}: a product multiplies a usual rate by a " + f"number of weeks, and {weeks.id} has unit {weeks.unit.value} and " + f"dtype {weeks.dtype}." + ) + + def _check_group_rule(engine: str, binding: InputBinding) -> None: rule = binding.group_rule items = [concept(concept_id) for concept_id in binding.concepts] diff --git a/packages/microcosm-frame/src/microcosm/frame/concepts.py b/packages/microcosm-frame/src/microcosm/frame/concepts.py index 943f3e57e9..8a1b81ee7f 100644 --- a/packages/microcosm-frame/src/microcosm/frame/concepts.py +++ b/packages/microcosm-frame/src/microcosm/frame/concepts.py @@ -172,7 +172,8 @@ class TemporalBasis(StrEnum): ``annual_flow`` matches ``microcosm.build.monetary_targets``. """ - #: An amount or count accumulated over the reference year. + #: An amount or count accumulated over the reference year. Its period is + #: ``year``. ANNUAL_FLOW = "annual_flow" #: The record's state at the survey reference date. REFERENCE_STATE = "reference_state" @@ -519,7 +520,8 @@ class Concept: dtype: Kernel dtype kind: ``float``, ``int``, ``bool`` or ``str``. unit: The unit values are expressed in. period: Kernel period semantics: ``year``, ``month`` or ``point``. - temporal_basis: What the value measures over its period. + temporal_basis: What the value measures over its period. An annual + flow's period is ``year``. provenance: How values come to exist. transport: What transport does with the column. monetary: Currency and price-level handling; set exactly when the @@ -577,6 +579,11 @@ def __post_init__(self) -> None: raise ValueError( f"Concept {self.id!r} period must be one of {_PERIOD_SEMANTICS}." ) + if self.temporal_basis is TemporalBasis.ANNUAL_FLOW and self.period != "year": + raise ValueError( + f"Concept {self.id!r}: an annual flow accumulates over the year, " + f"so its period is year, not {self.period}." + ) self._validate_unit_contract() self._validate_bounds() self._validate_transport() diff --git a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py index 13ea6b0335..71e059e0b6 100644 --- a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py +++ b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py @@ -12,7 +12,7 @@ import copy import json import re -from contextlib import contextmanager +from contextlib import ExitStack, contextmanager from dataclasses import replace from types import MappingProxyType @@ -55,6 +55,8 @@ CanonicalConceptKind, ConceptAlignment, TemporalBasis, + TransportRule, + Unit, concept, derive_take_up_draws, ) @@ -680,6 +682,24 @@ def test_a_minimal_mapping_constructs(self) -> None: ), ({"transform": Share(parameter="p")}, "not an amount"), ({"transform": Scale(factor=0.5)}, "only float concepts scale"), + ( + { + "transform": Product(), + "concepts": ("fact:person.usual_weekly_hours",), + }, + r"^Binding for 'x': a product reads two concepts\.$", + ), + ( + { + "transform": Product(), + "concepts": ( + "fact:person.usual_weekly_hours", + "fact:person.weeks_worked", + "fact:person.age", + ), + }, + r"^Binding for 'x': a product reads two concepts\.$", + ), ({"transform": RelationshipRole(role=Role.REFERENCE_PERSON)}, "reads"), ({"transform": CoresidentChildCount()}, "both parents"), ({"transform": TakeUpThreshold(program="x")}, "seed"), @@ -1003,10 +1023,19 @@ def _refusal(kind: type, concept_id: str, engine_input: str = "x") -> str: @contextmanager -def _redeclared(concept_id: str, basis: TemporalBasis): - """Run with ``concept_id`` declared under ``basis``, and nothing else.""" +def _redeclared(concept_id: str, basis: TemporalBasis | None = None, **fields): + """Run with ``concept_id`` redeclared, and nothing else. + + ``basis`` sets its temporal basis. An annual flow also takes period + ``year``, which every annual flow must have; no other basis changes the + period. ``fields`` change other declared fields. + """ + if basis is not None: + fields["temporal_basis"] = basis + if basis is TemporalBasis.ANNUAL_FLOW: + fields.setdefault("period", "year") registry = dict(concepts_module.CONCEPT_BY_ID) - registry[concept_id] = replace(registry[concept_id], temporal_basis=basis) + registry[concept_id] = replace(registry[concept_id], **fields) patched = MappingProxyType(registry) with pytest.MonkeyPatch.context() as patch: # concept() reads the first; the mapping module imported the second. @@ -1037,6 +1066,71 @@ def _us_reading_the_stock(*bindings) -> dict: return edited +_HOURS = "fact:person.usual_weekly_hours" +_WEEKS = "fact:person.weeks_worked" +_CONCEPT_ID = re.compile(r"fact:[a-z]+\.[a-z0-9_]+") +#: Units of the numeric quantities that are neither amounts nor pointers, so +#: their dtype and unit can be redeclared freely. +_QUANTITIES = (Unit.YEARS, Unit.HOURS_PER_WEEK, Unit.WEEKS, Unit.UNIT_INTERVAL) + + +def _faults(first, second) -> set[str]: + """The ids that keep a product of ``first`` and ``second`` from validating. + + The rule as stated: a product reads one usual rate held as a float and one + annual flow counted in weeks (an int or a float), in either order. An + empty set means the product validates. + """ + items = (first, second) + faults = { + item.id + for item in items + if item.temporal_basis + not in (TemporalBasis.USUAL_RATE, TemporalBasis.ANNUAL_FLOW) + } + if first.temporal_basis is second.temporal_basis: + faults |= {first.id, second.id} + for item in items: + if item.temporal_basis is TemporalBasis.USUAL_RATE and item.dtype != "float": + faults.add(item.id) + if item.temporal_basis is TemporalBasis.ANNUAL_FLOW and ( + item.unit is not Unit.WEEKS or item.dtype not in ("int", "float") + ): + faults.add(item.id) + return faults + + +def _product(*concepts: str) -> InputBinding: + return _binding(concepts=concepts, transform=Product()) + + +def _uk_hours_reading(concepts: tuple[str, str]) -> dict: + """policyengine-uk's JSON form, with ``hours_worked`` reading ``concepts``. + + Whatever the product now reads leaves ``unmapped``; whatever it no longer + reads, and no other binding reads, joins it. + """ + mapping = MAPPINGS["policyengine-uk"] + edited = mapping.to_dict() + (index,) = ( + position + for position, binding in enumerate(mapping.bindings) + if binding.engine_input == "hours_worked" + ) + edited["bindings"][index]["concepts"] = list(concepts) + for concept_id in concepts: + edited["unmapped"].pop(concept_id, None) + others = { + concept_id + for position, binding in enumerate(mapping.bindings) + if position != index + for concept_id in binding.reads + } + for concept_id in set(mapping.bindings[index].concepts) - set(concepts) - others: + edited["unmapped"][concept_id] = "probe" + return edited + + class TestArithmeticReadsAnnualFlows: """A scale, sum, share or fraction reads annual flows only. @@ -1048,7 +1142,13 @@ class TestArithmeticReadsAnnualFlows: sum, share or fraction binding now checks the declared temporal basis of each concept it computes from. The household reference person that a binding allocated to the reference unit also reads, to place its value, - is not an operand. Other transforms are outside the rule. + is not an operand. + + A product has a rule of its own. Its one committed use, policyengine-uk's + ``hours_worked``, multiplies usual weekly hours by weeks worked, yet a + product of the stock and weeks worked validated too. A product now reads + exactly one usual rate, held as a float, and one annual flow counted in + weeks, in either order. Other transforms are outside both rules. """ @PROPERTY @@ -1153,19 +1253,21 @@ def test_redeclaring_any_read_of_any_other_committed_binding_keeps_it( self, ) -> None: # The complement, also exhaustive: no other committed binding depends - # on the basis of anything it reads (identities, roles, take-up, - # recodes, predicates, positivity tests, allocations and the hours - # product). + # on the basis of anything it reads (identities, roles, child counts, + # take-up, recodes, predicates, positivity tests and allocations), + # whichever basis it is redeclared under. The hours product used to be + # one of them; it now has a rule of its own, and redeclaring either of + # its operands refuses it (see the product tests below). others = [ (name, binding) for name, binding in _COMMITTED - if not isinstance(binding.transform, _ARITHMETIC) + if not isinstance(binding.transform, (*_ARITHMETIC, Product)) ] kinds = {type(binding.transform) for _, binding in others} - assert {Identity, Product, AllocateToReferencePerson, Positive} <= kinds + assert {Identity, AllocateToReferencePerson, Positive} <= kinds for name, binding in others: for concept_id in binding.reads: - for basis in _NOT_FLOWS: + for basis in TemporalBasis: with _redeclared(concept_id, basis): assert replace(binding) == binding, (name, binding.ref, basis) @@ -1302,6 +1404,278 @@ def test_every_committed_mapping_validates_and_its_arithmetic_reads_flows( concept(concept_id).temporal_basis is TemporalBasis.ANNUAL_FLOW ), (binding.ref, concept_id) + @PROPERTY + @given(data=st.data()) + def test_a_product_validates_exactly_when_it_reads_a_float_rate_and_weeks( + self, data + ) -> None: + # Start from the committed hours product. Each operand may be swapped + # for any concept, and its basis, dtype and unit redeclared. In either + # order, the binding validates exactly when the rule's text says it + # should, and a refusal names only operands that break the rule. + drawn = [] + for slot in (_HOURS, _WEEKS): + concept_id = data.draw( + st.one_of(st.just(slot), st.sampled_from([c.id for c in CONCEPTS])), + label="concept", + ) + basis = data.draw( + st.one_of(st.none(), st.sampled_from(TemporalBasis)), label="basis" + ) + fields = {} + if concept(concept_id).unit in _QUANTITIES: + dtype = data.draw( + st.one_of(st.none(), st.sampled_from(("float", "int", "str"))), + label="dtype", + ) + unit = data.draw( + st.one_of(st.none(), st.sampled_from(_QUANTITIES)), label="unit" + ) + if dtype is not None: + fields["dtype"] = dtype + if dtype == "str": + fields.update(lower=None, upper=None) + if unit is not None: + fields["unit"] = unit + drawn.append((concept_id, basis, fields)) + (first, *_), (second, *_) = drawn + assume(first != second) + with ExitStack() as stack: + for concept_id, basis, fields in drawn: + stack.enter_context(_redeclared(concept_id, basis, **fields)) + faults = _faults(concept(first), concept(second)) + for order in ((first, second), (second, first)): + if not faults: + assert _product(*order).concepts == order + continue + with pytest.raises( + ValueError, match=r"^Binding for 'x': a product" + ) as error: + _product(*order) + named = set(_CONCEPT_ID.findall(str(error.value))) + assert named and named <= faults, (order, faults, named) + + def test_of_all_committed_concepts_only_hours_and_weeks_multiply(self) -> None: + # Exhaustive: every ordered pair of distinct committed concepts. + valid = set() + for first in CONCEPTS: + for second in CONCEPTS: + if first.id == second.id: + continue + pair = (first.id, second.id) + try: + _product(*pair) + except ValueError as error: + named = set(_CONCEPT_ID.findall(str(error))) + assert str(error).startswith("Binding for 'x': a product"), pair + assert named and named <= _faults(first, second), (pair, named) + else: + assert not _faults(first, second), pair + valid.add(pair) + assert valid == {(_HOURS, _WEEKS), (_WEEKS, _HOURS)} + + @pytest.mark.parametrize( + ("concepts", "setup", "message", "fix"), + [ + pytest.param( + (_LIQUID_ASSETS, _WEEKS), + [], + "a product reads a usual rate and an annual flow, and " + "fact:person.liquid_financial_assets has temporal basis " + "reference_state.", + [(_LIQUID_ASSETS, TemporalBasis.USUAL_RATE, {})], + id="stock-times-weeks", + ), + pytest.param( + (_WEEKS, _LIQUID_ASSETS), + [], + "a product reads a usual rate and an annual flow, and " + "fact:person.liquid_financial_assets has temporal basis " + "reference_state.", + [(_LIQUID_ASSETS, TemporalBasis.USUAL_RATE, {})], + id="weeks-times-stock", + ), + pytest.param( + ("fact:person.take_up_seed", _WEEKS), + [], + "a product reads a usual rate and an annual flow, and " + "fact:person.take_up_seed has temporal basis persistent.", + [("fact:person.take_up_seed", TemporalBasis.USUAL_RATE, {})], + id="seed-times-weeks", + ), + pytest.param( + (_HOURS, "fact:person.age"), + [], + "a product reads a usual rate and an annual flow, and " + "fact:person.age has temporal basis reference_state.", + [("fact:person.age", TemporalBasis.ANNUAL_FLOW, {"unit": Unit.WEEKS})], + id="hours-times-age", + ), + pytest.param( + ("fact:person.employment_income", _WEEKS), + [], + "a product reads one usual rate and one annual flow, and " + "fact:person.employment_income and fact:person.weeks_worked both " + "have temporal basis annual_flow.", + [("fact:person.employment_income", TemporalBasis.USUAL_RATE, {})], + id="income-times-weeks", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_WEEKS, TemporalBasis.USUAL_RATE, {})], + "a product reads one usual rate and one annual flow, and " + "fact:person.usual_weekly_hours and fact:person.weeks_worked both " + "have temporal basis usual_rate.", + [(_WEEKS, TemporalBasis.ANNUAL_FLOW, {})], + id="two-rates", + ), + pytest.param( + (_HOURS, "fact:person.employment_income"), + [], + "a product multiplies a usual rate by a number of weeks, and " + "fact:person.employment_income has unit base_currency and dtype " + "float.", + [ + ( + "fact:person.employment_income", + None, + { + "unit": Unit.WEEKS, + "monetary": None, + "transport": TransportRule.CARRY, + }, + ) + ], + id="hours-times-income", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_HOURS, None, {"dtype": "int"})], + "a product's usual rate is a float, and " + "fact:person.usual_weekly_hours has dtype int.", + [(_HOURS, None, {"dtype": "float"})], + id="whole-number-rate", + ), + pytest.param( + (_WEEKS, _HOURS), + [(_WEEKS, None, {"unit": Unit.HOURS_PER_WEEK})], + "a product multiplies a usual rate by a number of weeks, and " + "fact:person.weeks_worked has unit hours_per_week and dtype int.", + [(_WEEKS, None, {"unit": Unit.WEEKS})], + id="weeks-in-hours", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_WEEKS, None, {"dtype": "str", "lower": None, "upper": None})], + "a product multiplies a usual rate by a number of weeks, and " + "fact:person.weeks_worked has unit weeks and dtype str.", + [(_WEEKS, None, {"dtype": "int"})], + id="weeks-as-text", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_HOURS, None, {"dtype": "str", "lower": None, "upper": None})], + "a product's usual rate is a float, and " + "fact:person.usual_weekly_hours has dtype str.", + [(_HOURS, None, {"dtype": "float", "lower": 0.0, "upper": 168.0})], + id="rate-as-text", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_WEEKS, None, {"unit": Unit.YEARS})], + "a product multiplies a usual rate by a number of weeks, and " + "fact:person.weeks_worked has unit years and dtype int.", + [(_WEEKS, None, {"unit": Unit.WEEKS})], + id="weeks-in-years", + ), + pytest.param( + (_HOURS, _WEEKS), + [(_WEEKS, None, {"unit": Unit.UNIT_INTERVAL})], + "a product multiplies a usual rate by a number of weeks, and " + "fact:person.weeks_worked has unit unit_interval and dtype int.", + [(_WEEKS, None, {"unit": Unit.WEEKS})], + id="weeks-in-unit-interval", + ), + ], + ) + def test_each_way_a_product_breaks_the_rule_is_refused_by_name( + self, concepts, setup, message, fix + ) -> None: + # The whole message is matched. Each case validates once the field at + # fault is redeclared to fit, so the rule is the only obstacle. + with ExitStack() as stack: + for concept_id, basis, fields in setup: + stack.enter_context(_redeclared(concept_id, basis, **fields)) + refusal = "^" + re.escape(f"Binding for 'x': {message}") + "$" + with pytest.raises(ValueError, match=refusal): + _product(*concepts) + for concept_id, basis, fields in fix: + stack.enter_context(_redeclared(concept_id, basis, **fields)) + assert _product(*concepts).concepts == concepts + + def test_redeclaring_either_operand_of_a_committed_product_refuses_it( + self, + ) -> None: + # Exhaustive: every committed product, each operand, and every basis. + # Any basis but the operand's own refuses the binding and names the + # operand. This is the intended change to the complement test above, + # which used to keep the hours product valid under every basis it + # tried (all but annual_flow). + products = [ + (name, binding) + for name, binding in _COMMITTED + if isinstance(binding.transform, Product) + ] + assert products + for name, binding in products: + assert not _faults(*map(concept, binding.concepts)), (name, binding.ref) + for concept_id in binding.concepts: + own = concept(concept_id).temporal_basis + for basis in TemporalBasis: + with _redeclared(concept_id, basis): + if basis is own: + assert replace(binding) == binding, (name, binding.ref) + continue + with pytest.raises(ValueError, match="a product") as error: + replace(binding) + assert concept_id in str(error.value), (name, binding.ref, basis) + + @pytest.mark.parametrize("slot", [0, 1], ids=["rate", "weeks"]) + def test_the_uk_hours_product_refuses_every_other_person_concept( + self, slot + ) -> None: + # Exhaustive over policyengine-uk's JSON form: each operand of + # hours_worked swapped for each other person concept, which leaves + # ``unmapped``. The product rule refuses every edit and names the + # newcomer. + committed = MAPPINGS["policyengine-uk"].bindings_for(_HOURS)[0].concepts + assert committed == (_HOURS, _WEEKS) + swapped = 0 + for item in CONCEPTS: + if item.entity != "person" or item.id in committed: + continue + concepts = list(committed) + concepts[slot] = item.id + with pytest.raises(ValueError, match="'hours_worked': a product") as error: + ConceptMapping.from_dict(_uk_hours_reading(tuple(concepts))) + assert item.id in str(error.value) + swapped += 1 + assert swapped == sum(item.entity == "person" for item in CONCEPTS) - 2 + + def test_the_stock_times_weeks_worked_is_refused_in_the_uk_mapping(self) -> None: + # The case that prompted the rule, in a mapping's JSON form. It + # validates if the stock is declared a usual rate. + edited = _uk_hours_reading((_LIQUID_ASSETS, _WEEKS)) + refusal = ( + r"^Binding for 'hours_worked': a product reads a usual rate and an " + r"annual flow, and fact:person\.liquid_financial_assets has temporal " + r"basis reference_state\.$" + ) + with pytest.raises(ValueError, match=refusal): + ConceptMapping.from_dict(edited) + with _redeclared(_LIQUID_ASSETS, TemporalBasis.USUAL_RATE): + ConceptMapping.from_dict(edited) + class TestCoverageReport: @pytest.mark.parametrize("name", sorted(MAPPINGS)) diff --git a/packages/microcosm-frame/tests/engine_free/shared/test_concepts.py b/packages/microcosm-frame/tests/engine_free/shared/test_concepts.py index 1f97a493b4..c809b599d9 100644 --- a/packages/microcosm-frame/tests/engine_free/shared/test_concepts.py +++ b/packages/microcosm-frame/tests/engine_free/shared/test_concepts.py @@ -6,6 +6,7 @@ """ import json +import re from dataclasses import replace import numpy as np @@ -236,12 +237,46 @@ class TestConceptValidation: ({"unit": Unit.PERSON_ID}, "int named"), ({"lower": 200.0}, "empty"), ({"transport": TransportRule.QUANTILE_MAP}, "quantile"), + ({"temporal_basis": TemporalBasis.ANNUAL_FLOW}, "its period is year"), ], ) def test_bad_declarations_are_refused(self, change, message) -> None: with pytest.raises(ValueError, match=message): replace(concept("fact:person.age"), **change) + def test_an_annual_flow_has_period_year_and_nothing_else_is_tied(self) -> None: + # Exhaustive: every committed concept, redeclared under every basis + # and every period. Exactly an annual flow outside period year is + # refused, with the whole message. Every other basis takes any + # period, so the tie runs one way: usual weekly hours, a usual rate, + # has period year. + flows = [ + item + for item in CONCEPTS + if item.temporal_basis is TemporalBasis.ANNUAL_FLOW + ] + assert len(flows) == 14 + assert {item.period for item in flows} == {"year"} + assert concept("fact:person.usual_weekly_hours").period == "year" + for item in CONCEPTS: + for basis in TemporalBasis: + for period in ("year", "month", "point"): + change = {"temporal_basis": basis, "period": period} + if basis is TemporalBasis.ANNUAL_FLOW and period != "year": + refusal = ( + f"^Concept {re.escape(repr(item.id))}: an annual flow " + f"accumulates over the year, so its period is year, " + f"not {period}\\.$" + ) + with pytest.raises(ValueError, match=refusal): + replace(item, **change) + else: + redeclared = replace(item, **change) + assert (redeclared.temporal_basis, redeclared.period) == ( + basis, + period, + ) + def test_generated_concepts_must_carry(self) -> None: with pytest.raises(ValueError, match="persistence"): replace(concept("fact:person.take_up_seed"), transport=TransportRule.DROP) From 38c313157deada0f5275ee67cdcb4464ad614f0c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 7 Oct 2026 12:12:00 -0400 Subject: [PATCH 2/4] Draw distinct product operands instead of assuming them; record coverage The product property drew both operands independently and rejected collisions with assume(), which discarded up to a quarter of the examples. It now draws the second operand from the concepts the first did not take, and records whether each example validates or is refused as a Hypothesis event (the valid branch runs in about 4-8% of examples). The ADR says "any other concept" to match. Co-Authored-By: Claude Opus 5.5 --- docs/concept-schema-transport-adr.md | 2 +- .../engine_free/shared/test_concept_mapping.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/concept-schema-transport-adr.md b/docs/concept-schema-transport-adr.md index 36f60457d6..b8c68f4576 100644 --- a/docs/concept-schema-transport-adr.md +++ b/docs/concept-schema-transport-adr.md @@ -312,7 +312,7 @@ These hold for every input and are tested (Hypothesis properties unless noted): two usual rates, two annual flows, a usual rate that is not a float, or an annual flow that is not a number of weeks, such as a year's income. A refusal of two concepts names only operands at fault. A Hypothesis property - swaps either operand of the committed product for any concept and redeclares + swaps either operand of the committed product for any other concept and redeclares its basis, and, for a numeric quantity that is neither an amount nor a pointer, its dtype and unit. In either order, the binding validates exactly when this rule says it should. Each way to break the rule is also refused by diff --git a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py index 71e059e0b6..a7384c2b9c 100644 --- a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py +++ b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py @@ -19,7 +19,7 @@ import numpy as np import pandas as pd import pytest -from hypothesis import assume, given, settings +from hypothesis import assume, event, given, settings from hypothesis import strategies as st from microcosm.frame import concept_mapping as concept_mapping_module @@ -1413,12 +1413,12 @@ def test_a_product_validates_exactly_when_it_reads_a_float_rate_and_weeks( # for any concept, and its basis, dtype and unit redeclared. In either # order, the binding validates exactly when the rule's text says it # should, and a refusal names only operands that break the rule. - drawn = [] + drawn, taken = [], set() for slot in (_HOURS, _WEEKS): - concept_id = data.draw( - st.one_of(st.just(slot), st.sampled_from([c.id for c in CONCEPTS])), - label="concept", - ) + others = st.sampled_from([c.id for c in CONCEPTS if c.id not in taken]) + choices = others if slot in taken else st.one_of(st.just(slot), others) + concept_id = data.draw(choices, label="concept") + taken.add(concept_id) basis = data.draw( st.one_of(st.none(), st.sampled_from(TemporalBasis)), label="basis" ) @@ -1439,11 +1439,11 @@ def test_a_product_validates_exactly_when_it_reads_a_float_rate_and_weeks( fields["unit"] = unit drawn.append((concept_id, basis, fields)) (first, *_), (second, *_) = drawn - assume(first != second) with ExitStack() as stack: for concept_id, basis, fields in drawn: stack.enter_context(_redeclared(concept_id, basis, **fields)) faults = _faults(concept(first), concept(second)) + event("refused" if faults else "validates") for order in ((first, second), (second, first)): if not faults: assert _product(*order).concepts == order From ae38613f6e860017ec9c379a5e3aae6dd6b0ce6b Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 11 Nov 2026 08:46:56 -0500 Subject: [PATCH 3/4] Name the signed part in the annual-flow rule's text; date the product amendment The annual-flow rule covers six transforms, and its prose named five: a signed part computes from annual flows too, so "other transforms are outside both rules" contradicted the code. The ADR's lists, the test class docstring and the _FLOW_TRANSFORMS comment now name it. The ADR also gives the product and period rules their own dated amendment, says the product rule refuses a concept of any other basis, and says the rule leaves the usual rate's unit unchecked. No code changes. Co-Authored-By: Claude Fable 5.2 --- docs/concept-schema-transport-adr.md | 26 ++++++++++--------- .../src/microcosm/frame/concept_mapping.py | 4 +-- .../shared/test_concept_mapping.py | 4 +-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/docs/concept-schema-transport-adr.md b/docs/concept-schema-transport-adr.md index 992a1b3588..ad5ec1316b 100644 --- a/docs/concept-schema-transport-adr.md +++ b/docs/concept-schema-transport-adr.md @@ -19,16 +19,16 @@ schema version 2 adds `liquid_financial_assets`, the first stock concept, for New Zealand's Accommodation Supplement cash-asset test. Only the Axiom New Zealand mapping binds it, as a `Family` binding that `encode_groups` executes on built benefit units; the other three mappings list it as unmapped with a -reason. A scale, sum, scaled sum, share or fraction now computes from annual -flows only (invariant 11), so none of them can rescale the stock, add it to a -year's income, or split it. A product now multiplies one usual rate by one -annual flow counted in weeks, and every annual flow has period `year` (both -also invariant 11). +reason. A scale, sum, scaled sum, signed part, share or fraction now computes +from annual flows only (invariant 11), so none of them can rescale the stock, +add it to a year's income, or split it. **Amended 2026-10-10:** schema version 3 adds original `household.source_household_id` lineage for donor support diagnostics. The reader scopes explicit raw household identifiers by source year and carries the same key across support clones and their benefit units. Rules mappings leave this provenance concept unmapped. +**Amended 2026-11-11:** a product multiplies one usual rate by one annual flow +counted in weeks, and every annual flow has period `year` (both invariant 11). ## Context @@ -155,8 +155,9 @@ binds engine inputs to concepts. Each binding states: positivity test, sum, a sum then a fixed scale, product, allocation to the reference person, a predicate, a relationship role, a co-resident child count, a take-up threshold, or a test of a built unit's composition. A scale, - sum, scaled sum, share or fraction computes from annual flows only, and a - product multiplies one usual rate by one annual flow counted in weeks; + sum, scaled sum, signed part, share or fraction computes from annual flows + only, and a product multiplies one usual rate by one annual flow counted in + weeks; - the relation, in the same vocabulary; - the evidence, quoting the engine's own definition or how builds populate the input. @@ -323,7 +324,7 @@ These hold for every input and are tested (Hypothesis properties unless noted): wrongly typed field is named. So do unit rules, state bindings and input closures. 11. **Arithmetic reads annual flows, and a product multiplies a usual rate by - weeks.** A scale, sum, scaled sum, share or fraction binding + weeks.** A scale, sum, scaled sum, signed part, share or fraction binding computes only from concepts whose declared temporal basis is `annual_flow`: every concept in its `concepts`. A stock such as `liquid_financial_assets` has no weekly value and cannot be added to a @@ -348,10 +349,11 @@ These hold for every input and are tested (Hypothesis properties unless noted): year's weeks of activity, so only a number of weeks multiplies it to a year's total. The one committed product is policyengine-uk's hours input, usual weekly hours times weeks worked. The rule refuses a product of any - number of concepts but two, and one that reads a stock or a persistent draw, - two usual rates, two annual flows, a usual rate that is not a float, or an - annual flow that is not a number of weeks, such as a year's income. A - refusal of two concepts names only operands at fault. A Hypothesis property + number of concepts but two, and one that reads a concept of any other basis + (a stock or a persistent draw, for example), two usual rates, two annual + flows, a usual rate that is not a float, or an annual flow that is not a + number of weeks, such as a year's income. The rule leaves the usual rate's + unit unchecked. A refusal of two concepts names only operands at fault. A Hypothesis property swaps either operand of the committed product for any other concept and redeclares its basis, and, for a numeric quantity that is neither an amount nor a pointer, its dtype and unit. In either order, the binding validates exactly diff --git a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py index f0d923b33c..2dd61e9c70 100644 --- a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py +++ b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py @@ -930,8 +930,8 @@ def bind( #: Transforms that do arithmetic on a year's amount: a scale restates it per -#: week or month, a sum adds amounts (and a scaled sum scales the total), and -#: a share or fraction splits one. Each reads annual flows only, so none of +#: week or month, a sum adds amounts (a scaled sum scales the total, and a +#: signed part keeps one sign of it), and a share or fraction splits one. Each reads annual flows only, so none of #: them takes a stock (a value at the reference date), a usual rate or a #: persistent draw by accident; a binding that needs one needs a new transform. #: The rule checks the concepts a diff --git a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py index cd534cf4ca..e8605e3227 100644 --- a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py +++ b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py @@ -1554,8 +1554,8 @@ class TestArithmeticReadsAnnualFlows: cleared it from ``unmapped``, and found that both ``Scale(1/52)`` and ``Sum(interest_income, liquid_financial_assets)`` validated. A stock has no weekly value and cannot be added to a year's income, so every scale, - sum, scaled sum, share or fraction binding checks the declared temporal - basis of each concept it computes from. The household reference person + sum, scaled sum, signed part, share or fraction binding checks the declared + temporal basis of each concept it computes from. The household reference person that a binding allocated to the reference unit also reads, to place its value, is not an operand. From 5368b170fed558ed20278ca3f1c13e7f70df5de9 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 11 Nov 2026 09:07:27 -0500 Subject: [PATCH 4/4] Name the signed part in four more lists in the tests; rewrap a comment A reviewer of the previous commit found four places in test_concept_mapping.py that still listed five of the six transforms under the annual-flow rule: two docstrings and two comments. They now name the signed part. One test loops over every committed arithmetic binding, the UK mapping's two signed parts included, and its comment now says so. Also rewraps the _FLOW_TRANSFORMS comment and gives an ADR parenthesis a verb. No code changes: both Python files parse to the same tree as before once docstrings are set aside. Co-Authored-By: Claude Fable 5.2 --- docs/concept-schema-transport-adr.md | 3 ++- .../src/microcosm/frame/concept_mapping.py | 7 ++++--- .../engine_free/shared/test_concept_mapping.py | 15 ++++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/concept-schema-transport-adr.md b/docs/concept-schema-transport-adr.md index ad5ec1316b..54b743cd92 100644 --- a/docs/concept-schema-transport-adr.md +++ b/docs/concept-schema-transport-adr.md @@ -28,7 +28,8 @@ reader scopes explicit raw household identifiers by source year and carries the same key across support clones and their benefit units. Rules mappings leave this provenance concept unmapped. **Amended 2026-11-11:** a product multiplies one usual rate by one annual flow -counted in weeks, and every annual flow has period `year` (both invariant 11). +counted in weeks, and every annual flow has period `year` (invariant 11 states +both). ## Context diff --git a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py index 2dd61e9c70..90e3a22ffb 100644 --- a/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py +++ b/packages/microcosm-frame/src/microcosm/frame/concept_mapping.py @@ -931,9 +931,10 @@ def bind( #: Transforms that do arithmetic on a year's amount: a scale restates it per #: week or month, a sum adds amounts (a scaled sum scales the total, and a -#: signed part keeps one sign of it), and a share or fraction splits one. Each reads annual flows only, so none of -#: them takes a stock (a value at the reference date), a usual rate or a -#: persistent draw by accident; a binding that needs one needs a new transform. +#: signed part keeps one sign of it), and a share or fraction splits one. Each +#: reads annual flows only, so none of them takes a stock (a value at the +#: reference date), a usual rate or a persistent draw by accident; a binding +#: that needs one needs a new transform. #: The rule checks the concepts a #: binding computes from (``concepts``), not the household reference person a #: binding allocated to the reference unit also reads to place its value. A diff --git a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py index e8605e3227..cbf78b3a4f 100644 --- a/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py +++ b/packages/microcosm-frame/tests/engine_free/shared/test_concept_mapping.py @@ -1384,7 +1384,8 @@ def test_a_fraction_is_encoded_but_never_decoded(self) -> None: def _passes_type_checks(kind: type, concept_id: str) -> bool: """Whether a ``kind`` binding accepts ``concept_id`` on all but its basis. - A scale needs a float; a sum, scaled sum, share or fraction needs an amount. + A scale needs a float; a sum, scaled sum, signed part, share or fraction + needs an amount. These are the binding checks that ran before the flow rule existed. """ item = concept(concept_id) @@ -1547,7 +1548,7 @@ def _uk_hours_reading(concepts: tuple[str, str]) -> dict: class TestArithmeticReadsAnnualFlows: - """A scale, sum, scaled sum, share or fraction reads annual flows only. + """A scale, sum, scaled sum, signed part, share or fraction reads annual flows only. microcosm#1120 added the first stock, ``liquid_financial_assets``. Its review (finding F6) bound the US donor's ``bank_account_assets`` to it, @@ -1623,7 +1624,7 @@ def test_each_concept_that_is_not_an_annual_flow_is_refused_by_name( self, kind, concept_id ) -> None: # Today: a scale of the stock, usual weekly hours or the take-up seed, - # and a sum, scaled sum, share or fraction of the stock. + # and a sum, scaled sum, signed part, share or fraction of the stock. concepts = (concept_id,) if kind in (Sum, ScaledSum): concepts = ("fact:person.interest_income", concept_id) @@ -1635,10 +1636,10 @@ def test_each_concept_that_is_not_an_annual_flow_is_refused_by_name( def test_redeclaring_any_operand_of_any_committed_arithmetic_binding_refuses_it( self, ) -> None: - # Exhaustive: every committed scale, sum, scaled sum, share and fraction, - # every concept it computes from, and every temporal basis that is not - # an annual flow. Changing only that concept's declared basis refuses - # the binding, so the rule reads the declaration. Redeclaring a + # Exhaustive: every committed scale, sum, scaled sum, signed part, share + # and fraction, every concept it computes from, and every temporal basis + # that is not an annual flow. Changing only that concept's declared basis + # refuses the binding, so the rule reads the declaration. Redeclaring a # placement pointer the binding also reads leaves it valid. arithmetic = [ (name, binding)