diff --git a/changelog.d/v2plan-1f-117-parameter-children.added.md b/changelog.d/v2plan-1f-117-parameter-children.added.md new file mode 100644 index 000000000..23473a174 --- /dev/null +++ b/changelog.d/v2plan-1f-117-parameter-children.added.md @@ -0,0 +1 @@ +Catalog publication writes parameter trees into a new table, parameter_tree_children, built from the installed packages' catalog records for each version: one row per node and parameter under its parent's path, with its label, its position among its siblings, its direct child counts, and whether a reform can change it. A bracket schedule is one node whose children are its brackets, labelled "Bracket 1", "Bracket 2" and so on, and labels take the form the app's v3 workspace shows, with a path without a stored label getting one made from the last part of its name. Publication writes the tree of each version it publishes, and of the installed version when an earlier release stored it with catalog format 2 but without one; its evidence lists them. Other stored versions, and an installed version stored before catalog format 2, have no tree. diff --git a/changelog.d/v2plan-1f-117-parameter-children.changed.md b/changelog.d/v2plan-1f-117-parameter-children.changed.md new file mode 100644 index 000000000..a21e91b00 --- /dev/null +++ b/changelog.d/v2plan-1f-117-parameter-children.changed.md @@ -0,0 +1 @@ +/v2/parameters/children reads one level of the stored parameter tree with one indexed read. A bracket schedule is now listed as one node whose children are its brackets, instead of its brackets appearing beside its parent's groups under their raw names, and every child carries its label in the form the app's v3 workspace shows: the stored label with underscores as spaces and its first letter upper-cased, or one made from the last part of its name. reform_changeable=true lists only the children a reform can change, sorted by label, with child counts that count only those, following the rules of the tree the app's v3 workspace builds. The route now reads its query through the shared strict models, so unknown or repeated parameters, an unsupported country and a malformed policyengine_version return 422 (an unsupported country and a malformed version returned 400 before), and a catalog version without a tree returns 503 with a message naming the version. diff --git a/docs/engineering/skills/alembic-migrations.md b/docs/engineering/skills/alembic-migrations.md index 093358e22..a2e36b84c 100644 --- a/docs/engineering/skills/alembic-migrations.md +++ b/docs/engineering/skills/alembic-migrations.md @@ -146,9 +146,10 @@ check the declarations: - `api.v2.runtime-privileges-declared`: a unit test reads each connector module and fails when it needs a privilege its role isn't declared to hold. - `api.v2.runtime-privileges-tested-as-runtime-role`: the cross-database - integration tests run the v2 user services as `policyengine_v2_runtime` and - the comparison-run services as `policyengine_stage12_test`, holding only - their declared privileges. + integration tests run the v2 user services and the parameter tree read as + `policyengine_v2_runtime`, catalog publication as + `policyengine_v2_catalog_writer`, and the comparison-run services as + `policyengine_stage12_test`, holding only their declared privileges. - `api.v2.deployed-privileges-match`: every deploy runs `scripts/reconcile_v2_privileges.py apply` right after the schema upgrade, then `verify`, before the catalog is seeded. @@ -183,6 +184,8 @@ When a pull request adds or changes a v2 table, or a connector's use of one: uv run pytest -q tests/integration/test_v2_user_identity_persistence.py \ tests/integration/test_v2_saved_policy_view.py \ tests/integration/test_v2_stage12_comparison_persistence.py \ + tests/integration/test_v2_metadata_routes.py \ + tests/integration/test_v2_catalog_publication.py \ tests/integration/test_v2_privilege_reconciliation.py ``` diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 2d00bc6b6..e2507a1d2 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -234,7 +234,7 @@ flowchart LR | `api.migration.cross-database-integration` | Cross-database integration tests pass against real MySQL and Postgres. | mechanical | enforced | yes | CI job `v2-integration-check` | `pr.yml:v2-integration-check` | — | | `api.v2.deployed-privileges-match` | Every v2 deploy grants the managed roles each privilege policyengine_api/data/v2/privileges.py declares right after the schema upgrade, reports privileges it doesn't declare without revoking them, and stops before the catalog is seeded if a declared privilege is still missing. | mechanical | enforced | yes | CI job `seed`, `catalog-runtime-integration` | `seed-v2-database.yml:seed`, `v2-integration-check.yml:catalog-runtime-integration` | yes | | `api.v2.runtime-privileges-declared` | Every v2 table has an entry for each managed database role in policyengine_api/data/v2/privileges.py, and every privilege a v2 connector module needs (from its reads, creates, updates or deletes module name and its row locks) is declared for the role that runs it. | mechanical | enforced | yes | pytest `tests/unit/v2/test_privilege_manifest.py` | `pr.yml:test` | yes | -| `api.v2.runtime-privileges-tested-as-runtime-role` | The cross-database integration tests run the v2 user services as policyengine_v2_runtime and the comparison-run services as policyengine_stage12_, holding only their declared privileges, so a statement those privileges don't allow fails CI. | mechanical | enforced | yes | CI job `v2-integration-check`, `catalog-runtime-integration` | `pr.yml:v2-integration-check`, `v2-integration-check.yml:catalog-runtime-integration` | — | +| `api.v2.runtime-privileges-tested-as-runtime-role` | The cross-database integration tests run the v2 user services and the parameter tree read as policyengine_v2_runtime, catalog publication as policyengine_v2_catalog_writer, and the comparison-run services as policyengine_stage12_, holding only their declared privileges, so a statement those privileges don't allow fails CI. | mechanical | enforced | yes | CI job `v2-integration-check`, `catalog-runtime-integration` | `pr.yml:v2-integration-check`, `v2-integration-check.yml:catalog-runtime-integration` | — | ```mermaid flowchart LR diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 8e9ea0123..ae3ca794e 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -305,6 +305,55 @@ since they describe staged regions that were not published. Every other table must still match exactly. Region reference changes therefore take effect in the next catalog version that is published. +### Parameter tree + +Publication also writes parameter trees into `parameter_tree_children`, which +`/v2/parameters/children` reads one level at a time. A tree is built from a +catalog version's parameter and node records: every node and parameter is one +row under its parent's path, a `[` starts a child as a `.` does (so a bracket schedule is one node +whose children are its brackets, labelled "Bracket 1", "Bracket 2" and so on), +and labels take the form the app's newer interface shows in every view: the +stored label with underscores as spaces and its first letter upper-cased, or, +without one, the last part of the name in that form (so an unlabelled schedule +`...rates.uk` is "Uk"). Each row also records its position among all its +siblings and, if a reform can change it, among the siblings a reform can +change: a parameter under `gov` whose economy or household flag is set and whose path names +neither `taxsim`, `gov.abolitions` nor `pycache`, and every node above such a +parameter. These are the rules of the tree the app's newer interface builds +from v1 metadata. Trees are written only for versions with catalog format 2, +so every parameter in a tree has both flags. + +In the same transaction, the command writes the tree of each version in the +catalog it publishes, which comes from the installed packages, when that +version has none: the versions it publishes now, and the installed version when +an earlier release stored it with catalog format 2 but without a tree. An +installed version stored before catalog format 2 gets none, since its parameter +rows store no flags. The rows are built from the +catalog's own parameter and node records; only the stored parameters' IDs are +read, by name, so the rows reference them, after publication has checked that +the stored rows match the catalog. This is a row-only, idempotent insert of +package-derived rows, which the deployment-seeder passage of the Alembic skill +allows. The evidence's `parameter_trees_written` lists each version written, +with its model, version and row count, and publication fails if a version in +the catalog with catalog format 2 is still without a tree when it finishes. + +Other stored versions get no tree. Writing one for them would derive rows from +what earlier releases stored, a data migration that the Alembic skill's rules +leave to a human migration decision, so it is not done; such a version gets a +tree only if a person decides on that migration. + +`/v2/parameters/children` lists one level with one indexed read: +`parent_path` names the node ("" for the top level), and +`reform_changeable=true` keeps only the children a reform can change, sorted +by label, with child counts that count only those. Without it, children are +in path order, with a schedule's brackets in bracket order. A node carries +its child count, a parameter its summary. An unknown parent path has no +children. The route reads its query through the shared strict query models, +so unknown or repeated parameters, an unsupported country and a malformed +`policyengine_version` return 422 (the last two returned 400 before). A +catalog version without a tree returns 503 with a message that names the +version. + ## Validation evidence and retry Successful JSON evidence contains: @@ -315,6 +364,8 @@ Successful JSON evidence contains: - fallback counts and omitted-region counts by country and region type, and the stored and staged region counts of any stored version whose regions were kept; +- the model, version and row count of each catalog version whose parameter + tree was written; - elapsed publication time; and - a success outcome. diff --git a/migrations/v2/versions/3869eb814a90_store_each_catalog_version_s_parameter_.py b/migrations/v2/versions/3869eb814a90_store_each_catalog_version_s_parameter_.py new file mode 100644 index 000000000..7ac26f0e6 --- /dev/null +++ b/migrations/v2/versions/3869eb814a90_store_each_catalog_version_s_parameter_.py @@ -0,0 +1,115 @@ +"""store each catalog version's parameter tree + +Revision ID: 3869eb814a90 +Revises: f5c1d9b4685b +Create Date: 2026-10-22 22:51:53.959795 +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 = "3869eb814a90" +down_revision: Union[str, None] = "f5c1d9b4685b" +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.create_table( + "parameter_tree_children", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column("tax_benefit_model_version_id", sa.Uuid(), nullable=False), + sa.Column( + "parent_path", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=False + ), + sa.Column("path", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=False), + sa.Column("position", sa.Integer(), nullable=False), + sa.Column( + "label", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=False + ), + sa.Column( + "child_type", sqlmodel.sql.sqltypes.AutoString(length=16), nullable=False + ), + sa.Column("child_count", sa.Integer(), nullable=True), + sa.Column("reform_position", sa.Integer(), nullable=True), + sa.Column("reform_child_count", sa.Integer(), nullable=True), + sa.Column("parameter_id", sa.Uuid(), nullable=True), + sa.Column("economy", sa.Boolean(), nullable=True), + sa.Column("household", sa.Boolean(), nullable=True), + sa.CheckConstraint( + "(child_type = 'parameter') = (parameter_id IS NOT NULL)", + name=op.f("ck_parameter_tree_children_parameter_reference"), + ), + sa.CheckConstraint( + "child_type IN ('node', 'parameter')", + name=op.f("ck_parameter_tree_children_child_type"), + ), + sa.ForeignKeyConstraint( + ["parameter_id"], + ["parameters.id"], + name=op.f("fk_parameter_tree_children_parameter_id_parameters"), + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["tax_benefit_model_version_id"], + ["tax_benefit_model_versions.id"], + name=op.f( + "fk_parameter_tree_children_tax_benefit_model_version_id_tax_benefit_model_versions" + ), + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("id", name=op.f("pk_parameter_tree_children")), + sa.UniqueConstraint( + "tax_benefit_model_version_id", + "path", + name="uq_parameter_tree_children_model_version_path", + ), + ) + op.create_index( + "ix_parameter_tree_children_listing", + "parameter_tree_children", + ["tax_benefit_model_version_id", "parent_path", "position"], + unique=True, + ) + op.create_index( + op.f("ix_parameter_tree_children_parameter_id"), + "parameter_tree_children", + ["parameter_id"], + unique=False, + ) + op.create_index( + "ix_parameter_tree_children_reform_listing", + "parameter_tree_children", + ["tax_benefit_model_version_id", "parent_path", "reform_position"], + unique=True, + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index( + "ix_parameter_tree_children_reform_listing", + table_name="parameter_tree_children", + ) + op.drop_index( + op.f("ix_parameter_tree_children_parameter_id"), + table_name="parameter_tree_children", + ) + op.drop_index( + "ix_parameter_tree_children_listing", table_name="parameter_tree_children" + ) + op.drop_table("parameter_tree_children") + # ### end Alembic commands ### diff --git a/policyengine_api/data/v2/catalog/parameter_tree.py b/policyengine_api/data/v2/catalog/parameter_tree.py new file mode 100644 index 000000000..7f83b579d --- /dev/null +++ b/policyengine_api/data/v2/catalog/parameter_tree.py @@ -0,0 +1,250 @@ +"""Each catalog version's parameter tree, as rows of direct children. + +The tree is built from a catalog version's parameter and node records: catalog +publication builds it from the installed packages' catalog, for the versions it +publishes and for the installed version when it is stored with catalog format 2 +but without a tree. Every node and parameter of the version is one row, under +the path of its parent (the empty string for top-level paths). + +A ``[`` starts a child as a ``.`` does: a bracket schedule such as +``gov.hmrc.income_tax.rates.uk`` is one node whose children are its brackets +``uk[0]``, ``uk[1]`` and so on, labelled "Bracket 1", "Bracket 2"; a bracket's +own children (``rate``, ``threshold``) are unchanged. Labels take the form +the app's newer interface (v3) shows, in every view: a stored label with its +underscores as spaces and its first letter upper-cased, or, for a path with no +stored label, the last part of its name in the same form. + +Each row also records whether a reform can change it, which mirrors the tree +the app's newer interface builds from v1 metadata +(``app/src/libs/buildParameterTree.ts``): a parameter under ``gov`` whose +economy or household flag is set and whose path names neither ``taxsim``, +``gov.abolitions`` nor ``pycache``, and every node with such a parameter +below it. Trees are written only for versions with catalog format 2, so every +parameter has both flags. +""" + +from __future__ import annotations + +import re +from collections import defaultdict +from dataclasses import dataclass +from typing import TYPE_CHECKING, Literal + +if TYPE_CHECKING: + from collections.abc import Iterable + from uuid import UUID + +REFORM_TREE_PREFIX = "gov." +REFORM_EXCLUDED_PATH_PARTS = ("taxsim", "gov.abolitions", "pycache") + +_BRACKET_INDEX = re.compile(r"\[(\d+)\]$") +_NAME_PARTS = re.compile(r"[.\[]") + +ChildType = Literal["node", "parameter"] + + +@dataclass(frozen=True, slots=True) +class TreeNodeSource: + """A parameter node of a catalog version: its name and stored label.""" + + name: str + label: str | None + + +@dataclass(frozen=True, slots=True) +class TreeParameterSource: + """A parameter of a catalog version: its stored ID, name, label and flags.""" + + id: UUID + name: str + label: str | None + economy: bool | None + household: bool | None + + +@dataclass(frozen=True, slots=True) +class ParameterTreeChildRow: + """One direct child in a version's parameter tree. + + ``position`` orders the children of one parent; ``reform_position`` orders + only those a reform can change and is None for the others. Node rows carry + their direct child counts (all children, and those a reform can change); + parameter rows carry the parameter's ID and flags instead. + """ + + parent_path: str + path: str + position: int + label: str + child_type: ChildType + child_count: int | None + reform_changeable: bool + reform_position: int | None + reform_child_count: int | None + parameter_id: UUID | None + economy: bool | None + household: bool | None + + +def parent_path(path: str) -> str: + """Return the path of a node's or parameter's parent, or "" at the top.""" + delimiter_index = max(path.rfind("."), path.rfind("[")) + return path[:delimiter_index] if delimiter_index > 0 else "" + + +def bracket_index(path: str) -> int | None: + """Return the index of a bracket path such as ``rates.uk[0]``, else None.""" + match = _BRACKET_INDEX.search(path) + return int(match.group(1)) if match else None + + +def display_label(label: str) -> str: + """Return a label as v3 shows it: underscores as spaces, first letter upper.""" + words = label.replace("_", " ") + return words[:1].upper() + words[1:] + + +def generated_label(path: str) -> str: + """Return a label made from the last part of a name, as v3 makes one.""" + return display_label(_NAME_PARTS.split(path)[-1]) + + +def _child_label(path: str, stored_label: str | None) -> str: + index = bracket_index(path) + if index is not None: + return f"Bracket {index + 1}" + if stored_label and stored_label.strip(): + return display_label(stored_label) + return generated_label(path) + + +def is_reform_changeable_parameter(parameter: TreeParameterSource) -> bool: + """Return whether a reform can change this parameter, as v3's tree decides.""" + if not parameter.name.startswith(REFORM_TREE_PREFIX): + return False + if any(part in parameter.name for part in REFORM_EXCLUDED_PATH_PARTS): + return False + return bool(parameter.economy) or bool(parameter.household) + + +def _ancestors(path: str) -> list[str]: + ancestors = [] + current = parent_path(path) + while current: + ancestors.append(current) + current = parent_path(current) + return ancestors + + +def _position_key(path: str) -> tuple[int, int, str]: + index = bracket_index(path) + if index is not None: + return (0, index, "") + return (1, 0, path) + + +def _reform_position_key(path: str, label: str) -> tuple[int, int, str, str]: + index = bracket_index(path) + if index is not None: + return (0, index, "", "") + return (1, 0, label.casefold(), path) + + +@dataclass(frozen=True, slots=True) +class _TreeIndex: + """The maps a version's tree rows are built from.""" + + children: dict[str, list[str]] + labels: dict[str, str] + changeable: frozenset[str] + parameters_by_path: dict[str, TreeParameterSource] + + def row( + self, path: str, *, parent: str, position: int, reform_position: int | None + ) -> ParameterTreeChildRow: + direct_children = self.children.get(path, []) + parameter = self.parameters_by_path.get(path) + leaf = parameter if parameter is not None and not direct_children else None + return ParameterTreeChildRow( + parent_path=parent, + path=path, + position=position, + label=self.labels[path], + child_type="node" if leaf is None else "parameter", + child_count=len(direct_children) if leaf is None else None, + reform_changeable=path in self.changeable, + reform_position=reform_position, + reform_child_count=( + sum(child in self.changeable for child in direct_children) + if leaf is None + else None + ), + parameter_id=None if leaf is None else leaf.id, + economy=None if leaf is None else leaf.economy, + household=None if leaf is None else leaf.household, + ) + + +def _stored_label( + path: str, + stored_labels: dict[str, str | None], + parameters_by_path: dict[str, TreeParameterSource], +) -> str | None: + if path in stored_labels: + return stored_labels[path] + parameter = parameters_by_path.get(path) + return None if parameter is None else parameter.label + + +def build_parameter_tree( + *, + nodes: Iterable[TreeNodeSource], + parameters: Iterable[TreeParameterSource], +) -> tuple[ParameterTreeChildRow, ...]: + """Return one row per node and parameter of a version, parents first.""" + parameters_by_path = {parameter.name: parameter for parameter in parameters} + stored_labels = {node.name: node.label for node in nodes} + paths = set(stored_labels) | set(parameters_by_path) + for path in list(paths): + paths.update(_ancestors(path)) + paths.discard("") + + changeable: set[str] = set() + for parameter in parameters_by_path.values(): + if is_reform_changeable_parameter(parameter): + changeable.add(parameter.name) + changeable.update(_ancestors(parameter.name)) + + children: defaultdict[str, list[str]] = defaultdict(list) + for path in paths: + children[parent_path(path)].append(path) + + index = _TreeIndex( + children=dict(children), + labels={ + path: _child_label( + path, _stored_label(path, stored_labels, parameters_by_path) + ) + for path in paths + }, + changeable=frozenset(changeable), + parameters_by_path=parameters_by_path, + ) + rows: list[ParameterTreeChildRow] = [] + for parent in sorted(index.children, key=lambda path: (path.count("."), path)): + siblings = sorted(index.children[parent], key=_position_key) + reform_siblings = sorted( + (path for path in siblings if path in index.changeable), + key=lambda path: _reform_position_key(path, index.labels[path]), + ) + reform_positions = {path: order for order, path in enumerate(reform_siblings)} + rows.extend( + index.row( + path, + parent=parent, + position=position, + reform_position=reform_positions.get(path), + ) + for position, path in enumerate(siblings) + ) + return tuple(rows) diff --git a/policyengine_api/data/v2/catalog/publication.py b/policyengine_api/data/v2/catalog/publication.py index a8951d6b5..5bbec59f1 100644 --- a/policyengine_api/data/v2/catalog/publication.py +++ b/policyengine_api/data/v2/catalog/publication.py @@ -4,11 +4,16 @@ import logging import time +from dataclasses import replace from typing import TYPE_CHECKING import sqlalchemy as sa from sqlalchemy import Connection, Engine +from policyengine_api.data.v2.catalog.publication_parameter_tree import ( + assert_parameter_trees_written, + write_parameter_trees, +) from policyengine_api.data.v2.catalog.publication_reconciliation import ( assert_canonical_value_uniqueness, assert_country_matches, @@ -118,7 +123,14 @@ def publish_catalog( *, checkpoint: Callable[[str, Connection], None] | None = None, ) -> PublicationEvidence: - """Publish one complete catalog atomically and return non-secret evidence.""" + """Publish one complete catalog atomically and return non-secret evidence. + + In the same transaction it writes the parameter tree of each version in + the catalog that has none, built from the catalog: the versions it + publishes, and the installed version when an earlier release stored it with + catalog format 2 but without a tree. Other stored versions, and an + installed version stored before catalog format 2, keep having none. + """ started = time.monotonic() with engine.begin() as connection: _verify_expected_revision(connection) @@ -134,6 +146,7 @@ def publish_catalog( for country in catalog.countries: if country.country_id not in existing: publish_new_country(connection, country) + parameter_trees_written = write_parameter_trees(connection, catalog.countries) if checkpoint is not None: checkpoint("after_reconciliation", connection) @@ -144,13 +157,17 @@ def publish_catalog( include_regions=country.country_id not in existing, ) assert_canonical_value_uniqueness(connection) + assert_parameter_trees_written(connection, catalog.countries) if _protected_row_counts(connection) != before: msg = "publication changed simulation, report, or dataset-version rows" raise CatalogPublicationError(msg) if checkpoint is not None: checkpoint("after_validation", connection) - return _evidence(catalog, kept_region_summaries, started=started) + return replace( + _evidence(catalog, kept_region_summaries, started=started), + parameter_trees_written=parameter_trees_written, + ) def _check_stored_versions( diff --git a/policyengine_api/data/v2/catalog/publication_parameter_tree.py b/policyengine_api/data/v2/catalog/publication_parameter_tree.py new file mode 100644 index 000000000..1147a18ae --- /dev/null +++ b/policyengine_api/data/v2/catalog/publication_parameter_tree.py @@ -0,0 +1,200 @@ +"""Write the parameter tree of the installed catalog's versions. + +Publication writes the tree of each country version in the catalog it +publishes, which is extracted from the installed packages: versions it +publishes now, and the installed version when an earlier release stored it with +catalog format 2 but without a tree. An installed version stored before catalog +format 2 gets none, since its parameter rows store no flags. The rows are built +from the catalog's own parameter and node records (``parameter_tree``), never +from rows an earlier release stored. Only +the stored parameters' IDs are read, by name, so tree rows reference the stored +parameters; publication has already checked that those stored rows match the +catalog. Writing only inserts rows for a version that has no tree; no other +table changes. + +Other stored versions get no tree here: deriving their rows from what earlier +releases stored would be a data migration, which the migration rules leave to +a human decision. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import TYPE_CHECKING +from uuid import uuid4 + +import sqlalchemy as sa + +from policyengine_api.data.v2.catalog.parameter_tree import ( + ParameterTreeChildRow, + TreeNodeSource, + TreeParameterSource, + build_parameter_tree, +) +from policyengine_api.data.v2.catalog.publication_reconciliation import ( + StoredVersion, + stored_version, +) +from policyengine_api.data.v2.catalog.publication_staging import copy_rows +from policyengine_api.data.v2.catalog.publication_types import ( + CatalogPublicationError, +) +from policyengine_api.data.v2.models import ( + Parameter, + ParameterTreeChild, +) + +if TYPE_CHECKING: + from collections.abc import Iterator, Sequence + from uuid import UUID + + from sqlalchemy import Connection + + from policyengine_api.data.v2.catalog.records import CountryCatalog + +PARAMETERS = Parameter.__table__ +PARAMETER_TREE_CHILDREN = ParameterTreeChild.__table__ + + +def _version_for_tree(connection: Connection, country: CountryCatalog) -> StoredVersion: + stored = stored_version(connection, country) + if stored is None: + msg = ( + f"the {country.country_id} catalog version " + f"{country.model_version.version} is not stored" + ) + raise CatalogPublicationError(msg) + return stored + + +def _has_tree(connection: Connection, version_id: UUID) -> bool: + return bool( + connection.execute( + sa.select( + sa.exists().where( + PARAMETER_TREE_CHILDREN.c.tax_benefit_model_version_id == version_id + ) + ) + ).scalar_one() + ) + + +def _stored_parameter_ids(connection: Connection, version_id: UUID) -> dict[str, UUID]: + return { + row.name: row.id + for row in connection.execute( + sa.select(PARAMETERS.c.name, PARAMETERS.c.id).where( + PARAMETERS.c.tax_benefit_model_version_id == version_id + ) + ) + } + + +def catalog_parameter_tree( + country: CountryCatalog, stored_parameter_ids: dict[str, UUID] +) -> tuple[ParameterTreeChildRow, ...]: + """Build a country version's tree from its catalog records. + + Each parameter row references the stored parameter of the same name; + raise CatalogPublicationError if a catalog parameter has no stored row. + """ + missing = [ + parameter.name + for parameter in country.parameters + if parameter.name not in stored_parameter_ids + ] + if missing: + msg = ( + f"the stored {country.country_id} catalog version " + f"{country.model_version.version} lacks the parameter {missing[0]}" + ) + raise CatalogPublicationError(msg) + return build_parameter_tree( + nodes=[ + TreeNodeSource(name=node.name, label=node.label) + for node in country.parameter_nodes + ], + parameters=[ + TreeParameterSource( + id=stored_parameter_ids[parameter.name], + name=parameter.name, + label=parameter.label, + economy=parameter.economy, + household=parameter.household, + ) + for parameter in country.parameters + ], + ) + + +def _table_rows( + version_id: UUID, + rows: tuple[ParameterTreeChildRow, ...], +) -> Iterator[tuple[object, ...]]: + created_at = datetime.now(UTC) + column_names = [column.name for column in PARAMETER_TREE_CHILDREN.columns] + for row in rows: + values: dict[str, object] = { + "id": uuid4(), + "created_at": created_at, + "tax_benefit_model_version_id": version_id, + "parent_path": row.parent_path, + "path": row.path, + "position": row.position, + "label": row.label, + "child_type": row.child_type, + "child_count": row.child_count, + "reform_position": row.reform_position, + "reform_child_count": row.reform_child_count, + "parameter_id": row.parameter_id, + "economy": row.economy, + "household": row.household, + } + yield tuple(values[name] for name in column_names) + + +def write_parameter_trees( + connection: Connection, + countries: Sequence[CountryCatalog], +) -> tuple[tuple[str, str, int], ...]: + """Write the tree of each catalog country's stored version that has none. + + A version stored before catalog format 2 gets none: its parameter rows + store no flags, so a tree built with the catalog's flags would not match + them. Returns the model name, version and number of rows written for each + version, in model name order. Runs inside publication's transaction and + advisory lock, after the countries' versions are stored and checked. + """ + written = [] + for country in countries: + stored = _version_for_tree(connection, country) + if stored.catalog_format is None or _has_tree(connection, stored.id): + continue + rows = catalog_parameter_tree( + country, _stored_parameter_ids(connection, stored.id) + ) + count = copy_rows( + connection, + table=PARAMETER_TREE_CHILDREN, + rows=_table_rows(stored.id, rows), + ) + written.append((country.model.name, country.model_version.version, count)) + return tuple(sorted(written)) + + +def assert_parameter_trees_written( + connection: Connection, + countries: Sequence[CountryCatalog], +) -> None: + """Raise CatalogPublicationError if a catalog country's version has no tree. + + Versions stored before catalog format 2 are exempt: they get no tree. + """ + for country in countries: + stored = _version_for_tree(connection, country) + if stored.catalog_format is not None and not _has_tree(connection, stored.id): + msg = ( + f"the {country.country_id} catalog version " + f"{country.model_version.version} has no parameter tree" + ) + raise CatalogPublicationError(msg) diff --git a/policyengine_api/data/v2/catalog/publication_reconciliation.py b/policyengine_api/data/v2/catalog/publication_reconciliation.py index 14390b6ee..0000941d9 100644 --- a/policyengine_api/data/v2/catalog/publication_reconciliation.py +++ b/policyengine_api/data/v2/catalog/publication_reconciliation.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import dataclass from typing import TYPE_CHECKING import sqlalchemy as sa @@ -33,6 +34,8 @@ ) if TYPE_CHECKING: + from uuid import UUID + from policyengine_api.data.v2.catalog.records import CountryCatalog DATASETS = Dataset.__table__ @@ -76,16 +79,24 @@ 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. +@dataclass(frozen=True, slots=True) +class StoredVersion: + """A country's stored catalog version: its ID and catalog format. - None means it was published before catalog format 2, so its format 2 - fields are empty. + ``catalog_format`` is None for a version published before catalog format 2, + whose format 2 fields are empty. """ + + id: UUID + catalog_format: int | None + + +def stored_version( + connection: Connection, country: CountryCatalog +) -> StoredVersion | None: + """Return this country's stored catalog version, or None if it is not stored.""" statement = ( - sa.select(MODEL_VERSIONS.c.catalog_format) + sa.select(MODEL_VERSIONS.c.id, MODEL_VERSIONS.c.catalog_format) .select_from( MODEL_VERSIONS.join( MODELS, @@ -97,7 +108,10 @@ def stored_catalog_format( MODEL_VERSIONS.c.version == country.model_version.version, ) ) - return connection.execute(statement).scalar_one_or_none() + row = connection.execute(statement).one_or_none() + if row is None: + return None + return StoredVersion(id=row.id, catalog_format=row.catalog_format) def _comparison_pairs( @@ -375,8 +389,11 @@ def assert_country_matches( PolicyEngine.py version stays the same. A version published before catalog format 2 is compared without format 2's fields. """ - stored_format = stored_catalog_format(connection, country) - pairs = _comparison_pairs(country, with_format_2=stored_format is not None) + stored = stored_version(connection, country) + pairs = _comparison_pairs( + country, + with_format_2=stored is not None and stored.catalog_format is not None, + ) if include_regions: pairs = (*pairs, ("regions", *_region_comparison_pair(country))) for table_name, staged, actual in pairs: diff --git a/policyengine_api/data/v2/catalog/publication_types.py b/policyengine_api/data/v2/catalog/publication_types.py index 515232560..d547d0fa4 100644 --- a/policyengine_api/data/v2/catalog/publication_types.py +++ b/policyengine_api/data/v2/catalog/publication_types.py @@ -22,6 +22,10 @@ class PublicationEvidence: # Stored catalog versions whose region rows were kept although the staged # regions differ: country, version, stored and staged region counts. kept_region_summaries: tuple[tuple[str, str, int, int], ...] = () + # The catalog's versions whose parameter tree this publication wrote: + # model, version and tree rows. Empty when none needed one: each already + # has its tree, or was stored before catalog format 2 and gets none. + parameter_trees_written: tuple[tuple[str, str, int], ...] = () def as_dict(self) -> dict[str, object]: """Return the evidence as the JSON object printed after a publication.""" @@ -55,5 +59,9 @@ def as_dict(self) -> dict[str, object]: } for country_id, version, stored, staged in self.kept_region_summaries ], + "parameter_trees_written": [ + {"model": model_name, "version": version, "rows": rows} + for model_name, version, rows in self.parameter_trees_written + ], "elapsed_seconds": round(self.elapsed_seconds, 3), } diff --git a/policyengine_api/data/v2/migration_target.py b/policyengine_api/data/v2/migration_target.py index d463fad16..c22f670bc 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 = "f5c1d9b4685b" +V2_ALEMBIC_HEAD_REVISION = "3869eb814a90" 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/__init__.py b/policyengine_api/data/v2/models/__init__.py index 1b1a73baf..7cc03c7b4 100644 --- a/policyengine_api/data/v2/models/__init__.py +++ b/policyengine_api/data/v2/models/__init__.py @@ -46,6 +46,7 @@ DatasetVersion, Parameter, ParameterNode, + ParameterTreeChild, ParameterValue, Region, RegionType, @@ -124,6 +125,7 @@ "OutputStatus", "Parameter", "ParameterNode", + "ParameterTreeChild", "ParameterValue", "Policy", "Poverty", diff --git a/policyengine_api/data/v2/models/metadata.py b/policyengine_api/data/v2/models/metadata.py index 5431a6abc..2f841f0e2 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -2,11 +2,12 @@ from datetime import datetime from enum import StrEnum -from typing import TYPE_CHECKING, Any, Optional +from typing import TYPE_CHECKING, Any, Literal, Optional from uuid import UUID import sqlalchemy as sa from sqlmodel import Field, Relationship +from sqlmodel.sql.sqltypes import AutoString from typing_extensions import TypedDict from policyengine_api.data.v2.models.base import ( @@ -464,3 +465,72 @@ class ParameterValue(IdentifiedModel, table=True): parameter: Parameter = Relationship(back_populates="values") policy: Optional["Policy"] = Relationship(back_populates="parameter_values") dynamic: Optional["Dynamic"] = Relationship(back_populates="parameter_values") + + +class ParameterTreeChild(IdentifiedModel, table=True): + """One direct child in a catalog version's parameter tree. + + Catalog publication writes a version's rows from the installed packages' + catalog: for each version it publishes, and for the installed version when + it is stored with catalog format 2 but without a tree. Other versions have + no rows. Every node and parameter of the version is one row under its + parent's path ("" for top-level paths), so listing one level reads one + index. ``position`` orders all children of a parent; ``reform_position`` + orders only those a reform can change and is null for the others. Node rows + count their direct children; parameter rows reference their parameter and + copy its flags. + """ + + __tablename__ = "parameter_tree_children" + __table_args__ = ( + sa.UniqueConstraint( + "tax_benefit_model_version_id", + "path", + name="uq_parameter_tree_children_model_version_path", + ), + sa.Index( + "ix_parameter_tree_children_listing", + "tax_benefit_model_version_id", + "parent_path", + "position", + unique=True, + ), + sa.Index( + "ix_parameter_tree_children_reform_listing", + "tax_benefit_model_version_id", + "parent_path", + "reform_position", + unique=True, + ), + sa.CheckConstraint( + "child_type IN ('node', 'parameter')", + name="ck_parameter_tree_children_child_type", + ), + sa.CheckConstraint( + "(child_type = 'parameter') = (parameter_id IS NOT NULL)", + name="ck_parameter_tree_children_parameter_reference", + ), + ) + + tax_benefit_model_version_id: UUID = Field( + foreign_key="tax_benefit_model_versions.id", + ondelete="CASCADE", + ) + parent_path: str = Field(max_length=512) + path: str = Field(max_length=512) + position: int + label: str = Field(max_length=512) + # The column stays a plain string, kept to these two values by + # ck_parameter_tree_children_child_type. + child_type: Literal["node", "parameter"] = Field(sa_type=AutoString(length=16)) + child_count: int | None = None + reform_position: int | None = None + reform_child_count: int | None = None + parameter_id: UUID | None = Field( + default=None, + foreign_key="parameters.id", + ondelete="CASCADE", + index=True, + ) + economy: bool | None = None + household: bool | None = None diff --git a/policyengine_api/data/v2/privileges.py b/policyengine_api/data/v2/privileges.py index 5014230f0..02671e912 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -116,6 +116,8 @@ def _access( "dataset_versions": _access(catalog_writer=READ), "datasets": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), "parameter_nodes": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), + # Written with each catalog version that publication publishes. + "parameter_tree_children": _access(runtime=READ, catalog_writer=READ_INSERT), # The Public API also inserts a v2 policy's own parameter values. "parameter_values": _access(runtime=READ_INSERT, catalog_writer=READ_INSERT_UPDATE), "parameters": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index 0be11f8fc..f3a343827 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -55,6 +55,7 @@ MetadataRegion, MetadataVariable, ParameterFilter, + ParameterTreeLevel, ParameterValueFilter, ) from policyengine_api.services.v2.policies.types import ( @@ -223,13 +224,14 @@ def list_parameter_children( country_id: str, policyengine_version: str | None = None, *, - parent_path: str = "", + level: ParameterTreeLevel | None = None, offset: int = 0, limit: int = 100, ) -> MetadataPageResult[MetadataParameterChild]: - """Return the nodes and parameters one level below ``parent_path``. + """Return the nodes and parameters one level below the level's parent path. - An empty ``parent_path`` lists the top level of the parameter tree. + An empty parent path lists the top level of the parameter tree; the + level's ``reform_changeable`` keeps only what a reform can change. """ ... diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index a2dab29ee..5993303f8 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/common.py +++ b/policyengine_api/fastapi_routes/v2/metadata/common.py @@ -33,6 +33,7 @@ from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterTreeUnavailableError, ) from policyengine_api.vary_headers import unique_vary_values @@ -60,6 +61,19 @@ } """The extra response of the one metadata route that takes the country in its path.""" +PARAMETER_TREE_ERROR_RESPONSES: dict[int | str, dict[str, Any]] = { + 503: { + "model": V2ErrorResponse, + "description": ( + "The initialized v2 catalog is unavailable, or the catalog version " + "has no parameter tree (publication writes trees only for the " + "versions it publishes and for the installed version with catalog " + "format 2); the message then names the version." + ), + }, +} +"""The parameter tree route's 503, which also covers a version without a tree.""" + ERROR_RESPONSES: dict[int | str, dict[str, Any]] = { 404: { "model": V2ErrorResponse, @@ -348,8 +362,14 @@ def run_metadata_read( MetadataResourceNotFoundError, ) as error: return v2_error_response(404, str(error)) - except (V2ConfigurationError, MetadataCatalogUnavailableError): - return v2_error_response(503, "V2 metadata catalog is unavailable") + except (V2ConfigurationError, MetadataCatalogUnavailableError) as error: + # Only a missing parameter tree has a message written for callers. + message = ( + str(error) + if isinstance(error, ParameterTreeUnavailableError) + else "V2 metadata catalog is unavailable" + ) + return v2_error_response(503, message) except Exception: # noqa: BLE001 # preview must return typed errors return v2_error_response(500, "V2 metadata query failed") finally: diff --git a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py index 51383ccc0..b28148a52 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py +++ b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py @@ -10,6 +10,7 @@ from policyengine_api.fastapi_routes.v2.metadata.common import ( METADATA_RESPONSES, + PARAMETER_TREE_ERROR_RESPONSES, CacheableMetadataRoute, read_resource, take_utc_instant_for_read, @@ -29,6 +30,7 @@ ) from policyengine_api.services.v2.metadata.types import ( ParameterFilter, + ParameterTreeLevel, ParameterValueFilter, ) @@ -76,7 +78,19 @@ def list_parameters( @router.get( "/parameters/children", response_model=MetadataParameterChildPageResponse, - summary="List direct children of one parameter path", + responses=PARAMETER_TREE_ERROR_RESPONSES, + summary="List one level of a catalog version's parameter tree", + description=( + "Lists the direct children of `parent_path` (the top level when " + "empty). With `reform_changeable=true`, only the children a reform " + "can change, sorted by label, with child counts that count only " + "those. Invalid query parameters return 422, including a malformed " + "policyengine_version, an unsupported country, and an unknown or " + "repeated parameter. A catalog version without a tree (publication " + "writes trees only for the versions it publishes and for the " + "installed version with catalog format 2) returns 503 with a " + "message naming it." + ), ) def list_parameter_children( query: ParameterChildrenQueryDependency, @@ -87,7 +101,10 @@ def list_parameter_children( lambda reader: reader.list_parameter_children( query.country_id, query.policyengine_version, - parent_path=query.parent_path, + level=ParameterTreeLevel( + parent_path=query.parent_path, + reform_changeable=query.reform_changeable, + ), offset=query.offset, limit=query.limit, ), diff --git a/policyengine_api/query_parameters.py b/policyengine_api/query_parameters.py index 0d94c8484..8085bd650 100644 --- a/policyengine_api/query_parameters.py +++ b/policyengine_api/query_parameters.py @@ -261,11 +261,22 @@ class RegionCollectionQuery(CatalogPageQuery): class ParameterChildrenQuery(CatalogPageQuery): - """Query contract for a page of one parameter path's direct children.""" + """Query contract for one level of a catalog version's parameter tree.""" parent_path: str = Field( default="", - description="Dotted path whose direct children are listed; empty for the root", + max_length=512, + description=( + "Path of the node whose direct children are listed; empty for the " + "top level. A bracket schedule's brackets are its children." + ), + ) + reform_changeable: bool = Field( + default=False, + description=( + "List only the children a reform can change, sorted by label, with " + "child counts that count only those" + ), ) diff --git a/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_tree.py b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_tree.py index 49f232a8e..6a7eb34d1 100644 --- a/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_tree.py +++ b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_tree.py @@ -1,4 +1,4 @@ -"""Database selections for direct parameter-tree children.""" +"""Database selections for one level of a catalog version's parameter tree.""" from __future__ import annotations @@ -7,158 +7,91 @@ import sqlalchemy as sa from sqlmodel import Session, col -from policyengine_api.data.v2.models import Parameter, ParameterNode +from policyengine_api.data.v2.models import Parameter, ParameterTreeChild from policyengine_api.services.v2.metadata.database_connectors.reads import read_rows if TYPE_CHECKING: from uuid import UUID - from sqlalchemy.orm import Mapped - - -def _escaped_like(value: str) -> str: - return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - - -def _path_segment( - remainder: sa.ColumnElement[str], dialect: str -) -> sa.ColumnElement[str]: - dot_position = ( - sa.func.instr(remainder, ".") - if dialect == "sqlite" - else sa.func.strpos(remainder, ".") - ) - return sa.case( - (dot_position > 0, sa.func.substr(remainder, 1, dot_position - 1)), - else_=remainder, - ) - - -def _child_path( - column: Mapped[str], prefix: str, dialect: str -) -> sa.ColumnElement[str]: - remainder = sa.func.substr(column, len(prefix) + 1) - return sa.literal(prefix) + _path_segment(remainder, dialect) - - -def _direct_child_path( - column: Mapped[str], parent_path: sa.ColumnElement[str], dialect: str -) -> sa.ColumnElement[str]: - remainder = sa.func.substr(column, sa.func.length(parent_path) + 2) - return parent_path + "." + _path_segment(remainder, dialect) - - -def _has_path_prefix( - column: Mapped[str], parent_path: sa.ColumnElement[str] -) -> sa.ColumnElement[bool]: - return ( - sa.func.substr(column, 1, sa.func.length(parent_path) + 1) == parent_path + "." - ) + from policyengine_api.services.v2.metadata.types import ParameterTreeLevel def _parameter_children_statement( *, model_version_id: UUID, - parent_path: str, - dialect: str, + level: ParameterTreeLevel, offset: int, limit: int, ) -> sa.Select[Any]: - prefix = f"{parent_path}." if parent_path else "" - escaped_prefix = _escaped_like(prefix) - node_name = col(ParameterNode.name) - node_model_version_id = col(ParameterNode.tax_benefit_model_version_id) - parameter_name = col(Parameter.name) - parameter_model_version_id = col(Parameter.tax_benefit_model_version_id) - node_child_path = _child_path(node_name, prefix, dialect) - parameter_child_path = _child_path(parameter_name, prefix, dialect) - paths = sa.union( - sa.select(node_child_path.label("path")).where( - node_model_version_id == model_version_id, - node_name.like(f"{escaped_prefix}%", escape="\\"), - ), - sa.select(parameter_child_path.label("path")).where( - parameter_model_version_id == model_version_id, - parameter_name.like(f"{escaped_prefix}%", escape="\\"), - ), - ).subquery() - direct_child_paths = sa.union( - sa.select(_direct_child_path(node_name, paths.c.path, dialect).label("path")) - .where( - node_model_version_id == model_version_id, - _has_path_prefix(node_name, paths.c.path), - ) - .correlate(paths), - sa.select( - _direct_child_path(parameter_name, paths.c.path, dialect).label("path") - ) - .where( - parameter_model_version_id == model_version_id, - _has_path_prefix(parameter_name, paths.c.path), - ) - .correlate(paths), - ).subquery() - direct_child_count = ( - sa.select(sa.func.count()) - .select_from(direct_child_paths) - .correlate(paths) - .scalar_subquery() + position = ( + col(ParameterTreeChild.reform_position) + if level.reform_changeable + else col(ParameterTreeChild.position) + ) + child_count = ( + col(ParameterTreeChild.reform_child_count) + if level.reform_changeable + else col(ParameterTreeChild.child_count) ) - parameter_id = col(Parameter.id) - is_node = sa.or_(direct_child_count > 0, parameter_id.is_(None)) - return ( + statement: sa.Select[Any] = ( sa.select( - paths.c.path, - sa.func.coalesce(col(ParameterNode.label), col(Parameter.label)).label( - "label" - ), - sa.case((is_node, "node"), else_="parameter").label("type"), - sa.case((is_node, direct_child_count), else_=None).label("child_count"), + col(ParameterTreeChild.path).label("path"), + col(ParameterTreeChild.label).label("label"), + col(ParameterTreeChild.child_type).label("type"), + child_count.label("child_count"), Parameter, ) - .select_from( - paths.outerjoin( - ParameterNode, - sa.and_( - node_name == paths.c.path, - node_model_version_id == model_version_id, - ), - ).outerjoin( - Parameter, - sa.and_( - parameter_name == paths.c.path, - parameter_model_version_id == model_version_id, - ), - ) + .select_from(ParameterTreeChild) + .outerjoin( + Parameter, + col(Parameter.id) == col(ParameterTreeChild.parameter_id), + ) + .where( + col(ParameterTreeChild.tax_benefit_model_version_id) == model_version_id, + col(ParameterTreeChild.parent_path) == level.parent_path, ) - .order_by(paths.c.path) - .offset(offset) - .limit(limit + 1) ) + if level.reform_changeable: + statement = statement.where(position.is_not(None)) + return statement.order_by(position).offset(offset).limit(limit + 1) def read_parameter_children( session: Session, *, model_version_id: UUID, - parent_path: str, + level: ParameterTreeLevel, offset: int, limit: int, ) -> list[Any]: - """Return a page of the direct children of parent_path in a model version's tree. - - Each row has the child's path, label, type (node or parameter), child count - and the stored Parameter (None for a node). The session's dialect - selects SQLite or PostgreSQL string functions; limit + 1 rows are fetched so - the caller can tell whether another page exists. + """Return a page of the level's direct children from the stored tree. + + One read of a listing index: every child in tree order, or for a + reform-changeable level only the children a reform can change, in label order, + with child counts that count only those. Each row has the child's path, + label, type (node or parameter), child count and the stored Parameter + (None for a node); limit + 1 rows are fetched so the caller can tell + whether another page exists. """ return read_rows( session, _parameter_children_statement( model_version_id=model_version_id, - parent_path=parent_path, - dialect=session.get_bind().dialect.name, + level=level, offset=offset, limit=limit, ), ) + + +def read_parameter_tree_written(session: Session, *, model_version_id: UUID) -> bool: + """Return whether the version's parameter tree has any stored row.""" + rows = read_rows( + session, + sa.select( + sa.exists().where( + col(ParameterTreeChild.tax_benefit_model_version_id) == model_version_id + ) + ), + ) + return bool(rows[0][0]) diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index 4496e3b30..7b48720fc 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -21,6 +21,7 @@ ) from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_tree import ( # noqa: E501 # a dotted module path cannot be wrapped read_parameter_children, + read_parameter_tree_written, ) from policyengine_api.services.v2.metadata.database_connectors.reads_parameters import ( read_current_law_values_by_parameter_name, @@ -78,9 +79,11 @@ MetadataRegion, MetadataVariable, ParameterFilter, + ParameterTreeLevel, ParameterValueFilter, ) from policyengine_api.services.v2.metadata.validators import ( + ParameterTreeUnavailableError, require_metadata_resource, validate_economy_options, validate_metadata_page, @@ -345,25 +348,40 @@ def list_parameter_children( country_id: str, policyengine_version: str | None = None, *, - parent_path: str = "", + level: ParameterTreeLevel | None = None, offset: int = 0, limit: int = 100, ) -> MetadataPageResult[MetadataParameterChild]: - """Return a page of the direct children of parent_path in the parameter tree. - - An empty parent_path lists the top-level children. A child is a parameter - when it is a leaf with a parameter row, and a node otherwise. + """Return a page of one level of the version's stored parameter tree. + + The level's parent path "" lists the top-level children. A bracket + schedule is one node whose children are its brackets. With the level's + ``reform_changeable``, only the children a reform can change are listed, + sorted by label. Raise ParameterTreeUnavailableError when the version has + no tree: catalog publication writes trees only for the versions it + publishes and for the installed version stored with catalog format 2. """ + selected_level = level if level is not None else ParameterTreeLevel() selected = self._select_paginated_catalog( country_id, policyengine_version, offset=offset, limit=limit ) + session = self._database_session.session rows = read_parameter_children( - self._database_session.session, + session, model_version_id=selected.model_version.id, - parent_path=parent_path, + level=selected_level, offset=offset, limit=limit, ) + if not rows and not read_parameter_tree_written( + session, model_version_id=selected.model_version.id + ): + msg = ( + f"catalog version {selected.policyengine_version} has no parameter " + "tree: catalog publication writes trees only for the versions it " + "publishes and for the installed version with catalog format 2" + ) + raise ParameterTreeUnavailableError(msg) return page_result( selected, metadata_parameter_children(rows), diff --git a/policyengine_api/services/v2/metadata/transformations.py b/policyengine_api/services/v2/metadata/transformations.py index 00c197cac..83bf941ed 100644 --- a/policyengine_api/services/v2/metadata/transformations.py +++ b/policyengine_api/services/v2/metadata/transformations.py @@ -325,13 +325,15 @@ def metadata_region(region: Region) -> MetadataRegion: def metadata_parameter_children(rows: list[Any]) -> list[MetadataParameterChild]: """Build parameter-tree children from read_parameter_children rows. - Parameter leaves carry the same summary as the parameter routes, and a child - with no label is labelled with the last segment of its path. + Parameter leaves carry the same summary as the parameter routes. Every + stored child has a label in the form v3 shows it: its stored label with + underscores as spaces and its first letter upper-cased, "Bracket N" for a + bracket, or the last part of its name in that form. """ return [ MetadataParameterChild( path=row.path, - label=row.label or row.path.rsplit(".", 1)[-1], + label=row.label, type=row.type, child_count=row.child_count, parameter=( diff --git a/policyengine_api/services/v2/metadata/types.py b/policyengine_api/services/v2/metadata/types.py index 63a77944e..ce362d58f 100644 --- a/policyengine_api/services/v2/metadata/types.py +++ b/policyengine_api/services/v2/metadata/types.py @@ -66,6 +66,19 @@ class ParameterNodeLabelRow(NamedTuple): label: str | None +@dataclass(frozen=True) +class ParameterTreeLevel: + """Which level of a catalog version's parameter tree a listing reads. + + ``parent_path`` names the node whose direct children are listed ("" for + the top level); ``reform_changeable`` keeps only the children a reform can + change, sorted by label, with child counts that count only those. + """ + + parent_path: str = "" + reform_changeable: bool = False + + class StrictResponseModel(BaseModel): """Base for metadata results that rejects unknown fields and non-finite numbers.""" diff --git a/policyengine_api/services/v2/metadata/validators.py b/policyengine_api/services/v2/metadata/validators.py index bdc59c8d2..62e62a802 100644 --- a/policyengine_api/services/v2/metadata/validators.py +++ b/policyengine_api/services/v2/metadata/validators.py @@ -24,6 +24,16 @@ class InvalidMetadataPageError(ValueError): """Raised when collection pagination is outside the documented bounds.""" +class ParameterTreeUnavailableError(MetadataCatalogUnavailableError): + """Raised when a selected catalog version has no parameter tree. + + Catalog publication writes a tree, from the installed packages, for each + version it publishes and for the installed version when it is stored with + catalog format 2 but without one; any other version has none. The message + names the version and is safe to return to the caller. + """ + + def validate_metadata_page(offset: int, limit: int) -> tuple[int, int]: """Return offset and limit unchanged, or raise InvalidMetadataPageError. diff --git a/rules/registry.toml b/rules/registry.toml index 24faf8334..2f24327dc 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -669,9 +669,9 @@ test = "tests/unit/v2/test_privilege_manifest.py::test_rule_proof_runtime_privil [[rule]] id = "api.v2.runtime-privileges-tested-as-runtime-role" category = "migration" -statement = "The cross-database integration tests run the v2 user services as policyengine_v2_runtime and the comparison-run services as policyengine_stage12_, holding only their declared privileges, so a statement those privileges don't allow fails CI." +statement = "The cross-database integration tests run the v2 user services and the parameter tree read as policyengine_v2_runtime, catalog publication as policyengine_v2_catalog_writer, and the comparison-run services as policyengine_stage12_, holding only their declared privileges, so a statement those privileges don't allow fails CI." rationale = "docs/engineering/skills/alembic-migrations.md#table-privileges" -scope = ["tests/integration/conftest.py", "tests/integration/test_v2_user_identity_persistence.py", "tests/integration/test_v2_saved_policy_view.py", "tests/integration/test_v2_stage12_comparison_persistence.py"] +scope = ["tests/integration/conftest.py", "tests/integration/test_v2_user_identity_persistence.py", "tests/integration/test_v2_saved_policy_view.py", "tests/integration/test_v2_stage12_comparison_persistence.py", "tests/integration/test_v2_metadata_routes.py", "tests/integration/test_v2_catalog_publication.py"] tier = "mechanical" status = "enforced" autofix = false diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 89837594a..c71100c40 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1547,3 +1547,116 @@ def test_catalog_format_2_columns_leave_earlier_versions_empty() -> None: ) command.upgrade(config, "head") engine.dispose() + + +PARAMETER_TREE_PARENT_REVISION = "f5c1d9b4685b" + + +def test_parameter_tree_table_is_created_empty_and_dropped_without_data_loss() -> None: + """The parameter tree revision creates an empty table and downgrades cleanly. + + The revision writes no rows: catalog publication writes each version's + tree. A tree row is deleted with its version or its parameter, and + downgrading drops the table while every catalog row stays. + """ + config = _config() + engine = create_engine(_disposable_url()) + try: + command.downgrade(config, PARAMETER_TREE_PARENT_REVISION) + assert "parameter_tree_children" not in inspect(engine).get_table_names() + with engine.begin() as connection: + version_id = _catalog_version_published_before_format_2(connection) + + command.upgrade(config, "head") + + indexes = { + index["name"]: (index["column_names"], index["unique"]) + for index in inspect(engine).get_indexes("parameter_tree_children") + } + assert indexes["ix_parameter_tree_children_listing"] == ( + ["tax_benefit_model_version_id", "parent_path", "position"], + True, + ) + assert indexes["ix_parameter_tree_children_reform_listing"] == ( + ["tax_benefit_model_version_id", "parent_path", "reform_position"], + True, + ) + with engine.begin() as connection: + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_tree_children") + ).scalar_one() + == 0 + ) + parameter_id = connection.execute( + text( + "SELECT id FROM parameters " + "WHERE tax_benefit_model_version_id = :version_id" + ), + {"version_id": version_id}, + ).scalar_one() + for path, parent, position, child_type, child_count, reference in ( + ("gov", "", 0, "node", 1, None), + ("gov.probe", "gov", 0, "node", 1, None), + ("gov.probe.rate", "gov.probe", 0, "parameter", None, parameter_id), + ): + connection.execute( + text( + "INSERT INTO parameter_tree_children (id, " + "tax_benefit_model_version_id, parent_path, path, position, " + "label, child_type, child_count, parameter_id) VALUES " + "(:id, :version_id, :parent, :path, :position, :path, " + ":child_type, :child_count, :parameter_id)" + ), + { + "id": uuid4(), + "version_id": version_id, + "parent": parent, + "path": path, + "position": position, + "child_type": child_type, + "child_count": child_count, + "parameter_id": reference, + }, + ) + with pytest.raises(sa.exc.IntegrityError), connection.begin_nested(): + connection.execute( + text( + "INSERT INTO parameter_tree_children (id, " + "tax_benefit_model_version_id, parent_path, path, position, " + "label, child_type) VALUES (:id, :version_id, 'gov', " + "'gov.other', 0, 'Other', 'node')" + ), + {"id": uuid4(), "version_id": version_id}, + ) + connection.execute( + text("DELETE FROM parameters WHERE id = :parameter_id"), + {"parameter_id": parameter_id}, + ) + assert connection.execute( + text("SELECT path FROM parameter_tree_children ORDER BY path") + ).scalars().all() == ["gov", "gov.probe"] + + command.downgrade(config, PARAMETER_TREE_PARENT_REVISION) + + assert "parameter_tree_children" not in inspect(engine).get_table_names() + with engine.connect() as connection: + assert ( + connection.execute( + text( + "SELECT count(*) FROM variables " + "WHERE tax_benefit_model_version_id = :version_id" + ), + {"version_id": version_id}, + ).scalar_one() + == 1 + ) + finally: + command.upgrade(config, "head") + with engine.begin() as connection: + connection.execute( + text( + "DELETE FROM tax_benefit_models WHERE name = 'catalog-format-probe'" + ) + ) + engine.dispose() diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index a5ba64d1e..60077f905 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -6,6 +6,7 @@ from concurrent.futures import ThreadPoolExecutor from dataclasses import replace from pathlib import Path +from uuid import uuid4 import pytest from alembic import command @@ -33,6 +34,7 @@ TaxBenefitModel, TaxBenefitModelVersion, ) +from policyengine_api.data.v2.privileges import CATALOG_WRITER_ROLE from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL from tests.fixtures.v2_catalog import ( POLICYENGINE_VERSION, @@ -798,3 +800,265 @@ def test_republication_detects_a_changed_enum_label_parameter_path_unit_or_perio ) assert _stored_us_variables(publication_engine) == stored + + +def _stored_tree( + engine: Engine, + model_name: str, + version: str, + *, + parameter_column: str = "parameter.name", +) -> list[tuple]: + """Return a stored version's tree rows; each parameter by name by default.""" + with engine.connect() as connection: + return [ + tuple(row) + for row in connection.execute( + text( + "SELECT tree.parent_path, tree.path, tree.position, tree.label, " + "tree.child_type, tree.child_count, tree.reform_position, " + f"tree.reform_child_count, {parameter_column}, tree.economy, " + "tree.household " + "FROM parameter_tree_children AS tree " + "JOIN tax_benefit_model_versions AS version " + "ON version.id = tree.tax_benefit_model_version_id " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "LEFT JOIN parameters AS parameter " + "ON parameter.id = tree.parameter_id " + "WHERE model.name = :model_name AND version.version = :version " + "ORDER BY tree.parent_path, tree.position" + ), + {"model_name": model_name, "version": version}, + ) + ] + + +def _delete_tree(engine: Engine, version: str) -> None: + """Make the stored versions of one .py version look stored before the tree.""" + with engine.begin() as connection: + connection.execute( + text( + "DELETE FROM parameter_tree_children AS tree " + "USING tax_benefit_model_versions AS version " + "WHERE version.id = tree.tax_benefit_model_version_id " + "AND version.version = :version" + ), + {"version": version}, + ) + + +FIXTURE_TREE = [ + ("", "gov", 0, "Gov", "node", 1, 0, 1, None, None, None), + ("gov", "gov.example", 0, "Example policy", "node", 1, 0, 1, None, None, None), + ( + "gov.example", + "gov.example.rate", + 0, + "Example rate", + "parameter", + None, + 0, + None, + "gov.example.rate", + True, + False, + ), +] + + +def test_publication_writes_each_new_version_parameter_tree( + publication_engine: Engine, +) -> None: + catalog = normalized_catalog() + + evidence = publish_catalog(publication_engine, catalog) + + for country in catalog.countries: + assert ( + _stored_tree( + publication_engine, + country.model.name, + country.model_version.version, + ) + == FIXTURE_TREE + ) + assert evidence.parameter_trees_written == tuple( + (country.model.name, country.model_version.version, len(FIXTURE_TREE)) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ) + + +def _stored_tree_with_parameter_ids( + engine: Engine, model_name: str, version: str +) -> list[tuple]: + return _stored_tree( + engine, model_name, version, parameter_column="tree.parameter_id" + ) + + +def _with_other_parameter_ids(catalog): + """Return the catalog with a new ID for every parameter, and its values.""" + countries = [] + for country in catalog.countries: + parameters = [] + for parameter in country.parameters: + parameter_id = uuid4() + parameters.append( + replace( + parameter, + id=parameter_id, + values=tuple( + replace(value, id=uuid4(), parameter_id=parameter_id) + for value in parameter.values + ), + ) + ) + countries.append(replace(country, parameters=tuple(parameters))) + return replace(catalog, countries=tuple(countries)) + + +def test_publication_gives_the_installed_stored_version_its_tree( + publication_engine: Engine, +) -> None: + """The installed version, stored by a release without the tree, gets one. + + It is built from the installed packages' catalog, the same as a fresh + publication's; writing it again changes nothing. The version is first + stored with parameter IDs that differ from the catalog's, so the rebuilt + tree must reference the stored rows' IDs. + """ + catalog = normalized_catalog() + current_version = catalog.country("us").model_version.version + publish_catalog(publication_engine, _with_other_parameter_ids(catalog)) + fresh = { + country.model.name: _stored_tree_with_parameter_ids( + publication_engine, country.model.name, current_version + ) + for country in catalog.countries + } + _delete_tree(publication_engine, current_version) + before_ids = _identifiers(publication_engine) + + evidence = publish_catalog(publication_engine, catalog) + again = publish_catalog(publication_engine, catalog) + + assert evidence.parameter_trees_written == tuple( + (country.model.name, current_version, len(FIXTURE_TREE)) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ) + assert again.parameter_trees_written == () + for country in catalog.countries: + assert ( + _stored_tree_with_parameter_ids( + publication_engine, country.model.name, current_version + ) + == fresh[country.model.name] + ) + assert _identifiers(publication_engine) == before_ids + catalog_parameter_ids = { + parameter.id + for country in catalog.countries + for parameter in country.parameters + } + for rows in fresh.values(): + assert {row[8] for row in rows if row[8] is not None}.isdisjoint( + catalog_parameter_ids + ) + + +def test_an_installed_version_stored_before_format_2_gets_no_tree( + publication_engine: Engine, +) -> None: + """A version stored before catalog format 2 stores no flags, so no tree. + + Its tree would carry the installed catalog's flags, which its own + parameter rows do not store; it keeps returning the clear 503 instead, and + publication still succeeds. + """ + catalog = normalized_catalog() + current_version = catalog.country("us").model_version.version + publish_catalog(publication_engine, catalog) + _clear_format_2_fields(publication_engine) + _delete_tree(publication_engine, current_version) + + evidence = publish_catalog(publication_engine, catalog) + + assert evidence.as_dict()["outcome"] == "ok" + assert evidence.parameter_trees_written == () + for country in catalog.countries: + assert ( + _stored_tree(publication_engine, country.model.name, current_version) == [] + ) + + +def test_publication_writes_no_tree_for_another_version_stored_without_one( + publication_engine: Engine, +) -> None: + """A stored version that is not the installed one keeps having no tree. + + Writing it would derive rows from what an earlier release stored, which + the migration rules leave to a human migration decision. + """ + older = normalized_catalog(policyengine_version="1.0.0") + catalog = normalized_catalog() + older_version = older.country("us").model_version.version + publish_catalog(publication_engine, older) + _delete_tree(publication_engine, older_version) + + evidence = publish_catalog(publication_engine, catalog) + again = publish_catalog(publication_engine, catalog) + + assert {version for _model, version, _rows in evidence.parameter_trees_written} == { + catalog.country("us").model_version.version + } + assert again.parameter_trees_written == () + for country in catalog.countries: + assert _stored_tree(publication_engine, country.model.name, older_version) == [] + + +def test_publication_fails_when_a_published_version_is_left_without_a_tree( + publication_engine: Engine, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + publication_module, + "write_parameter_trees", + lambda _connection, _countries: (), + ) + + with pytest.raises(CatalogPublicationError, match="has no parameter tree"): + publish_catalog(publication_engine, normalized_catalog()) + + assert _counts(publication_engine)["tax_benefit_model_versions"] == 0 + + +def test_the_catalog_writer_role_publishes_the_parameter_tree( + publication_engine: Engine, + managed_v2_roles, + v2_engine_as_role, +) -> None: + """Publication as the catalog writer, with only its declared privileges. + + It writes each published version's tree, which needs the role's INSERT + grant on the tree table. + """ + writer_engine = v2_engine_as_role(CATALOG_WRITER_ROLE, poolclass=NullPool) + catalog = normalized_catalog() + try: + evidence = publish_catalog(writer_engine, catalog) + finally: + writer_engine.dispose() + + assert evidence.parameter_trees_written == tuple( + (country.model.name, country.model_version.version, len(FIXTURE_TREE)) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ) + for country in catalog.countries: + assert ( + _stored_tree( + publication_engine, + country.model.name, + country.model_version.version, + ) + == FIXTURE_TREE + ) diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index 7f479d183..d3a248cec 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -5,6 +5,7 @@ import os from datetime import UTC, datetime from pathlib import Path +from types import SimpleNamespace from uuid import uuid4 import pytest @@ -18,6 +19,7 @@ from sqlmodel import Session, select from policyengine_api.asgi_factory import create_asgi_app +from policyengine_api.data.v2.catalog.extraction import extract_catalog from policyengine_api.data.v2.catalog.publication import publish_catalog from policyengine_api.data.v2.models import ( Dataset, @@ -37,7 +39,15 @@ MetadataDatabaseSession, ) from policyengine_api.services.v2.metadata.services import V2MetadataService -from tests.fixtures.v2_catalog import POLICYENGINE_VERSION, normalized_catalog +from policyengine_api.services.v2.metadata.types import ParameterTreeLevel +from tests.fixtures.v2_catalog import ( + POLICYENGINE_VERSION, + bundle, + installed_version, + normalized_catalog, + region_references, + source_models, +) REPO = Path(__file__).parents[2] DISPOSABLE_DATABASE = "policyengine_v2_alembic_test" @@ -723,3 +733,319 @@ def test_postgres_current_values_and_their_lifetime_use_one_instant( expected_values ) assert response.headers["cache-control"] == expected_cache_control + + +SCHEDULE = "gov.hmrc.income_tax.rates.uk" + + +def _catalog_with_a_bracket_schedule(): + """Return the fixture catalog with a UK-style income tax schedule added. + + The schedule has a stored node row; a second schedule has none. + """ + models = source_models() + uk = models["uk"] + nodes = [ + SimpleNamespace(name=SCHEDULE, label="Income Tax main rates", description=None) + ] + parameters = [ + SimpleNamespace( + name=name, + label=label, + description=None, + data_type=float, + unit="/1", + economy=economy, + household=household, + parameter_values=[ + SimpleNamespace( + value=0.2, + start_date=datetime(2025, 1, 1, tzinfo=UTC), + end_date=None, + ) + ], + ) + for name, label, economy, household in ( + (f"{SCHEDULE}[0].rate", "Basic rate", True, True), + (f"{SCHEDULE}[0].threshold", "Basic rate threshold", True, True), + (f"{SCHEDULE}[1].rate", "Higher rate", True, True), + (f"{SCHEDULE}[1].threshold", "Higher rate threshold", False, False), + ("gov.hmrc.income_tax.rates.dividends[0].rate", None, True, False), + ) + ] + uk.parameter_nodes.extend(nodes) + uk.parameters.extend(parameters) + uk.parameter_nodes_by_name.update({node.name: node for node in nodes}) + uk.parameters_by_name.update( + {parameter.name: parameter for parameter in parameters} + ) + return extract_catalog( + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + region_references=region_references(), + ) + + +def _items(response) -> list[tuple]: + assert response.status_code == 200, response.text + return [ + (item["path"], item["label"], item["type"], item["child_count"]) + for item in response.json()["result"]["items"] + ] + + +def test_postgres_parameter_tree_nests_schedules_and_filters_reform_changes( + published_engine: Engine, +) -> None: + with published_engine.begin() as connection: + connection.execute(text("TRUNCATE tax_benefit_models CASCADE")) + publish_catalog(published_engine, _catalog_with_a_bracket_schedule()) + client = _client(published_engine) + query = {"country_id": "uk"} + + rates = client.get( + "/v2/parameters/children", + params={**query, "parent_path": "gov.hmrc.income_tax.rates"}, + ) + schedule = client.get( + "/v2/parameters/children", params={**query, "parent_path": SCHEDULE} + ) + bracket = client.get( + "/v2/parameters/children", + params={**query, "parent_path": f"{SCHEDULE}[1]"}, + ) + reform_bracket = client.get( + "/v2/parameters/children", + params={ + **query, + "parent_path": f"{SCHEDULE}[1]", + "reform_changeable": "true", + }, + ) + reform_rates = client.get( + "/v2/parameters/children", + params={ + **query, + "parent_path": "gov.hmrc.income_tax.rates", + "reform_changeable": "true", + }, + ) + + assert _items(rates) == [ + ("gov.hmrc.income_tax.rates.dividends", "Dividends", "node", 1), + (SCHEDULE, "Income Tax main rates", "node", 2), + ] + assert _items(schedule) == [ + (f"{SCHEDULE}[0]", "Bracket 1", "node", 2), + (f"{SCHEDULE}[1]", "Bracket 2", "node", 2), + ] + assert _items(bracket) == [ + (f"{SCHEDULE}[1].rate", "Higher rate", "parameter", None), + (f"{SCHEDULE}[1].threshold", "Higher rate threshold", "parameter", None), + ] + assert _items(reform_bracket) == [ + (f"{SCHEDULE}[1].rate", "Higher rate", "parameter", None), + ] + assert _items(reform_rates) == [ + ("gov.hmrc.income_tax.rates.dividends", "Dividends", "node", 1), + (SCHEDULE, "Income Tax main rates", "node", 2), + ] + leaf = bracket.json()["result"]["items"][0]["parameter"] + assert leaf["name"] == f"{SCHEDULE}[1].rate" + + +def test_postgres_parameter_tree_reads_as_the_runtime_role( + published_engine: Engine, + managed_v2_roles, + v2_engine_as_role, +) -> None: + """The Public API's runtime role can read the tree with its declared privileges.""" + runtime_engine = v2_engine_as_role(managed_v2_roles.runtime, poolclass=NullPool) + try: + with Session(runtime_engine) as session: + result = V2MetadataService( + MetadataDatabaseSession(session), + running_policyengine_version=POLICYENGINE_VERSION, + ).list_parameter_children( + "us", + level=ParameterTreeLevel(parent_path="gov", reform_changeable=True), + ) + finally: + runtime_engine.dispose() + + assert [(item.path, item.child_count) for item in result.items] == [ + ("gov.example", 1) + ] + + +def _catalog_shaped_like_policyengine_py_output(): + """Return the fixture catalog with US records shaped like PolicyEngine.py's. + + Bracket parameters carry the labels PolicyEngine.py generates, and a + breakdown parameter its parent's label with the key. One schedule has no + node row, as the locked PolicyEngine.py publishes; the other has one with + its label, as the release that publishes schedules will. + """ + models = source_models() + us = models["us"] + nodes = [ + SimpleNamespace( + name="gov.irs.deductions.standard.amount", + label="Standard deduction", + description=None, + ), + SimpleNamespace( + name="gov.aca.age_curves.ak", + label="ACA premium age curve factors - AK", + description=None, + ), + ] + labelled_parameters = [ + (f"gov.aca.age_curves.{state}[{index}].{field}", f"{label} ({tier} {field})") + for state, label in ( + ("al", "ACA premium age curve factors - AL"), + ("ak", "ACA premium age curve factors - AK"), + ) + for index, tier in ((0, "tier 1"), (1, "tier 2")) + for field in ("amount", "threshold") + ] + [ + ("gov.irs.deductions.standard.amount.SINGLE", "Standard deduction (Single)"), + ("gov.irs.deductions.standard.amount.JOINT", "Standard deduction (Joint)"), + ] + parameters = [ + SimpleNamespace( + name=name, + label=label, + description=None, + data_type=float, + unit="currency-USD", + economy=True, + household=True, + parameter_values=[ + SimpleNamespace( + value=1.0, + start_date=datetime(2025, 1, 1, tzinfo=UTC), + end_date=None, + ) + ], + ) + for name, label in labelled_parameters + ] + us.parameter_nodes.extend(nodes) + us.parameters.extend(parameters) + us.parameter_nodes_by_name.update({node.name: node for node in nodes}) + us.parameters_by_name.update( + {parameter.name: parameter for parameter in parameters} + ) + return extract_catalog( + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + region_references=region_references(), + ) + + +def test_postgres_parameter_tree_from_records_shaped_like_policyengine_py( + published_engine: Engine, +) -> None: + with published_engine.begin() as connection: + connection.execute(text("TRUNCATE tax_benefit_models CASCADE")) + publish_catalog(published_engine, _catalog_shaped_like_policyengine_py_output()) + client = _client(published_engine) + + def level(parent_path: str) -> list[tuple]: + return _items( + client.get( + "/v2/parameters/children", + params={"country_id": "us", "parent_path": parent_path}, + ) + ) + + assert level("gov.aca.age_curves") == [ + ("gov.aca.age_curves.ak", "ACA premium age curve factors - AK", "node", 2), + ("gov.aca.age_curves.al", "Al", "node", 2), + ] + assert level("gov.aca.age_curves.al") == [ + ("gov.aca.age_curves.al[0]", "Bracket 1", "node", 2), + ("gov.aca.age_curves.al[1]", "Bracket 2", "node", 2), + ] + assert level("gov.aca.age_curves.al[1]") == [ + ( + "gov.aca.age_curves.al[1].amount", + "ACA premium age curve factors - AL (tier 2 amount)", + "parameter", + None, + ), + ( + "gov.aca.age_curves.al[1].threshold", + "ACA premium age curve factors - AL (tier 2 threshold)", + "parameter", + None, + ), + ] + assert level("gov.irs.deductions.standard") == [ + ("gov.irs.deductions.standard.amount", "Standard deduction", "node", 2), + ] + assert level("gov.irs.deductions.standard.amount") == [ + ( + "gov.irs.deductions.standard.amount.JOINT", + "Standard deduction (Joint)", + "parameter", + None, + ), + ( + "gov.irs.deductions.standard.amount.SINGLE", + "Standard deduction (Single)", + "parameter", + None, + ), + ] + + +def test_postgres_a_version_stored_before_format_2_returns_the_tree_503( + published_engine: Engine, +) -> None: + """A version stored before catalog format 2 gets no tree on republication. + + Its parameter tree request returns 503 with the message naming it. + """ + with published_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, " + "unit = NULL, definition_period = NULL" + ) + ) + connection.execute( + text("UPDATE parameters SET economy = NULL, household = NULL") + ) + connection.execute(text("DELETE FROM parameter_tree_children")) + evidence = publish_catalog(published_engine, normalized_catalog()) + + response = _client(published_engine).get( + "/v2/parameters/children", + params={"country_id": "us", "policyengine_version": POLICYENGINE_VERSION}, + ) + + assert evidence.parameter_trees_written == () + assert response.status_code == 503 + assert response.json() == { + "status": "error", + "message": ( + f"catalog version {POLICYENGINE_VERSION} has no parameter tree: " + "catalog publication writes trees only for the versions it publishes " + "and for the installed version with catalog format 2" + ), + } + assert "cache-control" not in response.headers diff --git a/tests/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index d6eb45ae0..4bdc47718 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -44,6 +44,7 @@ "legacy_policy_mappings", "legacy_user_mappings", "legacy_user_policy_mappings", + "parameter_tree_children", "stage12_evaluation_reports", "stage12_evaluation_simulations", "user_identities", @@ -66,6 +67,7 @@ "provenance FROM user_policy_associations" ) SAVED_POLICY_VIEW_REMOVAL = "DROP VIEW user_policies" +CATALOG_FORMAT_2_REVISION = "f5c1d9b4685b" DISPOSABLE_URL = ( "postgresql+psycopg://postgres:test-password@127.0.0.1:5432/" f"{DISPOSABLE_DATABASE_NAME}" @@ -247,6 +249,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, + CATALOG_FORMAT_2_REVISION, SAVED_POLICY_RENAME_REVISION, SAVED_POLICY_RENAME_PARENT_REVISION, "91af4fc8d8c1", @@ -861,3 +864,46 @@ def test_catalog_format_2_revision_adds_only_nullable_columns() -> None: assert "op.execute(" not in body assert "op.bulk_insert(" not in body assert "server_default" not in body + + +def test_parameter_tree_revision_creates_only_the_children_table() -> None: + """The revision after catalog format 2 creates the parameter tree's table. + + It is generated, creates the table with its two listing indexes and its + parameter index, drops them on downgrade, and writes no data: rows are + written by catalog publication. + """ + script = ScriptDirectory.from_config(Config(str(REPO / "alembic-v2.ini"))) + children = [ + revision + for revision in script.walk_revisions() + if revision.down_revision == CATALOG_FORMAT_2_REVISION + ] + assert len(children) == 1 + assert children[0].revision == V2_ALEMBIC_HEAD_REVISION + 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 + ) + assert upgrade.count("op.create_table(") == 1 + assert '"parameter_tree_children"' in upgrade + assert upgrade.count("op.create_index(") == 3 + assert downgrade.count("op.drop_index(") == 3 + assert downgrade.count("op.drop_table(") == 1 + for index_name in ( + "ix_parameter_tree_children_listing", + "ix_parameter_tree_children_reform_listing", + "ix_parameter_tree_children_parameter_id", + ): + assert index_name in upgrade + assert index_name in downgrade + for body in (upgrade, downgrade): + assert "op.execute(" not in body + assert "op.bulk_insert(" not in body + assert "op.add_column(" not in body diff --git a/tests/unit/v2/test_catalog_publication.py b/tests/unit/v2/test_catalog_publication.py index 47cb262ba..377c8c230 100644 --- a/tests/unit/v2/test_catalog_publication.py +++ b/tests/unit/v2/test_catalog_publication.py @@ -3,9 +3,11 @@ from __future__ import annotations import logging +import re from io import StringIO from pathlib import Path from unittest.mock import patch +from uuid import uuid4 import pytest import sqlalchemy as sa @@ -20,6 +22,12 @@ publication_reconciliation, publication_staging, ) +from policyengine_api.data.v2.catalog.publication_parameter_tree import ( + catalog_parameter_tree, +) +from policyengine_api.data.v2.catalog.publication_types import ( + CatalogPublicationError, +) from policyengine_api.data.v2.migration_target import V2_ALEMBIC_HEAD_REVISION from policyengine_api.data.v2.models import ( Dataset, @@ -284,6 +292,7 @@ def test_completion_evidence_contains_only_reviewed_non_secret_fields() -> None: elapsed_seconds=2.3456, omitted_region_summaries=(("us", "place", 333),), kept_region_summaries=(("uk", "6.7.5", 5, 1_015),), + parameter_trees_written=(("policyengine-uk", "6.7.5", 2_430),), ).as_dict() assert evidence == { @@ -305,6 +314,13 @@ def test_completion_evidence_contains_only_reviewed_non_secret_fields() -> None: "staged_regions": 1_015, } ], + "parameter_trees_written": [ + { + "model": "policyengine-uk", + "version": "6.7.5", + "rows": 2_430, + } + ], "elapsed_seconds": 2.346, } assert ( @@ -389,3 +405,31 @@ def test_matching_kept_regions_log_nothing( caplog: pytest.LogCaptureFixture, ) -> None: assert _kept_region_records(monkeypatch, caplog, (0, 0)) == [] + + +def test_a_catalog_tree_needs_a_stored_row_for_every_parameter() -> None: + """Each tree parameter references the stored parameter of the same name. + + The stored IDs differ from the catalog's, so the tree must take them from + the stored rows, not from the catalog records. + """ + country = normalized_catalog().country("us") + stored_ids = {parameter.name: uuid4() for parameter in country.parameters} + + rows = catalog_parameter_tree(country, stored_ids) + leaves = [row for row in rows if row.child_type == "parameter"] + + assert [(row.path, row.parameter_id) for row in leaves] == [ + (parameter.name, stored_ids[parameter.name]) for parameter in country.parameters + ] + assert {row.parameter_id for row in leaves}.isdisjoint( + parameter.id for parameter in country.parameters + ) + with pytest.raises( + CatalogPublicationError, + match=re.escape( + f"the stored us catalog version {country.model_version.version} " + "lacks the parameter gov.example.rate" + ), + ): + catalog_parameter_tree(country, {}) diff --git a/tests/unit/v2/test_import_side_effects.py b/tests/unit/v2/test_import_side_effects.py index 7b753004d..af6913897 100644 --- a/tests/unit/v2/test_import_side_effects.py +++ b/tests/unit/v2/test_import_side_effects.py @@ -69,7 +69,7 @@ def reject_ddl(*args, **kwargs): import sys after = set(pathlib.Path.cwd().iterdir()) assert before == after -assert len(V2_METADATA.tables) == 39 +assert len(V2_METADATA.tables) == 40 assert "policyengine_api.data.v2.catalog.initialization" not in sys.modules assert "policyengine_api.data.v2.policy_migration_qualification" not in sys.modules assert "policyengine_api.data.v2.household_migration_qualification" not in sys.modules diff --git a/tests/unit/v2/test_metadata_routes.py b/tests/unit/v2/test_metadata_routes.py index 037b2952f..e2b7ff1ff 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -54,10 +54,12 @@ MetadataTimePeriodOption, MetadataVariable, ParameterFilter, + ParameterTreeLevel, ) from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterTreeUnavailableError, ) @@ -1473,3 +1475,183 @@ def test_openapi_requires_every_catalog_format_2_field_as_nullable() -> None: assert set(schema["properties"]) <= set(schema["required"]), schema_name for field in fields: assert {"type": "null"} in schema["properties"][field]["anyOf"], field + + +def test_parameter_children_route_forwards_its_level_and_page() -> None: + results, _ids = _resource_results() + reader = ResourceReader(results) + + response = _client(lambda: reader).get( + "/v2/parameters/children", + params={ + "country_id": "UK", + "policyengine_version": "4.19.0", + "parent_path": "gov.hmrc.income_tax.rates.uk", + "reform_changeable": "true", + "offset": 10, + "limit": 20, + }, + ) + + assert response.status_code == 200 + assert reader.calls == [ + ( + "list_parameter_children", + ("uk", "4.19.0"), + { + "level": ParameterTreeLevel( + parent_path="gov.hmrc.income_tax.rates.uk", + reform_changeable=True, + ), + "offset": 10, + "limit": 20, + }, + ) + ] + + +def test_parameter_children_route_defaults_to_the_full_top_level() -> None: + results, _ids = _resource_results() + reader = ResourceReader(results) + + response = _client(lambda: reader).get( + "/v2/parameters/children", params={"country_id": "us"} + ) + + assert response.status_code == 200 + assert reader.calls == [ + ( + "list_parameter_children", + ("us", None), + {"level": ParameterTreeLevel(), "offset": 0, "limit": 100}, + ) + ] + + +@pytest.mark.parametrize( + "query", + [ + "country_id=us&parentpath=gov", + "country_id=us&parent_path=gov&parent_path=gov.irs", + "country_id=us&reform_changeable=yes-please", + "country_id=us&limit=501", + "country_id=ca", + "parent_path=gov", + "country_id=us&parent_path=" + "x" * 513, + ], +) +def test_parameter_children_route_rejects_invalid_queries(query: str) -> None: + calls = [] + + def factory(): + calls.append("called") + results, _ids = _resource_results() + return ResourceReader(results) + + response = _client(factory).get(f"/v2/parameters/children?{query}") + + assert response.status_code == 422 + assert response.json() == { + "status": "error", + "message": "Invalid API v2 request", + } + assert calls == [] + + +def test_parameter_children_route_reports_an_unwritten_tree_as_unavailable() -> None: + results, _ids = _resource_results() + message = ( + "catalog version 6.7.5 has no parameter tree: catalog publication writes " + "trees only for the versions it publishes and for the installed version " + "with catalog format 2" + ) + reader = ResourceReader(results, error=ParameterTreeUnavailableError(message)) + + response = _client(lambda: reader).get( + "/v2/parameters/children", params={"country_id": "us"} + ) + + assert response.status_code == 503 + assert response.json() == {"status": "error", "message": message} + + +def test_parameter_children_openapi_declares_the_query_contract() -> None: + results, _ids = _resource_results() + schema = _client(lambda: ResourceReader(results)).get("/v2/openapi.json").json() + + parameters = { + parameter["name"]: parameter + for parameter in schema["paths"]["/v2/parameters/children"]["get"]["parameters"] + } + + assert set(parameters) == { + "country_id", + "policyengine_version", + "offset", + "limit", + "parent_path", + "reform_changeable", + } + assert parameters["country_id"]["required"] is True + assert parameters["parent_path"]["schema"]["default"] == "" + assert parameters["parent_path"]["schema"]["maxLength"] == 512 + assert parameters["reform_changeable"]["schema"]["type"] == "boolean" + assert parameters["reform_changeable"]["schema"]["default"] is False + assert parameters["limit"]["schema"]["maximum"] == 500 + description = schema["paths"]["/v2/parameters/children"]["get"]["description"] + assert "422" in description + assert "malformed policyengine_version" in description + assert "503" in description + + +def test_parameter_children_openapi_declares_exactly_its_error_responses() -> None: + results, _ids = _resource_results() + schema = _client(lambda: ResourceReader(results)).get("/v2/openapi.json").json() + responses = schema["paths"]["/v2/parameters/children"]["get"]["responses"] + parameter_responses = schema["paths"]["/v2/parameters"]["get"]["responses"] + + assert set(responses) == {"200", "304", "404", "405", "422", "500", "503"} + assert "PolicyEngine.py version" in responses["422"]["description"] + assert "parameter tree" in responses["503"]["description"] + assert "parameter tree" not in parameter_responses["503"]["description"] + + +def test_parameter_tree_responses_are_cached_like_other_metadata() -> None: + """A version's tree never changes, so it is cached as other metadata is.""" + results, _ids = _resource_results() + client = _client(lambda: ResourceReader(results)) + path = "/v2/parameters/children?country_id=us&parent_path=gov.example" + + running = client.get(path, headers={"Origin": "https://app.policyengine.org"}) + named = client.get(f"{path}&policyengine_version=4.20.3") + revalidated = client.get( + f"{path}&policyengine_version=4.20.3", + headers={"If-None-Match": named.headers["etag"]}, + ) + + assert running.status_code == named.status_code == 200 + assert running.headers["cache-control"] == "public, max-age=300" + assert named.headers["cache-control"] == "public, max-age=86400" + assert running.headers["etag"] + assert "origin" in _vary_values(running) + assert revalidated.status_code == 304 + assert revalidated.content == b"" + assert revalidated.headers["etag"] == named.headers["etag"] + + +def test_a_missing_parameter_tree_is_not_cached() -> None: + results, _ids = _resource_results() + reader = ResourceReader( + results, + error=ParameterTreeUnavailableError("catalog version 4.20.3 has no tree"), + ) + + response = _client(lambda: reader).get( + "/v2/parameters/children", + params={"country_id": "us", "policyengine_version": "4.20.3"}, + ) + + assert response.status_code == 503 + assert "etag" not in response.headers + assert "cache-control" not in response.headers + assert "origin" not in _vary_values(response) diff --git a/tests/unit/v2/test_metadata_service.py b/tests/unit/v2/test_metadata_service.py index b2f35d7be..4ff27c3ab 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -18,11 +18,17 @@ MetadataCatalogVersionNotFoundError, UnsupportedPreviewCountryError, ) +from policyengine_api.data.v2.catalog.parameter_tree import ( + TreeNodeSource, + TreeParameterSource, + build_parameter_tree, +) from policyengine_api.data.v2.models import ( V2_METADATA, Dataset, Parameter, ParameterNode, + ParameterTreeChild, ParameterValue, Region, RegionType, @@ -45,11 +51,13 @@ MetadataEnumValue, ParameterFilter, ParameterNodeLabelRow, + ParameterTreeLevel, ParameterValueFilter, ) from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterTreeUnavailableError, ) from tests.fixtures.v2_catalog import POLICYENGINE_VERSION, normalized_catalog @@ -248,6 +256,57 @@ def _model_version( ).one() +def _write_parameter_tree(session: Session, model_version_id) -> None: + """Write a version's tree from its stored rows, as catalog publication does.""" + nodes = session.exec( + select(ParameterNode).where( + ParameterNode.tax_benefit_model_version_id == model_version_id + ) + ).all() + parameters = session.exec( + select(Parameter).where( + Parameter.tax_benefit_model_version_id == model_version_id + ) + ).all() + rows = build_parameter_tree( + nodes=[TreeNodeSource(name=node.name, label=node.label) for node in nodes], + parameters=[ + TreeParameterSource( + id=parameter.id, + name=parameter.name, + label=parameter.label, + economy=parameter.economy, + household=parameter.household, + ) + for parameter in parameters + ], + ) + session.add_all( + ParameterTreeChild( + tax_benefit_model_version_id=model_version_id, + parent_path=row.parent_path, + path=row.path, + position=row.position, + label=row.label, + child_type=row.child_type, + child_count=row.child_count, + reform_position=row.reform_position, + reform_child_count=row.reform_child_count, + parameter_id=row.parameter_id, + economy=row.economy, + household=row.household, + ) + for row in rows + ) + session.commit() + + +def _level(parent_path: str = "", *, reform_changeable: bool = False): + return ParameterTreeLevel( + parent_path=parent_path, reform_changeable=reform_changeable + ) + + def _service( session: Session, *, selected_catalogs: SelectedCatalogStore | None = None ) -> V2MetadataService: @@ -433,14 +492,15 @@ def test_parameter_children_are_loaded_one_level_at_a_time( ) catalog_session.add_all([nested_node, nested_parameter]) catalog_session.commit() + _write_parameter_tree(catalog_session, model_version.id) service = _service(catalog_session) root = service.list_parameter_children("us") - government = service.list_parameter_children("us", parent_path="gov") - example = service.list_parameter_children("us", parent_path="gov.example") + government = service.list_parameter_children("us", level=_level("gov")) + example = service.list_parameter_children("us", level=_level("gov.example")) nested = service.list_parameter_children( "us", - parent_path="gov.example.nested", + level=_level("gov.example.nested"), ) assert [(item.path, item.type) for item in root.items] == [("gov", "node")] @@ -460,6 +520,210 @@ def test_parameter_children_are_loaded_one_level_at_a_time( assert example.items[1].parameter is not None +def _add_parameters(session: Session, model_version, parameters) -> None: + session.add_all( + Parameter( + id=uuid4(), + tax_benefit_model_version_id=model_version.id, + name=name, + label=label, + description=None, + data_type="float", + unit="/1", + economy=economy, + household=household, + ) + for name, label, economy, household in parameters + ) + + +RATES = "gov.example.rates" + + +def test_bracket_schedules_are_one_node_whose_children_are_their_brackets( + catalog_session: Session, +) -> None: + model_version = _model_version(catalog_session, "policyengine-us") + catalog_session.add_all( + [ + ParameterNode( + id=uuid4(), + tax_benefit_model_version_id=model_version.id, + name=RATES, + label="Example rates", + description=None, + ), + ParameterNode( + id=uuid4(), + tax_benefit_model_version_id=model_version.id, + name=f"{RATES}.main", + label="Main rates", + description=None, + ), + ] + ) + _add_parameters( + catalog_session, + model_version, + [ + (f"{RATES}.main[0].rate", "Basic rate", True, True), + (f"{RATES}.main[0].threshold", "Basic threshold", True, True), + (f"{RATES}.main[1].rate", "Higher rate", True, True), + (f"{RATES}.main[1].threshold", "Higher threshold", True, True), + (f"{RATES}.dividends[0].rate", None, True, True), + ], + ) + catalog_session.commit() + _write_parameter_tree(catalog_session, model_version.id) + service = _service(catalog_session) + + rates = service.list_parameter_children("us", level=_level(RATES)) + schedule = service.list_parameter_children("us", level=_level(f"{RATES}.main")) + bracket = service.list_parameter_children("us", level=_level(f"{RATES}.main[0]")) + + assert [ + (item.path, item.label, item.type, item.child_count) for item in rates.items + ] == [ + (f"{RATES}.dividends", "Dividends", "node", 1), + (f"{RATES}.main", "Main rates", "node", 2), + ] + assert [ + (item.path, item.label, item.type, item.child_count) for item in schedule.items + ] == [ + (f"{RATES}.main[0]", "Bracket 1", "node", 2), + (f"{RATES}.main[1]", "Bracket 2", "node", 2), + ] + assert [(item.path, item.label, item.type) for item in bracket.items] == [ + (f"{RATES}.main[0].rate", "Basic rate", "parameter"), + (f"{RATES}.main[0].threshold", "Basic threshold", "parameter"), + ] + assert bracket.items[0].parameter is not None + assert bracket.items[0].parameter.name == f"{RATES}.main[0].rate" + assert bracket.items[0].child_count is None + + +def test_the_reform_changeable_level_lists_only_what_a_reform_can_change( + catalog_session: Session, +) -> None: + model_version = _model_version(catalog_session, "policyengine-us") + _add_parameters( + catalog_session, + model_version, + [ + ("gov.tax.zero_rate", "Zero rate", True, False), + ("gov.tax.allowance", "Allowance", False, True), + ("gov.tax.internal", "Internal", False, False), + ("gov.abolitions.tax", "Abolish tax", True, True), + ], + ) + catalog_session.commit() + _write_parameter_tree(catalog_session, model_version.id) + service = _service(catalog_session) + + full = service.list_parameter_children("us", level=_level("gov.tax")) + changeable = service.list_parameter_children( + "us", level=_level("gov.tax", reform_changeable=True) + ) + government = service.list_parameter_children( + "us", level=_level("gov", reform_changeable=True) + ) + + assert [item.path for item in full.items] == [ + "gov.tax.allowance", + "gov.tax.internal", + "gov.tax.zero_rate", + ] + assert [item.label for item in changeable.items] == ["Allowance", "Zero rate"] + assert {item.path: item.child_count for item in government.items} == { + "gov.example": 1, + "gov.tax": 2, + } + + +def test_parameter_tree_levels_are_paged(catalog_session: Session) -> None: + model_version = _model_version(catalog_session, "policyengine-us") + _add_parameters( + catalog_session, + model_version, + [(f"gov.paged.item_{index}", None, True, True) for index in range(3)], + ) + catalog_session.commit() + _write_parameter_tree(catalog_session, model_version.id) + service = _service(catalog_session) + + first = service.list_parameter_children( + "us", level=_level("gov.paged"), offset=0, limit=2 + ) + last = service.list_parameter_children( + "us", level=_level("gov.paged"), offset=2, limit=2 + ) + + assert [item.path for item in first.items] == [ + "gov.paged.item_0", + "gov.paged.item_1", + ] + assert first.has_more is True + assert [item.path for item in last.items] == ["gov.paged.item_2"] + assert last.has_more is False + + +def test_the_reform_changeable_level_is_paged_in_label_order( + catalog_session: Session, +) -> None: + model_version = _model_version(catalog_session, "policyengine-us") + _add_parameters( + catalog_session, + model_version, + [ + ("gov.paged.c_item", "Cherry", True, False), + ("gov.paged.a_item", "Banana", False, True), + ("gov.paged.b_item", "Apple", True, True), + ("gov.paged.d_item", "Date", False, False), + ], + ) + catalog_session.commit() + _write_parameter_tree(catalog_session, model_version.id) + service = _service(catalog_session) + level = _level("gov.paged", reform_changeable=True) + + first = service.list_parameter_children("us", level=level, offset=0, limit=2) + last = service.list_parameter_children("us", level=level, offset=2, limit=2) + + assert [item.label for item in first.items] == ["Apple", "Banana"] + assert first.has_more is True + assert [item.label for item in last.items] == ["Cherry"] + assert last.has_more is False + + +def test_an_unwritten_parameter_tree_is_reported_as_unavailable( + catalog_session: Session, +) -> None: + service = _service(catalog_session) + + with pytest.raises( + ParameterTreeUnavailableError, + match=( + f"catalog version {POLICYENGINE_VERSION} has no parameter tree: " + "catalog publication writes trees only for the versions it publishes" + ), + ): + service.list_parameter_children("us") + + +def test_an_unknown_parent_in_a_written_tree_has_no_children( + catalog_session: Session, +) -> None: + model_version = _model_version(catalog_session, "policyengine-us") + _write_parameter_tree(catalog_session, model_version.id) + + result = _service(catalog_session).list_parameter_children( + "us", level=_level("gov.nowhere") + ) + + assert result.items == [] + assert result.has_more is False + + def test_resource_filters_and_details_remain_inside_selected_catalog( catalog_session: Session, ) -> None: @@ -1122,12 +1386,15 @@ def test_value_and_node_label_reads_return_typed_rows( def test_parameter_tree_leaves_carry_the_same_summary_as_parameter_pages( catalog_session: Session, ) -> None: + _write_parameter_tree( + catalog_session, _model_version(catalog_session, "policyengine-us").id + ) service = _service(catalog_session) leaf = next( child for child in service.list_parameter_children( - "us", parent_path="gov.example" + "us", level=_level("gov.example") ).items if child.type == "parameter" ) diff --git a/tests/unit/v2/test_models.py b/tests/unit/v2/test_models.py index c93d214b0..464a76d28 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -1,5 +1,7 @@ """Structural contract tests for the complete reviewed v2 SQLModel schema.""" +from typing import Literal + import sqlalchemy as sa from sqlalchemy.dialects import postgresql from sqlalchemy.orm import configure_mappers @@ -17,6 +19,7 @@ LegacyPolicyMapping, LegacyUserMapping, LegacyUserPolicyMapping, + ParameterTreeChild, ParameterValue, Policy, Simulation, @@ -752,3 +755,91 @@ def test_catalog_format_2_json_columns_have_typed_shapes() -> None: (CatalogEnumValue, EnumValueRecord), ): assert get_type_hints(typed_shape) == get_type_hints(record) + + +def test_parameter_tree_children_have_one_row_per_path_in_listing_order() -> None: + """Each version's tree rows are unique by path and listed per parent by index. + + The full listing and the listing of what a reform can change each read one + unique index (version, parent path, position); a parameter row references + its parameter, and both references are removed with their rows. + """ + table = V2_METADATA.tables["parameter_tree_children"] + + assert {column.name for column in table.columns} == { + "id", + "created_at", + "tax_benefit_model_version_id", + "parent_path", + "path", + "position", + "label", + "child_type", + "child_count", + "reform_position", + "reform_child_count", + "parameter_id", + "economy", + "household", + } + for column_name in ("parent_path", "path", "label"): + assert table.c[column_name].type.length == 512 + assert not table.c[column_name].nullable + for column_name in ( + "child_count", + "reform_position", + "reform_child_count", + "parameter_id", + "economy", + "household", + ): + assert table.c[column_name].nullable + unique_constraints = { + tuple(column.name for column in constraint.columns) + for constraint in table.constraints + if isinstance(constraint, sa.UniqueConstraint) + } + assert ("tax_benefit_model_version_id", "path") in unique_constraints + indexes = { + index.name: (tuple(column.name for column in index.columns), index.unique) + for index in table.indexes + } + assert indexes["ix_parameter_tree_children_listing"] == ( + ("tax_benefit_model_version_id", "parent_path", "position"), + True, + ) + assert indexes["ix_parameter_tree_children_reform_listing"] == ( + ("tax_benefit_model_version_id", "parent_path", "reform_position"), + True, + ) + assert indexes["ix_parameter_tree_children_parameter_id"] == ( + ("parameter_id",), + False, + ) + foreign_keys = { + foreign_key.parent.name: (foreign_key.column.table.name, foreign_key.ondelete) + for foreign_key in table.foreign_keys + } + assert foreign_keys == { + "tax_benefit_model_version_id": ("tax_benefit_model_versions", "CASCADE"), + "parameter_id": ("parameters", "CASCADE"), + } + checks = { + constraint.name + for constraint in table.constraints + if isinstance(constraint, sa.CheckConstraint) + } + assert checks == { + "ck_parameter_tree_children_child_type", + "ck_parameter_tree_children_parameter_reference", + } + + +def test_parameter_tree_child_type_is_typed_with_its_two_values() -> None: + """Reads of a tree row keep the child type's two values as a type.""" + annotation = ParameterTreeChild.model_fields["child_type"].annotation + + assert annotation == Literal["node", "parameter"] + column = V2_METADATA.tables["parameter_tree_children"].c["child_type"] + assert isinstance(column.type, AutoString) + assert column.type.length == 16 diff --git a/tests/unit/v2/test_parameter_tree.py b/tests/unit/v2/test_parameter_tree.py new file mode 100644 index 000000000..e61c57bf3 --- /dev/null +++ b/tests/unit/v2/test_parameter_tree.py @@ -0,0 +1,393 @@ +"""Each catalog version's parameter tree, derived as rows of direct children.""" + +from __future__ import annotations + +from uuid import uuid4 + +from policyengine_api.data.v2.catalog.parameter_tree import ( + ParameterTreeChildRow, + TreeNodeSource, + TreeParameterSource, + build_parameter_tree, + generated_label, + parent_path, +) + + +def _parameter( + name: str, + *, + label: str | None = None, + economy: bool | None = True, + household: bool | None = True, +) -> TreeParameterSource: + return TreeParameterSource( + id=uuid4(), + name=name, + label=label, + economy=economy, + household=household, + ) + + +def _rows_by_path( + rows: tuple[ParameterTreeChildRow, ...], +) -> dict[str, ParameterTreeChildRow]: + paths = [row.path for row in rows] + assert len(paths) == len(set(paths)), "each path appears once" + return {row.path: row for row in rows} + + +def _children( + rows: tuple[ParameterTreeChildRow, ...], parent: str +) -> list[ParameterTreeChildRow]: + return sorted( + (row for row in rows if row.parent_path == parent), + key=lambda row: row.position, + ) + + +def _reform_children( + rows: tuple[ParameterTreeChildRow, ...], parent: str +) -> list[ParameterTreeChildRow]: + visible = [ + row for row in rows if row.parent_path == parent and row.reform_changeable + ] + positions = [row.reform_position for row in visible] + assert None not in positions + assert sorted(positions) == list(range(len(visible))) + return sorted(visible, key=lambda row: row.reform_position or 0) + + +UK_RATES = "gov.hmrc.income_tax.rates" + + +def _uk_income_tax_tree() -> tuple[ParameterTreeChildRow, ...]: + """UK income tax: a schedule with a stored node row and one without.""" + nodes = [ + TreeNodeSource(name="gov", label=None), + TreeNodeSource(name="gov.hmrc", label="HMRC"), + TreeNodeSource(name="gov.hmrc.income_tax", label="Income Tax"), + TreeNodeSource(name=UK_RATES, label="Income Tax rates"), + TreeNodeSource(name=f"{UK_RATES}.uk", label="Income Tax main rates"), + TreeNodeSource(name=f"{UK_RATES}.property", label=None), + ] + parameters = [ + _parameter(f"{UK_RATES}.uk[0].rate", label="Basic rate"), + _parameter(f"{UK_RATES}.uk[0].threshold", label="Basic rate threshold"), + _parameter(f"{UK_RATES}.uk[1].rate", label="Higher rate"), + _parameter(f"{UK_RATES}.uk[1].threshold", label="Higher rate threshold"), + _parameter(f"{UK_RATES}.dividends[0].rate"), + _parameter(f"{UK_RATES}.dividends[0].threshold"), + _parameter(f"{UK_RATES}.property.allowance", label="Property allowance"), + ] + return build_parameter_tree(nodes=nodes, parameters=parameters) + + +def test_parent_path_treats_a_bracket_as_a_child_of_its_schedule() -> None: + assert parent_path("gov") == "" + assert parent_path("gov.hmrc") == "gov" + assert parent_path(f"{UK_RATES}.uk") == UK_RATES + assert parent_path(f"{UK_RATES}.uk[0]") == f"{UK_RATES}.uk" + assert parent_path(f"{UK_RATES}.uk[0].rate") == f"{UK_RATES}.uk[0]" + + +def test_generated_labels_follow_the_last_part_of_the_name() -> None: + assert generated_label(f"{UK_RATES}.dividends") == "Dividends" + assert generated_label("gov.example.non_primary_residence") == ( + "Non primary residence" + ) + assert generated_label(f"{UK_RATES}.uk[0].rate") == "Rate" + + +def test_uk_schedules_nest_their_brackets_under_one_node() -> None: + rows = _uk_income_tax_tree() + by_path = _rows_by_path(rows) + + rates_children = _children(rows, UK_RATES) + assert [row.path for row in rates_children] == [ + f"{UK_RATES}.dividends", + f"{UK_RATES}.property", + f"{UK_RATES}.uk", + ] + assert all(row.child_type == "node" for row in rates_children) + + stored_schedule = by_path[f"{UK_RATES}.uk"] + assert stored_schedule.label == "Income Tax main rates" + assert stored_schedule.child_count == 2 + assert [row.path for row in _children(rows, f"{UK_RATES}.uk")] == [ + f"{UK_RATES}.uk[0]", + f"{UK_RATES}.uk[1]", + ] + assert [row.label for row in _children(rows, f"{UK_RATES}.uk")] == [ + "Bracket 1", + "Bracket 2", + ] + + unstored_schedule = by_path[f"{UK_RATES}.dividends"] + assert unstored_schedule.label == "Dividends" + assert unstored_schedule.child_type == "node" + assert unstored_schedule.child_count == 1 + + first_bracket = by_path[f"{UK_RATES}.uk[0]"] + assert first_bracket.child_type == "node" + assert first_bracket.child_count == 2 + assert [ + (row.path, row.label, row.child_type) + for row in _children(rows, f"{UK_RATES}.uk[0]") + ] == [ + (f"{UK_RATES}.uk[0].rate", "Basic rate", "parameter"), + (f"{UK_RATES}.uk[0].threshold", "Basic rate threshold", "parameter"), + ] + + +def test_us_schedule_brackets_are_ordered_by_number_not_by_text() -> None: + schedule = "gov.aca.age_curves.al" + parameters = [ + _parameter(f"{schedule}[{index}].{part}") + for index in (0, 1, 2, 10, 11) + for part in ("amount", "threshold") + ] + nodes = [TreeNodeSource(name=schedule, label="ACA premium age curve factors - AL")] + + rows = build_parameter_tree(nodes=nodes, parameters=parameters) + brackets = _children(rows, schedule) + + assert [row.path for row in brackets] == [ + f"{schedule}[0]", + f"{schedule}[1]", + f"{schedule}[2]", + f"{schedule}[10]", + f"{schedule}[11]", + ] + assert [row.label for row in brackets] == [ + "Bracket 1", + "Bracket 2", + "Bracket 3", + "Bracket 11", + "Bracket 12", + ] + assert [row.path for row in _reform_children(rows, schedule)] == [ + row.path for row in brackets + ] + assert _rows_by_path(rows)[schedule].label == "ACA premium age curve factors - AL" + assert [row.path for row in _children(rows, "")] == ["gov"] + assert [row.path for row in _children(rows, "gov")] == ["gov.aca"] + + +def test_a_bracket_keeps_its_bracket_label_over_a_stored_one() -> None: + schedule = f"{UK_RATES}.uk" + rows = build_parameter_tree( + nodes=[TreeNodeSource(name=f"{schedule}[0]", label="First band")], + parameters=[_parameter(f"{schedule}[0].rate")], + ) + + assert _rows_by_path(rows)[f"{schedule}[0]"].label == "Bracket 1" + + +def test_parameters_carry_their_ids_and_flags_and_nodes_their_counts() -> None: + rate = _parameter("gov.example.rate", label="Example rate", household=False) + rows = build_parameter_tree( + nodes=[TreeNodeSource(name="gov.example", label="Example policy")], + parameters=[rate], + ) + by_path = _rows_by_path(rows) + + leaf = by_path["gov.example.rate"] + assert leaf.child_type == "parameter" + assert leaf.parameter_id == rate.id + assert (leaf.economy, leaf.household) == (True, False) + assert leaf.child_count is None + assert leaf.reform_child_count is None + node = by_path["gov.example"] + assert node.parameter_id is None + assert (node.economy, node.household) == (None, None) + assert (node.child_count, node.reform_child_count) == (1, 1) + + +def test_a_stored_node_without_children_is_an_empty_node() -> None: + rows = build_parameter_tree( + nodes=[TreeNodeSource(name="gov.empty", label="Nothing here")], + parameters=[_parameter("gov.example.rate")], + ) + empty = _rows_by_path(rows)["gov.empty"] + + assert empty.child_type == "node" + assert empty.child_count == 0 + assert empty.reform_child_count == 0 + assert empty.reform_changeable is False + assert empty.reform_position is None + + +def test_the_reform_view_keeps_only_what_a_reform_can_change() -> None: + parameters = [ + _parameter("gov.tax.rate", label="Tax rate", economy=True, household=False), + _parameter("gov.tax.credit", label="Credit", economy=False, household=True), + _parameter("gov.tax.hidden", label="Hidden", economy=False, household=False), + _parameter("gov.unchangeable.only", economy=False, household=False), + _parameter("gov.abolitions.income_tax", label="Abolish income tax"), + _parameter("gov.contrib.taxsim.rate", label="TAXSIM rate"), + _parameter("calibration.gov.target", label="Calibration target"), + ] + rows = build_parameter_tree(nodes=[], parameters=parameters) + by_path = _rows_by_path(rows) + + assert [row.path for row in _reform_children(rows, "")] == ["gov"] + assert [row.path for row in _reform_children(rows, "gov")] == ["gov.tax"] + assert [row.path for row in _reform_children(rows, "gov.tax")] == [ + "gov.tax.credit", + "gov.tax.rate", + ] + assert by_path["gov.tax"].child_count == 3 + assert by_path["gov.tax"].reform_child_count == 2 + assert by_path["gov"].child_count == 4 + assert by_path["gov"].reform_child_count == 1 + for path in ( + "gov.tax.hidden", + "gov.unchangeable", + "gov.abolitions", + "gov.contrib", + "calibration", + ): + assert by_path[path].reform_changeable is False + assert by_path[path].reform_position is None + + +def test_the_reform_view_is_sorted_by_label_as_v3_shows_it() -> None: + parameters = [ + _parameter("gov.b_first", label="zebra crossing"), + _parameter("gov.a_second", label="Apple orchard"), + _parameter("gov.c_third"), + _parameter("gov.d_fourth", label="banana_split"), + ] + rows = build_parameter_tree(nodes=[], parameters=parameters) + + assert [row.path for row in _children(rows, "gov")] == [ + "gov.a_second", + "gov.b_first", + "gov.c_third", + "gov.d_fourth", + ] + assert [row.label for row in _reform_children(rows, "gov")] == [ + "Apple orchard", + "Banana split", + "C third", + "Zebra crossing", + ] + + +def test_every_row_has_its_parent_and_counts_match_the_rows() -> None: + rows = _uk_income_tax_tree() + by_path = _rows_by_path(rows) + + for row in rows: + if row.parent_path: + assert row.parent_path in by_path + children = [child for child in rows if child.parent_path == row.path] + if row.child_type == "node": + assert row.child_count == len(children) + assert row.reform_child_count == sum( + child.reform_changeable for child in children + ) + else: + assert children == [] + for parent in {row.parent_path for row in rows}: + positions = [row.position for row in rows if row.parent_path == parent] + assert sorted(positions) == list(range(len(positions))) + + +def test_stored_labels_take_the_form_v3_shows_in_both_views() -> None: + rows = build_parameter_tree( + nodes=[ + TreeNodeSource( + name="gov.transport", label="assumed uncapped single bus fare" + ) + ], + parameters=[ + _parameter("gov.transport.fare", label="ACA rating area NY (Child_only)"), + ], + ) + by_path = _rows_by_path(rows) + + assert by_path["gov.transport"].label == "Assumed uncapped single bus fare" + assert by_path["gov.transport.fare"].label == "ACA rating area NY (Child only)" + assert by_path["gov.transport"].reform_changeable is True + + +def test_schedules_without_a_stored_label_get_one_from_their_name() -> None: + rows = build_parameter_tree( + nodes=[], + parameters=[ + _parameter(f"{UK_RATES}.uk[0].rate"), + _parameter("gov.aca.age_curves.al[0].amount"), + ], + ) + by_path = _rows_by_path(rows) + + assert by_path[f"{UK_RATES}.uk"].label == "Uk" + assert by_path["gov.aca.age_curves.al"].label == "Al" + assert by_path[f"{UK_RATES}.uk[0]"].label == "Bracket 1" + + +AGE_CURVE = "gov.aca.age_curves.al" +STANDARD_DEDUCTION = "gov.irs.deductions.standard.amount" +PY_SHAPED_PARAMETERS = ( + # Bracket parameters labelled as PolicyEngine.py generates them. + (f"{AGE_CURVE}[0].amount", "ACA premium age curve factors - AL (tier 1 amount)"), + ( + f"{AGE_CURVE}[0].threshold", + "ACA premium age curve factors - AL (tier 1 threshold)", + ), + (f"{AGE_CURVE}[1].amount", "ACA premium age curve factors - AL (tier 2 amount)"), + # Breakdown parameters labelled from their parent node's label. + (f"{STANDARD_DEDUCTION}.SINGLE", "Standard deduction (Single)"), + (f"{STANDARD_DEDUCTION}.JOINT", "Standard deduction (Joint)"), +) + + +def _py_shaped_tree(*, schedule_has_node_row: bool): + nodes = [TreeNodeSource(name=STANDARD_DEDUCTION, label="Standard deduction")] + if schedule_has_node_row: + nodes.append( + TreeNodeSource(name=AGE_CURVE, label="ACA premium age curve factors - AL") + ) + return build_parameter_tree( + nodes=nodes, + parameters=[ + _parameter(name, label=label) for name, label in PY_SHAPED_PARAMETERS + ], + ) + + +def test_a_tree_from_the_locked_policyengine_py_labels_its_schedule_by_name() -> None: + """The locked PolicyEngine.py stores no node row for a bracket schedule.""" + rows = _py_shaped_tree(schedule_has_node_row=False) + by_path = _rows_by_path(rows) + + assert by_path[AGE_CURVE].label == "Al" + assert [(row.label, row.child_count) for row in _children(rows, AGE_CURVE)] == [ + ("Bracket 1", 2), + ("Bracket 2", 1), + ] + assert [row.label for row in _children(rows, f"{AGE_CURVE}[0]")] == [ + "ACA premium age curve factors - AL (tier 1 amount)", + "ACA premium age curve factors - AL (tier 1 threshold)", + ] + assert by_path[STANDARD_DEDUCTION].label == "Standard deduction" + assert [row.label for row in _children(rows, STANDARD_DEDUCTION)] == [ + "Standard deduction (Joint)", + "Standard deduction (Single)", + ] + + +def test_a_tree_from_a_release_with_schedule_nodes_uses_the_schedule_label() -> None: + """A PolicyEngine.py release that publishes schedules stores their label.""" + rows = _py_shaped_tree(schedule_has_node_row=True) + + assert _rows_by_path(rows)[AGE_CURVE].label == ( + "ACA premium age curve factors - AL" + ) + assert [row.label for row in _children(rows, AGE_CURVE)] == [ + "Bracket 1", + "Bracket 2", + ]