refactor: split the author-time guard subsystem out of feature_chain_parser

docs/docs/in_depth/property-mapping.md

@@ -558,6 +558,9 @@ The matcher must be a `classmethod`; a `staticmethod` matcher on a class that de
 The guard is installed at class definition, so mutating `PROPERTY_MAPPING` or replacing
 `match_feature_group_criteria` after the class body escapes it.
 
+This guard, the name-path presence guard, and the class-definition diagnostics below live in
+`feature_chain_author_guards.py`, which imports `feature_chain_parser` and never the reverse.
+
 ## Guarding against a universal configuration matcher
 
 A key is unconditionally required only when it declares no `default` and no `required_when`. A

mloda/core/abstract_plugins/components/feature_chainer/feature_chain_parser_mixin.py

@@ -61,6 +61,13 @@
 from mloda.core.abstract_plugins.components.feature import Feature
 from mloda.core.abstract_plugins.components.feature_name import FeatureName
 from mloda.core.abstract_plugins.components.options import Options
+from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_author_guards import (
+    install_name_path_presence_guard,
+    install_required_when_guard,
+    validate_name_binding,
+    warn_captureless_without_binding,
+    warn_universal_optional_matcher,
+)
 from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser import (
     FeatureChainParser,
     CHAIN_SEPARATOR,
@@ -100,7 +107,7 @@ class FeatureChainParserMixin:
     True and the option value is absent, ``match_feature_group_criteria`` rejects the match.
     When the predicate returns False, the option is treated as optional. The predicates are
     enforced by a guard installed on the class at definition time (see
-    ``FeatureChainParser.install_required_when_guard``), so overriding
+    ``feature_chain_author_guards.install_required_when_guard``), so overriding
     ``match_feature_group_criteria`` keeps the contract.
 
     This works for both string-based and configuration-based feature creation. For
@@ -132,11 +139,11 @@ def __init_subclass__(cls, **kwargs: Any) -> None:
         # The mixin sits first in the MRO of ``class X(FeatureChainParserMixin, FeatureGroup)``,
         # so super() is what lets FeatureGroup's own class-definition validation still run.
         super().__init_subclass__(**kwargs)
-        FeatureChainParser.validate_name_binding(cls)
-        FeatureChainParser.warn_captureless_without_binding(cls)
-        FeatureChainParser.install_name_path_presence_guard(cls)
-        FeatureChainParser.install_required_when_guard(cls)
-        FeatureChainParser.warn_universal_optional_matcher(cls)
+        validate_name_binding(cls)
+        warn_captureless_without_binding(cls)
+        install_name_path_presence_guard(cls)
+        install_required_when_guard(cls)
+        warn_universal_optional_matcher(cls)
 
     @classmethod
     def _validate_string_match(cls, _feature_name: str, _operation_config: str, _in_feature: str) -> bool:

mloda/core/abstract_plugins/feature_group.py

@@ -12,6 +12,12 @@
 
 from mloda.core.abstract_plugins.components.domain import Domain
 from mloda.core.abstract_plugins.components.base_feature_group_version import BaseFeatureGroupVersion
+from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_author_guards import (
+    install_name_path_presence_guard,
+    install_required_when_guard,
+    validate_name_binding,
+    warn_captureless_without_binding,
+)
 from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser import (
     CHAIN_SEPARATOR,
     COLUMN_SEPARATOR,
@@ -92,10 +98,10 @@ def calculate_feature(cls, data, features):
     def __init_subclass__(cls, **kwargs: Any) -> None:
         super().__init_subclass__(**kwargs)
         FeatureChainParser.validate_property_mapping_defaults(cls.__name__, cls.PROPERTY_MAPPING)
-        FeatureChainParser.validate_name_binding(cls)
-        FeatureChainParser.warn_captureless_without_binding(cls)
-        FeatureChainParser.install_name_path_presence_guard(cls)
-        FeatureChainParser.install_required_when_guard(cls)
+        validate_name_binding(cls)
+        warn_captureless_without_binding(cls)
+        install_name_path_presence_guard(cls)
+        install_required_when_guard(cls)
         cls._validate_subtype_declaration()
 
     @classmethod

tests/test_core/test_abstract_plugins/test_components/feature_chainer/test_author_guards_module_split.py

@@ -0,0 +1,210 @@
+"""Pins the split of the author-time guard subsystem out of feature_chain_parser.
+
+Ownership of the moved names, the runtime match path staying put, the acyclic direction
+feature_chain_parser <- feature_chain_author_guards, and the absence of re-exports back
+onto FeatureChainParser.
+"""
+
+from __future__ import annotations
+
+import ast
+import importlib
+import inspect
+import subprocess  # nosec B404
+import sys
+from collections.abc import Sequence
+from pathlib import Path
+from types import ModuleType
+
+from mloda.core.abstract_plugins.components.feature_chainer import feature_chain_parser
+from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser import FeatureChainParser
+
+PARSER_MODULE = "mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser"
+GUARDS_MODULE = "mloda.core.abstract_plugins.components.feature_chainer.feature_chain_author_guards"
+
+# Author-time constants that move with the guards.
+GUARDS_CONSTANTS = (
+    "REQUIRED_WHEN_GUARD_FLAG",
+    "NAME_PATH_PRESENCE_GUARD_FLAG",
+    "CAPTURELESS_DIAGNOSTIC_FLAG",
+    "_UNIVERSAL_MATCHER_PROBE_NAME",
+    "REQUIRED_WHEN_GUARD_DEPTH",
+    "NAME_PATH_PRESENCE_GUARD_DEPTH",
+)
+
+# Class-definition-time validation and guard installation, as module-level functions.
+GUARDS_FUNCTIONS = (
+    "validate_name_binding",
+    "warn_captureless_without_binding",
+    "warn_universal_optional_matcher",
+    "check_required_when",
+    "install_required_when_guard",
+    "install_name_path_presence_guard",
+    "_matcher_is_staticmethod",
+    "_reject_staticmethod_matcher",
+    "_resolve_match_arguments",
+    "_pattern_named_and_total_groups",
+    "_flatten_patterns",
+    "_str_reachable_values",
+)
+
+# The public moved names, which must NOT come back as FeatureChainParser attributes.
+NO_REEXPORT_NAMES = (
+    "validate_name_binding",
+    "warn_captureless_without_binding",
+    "warn_universal_optional_matcher",
+    "install_required_when_guard",
+    "install_name_path_presence_guard",
+    "check_required_when",
+)
+
+# The runtime match path, which stays on FeatureChainParser, including the members the guards call back into.
+PARSER_KEPT_METHODS = (
+    "parse_name",
+    "parse_feature_name",
+    "match_configuration_feature_chain_parser",
+    "build_effective_options",
+    "bind_name_captures",
+    "prefix_patterns_of",
+    "has_required_when_predicates",
+    "_name_identifies_group",
+    "_merge_bindings",
+    "_check_name_path_required_presence",
+    "_can_skip_required_check",
+    "_name_path_missing_required_keys",
+    "extract_property_values",
+    "name_path_presence_rejection_reason",
+    "extract_in_feature",
+    "validate_property_mapping_defaults",
+)
+
+# Module-level names of the parser that carry a __module__ to check.
+PARSER_KEPT_MODULE_LEVEL = (
+    "record_match_rejection",
+    "option_key_is_present",
+    "_contained_raise_log_level",
+    "PropertyValueRejection",
+)
+
+# Module-level parser constants, which have no __module__ to check.
+PARSER_KEPT_CONSTANTS = (
+    "MATCH_REJECTION_REASONS",
+    "CHAIN_SEPARATOR",
+    "COLUMN_SEPARATOR",
+    "INPUT_SEPARATOR",
+)
+
+_SUBPROCESS_TIMEOUT = 8.0
+
+# Import the parser first, record whether the guards came along, then prove the guards module exists.
+_CLEAN_IMPORT_SNIPPET = (
+    "import importlib\n"
+    "import sys\n"
+    f"importlib.import_module({PARSER_MODULE!r})\n"
+    f"pulled_in = {GUARDS_MODULE!r} in sys.modules\n"
+    f"importlib.import_module({GUARDS_MODULE!r})\n"
+    "print('present' if pulled_in else 'absent')\n"
+)
+
+
+def _chainer_dir() -> Path:
+    """Directory of the feature_chainer package, read off the parser module that already exists."""
+    parser_file = feature_chain_parser.__file__
+    assert parser_file is not None
+    return Path(parser_file).parent
+
+
+def _repo_root() -> Path:
+    """Repo root, four levels above mloda/core/abstract_plugins/components/feature_chainer."""
+    return _chainer_dir().parents[4]
+
+
+def _missing_names(module: ModuleType, names: Sequence[str]) -> list[str]:
+    return [name for name in names if not hasattr(module, name)]
+
+
+def _imported_modules(path: Path) -> set[str]:
+    """Module names every import statement in one file references.
+
+    Alias names are included so the `from . import sibling` form is covered too.
+    """
+    tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
+    modules: set[str] = set()
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Import):
+            modules.update(alias.name for alias in node.names)
+        elif isinstance(node, ast.ImportFrom):
+            if node.module is not None:
+                modules.add(node.module)
+            modules.update(alias.name for alias in node.names)
+    return modules
+
+
+def _references(modules: set[str], target: str) -> list[str]:
+    return sorted(module for module in modules if module == target or module.endswith(f".{target}"))
+
+
+def test_author_guards_module_defines_the_moved_constants() -> None:
+    guards = importlib.import_module(GUARDS_MODULE)
+    missing = _missing_names(guards, GUARDS_CONSTANTS)
+    assert missing == [], f"{GUARDS_MODULE} does not define {missing}"
+
+
+def test_author_guards_module_defines_the_moved_functions() -> None:
+    guards = importlib.import_module(GUARDS_MODULE)
+    missing = _missing_names(guards, GUARDS_FUNCTIONS)
+    assert missing == [], f"{GUARDS_MODULE} does not define {missing}"
+
+
+def test_moved_functions_are_module_level_functions() -> None:
+    """The moved guards are plain functions taking the owner class, not classmethods of some new class."""
+    guards = importlib.import_module(GUARDS_MODULE)
+    for name in GUARDS_FUNCTIONS:
+        member = getattr(guards, name)
+        assert inspect.isfunction(member), f"{GUARDS_MODULE}.{name} is {type(member)!r}, not a module-level function"
+        assert member.__module__ == GUARDS_MODULE, f"{name} is defined in {member.__module__}, not {GUARDS_MODULE}"
+
+
+def test_moved_names_are_not_reexported_on_the_parser_class() -> None:
+    """No compatibility shim: a re-export would let call sites keep the old spelling and undo the split."""
+    reexported = [name for name in NO_REEXPORT_NAMES if hasattr(FeatureChainParser, name)]
+    assert reexported == [], f"FeatureChainParser still exposes {reexported}, so the split is only cosmetic"
+
+
+def test_runtime_match_path_stays_on_the_parser_class() -> None:
+    for name in PARSER_KEPT_METHODS:
+        assert hasattr(FeatureChainParser, name), f"FeatureChainParser no longer defines {name}"
+        owner = getattr(FeatureChainParser, name).__module__
+        assert owner == PARSER_MODULE, f"{name} moved out of {PARSER_MODULE} to {owner}"
+
+
+def test_parser_module_level_names_stay_defined_in_the_parser() -> None:
+    for name in PARSER_KEPT_MODULE_LEVEL:
+        assert hasattr(feature_chain_parser, name), f"{PARSER_MODULE} no longer defines {name}"
+        owner = getattr(feature_chain_parser, name).__module__
+        assert owner == PARSER_MODULE, f"{name} moved out of {PARSER_MODULE} to {owner}"
+    missing = _missing_names(feature_chain_parser, PARSER_KEPT_CONSTANTS)
+    assert missing == [], f"{PARSER_MODULE} no longer defines {missing}"
+
+
+def test_parser_does_not_import_the_author_guards() -> None:
+    """The guards import the parser, never the reverse; that one direction is what keeps the split acyclic."""
+    guards_path = _chainer_dir() / "feature_chain_author_guards.py"
+    assert guards_path.is_file(), f"{guards_path} does not exist"
+    parser_path = _chainer_dir() / "feature_chain_parser.py"
+    offenders = _references(_imported_modules(parser_path), "feature_chain_author_guards")
+    assert offenders == [], f"{parser_path.name} imports {offenders}, which closes the cycle"
+
+
+def test_parser_imports_in_a_clean_interpreter_without_the_author_guards() -> None:
+    """Runtime counterpart of the static check: a fresh interpreter must not pull the guards in transitively."""
+    # Safe: fixed argv (sys.executable plus a literal snippet), no shell, no user input.
+    result = subprocess.run(  # nosec B603
+        [sys.executable, "-c", _CLEAN_IMPORT_SNIPPET],
+        capture_output=True,
+        text=True,
+        timeout=_SUBPROCESS_TIMEOUT,
+        cwd=_repo_root(),
+    )
+    assert result.returncode == 0, f"importing {PARSER_MODULE} in a fresh interpreter failed:\n{result.stderr}"
+    assert result.stdout.strip() == "absent", f"{GUARDS_MODULE} was imported transitively by {PARSER_MODULE}"

tests/test_core/test_abstract_plugins/test_components/feature_chainer/test_captureless_no_binding.py

@@ -30,7 +30,8 @@
 
 CLEANED_TEXT_NAME = "x__cleaned_text"
 CLEANED_TEXT_PATTERN = r".*__cleaned_text$"
-FEATURE_CHAIN_PARSER_LOGGER = "mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser"
+# The captureless diagnostic is emitted by feature_chain_author_guards.warn_captureless_without_binding.
+AUTHOR_GUARDS_LOGGER = "mloda.core.abstract_plugins.components.feature_chainer.feature_chain_author_guards"
 
 # "cleaned" is exactly the token the retired code fabricated from ``__cleaned_text``.
 FABRICATED_TOKEN = "cleaned"  # nosec B105
@@ -162,7 +163,7 @@ class _CapturelessNoMarkerC772(FeatureChainParserMixin):
             record
             for record in caplog.records
             if record.levelno == logging.WARNING
-            and record.name == FEATURE_CHAIN_PARSER_LOGGER
+            and record.name == AUTHOR_GUARDS_LOGGER
             and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
         ]
         assert warnings
@@ -186,7 +187,7 @@ class _CapturelessWithMarkerC772(FeatureChainParserMixin):
             record
             for record in caplog.records
             if record.levelno == logging.WARNING
-            and record.name == FEATURE_CHAIN_PARSER_LOGGER
+            and record.name == AUTHOR_GUARDS_LOGGER
             and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
         ]
         assert not warnings
@@ -303,7 +304,7 @@ class _X_rf772b(FeatureChainParserMixin, FeatureGroup):
         warnings = [
             record
             for record in caplog.records
-            if record.name == FEATURE_CHAIN_PARSER_LOGGER and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
+            if record.name == AUTHOR_GUARDS_LOGGER and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
         ]
         assert len(warnings) == 1
 
@@ -324,6 +325,6 @@ class _Y_rf772b(FeatureChainParserMixin):
         warnings = [
             record
             for record in caplog.records
-            if record.name == FEATURE_CHAIN_PARSER_LOGGER and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
+            if record.name == AUTHOR_GUARDS_LOGGER and "RECOGNITION_ONLY_PATTERN" in record.getMessage()
         ]
         assert len(warnings) == 1

tests/test_core/test_abstract_plugins/test_components/feature_chainer/test_explicit_none_opt_in.py

@@ -18,6 +18,7 @@
 
 import pytest
 
+from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_author_guards import check_required_when
 from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser import (
     FeatureChainParser,
     PropertyValueRejection,
@@ -129,32 +130,19 @@ def test_opted_in_explicit_none_satisfies_required_when(self) -> None:
         """D1: an opted-in satisfied requirement is met by an explicit None, which now counts as present."""
         property_mapping = {"k": PropertySpec("k", required_when=_always_required, allow_explicit_none=True)}
 
-        assert (
-            FeatureChainParser.check_required_when(
-                "Owner", "any_feature", [], property_mapping, Options(context={"k": None})
-            )
-            is True
-        )
+        assert check_required_when("Owner", "any_feature", [], property_mapping, Options(context={"k": None})) is True
 
     def test_flagless_explicit_none_leaves_requirement_unmet(self) -> None:
         """D2 contrast (DoD 3): without the flag the explicit None reads as absent, so the requirement is unmet."""
         property_mapping = {"k": PropertySpec("k", required_when=_always_required)}
 
-        assert (
-            FeatureChainParser.check_required_when(
-                "Owner", "any_feature", [], property_mapping, Options(context={"k": None})
-            )
-            is False
-        )
+        assert check_required_when("Owner", "any_feature", [], property_mapping, Options(context={"k": None})) is False
 
     def test_opted_in_absent_key_leaves_requirement_unmet(self) -> None:
         """D3: absent stays absent; an opted-in required key that is truly absent is still unmet."""
         property_mapping = {"k": PropertySpec("k", required_when=_always_required, allow_explicit_none=True)}
 
-        assert (
-            FeatureChainParser.check_required_when("Owner", "any_feature", [], property_mapping, Options(context={}))
-            is False
-        )
+        assert check_required_when("Owner", "any_feature", [], property_mapping, Options(context={})) is False
 
 
 class TestMatchGuardHonorsExplicitNone:

tests/test_core/test_abstract_plugins/test_components/feature_chainer/test_name_path_presence_guard.py

@@ -2,7 +2,7 @@
 
 The rule is enforced inside ``FeatureChainParserMixin.match_parser_criteria``, so a provider that
 overrides ``match_feature_group_criteria`` and returns True WITHOUT delegating bypasses it entirely.
-Like ``required_when`` (``FeatureChainParser.install_required_when_guard``), the enforcement must be
+Like ``required_when`` (``feature_chain_author_guards.install_required_when_guard``), the enforcement must be
 a definition-time wrapper around the matcher: it turns the bypass into a warned non-match, honors
 the same exemptions as the inner rule, applies only when the name identifies the group, and never
 duplicates the inner path's warning.