From d6626794366003b53c2c1fe554a15306bdf2a98a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:04:55 +0400 Subject: [PATCH 01/14] Add nullable catalog format 2 columns for v3's metadata Catalog format 2 stores the metadata the app's v3 workspace reads, which PolicyEngine.py now publishes in its public records: - model versions: the catalog format, entities and basic inputs; - variables: the input and hidden-input flags, the module, the parameter named by adds or subtracts, and enum value labels; - parameters: the economy and household flags. Revision f5c1d9b4685b was autogenerated against the v2 configuration and adds every column as nullable with no default, so catalog versions published before format 2 keep them empty. It writes no data. Co-Authored-By: Claude Opus 5.5 --- ...9b4685b_store_catalog_format_2_metadata.py | 80 +++++++++++ policyengine_api/data/v2/migration_target.py | 2 +- policyengine_api/data/v2/models/metadata.py | 17 +++ .../integration/test_alembic_v2_lifecycle.py | 131 ++++++++++++++++++ tests/unit/v2/test_alembic_v2.py | 50 +++++++ tests/unit/v2/test_models.py | 45 ++++++ 6 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py diff --git a/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py b/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py new file mode 100644 index 000000000..b0dc56d28 --- /dev/null +++ b/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py @@ -0,0 +1,80 @@ +"""store catalog format 2 metadata + +Revision ID: f5c1d9b4685b +Revises: fa7fa1035444 +Create Date: 2026-10-22 18:59:48.186978 +Generation: uv run alembic -c alembic-v2.ini revision --autogenerate +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel + + +revision: str = "f5c1d9b4685b" +down_revision: Union[str, None] = "fa7fa1035444" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("parameters", sa.Column("economy", sa.Boolean(), nullable=True)) + op.add_column("parameters", sa.Column("household", sa.Boolean(), nullable=True)) + op.add_column( + "tax_benefit_model_versions", + sa.Column("catalog_format", sa.Integer(), nullable=True), + ) + op.add_column( + "tax_benefit_model_versions", sa.Column("entities", sa.JSON(), nullable=True) + ) + op.add_column( + "tax_benefit_model_versions", + sa.Column("basic_inputs", sa.JSON(), nullable=True), + ) + op.add_column( + "variables", sa.Column("is_input_variable", sa.Boolean(), nullable=True) + ) + op.add_column("variables", sa.Column("hidden_input", sa.Boolean(), nullable=True)) + op.add_column( + "variables", + sa.Column( + "module_name", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True + ), + ) + op.add_column( + "variables", + sa.Column( + "adds_parameter_path", + sqlmodel.sql.sqltypes.AutoString(length=512), + nullable=True, + ), + ) + op.add_column( + "variables", + sa.Column( + "subtracts_parameter_path", + sqlmodel.sql.sqltypes.AutoString(length=512), + nullable=True, + ), + ) + op.add_column("variables", sa.Column("enum_values", sa.JSON(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("variables", "enum_values") + op.drop_column("variables", "subtracts_parameter_path") + op.drop_column("variables", "adds_parameter_path") + op.drop_column("variables", "module_name") + op.drop_column("variables", "hidden_input") + op.drop_column("variables", "is_input_variable") + op.drop_column("tax_benefit_model_versions", "basic_inputs") + op.drop_column("tax_benefit_model_versions", "entities") + op.drop_column("tax_benefit_model_versions", "catalog_format") + op.drop_column("parameters", "household") + op.drop_column("parameters", "economy") + # ### end Alembic commands ### diff --git a/policyengine_api/data/v2/migration_target.py b/policyengine_api/data/v2/migration_target.py index a57c2419c..d463fad16 100644 --- a/policyengine_api/data/v2/migration_target.py +++ b/policyengine_api/data/v2/migration_target.py @@ -24,7 +24,7 @@ from collections.abc import Mapping V2_ALEMBIC_DISPOSABLE_TEST = "V2_ALEMBIC_DISPOSABLE_TEST" -V2_ALEMBIC_HEAD_REVISION = "fa7fa1035444" +V2_ALEMBIC_HEAD_REVISION = "f5c1d9b4685b" DISPOSABLE_DATABASE_NAME = "policyengine_v2_alembic_test" DISPOSABLE_HOSTS = frozenset({"localhost", "127.0.0.1", "::1", "postgres"}) MIGRATION_ROLE = "policyengine_v2_migrator" diff --git a/policyengine_api/data/v2/models/metadata.py b/policyengine_api/data/v2/models/metadata.py index 37f04f7be..ca0153467 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -85,6 +85,11 @@ class TaxBenefitModelVersion(IdentifiedModel, table=True): description: str | None = None current_law_id: int metadata_time_periods: list[int] = Field(sa_type=sa.JSON) + # Catalog format 2 fields. Versions published before format 2 keep them + # null; entities are {key, plural, is_person} objects. + catalog_format: int | None = None + entities: list[dict[str, Any]] | None = Field(default=None, sa_type=sa.JSON) + basic_inputs: list[str] | None = Field(default=None, sa_type=sa.JSON) model: TaxBenefitModel = Relationship(back_populates="versions") variables: list["Variable"] = Relationship( @@ -277,6 +282,15 @@ class Variable(IdentifiedModel, table=True): default_value: Any = Field(default=None, sa_type=sa.JSON) adds: list[str] | None = Field(default=None, sa_type=sa.JSON) subtracts: list[str] | None = Field(default=None, sa_type=sa.JSON) + # Catalog format 2 fields; null in versions published before it. A + # parameter path names the parameter whose dated value lists the variables + # this one adds or subtracts. Enum values are {value, label} objects. + is_input_variable: bool | None = None + hidden_input: bool | None = None + module_name: str | None = Field(default=None, max_length=512) + adds_parameter_path: str | None = Field(default=None, max_length=512) + subtracts_parameter_path: str | None = Field(default=None, max_length=512) + enum_values: list[dict[str, Any]] | None = Field(default=None, sa_type=sa.JSON) tax_benefit_model_version_id: UUID = Field( foreign_key="tax_benefit_model_versions.id", ondelete="CASCADE", @@ -337,6 +351,9 @@ class Parameter(IdentifiedModel, table=True): description: str | None = None data_type: str | None = Field(default=None, max_length=128) unit: str | None = Field(default=None, max_length=128) + # Catalog format 2 fields; null in versions published before it. + economy: bool | None = None + household: bool | None = None tax_benefit_model_version_id: UUID = Field( foreign_key="tax_benefit_model_versions.id", ondelete="CASCADE", diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 5ef7c6169..61ff7a72b 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1414,3 +1414,134 @@ def test_previous_release_still_finds_user_policies() -> None: if saved is not None: _delete_saved_policy_rows(engine, parent, saved) engine.dispose() + + +CATALOG_FORMAT_2_PARENT_REVISION = "fa7fa1035444" +CATALOG_FORMAT_2_COLUMNS = { + "tax_benefit_model_versions": ("catalog_format", "entities", "basic_inputs"), + "variables": ( + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", + ), + "parameters": ("economy", "household"), +} + + +def _catalog_version_published_before_format_2(connection: sa.Connection) -> UUID: + """Store a model version with one variable and one parameter; return its id.""" + model_id = uuid4() + version_id = uuid4() + connection.execute( + text( + "INSERT INTO tax_benefit_models (id, name) " + "VALUES (:model_id, 'catalog-format-probe')" + ), + {"model_id": model_id}, + ) + connection.execute( + text( + "INSERT INTO tax_benefit_model_versions " + "(id, model_id, version, current_law_id, metadata_time_periods) " + "VALUES (:version_id, :model_id, '0.0.1-probe', 1, '[2026]')" + ), + {"version_id": version_id, "model_id": model_id}, + ) + connection.execute( + text( + "INSERT INTO variables " + "(id, name, entity, default_value, tax_benefit_model_version_id) " + "VALUES (:variable_id, 'probe_income', 'person', '0', :version_id)" + ), + {"variable_id": uuid4(), "version_id": version_id}, + ) + connection.execute( + text( + "INSERT INTO parameters (id, name, tax_benefit_model_version_id) " + "VALUES (:parameter_id, 'gov.probe.rate', :version_id)" + ), + {"parameter_id": uuid4(), "version_id": version_id}, + ) + return version_id + + +def _format_2_values(connection: sa.Connection, version_id: UUID) -> list[tuple]: + """Return the format 2 columns of the version's model, variable and parameter.""" + rows = [] + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + key = ( + "id" + if table_name == "tax_benefit_model_versions" + else "tax_benefit_model_version_id" + ) + rows.append( + tuple( + connection.execute( + text( + f"SELECT {', '.join(columns)} FROM {table_name} " + f"WHERE {key} = :version_id" + ), + {"version_id": version_id}, + ).one() + ) + ) + return rows + + +def test_catalog_format_2_columns_leave_earlier_versions_empty() -> None: + """A version stored before catalog format 2 keeps its format 2 fields null. + + Upgrading adds the columns as nullable without writing to stored rows, and + downgrading drops them and keeps every row. + """ + config = _config() + engine = create_engine(_disposable_url()) + version_id = None + try: + command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + assert set(columns).isdisjoint(_columns(engine, table_name)) + with engine.begin() as connection: + version_id = _catalog_version_published_before_format_2(connection) + + command.upgrade(config, "head") + + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + stored_columns = _columns(engine, table_name) + for column_name in columns: + assert stored_columns[column_name]["nullable"] + assert stored_columns[column_name]["default"] is None + with engine.connect() as connection: + assert _format_2_values(connection, version_id) == [ + (None, None, None), + (None, None, None, None, None, None), + (None, None), + ] + + command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) + + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + assert set(columns).isdisjoint(_columns(engine, table_name)) + with engine.connect() as connection: + counts = connection.execute( + text( + "SELECT (SELECT count(*) FROM variables " + "WHERE tax_benefit_model_version_id = :version_id), " + "(SELECT count(*) FROM parameters " + "WHERE tax_benefit_model_version_id = :version_id)" + ), + {"version_id": version_id}, + ).one() + assert tuple(counts) == (1, 1) + finally: + with engine.begin() as connection: + connection.execute( + text( + "DELETE FROM tax_benefit_models WHERE name = 'catalog-format-probe'" + ) + ) + command.upgrade(config, "head") + engine.dispose() diff --git a/tests/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index 94d79f577..5a3d20de6 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -247,6 +247,7 @@ def test_v2_revision_chain_has_generated_policy_and_user_identity_changes() -> N assert script.get_heads() == [V2_ALEMBIC_HEAD_REVISION] assert [revision.revision for revision in script.walk_revisions()] == [ V2_ALEMBIC_HEAD_REVISION, + SAVED_POLICY_RENAME_REVISION, SAVED_POLICY_RENAME_PARENT_REVISION, "91af4fc8d8c1", "60d6518b6a98", @@ -809,3 +810,52 @@ def test_no_other_v2_revision_runs_hand_written_sql() -> None: } assert revisions_with_sql == {SAVED_POLICY_RENAME_REVISION} + + +def test_catalog_format_2_revision_adds_only_nullable_columns() -> None: + """The revision after the saved-policy rename adds catalog format 2's columns. + + It is generated, adds each column as nullable so versions published earlier + keep them empty, drops them on downgrade, and writes no data. + """ + script = ScriptDirectory.from_config(Config(str(REPO / "alembic-v2.ini"))) + children = [ + revision + for revision in script.walk_revisions() + if revision.down_revision == SAVED_POLICY_RENAME_REVISION + ] + assert len(children) == 1 + revision = Path(children[0].path).read_text(encoding="utf-8") + upgrade = revision[ + revision.index("def upgrade()") : revision.index("def downgrade()") + ] + downgrade = revision[revision.index("def downgrade()") :] + + assert ( + "Generation: uv run alembic -c alembic-v2.ini revision --autogenerate" + in revision + ) + added_columns = { + "tax_benefit_model_versions": ("catalog_format", "entities", "basic_inputs"), + "variables": ( + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", + ), + "parameters": ("economy", "household"), + } + column_count = sum(len(columns) for columns in added_columns.values()) + assert upgrade.count("op.add_column(") == column_count + assert upgrade.count("nullable=True") == column_count + assert downgrade.count("op.drop_column(") == column_count + for table_name, columns in added_columns.items(): + for column_name in columns: + assert f'"{column_name}"' in upgrade + assert f'op.drop_column("{table_name}", "{column_name}")' in downgrade + for body in (upgrade, downgrade): + assert "op.execute(" not in body + assert "op.bulk_insert(" not in body + assert "server_default" not in body diff --git a/tests/unit/v2/test_models.py b/tests/unit/v2/test_models.py index 17b87ec03..cdca61496 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -4,6 +4,7 @@ from sqlalchemy.dialects import postgresql from sqlalchemy.orm import configure_mappers from sqlalchemy.schema import CreateTable +from sqlmodel.sql.sqltypes import AutoString from policyengine_api.data.v1_models import V1Base from policyengine_api.data.v2.models import ( @@ -676,3 +677,47 @@ def test_saved_policy_table_is_named_like_the_other_association_tables() -> None ("user_report_associations", "user_policy_id"), ("user_report_associations", "baseline_user_policy_id"), } + + +def test_catalog_format_2_columns_are_nullable_for_earlier_versions() -> None: + """Catalog format 2 stores v3's metadata; versions published before it keep null. + + The model version records its catalog format, entities and basic inputs; each + variable its input flags, module, parameter-named parts and enum labels; each + parameter its economy and household flags. + """ + expected_columns = { + "tax_benefit_model_versions": { + "catalog_format": sa.Integer, + "entities": sa.JSON, + "basic_inputs": sa.JSON, + }, + "variables": { + "is_input_variable": sa.Boolean, + "hidden_input": sa.Boolean, + "module_name": AutoString, + "adds_parameter_path": AutoString, + "subtracts_parameter_path": AutoString, + "enum_values": sa.JSON, + }, + "parameters": { + "economy": sa.Boolean, + "household": sa.Boolean, + }, + } + + for table_name, columns in expected_columns.items(): + table = V2_METADATA.tables[table_name] + for column_name, column_type in columns.items(): + column = table.c[column_name] + assert column.nullable, f"{table_name}.{column_name}" + assert column.server_default is None, f"{table_name}.{column_name}" + assert isinstance(column.type, column_type), f"{table_name}.{column_name}" + + variables = V2_METADATA.tables["variables"] + for column_name in ( + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + ): + assert variables.c[column_name].type.length == 512 From 14673dc46b23c5bf342ed61a76585ee8a533acec Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:14:39 +0400 Subject: [PATCH 02/14] Extract v3's metadata from PolicyEngine.py's public records as catalog format 2 The extractor now reads, only from PolicyEngine.py's public records: - each model version's entities (key, plural, person flag) and basic inputs; - each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, and its enum values' declared labels; - each parameter's economy and household flags. Bracket schedules already arrive as public parameter nodes, with their stored label or none. Extraction fails with a specific message when the public model publishes no entities or basic inputs, has other than one person entity, names a basic input or a parameter path that is not published, gives enum labels that do not match the possible values or are not JSON scalars, or gives a flag that is not a boolean. normalize_model_metadata normalizes this metadata apart from the certified data release, and an opt-in parity test compares every new field for the US and the UK with what /{country}/metadata builds. The v2 integration check runs it with the installed-catalog test. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/v2-integration-check.yml | 4 +- .../data/v2/catalog/extraction.py | 266 ++++++++++++-- policyengine_api/data/v2/catalog/records.py | 54 ++- tests/fixtures/v2_catalog.py | 13 + .../test_v2_catalog_metadata_parity.py | 178 +++++++++ tests/unit/test_alembic_workflows.py | 1 + tests/unit/v2/test_catalog_metadata_fields.py | 337 ++++++++++++++++++ 7 files changed, 828 insertions(+), 25 deletions(-) create mode 100644 tests/integration/test_v2_catalog_metadata_parity.py create mode 100644 tests/unit/v2/test_catalog_metadata_fields.py diff --git a/.github/workflows/v2-integration-check.yml b/.github/workflows/v2-integration-check.yml index 7959a83c2..9c4cd7493 100644 --- a/.github/workflows/v2-integration-check.yml +++ b/.github/workflows/v2-integration-check.yml @@ -73,8 +73,8 @@ jobs: run: uv run python scripts/reconcile_v2_privileges.py apply --create-missing-roles - name: Verify the managed v2 roles hold their declared privileges run: uv run python scripts/reconcile_v2_privileges.py verify - - name: Verify the installed PolicyEngine.py catalog interface - run: uv run coverage run --branch -m pytest -q tests/integration/test_v2_catalog_installed.py + - name: Verify the installed PolicyEngine.py catalog interface and its parity with v1 metadata + run: uv run coverage run --branch -m pytest -q tests/integration/test_v2_catalog_installed.py tests/integration/test_v2_catalog_metadata_parity.py env: RUN_V2_CATALOG_COMPATIBILITY: "1" - name: Test v2 metadata publication and resource routes diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 410340cea..7e795cd70 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -5,6 +5,7 @@ import math import re from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass from datetime import UTC, date, datetime, timedelta from enum import Enum from importlib import metadata as importlib_metadata @@ -13,8 +14,11 @@ from policyengine_api.data.v2.catalog.errors import CatalogExtractionError from policyengine_api.data.v2.catalog.records import ( + CATALOG_FORMAT, CountryCatalog, DatasetRecord, + EntityRecord, + EnumValueRecord, FallbackSummary, ModelRecord, ModelVersionRecord, @@ -91,6 +95,20 @@ def _optional_text( return value +def _optional_bool(value: object, *, field_name: str) -> bool | None: + if value is not None and not isinstance(value, bool): + msg = f"{field_name} must be a boolean or null" + raise CatalogExtractionError(msg) + return value + + +def _required_bool(value: object, *, field_name: str) -> bool: + if not isinstance(value, bool): + msg = f"{field_name} must be a boolean" + raise CatalogExtractionError(msg) + return value + + def _type_name(value: object) -> str | None: if value is None: return None @@ -277,6 +295,12 @@ def _normalize_variable( else: normalized_possible_values = None + enum_values = _normalize_enum_values( + name, + getattr(source, "enum_values", None), + possible_values=normalized_possible_values, + ) + def optional_names(attribute: str) -> list[str] | None: values = getattr(source, attribute, None) if values is None: @@ -316,9 +340,70 @@ def optional_names(attribute: str) -> list[str] | None: default_value=normalize_json_value(getattr(source, "default_value", None)), adds=optional_names("adds"), subtracts=optional_names("subtracts"), + is_input_variable=_optional_bool( + getattr(source, "is_input_variable", None), + field_name=f"variable {name} is_input_variable", + ), + hidden_input=_optional_bool( + getattr(source, "hidden_input", None), + field_name=f"variable {name} hidden_input", + ), + module_name=_optional_text( + getattr(source, "module_name", None), + field_name=f"variable {name} module_name", + maximum=512, + ), + adds_parameter_path=_optional_text( + getattr(source, "adds_parameter_path", None), + field_name=f"variable {name} adds_parameter_path", + maximum=512, + ), + subtracts_parameter_path=_optional_text( + getattr(source, "subtracts_parameter_path", None), + field_name=f"variable {name} subtracts_parameter_path", + maximum=512, + ), + enum_values=enum_values, ) +def _normalize_enum_values( + name: str, + source_values: object, + *, + possible_values: list[str] | None, +) -> tuple[EnumValueRecord, ...] | None: + """Normalize an enum variable's value labels, which must match its values. + + Each label is kept as declared when it is a JSON scalar. + """ + if source_values is None and possible_values is None: + return None + if ( + source_values is None + or not isinstance(source_values, Sequence) + or isinstance(source_values, (str, bytes)) + ): + msg = f"variable {name} enum values do not match its possible values" + raise CatalogExtractionError(msg) + records: list[EnumValueRecord] = [] + for source_value in source_values: + value = _required_text( + getattr(source_value, "value", None), + field_name=f"variable {name} enum value", + maximum=255, + ) + label = normalize_json_value(getattr(source_value, "label", None)) + if isinstance(label, (dict, list)): + msg = f"variable {name} enum label for {value} must be a JSON scalar" + raise CatalogExtractionError(msg) + records.append(EnumValueRecord(value=value, label=label)) + if [record.value for record in records] != possible_values: + msg = f"variable {name} enum values do not match its possible values" + raise CatalogExtractionError(msg) + return tuple(records) + + def _normalize_parameter( source: object, *, @@ -409,6 +494,14 @@ def _normalize_parameter( maximum=128, ), values=tuple(normalized_values), + economy=_required_bool( + getattr(source, "economy", None), + field_name=f"parameter {name} economy", + ), + household=_required_bool( + getattr(source, "household", None), + field_name=f"parameter {name} household", + ), ) @@ -526,6 +619,150 @@ def _normalize_parameter_nodes( return tuple(normalized_nodes) +def _normalize_entities(country_id: str, source: object) -> tuple[EntityRecord, ...]: + """Normalize the public model's entities, in its order, with one person entity.""" + source_entities = getattr(source, "entities", None) + if not isinstance(source_entities, Sequence) or isinstance( + source_entities, (str, bytes) + ): + msg = ( + f"{country_id} public model publishes no entities; catalog format 2 " + "needs PolicyEngine.py's public entity records" + ) + raise CatalogExtractionError(msg) + entities: list[EntityRecord] = [] + for source_entity in source_entities: + key = _required_text( + getattr(source_entity, "key", None), + field_name=f"{country_id} entity key", + maximum=128, + ) + if any(entity.key == key for entity in entities): + msg = f"{country_id} entity key {key!r} is repeated" + raise CatalogExtractionError(msg) + entities.append( + EntityRecord( + key=key, + plural=_required_text( + getattr(source_entity, "plural", None), + field_name=f"{country_id} entity {key} plural", + maximum=128, + ), + is_person=_required_bool( + getattr(source_entity, "is_person", None), + field_name=f"{country_id} entity {key} is_person", + ), + ) + ) + if sum(entity.is_person for entity in entities) != 1: + msg = f"{country_id} public model must have exactly one person entity" + raise CatalogExtractionError(msg) + return tuple(entities) + + +def _normalize_basic_inputs( + country_id: str, + source: object, + *, + variable_names: frozenset[str], +) -> tuple[str, ...]: + """Normalize the country's default household fields; each is a variable.""" + source_inputs = getattr(source, "basic_inputs", None) + if not isinstance(source_inputs, Sequence) or isinstance( + source_inputs, (str, bytes) + ): + msg = ( + f"{country_id} public model publishes no basic inputs; catalog format 2 " + "needs PolicyEngine.py's public basic inputs" + ) + raise CatalogExtractionError(msg) + basic_inputs = tuple( + _required_text(value, field_name=f"{country_id} basic input", maximum=512) + for value in source_inputs + ) + for name in basic_inputs: + if name not in variable_names: + msg = f"{country_id} basic input {name!r} is not a published variable" + raise CatalogExtractionError(msg) + return basic_inputs + + +def _verify_parameter_paths( + variables: tuple[VariableRecord, ...], + *, + parameter_names: frozenset[str], +) -> None: + """Require each parameter named by ``adds`` or ``subtracts`` to be published.""" + for variable in variables: + for field_name, path in ( + ("adds_parameter_path", variable.adds_parameter_path), + ("subtracts_parameter_path", variable.subtracts_parameter_path), + ): + if path is not None and path not in parameter_names: + msg = ( + f"variable {variable.name} {field_name} {path!r} is not a " + "published parameter" + ) + raise CatalogExtractionError(msg) + + +@dataclass(frozen=True, slots=True) +class ModelMetadata: + """The model metadata of one country's catalog, without regions or datasets.""" + + entities: tuple[EntityRecord, ...] + basic_inputs: tuple[str, ...] + variables: tuple[VariableRecord, ...] + parameter_nodes: tuple[ParameterNodeRecord, ...] + parameters: tuple[ParameterRecord, ...] + + +def normalize_model_metadata( + country_id: str, + source: object, + *, + model_version_id: UUID, +) -> ModelMetadata: + """Normalize a public model's entities, variables and parameter tree. + + Everything comes from PolicyEngine.py's public records. Separate from the + certified data release, so the metadata can be checked on its own. + """ + variables = tuple( + _normalize_variable(variable, model_version_id=model_version_id) + for variable in _public_named_records( + source, + attribute="variables_by_name", + entity_name="variable", + ) + ) + parameters = tuple( + _normalize_parameter(parameter, model_version_id=model_version_id) + for parameter in _public_named_records( + source, + attribute="parameters_by_name", + entity_name="parameter", + ) + ) + _verify_parameter_paths( + variables, + parameter_names=frozenset(parameter.name for parameter in parameters), + ) + return ModelMetadata( + entities=_normalize_entities(country_id, source), + basic_inputs=_normalize_basic_inputs( + country_id, + source, + variable_names=frozenset(variable.name for variable in variables), + ), + variables=variables, + parameter_nodes=_normalize_parameter_nodes( + source, model_version_id=model_version_id + ), + parameters=parameters, + ) + + def _verified_default_dataset(country_id: str, release_manifest: object) -> str: """Return the reviewed default dataset once the manifest names it as its default.""" reviewed_default = REVIEWED_DEFAULT_DATASETS[country_id] @@ -581,24 +818,8 @@ def _normalize_country( expected_country_package_version=expected_country_package_version, ) - variables = tuple( - _normalize_variable(variable, model_version_id=model_version_id) - for variable in _public_named_records( - source, - attribute="variables_by_name", - entity_name="variable", - ) - ) - parameter_nodes = _normalize_parameter_nodes( - source, model_version_id=model_version_id - ) - parameters = tuple( - _normalize_parameter(parameter, model_version_id=model_version_id) - for parameter in _public_named_records( - source, - attribute="parameters_by_name", - entity_name="parameter", - ) + metadata = normalize_model_metadata( + country_id, source, model_version_id=model_version_id ) reviewed_default = _verified_default_dataset(country_id, release_manifest) @@ -650,10 +871,13 @@ def _normalize_country( ), current_law_id=CURRENT_LAW_IDS[country_id], metadata_time_periods=METADATA_TIME_PERIODS[country_id], + catalog_format=CATALOG_FORMAT, + entities=metadata.entities, + basic_inputs=metadata.basic_inputs, ), - variables=variables, - parameter_nodes=parameter_nodes, - parameters=parameters, + variables=metadata.variables, + parameter_nodes=metadata.parameter_nodes, + parameters=metadata.parameters, datasets=datasets, regions=tuple(sorted(regions, key=lambda record: record.code)), fallback_summaries=tuple( diff --git a/policyengine_api/data/v2/catalog/records.py b/policyengine_api/data/v2/catalog/records.py index f05233202..56eaafc95 100644 --- a/policyengine_api/data/v2/catalog/records.py +++ b/policyengine_api/data/v2/catalog/records.py @@ -11,6 +11,37 @@ from uuid import UUID +CATALOG_FORMAT = 2 +"""The catalog content format this extractor publishes. + +Format 2 adds the metadata the app's v3 workspace reads, from PolicyEngine.py's +public records: entities and basic inputs; each variable's input and +hidden-input flags, module, the parameter its ``adds`` or ``subtracts`` names, +and enum value labels; each parameter's economy and household flags. Versions +published before it store null for these fields. +""" + + +@dataclass(frozen=True, slots=True) +class EntityRecord: + """A model entity: its key, plural name, and whether it is the person entity.""" + + key: str + plural: str + is_person: bool + + +@dataclass(frozen=True, slots=True) +class EnumValueRecord: + """One value of an enum variable: its stored name and its declared label. + + The label is kept as the country package declares it, usually a string. + """ + + value: str + label: str | int | float | bool | None + + @dataclass(frozen=True, slots=True) class ModelRecord: """One country's tax-benefit model as named in the catalog.""" @@ -35,11 +66,19 @@ class ModelVersionRecord: description: str | None current_law_id: int metadata_time_periods: tuple[int, ...] + catalog_format: int + entities: tuple[EntityRecord, ...] + basic_inputs: tuple[str, ...] @dataclass(frozen=True, slots=True) class VariableRecord: - """One model variable, with its entity, type and the variables it sums.""" + """One model variable, with its entity, type and the variables it sums. + + ``adds_parameter_path`` and ``subtracts_parameter_path`` name the parameter + whose dated value lists the variables summed; ``adds`` and ``subtracts`` + keep the list PolicyEngine.py resolved from it. + """ id: UUID model_version_id: UUID @@ -52,11 +91,20 @@ class VariableRecord: default_value: Any adds: list[str] | None subtracts: list[str] | None + is_input_variable: bool | None + hidden_input: bool | None + module_name: str | None + adds_parameter_path: str | None + subtracts_parameter_path: str | None + enum_values: tuple[EnumValueRecord, ...] | None @dataclass(frozen=True, slots=True) class ParameterNodeRecord: - """A non-leaf node of the parameter tree; it groups parameters and has no values.""" + """A non-leaf node of the parameter tree; it groups parameters and has no values. + + Bracket schedules are nodes too, with their stored label or none. + """ id: UUID model_version_id: UUID @@ -88,6 +136,8 @@ class ParameterRecord: data_type: str | None unit: str | None values: tuple[ParameterValueRecord, ...] + economy: bool + household: bool @dataclass(frozen=True, slots=True) diff --git a/tests/fixtures/v2_catalog.py b/tests/fixtures/v2_catalog.py index 61e569f95..73f293237 100644 --- a/tests/fixtures/v2_catalog.py +++ b/tests/fixtures/v2_catalog.py @@ -221,6 +221,12 @@ def _model_source( default_value=0.0, adds=None, subtracts=None, + is_input_variable=True, + hidden_input=False, + module_name="household.income.employment_income", + adds_parameter_path=None, + subtracts_parameter_path=None, + enum_values=None, ) parameter_node = SimpleNamespace( name="gov.example", @@ -233,6 +239,8 @@ def _model_source( description="An example rate", data_type=float, unit="/1", + economy=True, + household=False, parameter_values=[ SimpleNamespace( value=0.1, @@ -265,6 +273,11 @@ def _model_source( region_datasets=region_datasets, data_package=data_package, ) + version_holder.entities = [ + SimpleNamespace(key="person", plural="people", is_person=True), + SimpleNamespace(key="household", plural="households", is_person=False), + ] + version_holder.basic_inputs = ["employment_income"] version_holder.variables = [variable] version_holder.parameter_nodes = [parameter_node] version_holder.parameters = [parameter] diff --git a/tests/integration/test_v2_catalog_metadata_parity.py b/tests/integration/test_v2_catalog_metadata_parity.py new file mode 100644 index 000000000..9b9bd7790 --- /dev/null +++ b/tests/integration/test_v2_catalog_metadata_parity.py @@ -0,0 +1,178 @@ +"""Parity of catalog format 2's fields with the v1 ``/{country}/metadata`` document. + +The catalog reads these fields from PolicyEngine.py's public records; the v1 +document builds them from the country package's tax-benefit system. Each new +field must agree for the US and the UK. The check normalizes the installed +public models' metadata only, so it does not depend on the certified data +release that region publication reads. +""" + +from __future__ import annotations + +import os +from typing import TYPE_CHECKING, Any +from uuid import uuid4 + +import pytest + +from policyengine_api.data.v2.catalog.extraction import normalize_model_metadata + +if TYPE_CHECKING: + from policyengine_api.data.v2.catalog.extraction import ModelMetadata + +pytestmark = pytest.mark.skipif( + os.environ.get("RUN_V2_CATALOG_COMPATIBILITY") != "1", + reason=("loads the complete public catalog; set RUN_V2_CATALOG_COMPATIBILITY=1"), +) + +COUNTRY_PACKAGES = {"us": "policyengine_us", "uk": "policyengine_uk"} + + +@pytest.fixture(scope="module", params=("us", "uk")) +def country_pair(request: pytest.FixtureRequest) -> tuple[ModelMetadata, Any]: + """Return one country's normalized metadata and its v1 metadata builder.""" + import policyengine + + from policyengine_api.country import PolicyEngineCountry + + country_id = request.param + metadata = normalize_model_metadata( + country_id, + getattr(policyengine, country_id).model, + model_version_id=uuid4(), + ) + return metadata, PolicyEngineCountry(COUNTRY_PACKAGES[country_id], country_id) + + +def _report(mismatches: list[str]) -> str: + shown = "\n".join(mismatches[:20]) + return f"{len(mismatches)} mismatches, first {min(len(mismatches), 20)}:\n{shown}" + + +def test_entities_and_basic_inputs_match_v1( + country_pair: tuple[ModelMetadata, Any], +) -> None: + metadata, v1_country = country_pair + v1_entities = v1_country.metadata["entities"] + + assert [ + (entity.key, entity.plural, entity.is_person) for entity in metadata.entities + ] == [ + (key, entity["plural"], entity["is_person"]) + for key, entity in v1_entities.items() + ] + assert list(metadata.basic_inputs) == (v1_country.metadata["basicInputs"]) + + +def test_variable_flags_modules_enum_labels_and_parts_match_v1( + country_pair: tuple[ModelMetadata, Any], +) -> None: + metadata, v1_country = country_pair + v1_variables = v1_country.metadata["variables"] + mismatches: list[str] = [] + + assert {variable.name for variable in metadata.variables} == set(v1_variables) + for variable in metadata.variables: + v1_variable = v1_variables[variable.name] + enum_values = ( + [ + {"value": item.value, "label": item.label} + for item in variable.enum_values + ] + if variable.enum_values is not None + else None + ) + comparisons = { + "is_input_variable": ( + variable.is_input_variable, + v1_variable["isInputVariable"], + ), + "hidden_input": (variable.hidden_input, v1_variable["hidden_input"]), + "module_name": (variable.module_name, v1_variable["moduleName"]), + "enum_values": (enum_values, v1_variable.get("possibleValues")), + "adds_parameter_path": ( + variable.adds_parameter_path, + v1_variable["adds"] if isinstance(v1_variable["adds"], str) else None, + ), + "subtracts_parameter_path": ( + variable.subtracts_parameter_path, + v1_variable["subtracts"] + if isinstance(v1_variable["subtracts"], str) + else None, + ), + } + mismatches.extend( + f"{variable.name}.{field}: catalog {stored!r}, v1 {expected!r}" + for field, (stored, expected) in comparisons.items() + if stored != expected or type(stored) is not type(expected) + ) + + assert not mismatches, _report(mismatches) + + +def test_parameter_flags_match_v1(country_pair: tuple[ModelMetadata, Any]) -> None: + metadata, v1_country = country_pair + v1_parameters = { + name: entry + for name, entry in v1_country.metadata["parameters"].items() + if entry["type"] == "parameter" + } + catalog_parameters = { + parameter.name: parameter + for parameter in metadata.parameters + if parameter.name.startswith("gov") + } + mismatches: list[str] = [] + + assert set(catalog_parameters) == set(v1_parameters) + for name, parameter in catalog_parameters.items(): + expected = (v1_parameters[name]["economy"], v1_parameters[name]["household"]) + if (parameter.economy, parameter.household) != expected: + mismatches.append( + f"{name}: catalog {(parameter.economy, parameter.household)}, " + f"v1 {expected}" + ) + + assert not mismatches, _report(mismatches) + + +def test_bracket_schedules_are_nodes_with_the_stored_label_v1_shows( + country_pair: tuple[ModelMetadata, Any], +) -> None: + """Every schedule v1 lists is a catalog node; a stored label matches v1's. + + v1 falls back to the name's last part when a schedule has no stored label; + the catalog stores no label, and the routes generate one. + """ + from policyengine_core.parameters import ParameterScale + + metadata, v1_country = country_pair + nodes = {node.name: node for node in metadata.parameter_nodes} + v1_parameters = v1_country.metadata["parameters"] + schedules = [ + parameter + for parameter in v1_country.tax_benefit_system.parameters.get_descendants() + if isinstance(parameter, ParameterScale) and parameter.name.startswith("gov") + ] + mismatches: list[str] = [] + + assert schedules + for schedule in schedules: + node = nodes.get(schedule.name) + stored_label = schedule.metadata.get("label") + if node is None: + mismatches.append(f"{schedule.name}: not a catalog node") + elif node.label != stored_label: + mismatches.append( + f"{schedule.name}: catalog label {node.label!r}, " + f"stored label {stored_label!r}" + ) + elif stored_label is not None and ( + v1_parameters[schedule.name]["label"] != stored_label + ): + mismatches.append( + f"{schedule.name}: catalog label {node.label!r}, " + f"v1 label {v1_parameters[schedule.name]['label']!r}" + ) + + assert not mismatches, _report(mismatches) diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index a0e85c6c5..91992c01a 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -219,6 +219,7 @@ def test_reusable_v2_integration_check_uses_disposable_databases_and_coverage(): assert "alembic -c alembic-v2.ini upgrade head" in workflow assert "test_alembic_v2_lifecycle.sh" not in workflow assert "test_v2_catalog_installed.py" in workflow + assert "test_v2_catalog_metadata_parity.py" in workflow assert "RUN_V2_CATALOG_COMPATIBILITY" in workflow assert "test_v2_catalog_publication.py" in workflow assert "test_v2_metadata_routes.py" in workflow diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py new file mode 100644 index 000000000..cdab9464f --- /dev/null +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -0,0 +1,337 @@ +"""Catalog format 2: the metadata v3 reads, taken from .py's public records.""" + +from __future__ import annotations + +from datetime import UTC, datetime +from types import SimpleNamespace +from typing import TYPE_CHECKING + +import pytest + +from policyengine_api.data.v2.catalog.extraction import ( + CatalogExtractionError, + extract_catalog, +) +from policyengine_api.data.v2.catalog.records import ( + CATALOG_FORMAT, + EntityRecord, + EnumValueRecord, + NormalizedCatalog, +) +from tests.fixtures.v2_catalog import ( + POLICYENGINE_VERSION, + bundle, + installed_version, + normalized_catalog, + region_references, + source_models, +) + +if TYPE_CHECKING: + from collections.abc import Callable + + +def _extract(models: dict[str, SimpleNamespace]) -> NormalizedCatalog: + return extract_catalog( + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + region_references=region_references(), + ) + + +def _add_variable(model: SimpleNamespace, **fields: object) -> None: + variable = SimpleNamespace( + label=None, + entity="person", + description=None, + data_type=float, + possible_values=None, + default_value=0.0, + adds=None, + subtracts=None, + is_input_variable=False, + hidden_input=False, + module_name=None, + adds_parameter_path=None, + subtracts_parameter_path=None, + enum_values=None, + ) + for name, value in fields.items(): + setattr(variable, name, value) + model.variables.append(variable) + model.variables_by_name[variable.name] = variable + + +def _add_parameter(model: SimpleNamespace, name: str, *, value: object) -> None: + parameter = SimpleNamespace( + name=name, + label=None, + description=None, + data_type=type(value), + unit=None, + economy=True, + household=True, + parameter_values=[ + SimpleNamespace( + value=value, + start_date=datetime(2025, 1, 1, tzinfo=UTC), + end_date=None, + ) + ], + ) + model.parameters.append(parameter) + model.parameters_by_name[name] = parameter + + +def _add_parameter_node(model: SimpleNamespace, name: str, label: str | None) -> None: + node = SimpleNamespace(name=name, label=label, description=None) + model.parameter_nodes.append(node) + model.parameter_nodes_by_name[name] = node + + +def test_catalog_format_2_publishes_entities_and_basic_inputs() -> None: + catalog = normalized_catalog() + + for country_id in ("us", "uk"): + model_version = catalog.country(country_id).model_version + assert model_version.catalog_format == CATALOG_FORMAT == 2 + assert model_version.entities == ( + EntityRecord(key="person", plural="people", is_person=True), + EntityRecord(key="household", plural="households", is_person=False), + ) + assert model_version.basic_inputs == ("employment_income",) + + +def test_variables_carry_input_flags_module_and_unset_parts() -> None: + variable = normalized_catalog().country("us").variables[0] + + assert variable.name == "employment_income" + assert variable.is_input_variable is True + assert variable.hidden_input is False + assert variable.module_name == "household.income.employment_income" + assert variable.adds_parameter_path is None + assert variable.subtracts_parameter_path is None + assert variable.enum_values is None + + +def test_enum_values_keep_their_declared_labels_in_order() -> None: + models = source_models() + _add_variable( + models["us"], + name="filing_status", + data_type="Enum", + possible_values=["SINGLE", "JOINT", "EXEMPT"], + default_value="SINGLE", + is_input_variable=True, + enum_values=[ + SimpleNamespace(value="SINGLE", label="Single"), + SimpleNamespace(value="JOINT", label=2), + SimpleNamespace(value="EXEMPT", label=None), + ], + ) + + variables = { + variable.name: variable for variable in _extract(models).country("us").variables + } + + assert variables["filing_status"].enum_values == ( + EnumValueRecord(value="SINGLE", label="Single"), + EnumValueRecord(value="JOINT", label=2), + EnumValueRecord(value="EXEMPT", label=None), + ) + assert isinstance(variables["filing_status"].enum_values[1].label, int) + + +def test_parameter_named_parts_record_the_parameter_path_beside_the_list() -> None: + models = source_models() + _add_parameter( + models["us"], + "gov.example.countable_income", + value=["employment_income"], + ) + _add_parameter(models["us"], "gov.example.deductions", value=["employment_income"]) + _add_variable( + models["us"], + name="countable_income", + adds=["employment_income"], + subtracts=["employment_income"], + adds_parameter_path="gov.example.countable_income", + subtracts_parameter_path="gov.example.deductions", + ) + + variables = { + variable.name: variable for variable in _extract(models).country("us").variables + } + + countable_income = variables["countable_income"] + assert countable_income.adds_parameter_path == "gov.example.countable_income" + assert countable_income.subtracts_parameter_path == "gov.example.deductions" + assert countable_income.adds == ["employment_income"] + assert countable_income.subtracts == ["employment_income"] + + +def test_parameters_carry_their_economy_and_household_flags() -> None: + parameter = normalized_catalog().country("us").parameters[0] + + assert parameter.name == "gov.example.rate" + assert (parameter.economy, parameter.household) == (True, False) + + +def test_bracket_schedules_are_stored_as_nodes_with_their_stored_label() -> None: + models = source_models() + _add_parameter_node(models["uk"], "gov.example.rates", "Example rates") + _add_parameter_node(models["uk"], "gov.example.rates[0]", None) + _add_parameter_node(models["uk"], "gov.example.thresholds", None) + _add_parameter_node(models["uk"], "gov.example.thresholds[0]", None) + _add_parameter(models["uk"], "gov.example.rates[0].rate", value=0.2) + _add_parameter(models["uk"], "gov.example.thresholds[0].threshold", value=0) + + nodes = {node.name: node for node in _extract(models).country("uk").parameter_nodes} + + assert nodes["gov.example.rates"].label == "Example rates" + assert nodes["gov.example.thresholds"].label is None + assert "gov.example.rates[0]" in nodes + + +def _remove_entities(models: dict[str, SimpleNamespace]) -> None: + del models["us"].entities + + +def _make_entity_flag_text(models: dict[str, SimpleNamespace]) -> None: + models["us"].entities[0].is_person = "yes" + + +def _remove_person_entity(models: dict[str, SimpleNamespace]) -> None: + models["us"].entities = models["us"].entities[1:] + + +def _repeat_entity_key(models: dict[str, SimpleNamespace]) -> None: + models["us"].entities.append( + SimpleNamespace(key="household", plural="homes", is_person=False) + ) + + +def _remove_basic_inputs(models: dict[str, SimpleNamespace]) -> None: + del models["us"].basic_inputs + + +def _name_unknown_basic_input(models: dict[str, SimpleNamespace]) -> None: + models["us"].basic_inputs = ["employment_income", "unpublished_input"] + + +def _make_input_flag_text(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].is_input_variable = "true" + + +def _make_hidden_flag_number(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].hidden_input = 0 + + +def _name_unknown_adds_parameter(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].adds_parameter_path = "gov.example.missing" + + +def _name_unknown_subtracts_parameter(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].subtracts_parameter_path = "gov.example.missing" + + +def _mismatch_enum_values(models: dict[str, SimpleNamespace]) -> None: + variable = models["us"].variables[0] + variable.possible_values = ["SINGLE", "JOINT"] + variable.enum_values = [SimpleNamespace(value="SINGLE", label="Single")] + + +def _give_enum_labels_without_values(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].enum_values = [ + SimpleNamespace(value="SINGLE", label="Single") + ] + + +def _make_enum_label_an_object(models: dict[str, SimpleNamespace]) -> None: + variable = models["us"].variables[0] + variable.possible_values = ["SINGLE"] + variable.enum_values = [SimpleNamespace(value="SINGLE", label={"text": "Single"})] + + +def _make_economy_flag_missing(models: dict[str, SimpleNamespace]) -> None: + del models["us"].parameters[0].economy + + +def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: + models["us"].parameters[0].household = "false" + + +@pytest.mark.parametrize( + ("break_models", "message"), + [ + ( + _remove_entities, + "^us public model publishes no entities; catalog format 2 needs " + "PolicyEngine.py's public entity records$", + ), + (_make_entity_flag_text, "^us entity person is_person must be a boolean$"), + ( + _remove_person_entity, + "^us public model must have exactly one person entity$", + ), + (_repeat_entity_key, "^us entity key 'household' is repeated$"), + ( + _remove_basic_inputs, + "^us public model publishes no basic inputs; catalog format 2 needs " + "PolicyEngine.py's public basic inputs$", + ), + ( + _name_unknown_basic_input, + "^us basic input 'unpublished_input' is not a published variable$", + ), + ( + _make_input_flag_text, + "^variable employment_income is_input_variable must be a boolean or null$", + ), + ( + _make_hidden_flag_number, + "^variable employment_income hidden_input must be a boolean or null$", + ), + ( + _name_unknown_adds_parameter, + "^variable employment_income adds_parameter_path 'gov.example.missing' " + "is not a published parameter$", + ), + ( + _name_unknown_subtracts_parameter, + "^variable employment_income subtracts_parameter_path " + "'gov.example.missing' is not a published parameter$", + ), + ( + _mismatch_enum_values, + "^variable employment_income enum values do not match its possible values$", + ), + ( + _give_enum_labels_without_values, + "^variable employment_income enum values do not match its possible values$", + ), + ( + _make_enum_label_an_object, + "^variable employment_income enum label for SINGLE must be a JSON scalar$", + ), + ( + _make_economy_flag_missing, + "^parameter gov.example.rate economy must be a boolean$", + ), + ( + _make_household_flag_text, + "^parameter gov.example.rate household must be a boolean$", + ), + ], +) +def test_inconsistent_format_2_metadata_fails_with_a_specific_message( + break_models: Callable[[dict[str, SimpleNamespace]], None], + message: str, +) -> None: + models = source_models() + break_models(models) + + with pytest.raises(CatalogExtractionError, match=message): + _extract(models) From 07b7b0cfe3a060c25768f094c6532287057a6d27 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:26:22 +0400 Subject: [PATCH 03/14] Store catalog format 2 fields when a catalog version is published Publication now stages and inserts each model version's catalog format, entities and basic inputs, each variable's input and hidden-input flags, module, parameter paths and enum values (as ordered value and label objects), and each parameter's economy and household flags. Publication never rewrites a stored version. When the same PolicyEngine.py version is published again, a version stored before catalog format 2 (a null catalog_format) is compared without format 2's fields, so it keeps them empty, and a version stored with format 2 must match on them too. The Stage 9 metadata document describes the format, and the changelog fragment covers the whole change. Co-Authored-By: Claude Opus 5.5 --- ...an-1f-115-catalog-metadata-fields.added.md | 1 + docs/migration/stage-9-v2-metadata.md | 26 ++++ .../v2/catalog/publication_reconciliation.py | 102 +++++++++++++- .../data/v2/catalog/publication_staging.py | 41 ++++++ .../test_v2_catalog_publication.py | 128 ++++++++++++++++++ tests/unit/v2/test_catalog_publication.py | 50 +++++-- tests/unit/v2/test_catalog_staging_rows.py | 40 ++++++ 7 files changed, 370 insertions(+), 18 deletions(-) create mode 100644 changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md diff --git a/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md new file mode 100644 index 000000000..7394dc074 --- /dev/null +++ b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md @@ -0,0 +1 @@ +Catalog format 2 stores the metadata the app's v3 workspace reads, taken only from PolicyEngine.py's public records: each model version's entities and basic inputs; each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, and its enum values' labels; each parameter's economy and household flags. Bracket schedules are stored as parameter nodes with their stored label. A generated revision adds the columns as nullable, so catalog versions published earlier keep them empty and are compared without them when republished. An opt-in parity test, run by the v2 integration check, compares every new field for the US and the UK with /{country}/metadata. diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 3825ad81c..3eb0a1731 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -99,6 +99,32 @@ metadata time-period options are stored on the model-version snapshot. PolicyEngine Core and country-package versions are derived diagnostic evidence and are not separately persisted. +Catalog format 2 adds the metadata the app's v3 workspace reads, taken only +from PolicyEngine.py's public records, which carry it from the release that +publishes entities, basic inputs and these flags: + +| Record | Fields | +| --- | --- | +| Model version | `catalog_format` (2), `entities` (`key`, `plural`, `is_person`, in the package's order), `basic_inputs` (the household builder's default fields) | +| Variable | `is_input_variable`, `hidden_input`, `module_name`, `adds_parameter_path` and `subtracts_parameter_path` (the parameter whose dated value lists the variables summed; `adds` and `subtracts` keep the list PolicyEngine.py resolved), `enum_values` (`value` and declared `label`, in order) | +| Parameter | `economy`, `household` | + +Bracket schedules are parameter nodes with their stored label, or none when +the package stores none. Extraction stops if the public model publishes no +entities or basic inputs, has other than one person entity, names a basic +input or a parameter path that is not published, gives enum labels that do +not match the variable's possible values or are not JSON scalars, or gives a +flag that is not a boolean. An opt-in test +(`tests/integration/test_v2_catalog_metadata_parity.py`, run by the v2 +integration check) compares every format 2 field for the US and the UK with +what `/{country}/metadata` builds. + +Publication never rewrites a stored version. A version published before +format 2 has a null `catalog_format` and null format 2 fields, and later +publications of the same PolicyEngine.py version compare it without those +fields. The first PolicyEngine.py version published after format 2 deploys +carries them. + Parameter extraction requires the PolicyEngine.py public history contract from 5.0.4 and later: values are ordered from oldest to newest, bounded `end_date` values are inclusive and fall one day before the following `start_date`, and diff --git a/policyengine_api/data/v2/catalog/publication_reconciliation.py b/policyengine_api/data/v2/catalog/publication_reconciliation.py index 8a0c9c0aa..9ddcad20d 100644 --- a/policyengine_api/data/v2/catalog/publication_reconciliation.py +++ b/policyengine_api/data/v2/catalog/publication_reconciliation.py @@ -76,10 +76,45 @@ def _versioned_table_from(table: sa.Table) -> sa.Join: ) +def stored_catalog_format( + connection: Connection, country: CountryCatalog +) -> int | None: + """Return the catalog format this country's stored version was published with. + + None means it was published before catalog format 2, so its format 2 + fields are empty. + """ + statement = ( + sa.select(MODEL_VERSIONS.c.catalog_format) + .select_from( + MODEL_VERSIONS.join( + MODELS, + MODELS.c.id == MODEL_VERSIONS.c.model_id, + ) + ) + .where( + MODELS.c.name == country.model.name, + MODEL_VERSIONS.c.version == country.model_version.version, + ) + ) + return connection.execute(statement).scalar_one_or_none() + + def _comparison_pairs( country: CountryCatalog, + *, + with_format_2: bool, ) -> tuple[tuple[str, Select, Select], ...]: - """Return each compared table's name with its staged and stored rows.""" + """Return each compared table's name with its staged and stored rows. + + Without ``with_format_2``, catalog format 2's fields are left out: a + version published before format 2 keeps them empty, and publication never + rewrites a stored version. + """ + + def format_2(*columns: sa.ColumnElement) -> tuple[sa.ColumnElement, ...]: + return columns if with_format_2 else () + staged_country = STAGE_CATALOG_MODELS.c.country_id == country.country_id selected_version = sa.and_( MODELS.c.name == country.model.name, @@ -90,12 +125,22 @@ def _comparison_pairs( STAGE_CATALOG_MODELS.c.version_description, STAGE_CATALOG_MODELS.c.current_law_id, STAGE_CATALOG_MODELS.c.metadata_time_periods, + *format_2( + STAGE_CATALOG_MODELS.c.catalog_format, + STAGE_CATALOG_MODELS.c.entities, + STAGE_CATALOG_MODELS.c.basic_inputs, + ), ).where(staged_country) actual_model_version = ( sa.select( MODEL_VERSIONS.c.description, MODEL_VERSIONS.c.current_law_id, sa.cast(MODEL_VERSIONS.c.metadata_time_periods, JSONB), + *format_2( + MODEL_VERSIONS.c.catalog_format, + sa.cast(MODEL_VERSIONS.c.entities, JSONB), + sa.cast(MODEL_VERSIONS.c.basic_inputs, JSONB), + ), ) .select_from( MODEL_VERSIONS.join( @@ -116,6 +161,14 @@ def _comparison_pairs( STAGE_CATALOG_VARIABLES.c.default_value, STAGE_CATALOG_VARIABLES.c.adds, STAGE_CATALOG_VARIABLES.c.subtracts, + *format_2( + STAGE_CATALOG_VARIABLES.c.is_input_variable, + STAGE_CATALOG_VARIABLES.c.hidden_input, + STAGE_CATALOG_VARIABLES.c.module_name, + STAGE_CATALOG_VARIABLES.c.adds_parameter_path, + STAGE_CATALOG_VARIABLES.c.subtracts_parameter_path, + STAGE_CATALOG_VARIABLES.c.enum_values, + ), ).where(STAGE_CATALOG_VARIABLES.c.country_id == country.country_id) actual_variables = ( sa.select( @@ -128,6 +181,14 @@ def _comparison_pairs( sa.cast(VARIABLES.c.default_value, JSONB), sa.cast(VARIABLES.c.adds, JSONB), sa.cast(VARIABLES.c.subtracts, JSONB), + *format_2( + VARIABLES.c.is_input_variable, + VARIABLES.c.hidden_input, + VARIABLES.c.module_name, + VARIABLES.c.adds_parameter_path, + VARIABLES.c.subtracts_parameter_path, + sa.cast(VARIABLES.c.enum_values, JSONB), + ), ) .select_from(_versioned_table_from(VARIABLES)) .where(selected_version) @@ -154,6 +215,10 @@ def _comparison_pairs( STAGE_CATALOG_PARAMETERS.c.description, STAGE_CATALOG_PARAMETERS.c.data_type, STAGE_CATALOG_PARAMETERS.c.unit, + *format_2( + STAGE_CATALOG_PARAMETERS.c.economy, + STAGE_CATALOG_PARAMETERS.c.household, + ), ).where(STAGE_CATALOG_PARAMETERS.c.country_id == country.country_id) actual_parameters = ( sa.select( @@ -162,6 +227,7 @@ def _comparison_pairs( PARAMETERS.c.description, PARAMETERS.c.data_type, PARAMETERS.c.unit, + *format_2(PARAMETERS.c.economy, PARAMETERS.c.household), ) .select_from(_versioned_table_from(PARAMETERS)) .where(selected_version) @@ -302,9 +368,11 @@ def assert_country_matches( row both fail; the error names the first table that differs, never its rows. Without ``include_regions``, region rows are not compared: they come from the Public API's reference files, which may change while the - PolicyEngine.py version stays the same. + PolicyEngine.py version stays the same. A version published before catalog + format 2 is compared without format 2's fields. """ - pairs = _comparison_pairs(country) + stored_format = stored_catalog_format(connection, country) + pairs = _comparison_pairs(country, with_format_2=stored_format is not None) if include_regions: pairs = (*pairs, ("regions", *_region_comparison_pair(country))) for table_name, staged, actual in pairs: @@ -367,6 +435,9 @@ def count_region_differences( "description", "current_law_id", "metadata_time_periods", + "catalog_format", + "entities", + "basic_inputs", ], sa.select( STAGE_CATALOG_MODELS.c.version_id, @@ -379,6 +450,12 @@ def count_region_differences( STAGE_CATALOG_MODELS.c.metadata_time_periods, MODEL_VERSIONS.c.metadata_time_periods.type, ), + STAGE_CATALOG_MODELS.c.catalog_format, + sa.cast(STAGE_CATALOG_MODELS.c.entities, MODEL_VERSIONS.c.entities.type), + sa.cast( + STAGE_CATALOG_MODELS.c.basic_inputs, + MODEL_VERSIONS.c.basic_inputs.type, + ), ) .select_from( STAGE_CATALOG_MODELS.join( @@ -442,6 +519,12 @@ def count_region_differences( "default_value", "adds", "subtracts", + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", "tax_benefit_model_version_id", ], sa.select( @@ -465,6 +548,15 @@ def count_region_differences( STAGE_CATALOG_VARIABLES.c.subtracts, VARIABLES.c.subtracts.type, ), + STAGE_CATALOG_VARIABLES.c.is_input_variable, + STAGE_CATALOG_VARIABLES.c.hidden_input, + STAGE_CATALOG_VARIABLES.c.module_name, + STAGE_CATALOG_VARIABLES.c.adds_parameter_path, + STAGE_CATALOG_VARIABLES.c.subtracts_parameter_path, + sa.cast( + STAGE_CATALOG_VARIABLES.c.enum_values, + VARIABLES.c.enum_values.type, + ), MODEL_VERSIONS.c.id, ) .select_from( @@ -528,6 +620,8 @@ def count_region_differences( "description", "data_type", "unit", + "economy", + "household", "tax_benefit_model_version_id", ], sa.select( @@ -538,6 +632,8 @@ def count_region_differences( STAGE_CATALOG_PARAMETERS.c.description, STAGE_CATALOG_PARAMETERS.c.data_type, STAGE_CATALOG_PARAMETERS.c.unit, + STAGE_CATALOG_PARAMETERS.c.economy, + STAGE_CATALOG_PARAMETERS.c.household, MODEL_VERSIONS.c.id, ) .select_from( diff --git a/policyengine_api/data/v2/catalog/publication_staging.py b/policyengine_api/data/v2/catalog/publication_staging.py index 76820b27b..e0dfa3c31 100644 --- a/policyengine_api/data/v2/catalog/publication_staging.py +++ b/policyengine_api/data/v2/catalog/publication_staging.py @@ -16,6 +16,7 @@ from policyengine_api.data.v2.catalog.records import ( CountryCatalog, NormalizedCatalog, + VariableRecord, iter_batches, ) @@ -39,6 +40,9 @@ sa.Column("version_description", sa.Text), sa.Column("current_law_id", sa.Integer, nullable=False), sa.Column("metadata_time_periods", JSONB, nullable=False), + sa.Column("catalog_format", sa.Integer, nullable=False), + sa.Column("entities", JSONB, nullable=False), + sa.Column("basic_inputs", JSONB, nullable=False), prefixes=["TEMPORARY"], postgresql_on_commit="DROP", ) @@ -57,6 +61,12 @@ sa.Column("default_value", JSONB, nullable=False), sa.Column("adds", JSONB), sa.Column("subtracts", JSONB), + sa.Column("is_input_variable", sa.Boolean), + sa.Column("hidden_input", sa.Boolean), + sa.Column("module_name", sa.Text), + sa.Column("adds_parameter_path", sa.Text), + sa.Column("subtracts_parameter_path", sa.Text), + sa.Column("enum_values", JSONB), prefixes=["TEMPORARY"], postgresql_on_commit="DROP", ) @@ -83,6 +93,8 @@ sa.Column("description", sa.Text), sa.Column("data_type", sa.Text), sa.Column("unit", sa.Text), + sa.Column("economy", sa.Boolean, nullable=False), + sa.Column("household", sa.Boolean, nullable=False), prefixes=["TEMPORARY"], postgresql_on_commit="DROP", ) @@ -163,6 +175,27 @@ def _model_rows(country: CountryCatalog) -> Iterator[tuple[object, ...]]: country.model_version.description, country.model_version.current_law_id, Jsonb(country.model_version.metadata_time_periods), + country.model_version.catalog_format, + Jsonb( + [ + { + "key": entity.key, + "plural": entity.plural, + "is_person": entity.is_person, + } + for entity in country.model_version.entities + ] + ), + Jsonb(list(country.model_version.basic_inputs)), + ) + + +def _enum_values_json(record: VariableRecord) -> Jsonb | None: + """Return an enum variable's values as ordered value and label objects.""" + if record.enum_values is None: + return None + return Jsonb( + [{"value": item.value, "label": item.label} for item in record.enum_values] ) @@ -182,6 +215,12 @@ def _variable_rows(country: CountryCatalog) -> Iterator[tuple[object, ...]]: Jsonb(record.default_value), _optional_json(record.adds), _optional_json(record.subtracts), + record.is_input_variable, + record.hidden_input, + record.module_name, + record.adds_parameter_path, + record.subtracts_parameter_path, + _enum_values_json(record), ) @@ -213,6 +252,8 @@ def _parameter_rows(country: CountryCatalog) -> Iterator[tuple[object, ...]]: record.description, record.data_type, record.unit, + record.economy, + record.household, ) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index edef7b93e..0e4033379 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -550,3 +550,131 @@ def test_new_version_is_additive_and_concurrent_retries_serialize( "reports": 0, "report_runs": 0, } + + +def _format_2_values(engine: Engine) -> dict[str, object]: + """Return the US version's stored catalog format 2 fields.""" + with engine.connect() as connection: + version = connection.execute( + text( + "SELECT version.catalog_format, version.entities, version.basic_inputs " + "FROM tax_benefit_model_versions AS version " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "WHERE model.name = 'policyengine-us'" + ) + ).one() + variable = connection.execute( + text( + "SELECT is_input_variable, hidden_input, module_name, " + "adds_parameter_path, subtracts_parameter_path, enum_values " + "FROM variables WHERE name = 'employment_income' " + "AND tax_benefit_model_version_id = (" + "SELECT version.id FROM tax_benefit_model_versions AS version " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "WHERE model.name = 'policyengine-us')" + ) + ).one() + parameter = connection.execute( + text( + "SELECT economy, household FROM parameters " + "WHERE name = 'gov.example.rate' " + "AND tax_benefit_model_version_id = (" + "SELECT version.id FROM tax_benefit_model_versions AS version " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "WHERE model.name = 'policyengine-us')" + ) + ).one() + return { + "version": tuple(version), + "variable": tuple(variable), + "parameter": tuple(parameter), + } + + +def _clear_format_2_fields(engine: Engine) -> None: + """Make every stored version look as if published before catalog format 2.""" + with engine.begin() as connection: + connection.execute( + text( + "UPDATE tax_benefit_model_versions SET catalog_format = NULL, " + "entities = NULL, basic_inputs = NULL" + ) + ) + connection.execute( + text( + "UPDATE variables SET is_input_variable = NULL, hidden_input = NULL, " + "module_name = NULL, adds_parameter_path = NULL, " + "subtracts_parameter_path = NULL, enum_values = NULL" + ) + ) + connection.execute( + text("UPDATE parameters SET economy = NULL, household = NULL") + ) + + +def test_publication_stores_catalog_format_2_fields( + publication_engine: Engine, +) -> None: + publish_catalog(publication_engine, normalized_catalog()) + + assert _format_2_values(publication_engine) == { + "version": ( + 2, + [ + {"key": "person", "plural": "people", "is_person": True}, + {"key": "household", "plural": "households", "is_person": False}, + ], + ["employment_income"], + ), + "variable": ( + True, + False, + "household.income.employment_income", + None, + None, + None, + ), + "parameter": (True, False), + } + + +def test_version_stored_before_format_2_keeps_its_fields_empty_on_republish( + publication_engine: Engine, +) -> None: + catalog = normalized_catalog() + publish_catalog(publication_engine, catalog) + _clear_format_2_fields(publication_engine) + before_ids = _identifiers(publication_engine) + + result = publish_catalog(publication_engine, catalog) + + assert result.as_dict()["outcome"] == "ok" + assert _identifiers(publication_engine) == before_ids + assert _format_2_values(publication_engine) == { + "version": (None, None, None), + "variable": (None, None, None, None, None, None), + "parameter": (None, None), + } + + +def test_version_stored_with_format_2_fails_when_a_format_2_field_changes( + publication_engine: Engine, +) -> None: + catalog = normalized_catalog() + publish_catalog(publication_engine, catalog) + before_counts = _counts(publication_engine) + us = catalog.country("us") + changed_parameter = replace(us.parameters[0], household=True) + changed = replace( + catalog, + countries=( + replace(us, parameters=(changed_parameter,)), + catalog.country("uk"), + ), + ) + + with pytest.raises(CatalogPublicationError, match="differs"): + publish_catalog(publication_engine, changed) + + assert _counts(publication_engine) == before_counts + assert _format_2_values(publication_engine)["parameter"] == (True, False) diff --git a/tests/unit/v2/test_catalog_publication.py b/tests/unit/v2/test_catalog_publication.py index 2341003dc..afdc7f5a2 100644 --- a/tests/unit/v2/test_catalog_publication.py +++ b/tests/unit/v2/test_catalog_publication.py @@ -214,22 +214,42 @@ def test_publication_uses_mapped_tables_and_sqlalchemy_statements() -> None: assert all("VALUES" not in statement for statement in compiled_inserts) catalog = normalized_catalog() - comparisons = publication_reconciliation._comparison_pairs(catalog.country("us")) - assert [table_name for table_name, _staged, _stored in comparisons] == [ - "model version", - "variables", - "parameter nodes", - "parameters", - "parameter values", - "datasets", - ] - statements = [ - statement for _table_name, *pair in comparisons for statement in pair - ] + list(publication_reconciliation._region_comparison_pair(catalog.country("us"))) - assert all( - isinstance(statement, sa.sql.Select) and not isinstance(statement, TextClause) - for statement in statements + format_1_comparisons, format_2_comparisons = ( + publication_reconciliation._comparison_pairs( + catalog.country("us"), with_format_2=with_format_2 + ) + for with_format_2 in (False, True) ) + for comparisons in (format_1_comparisons, format_2_comparisons): + assert [table_name for table_name, _staged, _stored in comparisons] == [ + "model version", + "variables", + "parameter nodes", + "parameters", + "parameter values", + "datasets", + ] + statements = [ + statement for _table_name, *pair in comparisons for statement in pair + ] + list( + publication_reconciliation._region_comparison_pair(catalog.country("us")) + ) + assert all( + isinstance(statement, sa.sql.Select) + and not isinstance(statement, TextClause) + for statement in statements + ) + assert all( + len(staged.selected_columns) == len(stored.selected_columns) + for _table_name, staged, stored in comparisons + ) + # Format 2 compares model versions, variables and parameters on more fields. + assert [ + len(format_2[1].selected_columns) - len(format_1[1].selected_columns) + for format_1, format_2 in zip( + format_1_comparisons, format_2_comparisons, strict=True + ) + ] == [3, 6, 0, 2, 0, 0] class Result: def scalar_one(self): diff --git a/tests/unit/v2/test_catalog_staging_rows.py b/tests/unit/v2/test_catalog_staging_rows.py index f43f2772f..549743810 100644 --- a/tests/unit/v2/test_catalog_staging_rows.py +++ b/tests/unit/v2/test_catalog_staging_rows.py @@ -3,13 +3,16 @@ from __future__ import annotations import inspect +from dataclasses import replace from psycopg.types.json import Jsonb from policyengine_api.data.v2.catalog.publication_staging import ( + STAGE_CATALOG_VARIABLES, STAGING_TABLES, _catalog_rows, ) +from policyengine_api.data.v2.catalog.records import EnumValueRecord from tests.fixtures.v2_catalog import normalized_catalog @@ -49,6 +52,15 @@ def test_catalog_rows_project_every_record_field_in_column_order() -> None: country.model_version.description, country.model_version.current_law_id, ("jsonb", country.model_version.metadata_time_periods), + 2, + ( + "jsonb", + [ + {"key": "person", "plural": "people", "is_person": True}, + {"key": "household", "plural": "households", "is_person": False}, + ], + ), + ("jsonb", ["employment_income"]), ) ] assert rows["stage_catalog_variables"] == [ @@ -66,6 +78,12 @@ def test_catalog_rows_project_every_record_field_in_column_order() -> None: ("jsonb", record.default_value), None if record.adds is None else ("jsonb", record.adds), None if record.subtracts is None else ("jsonb", record.subtracts), + record.is_input_variable, + record.hidden_input, + record.module_name, + record.adds_parameter_path, + record.subtracts_parameter_path, + None, ) for record in country.variables ] @@ -82,6 +100,8 @@ def test_catalog_rows_project_every_record_field_in_column_order() -> None: record.description, record.data_type, record.unit, + record.economy, + record.household, ) for record in country.parameters ] @@ -121,3 +141,23 @@ def test_catalog_rows_project_every_record_field_in_column_order() -> None: ] assert len(rows["stage_catalog_parameter_values"]) == 2 assert len(rows["stage_catalog_regions"]) == 3 + + +def test_enum_values_are_staged_as_value_and_label_objects_in_order() -> None: + country = normalized_catalog().country("us") + variable = replace( + country.variables[0], + possible_values=["SINGLE", "JOINT"], + enum_values=( + EnumValueRecord(value="SINGLE", label="Single"), + EnumValueRecord(value="JOINT", label=2), + ), + ) + country = replace(country, variables=(variable,)) + + (row,) = (_plain(row) for row in _catalog_rows(country)[STAGE_CATALOG_VARIABLES]) + + assert row[-1] == ( + "jsonb", + [{"value": "SINGLE", "label": "Single"}, {"value": "JOINT", "label": 2}], + ) From b8a75d12c139961f2ebcfa0153b39f19b98fdaf8 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:31:18 +0400 Subject: [PATCH 04/14] Name the missing catalog format 2 fields when PolicyEngine.py lacks them Extraction now checks first that the installed PolicyEngine.py publishes catalog format 2's fields on the model version, every variable and every parameter, and fails naming each missing one. On 6.7.5 it used to stop at the first enum variable with "enum values do not match its possible values", which hid the cause. An enum label that is not JSON-compatible, such as NaN, now fails naming its variable and value. Entities or basic inputs that are present but not a sequence fail with "must be a sequence", and the Stage 9 document says that an empty basic-input list is stored as empty. Co-Authored-By: Claude Opus 5.5 --- docs/migration/stage-9-v2-metadata.md | 12 +-- .../data/v2/catalog/extraction.py | 88 +++++++++++++++---- tests/unit/v2/test_catalog_metadata_fields.py | 88 ++++++++++++++++++- 3 files changed, 160 insertions(+), 28 deletions(-) diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 3eb0a1731..0d716beee 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -110,11 +110,13 @@ publishes entities, basic inputs and these flags: | Parameter | `economy`, `household` | Bracket schedules are parameter nodes with their stored label, or none when -the package stores none. Extraction stops if the public model publishes no -entities or basic inputs, has other than one person entity, names a basic -input or a parameter path that is not published, gives enum labels that do -not match the variable's possible values or are not JSON scalars, or gives a -flag that is not a boolean. An opt-in test +the package stores none. An empty basic-input list is stored as empty. +Extraction stops before reading any record if the installed PolicyEngine.py +does not publish these fields, as 6.7.5 does not, and names each missing +field. It also stops if the model has other than one person entity, names a +basic input or a parameter path that is not published, gives enum labels that +do not match the variable's possible values or are not JSON scalars, or gives +a flag that is not a boolean. An opt-in test (`tests/integration/test_v2_catalog_metadata_parity.py`, run by the v2 integration check) compares every format 2 field for the US and the UK with what `/{country}/metadata` builds. diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 7e795cd70..34ffcc803 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -393,7 +393,14 @@ def _normalize_enum_values( field_name=f"variable {name} enum value", maximum=255, ) - label = normalize_json_value(getattr(source_value, "label", None)) + try: + label = normalize_json_value(getattr(source_value, "label", None)) + except CatalogExtractionError as error: + msg = ( + f"variable {name} enum label for {value} is not JSON-compatible: " + f"{error}" + ) + raise CatalogExtractionError(msg) from error if isinstance(label, (dict, list)): msg = f"variable {name} enum label for {value} must be a JSON scalar" raise CatalogExtractionError(msg) @@ -619,17 +626,63 @@ def _normalize_parameter_nodes( return tuple(normalized_nodes) -def _normalize_entities(country_id: str, source: object) -> tuple[EntityRecord, ...]: - """Normalize the public model's entities, in its order, with one person entity.""" - source_entities = getattr(source, "entities", None) - if not isinstance(source_entities, Sequence) or isinstance( - source_entities, (str, bytes) - ): +# The public fields catalog format 2 reads, by the record that carries them. +FORMAT_2_FIELDS: tuple[tuple[str, str, tuple[str, ...]], ...] = ( + ("model version", "", ("entities", "basic_inputs")), + ( + "variable", + "variables_by_name", + ( + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", + ), + ), + ("parameter", "parameters_by_name", ("economy", "household")), +) + + +def _require_format_2_fields(country_id: str, source: object) -> None: + """Fail first, naming each field, when PolicyEngine.py does not publish them. + + Older releases, such as 6.7.5, publish none of catalog format 2's fields. + """ + missing: list[str] = [] + for record_name, mapping_attribute, field_names in FORMAT_2_FIELDS: + if mapping_attribute: + records = getattr(source, mapping_attribute, None) + holders = list(records.values()) if isinstance(records, Mapping) else [] + else: + holders = [source] + absent = [ + field_name + for field_name in field_names + if any(not hasattr(holder, field_name) for holder in holders) + ] + if absent: + missing.append(f"{record_name} {', '.join(absent)}") + if missing: msg = ( - f"{country_id} public model publishes no entities; catalog format 2 " - "needs PolicyEngine.py's public entity records" + "the installed PolicyEngine.py does not publish catalog format 2's " + f"fields for {country_id}: {'; '.join(missing)}" ) raise CatalogExtractionError(msg) + + +def _source_sequence(country_id: str, source: object, attribute: str) -> Sequence: + values = getattr(source, attribute, None) + if not isinstance(values, Sequence) or isinstance(values, (str, bytes)): + msg = f"{country_id} public model {attribute} must be a sequence" + raise CatalogExtractionError(msg) + return values + + +def _normalize_entities(country_id: str, source: object) -> tuple[EntityRecord, ...]: + """Normalize the public model's entities, in its order, with one person entity.""" + source_entities = _source_sequence(country_id, source, "entities") entities: list[EntityRecord] = [] for source_entity in source_entities: key = _required_text( @@ -666,16 +719,12 @@ def _normalize_basic_inputs( *, variable_names: frozenset[str], ) -> tuple[str, ...]: - """Normalize the country's default household fields; each is a variable.""" - source_inputs = getattr(source, "basic_inputs", None) - if not isinstance(source_inputs, Sequence) or isinstance( - source_inputs, (str, bytes) - ): - msg = ( - f"{country_id} public model publishes no basic inputs; catalog format 2 " - "needs PolicyEngine.py's public basic inputs" - ) - raise CatalogExtractionError(msg) + """Normalize the country's default household fields; each is a variable. + + An empty list, which PolicyEngine.py gives a package that declares none, is + stored as empty. + """ + source_inputs = _source_sequence(country_id, source, "basic_inputs") basic_inputs = tuple( _required_text(value, field_name=f"{country_id} basic input", maximum=512) for value in source_inputs @@ -728,6 +777,7 @@ def normalize_model_metadata( Everything comes from PolicyEngine.py's public records. Separate from the certified data release, so the metadata can be checked on its own. """ + _require_format_2_fields(country_id, source) variables = tuple( _normalize_variable(variable, model_version_id=model_version_id) for variable in _public_named_records( diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py index cdab9464f..ab6f6414b 100644 --- a/tests/unit/v2/test_catalog_metadata_fields.py +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -217,6 +217,24 @@ def _remove_basic_inputs(models: dict[str, SimpleNamespace]) -> None: del models["us"].basic_inputs +def _make_entities_null(models: dict[str, SimpleNamespace]) -> None: + models["us"].entities = None + + +def _make_basic_inputs_text(models: dict[str, SimpleNamespace]) -> None: + models["us"].basic_inputs = "employment_income" + + +def _remove_variable_enum_values(models: dict[str, SimpleNamespace]) -> None: + del models["us"].variables[0].enum_values + + +def _make_enum_label_nan(models: dict[str, SimpleNamespace]) -> None: + variable = models["us"].variables[0] + variable.possible_values = ["SINGLE"] + variable.enum_values = [SimpleNamespace(value="SINGLE", label=float("nan"))] + + def _name_unknown_basic_input(models: dict[str, SimpleNamespace]) -> None: models["us"].basic_inputs = ["employment_income", "unpublished_input"] @@ -259,6 +277,10 @@ def _make_economy_flag_missing(models: dict[str, SimpleNamespace]) -> None: del models["us"].parameters[0].economy +def _make_economy_flag_null(models: dict[str, SimpleNamespace]) -> None: + models["us"].parameters[0].economy = None + + def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: models["us"].parameters[0].household = "false" @@ -268,9 +290,10 @@ def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: [ ( _remove_entities, - "^us public model publishes no entities; catalog format 2 needs " - "PolicyEngine.py's public entity records$", + "^the installed PolicyEngine.py does not publish catalog format 2's " + "fields for us: model version entities$", ), + (_make_entities_null, "^us public model entities must be a sequence$"), (_make_entity_flag_text, "^us entity person is_person must be a boolean$"), ( _remove_person_entity, @@ -279,8 +302,22 @@ def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: (_repeat_entity_key, "^us entity key 'household' is repeated$"), ( _remove_basic_inputs, - "^us public model publishes no basic inputs; catalog format 2 needs " - "PolicyEngine.py's public basic inputs$", + "^the installed PolicyEngine.py does not publish catalog format 2's " + "fields for us: model version basic_inputs$", + ), + ( + _make_basic_inputs_text, + "^us public model basic_inputs must be a sequence$", + ), + ( + _remove_variable_enum_values, + "^the installed PolicyEngine.py does not publish catalog format 2's " + "fields for us: variable enum_values$", + ), + ( + _make_enum_label_nan, + "^variable employment_income enum label for SINGLE is not " + "JSON-compatible: NaN is not JSON-compatible$", ), ( _name_unknown_basic_input, @@ -318,6 +355,11 @@ def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: ), ( _make_economy_flag_missing, + "^the installed PolicyEngine.py does not publish catalog format 2's " + "fields for us: parameter economy$", + ), + ( + _make_economy_flag_null, "^parameter gov.example.rate economy must be a boolean$", ), ( @@ -335,3 +377,41 @@ def test_inconsistent_format_2_metadata_fails_with_a_specific_message( with pytest.raises(CatalogExtractionError, match=message): _extract(models) + + +def test_a_release_without_format_2_fields_fails_naming_each_missing_field() -> None: + """An older PolicyEngine.py fails before any record is checked, naming what lacks. + + PolicyEngine.py 6.7.5, for example, publishes none of these fields. + """ + models = source_models() + model = models["us"] + del model.entities, model.basic_inputs + for variable in model.variables: + for field_name in ( + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", + ): + delattr(variable, field_name) + for parameter in model.parameters: + del parameter.economy, parameter.household + + with pytest.raises( + CatalogExtractionError, + match=r"^the installed PolicyEngine.py does not publish catalog format 2's " + r"fields for us: model version entities, basic_inputs; variable " + r"is_input_variable, hidden_input, module_name, adds_parameter_path, " + r"subtracts_parameter_path, enum_values; parameter economy, household$", + ): + _extract(models) + + +def test_an_empty_basic_input_list_is_stored_empty() -> None: + models = source_models() + models["us"].basic_inputs = [] + + assert _extract(models).country("us").model_version.basic_inputs == () From a94b2e2d4b6a25910b92c5b7f88cb3e11ed0ea97 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:35:19 +0400 Subject: [PATCH 05/14] Compare enum label types in the metadata parity test The parity test compared enum labels with ==, which treats 1, 1.0 and True as equal, so a label stored with the wrong type passed. It now compares each label's type name as well. Co-Authored-By: Claude Opus 5.5 --- .../test_v2_catalog_metadata_parity.py | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_v2_catalog_metadata_parity.py b/tests/integration/test_v2_catalog_metadata_parity.py index 9b9bd7790..f988d62bc 100644 --- a/tests/integration/test_v2_catalog_metadata_parity.py +++ b/tests/integration/test_v2_catalog_metadata_parity.py @@ -18,6 +18,8 @@ from policyengine_api.data.v2.catalog.extraction import normalize_model_metadata if TYPE_CHECKING: + from collections.abc import Iterable, Mapping + from policyengine_api.data.v2.catalog.extraction import ModelMetadata pytestmark = pytest.mark.skipif( @@ -49,6 +51,20 @@ def _report(mismatches: list[str]) -> str: return f"{len(mismatches)} mismatches, first {min(len(mismatches), 20)}:\n{shown}" +def _typed_enum_values( + values: Iterable[Mapping[str, Any]], +) -> list[tuple[str, Any, str]]: + """Return each enum value with its label and the label's type name. + + Comparing the type name as well keeps 1, 1.0 and True apart, which ``==`` + treats as equal. + """ + return [ + (value["value"], value["label"], type(value["label"]).__name__) + for value in values + ] + + def test_entities_and_basic_inputs_match_v1( country_pair: tuple[ModelMetadata, Any], ) -> None: @@ -75,13 +91,18 @@ def test_variable_flags_modules_enum_labels_and_parts_match_v1( for variable in metadata.variables: v1_variable = v1_variables[variable.name] enum_values = ( - [ + _typed_enum_values( {"value": item.value, "label": item.label} for item in variable.enum_values - ] + ) if variable.enum_values is not None else None ) + v1_enum_values = ( + _typed_enum_values(v1_variable["possibleValues"]) + if "possibleValues" in v1_variable + else None + ) comparisons = { "is_input_variable": ( variable.is_input_variable, @@ -89,7 +110,7 @@ def test_variable_flags_modules_enum_labels_and_parts_match_v1( ), "hidden_input": (variable.hidden_input, v1_variable["hidden_input"]), "module_name": (variable.module_name, v1_variable["moduleName"]), - "enum_values": (enum_values, v1_variable.get("possibleValues")), + "enum_values": (enum_values, v1_enum_values), "adds_parameter_path": ( variable.adds_parameter_path, v1_variable["adds"] if isinstance(v1_variable["adds"], str) else None, From cca58e31ad34567a10efdae297eddbbd80ac0ff7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:36:29 +0400 Subject: [PATCH 06/14] Test that enum labels and parameter paths round-trip through publication The publication tests stored only null enum values and parameter paths, so the JSON inserts and the comparison on republication never ran on stored values. A fixture now adds an enum variable with a string and an integer label, and a variable whose adds and subtracts are named by parameters. The tests check that they are stored exactly, the integer label as an integer, that republishing the same catalog changes nothing, and that a changed enum label or parameter path fails without changing the stored rows. Co-Authored-By: Claude Opus 5.5 --- tests/fixtures/v2_catalog.py | 69 +++++++++++ .../test_v2_catalog_publication.py | 111 +++++++++++++++++- 2 files changed, 179 insertions(+), 1 deletion(-) diff --git a/tests/fixtures/v2_catalog.py b/tests/fixtures/v2_catalog.py index 73f293237..6c0a47027 100644 --- a/tests/fixtures/v2_catalog.py +++ b/tests/fixtures/v2_catalog.py @@ -319,3 +319,72 @@ def normalized_catalog( installed_version=installed_version, region_references=region_references(), ) + + +def add_enum_and_parameter_part_records(model: SimpleNamespace) -> None: + """Add an enum variable and a variable whose parts are named by parameters. + + The enum's labels are a string and an integer. The two parameters hold the + lists ``countable_income`` adds and subtracts. + """ + for name in ("gov.example.countable_income", "gov.example.deductions"): + parameter = SimpleNamespace( + name=name, + label=None, + description=None, + data_type=list, + unit=None, + economy=True, + household=True, + parameter_values=[ + SimpleNamespace( + value=["employment_income"], + start_date=datetime(2025, 1, 1, tzinfo=UTC), + end_date=None, + ) + ], + ) + model.parameters.append(parameter) + model.parameters_by_name[name] = parameter + variables = ( + SimpleNamespace( + name="filing_status", + label="Filing status", + entity="person", + description=None, + data_type="Enum", + possible_values=["SINGLE", "JOINT"], + default_value="SINGLE", + adds=None, + subtracts=None, + is_input_variable=True, + hidden_input=False, + module_name="household.demographic.filing_status", + adds_parameter_path=None, + subtracts_parameter_path=None, + enum_values=[ + SimpleNamespace(value="SINGLE", label="Single"), + SimpleNamespace(value="JOINT", label=2), + ], + ), + SimpleNamespace( + name="countable_income", + label="Countable income", + entity="person", + description=None, + data_type=float, + possible_values=None, + default_value=0.0, + adds=["employment_income"], + subtracts=["employment_income"], + is_input_variable=False, + hidden_input=False, + module_name="household.income.countable_income", + adds_parameter_path="gov.example.countable_income", + subtracts_parameter_path="gov.example.deductions", + enum_values=None, + ), + ) + for variable in variables: + model.variables.append(variable) + model.variables_by_name[variable.name] = variable diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 0e4033379..18044b240 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -16,11 +16,13 @@ from sqlmodel import Session, select from policyengine_api.data.v2.catalog import publication as publication_module +from policyengine_api.data.v2.catalog.extraction import extract_catalog from policyengine_api.data.v2.catalog.publication import ( CatalogPublicationError, publish_catalog, unpublished_countries, ) +from policyengine_api.data.v2.catalog.records import EnumValueRecord from policyengine_api.data.v2.models import ( Dataset, DatasetVersion, @@ -32,7 +34,15 @@ TaxBenefitModelVersion, ) from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL -from tests.fixtures.v2_catalog import normalized_catalog +from tests.fixtures.v2_catalog import ( + POLICYENGINE_VERSION, + add_enum_and_parameter_part_records, + bundle, + installed_version, + normalized_catalog, + region_references, + source_models, +) REPO = Path(__file__).parents[2] DISPOSABLE_DATABASE = "policyengine_v2_alembic_test" @@ -678,3 +688,102 @@ def test_version_stored_with_format_2_fails_when_a_format_2_field_changes( assert _counts(publication_engine) == before_counts assert _format_2_values(publication_engine)["parameter"] == (True, False) + + +def _catalog_with_enum_and_parameter_parts(): + models = source_models() + add_enum_and_parameter_part_records(models["us"]) + return extract_catalog( + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + region_references=region_references(), + ) + + +def _stored_us_variables(engine: Engine) -> dict[str, tuple[object, ...]]: + with engine.connect() as connection: + rows = connection.execute( + text( + "SELECT variable.name, variable.enum_values, " + "variable.adds_parameter_path, variable.subtracts_parameter_path " + "FROM variables AS variable " + "JOIN tax_benefit_model_versions AS version " + "ON version.id = variable.tax_benefit_model_version_id " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "WHERE model.name = 'policyengine-us'" + ) + ).all() + return {row[0]: tuple(row[1:]) for row in rows} + + +def _with_changed_us_variable(catalog, name: str, **changes: object): + us = catalog.country("us") + variables = tuple( + replace(variable, **changes) if variable.name == name else variable + for variable in us.variables + ) + return replace( + catalog, + countries=(replace(us, variables=variables), catalog.country("uk")), + ) + + +def test_enum_labels_and_parameter_paths_round_trip_exactly( + publication_engine: Engine, +) -> None: + catalog = _catalog_with_enum_and_parameter_parts() + + publish_catalog(publication_engine, catalog) + before_ids = _identifiers(publication_engine) + stored = _stored_us_variables(publication_engine) + republished = publish_catalog(publication_engine, catalog) + + assert stored["filing_status"] == ( + [{"value": "SINGLE", "label": "Single"}, {"value": "JOINT", "label": 2}], + None, + None, + ) + assert type(stored["filing_status"][0][1]["label"]) is int + assert stored["countable_income"] == ( + None, + "gov.example.countable_income", + "gov.example.deductions", + ) + assert republished.as_dict()["outcome"] == "ok" + assert _identifiers(publication_engine) == before_ids + + +@pytest.mark.parametrize( + ("variable_name", "changes"), + [ + ( + "filing_status", + { + "enum_values": ( + EnumValueRecord(value="SINGLE", label="Single filer"), + EnumValueRecord(value="JOINT", label=2), + ) + }, + ), + ("countable_income", {"adds_parameter_path": "gov.example.deductions"}), + ("countable_income", {"subtracts_parameter_path": None}), + ], +) +def test_republication_detects_a_changed_enum_label_or_parameter_path( + publication_engine: Engine, + variable_name: str, + changes: dict[str, object], +) -> None: + catalog = _catalog_with_enum_and_parameter_parts() + publish_catalog(publication_engine, catalog) + stored = _stored_us_variables(publication_engine) + + with pytest.raises(CatalogPublicationError, match=r"in its variables$"): + publish_catalog( + publication_engine, + _with_changed_us_variable(catalog, variable_name, **changes), + ) + + assert _stored_us_variables(publication_engine) == stored From e46611fca4221d9fee6ec45172a78cd88244411e Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:52:01 +0400 Subject: [PATCH 07/14] Type the catalog's entity and enum-value columns TaxBenefitModelVersion.entities and Variable.enum_values were typed as lists of untyped dictionaries. They are now lists of the TypedDicts CatalogEntity (key, plural, is_person) and CatalogEnumValue (value and declared label), which match EntityRecord and EnumValueRecord, and publication builds the stored JSON through them, so readers of these columns get the keys and value types without checking the shape again. The column types are unchanged, so there is no schema change. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/publication_staging.py | 16 ++++++---- policyengine_api/data/v2/models/__init__.py | 4 +++ policyengine_api/data/v2/models/metadata.py | 31 ++++++++++++++++--- tests/unit/v2/test_models.py | 27 ++++++++++++++++ 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/policyengine_api/data/v2/catalog/publication_staging.py b/policyengine_api/data/v2/catalog/publication_staging.py index e0dfa3c31..3d341e488 100644 --- a/policyengine_api/data/v2/catalog/publication_staging.py +++ b/policyengine_api/data/v2/catalog/publication_staging.py @@ -19,6 +19,7 @@ VariableRecord, iter_batches, ) +from policyengine_api.data.v2.models import CatalogEntity, CatalogEnumValue if TYPE_CHECKING: from collections.abc import Callable, Iterable, Iterator, Sequence @@ -178,11 +179,11 @@ def _model_rows(country: CountryCatalog) -> Iterator[tuple[object, ...]]: country.model_version.catalog_format, Jsonb( [ - { - "key": entity.key, - "plural": entity.plural, - "is_person": entity.is_person, - } + CatalogEntity( + key=entity.key, + plural=entity.plural, + is_person=entity.is_person, + ) for entity in country.model_version.entities ] ), @@ -195,7 +196,10 @@ def _enum_values_json(record: VariableRecord) -> Jsonb | None: if record.enum_values is None: return None return Jsonb( - [{"value": item.value, "label": item.label} for item in record.enum_values] + [ + CatalogEnumValue(value=item.value, label=item.label) + for item in record.enum_values + ] ) diff --git a/policyengine_api/data/v2/models/__init__.py b/policyengine_api/data/v2/models/__init__.py index c63655bb6..1b1a73baf 100644 --- a/policyengine_api/data/v2/models/__init__.py +++ b/policyengine_api/data/v2/models/__init__.py @@ -40,6 +40,8 @@ HouseholdJobStatus, ) from policyengine_api.data.v2.models.metadata import ( # noqa: E402 + CatalogEntity, + CatalogEnumValue, Dataset, DatasetVersion, Parameter, @@ -99,6 +101,8 @@ "AggregateOutput", "AggregateType", "BudgetSummary", + "CatalogEntity", + "CatalogEnumValue", "ChangeAggregate", "CongressionalDistrictImpact", "ConstituencyImpact", diff --git a/policyengine_api/data/v2/models/metadata.py b/policyengine_api/data/v2/models/metadata.py index ca0153467..62a92b089 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -7,6 +7,7 @@ import sqlalchemy as sa from sqlmodel import Field, Relationship +from typing_extensions import TypedDict from policyengine_api.data.v2.models.base import ( IdentifiedModel, @@ -36,6 +37,28 @@ class RegionType(StrEnum): PLACE = "place" +class CatalogEntity(TypedDict): + """One entity of a catalog version's ``entities`` column. + + ``is_person`` marks the person entity; each version has exactly one. + """ + + key: str + plural: str + is_person: bool + + +class CatalogEnumValue(TypedDict): + """One value of a variable's ``enum_values`` column. + + ``value`` is the stored name and ``label`` the label the country package + declares, kept as declared (usually a string). + """ + + value: str + label: str | int | float | bool | None + + class TaxBenefitModel(TimestampedModel, table=True): """A country's tax-benefit model, unique by name. @@ -86,9 +109,9 @@ class TaxBenefitModelVersion(IdentifiedModel, table=True): current_law_id: int metadata_time_periods: list[int] = Field(sa_type=sa.JSON) # Catalog format 2 fields. Versions published before format 2 keep them - # null; entities are {key, plural, is_person} objects. + # null. catalog_format: int | None = None - entities: list[dict[str, Any]] | None = Field(default=None, sa_type=sa.JSON) + entities: list[CatalogEntity] | None = Field(default=None, sa_type=sa.JSON) basic_inputs: list[str] | None = Field(default=None, sa_type=sa.JSON) model: TaxBenefitModel = Relationship(back_populates="versions") @@ -284,13 +307,13 @@ class Variable(IdentifiedModel, table=True): subtracts: list[str] | None = Field(default=None, sa_type=sa.JSON) # Catalog format 2 fields; null in versions published before it. A # parameter path names the parameter whose dated value lists the variables - # this one adds or subtracts. Enum values are {value, label} objects. + # this one adds or subtracts. is_input_variable: bool | None = None hidden_input: bool | None = None module_name: str | None = Field(default=None, max_length=512) adds_parameter_path: str | None = Field(default=None, max_length=512) subtracts_parameter_path: str | None = Field(default=None, max_length=512) - enum_values: list[dict[str, Any]] | None = Field(default=None, sa_type=sa.JSON) + enum_values: list[CatalogEnumValue] | None = Field(default=None, sa_type=sa.JSON) tax_benefit_model_version_id: UUID = Field( foreign_key="tax_benefit_model_versions.id", ondelete="CASCADE", diff --git a/tests/unit/v2/test_models.py b/tests/unit/v2/test_models.py index cdca61496..8cff45e68 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -721,3 +721,30 @@ def test_catalog_format_2_columns_are_nullable_for_earlier_versions() -> None: "subtracts_parameter_path", ): assert variables.c[column_name].type.length == 512 + + +def test_catalog_format_2_json_columns_have_typed_shapes() -> None: + """The entity and enum-value columns name typed shapes the catalog records match. + + Readers of these columns get the keys and value types without re-checking. + """ + from typing import get_type_hints + + from policyengine_api.data.v2.catalog.records import EntityRecord, EnumValueRecord + from policyengine_api.data.v2.models import ( + CatalogEntity, + CatalogEnumValue, + Variable, + ) + + assert TaxBenefitModelVersion.model_fields["entities"].annotation == ( + list[CatalogEntity] | None + ) + assert Variable.model_fields["enum_values"].annotation == ( + list[CatalogEnumValue] | None + ) + for typed_shape, record in ( + (CatalogEntity, EntityRecord), + (CatalogEnumValue, EnumValueRecord), + ): + assert get_type_hints(typed_shape) == get_type_hints(record) From 494f43465d8b513924d3bf62fa4b479fa9505b47 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:50:42 +0400 Subject: [PATCH 08/14] Take each country's default dataset from the PolicyEngine.py release The catalog fixed its own default datasets (populace_us_2024 and microcosm_uk_2024_25) and refused a release that named another. Each country's default dataset now comes from the installed PolicyEngine.py release's manifest. Publication still stops if the manifest names no default, if the release does not certify it, or if the release routes national regions to another dataset; those messages now name the datasets instead of a reviewed selection. Region scoping's argument is renamed to default_dataset, and the Stage 9 document and the changelog fragment describe the release's default. Co-Authored-By: Claude Opus 5.5 --- ...an-1f-115-catalog-metadata-fields.added.md | 2 +- docs/migration/stage-9-v2-metadata.md | 18 +++--- .../data/v2/catalog/extraction.py | 24 ++++---- .../data/v2/catalog/region_scoping.py | 25 ++++---- tests/unit/v2/test_catalog_extraction.py | 60 ++++++++++++++++++- .../v2/test_catalog_extraction_failures.py | 7 ++- 6 files changed, 100 insertions(+), 36 deletions(-) diff --git a/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md index 7394dc074..fe578f85e 100644 --- a/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md +++ b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md @@ -1 +1 @@ -Catalog format 2 stores the metadata the app's v3 workspace reads, taken only from PolicyEngine.py's public records: each model version's entities and basic inputs; each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, and its enum values' labels; each parameter's economy and household flags. Bracket schedules are stored as parameter nodes with their stored label. A generated revision adds the columns as nullable, so catalog versions published earlier keep them empty and are compared without them when republished. An opt-in parity test, run by the v2 integration check, compares every new field for the US and the UK with /{country}/metadata. +Catalog format 2 stores the metadata the app's v3 workspace reads, taken only from PolicyEngine.py's public records: each model version's entities and basic inputs; each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, and its enum values' labels; each parameter's economy and household flags. Bracket schedules are stored as parameter nodes with their stored label. A generated revision adds the columns as nullable, so catalog versions published earlier keep them empty and are compared without them when republished. An opt-in parity test, run by the v2 integration check, compares every new field for the US and the UK with /{country}/metadata. Each country's default dataset now comes from the installed PolicyEngine.py release's manifest; the Public API no longer fixes its own (populace_us_2024 and microcosm_uk_2024_25), and publication stops if the release names no default or does not certify it. diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 0d716beee..db5e02d61 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -139,15 +139,19 @@ Regions come from the Public API's checked-in reference files, one per country (`policyengine_api/data/v2/catalog/region_reference_files/`), not from PolicyEngine.py's region registry. Each row lists a region's code, label, type, parent, US state, the value that selects its households, and the v1 identifier -it replaces. A scoping rule per country and region type gives each region its -dataset from the certified data release and its filter: +it replaces. Each country's default dataset is the one the installed +PolicyEngine.py release names in its manifest (`default_dataset`); the Public +API sets none of its own. Publication stops if the manifest names no default, +if the release does not certify it, or if the release routes national regions +to another dataset. A scoping rule per country and region type gives each +region its dataset from the certified data release and its filter: | Regions | Dataset | Filter | | --- | --- | --- | -| US national | the release's national dataset (`populace_us_2024`) | none | +| US national | the release's default dataset (`populace_us_2024` in 6.10.0) | none | | US states and congressional districts | the release's state and district dataset | `row_filter` on `state_fips` or `congressional_district_geoid` | -| UK national | the release's national dataset (`microcosm_uk_2024_25`) | none | -| UK nations | the release's national dataset | `row_filter` on `region` | +| UK national | the release's default dataset (`microcosm_uk_2024_25` in 6.10.0) | none | +| UK nations | the release's default dataset | `row_filter` on `region` | | UK constituencies and local authorities | the release's local-area dataset (`microcosm_uk_2024_25_dense`), which carries their codes | `row_filter` on `constituency_code_oa` or `la_code_oa`, value the ONS code | If the release names no dataset for a US state or district, those regions use @@ -165,8 +169,8 @@ household columns. It does not check that a column holds rows for every region's value. Review the fallback and omitted-region summaries on every new PolicyEngine.py -release. A changed count may identify an upstream catalog change that needs a -reviewed dataset selection even when publication otherwise succeeds. +release. A changed count may identify an upstream change to the certified data +release even when publication otherwise succeeds. A catalog version is identified by its country and PolicyEngine.py version and never changes once published. When the command republishes a stored version diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 34ffcc803..51c7bd5b2 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -49,10 +49,6 @@ "policyengine-us", "policyengine-uk", ) -REVIEWED_DEFAULT_DATASETS = { - "us": "populace_us_2024", - "uk": "microcosm_uk_2024_25", -} CURRENT_LAW_IDS = {"us": 2, "uk": 1} METADATA_TIME_PERIODS = { "us": tuple(range(2035, 2021, -1)), @@ -813,14 +809,17 @@ def normalize_model_metadata( ) -def _verified_default_dataset(country_id: str, release_manifest: object) -> str: - """Return the reviewed default dataset once the manifest names it as its default.""" - reviewed_default = REVIEWED_DEFAULT_DATASETS[country_id] - manifest_default = getattr(release_manifest, "default_dataset", None) - if manifest_default != reviewed_default: - msg = f"{country_id} default dataset does not match the reviewed selection" +def _release_default_dataset(country_id: str, release_manifest: object) -> str: + """Return the default dataset the installed PolicyEngine.py release names. + + The Public API sets no default of its own; region scoping then requires the + release to certify it. + """ + default_dataset = getattr(release_manifest, "default_dataset", None) + if not isinstance(default_dataset, str) or not default_dataset: + msg = f"{country_id} release manifest names no default dataset" raise CatalogExtractionError(msg) - return reviewed_default + return default_dataset def _normalize_region( @@ -872,12 +871,11 @@ def _normalize_country( country_id, source, model_version_id=model_version_id ) - reviewed_default = _verified_default_dataset(country_id, release_manifest) scoping = scope_regions( country_id, region_references, release_manifest, - reviewed_default=reviewed_default, + default_dataset=_release_default_dataset(country_id, release_manifest), ) datasets = tuple( diff --git a/policyengine_api/data/v2/catalog/region_scoping.py b/policyengine_api/data/v2/catalog/region_scoping.py index 3432dcb4c..aa11d8cc7 100644 --- a/policyengine_api/data/v2/catalog/region_scoping.py +++ b/policyengine_api/data/v2/catalog/region_scoping.py @@ -237,13 +237,14 @@ def scope_regions( references: tuple[RegionReference, ...], release_manifest: object, *, - reviewed_default: str, + default_dataset: str, ) -> RegionScoping: """Give every runnable reference region its certified dataset and filter. - The national region must use the reviewed default dataset. A region whose - type has no scoping rule is left out, and a runnable region may not sit - under a region that is left out. + ``default_dataset`` is the release's own default. The release must certify + it, and the national region must use it. A region whose type has no + scoping rule is left out, and a runnable region may not sit under a region + that is left out. """ if not isinstance(release_manifest, ReleaseManifest): msg = ( @@ -252,12 +253,16 @@ def scope_regions( ) raise CatalogExtractionError(msg) artifacts = certified_artifacts(country_id, release_manifest) - if reviewed_default not in artifacts: - msg = f"{country_id} release does not certify {reviewed_default}" + if default_dataset not in artifacts: + msg = f"{country_id} release does not certify {default_dataset}" raise CatalogExtractionError(msg) role_datasets = _role_datasets(country_id, release_manifest, artifacts) - if role_datasets.get(NATIONAL_DATASET_ROLE, reviewed_default) != reviewed_default: - msg = f"{country_id} national region does not use the reviewed dataset" + national_dataset = role_datasets.get(NATIONAL_DATASET_ROLE, default_dataset) + if national_dataset != default_dataset: + msg = ( + f"{country_id} release routes national regions to {national_dataset}, " + f"not to its default dataset {default_dataset}" + ) raise CatalogExtractionError(msg) omitted = { @@ -287,7 +292,7 @@ def scope_regions( "cannot select its households" ) raise CatalogExtractionError(msg) - dataset_name = reviewed_default + dataset_name = default_dataset if rule.dataset_role != NATIONAL_DATASET_ROLE: fallback_counts[reference.region_type] += 1 regions.append( @@ -301,7 +306,7 @@ def scope_regions( return RegionScoping( regions=tuple(regions), dataset_names=frozenset( - {reviewed_default, *(region.dataset_name for region in regions)} + {default_dataset, *(region.dataset_name for region in regions)} ), certified_artifacts=artifacts, fallback_counts=dict(sorted(fallback_counts.items())), diff --git a/tests/unit/v2/test_catalog_extraction.py b/tests/unit/v2/test_catalog_extraction.py index bc6d45eca..66ace2ced 100644 --- a/tests/unit/v2/test_catalog_extraction.py +++ b/tests/unit/v2/test_catalog_extraction.py @@ -367,9 +367,65 @@ def test_rejects_incomplete_public_model_catalogs() -> None: installed_version=installed_version, ) + +def _use_another_certified_default(model: SimpleNamespace, dataset_name: str) -> None: + """Make the release default to another dataset it certifies, for every region.""" + manifest = model.release_manifest + manifest.default_dataset = dataset_name + manifest.datasets[dataset_name] = SimpleNamespace( + path=f"{dataset_name}.h5", + revision="fixture", + sha256="0" * 64, + repo_id=None, + repo_type=None, + ) + for template in manifest.region_datasets.values(): + template.path_template = f"{dataset_name}.h5" + + +def test_each_country_uses_the_default_dataset_its_release_names() -> None: + """The release's own default is used; the Public API fixes none of its own.""" + models = source_models() + _use_another_certified_default(models["us"], "populace_us_2025") + _use_another_certified_default(models["uk"], "microcosm_uk_2025_26") + + catalog = extract_catalog( + region_references=region_references(), + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + ) + + for country_id, default_dataset in ( + ("us", "populace_us_2025"), + ("uk", "microcosm_uk_2025_26"), + ): + country = catalog.country(country_id) + dataset_names = {dataset.id: dataset.name for dataset in country.datasets} + national = next( + region for region in country.regions if region.region_type == "national" + ) + assert dataset_names[national.default_dataset_id] == default_dataset + assert set(dataset_names.values()) == {default_dataset} + + +@pytest.mark.parametrize( + ("default_dataset", "message"), + [ + (None, "^us release manifest names no default dataset$"), + ("", "^us release manifest names no default dataset$"), + ("populace_us_2025", "^us release does not certify populace_us_2025$"), + ], +) +def test_a_missing_or_uncertified_release_default_fails( + default_dataset: str | None, + message: str, +) -> None: models = source_models() - models["us"].release_manifest.default_dataset = "unreviewed_us_2025" - with pytest.raises(CatalogExtractionError, match="reviewed selection"): + models["us"].release_manifest.default_dataset = default_dataset + + with pytest.raises(CatalogExtractionError, match=message): extract_catalog( region_references=region_references(), bundle=bundle(), diff --git a/tests/unit/v2/test_catalog_extraction_failures.py b/tests/unit/v2/test_catalog_extraction_failures.py index 70f5f954c..b09bff9fd 100644 --- a/tests/unit/v2/test_catalog_extraction_failures.py +++ b/tests/unit/v2/test_catalog_extraction_failures.py @@ -160,7 +160,7 @@ def _remove_the_dataset_map( models["us"].release_manifest.datasets = None -def _remove_the_reviewed_default_dataset( +def _remove_the_release_default_dataset( models: dict[str, SimpleNamespace], _references: References ) -> None: del models["us"].release_manifest.datasets["populace_us_2024"] @@ -214,7 +214,7 @@ def _remove_every_artifact_repository( "^us release manifest lacks datasets, region_datasets or data_package$", ), ( - _remove_the_reviewed_default_dataset, + _remove_the_release_default_dataset, "^us release does not certify populace_us_2024$", ), ( @@ -241,7 +241,8 @@ def _remove_every_artifact_repository( ), ( _point_national_region_at_another_dataset, - "^uk national region does not use the reviewed dataset$", + "^uk release routes national regions to frs_2024_25, not to its default " + "dataset microcosm_uk_2024_25$", ), ( _name_an_uncertified_region_dataset, From 1f69e91440248f3d5c317ccd02cf39f8b18de5b3 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:42:26 +0400 Subject: [PATCH 09/14] Say that the locked PolicyEngine.py 6.10.0 also lacks catalog format 2's fields The Stage 9 document, the format 2 field check's docstring and its test named only 6.7.5 as a release without the fields; the locked 6.10.0 lacks them too. The Stage 9 regions table also names the UK local-area dataset as the one 6.10.0 uses, like the rows above it. Documentation only. Co-Authored-By: Claude Opus 5.5 --- docs/migration/stage-9-v2-metadata.md | 6 +++--- policyengine_api/data/v2/catalog/extraction.py | 3 ++- tests/unit/v2/test_catalog_metadata_fields.py | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index db5e02d61..b8f99b3e4 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -112,8 +112,8 @@ publishes entities, basic inputs and these flags: Bracket schedules are parameter nodes with their stored label, or none when the package stores none. An empty basic-input list is stored as empty. Extraction stops before reading any record if the installed PolicyEngine.py -does not publish these fields, as 6.7.5 does not, and names each missing -field. It also stops if the model has other than one person entity, names a +does not publish these fields, as neither 6.7.5 nor the locked 6.10.0 does, +and names each missing field. It also stops if the model has other than one person entity, names a basic input or a parameter path that is not published, gives enum labels that do not match the variable's possible values or are not JSON scalars, or gives a flag that is not a boolean. An opt-in test @@ -152,7 +152,7 @@ region its dataset from the certified data release and its filter: | US states and congressional districts | the release's state and district dataset | `row_filter` on `state_fips` or `congressional_district_geoid` | | UK national | the release's default dataset (`microcosm_uk_2024_25` in 6.10.0) | none | | UK nations | the release's default dataset | `row_filter` on `region` | -| UK constituencies and local authorities | the release's local-area dataset (`microcosm_uk_2024_25_dense`), which carries their codes | `row_filter` on `constituency_code_oa` or `la_code_oa`, value the ONS code | +| UK constituencies and local authorities | the release's local-area dataset (`microcosm_uk_2024_25_dense` in 6.10.0), which carries their codes | `row_filter` on `constituency_code_oa` or `la_code_oa`, value the ONS code | If the release names no dataset for a US state or district, those regions use the default dataset and the command emits one summary warning with the affected diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 51c7bd5b2..1faad4e4d 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -644,7 +644,8 @@ def _normalize_parameter_nodes( def _require_format_2_fields(country_id: str, source: object) -> None: """Fail first, naming each field, when PolicyEngine.py does not publish them. - Older releases, such as 6.7.5, publish none of catalog format 2's fields. + Releases without them, such as 6.7.5 and the locked 6.10.0, publish none of + catalog format 2's fields. """ missing: list[str] = [] for record_name, mapping_attribute, field_names in FORMAT_2_FIELDS: diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py index ab6f6414b..8ef59e60e 100644 --- a/tests/unit/v2/test_catalog_metadata_fields.py +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -380,9 +380,9 @@ def test_inconsistent_format_2_metadata_fails_with_a_specific_message( def test_a_release_without_format_2_fields_fails_naming_each_missing_field() -> None: - """An older PolicyEngine.py fails before any record is checked, naming what lacks. + """A release without the fields fails before any record is checked, naming each. - PolicyEngine.py 6.7.5, for example, publishes none of these fields. + PolicyEngine.py 6.7.5 and the locked 6.10.0, for example, publish none of them. """ models = source_models() model = models["us"] From 5c565e61f98e08b7809a5f75017818107a3b2e85 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:17:30 +0400 Subject: [PATCH 10/14] Add each variable's unit and definition period to the catalog format 2 columns The app's v3 household builder and breakdown need each variable's unit (currency formatting) and definition period ("per month" or "per year" labels, household validation), and PolicyEngine.py's public Variable record carries both. The v2 catalog stored neither. Revision f5c1d9b4685b is not merged yet, so it was regenerated with autogenerate against the v2 configuration, keeping its ID through --rev-id and its parent fa7fa1035444. It now adds 13 nullable columns with no defaults, including variables.unit (128 characters) and variables.definition_period (32 characters), and writes no data. Co-Authored-By: Claude Opus 5.5 --- ...c1d9b4685b_store_catalog_format_2_metadata.py | 16 +++++++++++++++- policyengine_api/data/v2/models/metadata.py | 4 ++++ tests/integration/test_alembic_v2_lifecycle.py | 4 +++- tests/unit/v2/test_alembic_v2.py | 2 ++ tests/unit/v2/test_models.py | 4 ++++ 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py b/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py index b0dc56d28..fbaefb08f 100644 --- a/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py +++ b/migrations/v2/versions/f5c1d9b4685b_store_catalog_format_2_metadata.py @@ -2,7 +2,7 @@ Revision ID: f5c1d9b4685b Revises: fa7fa1035444 -Create Date: 2026-10-22 18:59:48.186978 +Create Date: 2026-10-26 20:13:40.733924 Generation: uv run alembic -c alembic-v2.ini revision --autogenerate """ @@ -61,11 +61,25 @@ def upgrade() -> None: ), ) op.add_column("variables", sa.Column("enum_values", sa.JSON(), nullable=True)) + op.add_column( + "variables", + sa.Column("unit", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=True), + ) + op.add_column( + "variables", + sa.Column( + "definition_period", + sqlmodel.sql.sqltypes.AutoString(length=32), + nullable=True, + ), + ) # ### end Alembic commands ### def downgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("variables", "definition_period") + op.drop_column("variables", "unit") op.drop_column("variables", "enum_values") op.drop_column("variables", "subtracts_parameter_path") op.drop_column("variables", "adds_parameter_path") diff --git a/policyengine_api/data/v2/models/metadata.py b/policyengine_api/data/v2/models/metadata.py index 62a92b089..5431a6abc 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -314,6 +314,10 @@ class Variable(IdentifiedModel, table=True): adds_parameter_path: str | None = Field(default=None, max_length=512) subtracts_parameter_path: str | None = Field(default=None, max_length=512) enum_values: list[CatalogEnumValue] | None = Field(default=None, sa_type=sa.JSON) + # The unit (for example ``currency-USD``) and the period a value covers + # (``year``, ``month`` or ``eternity``), as the country package declares. + unit: str | None = Field(default=None, max_length=128) + definition_period: str | None = Field(default=None, max_length=32) tax_benefit_model_version_id: UUID = Field( foreign_key="tax_benefit_model_versions.id", ondelete="CASCADE", diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 61ff7a72b..89837594a 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1426,6 +1426,8 @@ def test_previous_release_still_finds_user_policies() -> None: "adds_parameter_path", "subtracts_parameter_path", "enum_values", + "unit", + "definition_period", ), "parameters": ("economy", "household"), } @@ -1517,7 +1519,7 @@ def test_catalog_format_2_columns_leave_earlier_versions_empty() -> None: with engine.connect() as connection: assert _format_2_values(connection, version_id) == [ (None, None, None), - (None, None, None, None, None, None), + (None, None, None, None, None, None, None, None), (None, None), ] diff --git a/tests/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index 5a3d20de6..d6eb45ae0 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -844,6 +844,8 @@ def test_catalog_format_2_revision_adds_only_nullable_columns() -> None: "adds_parameter_path", "subtracts_parameter_path", "enum_values", + "unit", + "definition_period", ), "parameters": ("economy", "household"), } diff --git a/tests/unit/v2/test_models.py b/tests/unit/v2/test_models.py index 8cff45e68..c93d214b0 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -699,6 +699,8 @@ def test_catalog_format_2_columns_are_nullable_for_earlier_versions() -> None: "adds_parameter_path": AutoString, "subtracts_parameter_path": AutoString, "enum_values": sa.JSON, + "unit": AutoString, + "definition_period": AutoString, }, "parameters": { "economy": sa.Boolean, @@ -721,6 +723,8 @@ def test_catalog_format_2_columns_are_nullable_for_earlier_versions() -> None: "subtracts_parameter_path", ): assert variables.c[column_name].type.length == 512 + assert variables.c["unit"].type.length == 128 + assert variables.c["definition_period"].type.length == 32 def test_catalog_format_2_json_columns_have_typed_shapes() -> None: From 376846411efd8bd95bd696166b1a967a544523c2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:23:52 +0400 Subject: [PATCH 11/14] Extract each variable's unit and definition period from PolicyEngine.py's public records Extraction now reads each variable's unit and definition period from PolicyEngine.py's public Variable record (both optional strings, at most 128 and 32 characters), and the format 2 field check names them when a release does not publish them. 6.7.5 and the locked 6.10.0 already carry these two, so on 6.10.0 the check still names only the other fields. The parity test compares both with the unit and definitionPeriod that /{country}/metadata builds for every US and UK variable. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/extraction.py | 16 +++++++- policyengine_api/data/v2/catalog/records.py | 2 + tests/fixtures/v2_catalog.py | 6 +++ .../test_v2_catalog_metadata_parity.py | 5 +++ tests/unit/v2/test_catalog_metadata_fields.py | 37 ++++++++++++++++++- 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 1faad4e4d..84606329f 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -360,6 +360,16 @@ def optional_names(attribute: str) -> list[str] | None: maximum=512, ), enum_values=enum_values, + unit=_optional_text( + getattr(source, "unit", None), + field_name=f"variable {name} unit", + maximum=128, + ), + definition_period=_optional_text( + getattr(source, "definition_period", None), + field_name=f"variable {name} definition_period", + maximum=32, + ), ) @@ -635,6 +645,8 @@ def _normalize_parameter_nodes( "adds_parameter_path", "subtracts_parameter_path", "enum_values", + "unit", + "definition_period", ), ), ("parameter", "parameters_by_name", ("economy", "household")), @@ -644,8 +656,8 @@ def _normalize_parameter_nodes( def _require_format_2_fields(country_id: str, source: object) -> None: """Fail first, naming each field, when PolicyEngine.py does not publish them. - Releases without them, such as 6.7.5 and the locked 6.10.0, publish none of - catalog format 2's fields. + Older releases, such as 6.7.5 and the locked 6.10.0, publish only the + variables' unit and definition period of catalog format 2's fields. """ missing: list[str] = [] for record_name, mapping_attribute, field_names in FORMAT_2_FIELDS: diff --git a/policyengine_api/data/v2/catalog/records.py b/policyengine_api/data/v2/catalog/records.py index 56eaafc95..622a35cb6 100644 --- a/policyengine_api/data/v2/catalog/records.py +++ b/policyengine_api/data/v2/catalog/records.py @@ -97,6 +97,8 @@ class VariableRecord: adds_parameter_path: str | None subtracts_parameter_path: str | None enum_values: tuple[EnumValueRecord, ...] | None + unit: str | None + definition_period: str | None @dataclass(frozen=True, slots=True) diff --git a/tests/fixtures/v2_catalog.py b/tests/fixtures/v2_catalog.py index 6c0a47027..d24c6ff44 100644 --- a/tests/fixtures/v2_catalog.py +++ b/tests/fixtures/v2_catalog.py @@ -227,6 +227,8 @@ def _model_source( adds_parameter_path=None, subtracts_parameter_path=None, enum_values=None, + unit="currency-USD", + definition_period="year", ) parameter_node = SimpleNamespace( name="gov.example", @@ -366,6 +368,8 @@ def add_enum_and_parameter_part_records(model: SimpleNamespace) -> None: SimpleNamespace(value="SINGLE", label="Single"), SimpleNamespace(value="JOINT", label=2), ], + unit=None, + definition_period="year", ), SimpleNamespace( name="countable_income", @@ -383,6 +387,8 @@ def add_enum_and_parameter_part_records(model: SimpleNamespace) -> None: adds_parameter_path="gov.example.countable_income", subtracts_parameter_path="gov.example.deductions", enum_values=None, + unit="currency-USD", + definition_period="month", ), ) for variable in variables: diff --git a/tests/integration/test_v2_catalog_metadata_parity.py b/tests/integration/test_v2_catalog_metadata_parity.py index f988d62bc..3a426ad0f 100644 --- a/tests/integration/test_v2_catalog_metadata_parity.py +++ b/tests/integration/test_v2_catalog_metadata_parity.py @@ -111,6 +111,11 @@ def test_variable_flags_modules_enum_labels_and_parts_match_v1( "hidden_input": (variable.hidden_input, v1_variable["hidden_input"]), "module_name": (variable.module_name, v1_variable["moduleName"]), "enum_values": (enum_values, v1_enum_values), + "unit": (variable.unit, v1_variable["unit"]), + "definition_period": ( + variable.definition_period, + v1_variable["definitionPeriod"], + ), "adds_parameter_path": ( variable.adds_parameter_path, v1_variable["adds"] if isinstance(v1_variable["adds"], str) else None, diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py index 8ef59e60e..48c85587f 100644 --- a/tests/unit/v2/test_catalog_metadata_fields.py +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -57,6 +57,8 @@ def _add_variable(model: SimpleNamespace, **fields: object) -> None: adds_parameter_path=None, subtracts_parameter_path=None, enum_values=None, + unit=None, + definition_period="year", ) for name, value in fields.items(): setattr(variable, name, value) @@ -114,6 +116,8 @@ def test_variables_carry_input_flags_module_and_unset_parts() -> None: assert variable.adds_parameter_path is None assert variable.subtracts_parameter_path is None assert variable.enum_values is None + assert variable.unit == "currency-USD" + assert variable.definition_period == "year" def test_enum_values_keep_their_declared_labels_in_order() -> None: @@ -239,6 +243,18 @@ def _name_unknown_basic_input(models: dict[str, SimpleNamespace]) -> None: models["us"].basic_inputs = ["employment_income", "unpublished_input"] +def _remove_variable_unit(models: dict[str, SimpleNamespace]) -> None: + del models["us"].variables[0].unit + + +def _make_unit_a_number(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].unit = 1 + + +def _make_definition_period_a_list(models: dict[str, SimpleNamespace]) -> None: + models["us"].variables[0].definition_period = ["year"] + + def _make_input_flag_text(models: dict[str, SimpleNamespace]) -> None: models["us"].variables[0].is_input_variable = "true" @@ -323,6 +339,19 @@ def _make_household_flag_text(models: dict[str, SimpleNamespace]) -> None: _name_unknown_basic_input, "^us basic input 'unpublished_input' is not a published variable$", ), + ( + _remove_variable_unit, + "^the installed PolicyEngine.py does not publish catalog format 2's " + "fields for us: variable unit$", + ), + ( + _make_unit_a_number, + "^variable employment_income unit must be a string or null$", + ), + ( + _make_definition_period_a_list, + "^variable employment_income definition_period must be a string or null$", + ), ( _make_input_flag_text, "^variable employment_income is_input_variable must be a boolean or null$", @@ -382,7 +411,8 @@ def test_inconsistent_format_2_metadata_fails_with_a_specific_message( def test_a_release_without_format_2_fields_fails_naming_each_missing_field() -> None: """A release without the fields fails before any record is checked, naming each. - PolicyEngine.py 6.7.5 and the locked 6.10.0, for example, publish none of them. + PolicyEngine.py 6.7.5 and the locked 6.10.0, for example, publish only the + variables' unit and definition period of these fields. """ models = source_models() model = models["us"] @@ -395,6 +425,8 @@ def test_a_release_without_format_2_fields_fails_naming_each_missing_field() -> "adds_parameter_path", "subtracts_parameter_path", "enum_values", + "unit", + "definition_period", ): delattr(variable, field_name) for parameter in model.parameters: @@ -405,7 +437,8 @@ def test_a_release_without_format_2_fields_fails_naming_each_missing_field() -> match=r"^the installed PolicyEngine.py does not publish catalog format 2's " r"fields for us: model version entities, basic_inputs; variable " r"is_input_variable, hidden_input, module_name, adds_parameter_path, " - r"subtracts_parameter_path, enum_values; parameter economy, household$", + r"subtracts_parameter_path, enum_values, unit, definition_period; parameter " + r"economy, household$", ): _extract(models) From 7c0eac46f41179edc6b8c25447a86181fa7a3618 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:27:56 +0400 Subject: [PATCH 12/14] Store each variable's unit and definition period when a catalog version is published Publication now stages and inserts each variable's unit and definition period, and a stored format 2 version must match on both when the same PolicyEngine.py version is published again; a version stored before format 2 keeps them empty. The Stage 9 document and the changelog fragment list both fields and say that 6.7.5 and the locked 6.10.0 already publish these two. Co-Authored-By: Claude Opus 5.5 --- ...an-1f-115-catalog-metadata-fields.added.md | 2 +- docs/migration/stage-9-v2-metadata.md | 24 +++++++++---------- .../v2/catalog/publication_reconciliation.py | 8 +++++++ .../data/v2/catalog/publication_staging.py | 4 ++++ .../test_v2_catalog_publication.py | 19 +++++++++++---- tests/unit/v2/test_catalog_publication.py | 2 +- tests/unit/v2/test_catalog_staging_rows.py | 5 +++- 7 files changed, 45 insertions(+), 19 deletions(-) diff --git a/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md index fe578f85e..03f5b55f3 100644 --- a/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md +++ b/changelog.d/v2plan-1f-115-catalog-metadata-fields.added.md @@ -1 +1 @@ -Catalog format 2 stores the metadata the app's v3 workspace reads, taken only from PolicyEngine.py's public records: each model version's entities and basic inputs; each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, and its enum values' labels; each parameter's economy and household flags. Bracket schedules are stored as parameter nodes with their stored label. A generated revision adds the columns as nullable, so catalog versions published earlier keep them empty and are compared without them when republished. An opt-in parity test, run by the v2 integration check, compares every new field for the US and the UK with /{country}/metadata. Each country's default dataset now comes from the installed PolicyEngine.py release's manifest; the Public API no longer fixes its own (populace_us_2024 and microcosm_uk_2024_25), and publication stops if the release names no default or does not certify it. +Catalog format 2 stores the metadata the app's v3 workspace reads, taken only from PolicyEngine.py's public records: each model version's entities and basic inputs; each variable's input and hidden-input flags, module, the parameter path its adds or subtracts names, its enum values' labels, its unit and its definition period; each parameter's economy and household flags. Bracket schedules are stored as parameter nodes with their stored label. A generated revision adds the columns as nullable, so catalog versions published earlier keep them empty and are compared without them when republished. An opt-in parity test, run by the v2 integration check, compares every new field for the US and the UK with /{country}/metadata. Each country's default dataset now comes from the installed PolicyEngine.py release's manifest; the Public API no longer fixes its own (populace_us_2024 and microcosm_uk_2024_25), and publication stops if the release names no default or does not certify it. diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index b8f99b3e4..0ad3eb455 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -106,20 +106,20 @@ publishes entities, basic inputs and these flags: | Record | Fields | | --- | --- | | Model version | `catalog_format` (2), `entities` (`key`, `plural`, `is_person`, in the package's order), `basic_inputs` (the household builder's default fields) | -| Variable | `is_input_variable`, `hidden_input`, `module_name`, `adds_parameter_path` and `subtracts_parameter_path` (the parameter whose dated value lists the variables summed; `adds` and `subtracts` keep the list PolicyEngine.py resolved), `enum_values` (`value` and declared `label`, in order) | +| Variable | `is_input_variable`, `hidden_input`, `module_name`, `adds_parameter_path` and `subtracts_parameter_path` (the parameter whose dated value lists the variables summed; `adds` and `subtracts` keep the list PolicyEngine.py resolved), `enum_values` (`value` and declared `label`, in order), `unit` (for example `currency-USD`) and `definition_period` (`year`, `month` or `eternity`), which the household builder and breakdown read | | Parameter | `economy`, `household` | -Bracket schedules are parameter nodes with their stored label, or none when -the package stores none. An empty basic-input list is stored as empty. -Extraction stops before reading any record if the installed PolicyEngine.py -does not publish these fields, as neither 6.7.5 nor the locked 6.10.0 does, -and names each missing field. It also stops if the model has other than one person entity, names a -basic input or a parameter path that is not published, gives enum labels that -do not match the variable's possible values or are not JSON scalars, or gives -a flag that is not a boolean. An opt-in test -(`tests/integration/test_v2_catalog_metadata_parity.py`, run by the v2 -integration check) compares every format 2 field for the US and the UK with -what `/{country}/metadata` builds. +Bracket schedules are parameter nodes with their stored label, or none when the +package stores none. An empty basic-input list is stored as empty. Extraction +stops before reading any record if the installed PolicyEngine.py does not +publish these fields, and names each missing field: 6.7.5 and the locked 6.10.0 +publish only the variables' unit and definition period. It also stops if the +model has other than one person entity, names a basic input or a parameter path +that is not published, gives enum labels that do not match the variable's +possible values or are not JSON scalars, or gives a flag that is not a boolean. +An opt-in test (`tests/integration/test_v2_catalog_metadata_parity.py`, run by +the v2 integration check) compares every format 2 field for the US and the UK +with what `/{country}/metadata` builds. Publication never rewrites a stored version. A version published before format 2 has a null `catalog_format` and null format 2 fields, and later diff --git a/policyengine_api/data/v2/catalog/publication_reconciliation.py b/policyengine_api/data/v2/catalog/publication_reconciliation.py index 9ddcad20d..14390b6ee 100644 --- a/policyengine_api/data/v2/catalog/publication_reconciliation.py +++ b/policyengine_api/data/v2/catalog/publication_reconciliation.py @@ -168,6 +168,8 @@ def format_2(*columns: sa.ColumnElement) -> tuple[sa.ColumnElement, ...]: STAGE_CATALOG_VARIABLES.c.adds_parameter_path, STAGE_CATALOG_VARIABLES.c.subtracts_parameter_path, STAGE_CATALOG_VARIABLES.c.enum_values, + STAGE_CATALOG_VARIABLES.c.unit, + STAGE_CATALOG_VARIABLES.c.definition_period, ), ).where(STAGE_CATALOG_VARIABLES.c.country_id == country.country_id) actual_variables = ( @@ -188,6 +190,8 @@ def format_2(*columns: sa.ColumnElement) -> tuple[sa.ColumnElement, ...]: VARIABLES.c.adds_parameter_path, VARIABLES.c.subtracts_parameter_path, sa.cast(VARIABLES.c.enum_values, JSONB), + VARIABLES.c.unit, + VARIABLES.c.definition_period, ), ) .select_from(_versioned_table_from(VARIABLES)) @@ -525,6 +529,8 @@ def count_region_differences( "adds_parameter_path", "subtracts_parameter_path", "enum_values", + "unit", + "definition_period", "tax_benefit_model_version_id", ], sa.select( @@ -557,6 +563,8 @@ def count_region_differences( STAGE_CATALOG_VARIABLES.c.enum_values, VARIABLES.c.enum_values.type, ), + STAGE_CATALOG_VARIABLES.c.unit, + STAGE_CATALOG_VARIABLES.c.definition_period, MODEL_VERSIONS.c.id, ) .select_from( diff --git a/policyengine_api/data/v2/catalog/publication_staging.py b/policyengine_api/data/v2/catalog/publication_staging.py index 3d341e488..f4243fa64 100644 --- a/policyengine_api/data/v2/catalog/publication_staging.py +++ b/policyengine_api/data/v2/catalog/publication_staging.py @@ -68,6 +68,8 @@ sa.Column("adds_parameter_path", sa.Text), sa.Column("subtracts_parameter_path", sa.Text), sa.Column("enum_values", JSONB), + sa.Column("unit", sa.Text), + sa.Column("definition_period", sa.Text), prefixes=["TEMPORARY"], postgresql_on_commit="DROP", ) @@ -225,6 +227,8 @@ def _variable_rows(country: CountryCatalog) -> Iterator[tuple[object, ...]]: record.adds_parameter_path, record.subtracts_parameter_path, _enum_values_json(record), + record.unit, + record.definition_period, ) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 18044b240..92adc02d9 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -576,7 +576,8 @@ def _format_2_values(engine: Engine) -> dict[str, object]: variable = connection.execute( text( "SELECT is_input_variable, hidden_input, module_name, " - "adds_parameter_path, subtracts_parameter_path, enum_values " + "adds_parameter_path, subtracts_parameter_path, enum_values, " + "unit, definition_period " "FROM variables WHERE name = 'employment_income' " "AND tax_benefit_model_version_id = (" "SELECT version.id FROM tax_benefit_model_versions AS version " @@ -614,7 +615,8 @@ def _clear_format_2_fields(engine: Engine) -> None: text( "UPDATE variables SET is_input_variable = NULL, hidden_input = NULL, " "module_name = NULL, adds_parameter_path = NULL, " - "subtracts_parameter_path = NULL, enum_values = NULL" + "subtracts_parameter_path = NULL, enum_values = NULL, " + "unit = NULL, definition_period = NULL" ) ) connection.execute( @@ -643,6 +645,8 @@ def test_publication_stores_catalog_format_2_fields( None, None, None, + "currency-USD", + "year", ), "parameter": (True, False), } @@ -662,7 +666,7 @@ def test_version_stored_before_format_2_keeps_its_fields_empty_on_republish( assert _identifiers(publication_engine) == before_ids assert _format_2_values(publication_engine) == { "version": (None, None, None), - "variable": (None, None, None, None, None, None), + "variable": (None, None, None, None, None, None, None, None), "parameter": (None, None), } @@ -707,7 +711,8 @@ def _stored_us_variables(engine: Engine) -> dict[str, tuple[object, ...]]: rows = connection.execute( text( "SELECT variable.name, variable.enum_values, " - "variable.adds_parameter_path, variable.subtracts_parameter_path " + "variable.adds_parameter_path, variable.subtracts_parameter_path, " + "variable.unit, variable.definition_period " "FROM variables AS variable " "JOIN tax_benefit_model_versions AS version " "ON version.id = variable.tax_benefit_model_version_id " @@ -744,12 +749,16 @@ def test_enum_labels_and_parameter_paths_round_trip_exactly( [{"value": "SINGLE", "label": "Single"}, {"value": "JOINT", "label": 2}], None, None, + None, + "year", ) assert type(stored["filing_status"][0][1]["label"]) is int assert stored["countable_income"] == ( None, "gov.example.countable_income", "gov.example.deductions", + "currency-USD", + "month", ) assert republished.as_dict()["outcome"] == "ok" assert _identifiers(publication_engine) == before_ids @@ -769,6 +778,8 @@ def test_enum_labels_and_parameter_paths_round_trip_exactly( ), ("countable_income", {"adds_parameter_path": "gov.example.deductions"}), ("countable_income", {"subtracts_parameter_path": None}), + ("filing_status", {"unit": "currency-USD"}), + ("countable_income", {"definition_period": "year"}), ], ) def test_republication_detects_a_changed_enum_label_or_parameter_path( diff --git a/tests/unit/v2/test_catalog_publication.py b/tests/unit/v2/test_catalog_publication.py index afdc7f5a2..47cb262ba 100644 --- a/tests/unit/v2/test_catalog_publication.py +++ b/tests/unit/v2/test_catalog_publication.py @@ -249,7 +249,7 @@ def test_publication_uses_mapped_tables_and_sqlalchemy_statements() -> None: for format_1, format_2 in zip( format_1_comparisons, format_2_comparisons, strict=True ) - ] == [3, 6, 0, 2, 0, 0] + ] == [3, 8, 0, 2, 0, 0] class Result: def scalar_one(self): diff --git a/tests/unit/v2/test_catalog_staging_rows.py b/tests/unit/v2/test_catalog_staging_rows.py index 549743810..e5c86c3d6 100644 --- a/tests/unit/v2/test_catalog_staging_rows.py +++ b/tests/unit/v2/test_catalog_staging_rows.py @@ -84,6 +84,8 @@ def test_catalog_rows_project_every_record_field_in_column_order() -> None: record.adds_parameter_path, record.subtracts_parameter_path, None, + record.unit, + record.definition_period, ) for record in country.variables ] @@ -157,7 +159,8 @@ def test_enum_values_are_staged_as_value_and_label_objects_in_order() -> None: (row,) = (_plain(row) for row in _catalog_rows(country)[STAGE_CATALOG_VARIABLES]) - assert row[-1] == ( + enum_values_column = [column.name for column in STAGE_CATALOG_VARIABLES.columns] + assert row[enum_values_column.index("enum_values")] == ( "jsonb", [{"value": "SINGLE", "label": "Single"}, {"value": "JOINT", "label": 2}], ) From 7e0fb951845418ced7f4171dfa5412662c6b0e09 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:32:24 +0400 Subject: [PATCH 13/14] Name the unit and definition period in the format 2 test and fixture names After the unit and definition period were added, the round-trip, republication, parity and extraction tests and the fixture helper still named only the earlier fields, so a failure did not say which field it covers. They now name the unit and period (the fixture helper becomes add_format_2_variable_records, with a docstring listing what each variable fills), and the staging-row test's list of every staged column name is called staged_column_names. Tests only; behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- tests/fixtures/v2_catalog.py | 10 ++++++---- .../integration/test_v2_catalog_metadata_parity.py | 2 +- tests/integration/test_v2_catalog_publication.py | 14 +++++++------- tests/unit/v2/test_catalog_metadata_fields.py | 2 +- tests/unit/v2/test_catalog_staging_rows.py | 4 ++-- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/fixtures/v2_catalog.py b/tests/fixtures/v2_catalog.py index d24c6ff44..05bbf00e1 100644 --- a/tests/fixtures/v2_catalog.py +++ b/tests/fixtures/v2_catalog.py @@ -323,11 +323,13 @@ def normalized_catalog( ) -def add_enum_and_parameter_part_records(model: SimpleNamespace) -> None: - """Add an enum variable and a variable whose parts are named by parameters. +def add_format_2_variable_records(model: SimpleNamespace) -> None: + """Add two variables that between them fill every format 2 variable field. - The enum's labels are a string and an integer. The two parameters hold the - lists ``countable_income`` adds and subtracts. + ``filing_status`` is an enum whose labels are a string and an integer, with + no unit and a yearly period. ``countable_income`` names the parameters that + hold the lists it adds and subtracts, with a currency unit and a monthly + period. """ for name in ("gov.example.countable_income", "gov.example.deductions"): parameter = SimpleNamespace( diff --git a/tests/integration/test_v2_catalog_metadata_parity.py b/tests/integration/test_v2_catalog_metadata_parity.py index 3a426ad0f..079e31610 100644 --- a/tests/integration/test_v2_catalog_metadata_parity.py +++ b/tests/integration/test_v2_catalog_metadata_parity.py @@ -80,7 +80,7 @@ def test_entities_and_basic_inputs_match_v1( assert list(metadata.basic_inputs) == (v1_country.metadata["basicInputs"]) -def test_variable_flags_modules_enum_labels_and_parts_match_v1( +def test_variable_flags_modules_enums_parts_units_and_periods_match_v1( country_pair: tuple[ModelMetadata, Any], ) -> None: metadata, v1_country = country_pair diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 92adc02d9..a5ba64d1e 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -36,7 +36,7 @@ from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL from tests.fixtures.v2_catalog import ( POLICYENGINE_VERSION, - add_enum_and_parameter_part_records, + add_format_2_variable_records, bundle, installed_version, normalized_catalog, @@ -694,9 +694,9 @@ def test_version_stored_with_format_2_fails_when_a_format_2_field_changes( assert _format_2_values(publication_engine)["parameter"] == (True, False) -def _catalog_with_enum_and_parameter_parts(): +def _catalog_with_format_2_variable_records(): models = source_models() - add_enum_and_parameter_part_records(models["us"]) + add_format_2_variable_records(models["us"]) return extract_catalog( bundle=bundle(), policyengine_version=POLICYENGINE_VERSION, @@ -735,10 +735,10 @@ def _with_changed_us_variable(catalog, name: str, **changes: object): ) -def test_enum_labels_and_parameter_paths_round_trip_exactly( +def test_enum_labels_parameter_paths_units_and_periods_round_trip_exactly( publication_engine: Engine, ) -> None: - catalog = _catalog_with_enum_and_parameter_parts() + catalog = _catalog_with_format_2_variable_records() publish_catalog(publication_engine, catalog) before_ids = _identifiers(publication_engine) @@ -782,12 +782,12 @@ def test_enum_labels_and_parameter_paths_round_trip_exactly( ("countable_income", {"definition_period": "year"}), ], ) -def test_republication_detects_a_changed_enum_label_or_parameter_path( +def test_republication_detects_a_changed_enum_label_parameter_path_unit_or_period( publication_engine: Engine, variable_name: str, changes: dict[str, object], ) -> None: - catalog = _catalog_with_enum_and_parameter_parts() + catalog = _catalog_with_format_2_variable_records() publish_catalog(publication_engine, catalog) stored = _stored_us_variables(publication_engine) diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py index 48c85587f..36a1095e0 100644 --- a/tests/unit/v2/test_catalog_metadata_fields.py +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -106,7 +106,7 @@ def test_catalog_format_2_publishes_entities_and_basic_inputs() -> None: assert model_version.basic_inputs == ("employment_income",) -def test_variables_carry_input_flags_module_and_unset_parts() -> None: +def test_variables_carry_flags_module_unit_period_and_unset_parts() -> None: variable = normalized_catalog().country("us").variables[0] assert variable.name == "employment_income" diff --git a/tests/unit/v2/test_catalog_staging_rows.py b/tests/unit/v2/test_catalog_staging_rows.py index e5c86c3d6..e8b6f0bd4 100644 --- a/tests/unit/v2/test_catalog_staging_rows.py +++ b/tests/unit/v2/test_catalog_staging_rows.py @@ -159,8 +159,8 @@ def test_enum_values_are_staged_as_value_and_label_objects_in_order() -> None: (row,) = (_plain(row) for row in _catalog_rows(country)[STAGE_CATALOG_VARIABLES]) - enum_values_column = [column.name for column in STAGE_CATALOG_VARIABLES.columns] - assert row[enum_values_column.index("enum_values")] == ( + staged_column_names = [column.name for column in STAGE_CATALOG_VARIABLES.columns] + assert row[staged_column_names.index("enum_values")] == ( "jsonb", [{"value": "SINGLE", "label": "Single"}, {"value": "JOINT", "label": 2}], ) From f803106ee3cf68983c64b1efec72fc6d0c761161 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:35:53 +0400 Subject: [PATCH 14/14] Check format 2's variable text fields against their column lengths Extraction wrote the length limits of the module name, the two parameter paths, the unit and the definition period a second time, beside the Variable model's max_length. It now reads each limit from the column that stores the field (VARIABLE_TEXT_LIMITS), so the two cannot drift apart, and a test feeds each field one character too many and checks that extraction stops with the field's name and limit. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/extraction.py | 34 +++++++++++++--- tests/unit/v2/test_catalog_metadata_fields.py | 40 +++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 84606329f..4400c004a 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -39,6 +39,7 @@ filtered_artifacts, scope_regions, ) +from policyengine_api.data.v2.models import Variable if TYPE_CHECKING: from policyengine_api.json_types import JSONValue @@ -49,6 +50,29 @@ "policyengine-us", "policyengine-uk", ) + + +def _variable_column_length(column_name: str) -> int: + """Return the length limit of one text column of the v2 ``variables`` table.""" + length = getattr(Variable.__table__.c[column_name].type, "length", None) + if not isinstance(length, int): + msg = f"variables.{column_name} has no length limit" + raise TypeError(msg) + return length + + +# Catalog format 2's variable text fields, checked against the columns that +# store them, so the two limits cannot drift apart. +VARIABLE_TEXT_LIMITS: Mapping[str, int] = { + column_name: _variable_column_length(column_name) + for column_name in ( + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "unit", + "definition_period", + ) +} CURRENT_LAW_IDS = {"us": 2, "uk": 1} METADATA_TIME_PERIODS = { "us": tuple(range(2035, 2021, -1)), @@ -347,28 +371,28 @@ def optional_names(attribute: str) -> list[str] | None: module_name=_optional_text( getattr(source, "module_name", None), field_name=f"variable {name} module_name", - maximum=512, + maximum=VARIABLE_TEXT_LIMITS["module_name"], ), adds_parameter_path=_optional_text( getattr(source, "adds_parameter_path", None), field_name=f"variable {name} adds_parameter_path", - maximum=512, + maximum=VARIABLE_TEXT_LIMITS["adds_parameter_path"], ), subtracts_parameter_path=_optional_text( getattr(source, "subtracts_parameter_path", None), field_name=f"variable {name} subtracts_parameter_path", - maximum=512, + maximum=VARIABLE_TEXT_LIMITS["subtracts_parameter_path"], ), enum_values=enum_values, unit=_optional_text( getattr(source, "unit", None), field_name=f"variable {name} unit", - maximum=128, + maximum=VARIABLE_TEXT_LIMITS["unit"], ), definition_period=_optional_text( getattr(source, "definition_period", None), field_name=f"variable {name} definition_period", - maximum=32, + maximum=VARIABLE_TEXT_LIMITS["definition_period"], ), ) diff --git a/tests/unit/v2/test_catalog_metadata_fields.py b/tests/unit/v2/test_catalog_metadata_fields.py index 36a1095e0..6b8cc3ddc 100644 --- a/tests/unit/v2/test_catalog_metadata_fields.py +++ b/tests/unit/v2/test_catalog_metadata_fields.py @@ -448,3 +448,43 @@ def test_an_empty_basic_input_list_is_stored_empty() -> None: models["us"].basic_inputs = [] assert _extract(models).country("us").model_version.basic_inputs == () + + +FORMAT_2_VARIABLE_TEXT_FIELDS = ( + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "unit", + "definition_period", +) + + +def _variable_column_length(field_name: str) -> int: + from policyengine_api.data.v2.models import Variable + + return Variable.__table__.c[field_name].type.length + + +def test_variable_text_limits_are_the_variable_column_lengths() -> None: + """Extraction checks each format 2 text field against its column's length.""" + from policyengine_api.data.v2.catalog.extraction import VARIABLE_TEXT_LIMITS + + column_lengths = { + field_name: _variable_column_length(field_name) + for field_name in FORMAT_2_VARIABLE_TEXT_FIELDS + } + + assert column_lengths == VARIABLE_TEXT_LIMITS + + +@pytest.mark.parametrize("field_name", FORMAT_2_VARIABLE_TEXT_FIELDS) +def test_an_over_long_format_2_text_field_stops_extraction(field_name: str) -> None: + length = _variable_column_length(field_name) + models = source_models() + setattr(models["us"].variables[0], field_name, "x" * (length + 1)) + + with pytest.raises( + CatalogExtractionError, + match=rf"^variable employment_income {field_name} exceeds {length} characters$", + ): + _extract(models)