From 9882db0ba54816a1b001ff7e7a5352b71b6278f8 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:48:57 +0400 Subject: [PATCH 01/19] Build each catalog version's parameter search file A pure module builds the file v3's parameter search reads from a version's parameters, parameter nodes and current-law values: every parameter a reform can change (by the parameter tree's rule) with its path, stored label, unit, description and whether a draft can edit its current-law value; the labels of the other parameters under gov, which the search's concept clusters, aliases and member labels read; and every node under gov that has a stored label. Labels stay as stored (null when none), so the search's placeholder detection keeps working when the reader applies v1's fallback. Editability is stated for the earliest value plus the dates from which it changes, because a bracket that stops applying has a null value from that date; the file therefore never depends on the day it is built. The file is encoded as compact UTF-8 JSON, gzip-compressed with no timestamp so the same rows always give the same bytes, with the SHA-256 of the uncompressed JSON. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/parameter_search_file.py | 193 ++++++++++++ .../data/v2/catalog/parameter_tree.py | 24 +- tests/unit/v2/test_parameter_search_file.py | 296 ++++++++++++++++++ 3 files changed, 508 insertions(+), 5 deletions(-) create mode 100644 policyengine_api/data/v2/catalog/parameter_search_file.py create mode 100644 tests/unit/v2/test_parameter_search_file.py diff --git a/policyengine_api/data/v2/catalog/parameter_search_file.py b/policyengine_api/data/v2/catalog/parameter_search_file.py new file mode 100644 index 000000000..e93eb8abb --- /dev/null +++ b/policyengine_api/data/v2/catalog/parameter_search_file.py @@ -0,0 +1,193 @@ +"""Each catalog version's parameter search file: what v3's search reads. + +v3's parameter search (the app's ``parameterSearch.ts``) indexes every +parameter a reform can change by its path, label, unit and description, and +offers only those whose current value a draft can edit: a number, a yes/no +value or an infinite threshold. Its breadcrumbs and schedule names read the +labels of the nodes above a parameter, and its concept clusters, acronym +aliases and member labels also read the label of every other parameter under +``gov``. The file holds exactly those fields for one catalog version, so the +search runs without the catalog's values. + +Labels are the stored labels, or null when none is stored. The search treats +a label equal to the path's last key as a placeholder, so a reader applies +v1's fallback itself (the key with underscores as spaces, "bracket N" for a +bracket) rather than a display label made here. + +Whether a value can be edited depends on the date, because a bracket that +stops applying has a null value from that date. The file states editability +for the earliest value and the dates from which it changes, so its content +never depends on the day it was built. +""" + +from __future__ import annotations + +import gzip +import hashlib +import json +from collections import defaultdict +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from policyengine_api.data.v2.catalog.parameter_tree import ( + REFORM_TREE_PREFIX, + is_reform_changeable, +) + +if TYPE_CHECKING: + from collections.abc import Iterable + from datetime import date + +PARAMETER_SEARCH_FILE_FORMAT = 1 + +# Repeated builds of the same rows must give the same bytes, so the gzip header +# carries no timestamp. +_GZIP_MTIME = 0 +_GZIP_LEVEL = 9 + + +@dataclass(frozen=True, slots=True) +class SearchParameterSource: + """A stored parameter's search fields and flags.""" + + name: str + label: str | None + unit: str | None + description: str | None + economy: bool | None + household: bool | None + + +@dataclass(frozen=True, slots=True) +class SearchNodeSource: + """A stored parameter node: its name and stored label.""" + + name: str + label: str | None + + +@dataclass(frozen=True, slots=True) +class DraftabilityChange: + """Whether a parameter's current-law value from ``start_date`` can be edited. + + A value can be edited in a draft when it is a number, a yes/no value or the + text "Infinity" or "-Infinity", as v3's draft editor decides. + """ + + parameter_name: str + start_date: date + draftable: bool + + +@dataclass(frozen=True, slots=True) +class EncodedParameterSearchFile: + """A search file as stored: gzip-compressed JSON and its content hash. + + ``content_sha256`` is the SHA-256 of the uncompressed JSON, so the same + content has the same hash whatever compression produced the bytes. + """ + + compressed_content: bytes + content_sha256: str + uncompressed_size: int + compressed_size: int + + +def _editability( + changes: Iterable[DraftabilityChange], +) -> dict[str, tuple[bool, list[dict[str, Any]]]]: + """Return each parameter's earliest editability and the later changes.""" + by_parameter: defaultdict[str, list[DraftabilityChange]] = defaultdict(list) + for change in changes: + by_parameter[change.parameter_name].append(change) + editability: dict[str, tuple[bool, list[dict[str, Any]]]] = {} + for name, parameter_changes in by_parameter.items(): + ordered = sorted(parameter_changes, key=lambda change: change.start_date) + earliest = ordered[0].draftable + later: list[dict[str, Any]] = [] + current = earliest + for change in ordered[1:]: + if change.draftable != current: + later.append( + { + "start_date": change.start_date.isoformat(), + "editable": change.draftable, + } + ) + current = change.draftable + editability[name] = (earliest, later) + return editability + + +def _stored_label(label: str | None) -> str | None: + return label if label is not None and label.strip() else None + + +def build_parameter_search_file( + *, + country_id: str, + policyengine_version: str, + parameters: Iterable[SearchParameterSource], + nodes: Iterable[SearchNodeSource], + draftability: Iterable[DraftabilityChange], +) -> dict[str, Any]: + """Return one catalog version's search file, ordered by path. + + ``parameters`` lists every parameter a reform can change with its search + fields and editability; ``other_parameters`` lists the other parameters + under ``gov`` with their labels; ``nodes`` lists every node under ``gov`` + that has a stored label. A parameter without current-law values cannot be + edited. + """ + editability = _editability(draftability) + changeable: list[dict[str, Any]] = [] + others: list[dict[str, Any]] = [] + for parameter in sorted(parameters, key=lambda parameter: parameter.name): + if not parameter.name.startswith(REFORM_TREE_PREFIX): + continue + label = _stored_label(parameter.label) + if not is_reform_changeable( + parameter.name, economy=parameter.economy, household=parameter.household + ): + others.append({"path": parameter.name, "label": label}) + continue + editable, changes = editability.get(parameter.name, (False, [])) + entry: dict[str, Any] = { + "path": parameter.name, + "label": label, + "unit": parameter.unit, + "description": parameter.description, + "editable": editable, + } + if changes: + entry["editable_changes"] = changes + changeable.append(entry) + labelled_nodes = [ + {"path": node.name, "label": node.label} + for node in sorted(nodes, key=lambda node: node.name) + if node.name.startswith(REFORM_TREE_PREFIX) and _stored_label(node.label) + ] + return { + "format": PARAMETER_SEARCH_FILE_FORMAT, + "country_id": country_id, + "policyengine_version": policyengine_version, + "parameters": changeable, + "other_parameters": others, + "nodes": labelled_nodes, + } + + +def encode_parameter_search_file( + search_file: dict[str, Any], +) -> EncodedParameterSearchFile: + """Return the file as compact UTF-8 JSON, gzip-compressed, with its hash.""" + content = json.dumps( + search_file, ensure_ascii=False, separators=(",", ":"), allow_nan=False + ).encode("utf-8") + compressed = gzip.compress(content, compresslevel=_GZIP_LEVEL, mtime=_GZIP_MTIME) + return EncodedParameterSearchFile( + compressed_content=compressed, + content_sha256=hashlib.sha256(content).hexdigest(), + uncompressed_size=len(content), + compressed_size=len(compressed), + ) diff --git a/policyengine_api/data/v2/catalog/parameter_tree.py b/policyengine_api/data/v2/catalog/parameter_tree.py index a03456e88..bdcbfedf2 100644 --- a/policyengine_api/data/v2/catalog/parameter_tree.py +++ b/policyengine_api/data/v2/catalog/parameter_tree.py @@ -221,13 +221,27 @@ def _child_label( return display_label(stored_label) -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): +def is_reform_changeable( + name: str, *, economy: bool | None, household: bool | None +) -> bool: + """Return whether a reform can change the named parameter, as v3 decides. + + The parameter is under ``gov``, its path names neither ``taxsim``, + ``gov.abolitions`` nor ``pycache``, and its economy or household flag is + set; a parameter without flags is not changeable. + """ + if not name.startswith(REFORM_TREE_PREFIX): return False - if any(part in parameter.name for part in REFORM_EXCLUDED_PATH_PARTS): + if any(part in name for part in REFORM_EXCLUDED_PATH_PARTS): return False - return bool(parameter.economy) or bool(parameter.household) + return bool(economy) or bool(household) + + +def is_reform_changeable_parameter(parameter: TreeParameterSource) -> bool: + """Return whether a reform can change this parameter, as v3's tree decides.""" + return is_reform_changeable( + parameter.name, economy=parameter.economy, household=parameter.household + ) def _ancestors(path: str) -> list[str]: diff --git a/tests/unit/v2/test_parameter_search_file.py b/tests/unit/v2/test_parameter_search_file.py new file mode 100644 index 000000000..f5d7a8f4e --- /dev/null +++ b/tests/unit/v2/test_parameter_search_file.py @@ -0,0 +1,296 @@ +"""Each catalog version's parameter search file, built from stored rows.""" + +from __future__ import annotations + +import gzip +import hashlib +import json +from datetime import date + +from policyengine_api.data.v2.catalog.parameter_search_file import ( + PARAMETER_SEARCH_FILE_FORMAT, + DraftabilityChange, + SearchNodeSource, + SearchParameterSource, + build_parameter_search_file, + encode_parameter_search_file, +) + + +def _parameter( + name: str, + *, + label: str | None = None, + unit: str | None = None, + description: str | None = None, + flags: tuple[bool | None, bool | None] = (True, True), +) -> SearchParameterSource: + economy, household = flags + return SearchParameterSource( + name=name, + label=label, + unit=unit, + description=description, + economy=economy, + household=household, + ) + + +def _always_draftable(*names: str) -> list[DraftabilityChange]: + return [ + DraftabilityChange( + parameter_name=name, start_date=date(2020, 1, 1), draftable=True + ) + for name in names + ] + + +def _file( + parameters: list[SearchParameterSource], + *, + nodes: list[SearchNodeSource] | None = None, + draftability: list[DraftabilityChange] | None = None, +) -> dict: + return build_parameter_search_file( + country_id="uk", + policyengine_version="6.10.0", + parameters=parameters, + nodes=nodes or [], + draftability=( + draftability + if draftability is not None + else _always_draftable(*(parameter.name for parameter in parameters)) + ), + ) + + +def test_the_file_names_its_format_country_and_version() -> None: + search_file = _file([]) + + assert search_file["format"] == PARAMETER_SEARCH_FILE_FORMAT == 1 + assert search_file["country_id"] == "uk" + assert search_file["policyengine_version"] == "6.10.0" + assert search_file["parameters"] == [] + assert search_file["other_parameters"] == [] + assert search_file["nodes"] == [] + + +def test_each_changeable_parameter_has_what_v3_search_indexes() -> None: + search_file = _file( + [ + _parameter( + "gov.hmrc.income_tax.rates.uk[0].rate", + label="Basic rate", + unit="/1", + description="Rate on income in the basic band.", + ), + _parameter("gov.dwp.benefit_cap.single.in_london", unit="currency-GBP"), + ] + ) + + assert search_file["parameters"] == [ + { + "path": "gov.dwp.benefit_cap.single.in_london", + "label": None, + "unit": "currency-GBP", + "description": None, + "editable": True, + }, + { + "path": "gov.hmrc.income_tax.rates.uk[0].rate", + "label": "Basic rate", + "unit": "/1", + "description": "Rate on income in the basic band.", + "editable": True, + }, + ] + + +def test_parameters_a_reform_cannot_change_keep_only_their_labels() -> None: + search_file = _file( + [ + _parameter("gov.hmrc.vat.rate", label="VAT rate"), + _parameter( + "gov.economic_assumptions.cpi", + label="Consumer price index", + flags=(False, False), + ), + _parameter("gov.abolitions.income_tax", label="Abolish income tax"), + _parameter("gov.irs.taxsim.version", label="TAXSIM version"), + _parameter("calibration.uprating.factor", label="Uprating factor"), + ] + ) + + assert [entry["path"] for entry in search_file["parameters"]] == [ + "gov.hmrc.vat.rate" + ] + assert search_file["other_parameters"] == [ + {"path": "gov.abolitions.income_tax", "label": "Abolish income tax"}, + {"path": "gov.economic_assumptions.cpi", "label": "Consumer price index"}, + {"path": "gov.irs.taxsim.version", "label": "TAXSIM version"}, + ] + + +def test_a_parameter_with_either_flag_set_is_changeable() -> None: + search_file = _file( + [ + _parameter("gov.a.economy_only", flags=(True, False)), + _parameter("gov.a.household_only", flags=(False, True)), + ] + ) + + assert [entry["path"] for entry in search_file["parameters"]] == [ + "gov.a.economy_only", + "gov.a.household_only", + ] + + +def test_a_parameter_without_flags_is_not_changeable() -> None: + search_file = _file( + [ + _parameter("gov.a.rate", flags=(None, None)), + _parameter("gov.a.other", flags=(False, False)), + ] + ) + + assert search_file["parameters"] == [] + assert [entry["path"] for entry in search_file["other_parameters"]] == [ + "gov.a.other", + "gov.a.rate", + ] + + +def test_nodes_carry_their_stored_labels_only() -> None: + search_file = _file( + [_parameter("gov.hmrc.income_tax.rates.uk[0].rate")], + nodes=[ + SearchNodeSource(name="gov.hmrc", label="HMRC"), + SearchNodeSource(name="gov.hmrc.income_tax", label="Income Tax"), + SearchNodeSource( + name="gov.hmrc.income_tax.rates.uk", label="Income Tax main rates" + ), + SearchNodeSource(name="gov.hmrc.income_tax.rates", label=None), + SearchNodeSource(name="gov.hmrc.income_tax.rates.uk[0]", label=" "), + SearchNodeSource(name="calibration", label="Calibration"), + ], + ) + + assert search_file["nodes"] == [ + {"path": "gov.hmrc", "label": "HMRC"}, + {"path": "gov.hmrc.income_tax", "label": "Income Tax"}, + {"path": "gov.hmrc.income_tax.rates.uk", "label": "Income Tax main rates"}, + ] + + +def test_a_value_that_is_not_a_number_or_switch_cannot_be_edited() -> None: + search_file = _file( + [ + _parameter("gov.a.programs"), + _parameter("gov.a.no_values"), + _parameter("gov.a.threshold"), + ], + draftability=[ + DraftabilityChange("gov.a.programs", date(2020, 1, 1), draftable=False), + DraftabilityChange("gov.a.threshold", date(2020, 1, 1), draftable=True), + ], + ) + + editable = {entry["path"]: entry["editable"] for entry in search_file["parameters"]} + assert editable == { + "gov.a.no_values": False, + "gov.a.programs": False, + "gov.a.threshold": True, + } + assert all("editable_changes" not in entry for entry in search_file["parameters"]) + + +def test_editability_that_changes_over_time_lists_the_dates_it_changes() -> None: + # A bracket that stops applying has a null value from that date and may + # return later, so whether its current value can be edited depends on the + # day it is read; the file states it for every date instead. + search_file = _file( + [ + _parameter( + "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate" + ) + ], + draftability=[ + DraftabilityChange( + "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", + date(2003, 7, 10), + draftable=True, + ), + DraftabilityChange( + "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", + date(2015, 2, 12), + draftable=True, + ), + DraftabilityChange( + "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", + date(2020, 7, 8), + draftable=False, + ), + DraftabilityChange( + "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", + date(2021, 3, 31), + draftable=True, + ), + ], + ) + + (entry,) = search_file["parameters"] + assert entry["editable"] is True + assert entry["editable_changes"] == [ + {"start_date": "2020-07-08", "editable": False}, + {"start_date": "2021-03-31", "editable": True}, + ] + + +def test_the_earliest_value_decides_editability_before_any_change() -> None: + search_file = _file( + [_parameter("gov.hmrc.income_tax.rates.scotland.rates[0].rate")], + draftability=[ + DraftabilityChange( + "gov.hmrc.income_tax.rates.scotland.rates[0].rate", + date(2018, 4, 6), + draftable=True, + ), + DraftabilityChange( + "gov.hmrc.income_tax.rates.scotland.rates[0].rate", + date(2015, 1, 1), + draftable=False, + ), + ], + ) + + (entry,) = search_file["parameters"] + assert entry["editable"] is False + assert entry["editable_changes"] == [{"start_date": "2018-04-06", "editable": True}] + + +def test_the_encoded_file_is_compressed_json_with_its_content_hash() -> None: + search_file = _file( + [_parameter("gov.hmrc.vat.rate", label="VAT rate", unit="/1")], + nodes=[SearchNodeSource(name="gov.hmrc", label="HMRC")], + ) + + encoded = encode_parameter_search_file(search_file) + + content = gzip.decompress(encoded.compressed_content) + assert json.loads(content) == search_file + assert encoded.content_sha256 == hashlib.sha256(content).hexdigest() + assert encoded.uncompressed_size == len(content) + assert encoded.compressed_size == len(encoded.compressed_content) + + +def test_the_same_catalog_rows_always_encode_to_the_same_bytes() -> None: + parameters = [ + _parameter("gov.b.rate", label="B"), + _parameter("gov.a.rate", label="Ä rate"), + ] + first = encode_parameter_search_file(_file(parameters)) + second = encode_parameter_search_file(_file(list(reversed(parameters)))) + + assert first.compressed_content == second.compressed_content + assert first.content_sha256 == second.content_sha256 + assert "Ä rate" in gzip.decompress(first.compressed_content).decode("utf-8") From ef4d5d0a78b8d42fbebb8e43fcf9c12cb1f01ecb Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:56:07 +0400 Subject: [PATCH 02/19] Store each catalog version's parameter search file in its own table The new table parameter_search_files holds a catalog version's search file once per file format: the gzip-compressed JSON, the SHA-256 of the uncompressed JSON and both sizes. A later file format adds a row rather than changing a stored one, and a file is deleted with its version. The revision is autogenerated with no corrections and writes no rows; catalog publication writes the files. The Public API's runtime role may read the table and the catalog writer may read and insert rows. The parameter tree revision test now names its own revision instead of the head. Co-Authored-By: Claude Opus 5.5 --- ...store_each_catalog_version_s_parameter_.py | 64 +++++++++++++ policyengine_api/data/v2/migration_target.py | 2 +- policyengine_api/data/v2/models/__init__.py | 2 + policyengine_api/data/v2/models/metadata.py | 30 ++++++ policyengine_api/data/v2/privileges.py | 2 + .../integration/test_alembic_v2_lifecycle.py | 92 +++++++++++++++++++ tests/unit/v2/test_alembic_v2.py | 41 ++++++++- tests/unit/v2/test_import_side_effects.py | 2 +- tests/unit/v2/test_models.py | 42 +++++++++ 9 files changed, 274 insertions(+), 3 deletions(-) create mode 100644 migrations/v2/versions/c614b6bcccfc_store_each_catalog_version_s_parameter_.py diff --git a/migrations/v2/versions/c614b6bcccfc_store_each_catalog_version_s_parameter_.py b/migrations/v2/versions/c614b6bcccfc_store_each_catalog_version_s_parameter_.py new file mode 100644 index 000000000..0978e64e0 --- /dev/null +++ b/migrations/v2/versions/c614b6bcccfc_store_each_catalog_version_s_parameter_.py @@ -0,0 +1,64 @@ +"""store each catalog version's parameter search file + +Revision ID: c614b6bcccfc +Revises: 3869eb814a90 +Create Date: 2026-10-26 16:58:05.350610 +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 = "c614b6bcccfc" +down_revision: Union[str, None] = "3869eb814a90" +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_search_files", + 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("file_format", sa.Integer(), nullable=False), + sa.Column("compressed_content", sa.LargeBinary(), nullable=False), + sa.Column( + "content_sha256", + sqlmodel.sql.sqltypes.AutoString(length=64), + nullable=False, + ), + sa.Column("uncompressed_size", sa.Integer(), nullable=False), + sa.Column("compressed_size", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint( + ["tax_benefit_model_version_id"], + ["tax_benefit_model_versions.id"], + name=op.f( + "fk_parameter_search_files_tax_benefit_model_version_id_tax_benefit_model_versions" + ), + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("id", name=op.f("pk_parameter_search_files")), + sa.UniqueConstraint( + "tax_benefit_model_version_id", + "file_format", + name="uq_parameter_search_files_model_version_format", + ), + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("parameter_search_files") + # ### end Alembic commands ### diff --git a/policyengine_api/data/v2/migration_target.py b/policyengine_api/data/v2/migration_target.py index c22f670bc..420601582 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 = "3869eb814a90" +V2_ALEMBIC_HEAD_REVISION = "c614b6bcccfc" 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 f2ce889f0..8498cfbb6 100644 --- a/policyengine_api/data/v2/models/__init__.py +++ b/policyengine_api/data/v2/models/__init__.py @@ -47,6 +47,7 @@ DatasetVersion, Parameter, ParameterNode, + ParameterSearchFile, ParameterTreeChild, ParameterValue, Region, @@ -127,6 +128,7 @@ "OutputStatus", "Parameter", "ParameterNode", + "ParameterSearchFile", "ParameterTreeChild", "ParameterValue", "Policy", diff --git a/policyengine_api/data/v2/models/metadata.py b/policyengine_api/data/v2/models/metadata.py index 74f6f6ed0..b53e42a7b 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -533,3 +533,33 @@ class ParameterTreeChild(IdentifiedModel, table=True): ) economy: bool | None = None household: bool | None = None + + +class ParameterSearchFile(IdentifiedModel, table=True): + """One catalog version's parameter search file, as gzip-compressed JSON. + + Built from the installed packages' catalog when catalog publication + publishes the version (``parameter_search_file`` describes the content). + A version has at most one file per file format, so a later format adds a + row rather than changing a stored one. ``content_sha256`` is the SHA-256 of + the uncompressed JSON, which the route serves as the file's ETag. + """ + + __tablename__ = "parameter_search_files" + __table_args__ = ( + sa.UniqueConstraint( + "tax_benefit_model_version_id", + "file_format", + name="uq_parameter_search_files_model_version_format", + ), + ) + + tax_benefit_model_version_id: UUID = Field( + foreign_key="tax_benefit_model_versions.id", + ondelete="CASCADE", + ) + file_format: int + compressed_content: bytes = Field(sa_type=sa.LargeBinary) + content_sha256: str = Field(max_length=64) + uncompressed_size: int + compressed_size: int diff --git a/policyengine_api/data/v2/privileges.py b/policyengine_api/data/v2/privileges.py index 12817e5b1..542fa0f0f 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -114,6 +114,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, and for versions stored before it. + "parameter_search_files": _access(runtime=READ, catalog_writer=READ_INSERT), # 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. diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 1fc81384e..a8dd3d21b 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1494,3 +1494,95 @@ def test_parameter_tree_table_is_created_empty_and_dropped_without_data_loss() - ) ) engine.dispose() + + +PARAMETER_SEARCH_FILE_PARENT_REVISION = "3869eb814a90" + + +def test_parameter_search_file_table_is_created_empty_and_dropped_without_loss() -> ( + None +): + """The search file revision creates an empty table and downgrades cleanly. + + The revision writes no rows: catalog publication writes each version's + file. A version holds at most one file, the file is deleted with its + version, and downgrading drops the table while every catalog row stays. + """ + config = _config() + engine = create_engine(_disposable_url()) + try: + command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) + assert "parameter_search_files" 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") + + insert_file = text( + "INSERT INTO parameter_search_files (id, tax_benefit_model_version_id, " + "file_format, compressed_content, content_sha256, uncompressed_size, " + "compressed_size) VALUES (:id, :version_id, 1, :content, :sha, 2, 22)" + ) + file_values = { + "version_id": version_id, + "content": b"\x1f\x8b compressed bytes", + "sha": "0" * 64, + } + with engine.begin() as connection: + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_search_files") + ).scalar_one() + == 0 + ) + connection.execute(insert_file, {"id": uuid4(), **file_values}) + with pytest.raises(sa.exc.IntegrityError), connection.begin_nested(): + connection.execute(insert_file, {"id": uuid4(), **file_values}) + assert ( + connection.execute( + text( + "SELECT compressed_content FROM parameter_search_files " + "WHERE tax_benefit_model_version_id = :version_id" + ), + {"version_id": version_id}, + ).scalar_one() + == file_values["content"] + ) + + command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) + + assert "parameter_search_files" 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 + ) + + command.upgrade(config, "head") + with engine.begin() as connection: + connection.execute(insert_file, {"id": uuid4(), **file_values}) + connection.execute( + text("DELETE FROM tax_benefit_model_versions WHERE id = :version_id"), + {"version_id": version_id}, + ) + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_search_files") + ).scalar_one() + == 0 + ) + 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/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index 69602b913..f0562a2fe 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -45,6 +45,7 @@ "legacy_policy_mappings", "legacy_user_mappings", "legacy_user_policy_mappings", + "parameter_search_files", "parameter_tree_children", "stage12_evaluation_reports", "stage12_evaluation_simulations", @@ -59,6 +60,7 @@ # The catalog format 2 revision's parent, the countries revision. CATALOG_FORMAT_2_PARENT_REVISION = "fb1a3e40d3d6" CATALOG_FORMAT_2_REVISION = "f5c1d9b4685b" +PARAMETER_TREE_REVISION = "3869eb814a90" DISPOSABLE_URL = ( "postgresql+psycopg://postgres:test-password@127.0.0.1:5432/" f"{DISPOSABLE_DATABASE_NAME}" @@ -240,6 +242,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, + PARAMETER_TREE_REVISION, CATALOG_FORMAT_2_REVISION, CATALOG_FORMAT_2_PARENT_REVISION, "fbb7488ebe8a", @@ -901,7 +904,7 @@ def test_parameter_tree_revision_creates_only_the_children_table() -> None: if revision.down_revision == CATALOG_FORMAT_2_REVISION ] assert len(children) == 1 - assert children[0].revision == V2_ALEMBIC_HEAD_REVISION + assert children[0].revision == PARAMETER_TREE_REVISION revision = Path(children[0].path).read_text(encoding="utf-8") upgrade = revision[ revision.index("def upgrade()") : revision.index("def downgrade()") @@ -928,3 +931,39 @@ def test_parameter_tree_revision_creates_only_the_children_table() -> None: assert "op.execute(" not in body assert "op.bulk_insert(" not in body assert "op.add_column(" not in body + + +def test_parameter_search_file_revision_creates_only_the_files_table() -> None: + """The revision after the parameter tree creates the search files' table. + + It is generated, creates the table with its unique version constraint and + drops it on downgrade, and writes no data: catalog publication writes each + version's file. + """ + script = ScriptDirectory.from_config(Config(str(REPO / "alembic-v2.ini"))) + children = [ + revision + for revision in script.walk_revisions() + if revision.down_revision == PARAMETER_TREE_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_search_files"' in upgrade + assert "uq_parameter_search_files_model_version_format" in upgrade + assert downgrade.count("op.drop_table(") == 1 + assert '"parameter_search_files"' 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_import_side_effects.py b/tests/unit/v2/test_import_side_effects.py index 759a6b04a..a3010eff6 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) == 41 +assert len(V2_METADATA.tables) == 42 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_models.py b/tests/unit/v2/test_models.py index 783a067a9..62b1731d2 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -886,3 +886,45 @@ def test_parameter_tree_child_type_is_typed_with_its_two_values() -> None: column = V2_METADATA.tables["parameter_tree_children"].c["child_type"] assert isinstance(column.type, AutoString) assert column.type.length == 16 + + +def test_parameter_search_files_hold_one_compressed_file_per_version() -> None: + """Each catalog version has one search file per format, removed with it. + + The row holds the gzip-compressed JSON, its file format, the SHA-256 of the + uncompressed JSON (the response's ETag) and both sizes. + """ + table = V2_METADATA.tables["parameter_search_files"] + + assert {column.name for column in table.columns} == { + "id", + "created_at", + "tax_benefit_model_version_id", + "file_format", + "compressed_content", + "content_sha256", + "uncompressed_size", + "compressed_size", + } + for column in table.columns: + assert not column.nullable, column.name + assert isinstance(table.c["compressed_content"].type, sa.LargeBinary) + assert table.c["content_sha256"].type.length == 64 + unique_constraints = { + constraint.name: tuple(column.name for column in constraint.columns) + for constraint in table.constraints + if isinstance(constraint, sa.UniqueConstraint) + } + assert unique_constraints == { + "uq_parameter_search_files_model_version_format": ( + "tax_benefit_model_version_id", + "file_format", + ) + } + 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"), + } From f758b92e82341844df810c5f710639c910f91d52 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:15:24 +0400 Subject: [PATCH 03/19] Write the parameter search file of each catalog version being published Publication now builds, in its transaction and under its lock, the search file of each version it publishes from the installed packages' catalog, and inserts it if the version has none: each new version, and the installed version when an earlier release stored it before search files existed. Files are never built from rows an earlier release stored, so other stored versions are left without one. A stored file of the same format must equal the one built from the installed packages; publication fails if it differs or if a version it publishes has no file. The evidence lists each file written with its sizes. Co-Authored-By: Claude Opus 5.5 --- docs/migration/stage-9-v2-metadata.md | 38 +++ .../data/v2/catalog/parameter_search_file.py | 66 ++++- .../data/v2/catalog/publication.py | 20 +- .../publication_parameter_search_file.py | 122 +++++++++ .../data/v2/catalog/publication_types.py | 14 ++ .../test_v2_catalog_publication.py | 236 ++++++++++++++++++ tests/unit/v2/test_catalog_publication.py | 9 + 7 files changed, 499 insertions(+), 6 deletions(-) create mode 100644 policyengine_api/data/v2/catalog/publication_parameter_search_file.py diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index ca2f15096..b170a07cf 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -329,6 +329,40 @@ so unknown or repeated parameters, an unsupported country and a malformed catalog version without a tree returns 503 with a message that names the version. +### Parameter search file + +Publication also writes the parameter search file of each catalog version it +publishes into `parameter_search_files`: one gzip-compressed JSON file per +version and file format, with the SHA-256 of the uncompressed JSON. The file +holds what the app's newer interface's parameter search reads: + +- every parameter a reform can change (the parameter tree's rule) with its + path, stored label, unit, description, and whether a draft can edit its + current-law value: a number, a yes/no value or an infinite threshold. Where + that changes over time (a bracket that stops applying has a null value from + that date), `editable` describes the earliest value and `editable_changes` + lists the dates from which it changes, so the file does not depend on the + day it was built; +- the path and stored label of every other parameter under `gov`, which the + search's concept clusters, acronym aliases and member labels read; +- the path and stored label of every node under `gov` that has a stored label, + for breadcrumbs and schedule names. + +Labels are stored labels, or null: the search treats a label that only +restates its path's last key as a placeholder, so its reader applies v1's +fallback (the key with underscores as spaces, "bracket N" for a bracket). + +The file is built from the installed packages' catalog, never from rows an +earlier release stored, so publication writes one only for the versions it +publishes: each new version, and the installed version when an earlier +release stored it before search files existed. A version that is neither has +no file until a person decides otherwise. Writing only inserts; a stored file +of the same format must equal the one built from the installed packages, and +publication fails if it differs or if a version it publishes is left without a +file. A later file format adds a row for the version instead of changing the +stored one. The evidence's `parameter_search_files_written` lists each file +written with its model, version and uncompressed and compressed sizes in bytes. + ## Validation evidence and retry Successful JSON evidence contains: @@ -337,6 +371,10 @@ Successful JSON evidence contains: - the dependency versions derived from its packaged manifest; - catalog entity counts; - US fallback counts; +- the model, version and row count of each catalog version whose parameter + tree was written; +- the model, version and uncompressed and compressed sizes of each parameter + search file written; - elapsed publication time; and - a success outcome. diff --git a/policyengine_api/data/v2/catalog/parameter_search_file.py b/policyengine_api/data/v2/catalog/parameter_search_file.py index e93eb8abb..7fba7fe7c 100644 --- a/policyengine_api/data/v2/catalog/parameter_search_file.py +++ b/policyengine_api/data/v2/catalog/parameter_search_file.py @@ -18,6 +18,9 @@ stops applying has a null value from that date. The file states editability for the earliest value and the dates from which it changes, so its content never depends on the day it was built. + +Catalog publication builds the file from the installed packages' catalog +(``country_search_file``) for the version it publishes. """ from __future__ import annotations @@ -35,15 +38,22 @@ ) if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterable, Iterator from datetime import date + from policyengine_api.data.v2.catalog.records import ( + CountryCatalog, + ParameterRecord, + ) + PARAMETER_SEARCH_FILE_FORMAT = 1 # Repeated builds of the same rows must give the same bytes, so the gzip header # carries no timestamp. _GZIP_MTIME = 0 _GZIP_LEVEL = 9 +# v3's draft editor accepts these two text values as numbers. +_INFINITE_VALUES = frozenset({"Infinity", "-Infinity"}) @dataclass(frozen=True, slots=True) @@ -191,3 +201,57 @@ def encode_parameter_search_file( uncompressed_size=len(content), compressed_size=len(compressed), ) + + +def is_draftable_value(value: object) -> bool: + """Return whether a draft can edit a value, as v3's draft editor decides. + + A number (an infinite threshold, stored as the text "Infinity" or + "-Infinity", included) or a yes/no value can be edited; a list of variable + names, other text and a missing value cannot. + """ + if isinstance(value, bool | int | float): + return True + return isinstance(value, str) and value in _INFINITE_VALUES + + +def _parameter_draftability(parameter: ParameterRecord) -> Iterator[DraftabilityChange]: + """Yield the parameter's first value's editability and each later change.""" + previous: bool | None = None + for value in sorted(parameter.values, key=lambda value: value.start_date): + draftable = is_draftable_value(value.value_json) + if draftable != previous: + yield DraftabilityChange( + parameter_name=parameter.name, + start_date=value.start_date.date(), + draftable=draftable, + ) + previous = draftable + + +def country_search_file(country: CountryCatalog) -> dict[str, Any]: + """Return the search file of a country's catalog version from its records.""" + return build_parameter_search_file( + country_id=country.country_id, + policyengine_version=country.model_version.version, + parameters=( + SearchParameterSource( + name=parameter.name, + label=parameter.label, + unit=parameter.unit, + description=parameter.description, + economy=parameter.economy, + household=parameter.household, + ) + for parameter in country.parameters + ), + nodes=( + SearchNodeSource(name=node.name, label=node.label) + for node in country.parameter_nodes + ), + draftability=( + change + for parameter in country.parameters + for change in _parameter_draftability(parameter) + ), + ) diff --git a/policyengine_api/data/v2/catalog/publication.py b/policyengine_api/data/v2/catalog/publication.py index 3b47f5a6e..97a5db87a 100644 --- a/policyengine_api/data/v2/catalog/publication.py +++ b/policyengine_api/data/v2/catalog/publication.py @@ -9,6 +9,10 @@ import sqlalchemy as sa from sqlalchemy import Connection, Engine +from policyengine_api.data.v2.catalog.publication_parameter_search_file import ( + assert_parameter_search_files_written, + write_parameter_search_files, +) from policyengine_api.data.v2.catalog.publication_parameter_tree import ( assert_parameter_trees_written, write_parameter_trees, @@ -99,11 +103,12 @@ def publish_catalog( ) -> PublicationEvidence: """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. + In the same transaction it writes the parameter tree and the parameter + search file 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 them. Other + stored versions, and an installed version stored before catalog format 2, + keep having none. """ started = time.monotonic() with engine.begin() as connection: @@ -125,6 +130,9 @@ def publish_catalog( if country.country_id not in existing: publish_new_country(connection, country) parameter_trees_written = write_parameter_trees(connection, catalog.countries) + parameter_search_files_written = write_parameter_search_files( + connection, catalog.countries + ) if checkpoint is not None: checkpoint("after_reconciliation", connection) @@ -132,6 +140,7 @@ def publish_catalog( assert_country_matches(connection, country) assert_canonical_value_uniqueness(connection) assert_parameter_trees_written(connection, catalog.countries) + assert_parameter_search_files_written(connection, catalog.countries) if _protected_row_counts(connection) != before: msg = "publication changed simulation, report, or dataset-version rows" raise CatalogPublicationError(msg) @@ -151,6 +160,7 @@ def publish_catalog( fallback_summaries=fallback_summaries, elapsed_seconds=time.monotonic() - started, parameter_trees_written=parameter_trees_written, + parameter_search_files_written=parameter_search_files_written, ) diff --git a/policyengine_api/data/v2/catalog/publication_parameter_search_file.py b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py new file mode 100644 index 000000000..1debd312b --- /dev/null +++ b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py @@ -0,0 +1,122 @@ +"""Write the parameter search file of each catalog version being published. + +A file is built from the installed packages' catalog, never from rows an +earlier release stored, so publication writes one only for the versions it +publishes: each new version, and the installed version when an earlier +release stored it before search files existed. Writing only inserts. A stored +file of the same format must equal the one built from the installed packages, +because the stored version's other rows already match them. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from uuid import uuid4 + +import sqlalchemy as sa + +from policyengine_api.data.v2.catalog.parameter_search_file import ( + PARAMETER_SEARCH_FILE_FORMAT, + country_search_file, + encode_parameter_search_file, +) +from policyengine_api.data.v2.catalog.publication_types import ( + CatalogPublicationError, +) +from policyengine_api.data.v2.models import V2_METADATA + +if TYPE_CHECKING: + from collections.abc import Sequence + from uuid import UUID + + from sqlalchemy import Connection + + from policyengine_api.data.v2.catalog.records import CountryCatalog + +MODELS = V2_METADATA.tables["tax_benefit_models"] +MODEL_VERSIONS = V2_METADATA.tables["tax_benefit_model_versions"] +PARAMETER_SEARCH_FILES = V2_METADATA.tables["parameter_search_files"] + + +def _stored_version_id(connection: Connection, country: CountryCatalog) -> UUID: + version_id: UUID = connection.execute( + sa.select(MODEL_VERSIONS.c.id) + .select_from( + MODEL_VERSIONS.join(MODELS, MODELS.c.id == MODEL_VERSIONS.c.model_id) + ) + .where( + MODELS.c.name == country.model.name, + MODEL_VERSIONS.c.version == country.model_version.version, + ) + ).scalar_one() + return version_id + + +def _stored_content_sha256(connection: Connection, version_id: UUID) -> str | None: + content_sha256: str | None = connection.execute( + sa.select(PARAMETER_SEARCH_FILES.c.content_sha256).where( + PARAMETER_SEARCH_FILES.c.tax_benefit_model_version_id == version_id, + PARAMETER_SEARCH_FILES.c.file_format == PARAMETER_SEARCH_FILE_FORMAT, + ) + ).scalar_one_or_none() + return content_sha256 + + +def write_parameter_search_files( + connection: Connection, + countries: Sequence[CountryCatalog], +) -> tuple[tuple[str, str, int, int], ...]: + """Write the search file of each published version that has none. + + Returns the model name, version and uncompressed and compressed sizes of + each file written, in model name order. Runs inside publication's + transaction and advisory lock, after the versions are stored. + """ + written = [] + for country in sorted(countries, key=lambda item: item.model.name): + version_id = _stored_version_id(connection, country) + encoded = encode_parameter_search_file(country_search_file(country)) + stored_sha256 = _stored_content_sha256(connection, version_id) + if stored_sha256 is None: + connection.execute( + sa.insert(PARAMETER_SEARCH_FILES).values( + id=uuid4(), + tax_benefit_model_version_id=version_id, + file_format=PARAMETER_SEARCH_FILE_FORMAT, + compressed_content=encoded.compressed_content, + content_sha256=encoded.content_sha256, + uncompressed_size=encoded.uncompressed_size, + compressed_size=encoded.compressed_size, + ) + ) + written.append( + ( + country.model.name, + country.model_version.version, + encoded.uncompressed_size, + encoded.compressed_size, + ) + ) + elif stored_sha256 != encoded.content_sha256: + msg = ( + "the stored parameter search file of catalog version " + f"{country.model_version.version} of {country.model.name} differs " + "from the installed packages' catalog" + ) + raise CatalogPublicationError(msg) + return tuple(written) + + +def assert_parameter_search_files_written( + connection: Connection, + countries: Sequence[CountryCatalog], +) -> None: + """Raise CatalogPublicationError if a catalog country's version has no file.""" + for country in countries: + version_id = _stored_version_id(connection, country) + if _stored_content_sha256(connection, version_id) is None: + msg = ( + f"the {country.country_id} catalog version " + f"{country.model_version.version} has no parameter search file" + ) + raise CatalogPublicationError(msg) diff --git a/policyengine_api/data/v2/catalog/publication_types.py b/policyengine_api/data/v2/catalog/publication_types.py index a5b38b6ad..1733c2007 100644 --- a/policyengine_api/data/v2/catalog/publication_types.py +++ b/policyengine_api/data/v2/catalog/publication_types.py @@ -22,6 +22,9 @@ class PublicationEvidence: # 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], ...] = () + # Published catalog versions whose parameter search file publication wrote: + # model, version, and the file's uncompressed and compressed sizes in bytes. + parameter_search_files_written: tuple[tuple[str, str, int, int], ...] = () def as_dict(self) -> dict[str, object]: """Return the evidence as the JSON object printed after a publication.""" @@ -42,5 +45,16 @@ def as_dict(self) -> dict[str, object]: {"model": model_name, "version": version, "rows": rows} for model_name, version, rows in self.parameter_trees_written ], + "parameter_search_files_written": [ + { + "model": model_name, + "version": version, + "uncompressed_bytes": uncompressed, + "compressed_bytes": compressed, + } + for model_name, version, uncompressed, compressed in ( + self.parameter_search_files_written + ) + ], "elapsed_seconds": round(self.elapsed_seconds, 3), } diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 6a9f997ef..a25bde086 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -2,11 +2,15 @@ from __future__ import annotations +import gzip +import hashlib import json import os from concurrent.futures import ThreadPoolExecutor from dataclasses import replace +from datetime import UTC, datetime from pathlib import Path +from types import SimpleNamespace from uuid import uuid4 import pytest @@ -898,3 +902,235 @@ def test_the_catalog_writer_role_publishes_the_parameter_tree( ) == FIXTURE_TREE ) + + +def _stored_search_file(engine: Engine, model_name: str, version: str) -> dict: + """Return a stored version's search file, checking its hash and sizes.""" + with engine.connect() as connection: + row = connection.execute( + text( + "SELECT search_file.file_format, search_file.compressed_content, " + "search_file.content_sha256, search_file.uncompressed_size, " + "search_file.compressed_size " + "FROM parameter_search_files AS search_file " + "JOIN tax_benefit_model_versions AS version " + "ON version.id = search_file.tax_benefit_model_version_id " + "JOIN tax_benefit_models AS model ON model.id = version.model_id " + "WHERE model.name = :model_name AND version.version = :version" + ), + {"model_name": model_name, "version": version}, + ).one() + content = gzip.decompress(row.compressed_content) + assert row.file_format == 1 + assert row.content_sha256 == hashlib.sha256(content).hexdigest() + assert row.uncompressed_size == len(content) + assert row.compressed_size == len(row.compressed_content) + return json.loads(content) + + +def _delete_search_files(engine: Engine, version: str) -> None: + """Make the stored versions of one .py version look stored before the files.""" + with engine.begin() as connection: + connection.execute( + text( + "DELETE FROM parameter_search_files AS search_file " + "USING tax_benefit_model_versions AS version " + "WHERE version.id = search_file.tax_benefit_model_version_id " + "AND version.version = :version" + ), + {"version": version}, + ) + + +def _fixture_search_file(country_id: str, version: str) -> dict: + return { + "format": 1, + "country_id": country_id, + "policyengine_version": version, + "parameters": [ + { + "path": "gov.example.rate", + "label": "Example rate", + "unit": "/1", + "description": "An example rate", + "editable": True, + } + ], + "other_parameters": [], + "nodes": [{"path": "gov.example", "label": "Example policy"}], + } + + +def test_publication_writes_each_new_version_parameter_search_file( + publication_engine: Engine, +) -> None: + catalog = normalized_catalog() + + evidence = publish_catalog(publication_engine, catalog) + + for country in catalog.countries: + version = country.model_version.version + assert _stored_search_file( + publication_engine, country.model.name, version + ) == _fixture_search_file(country.country_id, version) + assert [ + (model_name, version) + for model_name, version, _uncompressed, _compressed in ( + evidence.parameter_search_files_written + ) + ] == [ + (country.model.name, country.model_version.version) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ] + + +def test_republication_writes_only_the_installed_version_search_file( + publication_engine: Engine, +) -> None: + """A stored version gets its file only when its packages are installed. + + Files are built from the installed packages' catalog, never from rows an + earlier release stored, so a version stored before search files existed + gets its file when publication runs with its packages installed, and other + stored versions are left without one. + """ + older = normalized_catalog(policyengine_version="1.0.0") + catalog = normalized_catalog() + older_country_version = older.country("us").model_version.version + current_country_version = catalog.country("us").model_version.version + publish_catalog(publication_engine, older) + publish_catalog(publication_engine, catalog) + _delete_search_files(publication_engine, older_country_version) + _delete_search_files(publication_engine, current_country_version) + before_ids = _identifiers(publication_engine) + before_counts = _counts(publication_engine) + + evidence = publish_catalog(publication_engine, catalog) + again = publish_catalog(publication_engine, catalog) + + assert [ + (model_name, version) + for model_name, version, _uncompressed, _compressed in ( + evidence.parameter_search_files_written + ) + ] == [ + (country.model.name, current_country_version) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ] + assert again.parameter_search_files_written == () + for country in catalog.countries: + assert _stored_search_file( + publication_engine, country.model.name, current_country_version + ) == _fixture_search_file(country.country_id, current_country_version) + with publication_engine.connect() as connection: + assert ( + connection.execute( + text( + "SELECT count(*) FROM parameter_search_files AS search_file " + "JOIN tax_benefit_model_versions AS version " + "ON version.id = search_file.tax_benefit_model_version_id " + "WHERE version.version = :version" + ), + {"version": older_country_version}, + ).scalar_one() + == 0 + ) + assert _identifiers(publication_engine) == before_ids + assert _counts(publication_engine) == before_counts + + +def test_republication_refuses_a_stored_search_file_that_differs( + publication_engine: Engine, +) -> None: + catalog = normalized_catalog() + publish_catalog(publication_engine, catalog) + with publication_engine.begin() as connection: + connection.execute( + text("UPDATE parameter_search_files SET content_sha256 = repeat('0', 64)") + ) + + with pytest.raises( + CatalogPublicationError, + match=r"stored parameter search file .* differs from the installed packages'", + ): + publish_catalog(publication_engine, catalog) + + +def test_search_file_editability_follows_the_stored_current_law_values( + publication_engine: Engine, +) -> None: + models = source_models() + add_format_2_variable_records(models["us"]) + bracket_rate = SimpleNamespace( + name="gov.example.bracket[1].rate", + label=None, + description=None, + data_type=float, + unit="/1", + economy=True, + household=True, + parameter_values=[ + SimpleNamespace( + value=0.02, + start_date=datetime(2015, 1, 1, tzinfo=UTC), + end_date=datetime(2020, 7, 7, tzinfo=UTC), + ), + SimpleNamespace( + value=None, + start_date=datetime(2020, 7, 8, tzinfo=UTC), + end_date=datetime(2021, 3, 30, tzinfo=UTC), + ), + SimpleNamespace( + value=0.02, + start_date=datetime(2021, 3, 31, tzinfo=UTC), + end_date=None, + ), + ], + ) + models["us"].parameters.append(bracket_rate) + models["us"].parameters_by_name[bracket_rate.name] = bracket_rate + catalog = extract_catalog( + bundle=bundle(), + policyengine_version=POLICYENGINE_VERSION, + models=models, + installed_version=installed_version, + ) + + publish_catalog(publication_engine, catalog) + + us = catalog.country("us") + search_file = _stored_search_file( + publication_engine, us.model.name, us.model_version.version + ) + entries = {entry["path"]: entry for entry in search_file["parameters"]} + assert entries["gov.example.bracket[1].rate"] == { + "path": "gov.example.bracket[1].rate", + "label": None, + "unit": "/1", + "description": None, + "editable": True, + "editable_changes": [ + {"start_date": "2020-07-08", "editable": False}, + {"start_date": "2021-03-31", "editable": True}, + ], + } + # A list of variable names is shown as "Can't be changed here". + assert entries["gov.example.countable_income"]["editable"] is False + assert entries["gov.example.deductions"]["editable"] is False + assert entries["gov.example.rate"]["editable"] is True + + +def test_publication_fails_when_a_published_version_has_no_search_file( + publication_engine: Engine, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + publication_module, + "write_parameter_search_files", + lambda _connection, _countries: (), + ) + + with pytest.raises(CatalogPublicationError, match="has no parameter search file"): + publish_catalog(publication_engine, normalized_catalog()) + + assert _counts(publication_engine)["tax_benefit_model_versions"] == 0 diff --git a/tests/unit/v2/test_catalog_publication.py b/tests/unit/v2/test_catalog_publication.py index 3eb8ef253..772ef228a 100644 --- a/tests/unit/v2/test_catalog_publication.py +++ b/tests/unit/v2/test_catalog_publication.py @@ -280,6 +280,7 @@ def test_completion_evidence_contains_only_reviewed_non_secret_fields() -> None: fallback_summaries=(("us", "state", 3),), elapsed_seconds=2.3456, parameter_trees_written=(("policyengine-uk", "6.7.5", 2_430),), + parameter_search_files_written=(("policyengine-uk", "6.7.5", 412_000, 61_000),), ).as_dict() assert evidence == { @@ -297,6 +298,14 @@ def test_completion_evidence_contains_only_reviewed_non_secret_fields() -> None: "rows": 2_430, } ], + "parameter_search_files_written": [ + { + "model": "policyengine-uk", + "version": "6.7.5", + "uncompressed_bytes": 412_000, + "compressed_bytes": 61_000, + } + ], "elapsed_seconds": 2.346, } assert ( From b8dd4d245b4a89998347d1926fbd8f77084fd100 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:30:03 +0400 Subject: [PATCH 04/19] Serve a catalog version's parameter search file at /v2/parameters/search-file The route reads the selected version's stored file of the current format with one indexed read and sends the stored gzip bytes unchanged to a client that accepts gzip, or the uncompressed JSON to any other. Each representation has a strong ETag from the SHA-256 of the uncompressed JSON, and a request whose If-None-Match names either gets 304. A named catalog version may be cached for a day and the running version for five minutes. A version without a file returns 503. The route reads its query through the shared strict catalog model, so unknown or repeated parameters and an unsupported country return 422, and OpenAPI declares the file's content schema and response headers. Co-Authored-By: Claude Opus 5.5 --- .../v2plan-1f-118-search-file.added.md | 1 + docs/engineering/migration-contracts.md | 3 +- docs/generated/migration_contracts.json | 17 +- docs/migration/stage-9-v2-metadata.md | 11 + .../fastapi_routes/metadata_protocols.py | 12 + .../v2/metadata/parameter_routes.py | 42 ++- .../v2/metadata/response_models.py | 54 +++ .../v2/metadata/search_file_response.py | 94 ++++++ .../reads_parameter_search_file.py | 40 +++ .../services/v2/metadata/services.py | 37 ++ .../services/v2/metadata/transformations.py | 21 ++ .../services/v2/metadata/types.py | 17 + .../services/v2/metadata/validators.py | 9 + tests/contract/registry.py | 14 + tests/integration/test_v2_metadata_routes.py | 71 ++++ .../unit/test_migration_contract_artifacts.py | 2 +- tests/unit/v2/test_metadata_route_caching.py | 7 +- tests/unit/v2/test_metadata_routes.py | 3 +- .../v2/test_metadata_search_file_routes.py | 319 ++++++++++++++++++ tests/unit/v2/test_metadata_service.py | 57 ++++ tests/unit/v2/test_parameter_search_file.py | 25 ++ 21 files changed, 848 insertions(+), 8 deletions(-) create mode 100644 changelog.d/v2plan-1f-118-search-file.added.md create mode 100644 policyengine_api/fastapi_routes/v2/metadata/search_file_response.py create mode 100644 policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py create mode 100644 tests/unit/v2/test_metadata_search_file_routes.py diff --git a/changelog.d/v2plan-1f-118-search-file.added.md b/changelog.d/v2plan-1f-118-search-file.added.md new file mode 100644 index 000000000..9880e1022 --- /dev/null +++ b/changelog.d/v2plan-1f-118-search-file.added.md @@ -0,0 +1 @@ +Catalog publication writes a parameter search file for each catalog version it publishes into a new table, parameter_search_files, built from the installed packages' catalog: every parameter a reform can change with its path, stored label, unit, description and whether a draft can edit its current-law value (with the dates from which that changes), the stored labels of the other parameters under gov, and every labelled node under gov, as gzip-compressed JSON with its SHA-256. A new route, /v2/parameters/search-file, serves a version's file with a strong ETag, 304 revalidation and long-lived caching for a named version, as gzip to clients that accept it. A version stored by an earlier release has no file and returns 503. diff --git a/docs/engineering/migration-contracts.md b/docs/engineering/migration-contracts.md index 5ebf6d51f..556de0a1f 100644 --- a/docs/engineering/migration-contracts.md +++ b/docs/engineering/migration-contracts.md @@ -8,7 +8,7 @@ Generated from `policyengine_api/migration_registry.py` and `tests/contract/regi | --- | ---: | | route group count | 9 | | workflow count | 16 | -| request count | 89 | +| request count | 90 | | db entity count | 6 | | sim flow count | 3 | @@ -210,6 +210,7 @@ Generated from `policyengine_api/migration_registry.py` and `tests/contract/regi | `GET` | `/v2/datasets/{dataset_id}?country_id=us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.item` | | `GET` | `/v2/regions/{region_id}?country_id=us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.item` | | `GET` | `/v2/regions/by-code/state/ca?country_id=us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.item` | +| `GET` | `/v2/parameters/search-file?country_id=us` | 200 | `metadata` | none | `format`, `country_id`, `policyengine_version`, `parameters`, `other_parameters`, `nodes` | | `GET` | `/v2/tax-benefit-models/by-country/us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.model`, `result.model_version` | | `GET` | `/v2/economy-options?country_id=us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.current_law_id`, `result.region`, `result.time_period`, `result.datasets` | | `GET` | `/v2/model-catalog?country_id=us` | 200 | `metadata` | none | `status`, `message`, `result.policyengine_version`, `result.catalog_format`, `result.entities`, `result.basic_inputs`, `result.regions`, `result.datasets`, `result.economy_options` | diff --git a/docs/generated/migration_contracts.json b/docs/generated/migration_contracts.json index b49108e4a..26b037711 100644 --- a/docs/generated/migration_contracts.json +++ b/docs/generated/migration_contracts.json @@ -1,7 +1,7 @@ { "metadata": { "db_entity_count": 6, - "request_count": 89, + "request_count": 90, "route_group_count": 9, "sim_flow_count": 3, "workflow_count": 16 @@ -1552,6 +1552,21 @@ "result.item" ] }, + { + "expected_status": 200, + "headers": [], + "method": "GET", + "path": "/v2/parameters/search-file?country_id=us", + "route_group": "metadata", + "stable_response_fields": [ + "format", + "country_id", + "policyengine_version", + "parameters", + "other_parameters", + "nodes" + ] + }, { "expected_status": 200, "headers": [], diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index b170a07cf..833575e89 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -363,6 +363,17 @@ file. A later file format adds a row for the version instead of changing the stored one. The evidence's `parameter_search_files_written` lists each file written with its model, version and uncompressed and compressed sizes in bytes. +`/v2/parameters/search-file` (query `country_id` and optional +`policyengine_version`) serves the selected version's file of the current +format. A client that accepts gzip receives the stored bytes with +`Content-Encoding: gzip`; any other client receives the uncompressed JSON. The +body is the file itself, not the `status`/`result` envelope. Each +representation has its own strong `ETag`, made from the SHA-256 of the +uncompressed JSON (`""`, or `"-gzip"` for the gzip one). The +cache lifetime, `Vary` and the 304 for a matching `If-None-Match` are those of +every metadata route. A version without a file of the current format returns +503. + ## Validation evidence and retry Successful JSON evidence contains: diff --git a/policyengine_api/fastapi_routes/metadata_protocols.py b/policyengine_api/fastapi_routes/metadata_protocols.py index eaac7cdaf..5c9816ff3 100644 --- a/policyengine_api/fastapi_routes/metadata_protocols.py +++ b/policyengine_api/fastapi_routes/metadata_protocols.py @@ -22,6 +22,7 @@ MetadataModelVersionDetail, MetadataPageResult, MetadataParameterChild, + MetadataParameterSearchFile, MetadataParameterSummary, MetadataRegion, MetadataVariable, @@ -166,6 +167,17 @@ def list_parameter_children( """ ... + def get_parameter_search_file( + self, + country_id: str, + policyengine_version: str | None = None, + ) -> MetadataParameterSearchFile: + """Return the selected catalog version's stored parameter search file. + + Raise ``ParameterSearchFileUnavailableError`` when the version has none. + """ + ... + def list_parameter_values( self, country_id: str, diff --git a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py index b28148a52..a376cd967 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py +++ b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py @@ -6,13 +6,14 @@ from uuid import UUID from fastapi import APIRouter, Request -from starlette.responses import JSONResponse +from starlette.responses import JSONResponse, Response from policyengine_api.fastapi_routes.v2.metadata.common import ( METADATA_RESPONSES, PARAMETER_TREE_ERROR_RESPONSES, CacheableMetadataRoute, read_resource, + run_metadata_read, take_utc_instant_for_read, ) from policyengine_api.fastapi_routes.v2.metadata.query_dependencies import ( @@ -25,9 +26,14 @@ MetadataParameterChildPageResponse, MetadataParameterDetailResponse, MetadataParameterPageResponse, + MetadataParameterSearchFileContent, MetadataParameterValueDetailResponse, MetadataParameterValuePageResponse, ) +from policyengine_api.fastapi_routes.v2.metadata.search_file_response import ( + SEARCH_FILE_RESPONSE_HEADERS, + parameter_search_file_response, +) from policyengine_api.services.v2.metadata.types import ( ParameterFilter, ParameterTreeLevel, @@ -41,7 +47,7 @@ def build_v2_metadata_parameter_router( dependencies: NativeRouteDependencies, ) -> APIRouter: - """Build the v2 parameter, parameter-tree and parameter-value routes. + """Build the v2 parameter, parameter-tree, search-file and parameter-value routes. Each request opens and closes its own metadata reader, so building the router touches no database. @@ -110,6 +116,38 @@ def list_parameter_children( ), ) + @router.get( + "/parameters/search-file", + response_model=None, + response_class=Response, + responses={ + 200: { + "model": MetadataParameterSearchFileContent, + "description": ( + "The catalog version's parameter search file, gzip-encoded " + "when the request accepts gzip." + ), + "headers": SEARCH_FILE_RESPONSE_HEADERS, + }, + }, + summary="Get a catalog version's parameter search file", + ) + def get_parameter_search_file( + request: Request, + query: CatalogQueryDependency, + ) -> Response: + result = run_metadata_read( + dependencies, + lambda reader: reader.get_parameter_search_file( + query.country_id, query.policyengine_version + ), + ) + if isinstance(result, JSONResponse): + return result + return parameter_search_file_response( + result, accept_encoding=request.headers.get("accept-encoding") + ) + @router.get( "/parameters/{parameter_id}", response_model=MetadataParameterDetailResponse, diff --git a/policyengine_api/fastapi_routes/v2/metadata/response_models.py b/policyengine_api/fastapi_routes/v2/metadata/response_models.py index 697ab6f76..0f86e7e13 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/response_models.py +++ b/policyengine_api/fastapi_routes/v2/metadata/response_models.py @@ -2,6 +2,7 @@ from __future__ import annotations +from datetime import date from typing import Generic, Literal, TypeVar from policyengine_api.services.v2.metadata.types import ( @@ -160,3 +161,56 @@ class MetadataCatalogParametersResponse( MetadataResourceSuccessResponse[MetadataCatalogParametersResult] ): """Success response holding parameters fetched by name, with their values.""" + + +class MetadataParameterSearchFileEditabilityChange(StrictResponseModel): + """From ``start_date`` (an ISO date), whether a draft can edit the value.""" + + start_date: date + editable: bool + + +class MetadataParameterSearchFileParameter(StrictResponseModel): + """A parameter a reform can change, with what v3's search indexes. + + ``label`` is the stored label, or null when none is stored. ``editable`` + says whether a draft can edit the earliest current-law value (a number, a + yes/no value or an infinite threshold); ``editable_changes``, when + present, lists the dates from which that changes. + """ + + path: str + label: str | None + unit: str | None + description: str | None + editable: bool + editable_changes: list[MetadataParameterSearchFileEditabilityChange] | None = None + + +class MetadataParameterSearchFileLabel(StrictResponseModel): + """Another parameter under ``gov``: its path and stored label, or null.""" + + path: str + label: str | None + + +class MetadataParameterSearchFileNode(StrictResponseModel): + """A node under ``gov`` that has a stored label.""" + + path: str + label: str + + +class MetadataParameterSearchFileContent(StrictResponseModel): + """One catalog version's parameter search file, ordered by path. + + It holds what v3's parameter search reads, without the catalog's values. + A reader applies v1's label fallback where a label is null. + """ + + format: int + country_id: str + policyengine_version: str + parameters: list[MetadataParameterSearchFileParameter] + other_parameters: list[MetadataParameterSearchFileLabel] + nodes: list[MetadataParameterSearchFileNode] diff --git a/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py new file mode 100644 index 000000000..95ceb35a9 --- /dev/null +++ b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py @@ -0,0 +1,94 @@ +"""HTTP representation of a catalog version's stored parameter search file. + +The file is stored gzip-compressed, so a client that accepts gzip receives +the stored bytes unchanged with ``Content-Encoding: gzip``, and any other +client receives the uncompressed JSON. Each representation has its own strong +ETag, made from the SHA-256 of the uncompressed JSON (``""``, or +``"-gzip"`` for the gzip one). The metadata route class +(``CacheableMetadataRoute``) then adds the cache lifetime and ``Vary`` and +answers a matching ``If-None-Match`` with 304, as for every metadata route. +""" + +from __future__ import annotations + +import gzip +from typing import TYPE_CHECKING, Any, Final + +from starlette.responses import Response + +from policyengine_api.fastapi_routes.v2.metadata.common import ( + CACHE_HEADER_DESCRIPTIONS, +) + +if TYPE_CHECKING: + from policyengine_api.services.v2.metadata.types import ( + MetadataParameterSearchFile, + ) + +_GZIP_ETAG_SUFFIX: Final = "-gzip" +_GZIP_CODINGS: Final = frozenset({"gzip", "x-gzip"}) + +SEARCH_FILE_RESPONSE_HEADERS: Final[dict[str, dict[str, Any]]] = { + **CACHE_HEADER_DESCRIPTIONS, + "ETag": { + "description": ( + "Strong tag of this representation: the SHA-256 of the uncompressed " + 'JSON, with "-gzip" appended for the gzip-encoded one.' + ), + "schema": {"type": "string"}, + }, + "Content-Encoding": { + "description": "gzip when the request accepts gzip; absent otherwise.", + "schema": {"type": "string"}, + }, +} + + +def _quality(parameters: str) -> float: + for parameter in parameters.split(";"): + name, _, value = parameter.strip().partition("=") + if name.strip().lower() == "q": + try: + return float(value) + except ValueError: + return 0.0 + return 1.0 + + +def accepts_gzip(accept_encoding: str | None) -> bool: + """Return whether an Accept-Encoding value accepts gzip. + + An explicit gzip entry decides; otherwise ``*`` does. A quality of 0 + refuses the coding. + """ + wildcard_quality: float | None = None + for entry in (accept_encoding or "").split(","): + coding, _, parameters = entry.partition(";") + name = coding.strip().lower() + if name in _GZIP_CODINGS: + return _quality(parameters) > 0 + if name == "*": + wildcard_quality = _quality(parameters) + return wildcard_quality is not None and wildcard_quality > 0 + + +def parameter_search_file_response( + search_file: MetadataParameterSearchFile, + *, + accept_encoding: str | None, +) -> Response: + """Return the file in the encoding the request accepts, with its ETag.""" + if accepts_gzip(accept_encoding): + return Response( + content=search_file.compressed_content, + media_type="application/json", + headers={ + "ETag": f'"{search_file.content_sha256}{_GZIP_ETAG_SUFFIX}"', + "Content-Encoding": "gzip", + }, + ) + return Response( + content=gzip.decompress(search_file.compressed_content), + media_type="application/json", + headers={"ETag": f'"{search_file.content_sha256}"'}, + ) diff --git a/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py new file mode 100644 index 000000000..ec8573cce --- /dev/null +++ b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py @@ -0,0 +1,40 @@ +"""Database selection of a catalog version's stored parameter search file.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import sqlalchemy as sa +from sqlmodel import Session, col + +from policyengine_api.data.v2.models import ParameterSearchFile +from policyengine_api.services.v2.metadata.database_connectors.reads import read_rows + +if TYPE_CHECKING: + from uuid import UUID + + +def read_parameter_search_file( + session: Session, + *, + model_version_id: UUID, + file_format: int, +) -> list[Any]: + """Return the version's search file of the given format: one row, or none. + + One read of the unique (version, file format) index. The row has the + file format, the compressed content, the content hash and both sizes. + """ + return read_rows( + session, + sa.select( + col(ParameterSearchFile.file_format).label("file_format"), + col(ParameterSearchFile.compressed_content).label("compressed_content"), + col(ParameterSearchFile.content_sha256).label("content_sha256"), + col(ParameterSearchFile.uncompressed_size).label("uncompressed_size"), + col(ParameterSearchFile.compressed_size).label("compressed_size"), + ).where( + col(ParameterSearchFile.tax_benefit_model_version_id) == model_version_id, + col(ParameterSearchFile.file_format) == file_format, + ), + ) diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index 7b48720fc..889533a6c 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -8,6 +8,9 @@ SelectedCatalog, validate_policyengine_version, ) +from policyengine_api.data.v2.catalog.parameter_search_file import ( + PARAMETER_SEARCH_FILE_FORMAT, +) from policyengine_api.services.v2.metadata.database_connectors.reads import ( read_metadata_catalog, ) @@ -19,6 +22,9 @@ from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_nodes import ( # noqa: E501 # a dotted module path cannot be wrapped read_parameter_node_labels, ) +from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_search_file import ( # noqa: E501 # a dotted module path cannot be wrapped + read_parameter_search_file, +) 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, @@ -54,6 +60,7 @@ metadata_model_version, metadata_parameter, metadata_parameter_children, + metadata_parameter_search_file, metadata_parameter_value, metadata_region, metadata_variable, @@ -75,6 +82,7 @@ MetadataModelVersionDetail, MetadataPageResult, MetadataParameterChild, + MetadataParameterSearchFile, MetadataParameterSummary, MetadataRegion, MetadataVariable, @@ -83,6 +91,7 @@ ParameterValueFilter, ) from policyengine_api.services.v2.metadata.validators import ( + ParameterSearchFileUnavailableError, ParameterTreeUnavailableError, require_metadata_resource, validate_economy_options, @@ -389,6 +398,34 @@ def list_parameter_children( limit=limit, ) + def get_parameter_search_file( + self, + country_id: str, + policyengine_version: str | None = None, + ) -> MetadataParameterSearchFile: + """Return the selected catalog version's stored parameter search file. + + Raise ParameterSearchFileUnavailableError when the version has no file + of the current format: publication writes one only for the versions it + publishes. + """ + selected = self._select_catalog(country_id, policyengine_version) + search_file = metadata_parameter_search_file( + selected, + read_parameter_search_file( + self._database_session.session, + model_version_id=selected.model_version.id, + file_format=PARAMETER_SEARCH_FILE_FORMAT, + ), + ) + if search_file is None: + msg = ( + f"catalog version {selected.policyengine_version} has no parameter " + "search file" + ) + raise ParameterSearchFileUnavailableError(msg) + return search_file + def list_parameter_values( self, country_id: str, diff --git a/policyengine_api/services/v2/metadata/transformations.py b/policyengine_api/services/v2/metadata/transformations.py index 83bf941ed..4566097a8 100644 --- a/policyengine_api/services/v2/metadata/transformations.py +++ b/policyengine_api/services/v2/metadata/transformations.py @@ -25,6 +25,7 @@ MetadataModelVersionDetail, MetadataPageResult, MetadataParameterChild, + MetadataParameterSearchFile, MetadataParameterSummary, MetadataRegion, MetadataRegionOption, @@ -349,6 +350,26 @@ def national_region(regions: list[Region], country_id: str) -> Region | None: return next((region for region in regions if region.code == country_id), None) +def metadata_parameter_search_file( + selected: SelectedCatalog, rows: list[Any] +) -> MetadataParameterSearchFile | None: + """Build the selected version's search file from read_parameter_search_file rows. + + Return None when the version has no stored file of the format read. + """ + if not rows: + return None + row = rows[0] + return MetadataParameterSearchFile( + policyengine_version=selected.policyengine_version, + file_format=row.file_format, + compressed_content=bytes(row.compressed_content), + content_sha256=row.content_sha256, + uncompressed_size=row.uncompressed_size, + compressed_size=row.compressed_size, + ) + + def metadata_economy_options( selected: SelectedCatalog, *, regions: list[Region], national_dataset: Dataset ) -> MetadataEconomyOptionsResult: diff --git a/policyengine_api/services/v2/metadata/types.py b/policyengine_api/services/v2/metadata/types.py index 7d10cfc25..bdfa3da73 100644 --- a/policyengine_api/services/v2/metadata/types.py +++ b/policyengine_api/services/v2/metadata/types.py @@ -66,6 +66,23 @@ class ParameterNodeLabelRow(NamedTuple): label: str | None +@dataclass(frozen=True) +class MetadataParameterSearchFile: + """A catalog version's stored parameter search file and what identifies it. + + ``compressed_content`` is gzip-compressed JSON; ``content_sha256`` is the + SHA-256 of the uncompressed JSON, so it names the content whichever + encoding a response uses. + """ + + policyengine_version: str + file_format: int + compressed_content: bytes + content_sha256: str + uncompressed_size: int + compressed_size: int + + @dataclass(frozen=True) class ParameterTreeLevel: """Which level of a catalog version's parameter tree a listing reads. diff --git a/policyengine_api/services/v2/metadata/validators.py b/policyengine_api/services/v2/metadata/validators.py index 62e62a802..66c8f0885 100644 --- a/policyengine_api/services/v2/metadata/validators.py +++ b/policyengine_api/services/v2/metadata/validators.py @@ -24,6 +24,15 @@ class InvalidMetadataPageError(ValueError): """Raised when collection pagination is outside the documented bounds.""" +class ParameterSearchFileUnavailableError(MetadataCatalogUnavailableError): + """Raised when a selected catalog version has no parameter search file. + + Catalog publication writes a version's file only when it publishes the + version, from the installed packages' catalog, so a version stored by an + earlier release before search files existed has none. + """ + + class ParameterTreeUnavailableError(MetadataCatalogUnavailableError): """Raised when a selected catalog version has no parameter tree. diff --git a/tests/contract/registry.py b/tests/contract/registry.py index 77b2cbf28..c0e70f0f1 100644 --- a/tests/contract/registry.py +++ b/tests/contract/registry.py @@ -616,6 +616,20 @@ class WorkflowContract: "/v2/regions/by-code/state/ca?country_id=us", ) ), + ContractRequest( + method="GET", + path="/v2/parameters/search-file?country_id=us", + expected_status=200, + stable_response_fields=( + "format", + "country_id", + "policyengine_version", + "parameters", + "other_parameters", + "nodes", + ), + route_group="metadata", + ), ContractRequest( method="GET", path="/v2/tax-benefit-models/by-country/us", diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index 7e50322ce..b625e147f 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -2,6 +2,8 @@ from __future__ import annotations +import gzip +import json import os from datetime import UTC, datetime from pathlib import Path @@ -20,6 +22,9 @@ 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.parameter_search_file import ( + country_search_file, +) from policyengine_api.data.v2.catalog.publication import publish_catalog from policyengine_api.data.v2.models import ( Dataset, @@ -1059,3 +1064,69 @@ def test_postgres_a_version_stored_before_format_2_returns_the_tree_503( ), } assert "cache-control" not in response.headers + + +def test_postgres_search_file_is_served_with_its_etag_and_revalidated( + published_engine: Engine, +) -> None: + client = _client(published_engine) + expected = country_search_file(normalized_catalog().country("uk")) + + response = client.get( + "/v2/parameters/search-file", + params={"country_id": "uk", "policyengine_version": POLICYENGINE_VERSION}, + headers={"Accept-Encoding": "gzip"}, + ) + revalidated = client.get( + "/v2/parameters/search-file", + params={"country_id": "uk", "policyengine_version": POLICYENGINE_VERSION}, + headers={ + "Accept-Encoding": "gzip", + "If-None-Match": response.headers["etag"], + }, + ) + + assert response.status_code == 200, response.text + assert response.headers["content-encoding"] == "gzip" + assert response.headers["cache-control"] == "public, max-age=86400" + assert response.json() == expected + assert revalidated.status_code == 304 + assert revalidated.headers["etag"] == response.headers["etag"] + + +def test_postgres_search_file_of_a_version_without_one_is_unavailable( + published_engine: Engine, +) -> None: + with published_engine.begin() as connection: + connection.execute(text("DELETE FROM parameter_search_files")) + + response = _client(published_engine).get( + "/v2/parameters/search-file", params={"country_id": "us"} + ) + + assert response.status_code == 503 + assert response.json() == { + "status": "error", + "message": "V2 metadata catalog is unavailable", + } + + +def test_postgres_search_file_reads_as_the_runtime_role( + published_engine: Engine, + managed_v2_roles, + v2_engine_as_role, +) -> None: + """The Public API's runtime role reads the file with its declared privileges.""" + runtime_engine = v2_engine_as_role(managed_v2_roles.runtime, poolclass=NullPool) + try: + with Session(runtime_engine) as session: + search_file = V2MetadataService( + MetadataDatabaseSession(session), + running_policyengine_version=POLICYENGINE_VERSION, + ).get_parameter_search_file("us") + finally: + runtime_engine.dispose() + + assert json.loads(gzip.decompress(search_file.compressed_content)) == ( + country_search_file(normalized_catalog().country("us")) + ) diff --git a/tests/unit/test_migration_contract_artifacts.py b/tests/unit/test_migration_contract_artifacts.py index fc446b348..497d5489b 100644 --- a/tests/unit/test_migration_contract_artifacts.py +++ b/tests/unit/test_migration_contract_artifacts.py @@ -14,7 +14,7 @@ def test_migration_contract_payload_summarizes_route_contracts(): assert payload["metadata"] == { "route_group_count": 9, "workflow_count": 16, - "request_count": 89, + "request_count": 90, "db_entity_count": 6, "sim_flow_count": 3, } diff --git a/tests/unit/v2/test_metadata_route_caching.py b/tests/unit/v2/test_metadata_route_caching.py index ff1cdbd53..7db0f0cc4 100644 --- a/tests/unit/v2/test_metadata_route_caching.py +++ b/tests/unit/v2/test_metadata_route_caching.py @@ -332,10 +332,13 @@ def test_openapi_declares_caching_headers_and_not_modified_responses() -> None: ) ] - assert len(metadata_paths) == 21 + assert len(metadata_paths) == 22 for path in metadata_paths: responses = schema["paths"][path]["get"]["responses"] - assert set(responses["200"]["headers"]) == {"ETag", "Cache-Control", "Vary"} + expected_headers = {"ETag", "Cache-Control", "Vary"} + if path == "/v2/parameters/search-file": + expected_headers.add("Content-Encoding") + assert set(responses["200"]["headers"]) == expected_headers assert "$ref" in responses["200"]["content"]["application/json"]["schema"] assert set(responses["304"]["headers"]) == {"ETag", "Cache-Control", "Vary"} assert "content" not in responses["304"] diff --git a/tests/unit/v2/test_metadata_routes.py b/tests/unit/v2/test_metadata_routes.py index 8c8cfde71..6f7aae73e 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -314,6 +314,7 @@ def test_openapi_references_explicit_resource_response_schemas() -> None: "/v2/model-catalog/parameters", "/v2/parameters", "/v2/parameters/children", + "/v2/parameters/search-file", "/v2/parameters/{parameter_id}", "/v2/parameter-values", "/v2/parameter-values/{value_id}", @@ -748,7 +749,7 @@ def test_every_metadata_route_declares_the_shared_catalog_query() -> None: ) ] - assert len(metadata_paths) == 21 + assert len(metadata_paths) == 22 for path in metadata_paths: parameters = { parameter["name"]: parameter diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py new file mode 100644 index 000000000..abeee31aa --- /dev/null +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -0,0 +1,319 @@ +"""Route coverage for the parameter search file at /v2/parameters/search-file.""" + +from __future__ import annotations + +import gzip +from datetime import date + +import pytest + +from policyengine_api.data.v2.catalog.parameter_search_file import ( + DraftabilityChange, + SearchNodeSource, + SearchParameterSource, + build_parameter_search_file, + encode_parameter_search_file, +) +from policyengine_api.fastapi_routes.v2.metadata.response_models import ( + MetadataParameterSearchFileContent, +) +from policyengine_api.services.v2.metadata.types import ( + MetadataParameterSearchFile, +) +from policyengine_api.services.v2.metadata.validators import ( + ParameterSearchFileUnavailableError, +) +from tests.fixtures.v2_metadata_routes import ResourceReader, metadata_route_client + +SEARCH_FILE_PATH = "/v2/parameters/search-file" + + +SEARCH_FILE_CONTENT = { + "format": 1, + "country_id": "us", + "policyengine_version": "4.20.3", + "parameters": [ + { + "path": "gov.example.rate", + "label": "Rate", + "unit": "/1", + "description": None, + "editable": True, + } + ], + "other_parameters": [], + "nodes": [{"path": "gov.example", "label": "Example"}], +} + + +def _search_file_reader(*, error: Exception | None = None) -> ResourceReader: + encoded = encode_parameter_search_file(SEARCH_FILE_CONTENT) + stored = MetadataParameterSearchFile( + policyengine_version="4.20.3", + file_format=1, + compressed_content=encoded.compressed_content, + content_sha256=encoded.content_sha256, + uncompressed_size=encoded.uncompressed_size, + compressed_size=encoded.compressed_size, + ) + return ResourceReader({"get_parameter_search_file": stored}, error=error) + + +def _content_sha256() -> str: + return encode_parameter_search_file(SEARCH_FILE_CONTENT).content_sha256 + + +def test_search_file_route_sends_the_stored_gzip_bytes_to_a_gzip_client() -> None: + reader = _search_file_reader() + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, + params={"country_id": "US", "policyengine_version": "4.20.3"}, + headers={"Accept-Encoding": "gzip, deflate"}, + ) + + encoded = encode_parameter_search_file(SEARCH_FILE_CONTENT) + assert response.status_code == 200 + assert response.headers["content-encoding"] == "gzip" + assert response.headers["content-length"] == str(encoded.compressed_size) + assert response.headers["content-type"] == "application/json" + assert response.headers["etag"] == f'"{encoded.content_sha256}-gzip"' + assert response.headers["cache-control"] == "public, max-age=86400" + assert response.headers["vary"] == "Origin, Accept-Encoding" + assert response.json() == SEARCH_FILE_CONTENT + assert reader.calls == [("get_parameter_search_file", ("us", "4.20.3"), {})] + assert reader.closed + + +def test_search_file_route_sends_uncompressed_json_to_other_clients() -> None: + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us", "policyengine_version": "4.20.3"}, + headers={"Accept-Encoding": "identity"}, + ) + + encoded = encode_parameter_search_file(SEARCH_FILE_CONTENT) + assert response.status_code == 200 + assert "content-encoding" not in response.headers + assert response.content == gzip.decompress(encoded.compressed_content) + assert response.headers["etag"] == f'"{encoded.content_sha256}"' + + +def test_a_large_uncompressed_search_file_names_accept_encoding_once_in_vary() -> None: + content = { + **SEARCH_FILE_CONTENT, + "parameters": [ + { + "path": f"gov.example.rate_{index}", + "label": f"Rate {index}", + "unit": "/1", + "description": None, + "editable": True, + } + for index in range(200) + ], + } + encoded = encode_parameter_search_file(content) + stored = MetadataParameterSearchFile( + policyengine_version="4.20.3", + file_format=1, + compressed_content=encoded.compressed_content, + content_sha256=encoded.content_sha256, + uncompressed_size=encoded.uncompressed_size, + compressed_size=encoded.compressed_size, + ) + + response = metadata_route_client( + lambda: ResourceReader({"get_parameter_search_file": stored}) + ).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"Accept-Encoding": "identity"}, + ) + + assert encoded.uncompressed_size > 1000 + assert response.status_code == 200 + assert response.headers["vary"] == "Origin, Accept-Encoding" + assert response.json() == content + + +@pytest.mark.parametrize("accept_encoding", ["gzip;q=0", "br, gzip;q=0.0"]) +def test_search_file_route_respects_a_refused_gzip_encoding( + accept_encoding: str, +) -> None: + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"Accept-Encoding": accept_encoding}, + ) + + assert response.status_code == 200 + assert "content-encoding" not in response.headers + assert response.headers["etag"] == f'"{_content_sha256()}"' + + +def test_search_file_route_lets_the_running_version_be_cached_briefly() -> None: + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, params={"country_id": "us"} + ) + + assert response.status_code == 200 + assert response.headers["cache-control"] == "public, max-age=300" + + +@pytest.mark.parametrize( + ("if_none_match", "accept_encoding", "expected_etag_suffix"), + [ + ('"{sha}-gzip"', "gzip", "-gzip"), + ('W/"{sha}-gzip"', "gzip", "-gzip"), + ('"other", "{sha}"', "identity", ""), + ("*", "identity", ""), + ], +) +def test_search_file_route_answers_a_matching_revalidation_with_304( + if_none_match: str, + accept_encoding: str, + expected_etag_suffix: str, +) -> None: + sha = _content_sha256() + + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us", "policyengine_version": "4.20.3"}, + headers={ + "If-None-Match": if_none_match.format(sha=sha), + "Accept-Encoding": accept_encoding, + }, + ) + + assert response.status_code == 304 + assert response.content == b"" + assert response.headers["etag"] == f'"{sha}{expected_etag_suffix}"' + assert response.headers["cache-control"] == "public, max-age=86400" + assert response.headers["vary"] == "Origin, Accept-Encoding" + assert "content-encoding" not in response.headers + + +def test_a_tag_of_the_other_representation_does_not_revalidate() -> None: + # Strong tags differ by encoding, so a client switching to gzip refetches. + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"If-None-Match": f'"{_content_sha256()}"', "Accept-Encoding": "gzip"}, + ) + + assert response.status_code == 200 + assert response.headers["etag"] == f'"{_content_sha256()}-gzip"' + + +def test_search_file_route_sends_the_file_when_the_etag_differs() -> None: + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"If-None-Match": '"0000"', "Accept-Encoding": "gzip"}, + ) + + assert response.status_code == 200 + assert response.json() == SEARCH_FILE_CONTENT + + +def test_search_file_route_reports_a_missing_file_as_unavailable() -> None: + reader = _search_file_reader( + error=ParameterSearchFileUnavailableError("search file not written") + ) + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, params={"country_id": "us"} + ) + + assert response.status_code == 503 + assert response.json() == { + "status": "error", + "message": "V2 metadata catalog is unavailable", + } + assert "etag" not in response.headers + assert "cache-control" not in response.headers + assert reader.closed + + +@pytest.mark.parametrize( + "query", + [ + "country_id=us&format=1", + "country_id=us&country_id=uk", + "country_id=ca", + "", + "country_id=us&policyengine_version=4.20.3&policyengine_version=4.20.4", + ], +) +def test_search_file_route_rejects_invalid_queries(query: str) -> None: + calls = [] + + def factory(): + calls.append("called") + return _search_file_reader() + + response = metadata_route_client(factory).get(f"{SEARCH_FILE_PATH}?{query}") + + assert response.status_code == 422 + assert response.json() == { + "status": "error", + "message": "Invalid API v2 request", + } + assert calls == [] + + +def test_search_file_openapi_declares_its_query_and_content() -> None: + schema = metadata_route_client(_search_file_reader).get("/v2/openapi.json").json() + + operation = schema["paths"][SEARCH_FILE_PATH]["get"] + parameters = {parameter["name"]: parameter for parameter in operation["parameters"]} + assert set(parameters) == {"country_id", "policyengine_version"} + assert parameters["country_id"]["required"] is True + assert parameters["policyengine_version"]["required"] is False + ok = operation["responses"]["200"] + assert ok["content"]["application/json"]["schema"] == { + "$ref": "#/components/schemas/MetadataParameterSearchFileContent" + } + assert {"ETag", "Cache-Control", "Content-Encoding", "Vary"} <= set(ok["headers"]) + assert "304" in operation["responses"] + assert "MetadataParameterSearchFileParameter" in schema["components"]["schemas"] + + +def test_the_built_search_file_matches_the_declared_content_schema() -> None: + search_file = build_parameter_search_file( + country_id="uk", + policyengine_version="6.10.0", + parameters=[ + SearchParameterSource( + name="gov.hmrc.stamp_duty.rate[1].rate", + label=None, + unit="/1", + description="A band's rate.", + economy=True, + household=True, + ), + SearchParameterSource( + name="gov.economic_assumptions.cpi", + label="CPI", + unit=None, + description=None, + economy=False, + household=False, + ), + ], + nodes=[SearchNodeSource(name="gov.hmrc", label="HMRC")], + draftability=[ + DraftabilityChange( + "gov.hmrc.stamp_duty.rate[1].rate", date(2015, 1, 1), draftable=True + ), + DraftabilityChange( + "gov.hmrc.stamp_duty.rate[1].rate", date(2020, 7, 8), draftable=False + ), + ], + ) + + content = MetadataParameterSearchFileContent.model_validate(search_file) + + assert content.model_dump(mode="json", exclude_unset=True) == search_file + assert content.parameters[0].editable_changes is not None diff --git a/tests/unit/v2/test_metadata_service.py b/tests/unit/v2/test_metadata_service.py index a843a70a0..ef80afe41 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -17,9 +17,14 @@ MetadataCatalogVersionNotFoundError, UnsupportedPreviewCountryError, ) +from policyengine_api.data.v2.catalog.parameter_search_file import ( + country_search_file, + encode_parameter_search_file, +) from policyengine_api.data.v2.models import ( Dataset, Parameter, + ParameterSearchFile, ParameterValue, Region, Variable, @@ -41,6 +46,7 @@ from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterSearchFileUnavailableError, ) from tests.fixtures.v2_catalog import POLICYENGINE_VERSION, normalized_catalog from tests.fixtures.v2_metadata_service_helpers import ( @@ -618,3 +624,54 @@ def test_resource_entrypoints_are_defined_in_the_service_module() -> None: for method_name in method_names: method = getattr(V2MetadataService, method_name) assert method.__module__.endswith(".services") + + +def test_the_selected_version_search_file_is_returned_as_stored( + catalog_session: Session, +) -> None: + model_version = stored_model_version(catalog_session, "policyengine-us") + encoded = encode_parameter_search_file( + country_search_file(normalized_catalog().country("us")) + ) + catalog_session.add( + ParameterSearchFile( + tax_benefit_model_version_id=model_version.id, + file_format=1, + compressed_content=encoded.compressed_content, + content_sha256=encoded.content_sha256, + uncompressed_size=encoded.uncompressed_size, + compressed_size=encoded.compressed_size, + ) + ) + catalog_session.commit() + + search_file = catalog_service(catalog_session).get_parameter_search_file("us") + + assert search_file.policyengine_version == POLICYENGINE_VERSION + assert search_file.file_format == 1 + assert search_file.compressed_content == encoded.compressed_content + assert search_file.content_sha256 == encoded.content_sha256 + assert search_file.uncompressed_size == encoded.uncompressed_size + assert search_file.compressed_size == encoded.compressed_size + + +def test_a_version_without_a_search_file_of_the_current_format_is_unavailable( + catalog_session: Session, +) -> None: + model_version = stored_model_version(catalog_session, "policyengine-us") + catalog_session.add( + ParameterSearchFile( + tax_benefit_model_version_id=model_version.id, + file_format=99, + compressed_content=b"later format", + content_sha256="0" * 64, + uncompressed_size=12, + compressed_size=12, + ) + ) + catalog_session.commit() + + with pytest.raises( + ParameterSearchFileUnavailableError, match="has no parameter search file" + ): + catalog_service(catalog_session).get_parameter_search_file("us") diff --git a/tests/unit/v2/test_parameter_search_file.py b/tests/unit/v2/test_parameter_search_file.py index f5d7a8f4e..e70ba6834 100644 --- a/tests/unit/v2/test_parameter_search_file.py +++ b/tests/unit/v2/test_parameter_search_file.py @@ -7,6 +7,8 @@ import json from datetime import date +import pytest + from policyengine_api.data.v2.catalog.parameter_search_file import ( PARAMETER_SEARCH_FILE_FORMAT, DraftabilityChange, @@ -14,6 +16,7 @@ SearchParameterSource, build_parameter_search_file, encode_parameter_search_file, + is_draftable_value, ) @@ -294,3 +297,25 @@ def test_the_same_catalog_rows_always_encode_to_the_same_bytes() -> None: assert first.compressed_content == second.compressed_content assert first.content_sha256 == second.content_sha256 assert "Ä rate" in gzip.decompress(first.compressed_content).decode("utf-8") + + +@pytest.mark.parametrize( + ("value", "draftable"), + [ + (0.2, True), + (0, True), + (True, True), + (False, True), + ("Infinity", True), + ("-Infinity", True), + (None, False), + (["employment_income"], False), + ("SINGLE", False), + ({"a": 1}, False), + ], +) +def test_draftable_values_follow_v3s_draft_editor( + value: object, *, draftable: bool +) -> None: + # v3's isDraftableValue: a number, a yes/no switch, or "Infinity"/"-Infinity". + assert is_draftable_value(value) is draftable From 79c0f3c2f20f2c5d7f261aea14b654fe9efe6880 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:14:32 +0400 Subject: [PATCH 05/19] Include the labelled gov node in the parameter search file v3's concept clusters pair each node's key with its label, and v1's metadata labels the root node ("Government"), so the file now lists gov itself as well as the labelled nodes under it. Co-Authored-By: Claude Opus 5.5 --- docs/migration/stage-9-v2-metadata.md | 4 ++-- .../data/v2/catalog/parameter_search_file.py | 11 ++++++++--- .../fastapi_routes/v2/metadata/response_models.py | 2 +- tests/unit/v2/test_parameter_search_file.py | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 833575e89..811f66ea5 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -345,8 +345,8 @@ holds what the app's newer interface's parameter search reads: day it was built; - the path and stored label of every other parameter under `gov`, which the search's concept clusters, acronym aliases and member labels read; -- the path and stored label of every node under `gov` that has a stored label, - for breadcrumbs and schedule names. +- the path and stored label of `gov` and every node under it that has a stored + label, for breadcrumbs, schedule names and concept clusters. Labels are stored labels, or null: the search treats a label that only restates its path's last key as a placeholder, so its reader applies v1's diff --git a/policyengine_api/data/v2/catalog/parameter_search_file.py b/policyengine_api/data/v2/catalog/parameter_search_file.py index 7fba7fe7c..b335b7502 100644 --- a/policyengine_api/data/v2/catalog/parameter_search_file.py +++ b/policyengine_api/data/v2/catalog/parameter_search_file.py @@ -133,6 +133,11 @@ def _stored_label(label: str | None) -> str | None: return label if label is not None and label.strip() else None +def _is_gov_node(name: str) -> bool: + """Return whether a node is ``gov`` itself or below it, as in v1's metadata.""" + return name == REFORM_TREE_PREFIX.rstrip(".") or name.startswith(REFORM_TREE_PREFIX) + + def build_parameter_search_file( *, country_id: str, @@ -145,8 +150,8 @@ def build_parameter_search_file( ``parameters`` lists every parameter a reform can change with its search fields and editability; ``other_parameters`` lists the other parameters - under ``gov`` with their labels; ``nodes`` lists every node under ``gov`` - that has a stored label. A parameter without current-law values cannot be + under ``gov`` with their labels; ``nodes`` lists ``gov`` and every node + under it that has a stored label. A parameter without current-law values cannot be edited. """ editability = _editability(draftability) @@ -175,7 +180,7 @@ def build_parameter_search_file( labelled_nodes = [ {"path": node.name, "label": node.label} for node in sorted(nodes, key=lambda node: node.name) - if node.name.startswith(REFORM_TREE_PREFIX) and _stored_label(node.label) + if _is_gov_node(node.name) and _stored_label(node.label) ] return { "format": PARAMETER_SEARCH_FILE_FORMAT, diff --git a/policyengine_api/fastapi_routes/v2/metadata/response_models.py b/policyengine_api/fastapi_routes/v2/metadata/response_models.py index 0f86e7e13..15c78f83e 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/response_models.py +++ b/policyengine_api/fastapi_routes/v2/metadata/response_models.py @@ -195,7 +195,7 @@ class MetadataParameterSearchFileLabel(StrictResponseModel): class MetadataParameterSearchFileNode(StrictResponseModel): - """A node under ``gov`` that has a stored label.""" + """``gov`` or a node under it that has a stored label.""" path: str label: str diff --git a/tests/unit/v2/test_parameter_search_file.py b/tests/unit/v2/test_parameter_search_file.py index e70ba6834..a83719446 100644 --- a/tests/unit/v2/test_parameter_search_file.py +++ b/tests/unit/v2/test_parameter_search_file.py @@ -185,6 +185,20 @@ def test_nodes_carry_their_stored_labels_only() -> None: ] +def test_the_labelled_gov_node_itself_is_included() -> None: + # v3's concept clusters pair each node's key with its label, so the root + # node's "Government" label must be in the file, as v1's metadata has it. + search_file = _file( + [_parameter("gov.hmrc.vat.rate")], + nodes=[ + SearchNodeSource(name="gov", label="Government"), + SearchNodeSource(name="government_bonds", label="Bonds"), + ], + ) + + assert search_file["nodes"] == [{"path": "gov", "label": "Government"}] + + def test_a_value_that_is_not_a_number_or_switch_cannot_be_edited() -> None: search_file = _file( [ From 054cc8569392b5c952c4541966c13823230994c2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:17:35 +0400 Subject: [PATCH 06/19] Type the parameter search file and name its editability consistently The builder now returns a ParameterSearchFileDocument TypedDict whose entry shapes a test compares with the OpenAPI content models, so the two cannot drift apart unnoticed. Nodes are read as the parameter tree's TreeNodeSource, and the per-value property is named editable throughout (EditabilityChange, is_editable_value) instead of draftable. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/parameter_search_file.py | 126 ++++++++++++------ .../v2/test_metadata_search_file_routes.py | 42 ++++-- tests/unit/v2/test_parameter_search_file.py | 87 ++++++------ 3 files changed, 161 insertions(+), 94 deletions(-) diff --git a/policyengine_api/data/v2/catalog/parameter_search_file.py b/policyengine_api/data/v2/catalog/parameter_search_file.py index b335b7502..b2bce9ddf 100644 --- a/policyengine_api/data/v2/catalog/parameter_search_file.py +++ b/policyengine_api/data/v2/catalog/parameter_search_file.py @@ -30,10 +30,11 @@ import json from collections import defaultdict from dataclasses import dataclass -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, TypedDict from policyengine_api.data.v2.catalog.parameter_tree import ( REFORM_TREE_PREFIX, + TreeNodeSource, is_reform_changeable, ) @@ -69,15 +70,7 @@ class SearchParameterSource: @dataclass(frozen=True, slots=True) -class SearchNodeSource: - """A stored parameter node: its name and stored label.""" - - name: str - label: str | None - - -@dataclass(frozen=True, slots=True) -class DraftabilityChange: +class EditabilityChange: """Whether a parameter's current-law value from ``start_date`` can be edited. A value can be edited in a draft when it is a number, a yes/no value or the @@ -86,7 +79,56 @@ class DraftabilityChange: parameter_name: str start_date: date - draftable: bool + editable: bool + + +class EditabilityChangeEntry(TypedDict): + """From ``start_date`` (an ISO date), whether a draft can edit the value.""" + + start_date: str + editable: bool + + +class _SearchFileParameterFields(TypedDict): + path: str + label: str | None + unit: str | None + description: str | None + editable: bool + + +class SearchFileParameterEntry(_SearchFileParameterFields, total=False): + """A parameter a reform can change, as the file lists it. + + ``editable_changes`` is present only when editability changes over time. + """ + + editable_changes: list[EditabilityChangeEntry] + + +class SearchFileLabelEntry(TypedDict): + """Another parameter under ``gov``: its path and stored label, or null.""" + + path: str + label: str | None + + +class SearchFileNodeEntry(TypedDict): + """``gov`` or a node under it that has a stored label.""" + + path: str + label: str + + +class ParameterSearchFileDocument(TypedDict): + """One catalog version's search file, each list ordered by path.""" + + format: int + country_id: str + policyengine_version: str + parameters: list[SearchFileParameterEntry] + other_parameters: list[SearchFileLabelEntry] + nodes: list[SearchFileNodeEntry] @dataclass(frozen=True, slots=True) @@ -104,27 +146,27 @@ class EncodedParameterSearchFile: def _editability( - changes: Iterable[DraftabilityChange], -) -> dict[str, tuple[bool, list[dict[str, Any]]]]: + changes: Iterable[EditabilityChange], +) -> dict[str, tuple[bool, list[EditabilityChangeEntry]]]: """Return each parameter's earliest editability and the later changes.""" - by_parameter: defaultdict[str, list[DraftabilityChange]] = defaultdict(list) + by_parameter: defaultdict[str, list[EditabilityChange]] = defaultdict(list) for change in changes: by_parameter[change.parameter_name].append(change) - editability: dict[str, tuple[bool, list[dict[str, Any]]]] = {} + editability: dict[str, tuple[bool, list[EditabilityChangeEntry]]] = {} for name, parameter_changes in by_parameter.items(): ordered = sorted(parameter_changes, key=lambda change: change.start_date) - earliest = ordered[0].draftable - later: list[dict[str, Any]] = [] + earliest = ordered[0].editable + later: list[EditabilityChangeEntry] = [] current = earliest for change in ordered[1:]: - if change.draftable != current: + if change.editable != current: later.append( { "start_date": change.start_date.isoformat(), - "editable": change.draftable, + "editable": change.editable, } ) - current = change.draftable + current = change.editable editability[name] = (earliest, later) return editability @@ -143,9 +185,9 @@ def build_parameter_search_file( country_id: str, policyengine_version: str, parameters: Iterable[SearchParameterSource], - nodes: Iterable[SearchNodeSource], - draftability: Iterable[DraftabilityChange], -) -> dict[str, Any]: + nodes: Iterable[TreeNodeSource], + editability: Iterable[EditabilityChange], +) -> ParameterSearchFileDocument: """Return one catalog version's search file, ordered by path. ``parameters`` lists every parameter a reform can change with its search @@ -154,9 +196,9 @@ def build_parameter_search_file( under it that has a stored label. A parameter without current-law values cannot be edited. """ - editability = _editability(draftability) - changeable: list[dict[str, Any]] = [] - others: list[dict[str, Any]] = [] + editability_by_name = _editability(editability) + changeable: list[SearchFileParameterEntry] = [] + others: list[SearchFileLabelEntry] = [] for parameter in sorted(parameters, key=lambda parameter: parameter.name): if not parameter.name.startswith(REFORM_TREE_PREFIX): continue @@ -166,8 +208,8 @@ def build_parameter_search_file( ): others.append({"path": parameter.name, "label": label}) continue - editable, changes = editability.get(parameter.name, (False, [])) - entry: dict[str, Any] = { + editable, changes = editability_by_name.get(parameter.name, (False, [])) + entry: SearchFileParameterEntry = { "path": parameter.name, "label": label, "unit": parameter.unit, @@ -177,10 +219,10 @@ def build_parameter_search_file( if changes: entry["editable_changes"] = changes changeable.append(entry) - labelled_nodes = [ + labelled_nodes: list[SearchFileNodeEntry] = [ {"path": node.name, "label": node.label} for node in sorted(nodes, key=lambda node: node.name) - if _is_gov_node(node.name) and _stored_label(node.label) + if _is_gov_node(node.name) and node.label is not None and node.label.strip() ] return { "format": PARAMETER_SEARCH_FILE_FORMAT, @@ -193,7 +235,7 @@ def build_parameter_search_file( def encode_parameter_search_file( - search_file: dict[str, Any], + search_file: ParameterSearchFileDocument, ) -> EncodedParameterSearchFile: """Return the file as compact UTF-8 JSON, gzip-compressed, with its hash.""" content = json.dumps( @@ -208,7 +250,7 @@ def encode_parameter_search_file( ) -def is_draftable_value(value: object) -> bool: +def is_editable_value(value: object) -> bool: """Return whether a draft can edit a value, as v3's draft editor decides. A number (an infinite threshold, stored as the text "Infinity" or @@ -220,21 +262,21 @@ def is_draftable_value(value: object) -> bool: return isinstance(value, str) and value in _INFINITE_VALUES -def _parameter_draftability(parameter: ParameterRecord) -> Iterator[DraftabilityChange]: +def _parameter_editability(parameter: ParameterRecord) -> Iterator[EditabilityChange]: """Yield the parameter's first value's editability and each later change.""" previous: bool | None = None for value in sorted(parameter.values, key=lambda value: value.start_date): - draftable = is_draftable_value(value.value_json) - if draftable != previous: - yield DraftabilityChange( + editable = is_editable_value(value.value_json) + if editable != previous: + yield EditabilityChange( parameter_name=parameter.name, start_date=value.start_date.date(), - draftable=draftable, + editable=editable, ) - previous = draftable + previous = editable -def country_search_file(country: CountryCatalog) -> dict[str, Any]: +def country_search_file(country: CountryCatalog) -> ParameterSearchFileDocument: """Return the search file of a country's catalog version from its records.""" return build_parameter_search_file( country_id=country.country_id, @@ -251,12 +293,12 @@ def country_search_file(country: CountryCatalog) -> dict[str, Any]: for parameter in country.parameters ), nodes=( - SearchNodeSource(name=node.name, label=node.label) + TreeNodeSource(name=node.name, label=node.label) for node in country.parameter_nodes ), - draftability=( + editability=( change for parameter in country.parameters - for change in _parameter_draftability(parameter) + for change in _parameter_editability(parameter) ), ) diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index abeee31aa..584c35701 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -4,18 +4,28 @@ import gzip from datetime import date +from typing import get_type_hints import pytest from policyengine_api.data.v2.catalog.parameter_search_file import ( - DraftabilityChange, - SearchNodeSource, + EditabilityChange, + EditabilityChangeEntry, + ParameterSearchFileDocument, + SearchFileLabelEntry, + SearchFileNodeEntry, + SearchFileParameterEntry, SearchParameterSource, build_parameter_search_file, encode_parameter_search_file, ) +from policyengine_api.data.v2.catalog.parameter_tree import TreeNodeSource from policyengine_api.fastapi_routes.v2.metadata.response_models import ( MetadataParameterSearchFileContent, + MetadataParameterSearchFileEditabilityChange, + MetadataParameterSearchFileLabel, + MetadataParameterSearchFileNode, + MetadataParameterSearchFileParameter, ) from policyengine_api.services.v2.metadata.types import ( MetadataParameterSearchFile, @@ -28,7 +38,7 @@ SEARCH_FILE_PATH = "/v2/parameters/search-file" -SEARCH_FILE_CONTENT = { +SEARCH_FILE_CONTENT: ParameterSearchFileDocument = { "format": 1, "country_id": "us", "policyengine_version": "4.20.3", @@ -302,13 +312,13 @@ def test_the_built_search_file_matches_the_declared_content_schema() -> None: household=False, ), ], - nodes=[SearchNodeSource(name="gov.hmrc", label="HMRC")], - draftability=[ - DraftabilityChange( - "gov.hmrc.stamp_duty.rate[1].rate", date(2015, 1, 1), draftable=True + nodes=[TreeNodeSource(name="gov.hmrc", label="HMRC")], + editability=[ + EditabilityChange( + "gov.hmrc.stamp_duty.rate[1].rate", date(2015, 1, 1), editable=True ), - DraftabilityChange( - "gov.hmrc.stamp_duty.rate[1].rate", date(2020, 7, 8), draftable=False + EditabilityChange( + "gov.hmrc.stamp_duty.rate[1].rate", date(2020, 7, 8), editable=False ), ], ) @@ -317,3 +327,17 @@ def test_the_built_search_file_matches_the_declared_content_schema() -> None: assert content.model_dump(mode="json", exclude_unset=True) == search_file assert content.parameters[0].editable_changes is not None + + +def test_the_search_file_shapes_match_the_declared_content_models() -> None: + """The builder's typed shapes and the OpenAPI models name the same fields.""" + pairs = ( + (ParameterSearchFileDocument, MetadataParameterSearchFileContent), + (SearchFileParameterEntry, MetadataParameterSearchFileParameter), + (EditabilityChangeEntry, MetadataParameterSearchFileEditabilityChange), + (SearchFileLabelEntry, MetadataParameterSearchFileLabel), + (SearchFileNodeEntry, MetadataParameterSearchFileNode), + ) + for typed_shape, model in pairs: + assert set(get_type_hints(typed_shape)) == set(model.model_fields), model + assert SearchFileParameterEntry.__optional_keys__ == {"editable_changes"} diff --git a/tests/unit/v2/test_parameter_search_file.py b/tests/unit/v2/test_parameter_search_file.py index a83719446..52e6c1790 100644 --- a/tests/unit/v2/test_parameter_search_file.py +++ b/tests/unit/v2/test_parameter_search_file.py @@ -11,13 +11,14 @@ from policyengine_api.data.v2.catalog.parameter_search_file import ( PARAMETER_SEARCH_FILE_FORMAT, - DraftabilityChange, - SearchNodeSource, + EditabilityChange, + ParameterSearchFileDocument, SearchParameterSource, build_parameter_search_file, encode_parameter_search_file, - is_draftable_value, + is_editable_value, ) +from policyengine_api.data.v2.catalog.parameter_tree import TreeNodeSource def _parameter( @@ -39,10 +40,10 @@ def _parameter( ) -def _always_draftable(*names: str) -> list[DraftabilityChange]: +def _always_editable(*names: str) -> list[EditabilityChange]: return [ - DraftabilityChange( - parameter_name=name, start_date=date(2020, 1, 1), draftable=True + EditabilityChange( + parameter_name=name, start_date=date(2020, 1, 1), editable=True ) for name in names ] @@ -51,18 +52,18 @@ def _always_draftable(*names: str) -> list[DraftabilityChange]: def _file( parameters: list[SearchParameterSource], *, - nodes: list[SearchNodeSource] | None = None, - draftability: list[DraftabilityChange] | None = None, -) -> dict: + nodes: list[TreeNodeSource] | None = None, + editability: list[EditabilityChange] | None = None, +) -> ParameterSearchFileDocument: return build_parameter_search_file( country_id="uk", policyengine_version="6.10.0", parameters=parameters, nodes=nodes or [], - draftability=( - draftability - if draftability is not None - else _always_draftable(*(parameter.name for parameter in parameters)) + editability=( + editability + if editability is not None + else _always_editable(*(parameter.name for parameter in parameters)) ), ) @@ -167,14 +168,14 @@ def test_nodes_carry_their_stored_labels_only() -> None: search_file = _file( [_parameter("gov.hmrc.income_tax.rates.uk[0].rate")], nodes=[ - SearchNodeSource(name="gov.hmrc", label="HMRC"), - SearchNodeSource(name="gov.hmrc.income_tax", label="Income Tax"), - SearchNodeSource( + TreeNodeSource(name="gov.hmrc", label="HMRC"), + TreeNodeSource(name="gov.hmrc.income_tax", label="Income Tax"), + TreeNodeSource( name="gov.hmrc.income_tax.rates.uk", label="Income Tax main rates" ), - SearchNodeSource(name="gov.hmrc.income_tax.rates", label=None), - SearchNodeSource(name="gov.hmrc.income_tax.rates.uk[0]", label=" "), - SearchNodeSource(name="calibration", label="Calibration"), + TreeNodeSource(name="gov.hmrc.income_tax.rates", label=None), + TreeNodeSource(name="gov.hmrc.income_tax.rates.uk[0]", label=" "), + TreeNodeSource(name="calibration", label="Calibration"), ], ) @@ -191,8 +192,8 @@ def test_the_labelled_gov_node_itself_is_included() -> None: search_file = _file( [_parameter("gov.hmrc.vat.rate")], nodes=[ - SearchNodeSource(name="gov", label="Government"), - SearchNodeSource(name="government_bonds", label="Bonds"), + TreeNodeSource(name="gov", label="Government"), + TreeNodeSource(name="government_bonds", label="Bonds"), ], ) @@ -206,9 +207,9 @@ def test_a_value_that_is_not_a_number_or_switch_cannot_be_edited() -> None: _parameter("gov.a.no_values"), _parameter("gov.a.threshold"), ], - draftability=[ - DraftabilityChange("gov.a.programs", date(2020, 1, 1), draftable=False), - DraftabilityChange("gov.a.threshold", date(2020, 1, 1), draftable=True), + editability=[ + EditabilityChange("gov.a.programs", date(2020, 1, 1), editable=False), + EditabilityChange("gov.a.threshold", date(2020, 1, 1), editable=True), ], ) @@ -231,26 +232,26 @@ def test_editability_that_changes_over_time_lists_the_dates_it_changes() -> None "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate" ) ], - draftability=[ - DraftabilityChange( + editability=[ + EditabilityChange( "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", date(2003, 7, 10), - draftable=True, + editable=True, ), - DraftabilityChange( + EditabilityChange( "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", date(2015, 2, 12), - draftable=True, + editable=True, ), - DraftabilityChange( + EditabilityChange( "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", date(2020, 7, 8), - draftable=False, + editable=False, ), - DraftabilityChange( + EditabilityChange( "gov.hmrc.stamp_duty.residential.purchase.main.subsequent[1].rate", date(2021, 3, 31), - draftable=True, + editable=True, ), ], ) @@ -266,16 +267,16 @@ def test_editability_that_changes_over_time_lists_the_dates_it_changes() -> None def test_the_earliest_value_decides_editability_before_any_change() -> None: search_file = _file( [_parameter("gov.hmrc.income_tax.rates.scotland.rates[0].rate")], - draftability=[ - DraftabilityChange( + editability=[ + EditabilityChange( "gov.hmrc.income_tax.rates.scotland.rates[0].rate", date(2018, 4, 6), - draftable=True, + editable=True, ), - DraftabilityChange( + EditabilityChange( "gov.hmrc.income_tax.rates.scotland.rates[0].rate", date(2015, 1, 1), - draftable=False, + editable=False, ), ], ) @@ -288,7 +289,7 @@ def test_the_earliest_value_decides_editability_before_any_change() -> None: def test_the_encoded_file_is_compressed_json_with_its_content_hash() -> None: search_file = _file( [_parameter("gov.hmrc.vat.rate", label="VAT rate", unit="/1")], - nodes=[SearchNodeSource(name="gov.hmrc", label="HMRC")], + nodes=[TreeNodeSource(name="gov.hmrc", label="HMRC")], ) encoded = encode_parameter_search_file(search_file) @@ -314,7 +315,7 @@ def test_the_same_catalog_rows_always_encode_to_the_same_bytes() -> None: @pytest.mark.parametrize( - ("value", "draftable"), + ("value", "editable"), [ (0.2, True), (0, True), @@ -328,8 +329,8 @@ def test_the_same_catalog_rows_always_encode_to_the_same_bytes() -> None: ({"a": 1}, False), ], ) -def test_draftable_values_follow_v3s_draft_editor( - value: object, *, draftable: bool +def test_editable_values_follow_v3s_draft_editor( + value: object, *, editable: bool ) -> None: # v3's isDraftableValue: a number, a yes/no switch, or "Infinity"/"-Infinity". - assert is_draftable_value(value) is draftable + assert is_editable_value(value) is editable From 245d6ba150616a9502bdbc28919f2360dffb5d90 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:17:47 +0400 Subject: [PATCH 07/19] Describe when parameter search file rows are written in the privilege manifest Co-Authored-By: Claude Opus 5.5 --- policyengine_api/data/v2/privileges.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/policyengine_api/data/v2/privileges.py b/policyengine_api/data/v2/privileges.py index 542fa0f0f..7bf2897c8 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -114,7 +114,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, and for versions stored before it. + # Written by catalog publication for each version it publishes, from the + # installed packages' catalog; never rewritten. "parameter_search_files": _access(runtime=READ, catalog_writer=READ_INSERT), # Written with each catalog version that publication publishes. "parameter_tree_children": _access(runtime=READ, catalog_writer=READ_INSERT), From 387126796499a2d4edddda276dee5754adbf3f71 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:00:46 +0400 Subject: [PATCH 08/19] Gzip a response only for a client that accepts gzip Starlette's GZipMiddleware compressed whenever the text "gzip" appeared in Accept-Encoding, so a client refusing it (gzip;q=0) still received gzip; for the parameter search file, that gzip body even carried the uncompressed form's strong ETag. The application now uses NegotiatedGZipMiddleware, which weighs Accept-Encoding qualities and exact coding names, and the search file route decides its encoding with the same rule, so the encoding a route chooses is final. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/asgi_factory.py | 5 +- policyengine_api/content_encoding.py | 70 +++++++++++++++++++ .../v2/metadata/search_file_response.py | 36 ++-------- tests/unit/test_asgi_factory.py | 35 ++++++++++ .../v2/test_metadata_search_file_routes.py | 42 +++++++++++ 5 files changed, 156 insertions(+), 32 deletions(-) create mode 100644 policyengine_api/content_encoding.py diff --git a/policyengine_api/asgi_factory.py b/policyengine_api/asgi_factory.py index 45cc9f81f..415fd09bb 100644 --- a/policyengine_api/asgi_factory.py +++ b/policyengine_api/asgi_factory.py @@ -13,11 +13,11 @@ from fastapi.exceptions import RequestValidationError from starlette.datastructures import MutableHeaders from starlette.middleware.cors import CORSMiddleware -from starlette.middleware.gzip import GZipMiddleware from starlette.responses import PlainTextResponse, Response from starlette.routing import Match, Mount from policyengine_api.constants import VERSION +from policyengine_api.content_encoding import NegotiatedGZipMiddleware from policyengine_api.fastapi_routes.dependencies import NativeRouteDependencies from policyengine_api.fastapi_routes.health import build_core_health_router from policyengine_api.fastapi_routes.metadata import build_metadata_router @@ -447,7 +447,8 @@ def create_asgi_app( # Cloud Run because it does not have an nginx sidecar to handle gzip. # Measured on the real payload: level 9 = 5.5s -> 9.0MB, level 4 = 0.7s -> # 9.9MB — 7.5x faster for ~10% larger output. - app.add_middleware(GZipMiddleware, minimum_size=1000, compresslevel=4) + # It weighs Accept-Encoding qualities, so a client refusing gzip gets none. + app.add_middleware(NegotiatedGZipMiddleware, minimum_size=1000, compresslevel=4) app.exception_handler(Exception)(_add_headers_to_unhandled_errors) app.exception_handler(RequestValidationError)(_typed_v2_request_validation_error) app.exception_handler(V2RequestTooLargeError)(_oversized_v2_request) diff --git a/policyengine_api/content_encoding.py b/policyengine_api/content_encoding.py new file mode 100644 index 000000000..9586ece16 --- /dev/null +++ b/policyengine_api/content_encoding.py @@ -0,0 +1,70 @@ +"""Gzip negotiation that weighs Accept-Encoding qualities. + +Starlette's ``GZipMiddleware`` compresses whenever the text "gzip" appears in +Accept-Encoding, so ``gzip;q=0`` (an explicit refusal) still gets gzip. The +application and the routes that send stored gzip bytes decide with +``accepts_gzip`` instead, so a response's encoding is the one the client +accepts and every route agrees with the middleware. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Final + +from starlette.datastructures import Headers +from starlette.middleware.gzip import GZipMiddleware, GZipResponder, IdentityResponder + +if TYPE_CHECKING: + from starlette.types import Receive, Scope, Send + +_GZIP_CODINGS: Final = frozenset({"gzip", "x-gzip"}) + + +def _quality(parameters: str) -> float: + for parameter in parameters.split(";"): + name, _, value = parameter.strip().partition("=") + if name.strip().lower() == "q": + try: + return float(value) + except ValueError: + return 0.0 + return 1.0 + + +def accepts_gzip(accept_encoding: str | None) -> bool: + """Return whether an Accept-Encoding value accepts gzip. + + An explicit gzip entry decides; otherwise ``*`` does. A quality of 0 + refuses the coding, and a coding name must match exactly. + """ + wildcard_quality: float | None = None + for entry in (accept_encoding or "").split(","): + coding, _, parameters = entry.partition(";") + name = coding.strip().lower() + if name in _GZIP_CODINGS: + return _quality(parameters) > 0 + if name == "*": + wildcard_quality = _quality(parameters) + return wildcard_quality is not None and wildcard_quality > 0 + + +class NegotiatedGZipMiddleware(GZipMiddleware): + """``GZipMiddleware`` that compresses only for clients that accept gzip. + + Responses that already name a Content-Encoding pass through unchanged, as + in the parent class. + """ + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + """Choose the gzip or identity responder by ``accepts_gzip``.""" + if scope["type"] != "http": + await self.app(scope, receive, send) + return + accept_encoding = Headers(scope=scope).get("accept-encoding") + if accepts_gzip(accept_encoding): + responder = GZipResponder( + self.app, self.minimum_size, compresslevel=self.compresslevel + ) + await responder(scope, receive, send) + return + await IdentityResponder(self.app, self.minimum_size)(scope, receive, send) diff --git a/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py index 95ceb35a9..b7806db09 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py +++ b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py @@ -16,6 +16,7 @@ from starlette.responses import Response +from policyengine_api.content_encoding import accepts_gzip from policyengine_api.fastapi_routes.v2.metadata.common import ( CACHE_HEADER_DESCRIPTIONS, ) @@ -26,7 +27,6 @@ ) _GZIP_ETAG_SUFFIX: Final = "-gzip" -_GZIP_CODINGS: Final = frozenset({"gzip", "x-gzip"}) SEARCH_FILE_RESPONSE_HEADERS: Final[dict[str, dict[str, Any]]] = { **CACHE_HEADER_DESCRIPTIONS, @@ -44,40 +44,16 @@ } -def _quality(parameters: str) -> float: - for parameter in parameters.split(";"): - name, _, value = parameter.strip().partition("=") - if name.strip().lower() == "q": - try: - return float(value) - except ValueError: - return 0.0 - return 1.0 - - -def accepts_gzip(accept_encoding: str | None) -> bool: - """Return whether an Accept-Encoding value accepts gzip. - - An explicit gzip entry decides; otherwise ``*`` does. A quality of 0 - refuses the coding. - """ - wildcard_quality: float | None = None - for entry in (accept_encoding or "").split(","): - coding, _, parameters = entry.partition(";") - name = coding.strip().lower() - if name in _GZIP_CODINGS: - return _quality(parameters) > 0 - if name == "*": - wildcard_quality = _quality(parameters) - return wildcard_quality is not None and wildcard_quality > 0 - - def parameter_search_file_response( search_file: MetadataParameterSearchFile, *, accept_encoding: str | None, ) -> Response: - """Return the file in the encoding the request accepts, with its ETag.""" + """Return the file in the encoding the request accepts, with its ETag. + + It decides with the application's gzip middleware's rule, and the gzip + form names its Content-Encoding, so the middleware never re-encodes it. + """ if accepts_gzip(accept_encoding): return Response( content=search_file.compressed_content, diff --git a/tests/unit/test_asgi_factory.py b/tests/unit/test_asgi_factory.py index 6dbda05cb..f413ce884 100644 --- a/tests/unit/test_asgi_factory.py +++ b/tests/unit/test_asgi_factory.py @@ -544,6 +544,41 @@ def test_large_flask_fallback_response_supports_http_gzip(): assert response.text == "x" * 2_000 +@pytest.mark.parametrize( + "accept_encoding", + ["gzip;q=0", "br, gzip;q=0.0", "identity", "*;q=0", "gzipped"], +) +def test_large_responses_are_not_gzipped_for_a_client_that_refuses_gzip( + accept_encoding: str, +): + client = TestClient(create_asgi_app(create_test_wsgi_app())) + + response = client.get( + "/large-fallback", + headers={"Accept-Encoding": accept_encoding}, + ) + + assert response.status_code == 200 + assert "content-encoding" not in response.headers + assert response.text == "x" * 2_000 + + +@pytest.mark.parametrize("accept_encoding", ["gzip;q=0.5", "br, *", "GZIP"]) +def test_large_responses_are_gzipped_for_a_client_that_accepts_gzip( + accept_encoding: str, +): + client = TestClient(create_asgi_app(create_test_wsgi_app())) + + response = client.get( + "/large-fallback", + headers={"Accept-Encoding": accept_encoding}, + ) + + assert response.status_code == 200 + assert response.headers["content-encoding"] == "gzip" + assert response.text == "x" * 2_000 + + def test_request_headers_and_cookies_pass_through_to_flask_fallback(): client = TestClient(create_asgi_app(create_test_wsgi_app())) client.cookies.set("session_id", "session-123") diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index 584c35701..8d40a652a 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -341,3 +341,45 @@ def test_the_search_file_shapes_match_the_declared_content_models() -> None: for typed_shape, model in pairs: assert set(get_type_hints(typed_shape)) == set(model.model_fields), model assert SearchFileParameterEntry.__optional_keys__ == {"editable_changes"} + + +@pytest.mark.parametrize("accept_encoding", ["gzip;q=0", "identity"]) +def test_a_large_search_file_stays_uncompressed_for_a_client_refusing_gzip( + accept_encoding: str, +) -> None: + content: ParameterSearchFileDocument = { + **SEARCH_FILE_CONTENT, + "parameters": [ + { + "path": f"gov.example.rate_{index}", + "label": f"Rate {index}", + "unit": "/1", + "description": None, + "editable": True, + } + for index in range(200) + ], + } + encoded = encode_parameter_search_file(content) + stored = MetadataParameterSearchFile( + policyengine_version="4.20.3", + file_format=1, + compressed_content=encoded.compressed_content, + content_sha256=encoded.content_sha256, + uncompressed_size=encoded.uncompressed_size, + compressed_size=encoded.compressed_size, + ) + + response = metadata_route_client( + lambda: ResourceReader({"get_parameter_search_file": stored}) + ).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"Accept-Encoding": accept_encoding}, + ) + + assert encoded.uncompressed_size > 1000 + assert response.status_code == 200 + assert "content-encoding" not in response.headers + assert response.headers["etag"] == f'"{encoded.content_sha256}"' + assert response.content == gzip.decompress(encoded.compressed_content) From f0cbc47c187a7d9d5b2ffc2a03d114ff7761a27d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:03:26 +0400 Subject: [PATCH 09/19] Name the catalog version in the search file route's 503 A version stored before search files existed now returns 503 with the message the service writes for it, which names the version and says why it has no file, as the parameter tree route does; other catalog failures keep the general message. OpenAPI describes this 503 for the route. Co-Authored-By: Claude Opus 5.5 --- .../fastapi_routes/v2/metadata/common.py | 21 +++++++++++++++++-- .../v2/metadata/parameter_routes.py | 2 ++ .../services/v2/metadata/services.py | 3 ++- .../services/v2/metadata/validators.py | 3 ++- tests/integration/test_v2_metadata_routes.py | 6 +++++- .../v2/test_metadata_search_file_routes.py | 13 ++++++------ tests/unit/v2/test_metadata_service.py | 10 ++++++--- 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index 5993303f8..214b39a13 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, + ParameterSearchFileUnavailableError, ParameterTreeUnavailableError, ) from policyengine_api.vary_headers import unique_vary_values @@ -74,6 +75,18 @@ } """The parameter tree route's 503, which also covers a version without a tree.""" +PARAMETER_SEARCH_FILE_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 search file because it was published before search " + "files existed; the message then names the version." + ), + }, +} +"""The search file route's 503, which also covers a version without a file.""" + ERROR_RESPONSES: dict[int | str, dict[str, Any]] = { 404: { "model": V2ErrorResponse, @@ -363,10 +376,14 @@ def run_metadata_read( ) as error: return v2_error_response(404, str(error)) except (V2ConfigurationError, MetadataCatalogUnavailableError) as error: - # Only a missing parameter tree has a message written for callers. + # Only a missing parameter tree or search file has a message written + # for callers; it names the catalog version. message = ( str(error) - if isinstance(error, ParameterTreeUnavailableError) + if isinstance( + error, + ParameterTreeUnavailableError | ParameterSearchFileUnavailableError, + ) else "V2 metadata catalog is unavailable" ) return v2_error_response(503, message) diff --git a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py index a376cd967..60a2544fd 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_SEARCH_FILE_ERROR_RESPONSES, PARAMETER_TREE_ERROR_RESPONSES, CacheableMetadataRoute, read_resource, @@ -129,6 +130,7 @@ def list_parameter_children( ), "headers": SEARCH_FILE_RESPONSE_HEADERS, }, + **PARAMETER_SEARCH_FILE_ERROR_RESPONSES, }, summary="Get a catalog version's parameter search file", ) diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index 889533a6c..e9ab4f382 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -421,7 +421,8 @@ def get_parameter_search_file( if search_file is None: msg = ( f"catalog version {selected.policyengine_version} has no parameter " - "search file" + "search file: a file is written when a catalog version is " + "published, and this version was published before" ) raise ParameterSearchFileUnavailableError(msg) return search_file diff --git a/policyengine_api/services/v2/metadata/validators.py b/policyengine_api/services/v2/metadata/validators.py index 66c8f0885..77f97b265 100644 --- a/policyengine_api/services/v2/metadata/validators.py +++ b/policyengine_api/services/v2/metadata/validators.py @@ -29,7 +29,8 @@ class ParameterSearchFileUnavailableError(MetadataCatalogUnavailableError): Catalog publication writes a version's file only when it publishes the version, from the installed packages' catalog, so a version stored by an - earlier release before search files existed has none. + earlier release before search files existed has none. The message names + the version and is safe to return to the caller. """ diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index b625e147f..40072853a 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1107,7 +1107,11 @@ def test_postgres_search_file_of_a_version_without_one_is_unavailable( assert response.status_code == 503 assert response.json() == { "status": "error", - "message": "V2 metadata catalog is unavailable", + "message": ( + f"catalog version {POLICYENGINE_VERSION} has no parameter search " + "file: a file is written when a catalog version is published, and " + "this version was published before" + ), } diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index 8d40a652a..ffe9c04a3 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -228,19 +228,19 @@ def test_search_file_route_sends_the_file_when_the_etag_differs() -> None: def test_search_file_route_reports_a_missing_file_as_unavailable() -> None: - reader = _search_file_reader( - error=ParameterSearchFileUnavailableError("search file not written") + message = ( + "catalog version 4.20.3 has no parameter search file: a file is written " + "when a catalog version is published, and this version was published " + "before" ) + reader = _search_file_reader(error=ParameterSearchFileUnavailableError(message)) response = metadata_route_client(lambda: reader).get( SEARCH_FILE_PATH, params={"country_id": "us"} ) assert response.status_code == 503 - assert response.json() == { - "status": "error", - "message": "V2 metadata catalog is unavailable", - } + assert response.json() == {"status": "error", "message": message} assert "etag" not in response.headers assert "cache-control" not in response.headers assert reader.closed @@ -287,6 +287,7 @@ def test_search_file_openapi_declares_its_query_and_content() -> None: } assert {"ETag", "Cache-Control", "Content-Encoding", "Vary"} <= set(ok["headers"]) assert "304" in operation["responses"] + assert "no parameter search file" in operation["responses"]["503"]["description"] assert "MetadataParameterSearchFileParameter" in schema["components"]["schemas"] diff --git a/tests/unit/v2/test_metadata_service.py b/tests/unit/v2/test_metadata_service.py index ef80afe41..771ae0c6d 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -671,7 +671,11 @@ def test_a_version_without_a_search_file_of_the_current_format_is_unavailable( ) catalog_session.commit() - with pytest.raises( - ParameterSearchFileUnavailableError, match="has no parameter search file" - ): + with pytest.raises(ParameterSearchFileUnavailableError) as raised: catalog_service(catalog_session).get_parameter_search_file("us") + + assert str(raised.value) == ( + f"catalog version {POLICYENGINE_VERSION} has no parameter search file: a " + "file is written when a catalog version is published, and this version " + "was published before" + ) From 078c47bd749cc516552ff9d88bef0ed8a5e00a8d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:05:19 +0400 Subject: [PATCH 10/19] Test the search file route's CORS exposure and 404, and its publication roles Tests cover what the search file now shares with the other metadata routes and publication: browser scripts can read its ETag through CORS, with Vary naming Origin and Accept-Encoding; an unknown catalog version is a 404 without caching headers; publishing as the catalog writer role writes each version's file, which needs the role's declared INSERT grant; and one publication gives the installed version stored without them both its parameter tree and its search file. Co-Authored-By: Claude Opus 5.5 --- .../test_v2_catalog_publication.py | 65 +++++++++++++++++++ tests/integration/test_v2_metadata_routes.py | 12 ++++ .../v2/test_metadata_search_file_routes.py | 36 ++++++++++ 3 files changed, 113 insertions(+) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index a25bde086..69df232b0 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -1134,3 +1134,68 @@ def test_publication_fails_when_a_published_version_has_no_search_file( publish_catalog(publication_engine, normalized_catalog()) assert _counts(publication_engine)["tax_benefit_model_versions"] == 0 + + +def test_the_catalog_writer_role_publishes_the_search_files( + publication_engine: Engine, + managed_v2_roles, + v2_engine_as_role, +) -> None: + """Publication as the catalog writer writes each version's search file. + + That needs the role's declared INSERT grant on the search files' 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 [ + (model_name, version) + for model_name, version, _uncompressed, _compressed in ( + evidence.parameter_search_files_written + ) + ] == [ + (country.model.name, country.model_version.version) + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ] + for country in catalog.countries: + version = country.model_version.version + assert _stored_search_file( + publication_engine, country.model.name, version + ) == _fixture_search_file(country.country_id, version) + + +def test_one_publication_gives_the_installed_stored_version_its_tree_and_file( + publication_engine: Engine, +) -> None: + """The installed version stored without either gets both in one run.""" + catalog = normalized_catalog() + current_version = catalog.country("us").model_version.version + publish_catalog(publication_engine, catalog) + _delete_tree(publication_engine, current_version) + _delete_search_files(publication_engine, current_version) + + evidence = publish_catalog(publication_engine, catalog) + + model_names = [ + country.model.name + for country in sorted(catalog.countries, key=lambda item: item.model.name) + ] + assert [model for model, _version, _rows in evidence.parameter_trees_written] == ( + model_names + ) + assert [ + model + for model, _version, _raw, _size in evidence.parameter_search_files_written + ] == model_names + for country in catalog.countries: + assert ( + _stored_tree(publication_engine, country.model.name, current_version) + == FIXTURE_TREE + ) + assert _stored_search_file( + publication_engine, country.model.name, current_version + ) == _fixture_search_file(country.country_id, current_version) diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index 40072853a..8251b42e6 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1134,3 +1134,15 @@ def test_postgres_search_file_reads_as_the_runtime_role( assert json.loads(gzip.decompress(search_file.compressed_content)) == ( country_search_file(normalized_catalog().country("us")) ) + + +def test_postgres_search_file_of_an_unknown_catalog_version_is_absent( + published_engine: Engine, +) -> None: + response = _client(published_engine).get( + "/v2/parameters/search-file", + params={"country_id": "us", "policyengine_version": "99.0.0"}, + ) + + assert response.status_code == 404 + assert response.json()["status"] == "error" diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index ffe9c04a3..d812b0368 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -8,6 +8,9 @@ import pytest +from policyengine_api.data.v2.catalog.catalog_selection import ( + MetadataCatalogVersionNotFoundError, +) from policyengine_api.data.v2.catalog.parameter_search_file import ( EditabilityChange, EditabilityChangeEntry, @@ -384,3 +387,36 @@ def test_a_large_search_file_stays_uncompressed_for_a_client_refusing_gzip( assert "content-encoding" not in response.headers assert response.headers["etag"] == f'"{encoded.content_sha256}"' assert response.content == gzip.decompress(encoded.compressed_content) + + +def test_browser_scripts_can_read_the_search_file_entity_tag() -> None: + response = metadata_route_client(_search_file_reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"Origin": "https://app.policyengine.org", "Accept-Encoding": "gzip"}, + ) + + assert response.status_code == 200 + assert response.headers["access-control-allow-origin"] == ( + "https://app.policyengine.org" + ) + exposed_headers = { + value.strip().lower() + for value in response.headers["access-control-expose-headers"].split(",") + } + assert "etag" in exposed_headers + assert response.headers["vary"] == "Origin, Accept-Encoding" + + +def test_search_file_route_reports_an_unknown_catalog_version_as_absent() -> None: + reader = _search_file_reader( + error=MetadataCatalogVersionNotFoundError("catalog version 9.9.9 is absent") + ) + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, params={"country_id": "us", "policyengine_version": "9.9.9"} + ) + + assert response.status_code == 404 + assert "etag" not in response.headers + assert "cache-control" not in response.headers From 46af2ff527db5e4d38b0c7b6f859e6287cc2ac3f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:05:35 +0400 Subject: [PATCH 11/19] Describe the gzip negotiation fix and the search file's 503 in the changelog Co-Authored-By: Claude Opus 5.5 --- changelog.d/v2plan-1f-118-search-file.added.md | 2 +- changelog.d/v2plan-1f-118-search-file.fixed.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/v2plan-1f-118-search-file.fixed.md diff --git a/changelog.d/v2plan-1f-118-search-file.added.md b/changelog.d/v2plan-1f-118-search-file.added.md index 9880e1022..842482c27 100644 --- a/changelog.d/v2plan-1f-118-search-file.added.md +++ b/changelog.d/v2plan-1f-118-search-file.added.md @@ -1 +1 @@ -Catalog publication writes a parameter search file for each catalog version it publishes into a new table, parameter_search_files, built from the installed packages' catalog: every parameter a reform can change with its path, stored label, unit, description and whether a draft can edit its current-law value (with the dates from which that changes), the stored labels of the other parameters under gov, and every labelled node under gov, as gzip-compressed JSON with its SHA-256. A new route, /v2/parameters/search-file, serves a version's file with a strong ETag, 304 revalidation and long-lived caching for a named version, as gzip to clients that accept it. A version stored by an earlier release has no file and returns 503. +Catalog publication writes a parameter search file for each catalog version it publishes into a new table, parameter_search_files, built from the installed packages' catalog: every parameter a reform can change with its path, stored label, unit, description and whether a draft can edit its current-law value (with the dates from which that changes), the stored labels of the other parameters under gov, and gov and every labelled node under it, as gzip-compressed JSON with its SHA-256. A new route, /v2/parameters/search-file, serves a version's file with a strong ETag, 304 revalidation and long-lived caching for a named version, as gzip to clients that accept it. A version stored by an earlier release has no file and returns 503 with a message naming it. diff --git a/changelog.d/v2plan-1f-118-search-file.fixed.md b/changelog.d/v2plan-1f-118-search-file.fixed.md new file mode 100644 index 000000000..90ff0fc08 --- /dev/null +++ b/changelog.d/v2plan-1f-118-search-file.fixed.md @@ -0,0 +1 @@ +Responses are gzip-compressed only for a client whose Accept-Encoding accepts gzip: a client that refuses it (for example gzip;q=0) now receives an uncompressed response instead of a gzip one. From 7900995128a5a4a73a5d206545bf901e4ebda3fd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:08:36 +0400 Subject: [PATCH 12/19] Write no search file for an installed version stored before catalog format 2 A version stored before catalog format 2 has no stored economy or household flags, so a search file built from the installed catalog's flags would contradict its own parameter rows. Publication now writes a file for an already-stored installed version only when its stored catalog format is 2, and does not require one from a version stored before; such a version keeps returning the 503 that names it. Co-Authored-By: Claude Opus 5.5 --- docs/migration/stage-9-v2-metadata.md | 5 ++- .../publication_parameter_search_file.py | 36 ++++++++++++----- .../test_v2_catalog_publication.py | 39 +++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/docs/migration/stage-9-v2-metadata.md b/docs/migration/stage-9-v2-metadata.md index 811f66ea5..2c27be6f5 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -356,7 +356,10 @@ The file is built from the installed packages' catalog, never from rows an earlier release stored, so publication writes one only for the versions it publishes: each new version, and the installed version when an earlier release stored it before search files existed. A version that is neither has -no file until a person decides otherwise. Writing only inserts; a stored file +no file until a person decides otherwise. An installed version stored before +catalog format 2 (`catalog_format` null) gets none either: it has no stored +economy or household flags, so a file built from the installed catalog's +flags would contradict its own parameter rows. Writing only inserts; a stored file of the same format must equal the one built from the installed packages, and publication fails if it differs or if a version it publishes is left without a file. A later file format adds a row for the version instead of changing the diff --git a/policyengine_api/data/v2/catalog/publication_parameter_search_file.py b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py index 1debd312b..0e8dcd815 100644 --- a/policyengine_api/data/v2/catalog/publication_parameter_search_file.py +++ b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py @@ -6,6 +6,12 @@ release stored it before search files existed. Writing only inserts. A stored file of the same format must equal the one built from the installed packages, because the stored version's other rows already match them. + +A version stored before catalog format 2 (``catalog_format`` null) has no +stored economy or household flags, so a file built from the installed +catalog's flags would contradict its own parameter rows: it gets no file and +is not required to have one, and the route reports it as a version without a +file. """ from __future__ import annotations @@ -38,9 +44,12 @@ PARAMETER_SEARCH_FILES = V2_METADATA.tables["parameter_search_files"] -def _stored_version_id(connection: Connection, country: CountryCatalog) -> UUID: - version_id: UUID = connection.execute( - sa.select(MODEL_VERSIONS.c.id) +def _stored_version( + connection: Connection, country: CountryCatalog +) -> tuple[UUID, int | None]: + """Return the stored version's ID and catalog format (None before format 2).""" + row = connection.execute( + sa.select(MODEL_VERSIONS.c.id, MODEL_VERSIONS.c.catalog_format) .select_from( MODEL_VERSIONS.join(MODELS, MODELS.c.id == MODEL_VERSIONS.c.model_id) ) @@ -48,8 +57,10 @@ def _stored_version_id(connection: Connection, country: CountryCatalog) -> UUID: MODELS.c.name == country.model.name, MODEL_VERSIONS.c.version == country.model_version.version, ) - ).scalar_one() - return version_id + ).one() + version_id: UUID = row.id + catalog_format: int | None = row.catalog_format + return version_id, catalog_format def _stored_content_sha256(connection: Connection, version_id: UUID) -> str | None: @@ -66,7 +77,7 @@ def write_parameter_search_files( connection: Connection, countries: Sequence[CountryCatalog], ) -> tuple[tuple[str, str, int, int], ...]: - """Write the search file of each published version that has none. + """Write the search file of each published format 2 version that has none. Returns the model name, version and uncompressed and compressed sizes of each file written, in model name order. Runs inside publication's @@ -74,7 +85,9 @@ def write_parameter_search_files( """ written = [] for country in sorted(countries, key=lambda item: item.model.name): - version_id = _stored_version_id(connection, country) + version_id, catalog_format = _stored_version(connection, country) + if catalog_format is None: + continue encoded = encode_parameter_search_file(country_search_file(country)) stored_sha256 = _stored_content_sha256(connection, version_id) if stored_sha256 is None: @@ -111,9 +124,14 @@ def assert_parameter_search_files_written( connection: Connection, countries: Sequence[CountryCatalog], ) -> None: - """Raise CatalogPublicationError if a catalog country's version has no file.""" + """Raise CatalogPublicationError if a catalog country's version has no file. + + A version stored before catalog format 2 is exempt: it gets no file. + """ for country in countries: - version_id = _stored_version_id(connection, country) + version_id, catalog_format = _stored_version(connection, country) + if catalog_format is None: + continue if _stored_content_sha256(connection, version_id) is None: msg = ( f"the {country.country_id} catalog version " diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 69df232b0..0875d2864 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -1199,3 +1199,42 @@ def test_one_publication_gives_the_installed_stored_version_its_tree_and_file( assert _stored_search_file( publication_engine, country.model.name, current_version ) == _fixture_search_file(country.country_id, current_version) + + +def _mark_stored_before_catalog_format_2(engine: Engine, version: str) -> None: + """Make the stored versions of one .py version look stored before format 2.""" + with engine.begin() as connection: + connection.execute( + text( + "UPDATE tax_benefit_model_versions SET catalog_format = NULL " + "WHERE version = :version" + ), + {"version": version}, + ) + + +def test_an_installed_version_stored_before_catalog_format_2_gets_no_search_file( + publication_engine: Engine, +) -> None: + """A version stored without format 2's flags keeps having no search file. + + Building its file from the installed catalog's flags would contradict the + version's own stored parameter rows, so publication writes none for it and + does not require one; the route then reports the version without a file. + """ + catalog = normalized_catalog() + current_version = catalog.country("us").model_version.version + publish_catalog(publication_engine, catalog) + _delete_search_files(publication_engine, current_version) + _mark_stored_before_catalog_format_2(publication_engine, current_version) + + evidence = publish_catalog(publication_engine, catalog) + + assert evidence.parameter_search_files_written == () + with publication_engine.connect() as connection: + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_search_files") + ).scalar_one() + == 0 + ) From 09e1d89eb4004fa622d22266484bbcf453b3b757 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:57:04 +0400 Subject: [PATCH 13/19] Answer a search file revalidation from its stored hash alone A request with If-None-Match now reads only the stored file's SHA-256 first; when it names the representation's ETag, the route class answers 304 without the file being loaded, and otherwise the file is read and sent as before. The metadata route helpers' ETag comparison is now the shared names_entity_tag. Co-Authored-By: Claude Opus 5.5 --- .../fastapi_routes/metadata_protocols.py | 11 ++++ .../fastapi_routes/v2/metadata/common.py | 5 +- .../v2/metadata/parameter_routes.py | 54 +++++++++++++---- .../v2/metadata/search_file_response.py | 17 ++++-- .../reads_parameter_search_file.py | 22 +++++++ .../services/v2/metadata/services.py | 41 +++++++++++-- tests/integration/test_v2_metadata_routes.py | 7 ++- .../v2/test_metadata_search_file_routes.py | 58 ++++++++++++++++++- tests/unit/v2/test_metadata_service.py | 8 +++ 9 files changed, 196 insertions(+), 27 deletions(-) diff --git a/policyengine_api/fastapi_routes/metadata_protocols.py b/policyengine_api/fastapi_routes/metadata_protocols.py index 5c9816ff3..a50bde840 100644 --- a/policyengine_api/fastapi_routes/metadata_protocols.py +++ b/policyengine_api/fastapi_routes/metadata_protocols.py @@ -178,6 +178,17 @@ def get_parameter_search_file( """ ... + def get_parameter_search_file_hash( + self, + country_id: str, + policyengine_version: str | None = None, + ) -> str: + """Return the content hash of the selected version's search file only. + + Raise ``ParameterSearchFileUnavailableError`` when the version has none. + """ + ... + def list_parameter_values( self, country_id: str, diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index 214b39a13..1d94ac189 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/common.py +++ b/policyengine_api/fastapi_routes/v2/metadata/common.py @@ -253,7 +253,7 @@ def with_cache_headers(request: Request, response: Response) -> Response: if read_instant is not None: max_age = min(max_age, _seconds_left_in_utc_day(read_instant, utc_now())) headers = {"Cache-Control": f"public, max-age={max_age}", "ETag": entity_tag} - if _names_entity_tag(request.headers.get("if-none-match"), entity_tag): + if names_entity_tag(request.headers.get("if-none-match"), entity_tag): not_modified = Response(status_code=304, headers=headers) not_modified.headers["Vary"] = _cacheable_vary(None) return not_modified @@ -281,7 +281,8 @@ def _cacheable_vary(existing: str | None) -> str: return unique_vary_values(", ".join([existing or "", *CACHEABLE_VARY_VALUES])) -def _names_entity_tag(if_none_match: str | None, entity_tag: str) -> bool: +def names_entity_tag(if_none_match: str | None, entity_tag: str) -> bool: + """Return whether an If-None-Match value names ``entity_tag`` (or is ``*``).""" if if_none_match is None: return False candidates = {candidate.strip() for candidate in if_none_match.split(",")} diff --git a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py index 60a2544fd..11471f41c 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py +++ b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py @@ -13,6 +13,7 @@ PARAMETER_SEARCH_FILE_ERROR_RESPONSES, PARAMETER_TREE_ERROR_RESPONSES, CacheableMetadataRoute, + names_entity_tag, read_resource, run_metadata_read, take_utc_instant_for_read, @@ -34,6 +35,7 @@ from policyengine_api.fastapi_routes.v2.metadata.search_file_response import ( SEARCH_FILE_RESPONSE_HEADERS, parameter_search_file_response, + search_file_entity_tag, ) from policyengine_api.services.v2.metadata.types import ( ParameterFilter, @@ -43,6 +45,46 @@ if TYPE_CHECKING: from policyengine_api.fastapi_routes.dependencies import NativeRouteDependencies + from policyengine_api.query_parameters import CatalogQuery + + +def _serve_parameter_search_file( + dependencies: NativeRouteDependencies, + request: Request, + query: CatalogQuery, +) -> Response: + """Serve the selected version's search file in the encoding it accepts. + + A request that revalidates a copy (If-None-Match) first reads only the + stored hash, so a match is answered with 304 without loading the file. + """ + accept_encoding = request.headers.get("accept-encoding") + if_none_match = request.headers.get("if-none-match") + if if_none_match is not None: + content_sha256 = run_metadata_read( + dependencies, + lambda reader: reader.get_parameter_search_file_hash( + query.country_id, query.policyengine_version + ), + ) + if isinstance(content_sha256, JSONResponse): + return content_sha256 + entity_tag = search_file_entity_tag( + content_sha256, accept_encoding=accept_encoding + ) + if names_entity_tag(if_none_match, entity_tag): + # The route class answers a 200 whose ETag the request names + # with 304, so the file itself is never read. + return Response(headers={"ETag": entity_tag}) + result = run_metadata_read( + dependencies, + lambda reader: reader.get_parameter_search_file( + query.country_id, query.policyengine_version + ), + ) + if isinstance(result, JSONResponse): + return result + return parameter_search_file_response(result, accept_encoding=accept_encoding) def build_v2_metadata_parameter_router( @@ -138,17 +180,7 @@ def get_parameter_search_file( request: Request, query: CatalogQueryDependency, ) -> Response: - result = run_metadata_read( - dependencies, - lambda reader: reader.get_parameter_search_file( - query.country_id, query.policyengine_version - ), - ) - if isinstance(result, JSONResponse): - return result - return parameter_search_file_response( - result, accept_encoding=request.headers.get("accept-encoding") - ) + return _serve_parameter_search_file(dependencies, request, query) @router.get( "/parameters/{parameter_id}", diff --git a/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py index b7806db09..3a2b5b1b0 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py +++ b/policyengine_api/fastapi_routes/v2/metadata/search_file_response.py @@ -44,6 +44,13 @@ } +def search_file_entity_tag(content_sha256: str, *, accept_encoding: str | None) -> str: + """Return the ETag of the representation the request accepts.""" + if accepts_gzip(accept_encoding): + return f'"{content_sha256}{_GZIP_ETAG_SUFFIX}"' + return f'"{content_sha256}"' + + def parameter_search_file_response( search_file: MetadataParameterSearchFile, *, @@ -54,17 +61,17 @@ def parameter_search_file_response( It decides with the application's gzip middleware's rule, and the gzip form names its Content-Encoding, so the middleware never re-encodes it. """ + entity_tag = search_file_entity_tag( + search_file.content_sha256, accept_encoding=accept_encoding + ) if accepts_gzip(accept_encoding): return Response( content=search_file.compressed_content, media_type="application/json", - headers={ - "ETag": f'"{search_file.content_sha256}{_GZIP_ETAG_SUFFIX}"', - "Content-Encoding": "gzip", - }, + headers={"ETag": entity_tag, "Content-Encoding": "gzip"}, ) return Response( content=gzip.decompress(search_file.compressed_content), media_type="application/json", - headers={"ETag": f'"{search_file.content_sha256}"'}, + headers={"ETag": entity_tag}, ) diff --git a/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py index ec8573cce..caf126256 100644 --- a/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py +++ b/policyengine_api/services/v2/metadata/database_connectors/reads_parameter_search_file.py @@ -38,3 +38,25 @@ def read_parameter_search_file( col(ParameterSearchFile.file_format) == file_format, ), ) + + +def read_parameter_search_file_hash( + session: Session, + *, + model_version_id: UUID, + file_format: int, +) -> list[Any]: + """Return the content hash of the version's search file: one row, or none. + + The same index read as ``read_parameter_search_file``, without the + compressed content, so a revalidation does not load the file. + """ + return read_rows( + session, + sa.select( + col(ParameterSearchFile.content_sha256).label("content_sha256"), + ).where( + col(ParameterSearchFile.tax_benefit_model_version_id) == model_version_id, + col(ParameterSearchFile.file_format) == file_format, + ), + ) diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index e9ab4f382..4870ae9d0 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -24,6 +24,7 @@ ) from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_search_file import ( # noqa: E501 # a dotted module path cannot be wrapped read_parameter_search_file, + read_parameter_search_file_hash, ) from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_tree import ( # noqa: E501 # a dotted module path cannot be wrapped read_parameter_children, @@ -111,6 +112,17 @@ ) +def _parameter_search_file_unavailable( + selected: SelectedCatalog, +) -> ParameterSearchFileUnavailableError: + msg = ( + f"catalog version {selected.policyengine_version} has no parameter search " + "file: a file is written when a catalog version is published, and this " + "version was published before" + ) + return ParameterSearchFileUnavailableError(msg) + + class V2MetadataService: """Sequence metadata reads through a request-scoped database session. @@ -419,14 +431,31 @@ def get_parameter_search_file( ), ) if search_file is None: - msg = ( - f"catalog version {selected.policyengine_version} has no parameter " - "search file: a file is written when a catalog version is " - "published, and this version was published before" - ) - raise ParameterSearchFileUnavailableError(msg) + raise _parameter_search_file_unavailable(selected) return search_file + def get_parameter_search_file_hash( + self, + country_id: str, + policyengine_version: str | None = None, + ) -> str: + """Return the SHA-256 of the selected version's uncompressed search file. + + It reads the hash only, so a client revalidating its copy does not load + the file. Raise ParameterSearchFileUnavailableError as + ``get_parameter_search_file`` does. + """ + selected = self._select_catalog(country_id, policyengine_version) + rows = read_parameter_search_file_hash( + self._database_session.session, + model_version_id=selected.model_version.id, + file_format=PARAMETER_SEARCH_FILE_FORMAT, + ) + if not rows: + raise _parameter_search_file_unavailable(selected) + content_sha256: str = rows[0].content_sha256 + return content_sha256 + def list_parameter_values( self, country_id: str, diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index 8251b42e6..97cbfb881 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1124,16 +1124,19 @@ def test_postgres_search_file_reads_as_the_runtime_role( runtime_engine = v2_engine_as_role(managed_v2_roles.runtime, poolclass=NullPool) try: with Session(runtime_engine) as session: - search_file = V2MetadataService( + service = V2MetadataService( MetadataDatabaseSession(session), running_policyengine_version=POLICYENGINE_VERSION, - ).get_parameter_search_file("us") + ) + search_file = service.get_parameter_search_file("us") + content_sha256 = service.get_parameter_search_file_hash("us") finally: runtime_engine.dispose() assert json.loads(gzip.decompress(search_file.compressed_content)) == ( country_search_file(normalized_catalog().country("us")) ) + assert content_sha256 == search_file.content_sha256 def test_postgres_search_file_of_an_unknown_catalog_version_is_absent( diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index d812b0368..789491f9c 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -69,7 +69,13 @@ def _search_file_reader(*, error: Exception | None = None) -> ResourceReader: uncompressed_size=encoded.uncompressed_size, compressed_size=encoded.compressed_size, ) - return ResourceReader({"get_parameter_search_file": stored}, error=error) + return ResourceReader( + { + "get_parameter_search_file": stored, + "get_parameter_search_file_hash": encoded.content_sha256, + }, + error=error, + ) def _content_sha256() -> str: @@ -420,3 +426,53 @@ def test_search_file_route_reports_an_unknown_catalog_version_as_absent() -> Non assert response.status_code == 404 assert "etag" not in response.headers assert "cache-control" not in response.headers + + +def test_a_matching_revalidation_reads_only_the_stored_hash() -> None: + reader = _search_file_reader() + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us", "policyengine_version": "4.20.3"}, + headers={ + "If-None-Match": f'"{_content_sha256()}-gzip"', + "Accept-Encoding": "gzip", + }, + ) + + assert response.status_code == 304 + assert response.headers["etag"] == f'"{_content_sha256()}-gzip"' + assert [name for name, _args, _kwargs in reader.calls] == [ + "get_parameter_search_file_hash" + ] + assert reader.closed + + +def test_a_revalidation_that_does_not_match_reads_the_whole_file() -> None: + reader = _search_file_reader() + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, + params={"country_id": "us"}, + headers={"If-None-Match": '"stale"', "Accept-Encoding": "gzip"}, + ) + + assert response.status_code == 200 + assert response.json() == SEARCH_FILE_CONTENT + assert [name for name, _args, _kwargs in reader.calls] == [ + "get_parameter_search_file_hash", + "get_parameter_search_file", + ] + + +def test_a_request_without_if_none_match_reads_the_file_once() -> None: + reader = _search_file_reader() + + response = metadata_route_client(lambda: reader).get( + SEARCH_FILE_PATH, params={"country_id": "us"} + ) + + assert response.status_code == 200 + assert [name for name, _args, _kwargs in reader.calls] == [ + "get_parameter_search_file" + ] diff --git a/tests/unit/v2/test_metadata_service.py b/tests/unit/v2/test_metadata_service.py index 771ae0c6d..02db0db08 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -646,7 +646,11 @@ def test_the_selected_version_search_file_is_returned_as_stored( catalog_session.commit() search_file = catalog_service(catalog_session).get_parameter_search_file("us") + content_sha256 = catalog_service(catalog_session).get_parameter_search_file_hash( + "us" + ) + assert content_sha256 == encoded.content_sha256 assert search_file.policyengine_version == POLICYENGINE_VERSION assert search_file.file_format == 1 assert search_file.compressed_content == encoded.compressed_content @@ -673,6 +677,10 @@ def test_a_version_without_a_search_file_of_the_current_format_is_unavailable( with pytest.raises(ParameterSearchFileUnavailableError) as raised: catalog_service(catalog_session).get_parameter_search_file("us") + with pytest.raises(ParameterSearchFileUnavailableError) as raised_for_hash: + catalog_service(catalog_session).get_parameter_search_file_hash("us") + + assert str(raised_for_hash.value) == str(raised.value) assert str(raised.value) == ( f"catalog version {POLICYENGINE_VERSION} has no parameter search file: a " From 2a50440cf5f7c4735a5a26fefc0b5427234381dd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:58:33 +0400 Subject: [PATCH 14/19] Declare editable_changes as an optional list that is never null The file omits editable_changes when editability never changes and never writes null, so the OpenAPI model now declares it as an optional array with no null, and a test compares the builder's typed shapes with the models by JSON type and required keys as well as by field name. Co-Authored-By: Claude Opus 5.5 --- .../v2/metadata/response_models.py | 11 ++- .../v2/test_metadata_search_file_routes.py | 97 +++++++++---------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/policyengine_api/fastapi_routes/v2/metadata/response_models.py b/policyengine_api/fastapi_routes/v2/metadata/response_models.py index 15c78f83e..a5a44b774 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/response_models.py +++ b/policyengine_api/fastapi_routes/v2/metadata/response_models.py @@ -5,6 +5,8 @@ from datetime import date from typing import Generic, Literal, TypeVar +from pydantic import Field + from policyengine_api.services.v2.metadata.types import ( MetadataCanonicalParameterValue, MetadataCatalogParametersResult, @@ -175,8 +177,9 @@ class MetadataParameterSearchFileParameter(StrictResponseModel): ``label`` is the stored label, or null when none is stored. ``editable`` says whether a draft can edit the earliest current-law value (a number, a - yes/no value or an infinite threshold); ``editable_changes``, when - present, lists the dates from which that changes. + yes/no value or an infinite threshold); ``editable_changes``, present only + when that changes over time and never null, lists the dates from which it + changes. """ path: str @@ -184,7 +187,9 @@ class MetadataParameterSearchFileParameter(StrictResponseModel): unit: str | None description: str | None editable: bool - editable_changes: list[MetadataParameterSearchFileEditabilityChange] | None = None + editable_changes: list[MetadataParameterSearchFileEditabilityChange] = Field( + default_factory=list + ) class MetadataParameterSearchFileLabel(StrictResponseModel): diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index 789491f9c..960a164aa 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -4,9 +4,11 @@ import gzip from datetime import date -from typing import get_type_hints +from types import UnionType +from typing import Union, get_args, get_origin, get_type_hints import pytest +from pydantic import BaseModel from policyengine_api.data.v2.catalog.catalog_selection import ( MetadataCatalogVersionNotFoundError, @@ -339,60 +341,51 @@ def test_the_built_search_file_matches_the_declared_content_schema() -> None: assert content.parameters[0].editable_changes is not None -def test_the_search_file_shapes_match_the_declared_content_models() -> None: - """The builder's typed shapes and the OpenAPI models name the same fields.""" - pairs = ( - (ParameterSearchFileDocument, MetadataParameterSearchFileContent), - (SearchFileParameterEntry, MetadataParameterSearchFileParameter), - (EditabilityChangeEntry, MetadataParameterSearchFileEditabilityChange), - (SearchFileLabelEntry, MetadataParameterSearchFileLabel), - (SearchFileNodeEntry, MetadataParameterSearchFileNode), - ) - for typed_shape, model in pairs: - assert set(get_type_hints(typed_shape)) == set(model.model_fields), model - assert SearchFileParameterEntry.__optional_keys__ == {"editable_changes"} +def _json_shape(annotation: object, pairs: dict[object, type]) -> object: + """Return an annotation's JSON shape, naming each paired type by its model. + A ``date`` field is an ISO date string in JSON. + """ + if annotation in pairs: + return pairs[annotation].__name__ + if isinstance(annotation, type) and issubclass(annotation, BaseModel): + return annotation.__name__ + if annotation is date: + return str + origin = get_origin(annotation) + if origin is list: + return ("list", _json_shape(get_args(annotation)[0], pairs)) + if origin in (Union, UnionType): + return frozenset( + _json_shape(argument, pairs) for argument in get_args(annotation) + ) + return annotation -@pytest.mark.parametrize("accept_encoding", ["gzip;q=0", "identity"]) -def test_a_large_search_file_stays_uncompressed_for_a_client_refusing_gzip( - accept_encoding: str, -) -> None: - content: ParameterSearchFileDocument = { - **SEARCH_FILE_CONTENT, - "parameters": [ - { - "path": f"gov.example.rate_{index}", - "label": f"Rate {index}", - "unit": "/1", - "description": None, - "editable": True, - } - for index in range(200) - ], - } - encoded = encode_parameter_search_file(content) - stored = MetadataParameterSearchFile( - policyengine_version="4.20.3", - file_format=1, - compressed_content=encoded.compressed_content, - content_sha256=encoded.content_sha256, - uncompressed_size=encoded.uncompressed_size, - compressed_size=encoded.compressed_size, - ) - - response = metadata_route_client( - lambda: ResourceReader({"get_parameter_search_file": stored}) - ).get( - SEARCH_FILE_PATH, - params={"country_id": "us"}, - headers={"Accept-Encoding": accept_encoding}, - ) - assert encoded.uncompressed_size > 1000 - assert response.status_code == 200 - assert "content-encoding" not in response.headers - assert response.headers["etag"] == f'"{encoded.content_sha256}"' - assert response.content == gzip.decompress(encoded.compressed_content) +def test_the_search_file_shapes_match_the_declared_content_models() -> None: + """The builder's typed shapes and the OpenAPI models declare the same JSON. + + Field names and JSON types match, and a key the file may omit is optional + but never null in the model. + """ + pairs: dict[object, type] = { + ParameterSearchFileDocument: MetadataParameterSearchFileContent, + SearchFileParameterEntry: MetadataParameterSearchFileParameter, + EditabilityChangeEntry: MetadataParameterSearchFileEditabilityChange, + SearchFileLabelEntry: MetadataParameterSearchFileLabel, + SearchFileNodeEntry: MetadataParameterSearchFileNode, + } + for typed_shape, model in pairs.items(): + hints = get_type_hints(typed_shape) + assert set(hints) == set(model.model_fields), model + for name, annotation in hints.items(): + assert _json_shape(annotation, pairs) == _json_shape( + model.model_fields[name].annotation, pairs + ), (model, name) + optional_keys = getattr(typed_shape, "__optional_keys__", frozenset()) + for name, field in model.model_fields.items(): + assert field.is_required() is (name not in optional_keys), (model, name) + assert SearchFileParameterEntry.__optional_keys__ == {"editable_changes"} def test_browser_scripts_can_read_the_search_file_entity_tag() -> None: From 23f635a0c7a5083cd1a086db7468aa8e15b71904 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:59:29 +0400 Subject: [PATCH 15/19] Pin gzip negotiation precedence, streamed bodies and Starlette's responders Tests now pin accepts_gzip's precedence (an explicit gzip entry decides over the wildcard either way, x-gzip counts as gzip), that a streamed body stays uncompressed when gzip is refused and is gzipped when it is accepted, and that Starlette still provides the two responders NegotiatedGZipMiddleware builds, with the arguments it passes. A unit test follows a value list with a null between two numbers through the editability changes, in date order. Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_content_encoding.py | 96 +++++++++++++++++++++ tests/unit/v2/test_parameter_search_file.py | 47 +++++++++- 2 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_content_encoding.py diff --git a/tests/unit/test_content_encoding.py b/tests/unit/test_content_encoding.py new file mode 100644 index 000000000..e7f648be5 --- /dev/null +++ b/tests/unit/test_content_encoding.py @@ -0,0 +1,96 @@ +"""Gzip negotiation that weighs Accept-Encoding qualities.""" + +from __future__ import annotations + +import inspect + +import pytest +from starlette.applications import Starlette +from starlette.middleware import gzip as starlette_gzip +from starlette.responses import StreamingResponse +from starlette.routing import Route +from starlette.testclient import TestClient + +from policyengine_api.content_encoding import NegotiatedGZipMiddleware, accepts_gzip + +STREAMED_CHUNK = b"x" * 1_500 + + +@pytest.mark.parametrize( + ("accept_encoding", "accepted"), + [ + ("gzip", True), + ("GZIP", True), + ("x-gzip", True), + ("gzip;q=0.001", True), + ("gzip;q=0", False), + ("gzip;q=not-a-number", False), + ("*", True), + ("*;q=0", False), + # An explicit gzip entry decides over the wildcard either way. + ("*, gzip;q=0", False), + ("*;q=0, gzip", True), + ("br, deflate", False), + ("gzipped", False), + ("", False), + (None, False), + ], +) +def test_accepts_gzip_weighs_qualities_and_exact_coding_names( + accept_encoding: str | None, *, accepted: bool +) -> None: + assert accepts_gzip(accept_encoding) is accepted + + +async def _streamed(_request) -> StreamingResponse: + async def chunks(): + for _ in range(3): + yield STREAMED_CHUNK + + return StreamingResponse(chunks(), media_type="text/plain") + + +def _streaming_client() -> TestClient: + application = Starlette(routes=[Route("/streamed", _streamed)]) + return TestClient( + NegotiatedGZipMiddleware(application, minimum_size=1_000, compresslevel=4) + ) + + +def test_a_streamed_body_stays_uncompressed_when_gzip_is_refused() -> None: + response = _streaming_client().get( + "/streamed", headers={"Accept-Encoding": "gzip;q=0"} + ) + + assert response.status_code == 200 + assert "content-encoding" not in response.headers + assert response.content == STREAMED_CHUNK * 3 + + +def test_a_streamed_body_is_gzipped_when_gzip_is_accepted() -> None: + response = _streaming_client().get("/streamed", headers={"Accept-Encoding": "gzip"}) + + assert response.status_code == 200 + assert response.headers["content-encoding"] == "gzip" + assert response.content == STREAMED_CHUNK * 3 + + +def test_starlette_still_provides_the_responders_the_middleware_uses() -> None: + """NegotiatedGZipMiddleware builds Starlette's undocumented responders. + + If a Starlette upgrade renames them or changes how they are built, this + test names what changed. + """ + identity_responder = getattr(starlette_gzip, "IdentityResponder", None) + gzip_responder = getattr(starlette_gzip, "GZipResponder", None) + assert identity_responder is not None, "starlette.middleware.gzip.IdentityResponder" + assert gzip_responder is not None, "starlette.middleware.gzip.GZipResponder" + assert list(inspect.signature(identity_responder).parameters) == [ + "app", + "minimum_size", + ] + assert list(inspect.signature(gzip_responder).parameters) == [ + "app", + "minimum_size", + "compresslevel", + ] diff --git a/tests/unit/v2/test_parameter_search_file.py b/tests/unit/v2/test_parameter_search_file.py index 52e6c1790..5a70e7b56 100644 --- a/tests/unit/v2/test_parameter_search_file.py +++ b/tests/unit/v2/test_parameter_search_file.py @@ -1,11 +1,12 @@ -"""Each catalog version's parameter search file, built from stored rows.""" +"""Each catalog version's parameter search file, built from catalog records.""" from __future__ import annotations import gzip import hashlib import json -from datetime import date +from datetime import UTC, date, datetime +from uuid import uuid4 import pytest @@ -14,11 +15,16 @@ EditabilityChange, ParameterSearchFileDocument, SearchParameterSource, + _parameter_editability, build_parameter_search_file, encode_parameter_search_file, is_editable_value, ) from policyengine_api.data.v2.catalog.parameter_tree import TreeNodeSource +from policyengine_api.data.v2.catalog.records import ( + ParameterRecord, + ParameterValueRecord, +) def _parameter( @@ -334,3 +340,40 @@ def test_editable_values_follow_v3s_draft_editor( ) -> None: # v3's isDraftableValue: a number, a yes/no switch, or "Infinity"/"-Infinity". assert is_editable_value(value) is editable + + +def test_a_null_value_between_numbers_changes_editability_twice() -> None: + parameter_id = uuid4() + values = ( + (0.04, datetime(2015, 1, 1, tzinfo=UTC), datetime(2020, 12, 31, tzinfo=UTC)), + (None, datetime(2021, 1, 1, tzinfo=UTC), datetime(2022, 12, 31, tzinfo=UTC)), + (1.33, datetime(2023, 1, 1, tzinfo=UTC), None), + ) + parameter = ParameterRecord( + id=parameter_id, + model_version_id=uuid4(), + name="gov.irs.credits.premium_tax_credit.phase_out.ending_rate[1].amount", + label=None, + description=None, + data_type="float", + unit="/1", + values=tuple( + ParameterValueRecord( + id=uuid4(), + parameter_id=parameter_id, + value_json=value, + start_date=start_date, + end_date=end_date, + ) + # Listed out of date order on purpose. + for value, start_date, end_date in reversed(values) + ), + economy=True, + household=True, + ) + + assert list(_parameter_editability(parameter)) == [ + EditabilityChange(parameter.name, date(2015, 1, 1), editable=True), + EditabilityChange(parameter.name, date(2021, 1, 1), editable=False), + EditabilityChange(parameter.name, date(2023, 1, 1), editable=True), + ] From 9bd3f4168abac7deed2cc3e25652e7cd43b17919 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:00:51 +0400 Subject: [PATCH 16/19] Say plainly which catalog versions have a search file The search file route's 503 message, its OpenAPI description, the error's docstring and the changelog now state the rule instead of implying that republishing helps: only catalog format 2 versions that publication has published, or run with installed, since search files were added have a file. The fixed fragment also records that Accept-Encoding: * and differently cased values now receive gzip. The shared ETag description now says the tag hashes the uncompressed body, which the gzip form shares. Co-Authored-By: Claude Opus 5.5 --- changelog.d/v2plan-1f-118-search-file.added.md | 2 +- changelog.d/v2plan-1f-118-search-file.fixed.md | 2 +- policyengine_api/fastapi_routes/v2/metadata/common.py | 10 +++++++--- policyengine_api/services/v2/metadata/services.py | 4 ++-- policyengine_api/services/v2/metadata/validators.py | 9 +++++---- tests/integration/test_v2_metadata_routes.py | 5 +++-- tests/unit/v2/test_metadata_search_file_routes.py | 6 +++--- tests/unit/v2/test_metadata_service.py | 6 +++--- 8 files changed, 25 insertions(+), 19 deletions(-) diff --git a/changelog.d/v2plan-1f-118-search-file.added.md b/changelog.d/v2plan-1f-118-search-file.added.md index 842482c27..09c433355 100644 --- a/changelog.d/v2plan-1f-118-search-file.added.md +++ b/changelog.d/v2plan-1f-118-search-file.added.md @@ -1 +1 @@ -Catalog publication writes a parameter search file for each catalog version it publishes into a new table, parameter_search_files, built from the installed packages' catalog: every parameter a reform can change with its path, stored label, unit, description and whether a draft can edit its current-law value (with the dates from which that changes), the stored labels of the other parameters under gov, and gov and every labelled node under it, as gzip-compressed JSON with its SHA-256. A new route, /v2/parameters/search-file, serves a version's file with a strong ETag, 304 revalidation and long-lived caching for a named version, as gzip to clients that accept it. A version stored by an earlier release has no file and returns 503 with a message naming it. +Catalog publication writes a parameter search file for each catalog version it publishes into a new table, parameter_search_files, built from the installed packages' catalog: every parameter a reform can change with its path, stored label, unit, description and whether a draft can edit its current-law value (with the dates from which that changes), the stored labels of the other parameters under gov, and gov and every labelled node under it, as gzip-compressed JSON with its SHA-256. A new route, /v2/parameters/search-file, serves a version's file with a strong ETag, 304 revalidation and long-lived caching for a named version, as gzip to clients that accept it. Publication writes a file for each catalog format 2 version it publishes or runs with installed, so a version stored before catalog format 2, or stored before search files existed and not installed since, has none and returns 503 with a message naming it. diff --git a/changelog.d/v2plan-1f-118-search-file.fixed.md b/changelog.d/v2plan-1f-118-search-file.fixed.md index 90ff0fc08..274c88fe8 100644 --- a/changelog.d/v2plan-1f-118-search-file.fixed.md +++ b/changelog.d/v2plan-1f-118-search-file.fixed.md @@ -1 +1 @@ -Responses are gzip-compressed only for a client whose Accept-Encoding accepts gzip: a client that refuses it (for example gzip;q=0) now receives an uncompressed response instead of a gzip one. +Responses are gzip-compressed according to the qualities in Accept-Encoding instead of a search for the text "gzip": a client that refuses gzip (for example gzip;q=0) now receives an uncompressed response, and `Accept-Encoding: *` or a differently cased value such as GZIP now receives gzip, where both used to receive an uncompressed response. diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index 1d94ac189..17143afaf 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/common.py +++ b/policyengine_api/fastapi_routes/v2/metadata/common.py @@ -80,8 +80,9 @@ "model": V2ErrorResponse, "description": ( "The initialized v2 catalog is unavailable, or the catalog version " - "has no parameter search file because it was published before search " - "files existed; the message then names the version." + "has no parameter search file: only catalog format 2 versions that " + "publication has published, or run with installed, since search files " + "were added have one. The message then names the version." ), }, } @@ -116,7 +117,10 @@ CACHE_HEADER_DESCRIPTIONS: dict[str, dict[str, Any]] = { "ETag": { - "description": "Strong validator: a hash of the exact response body.", + "description": ( + "Strong validator: a hash of the uncompressed response body, so a " + "gzip-encoded response carries the same tag." + ), "schema": {"type": "string"}, }, "Cache-Control": { diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index 4870ae9d0..489d4e2da 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -117,8 +117,8 @@ def _parameter_search_file_unavailable( ) -> ParameterSearchFileUnavailableError: msg = ( f"catalog version {selected.policyengine_version} has no parameter search " - "file: a file is written when a catalog version is published, and this " - "version was published before" + "file: only catalog format 2 versions that catalog publication has " + "published, or run with installed, since search files were added have one" ) return ParameterSearchFileUnavailableError(msg) diff --git a/policyengine_api/services/v2/metadata/validators.py b/policyengine_api/services/v2/metadata/validators.py index 77f97b265..d74d7f5c0 100644 --- a/policyengine_api/services/v2/metadata/validators.py +++ b/policyengine_api/services/v2/metadata/validators.py @@ -27,10 +27,11 @@ class InvalidMetadataPageError(ValueError): class ParameterSearchFileUnavailableError(MetadataCatalogUnavailableError): """Raised when a selected catalog version has no parameter search file. - Catalog publication writes a version's file only when it publishes the - version, from the installed packages' catalog, so a version stored by an - earlier release before search files existed has none. The message names - the version and is safe to return to the caller. + Catalog publication writes a file, from the installed packages' catalog, + for a catalog format 2 version it publishes or runs with installed. Other + versions have none: those stored before catalog format 2, and those stored + before search files existed that publication has not run with installed + since. The message names the version and is safe to return to the caller. """ diff --git a/tests/integration/test_v2_metadata_routes.py b/tests/integration/test_v2_metadata_routes.py index 97cbfb881..82f42645d 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1109,8 +1109,9 @@ def test_postgres_search_file_of_a_version_without_one_is_unavailable( "status": "error", "message": ( f"catalog version {POLICYENGINE_VERSION} has no parameter search " - "file: a file is written when a catalog version is published, and " - "this version was published before" + "file: only catalog format 2 versions that catalog publication has " + "published, or run with installed, since search files were added have " + "one" ), } diff --git a/tests/unit/v2/test_metadata_search_file_routes.py b/tests/unit/v2/test_metadata_search_file_routes.py index 960a164aa..71cf630a3 100644 --- a/tests/unit/v2/test_metadata_search_file_routes.py +++ b/tests/unit/v2/test_metadata_search_file_routes.py @@ -240,9 +240,9 @@ def test_search_file_route_sends_the_file_when_the_etag_differs() -> None: def test_search_file_route_reports_a_missing_file_as_unavailable() -> None: message = ( - "catalog version 4.20.3 has no parameter search file: a file is written " - "when a catalog version is published, and this version was published " - "before" + "catalog version 4.20.3 has no parameter search file: only catalog format " + "2 versions that catalog publication has published, or run with " + "installed, since search files were added have one" ) reader = _search_file_reader(error=ParameterSearchFileUnavailableError(message)) diff --git a/tests/unit/v2/test_metadata_service.py b/tests/unit/v2/test_metadata_service.py index 02db0db08..b78d2f6b7 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -683,7 +683,7 @@ def test_a_version_without_a_search_file_of_the_current_format_is_unavailable( assert str(raised_for_hash.value) == str(raised.value) assert str(raised.value) == ( - f"catalog version {POLICYENGINE_VERSION} has no parameter search file: a " - "file is written when a catalog version is published, and this version " - "was published before" + f"catalog version {POLICYENGINE_VERSION} has no parameter search file: " + "only catalog format 2 versions that catalog publication has published, " + "or run with installed, since search files were added have one" ) From 8c7b7380745962dc09c714c0c95087bca6c7cd12 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:02:19 +0400 Subject: [PATCH 17/19] Type-check the gzip negotiation module with the v2 code policyengine_api.content_encoding is now in the mypy file list and in the api.quality.mypy-v2 rule's enforced paths and scope, so CI's typecheck-v2 job checks it with untyped definitions disallowed; the rules document is regenerated. Co-Authored-By: Claude Opus 5.5 --- docs/generated/rules.md | 2 +- pyproject.toml | 1 + rules/registry.toml | 5 +++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 2a5da9e5f..e53de3a53 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -334,7 +334,7 @@ flowchart LR | `api.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date and pick random samples (T201, S603, S607, S310, DTZ011, S311); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S310, S311, S603, S607, T201 for `scripts/**` | `pr.yml:lint` | — | | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.file-length` | A tracked Python file has at most 800 physical lines, or at most 1,500 if it is test code (a file under a tests or fixtures directory, or a conftest.py); a longer file is split by responsibility into modules, and keeps its public import path. | metric | enforced | yes | quality guard `file-length` | `pr.yml:quality-guards` | yes | -| `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, the user sign-in and account-deletion libraries, the country list and its startup check pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | +| `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, the gzip negotiation, the user sign-in and account-deletion libraries, the country list and its startup check pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `api.quality.ruff-lint` | All Python code passes ruff check with every ruff rule selected (ALL), with no ratchet, except the ignores in pyproject.toml: COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212. | mechanical | enforced | yes | ruff check (ALL) | `pr.yml:lint` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | diff --git a/pyproject.toml b/pyproject.toml index 70414d441..375d38612 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -163,6 +163,7 @@ files = [ "policyengine_api/fastapi_routes/resource_service_protocols.py", "policyengine_api/fastapi_routes/v2", "policyengine_api/services/v2", + "policyengine_api/content_encoding.py", "policyengine_api/libs/user_auth.py", "policyengine_api/libs/auth0_management.py", "policyengine_api/startup_country_check.py", diff --git a/rules/registry.toml b/rules/registry.toml index c3267be5e..47e0cd51f 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1037,9 +1037,9 @@ invocation = "ruff check ." [[rule]] id = "api.quality.mypy-v2" category = "quality" -statement = "Shared query models, FastAPI dependencies, v2 routes and services, the user sign-in and account-deletion libraries, the country list and its startup check pass mypy with untyped definitions disallowed." +statement = "Shared query models, FastAPI dependencies, v2 routes and services, the gzip negotiation, the user sign-in and account-deletion libraries, the country list and its startup check pass mypy with untyped definitions disallowed." rationale = "docs/engineering/skills/v2-code-organization.md" -scope = ["policyengine_api/fastapi_routes/v2/**/*.py", "policyengine_api/services/v2/**/*.py", "policyengine_api/libs/user_auth.py", "policyengine_api/libs/auth0_management.py", "policyengine_api/startup_country_check.py", "policyengine_api/supported_countries.py"] +scope = ["policyengine_api/fastapi_routes/v2/**/*.py", "policyengine_api/services/v2/**/*.py", "policyengine_api/content_encoding.py", "policyengine_api/libs/user_auth.py", "policyengine_api/libs/auth0_management.py", "policyengine_api/startup_country_check.py", "policyengine_api/supported_countries.py"] tier = "mechanical" status = "enforced" autofix = false @@ -1053,6 +1053,7 @@ paths = [ "policyengine_api/fastapi_routes/resource_service_protocols.py", "policyengine_api/fastapi_routes/v2", "policyengine_api/services/v2", + "policyengine_api/content_encoding.py", "policyengine_api/libs/user_auth.py", "policyengine_api/libs/auth0_management.py", "policyengine_api/startup_country_check.py", From f35d0ff0cc604ef184877b1eed1a92f29f2f0beb Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:11:05 +0400 Subject: [PATCH 18/19] Look up the stored catalog version through the shared helper The search file writer now reads a stored version's ID and catalog format with publication_reconciliation.stored_version, as the parameter tree writer does, instead of its own query. Co-Authored-By: Claude Opus 5.5 --- .../publication_parameter_search_file.py | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/policyengine_api/data/v2/catalog/publication_parameter_search_file.py b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py index 0e8dcd815..4bbda947e 100644 --- a/policyengine_api/data/v2/catalog/publication_parameter_search_file.py +++ b/policyengine_api/data/v2/catalog/publication_parameter_search_file.py @@ -26,6 +26,10 @@ country_search_file, encode_parameter_search_file, ) +from policyengine_api.data.v2.catalog.publication_reconciliation import ( + StoredVersion, + stored_version, +) from policyengine_api.data.v2.catalog.publication_types import ( CatalogPublicationError, ) @@ -39,28 +43,20 @@ from policyengine_api.data.v2.catalog.records import CountryCatalog -MODELS = V2_METADATA.tables["tax_benefit_models"] -MODEL_VERSIONS = V2_METADATA.tables["tax_benefit_model_versions"] PARAMETER_SEARCH_FILES = V2_METADATA.tables["parameter_search_files"] -def _stored_version( +def _version_for_search_file( connection: Connection, country: CountryCatalog -) -> tuple[UUID, int | None]: - """Return the stored version's ID and catalog format (None before format 2).""" - row = connection.execute( - sa.select(MODEL_VERSIONS.c.id, MODEL_VERSIONS.c.catalog_format) - .select_from( - MODEL_VERSIONS.join(MODELS, MODELS.c.id == MODEL_VERSIONS.c.model_id) - ) - .where( - MODELS.c.name == country.model.name, - MODEL_VERSIONS.c.version == country.model_version.version, +) -> 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" ) - ).one() - version_id: UUID = row.id - catalog_format: int | None = row.catalog_format - return version_id, catalog_format + raise CatalogPublicationError(msg) + return stored def _stored_content_sha256(connection: Connection, version_id: UUID) -> str | None: @@ -85,16 +81,16 @@ def write_parameter_search_files( """ written = [] for country in sorted(countries, key=lambda item: item.model.name): - version_id, catalog_format = _stored_version(connection, country) - if catalog_format is None: + stored = _version_for_search_file(connection, country) + if stored.catalog_format is None: continue encoded = encode_parameter_search_file(country_search_file(country)) - stored_sha256 = _stored_content_sha256(connection, version_id) + stored_sha256 = _stored_content_sha256(connection, stored.id) if stored_sha256 is None: connection.execute( sa.insert(PARAMETER_SEARCH_FILES).values( id=uuid4(), - tax_benefit_model_version_id=version_id, + tax_benefit_model_version_id=stored.id, file_format=PARAMETER_SEARCH_FILE_FORMAT, compressed_content=encoded.compressed_content, content_sha256=encoded.content_sha256, @@ -129,10 +125,10 @@ def assert_parameter_search_files_written( A version stored before catalog format 2 is exempt: it gets no file. """ for country in countries: - version_id, catalog_format = _stored_version(connection, country) - if catalog_format is None: + stored = _version_for_search_file(connection, country) + if stored.catalog_format is None: continue - if _stored_content_sha256(connection, version_id) is None: + if _stored_content_sha256(connection, stored.id) is None: msg = ( f"the {country.country_id} catalog version " f"{country.model_version.version} has no parameter search file" From f95842c32ecbb5aa3d81fb50905182d93f5260c0 Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Fri, 30 Oct 2026 21:30:13 +0400 Subject: [PATCH 19/19] Split the v2 catalog tables' lifecycle tests under the file-length limit The countries tests and the search file's revision test took test_alembic_v2_lifecycle.py to 1,588 lines, over the 1,500-line limit for test code. The lifecycle tests of the catalog tables' revisions (catalog format 2's columns, the parameter tree and the parameter search file) move unchanged to test_alembic_v2_catalog_tables.py, with their revisions and helpers. The three helpers both modules use move to tests/fixtures/integration/alembic_v2.py without their leading underscore: _disposable_url becomes disposable_v2_url, _config alembic_v2_config and _columns table_columns. The v2 Alembic check's script, its workflow test and the testing guide's commands name the new module. It reads the same disposable database settings as the lifecycle tests, which the job already sets. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/test_alembic_v2_lifecycle.sh | 3 +- docs/engineering/skills/testing.md | 5 +- tests/fixtures/integration/alembic_v2.py | 40 ++ .../test_alembic_v2_catalog_tables.py | 351 +++++++++++++++ .../integration/test_alembic_v2_lifecycle.py | 422 ++---------------- tests/unit/test_alembic_workflows.py | 1 + 6 files changed, 427 insertions(+), 395 deletions(-) create mode 100644 tests/fixtures/integration/alembic_v2.py create mode 100644 tests/integration/test_alembic_v2_catalog_tables.py diff --git a/.github/scripts/test_alembic_v2_lifecycle.sh b/.github/scripts/test_alembic_v2_lifecycle.sh index eab5e3e07..0ed37eba0 100755 --- a/.github/scripts/test_alembic_v2_lifecycle.sh +++ b/.github/scripts/test_alembic_v2_lifecycle.sh @@ -4,4 +4,5 @@ set -euo pipefail uv run --frozen --extra dev pytest -q \ tests/unit/v2/test_alembic_v2.py \ - tests/integration/test_alembic_v2_lifecycle.py + tests/integration/test_alembic_v2_lifecycle.py \ + tests/integration/test_alembic_v2_catalog_tables.py diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index bae95da2d..8bf5c5060 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -107,7 +107,7 @@ The generated-only v2 Alembic chain and disposable Postgres lifecycle: ```bash uv run pytest tests/unit/v2/test_alembic_v2.py tests/unit/test_alembic_workflows.py -q V2_ALEMBIC_DISPOSABLE_TEST=1 V2_MIGRATION_DATABASE_URL="postgresql+psycopg://.../policyengine_v2_alembic_test" \ - uv run pytest tests/integration/test_alembic_v2_lifecycle.py -q + uv run pytest tests/integration/test_alembic_v2_lifecycle.py tests/integration/test_alembic_v2_catalog_tables.py -q ``` The disposable lifecycle must start from empty Postgres, upgrade to `head`, @@ -305,6 +305,7 @@ V2_MIGRATION_DATABASE_URL="postgresql+psycopg://.../policyengine_v2_alembic_test uv run pytest \ tests/integration/test_alembic_mysql_lifecycle.py \ tests/integration/test_alembic_v2_lifecycle.py \ + tests/integration/test_alembic_v2_catalog_tables.py \ tests/integration/test_v2_policy_persistence.py \ tests/integration/test_v1_policy_dual_write.py \ tests/integration/test_mysql_policy_dual_write.py \ @@ -379,6 +380,7 @@ V2_MIGRATION_DATABASE_URL="postgresql+psycopg://.../policyengine_v2_alembic_test uv run --frozen pytest \ tests/integration/test_alembic_mysql_lifecycle.py \ tests/integration/test_alembic_v2_lifecycle.py \ + tests/integration/test_alembic_v2_catalog_tables.py \ tests/integration/test_v2_household_persistence.py \ tests/integration/test_v1_household_dual_write.py \ tests/integration/test_mysql_household_spm_dual_write.py -q @@ -436,6 +438,7 @@ V2_ALEMBIC_DISPOSABLE_TEST=1 \ V2_MIGRATION_DATABASE_URL="postgresql+psycopg://.../policyengine_v2_alembic_test" \ uv run --frozen pytest \ tests/integration/test_alembic_v2_lifecycle.py \ + tests/integration/test_alembic_v2_catalog_tables.py \ tests/integration/test_v2_user_identity_persistence.py -q ``` diff --git a/tests/fixtures/integration/alembic_v2.py b/tests/fixtures/integration/alembic_v2.py new file mode 100644 index 000000000..ce72f8f86 --- /dev/null +++ b/tests/fixtures/integration/alembic_v2.py @@ -0,0 +1,40 @@ +"""The disposable Postgres target and Alembic config of the v2 lifecycle tests.""" + +import os + +import pytest +from alembic.config import Config +from sqlalchemy import inspect + +from policyengine_api.constants import REPO +from policyengine_api.data.v2.migration_target import ( + V2_ALEMBIC_DISPOSABLE_TEST, + load_v2_alembic_settings, +) +from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL + + +def disposable_v2_url() -> str: + database_url = os.environ.get(V2_MIGRATION_DATABASE_URL, "") + if not database_url: + pytest.skip(f"{V2_MIGRATION_DATABASE_URL} is not set") + settings = load_v2_alembic_settings( + { + V2_MIGRATION_DATABASE_URL: database_url, + V2_ALEMBIC_DISPOSABLE_TEST: os.environ.get(V2_ALEMBIC_DISPOSABLE_TEST, ""), + } + ) + if not settings.disposable_test: + pytest.fail("v2 lifecycle tests require disposable-test mode") + return settings.url.render_as_string(hide_password=False) + + +def alembic_v2_config() -> Config: + return Config(str(REPO / "alembic-v2.ini")) + + +def table_columns(engine, table_name: str) -> dict[str, dict]: + return { + column["name"]: column + for column in inspect(engine).get_columns(table_name, schema="public") + } diff --git a/tests/integration/test_alembic_v2_catalog_tables.py b/tests/integration/test_alembic_v2_catalog_tables.py new file mode 100644 index 000000000..3c656d248 --- /dev/null +++ b/tests/integration/test_alembic_v2_catalog_tables.py @@ -0,0 +1,351 @@ +"""Exercise the v2 catalog tables' revisions against disposable Postgres.""" + +from uuid import UUID, uuid4 + +import pytest +import sqlalchemy as sa +from alembic import command +from sqlalchemy import create_engine, inspect, text + +from tests.fixtures.integration.alembic_v2 import ( + alembic_v2_config, + disposable_v2_url, + table_columns, +) + +CATALOG_FORMAT_2_PARENT_REVISION = "fb1a3e40d3d6" +CATALOG_FORMAT_2_COLUMNS = { + "tax_benefit_model_versions": ("catalog_format", "entities", "basic_inputs"), + "variables": ( + "is_input_variable", + "hidden_input", + "module_name", + "adds_parameter_path", + "subtracts_parameter_path", + "enum_values", + "unit", + "definition_period", + ), + "parameters": ("economy", "household"), +} + + +def _catalog_version_published_before_format_2(connection: sa.Connection) -> UUID: + """Store a model version with one variable and one parameter; return its id.""" + model_id = uuid4() + version_id = uuid4() + connection.execute( + text( + "INSERT INTO tax_benefit_models (id, name) " + "VALUES (:model_id, 'catalog-format-probe')" + ), + {"model_id": model_id}, + ) + connection.execute( + text( + "INSERT INTO tax_benefit_model_versions " + "(id, model_id, version, current_law_id, metadata_time_periods) " + "VALUES (:version_id, :model_id, '0.0.1-probe', 1, '[2026]')" + ), + {"version_id": version_id, "model_id": model_id}, + ) + connection.execute( + text( + "INSERT INTO variables " + "(id, name, entity, default_value, tax_benefit_model_version_id) " + "VALUES (:variable_id, 'probe_income', 'person', '0', :version_id)" + ), + {"variable_id": uuid4(), "version_id": version_id}, + ) + connection.execute( + text( + "INSERT INTO parameters (id, name, tax_benefit_model_version_id) " + "VALUES (:parameter_id, 'gov.probe.rate', :version_id)" + ), + {"parameter_id": uuid4(), "version_id": version_id}, + ) + return version_id + + +def _format_2_values(connection: sa.Connection, version_id: UUID) -> list[tuple]: + """Return the format 2 columns of the version's model, variable and parameter.""" + rows = [] + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + key = ( + "id" + if table_name == "tax_benefit_model_versions" + else "tax_benefit_model_version_id" + ) + rows.append( + tuple( + connection.execute( + text( + f"SELECT {', '.join(columns)} FROM {table_name} " + f"WHERE {key} = :version_id" + ), + {"version_id": version_id}, + ).one() + ) + ) + return rows + + +def test_catalog_format_2_columns_leave_earlier_versions_empty() -> None: + """A version stored before catalog format 2 keeps its format 2 fields null. + + Upgrading adds the columns as nullable without writing to stored rows, and + downgrading drops them and keeps every row. + """ + config = alembic_v2_config() + engine = create_engine(disposable_v2_url()) + version_id = None + try: + command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + assert set(columns).isdisjoint(table_columns(engine, table_name)) + with engine.begin() as connection: + version_id = _catalog_version_published_before_format_2(connection) + + command.upgrade(config, "head") + + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + stored_columns = table_columns(engine, table_name) + for column_name in columns: + assert stored_columns[column_name]["nullable"] + assert stored_columns[column_name]["default"] is None + with engine.connect() as connection: + assert _format_2_values(connection, version_id) == [ + (None, None, None), + (None, None, None, None, None, None, None, None), + (None, None), + ] + + command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) + + for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): + assert set(columns).isdisjoint(table_columns(engine, table_name)) + with engine.connect() as connection: + counts = connection.execute( + text( + "SELECT (SELECT count(*) FROM variables " + "WHERE tax_benefit_model_version_id = :version_id), " + "(SELECT count(*) FROM parameters " + "WHERE tax_benefit_model_version_id = :version_id)" + ), + {"version_id": version_id}, + ).one() + assert tuple(counts) == (1, 1) + finally: + with engine.begin() as connection: + connection.execute( + text( + "DELETE FROM tax_benefit_models WHERE name = 'catalog-format-probe'" + ) + ) + command.upgrade(config, "head") + engine.dispose() + + +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 = alembic_v2_config() + engine = create_engine(disposable_v2_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() + + +PARAMETER_SEARCH_FILE_PARENT_REVISION = "3869eb814a90" + + +def test_parameter_search_file_table_is_created_empty_and_dropped_without_loss() -> ( + None +): + """The search file revision creates an empty table and downgrades cleanly. + + The revision writes no rows: catalog publication writes each version's + file. A version holds at most one file, the file is deleted with its + version, and downgrading drops the table while every catalog row stays. + """ + config = alembic_v2_config() + engine = create_engine(disposable_v2_url()) + try: + command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) + assert "parameter_search_files" 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") + + insert_file = text( + "INSERT INTO parameter_search_files (id, tax_benefit_model_version_id, " + "file_format, compressed_content, content_sha256, uncompressed_size, " + "compressed_size) VALUES (:id, :version_id, 1, :content, :sha, 2, 22)" + ) + file_values = { + "version_id": version_id, + "content": b"\x1f\x8b compressed bytes", + "sha": "0" * 64, + } + with engine.begin() as connection: + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_search_files") + ).scalar_one() + == 0 + ) + connection.execute(insert_file, {"id": uuid4(), **file_values}) + with pytest.raises(sa.exc.IntegrityError), connection.begin_nested(): + connection.execute(insert_file, {"id": uuid4(), **file_values}) + assert ( + connection.execute( + text( + "SELECT compressed_content FROM parameter_search_files " + "WHERE tax_benefit_model_version_id = :version_id" + ), + {"version_id": version_id}, + ).scalar_one() + == file_values["content"] + ) + + command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) + + assert "parameter_search_files" 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 + ) + + command.upgrade(config, "head") + with engine.begin() as connection: + connection.execute(insert_file, {"id": uuid4(), **file_values}) + connection.execute( + text("DELETE FROM tax_benefit_model_versions WHERE id = :version_id"), + {"version_id": version_id}, + ) + assert ( + connection.execute( + text("SELECT count(*) FROM parameter_search_files") + ).scalar_one() + == 0 + ) + 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_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index a8dd3d21b..419d5b874 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1,7 +1,6 @@ """Exercise the isolated v2 Alembic lifecycle against disposable Postgres.""" import itertools -import os from collections.abc import Iterator from dataclasses import dataclass from datetime import UTC, datetime, timedelta @@ -16,15 +15,16 @@ from sqlalchemy import Engine, create_engine, inspect, text from sqlalchemy.dialects import postgresql -from policyengine_api.constants import REPO from policyengine_api.data.v2.migration_target import ( - V2_ALEMBIC_DISPOSABLE_TEST, V2_ALEMBIC_HEAD_REVISION, V2MigrationTargetError, - load_v2_alembic_settings, ) from policyengine_api.data.v2.models import V2_METADATA -from policyengine_api.data.v2.settings import V2_MIGRATION_DATABASE_URL +from tests.fixtures.integration.alembic_v2 import ( + alembic_v2_config, + disposable_v2_url, + table_columns, +) BASELINE_REVISION = "f5ef4347cb2a" STAGE_11_PREVIOUS_REVISION = "af34023a728f" @@ -35,25 +35,6 @@ V2_TABLE_NAMES = frozenset(table.name for table in V2_METADATA.tables.values()) -def _disposable_url() -> str: - database_url = os.environ.get(V2_MIGRATION_DATABASE_URL, "") - if not database_url: - pytest.skip(f"{V2_MIGRATION_DATABASE_URL} is not set") - settings = load_v2_alembic_settings( - { - V2_MIGRATION_DATABASE_URL: database_url, - V2_ALEMBIC_DISPOSABLE_TEST: os.environ.get(V2_ALEMBIC_DISPOSABLE_TEST, ""), - } - ) - if not settings.disposable_test: - pytest.fail("v2 lifecycle tests require disposable-test mode") - return settings.url.render_as_string(hide_password=False) - - -def _config() -> Config: - return Config(str(REPO / "alembic-v2.ini")) - - def _assert_head(engine) -> None: assert set(inspect(engine).get_table_names(schema="public")) == ( V2_TABLE_NAMES | {"alembic_version"} @@ -102,20 +83,13 @@ def _assert_head(engine) -> None: } <= set(inspect(engine).get_table_names(schema="public")) -def _columns(engine, table_name: str) -> dict[str, dict]: - return { - column["name"]: column - for column in inspect(engine).get_columns(table_name, schema="public") - } - - def _assert_pre_stage_12a_association_shape(engine) -> None: assert "user_identities" not in inspect(engine).get_table_names(schema="public") for table_name, reference in ( ("user_simulation_associations", "simulation"), ("user_report_associations", "report"), ): - columns = _columns(engine, table_name) + columns = table_columns(engine, table_name) assert {"country", "label"} <= columns.keys() assert { "country_id", @@ -131,10 +105,10 @@ def _assert_pre_stage_12a_association_shape(engine) -> None: ("user_policies", "policy"), ("user_household_associations", "household"), ): - columns = _columns(engine, table_name) + columns = table_columns(engine, table_name) assert f"legacy_{reference}_id" not in columns assert not columns[f"{reference}_id"]["nullable"] - user_columns = _columns(engine, "users") + user_columns = table_columns(engine, "users") assert "user_type" not in user_columns assert not user_columns["primary_country"]["nullable"] @@ -154,7 +128,7 @@ def _assert_stage_12a_association_shape(engine) -> None: ("user_simulation_associations", "simulation"), ("user_report_associations", "report"), ): - columns = _columns(engine, table_name) + columns = table_columns(engine, table_name) assert columns[f"{reference}_id"]["nullable"] assert columns[f"legacy_{reference}_id"]["nullable"] assert not columns["country_id"]["nullable"] @@ -177,10 +151,10 @@ def _assert_stage_12a_association_shape(engine) -> None: ) assert report_policy_key["options"]["ondelete"] == "SET NULL" for table_name in ("user_simulation_associations", "user_report_associations"): - columns = _columns(engine, table_name) + columns = table_columns(engine, table_name) assert columns["country"]["nullable"] assert columns["label"]["nullable"] - user_columns = _columns(engine, "users") + user_columns = table_columns(engine, "users") assert not user_columns["user_type"]["nullable"] assert "standard" in user_columns["user_type"]["default"] assert user_columns["primary_country"]["nullable"] @@ -423,8 +397,8 @@ def _assert_pre_countries_shape(engine) -> None: def test_empty_upgrade_check_base_downgrade_and_reupgrade() -> None: - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) try: @@ -712,8 +686,8 @@ def _assert_head_serves_previous_revision( new required column without a server default; ``read_only_tables`` are the parent's tables that only new code writes. """ - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) try: @@ -796,8 +770,8 @@ def test_countries_schema_serves_the_previous_revision() -> None: def test_upgrade_to_head_validates_the_resulting_schema_against_metadata() -> None: - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) try: @@ -822,8 +796,8 @@ def test_upgrade_to_head_validates_the_resulting_schema_against_metadata() -> No def test_baseline_uses_native_uuid_report_run_idempotency() -> None: - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) def idempotency_column_type(): @@ -902,8 +876,8 @@ def report_run_checks() -> set[str]: def test_baseline_region_default_enforces_same_model_dataset() -> None: - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) first_model_id = uuid4() second_model_id = uuid4() @@ -1021,8 +995,8 @@ def test_baseline_region_default_enforces_same_model_dataset() -> None: def test_stage_12a_reshape_refuses_retained_dormant_association_rows() -> None: """A retained pre-12A report association stops the reshape unchanged.""" - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) user_id = uuid4() model_id = uuid4() @@ -1081,8 +1055,8 @@ def test_countries_revision_keeps_existing_rows_and_rejects_unlisted_countries() None ): """Rows saved before the revision survive it, and only listed countries pass.""" - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) user_id = uuid4() @@ -1143,8 +1117,8 @@ def reflected(name: str) -> sa.Table: def test_countries_downgrade_refuses_records_of_an_unlisted_country() -> None: """The restored US-and-UK checks stop a downgrade that would orphan records.""" - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) user_id = uuid4() @@ -1181,8 +1155,8 @@ def test_countries_downgrade_refuses_records_of_an_unlisted_country() -> None: def test_a_household_only_country_row_lets_records_name_that_country() -> None: """Once a household-only country has a row, its records can be stored.""" - database_url = _disposable_url() - config = _config() + database_url = disposable_v2_url() + config = alembic_v2_config() engine = create_engine(database_url) user_id = uuid4() household_id = uuid4() @@ -1248,341 +1222,3 @@ def test_a_household_only_country_row_lets_records_name_that_country() -> None: transaction.rollback() finally: engine.dispose() - - -CATALOG_FORMAT_2_PARENT_REVISION = "fb1a3e40d3d6" -CATALOG_FORMAT_2_COLUMNS = { - "tax_benefit_model_versions": ("catalog_format", "entities", "basic_inputs"), - "variables": ( - "is_input_variable", - "hidden_input", - "module_name", - "adds_parameter_path", - "subtracts_parameter_path", - "enum_values", - "unit", - "definition_period", - ), - "parameters": ("economy", "household"), -} - - -def _catalog_version_published_before_format_2(connection: sa.Connection) -> UUID: - """Store a model version with one variable and one parameter; return its id.""" - model_id = uuid4() - version_id = uuid4() - connection.execute( - text( - "INSERT INTO tax_benefit_models (id, name) " - "VALUES (:model_id, 'catalog-format-probe')" - ), - {"model_id": model_id}, - ) - connection.execute( - text( - "INSERT INTO tax_benefit_model_versions " - "(id, model_id, version, current_law_id, metadata_time_periods) " - "VALUES (:version_id, :model_id, '0.0.1-probe', 1, '[2026]')" - ), - {"version_id": version_id, "model_id": model_id}, - ) - connection.execute( - text( - "INSERT INTO variables " - "(id, name, entity, default_value, tax_benefit_model_version_id) " - "VALUES (:variable_id, 'probe_income', 'person', '0', :version_id)" - ), - {"variable_id": uuid4(), "version_id": version_id}, - ) - connection.execute( - text( - "INSERT INTO parameters (id, name, tax_benefit_model_version_id) " - "VALUES (:parameter_id, 'gov.probe.rate', :version_id)" - ), - {"parameter_id": uuid4(), "version_id": version_id}, - ) - return version_id - - -def _format_2_values(connection: sa.Connection, version_id: UUID) -> list[tuple]: - """Return the format 2 columns of the version's model, variable and parameter.""" - rows = [] - for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): - key = ( - "id" - if table_name == "tax_benefit_model_versions" - else "tax_benefit_model_version_id" - ) - rows.append( - tuple( - connection.execute( - text( - f"SELECT {', '.join(columns)} FROM {table_name} " - f"WHERE {key} = :version_id" - ), - {"version_id": version_id}, - ).one() - ) - ) - return rows - - -def test_catalog_format_2_columns_leave_earlier_versions_empty() -> None: - """A version stored before catalog format 2 keeps its format 2 fields null. - - Upgrading adds the columns as nullable without writing to stored rows, and - downgrading drops them and keeps every row. - """ - config = _config() - engine = create_engine(_disposable_url()) - version_id = None - try: - command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) - for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): - assert set(columns).isdisjoint(_columns(engine, table_name)) - with engine.begin() as connection: - version_id = _catalog_version_published_before_format_2(connection) - - command.upgrade(config, "head") - - for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): - stored_columns = _columns(engine, table_name) - for column_name in columns: - assert stored_columns[column_name]["nullable"] - assert stored_columns[column_name]["default"] is None - with engine.connect() as connection: - assert _format_2_values(connection, version_id) == [ - (None, None, None), - (None, None, None, None, None, None, None, None), - (None, None), - ] - - command.downgrade(config, CATALOG_FORMAT_2_PARENT_REVISION) - - for table_name, columns in CATALOG_FORMAT_2_COLUMNS.items(): - assert set(columns).isdisjoint(_columns(engine, table_name)) - with engine.connect() as connection: - counts = connection.execute( - text( - "SELECT (SELECT count(*) FROM variables " - "WHERE tax_benefit_model_version_id = :version_id), " - "(SELECT count(*) FROM parameters " - "WHERE tax_benefit_model_version_id = :version_id)" - ), - {"version_id": version_id}, - ).one() - assert tuple(counts) == (1, 1) - finally: - with engine.begin() as connection: - connection.execute( - text( - "DELETE FROM tax_benefit_models WHERE name = 'catalog-format-probe'" - ) - ) - command.upgrade(config, "head") - engine.dispose() - - -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() - - -PARAMETER_SEARCH_FILE_PARENT_REVISION = "3869eb814a90" - - -def test_parameter_search_file_table_is_created_empty_and_dropped_without_loss() -> ( - None -): - """The search file revision creates an empty table and downgrades cleanly. - - The revision writes no rows: catalog publication writes each version's - file. A version holds at most one file, the file is deleted with its - version, and downgrading drops the table while every catalog row stays. - """ - config = _config() - engine = create_engine(_disposable_url()) - try: - command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) - assert "parameter_search_files" 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") - - insert_file = text( - "INSERT INTO parameter_search_files (id, tax_benefit_model_version_id, " - "file_format, compressed_content, content_sha256, uncompressed_size, " - "compressed_size) VALUES (:id, :version_id, 1, :content, :sha, 2, 22)" - ) - file_values = { - "version_id": version_id, - "content": b"\x1f\x8b compressed bytes", - "sha": "0" * 64, - } - with engine.begin() as connection: - assert ( - connection.execute( - text("SELECT count(*) FROM parameter_search_files") - ).scalar_one() - == 0 - ) - connection.execute(insert_file, {"id": uuid4(), **file_values}) - with pytest.raises(sa.exc.IntegrityError), connection.begin_nested(): - connection.execute(insert_file, {"id": uuid4(), **file_values}) - assert ( - connection.execute( - text( - "SELECT compressed_content FROM parameter_search_files " - "WHERE tax_benefit_model_version_id = :version_id" - ), - {"version_id": version_id}, - ).scalar_one() - == file_values["content"] - ) - - command.downgrade(config, PARAMETER_SEARCH_FILE_PARENT_REVISION) - - assert "parameter_search_files" 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 - ) - - command.upgrade(config, "head") - with engine.begin() as connection: - connection.execute(insert_file, {"id": uuid4(), **file_values}) - connection.execute( - text("DELETE FROM tax_benefit_model_versions WHERE id = :version_id"), - {"version_id": version_id}, - ) - assert ( - connection.execute( - text("SELECT count(*) FROM parameter_search_files") - ).scalar_one() - == 0 - ) - 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/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index 3d7e0eb63..39233c598 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -193,6 +193,7 @@ def test_reusable_v2_alembic_check_uses_only_disposable_postgres(): assert "bash .github/scripts/test_alembic_v2_lifecycle.sh" in workflow assert "test_alembic_v2.py" in lifecycle_script assert "test_alembic_v2_lifecycle.py" in lifecycle_script + assert "test_alembic_v2_catalog_tables.py" in lifecycle_script assert "redis:7.2-alpine" not in workflow assert "RUNTIME_CACHE_TEST_URL" not in workflow assert "test_v2_catalog_installed.py" not in workflow