From 016e0b171a6117ed6c8a688d508df7b976e309fd 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/18] 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 7f83b579d..e1668c203 100644 --- a/policyengine_api/data/v2/catalog/parameter_tree.py +++ b/policyengine_api/data/v2/catalog/parameter_tree.py @@ -118,13 +118,27 @@ def _child_label(path: str, stored_label: str | None) -> str: return generated_label(path) -def is_reform_changeable_parameter(parameter: TreeParameterSource) -> bool: - """Return whether a reform can change this parameter, as v3's tree decides.""" - if not parameter.name.startswith(REFORM_TREE_PREFIX): +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 69dfb12ef19758fe211e40584df09523d24f645e 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/18] 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 7cc03c7b4..784ca349c 100644 --- a/policyengine_api/data/v2/models/__init__.py +++ b/policyengine_api/data/v2/models/__init__.py @@ -46,6 +46,7 @@ DatasetVersion, Parameter, ParameterNode, + ParameterSearchFile, ParameterTreeChild, ParameterValue, Region, @@ -125,6 +126,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 2f841f0e2..1eec3e005 100644 --- a/policyengine_api/data/v2/models/metadata.py +++ b/policyengine_api/data/v2/models/metadata.py @@ -534,3 +534,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 02671e912..daa0d922d 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -116,6 +116,8 @@ def _access( "dataset_versions": _access(catalog_writer=READ), "datasets": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), "parameter_nodes": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), + # Written with each catalog version, 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 c71100c40..18bace37c 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -1660,3 +1660,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 4bdc47718..760b04caa 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -44,6 +44,7 @@ "legacy_policy_mappings", "legacy_user_mappings", "legacy_user_policy_mappings", + "parameter_search_files", "parameter_tree_children", "stage12_evaluation_reports", "stage12_evaluation_simulations", @@ -68,6 +69,7 @@ ) SAVED_POLICY_VIEW_REMOVAL = "DROP VIEW user_policies" 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}" @@ -249,6 +251,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, SAVED_POLICY_RENAME_REVISION, SAVED_POLICY_RENAME_PARENT_REVISION, @@ -880,7 +883,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()") @@ -907,3 +910,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 af6913897..2947f2cf3 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) == 40 +assert len(V2_METADATA.tables) == 41 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 464a76d28..544562401 100644 --- a/tests/unit/v2/test_models.py +++ b/tests/unit/v2/test_models.py @@ -843,3 +843,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 db9054597c26c62a38677efd818d5691d70c0d27 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/18] 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 | 36 +++ .../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 | 238 ++++++++++++++++++ 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 ae3ca794e..66bcf1b67 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -354,6 +354,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: @@ -366,6 +400,8 @@ Successful JSON evidence contains: kept; - 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 5bbec59f1..4baa53531 100644 --- a/policyengine_api/data/v2/catalog/publication.py +++ b/policyengine_api/data/v2/catalog/publication.py @@ -10,6 +10,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, @@ -125,11 +129,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: @@ -147,6 +152,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) @@ -158,6 +166,7 @@ def publish_catalog( ) 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) @@ -167,6 +176,7 @@ def publish_catalog( return replace( _evidence(catalog, kept_region_summaries, started=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 d547d0fa4..94be2fa87 100644 --- a/policyengine_api/data/v2/catalog/publication_types.py +++ b/policyengine_api/data/v2/catalog/publication_types.py @@ -26,6 +26,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.""" @@ -63,5 +66,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 60077f905..b25e40899 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -2,10 +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 @@ -1062,3 +1067,236 @@ 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, + region_references=region_references(), + ) + + 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 377c8c230..1b761f10d 100644 --- a/tests/unit/v2/test_catalog_publication.py +++ b/tests/unit/v2/test_catalog_publication.py @@ -293,6 +293,7 @@ def test_completion_evidence_contains_only_reviewed_non_secret_fields() -> None: omitted_region_summaries=(("us", "place", 333),), kept_region_summaries=(("uk", "6.7.5", 5, 1_015),), parameter_trees_written=(("policyengine-uk", "6.7.5", 2_430),), + parameter_search_files_written=(("policyengine-uk", "6.7.5", 412_000, 61_000),), ).as_dict() assert evidence == { @@ -321,6 +322,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 b40b572c383f579e748d0cd7e2d6a194f04b1aa7 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/18] 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/dependencies.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_routes.py | 317 +++++++++++++++++- tests/unit/v2/test_metadata_service.py | 57 ++++ tests/unit/v2/test_parameter_search_file.py | 25 ++ 19 files changed, 835 insertions(+), 9 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 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 411656a62..987cccf7d 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 | 92 | +| request count | 93 | | db entity count | 6 | | sim flow count | 3 | @@ -213,6 +213,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 9232097e8..03fdad522 100644 --- a/docs/generated/migration_contracts.json +++ b/docs/generated/migration_contracts.json @@ -1,7 +1,7 @@ { "metadata": { "db_entity_count": 6, - "request_count": 92, + "request_count": 93, "route_group_count": 9, "sim_flow_count": 3, "workflow_count": 16 @@ -1586,6 +1586,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 66bcf1b67..125b9c0a5 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -388,6 +388,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/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index f3a343827..c015ab2c3 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -51,6 +51,7 @@ MetadataModelVersionDetail, MetadataPageResult, MetadataParameterChild, + MetadataParameterSearchFile, MetadataParameterSummary, MetadataRegion, MetadataVariable, @@ -235,6 +236,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 ce362d58f..c0b03a818 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 b84980a1c..7b2a8aec4 100644 --- a/tests/contract/registry.py +++ b/tests/contract/registry.py @@ -643,6 +643,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 d3a248cec..8f4c1c860 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, @@ -1049,3 +1054,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 e35785e12..2b860ff3e 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": 92, + "request_count": 93, "db_entity_count": 6, "sim_flow_count": 3, } diff --git a/tests/unit/v2/test_metadata_routes.py b/tests/unit/v2/test_metadata_routes.py index e2b7ff1ff..bd7c25dee 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -2,7 +2,8 @@ from __future__ import annotations -from datetime import UTC, datetime +import gzip +from datetime import UTC, date, datetime from unittest.mock import patch from uuid import uuid4 @@ -17,12 +18,22 @@ MetadataCatalogVersionNotFoundError, UnsupportedPreviewCountryError, ) +from policyengine_api.data.v2.catalog.parameter_search_file import ( + DraftabilityChange, + SearchNodeSource, + SearchParameterSource, + build_parameter_search_file, + encode_parameter_search_file, +) from policyengine_api.data.v2.settings import V2ConfigurationError from policyengine_api.fastapi_routes import dependencies as route_dependencies from policyengine_api.fastapi_routes.dependencies import NativeRouteDependencies from policyengine_api.fastapi_routes.v2.metadata.response_cache import ( MetadataResponseCache, ) +from policyengine_api.fastapi_routes.v2.metadata.response_models import ( + MetadataParameterSearchFileContent, +) from policyengine_api.migration_flags import ( RouteImplementation, RouteImplementationSettings, @@ -48,6 +59,7 @@ MetadataModelVersionDetail, MetadataPageResult, MetadataParameterChild, + MetadataParameterSearchFile, MetadataParameterSummary, MetadataRegion, MetadataRegionOption, @@ -59,6 +71,7 @@ from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterSearchFileUnavailableError, ParameterTreeUnavailableError, ) @@ -620,6 +633,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}", @@ -1266,7 +1280,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 @@ -1384,10 +1398,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"] @@ -1655,3 +1672,295 @@ def test_a_missing_parameter_tree_is_not_cached() -> None: assert "etag" not in response.headers assert "cache-control" not in response.headers assert "origin" not in _vary_values(response) + + +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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 4ff27c3ab..53c8229f0 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -18,6 +18,10 @@ MetadataCatalogVersionNotFoundError, UnsupportedPreviewCountryError, ) +from policyengine_api.data.v2.catalog.parameter_search_file import ( + country_search_file, + encode_parameter_search_file, +) from policyengine_api.data.v2.catalog.parameter_tree import ( TreeNodeSource, TreeParameterSource, @@ -28,6 +32,7 @@ Dataset, Parameter, ParameterNode, + ParameterSearchFile, ParameterTreeChild, ParameterValue, Region, @@ -57,6 +62,7 @@ from policyengine_api.services.v2.metadata.validators import ( InvalidMetadataPageError, MetadataResourceNotFoundError, + ParameterSearchFileUnavailableError, ParameterTreeUnavailableError, ) from tests.fixtures.v2_catalog import POLICYENGINE_VERSION, normalized_catalog @@ -1621,3 +1627,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 = _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 = _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 = _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" + ): + _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 7eb8f50800e9a7dafcb7cfae86302a0f66d5051a 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/18] 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 125b9c0a5..eb57da2c5 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -370,8 +370,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 6f9ae2ea6cb0f92460ddb30f835ba84a729f653f 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/18] 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 ++++++++++++------ tests/unit/v2/test_metadata_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_routes.py b/tests/unit/v2/test_metadata_routes.py index bd7c25dee..37d4e8311 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -4,6 +4,7 @@ import gzip from datetime import UTC, date, datetime +from typing import get_type_hints from unittest.mock import patch from uuid import uuid4 @@ -19,12 +20,17 @@ UnsupportedPreviewCountryError, ) 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.data.v2.settings import V2ConfigurationError from policyengine_api.fastapi_routes import dependencies as route_dependencies from policyengine_api.fastapi_routes.dependencies import NativeRouteDependencies @@ -33,6 +39,10 @@ ) from policyengine_api.fastapi_routes.v2.metadata.response_models import ( MetadataParameterSearchFileContent, + MetadataParameterSearchFileEditabilityChange, + MetadataParameterSearchFileLabel, + MetadataParameterSearchFileNode, + MetadataParameterSearchFileParameter, ) from policyengine_api.migration_flags import ( RouteImplementation, @@ -1675,7 +1685,7 @@ def test_a_missing_parameter_tree_is_not_cached() -> None: SEARCH_FILE_PATH = "/v2/parameters/search-file" -SEARCH_FILE_CONTENT = { +SEARCH_FILE_CONTENT: ParameterSearchFileDocument = { "format": 1, "country_id": "us", "policyengine_version": "4.20.3", @@ -1949,13 +1959,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 ), ], ) @@ -1964,3 +1974,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 32566f3164328505698df81ca527f628fe3b700e 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/18] 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 daa0d922d..4d1a81c7f 100644 --- a/policyengine_api/data/v2/privileges.py +++ b/policyengine_api/data/v2/privileges.py @@ -116,7 +116,8 @@ def _access( "dataset_versions": _access(catalog_writer=READ), "datasets": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), "parameter_nodes": _access(runtime=READ, catalog_writer=READ_INSERT_UPDATE), - # Written with each catalog version, 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 5a1bb4d0a428b846c9524b9ccfc0fc4fd554abb4 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/18] 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 ++++++++++ tests/unit/v2/test_metadata_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 8f74b1bb2..c544df5cb 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, @@ -457,7 +457,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 3f221e35c..0d3f85a12 100644 --- a/tests/unit/test_asgi_factory.py +++ b/tests/unit/test_asgi_factory.py @@ -569,6 +569,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_routes.py b/tests/unit/v2/test_metadata_routes.py index 37d4e8311..623c7c9c7 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -1988,3 +1988,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 = _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 32939ade88cb91ea112d1eda7931324c4e69a4b8 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/18] 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 +++++- tests/unit/v2/test_metadata_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 8f4c1c860..c2aed83e8 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1097,7 +1097,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_routes.py b/tests/unit/v2/test_metadata_routes.py index 623c7c9c7..9e4e9413c 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -1875,19 +1875,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 = _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 @@ -1934,6 +1934,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 53c8229f0..96b1ed17b 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -1674,7 +1674,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: _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 4693c89edfd0787822a0145948ec088ed553437e 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/18] 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 ++++ tests/unit/v2/test_metadata_routes.py | 33 ++++++++++ 3 files changed, 110 insertions(+) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index b25e40899..8da9655f4 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -1300,3 +1300,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 c2aed83e8..a76829242 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1124,3 +1124,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_routes.py b/tests/unit/v2/test_metadata_routes.py index 9e4e9413c..3a204406a 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -2031,3 +2031,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 = _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 = _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 bd986d6af6e335faee479a77cc448c17dcafddf2 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/18] 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 76767f76eab2dbaee38167d6b58352d7debbf42a 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/18] 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 eb57da2c5..0979dfa92 100644 --- a/docs/migration/stage-9-v2-metadata.md +++ b/docs/migration/stage-9-v2-metadata.md @@ -381,7 +381,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 8da9655f4..ac6698fbe 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -1365,3 +1365,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 a5dd07f007e9c679246e9d29f617834caad00feb 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/18] 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/dependencies.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 ++- tests/unit/v2/test_metadata_routes.py | 58 ++++++++++++++++++- tests/unit/v2/test_metadata_service.py | 6 ++ 9 files changed, 194 insertions(+), 27 deletions(-) diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index c015ab2c3..af6bf560f 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -247,6 +247,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 a76829242..6c5b4efb6 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1114,16 +1114,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_routes.py b/tests/unit/v2/test_metadata_routes.py index 3a204406a..8c0c4a4a5 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -1713,7 +1713,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: @@ -2064,3 +2070,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 = _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 = _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 = _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 96b1ed17b..6dd3fea2f 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -1649,7 +1649,9 @@ def test_the_selected_version_search_file_is_returned_as_stored( catalog_session.commit() search_file = _service(catalog_session).get_parameter_search_file("us") + content_sha256 = _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 @@ -1676,6 +1678,10 @@ def test_a_version_without_a_search_file_of_the_current_format_is_unavailable( with pytest.raises(ParameterSearchFileUnavailableError) as raised: _service(catalog_session).get_parameter_search_file("us") + with pytest.raises(ParameterSearchFileUnavailableError) as raised_for_hash: + _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 c8dd1de00af4f72cb18d3aeb71d1e014a6b0d275 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/18] 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 ++- tests/unit/v2/test_metadata_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_routes.py b/tests/unit/v2/test_metadata_routes.py index 8c0c4a4a5..67c08e045 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -4,13 +4,15 @@ import gzip from datetime import UTC, date, datetime -from typing import get_type_hints +from types import UnionType +from typing import Union, get_args, get_origin, get_type_hints from unittest.mock import patch from uuid import uuid4 import pytest from fastapi.testclient import TestClient from flask import Flask, jsonify +from pydantic import BaseModel from policyengine_api.asgi_factory import create_asgi_app from policyengine_api.data.v2.catalog.catalog_selection import ( @@ -1983,60 +1985,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) - ], +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, } - 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 = _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) + 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 1d13cd0df64afce768450c6b3eb052dc65ba4ede 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/18] 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 a974a5d5d2beb93e0b2b870c0b3db45ab4807fb7 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/18] 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_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 6c5b4efb6..2c18dcdd2 100644 --- a/tests/integration/test_v2_metadata_routes.py +++ b/tests/integration/test_v2_metadata_routes.py @@ -1099,8 +1099,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_routes.py b/tests/unit/v2/test_metadata_routes.py index 67c08e045..0c4617360 100644 --- a/tests/unit/v2/test_metadata_routes.py +++ b/tests/unit/v2/test_metadata_routes.py @@ -1884,9 +1884,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 6dd3fea2f..452953a62 100644 --- a/tests/unit/v2/test_metadata_service.py +++ b/tests/unit/v2/test_metadata_service.py @@ -1684,7 +1684,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 97393e3de9394bf2e494203dc428816da7033fc4 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/18] 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 e2507a1d2..cd9ffd9f1 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -294,7 +294,7 @@ flowchart LR | `api.quality.command-line-modules-ruff-exemptions` | Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/privilege_reconciliation,data/v2/user_association_migration_qualification,worker_spm_release}.py` | `pr.yml:lint` | — | | `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.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries 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, and the user sign-in and account-deletion libraries 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 9285d2383..c92d2f24b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -159,6 +159,7 @@ files = [ "policyengine_api/fastapi_routes/dependencies.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", ] diff --git a/rules/registry.toml b/rules/registry.toml index 2f24327dc..78fe2f33b 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -918,9 +918,9 @@ invocation = "ruff check ." [[rule]] id = "api.quality.mypy-v2" category = "quality" -statement = "Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed." +statement = "Shared query models, FastAPI dependencies, v2 routes and services, the gzip negotiation, and the user sign-in and account-deletion libraries 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"] +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"] tier = "mechanical" status = "enforced" autofix = false @@ -932,6 +932,7 @@ paths = [ "policyengine_api/fastapi_routes/dependencies.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", ] From 0e6bd7f864a4a6ba254aef5ffa368b1556e06a52 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/18] 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"