Round-trip policy metadata, and read strategies fields by catalog alias

phileas/policy/policy.py

@@ -75,8 +75,12 @@ def __init__(
         ignored_patterns: List[str] | None = None,
         generators: dict | None = None,
         config: dict | None = None,
+        metadata: dict | None = None,
     ) -> None:
         self.name = name
+        #: Raw ``metadata`` object, kept as-is so keys the engine does not know
+        #: about survive a load-and-save round trip. Never read during filtering.
+        self.metadata: dict | None = metadata
         #: Raw Phileas-JSON ``identifiers`` object (entity field -> filter node).
         self.identifiers: dict = identifiers if identifiers is not None else {}
         #: Flat list of policy-level ignored terms.
@@ -99,6 +103,7 @@ def from_dict(cls, data: dict) -> "Policy":
             ignored_patterns=_parse_ignored_patterns(data.get("ignoredPatterns", [])),
             generators=data.get("generators", {}) or {},
             config=data.get("config", {}) or {},
+            metadata=data.get("metadata"),
         )
 
     def _analysis_flag(self, name: str) -> bool:
@@ -129,14 +134,17 @@ def from_yaml(cls, yaml_str: str) -> "Policy":
         return cls.from_dict(yaml.safe_load(yaml_str))
 
     def to_dict(self) -> dict:
-        return {
+        data = {
             "name": self.name,
             "identifiers": self.identifiers,
             "ignored": [{"terms": list(self.ignored)}] if self.ignored else [],
             "ignoredPatterns": [{"pattern": p} for p in self.ignored_patterns],
             **({"generators": self.generators} if self.generators else {}),
             **({"config": self.config} if self.config else {}),
         }
+        if self.metadata is not None:
+            data["metadata"] = self.metadata
+        return data
 
     def to_json(self) -> str:
         return json.dumps(self.to_dict(), indent=2)

phileas/services/filter_service.py

@@ -95,6 +95,20 @@
 ]
 
 
+def _strategies_node(node: dict, entity) -> list:
+    """Reads a filter's strategies array by its catalog field name, falling back to
+    the catalog's aliases. An alias is a name the field had in an earlier schema, so
+    a policy written against that schema keeps working."""
+    strategies = node.get(entity.phileas_strategies_field)
+    if strategies is not None:
+        return strategies
+    for alias in getattr(entity, "phileas_strategies_field_aliases", ()) or ():
+        strategies = node.get(alias)
+        if strategies is not None:
+            return strategies
+    return []
+
+
 class FilterService:
     def __init__(self, context_service: AbstractContextService | None = None) -> None:
         self._context_service = (
@@ -115,9 +129,7 @@ def filter(self, policy: Policy, context: str, document_id: str, text: str) -> F
             node = identifiers.get(entity.phileas_field)
             if not isinstance(node, dict) or node.get("enabled", True) is False:
                 continue
-            strategies = self._strategies(
-                node.get(entity.phileas_strategies_field, []), policy
-            )
+            strategies = self._strategies(_strategies_node(node, entity), policy)
             detected = filter_cls(node).detect(text, context)
             spans.extend(self._apply_strategies(detected, strategies, node, context))
 

tests/test_catalog.py

@@ -17,6 +17,12 @@
 from phileas.catalog import PhileasCatalog, get_catalog
 
 
+def _readable_strategies_fields(entity) -> set:
+    """Every strategies-array name an engine must read for an entity: the catalog's
+    primary name plus any aliases it records for earlier names."""
+    return {entity.phileas_strategies_field, *getattr(entity, "phileas_strategies_field_aliases", ())}
+
+
 class TestCatalogBridge:
     def test_singleton(self):
         assert get_catalog() is get_catalog()
@@ -38,8 +44,9 @@ def test_entity_lookup_case_insensitive(self):
     def test_quirky_field_names_come_from_catalog(self):
         """The engine must use the catalog's non-obvious field names."""
         cat = get_catalog()
-        # zip code uses a *singular* strategies field name.
-        assert cat.get_entity("ZIP_CODE").phileas_strategies_field == "zipCodeFilterStrategy"
+        # The zip code strategies field was renamed to the plural; the singular has to stay
+        # readable either as the primary name or as an alias, depending on the catalog version.
+        assert "zipCodeFilterStrategy" in _readable_strategies_fields(cat.get_entity("ZIP_CODE"))
         # bitcoin uses an abbreviated strategies field name.
         assert cat.get_entity("BITCOIN_ADDRESS").phileas_strategies_field == "bitcoinFilterStrategies"
 

tests/test_catalog_extra.py

@@ -92,10 +92,14 @@ def test_entity_has_strategies_field(self, name):
     def test_entity_name_matches_lookup(self, name):
         assert get_catalog().get_entity(name).name == name
 
-    def test_zip_code_quirk_singular_strategies_field(self):
-        # ZIP_CODE uses a *singular* strategies field name.
+    def test_zip_code_singular_strategies_field_stays_readable(self):
+        # ZIP_CODE used a singular strategies field name until schema 1.3.0 renamed it to
+        # the plural. Whichever the catalog calls primary, the singular must stay readable
+        # so a policy written against an earlier schema keeps working.
         entity = get_catalog().get_entity("ZIP_CODE")
-        assert entity.phileas_strategies_field == "zipCodeFilterStrategy"
+        readable = {entity.phileas_strategies_field,
+                    *getattr(entity, "phileas_strategies_field_aliases", ())}
+        assert "zipCodeFilterStrategy" in readable
 
     def test_bitcoin_quirk_abbreviated_strategies_field(self):
         # BITCOIN_ADDRESS uses an abbreviated strategies field name.

tests/test_integration_phisql.py

@@ -22,6 +22,7 @@
 
 from phileas.policy.policy import Policy
 from phileas.services.filter_service import FilterService
+from phileas.catalog import get_catalog
 
 
 def compile_and_run(source, text, context="ctx"):
@@ -83,11 +84,13 @@ def test_static_replace(self):
         out = compile_and_run(src, "Mail a@b.com here.").filtered_text
         assert "[EMAIL]" in out
 
-    def test_zip_code_quirky_field_round_trips(self):
-        # PhiSQL emits the singular zipCodeFilterStrategy; phileas must read it.
+    def test_zip_code_field_round_trips(self):
+        # PhiSQL emits the catalog's primary strategies field name, whichever it is;
+        # phileas must read what PhiSQL emits.
         src = "POLICY p; REDACT ZIP_CODE WITH REDACT;"
         policy_json = Compiler().compile(src).policy_json()
-        assert "zipCodeFilterStrategy" in policy_json["identifiers"]["zipCode"]
+        primary = get_catalog().get_entity("ZIP_CODE").phileas_strategies_field
+        assert primary in policy_json["identifiers"]["zipCode"]
         out = compile_and_run(src, "ZIP 90210 here.").filtered_text
         assert "90210" not in out
 

tests/test_policy.py

@@ -99,3 +99,19 @@ def test_to_dict_reemits_ignored_objects(self):
     def test_yaml_round_trip_ignored_patterns(self):
         p = Policy.from_dict({"identifiers": {}, "ignoredPatterns": [{"pattern": r"\d+"}]})
         assert Policy.from_yaml(p.to_yaml()).ignored_patterns == [r"\d+"]
+
+    def test_metadata_round_trips(self):
+        # Keys beyond description are allowed by the schema, so they have to survive too.
+        metadata = {
+            "description": "Client intake forms.",
+            "author": "records team",
+            "labels": ["intake", "pii"],
+        }
+        p = Policy.from_dict({"metadata": metadata, "identifiers": {}})
+        assert p.metadata == metadata
+        assert Policy.from_json(p.to_json()).metadata == metadata
+
+    def test_policy_without_metadata_omits_it(self):
+        p = Policy.from_dict({"identifiers": {}})
+        assert p.metadata is None
+        assert "metadata" not in p.to_dict()

tests/test_service.py

@@ -14,6 +14,7 @@
 
 """End-to-end tests for the catalog-driven FilterService."""
 
+from phileas.catalog import get_catalog
 from phileas.policy.policy import Policy
 from phileas.services.context.in_memory_context_service import InMemoryContextService
 from phileas.services.filter_service import FilterService
@@ -49,8 +50,21 @@ def test_disabled_node_skipped(self):
 
 
 class TestCatalogDrivenFieldNames:
-    def test_zip_code_singular_strategy_field(self):
-        # The catalog says the field is `zipCodeFilterStrategy` (singular).
+    def test_zip_code_strategy_field(self):
+        # Every name the catalog says is readable must actually drive the filter: the
+        # primary name, and any alias recording what the field was called earlier.
+        entity = get_catalog().get_entity("ZIP_CODE")
+        names = {entity.phileas_strategies_field,
+                 *getattr(entity, "phileas_strategies_field_aliases", ())}
+        for name in names:
+            r = run({"zipCode": {name: [{"strategy": "STATIC_REPLACE", "staticReplacement": "ZZZZZ"}]}},
+                    "ZIP 90210 here.")
+            assert "ZZZZZ" in r.filtered_text, name
+            assert "90210" not in r.filtered_text, name
+
+    def test_zip_code_singular_strategy_field_still_read(self):
+        # The field was singular until schema 1.3.0. A policy written against an
+        # earlier schema must keep redacting rather than silently losing its strategies.
         r = run({"zipCode": {"zipCodeFilterStrategy": [{"strategy": "STATIC_REPLACE", "staticReplacement": "ZZZZZ"}]}},
                 "ZIP 90210 here.")
         assert "ZZZZZ" in r.filtered_text