diff --git a/src/orcapod/contexts/data/v0.1.json b/src/orcapod/contexts/data/v0.1.json index a378cf1d8..9b6942c94 100644 --- a/src/orcapod/contexts/data/v0.1.json +++ b/src/orcapod/contexts/data/v0.1.json @@ -95,7 +95,8 @@ "_class": "orcapod.hashing.semantic_hashing.function_info_extractors.FunctionSignatureExtractor", "_config": { "include_module": true, - "include_defaults": true + "include_defaults": true, + "type_converter": {"_ref": "type_converter"} } }, "python_type_handler_registry": { @@ -110,7 +111,7 @@ [{"_type": "types.FunctionType"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.FunctionHandler", "_config": {"function_info_extractor": {"_ref": "function_semantic_hasher"}}}], [{"_type": "types.BuiltinFunctionType"},{"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.FunctionHandler", "_config": {"function_info_extractor": {"_ref": "function_semantic_hasher"}}}], [{"_type": "types.MethodType"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.FunctionHandler", "_config": {"function_info_extractor": {"_ref": "function_semantic_hasher"}}}], - [{"_type": "builtins.type"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.TypeObjectHandler", "_config": {}}], + [{"_type": "builtins.type"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.TypeObjectHandler", "_config": {"type_converter": {"_ref": "type_converter"}}}], [{"_type": "types.GenericAlias"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.GenericAliasHandler", "_config": {}}], [{"_type": "types.UnionType"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.UnionTypeHandler", "_config": {}}], [{"_type": "typing._GenericAlias"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.GenericAliasHandler", "_config": {}}], diff --git a/src/orcapod/hashing/hash_utils.py b/src/orcapod/hashing/hash_utils.py index ad5dfee52..f17d496cb 100644 --- a/src/orcapod/hashing/hash_utils.py +++ b/src/orcapod/hashing/hash_utils.py @@ -6,12 +6,16 @@ import zlib from collections.abc import Callable, Collection from pathlib import Path +from typing import TYPE_CHECKING import xxhash from upath import UPath from orcapod.types import ContentHash, PathLike +if TYPE_CHECKING: + from orcapod.protocols.semantic_types_protocols import TypeConverterProtocol + logger = logging.getLogger(__name__) @@ -32,29 +36,58 @@ def is_union_annotation(annotation: object) -> bool: return getattr(annotation, "__origin__", None) is typing.Union -def canonical_annotation_str(annotation: object) -> str: +def canonical_annotation_str( + annotation: object, + type_converter: "TypeConverterProtocol | None" = None, +) -> str: """Return a stable, canonical string for a type annotation. + Resolves types registered in *type_converter* (e.g. ``orcapod.File``) to + their stable ``logical_type_name`` (e.g. ``"orcapod.file"``) via + ``type_converter.get_logical_type()``, so that internal module relocations + do not change the string representation. + For union types (both PEP 604 ``X | Y`` and ``typing.Union[X, Y]``), members are sorted byte-wise so that ``str | Path`` and ``Path | str`` - produce the same canonical string. Non-union types fall through to - ``inspect.formatannotation``, preserving existing behaviour exactly. + produce the same canonical string. - The canonical ordering key is the fully qualified type name produced by - ``inspect.formatannotation`` (e.g. ``"pathlib.Path"``, ``"str"``), - sorted lexicographically. This is stable across Python versions and - machines and does not depend on ``id()`` or ``__hash__``. + For generic aliases (``list[X]``, ``dict[K, V]``), args are recursed with + the same converter so nested orcapod types are also canonicalized. + + Non-union, non-generic types not found in the converter fall through to + ``inspect.formatannotation``, preserving existing behaviour exactly. Args: annotation: A type annotation object. + type_converter: Optional ``TypeConverterProtocol``. When provided, + registered logical types resolve to their stable + ``logical_type_name`` via ``get_logical_type()``. Returns: A canonical string representation. """ + # Registered logical type: use stable canonical name (e.g. "orcapod.file") + if type_converter is not None and isinstance(annotation, type): + lt = type_converter.get_logical_type(annotation) + if lt is not None: + return lt.logical_type_name + + # Union types (PEP 604 X | Y and typing.Union): sort members for order-independence if is_union_annotation(annotation): args = getattr(annotation, "__args__", ()) or () - member_strs = sorted(canonical_annotation_str(a) for a in args) + member_strs = sorted(canonical_annotation_str(a, type_converter) for a in args) return " | ".join(member_strs) + + # Generic aliases (list[X], dict[K, V], etc.): recurse over args + origin = getattr(annotation, "__origin__", None) + if origin is not None and not is_union_annotation(annotation): + args = getattr(annotation, "__args__", None) or () + origin_str = canonical_annotation_str(origin, type_converter) + if args: + args_str = ", ".join(canonical_annotation_str(a, type_converter) for a in args) + return f"{origin_str}[{args_str}]" + return origin_str + return inspect.formatannotation(annotation) diff --git a/src/orcapod/hashing/semantic_hashing/builtin_handlers.py b/src/orcapod/hashing/semantic_hashing/builtin_handlers.py index 0d6c2fcfd..de6bab8ef 100644 --- a/src/orcapod/hashing/semantic_hashing/builtin_handlers.py +++ b/src/orcapod/hashing/semantic_hashing/builtin_handlers.py @@ -36,6 +36,7 @@ HandlerRegistryProtocol, SemanticHasherProtocol, ) + from orcapod.protocols.semantic_types_protocols import TypeConverterProtocol logger = logging.getLogger(__name__) @@ -83,14 +84,32 @@ def handle(self, obj: Any, hasher: "SemanticHasherProtocol") -> Any: class TypeObjectHandler: """Hasher for type objects (classes passed as values). - Returns a stable string of the form ``"type:."``. + Resolves types registered in the ``LogicalTypeRegistry`` exposed by + *type_converter* to their stable ``logical_type_name`` + (e.g. ``"type:orcapod.file"`` for ``op.File``). + Falls back to ``"type:."`` for unregistered types or + when no ``type_converter`` is provided. + + Args: + type_converter: Optional ``TypeConverterProtocol``. When provided, + ``type_converter.get_logical_type(obj)`` is called to resolve + registered logical types to their stable canonical name. When + ``None`` (the default), the fallback ``"type:."`` + serialisation is always used. """ + def __init__(self, type_converter: "TypeConverterProtocol | None" = None) -> None: + self._type_converter = type_converter + def handle(self, obj: Any, hasher: "SemanticHasherProtocol") -> Any: if not isinstance(obj, type): raise TypeError( f"TypeObjectHandler: expected a type/class, got {type(obj)!r}" ) + if self._type_converter is not None: + lt = self._type_converter.get_logical_type(obj) + if lt is not None: + return f"type:{lt.logical_type_name}" module: str = obj.__module__ or "" qualname: str = obj.__qualname__ return f"type:{module}.{qualname}" @@ -418,6 +437,7 @@ def register_builtin_python_type_handlers( function_info_extractor: Any = None, arrow_hasher: "ArrowHasherProtocol | None" = None, directory_hasher: Any = None, + type_converter: "TypeConverterProtocol | None" = None, ) -> None: """Register all built-in semantic hashers into *registry*. @@ -448,6 +468,11 @@ def register_builtin_python_type_handlers( When ``None``, lazy resolution via the default context is used. directory_hasher: Optional ``DirectoryHasherProtocol`` for directory tree hashing. Defaults to ``BasicDirectoryHasher(sha256)``. + type_converter: Optional ``TypeConverterProtocol`` forwarded to + ``TypeObjectHandler`` and ``FunctionSignatureExtractor`` for stable + canonical type-name resolution via ``get_logical_type()``. + When ``None`` (the default), both handlers fall back to the raw + ``"type:."`` serialisation. """ if file_hasher is None: from orcapod.hashing.file_hashers import FileHasher @@ -467,6 +492,7 @@ def register_builtin_python_type_handlers( function_info_extractor = FunctionSignatureExtractor( include_module=True, include_defaults=True, + type_converter=type_converter, ) bytes_hasher = BytesHandler() @@ -488,7 +514,7 @@ def register_builtin_python_type_handlers( registry.register(_types.BuiltinFunctionType, function_hasher) registry.register(_types.MethodType, function_hasher) - registry.register(type, TypeObjectHandler()) + registry.register(type, TypeObjectHandler(type_converter=type_converter)) registry.register(_types.UnionType, UnionTypeHandler()) generic_alias_hasher = GenericAliasHandler() diff --git a/src/orcapod/hashing/semantic_hashing/function_info_extractors.py b/src/orcapod/hashing/semantic_hashing/function_info_extractors.py index c7ea9b74f..cf036b78e 100644 --- a/src/orcapod/hashing/semantic_hashing/function_info_extractors.py +++ b/src/orcapod/hashing/semantic_hashing/function_info_extractors.py @@ -1,16 +1,73 @@ import inspect from collections.abc import Callable -from typing import Any, Literal +from typing import TYPE_CHECKING, Any, Literal -from orcapod.hashing.hash_utils import canonical_annotation_str, is_union_annotation +from orcapod.hashing.hash_utils import canonical_annotation_str from orcapod.protocols.hashing_protocols import FunctionInfoExtractorProtocol from orcapod.types import Schema +if TYPE_CHECKING: + from orcapod.protocols.semantic_types_protocols import TypeConverterProtocol -class FunctionNameExtractor: - """ - Extractor that only uses the function name for information extraction. + +def _format_param( + param: inspect.Parameter, + canonical_annotation: str | None, + include_defaults: bool, +) -> str: + """Reconstruct a parameter string from its structured components. + + Produces output identical to ``str(inspect.Parameter)`` for normal inputs, + but substitutes ``canonical_annotation`` for the raw annotation string without + any string search/replace. This avoids two failure modes of the substitution + approach: + + 1. Accidentally matching the annotation string inside a complex default + value's ``repr`` (e.g. a dataclass whose repr embeds the type name). + 2. Truncating annotations that contain ``=`` (e.g. ``Literal["a=b"]``) when + stripping defaults via ``str.split("=")``. + + The format follows CPython's ``inspect.Parameter.__str__`` exactly: + + - With annotation and default: ``name: type = repr(default)`` + - With annotation, no default: ``name: type`` + - No annotation, with default: ``name=repr(default)`` (no spaces around ``=``) + - No annotation, no default: ``name`` + - ``*args`` / ``**kwargs`` get the corresponding prefix. + + Args: + param: The parameter to format. + canonical_annotation: Canonical string for the annotation, or ``None`` + when the parameter carries no annotation. + include_defaults: Whether to include the default value. + + Returns: + A formatted parameter string. """ + if param.kind == inspect.Parameter.VAR_POSITIONAL: + prefix = "*" + elif param.kind == inspect.Parameter.VAR_KEYWORD: + prefix = "**" + else: + prefix = "" + + base = f"{prefix}{param.name}" + has_default = include_defaults and param.default is not inspect.Parameter.empty + + if canonical_annotation is not None: + # "name: type" or "name: type = default" + if has_default: + return f"{base}: {canonical_annotation} = {repr(param.default)}" + return f"{base}: {canonical_annotation}" + else: + # "name" or "name=default" (CPython omits spaces around = without annotation) + if has_default: + return f"{base}={repr(param.default)}" + return base + + +class FunctionNameExtractor: + """Extractor that only uses the function name for information extraction.""" def extract_function_info( self, @@ -26,16 +83,35 @@ def extract_function_info( class FunctionSignatureExtractor: - """ - Extractor that uses the function signature for information extraction. + """Extractor that uses the function signature for information extraction. + + When a ``type_converter`` is provided, orcapod logical types in annotations + are replaced with their stable ``logical_type_name`` + (e.g. ``"orcapod.file"``) rather than the full import path (e.g. + ``"orcapod.logical_types.file_type.File"``). This prevents internal + module reorganisations from invalidating cached function-pod signatures. + + For **parameter** annotations each parameter string is reconstructed from + its structured components (name, kind, canonical annotation, default) via + ``_format_param``, avoiding any string search/replace. + + For **return** annotations the raw annotation object is stored unchanged. + Canonicalisation happens at hash time: ``TypeObjectHandler`` (wired with the + same ``type_converter``) resolves registered types to their stable name, so + both registered and unregistered return types are handled consistently + without any special casing here. """ - def __init__(self, include_module: bool = True, include_defaults: bool = True): + def __init__( + self, + include_module: bool = True, + include_defaults: bool = True, + type_converter: "TypeConverterProtocol | None" = None, + ): self.include_module = include_module self.include_defaults = include_defaults + self._type_converter = type_converter - # FIXME: Fix this implementation!! - # BUG: Currently this is not using the input_types and output_types parameters def extract_function_info( self, func: Callable[..., Any], @@ -48,7 +124,7 @@ def extract_function_info( # Use eval_str=True so that string annotations produced by # ``from __future__ import annotations`` (PEP 563) are resolved to live - # type objects before we check for union types. + # type objects before we canonicalise them. try: sig = inspect.signature(func, eval_str=True) except (NameError, TypeError, AttributeError, SyntaxError): @@ -57,8 +133,7 @@ def extract_function_info( # module scope). sig = inspect.signature(func) - # Build the signature string - parts = {} + parts: dict[str, Any] = {} # Add module if requested if self.include_module and hasattr(func, "__module__"): @@ -67,26 +142,33 @@ def extract_function_info( # Add function name parts["name"] = function_name or func.__name__ - # Add parameters + tc = self._type_converter + + # Build each parameter string from structured components. + # Using _format_param instead of str(param) + string substitution avoids + # accidentally matching the annotation text inside a default value's repr, + # and correctly handles annotations that contain '=' (e.g. Literal["a=b"]). param_strs = [] - for name, param in sig.parameters.items(): - param_str = str(param) + for _, param in sig.parameters.items(): annotation = param.annotation - if annotation is not inspect.Parameter.empty and is_union_annotation(annotation): - old_ann = inspect.formatannotation(annotation) - new_ann = canonical_annotation_str(annotation) - # Replace ": " with ": " (first occurrence only). - # The ": " prefix distinguishes the annotation from the default value. - param_str = param_str.replace(f": {old_ann}", f": {new_ann}", 1) - if not self.include_defaults and "=" in param_str: - param_str = param_str.split("=")[0].strip() - param_strs.append(param_str) + canonical_ann = ( + canonical_annotation_str(annotation, tc) + if annotation is not inspect.Parameter.empty + else None + ) + param_strs.append(_format_param(param, canonical_ann, self.include_defaults)) parts["params"] = ", ".join(param_strs) - # Add return annotation if present - if sig.return_annotation is not inspect.Signature.empty: - parts["returns"] = sig.return_annotation + # Add return annotation if present. + # The raw annotation object is stored here and hashed by the type + # handler registry — TypeObjectHandler (configured with a type_converter) + # canonicalises registered logical types to their stable + # ``logical_type_name`` (e.g. ``"type:orcapod.file"``), so no special + # casing is required here. + ret_ann = sig.return_annotation + if ret_ann is not inspect.Signature.empty: + parts["returns"] = ret_ann return parts diff --git a/src/orcapod/protocols/semantic_types_protocols.py b/src/orcapod/protocols/semantic_types_protocols.py index f23031903..f97b0989d 100644 --- a/src/orcapod/protocols/semantic_types_protocols.py +++ b/src/orcapod/protocols/semantic_types_protocols.py @@ -8,6 +8,8 @@ if TYPE_CHECKING: import pyarrow as pa + from orcapod.logical_types.protocols import LogicalTypeProtocol + class TypeConverterProtocol(Protocol): def python_type_to_arrow_type(self, python_type: DataType) -> "pa.DataType": ... @@ -53,4 +55,6 @@ def get_arrow_to_python_converter( def ensure_types_registered_for_schemas(self, *schemas: Schema) -> None: ... + def get_logical_type(self, python_type: type) -> "LogicalTypeProtocol | None": ... + diff --git a/superpowers/plans/2026-09-02-itl-638-stable-type-annotation-hashing.md b/superpowers/plans/2026-09-02-itl-638-stable-type-annotation-hashing.md new file mode 100644 index 000000000..5e9956a75 --- /dev/null +++ b/superpowers/plans/2026-09-02-itl-638-stable-type-annotation-hashing.md @@ -0,0 +1,1245 @@ +# Stable Type Annotation Hashing (ITL-638) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use sensei:subagent-driven-development (recommended) or sensei:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace module-path-based type serialization in function pod signature hashing with stable canonical names from `LogicalTypeRegistry`, so that internal module reorganizations (e.g. `op.File` moving between subpackages) no longer invalidate cached function pod signatures. + +**Architecture:** Three coordinated changes: (1) extend `canonical_annotation_str` in `hash_utils.py` to accept a `LogicalTypeRegistry` and resolve registered types to their stable `logical_type_name`; (2) fix `TypeObjectHandler` to use this registry for bare type objects; (3) fix `FunctionSignatureExtractor` to canonicalize both parameter annotation strings and return annotation through the same helper. Guarded by pre-fix golden-value fixtures that confirm the change only touches orcapod logical types (e.g. `op.File`) and leaves builtins untouched. + +**Tech Stack:** Python 3.11+, pytest via `uv run`, `orcapod.logical_types.registry.LogicalTypeRegistry`, `orcapod.hashing` + +**Branch:** `eywalker/itl-638-function-pod-signature-hashing-uses-full-type-import-paths` + +--- + +## File Map + +| File | Action | Purpose | +|------|--------|---------| +| `tests/test_hashing/generate_type_annotation_golden.py` | Create | Script to generate pre-fix golden hashes | +| `tests/test_hashing/hash_samples/type_annotation_golden.json` | Generate+commit | Frozen pre-fix hash values | +| `tests/test_hashing/test_type_annotation_golden.py` | Create | Golden comparison + diff regression tests | +| `src/orcapod/hashing/hash_utils.py` | Modify | Extend `canonical_annotation_str` with registry param | +| `src/orcapod/hashing/semantic_hashing/builtin_handlers.py` | Modify | Fix `TypeObjectHandler` + wire registry in `register_builtin_python_type_handlers` | +| `src/orcapod/hashing/semantic_hashing/function_info_extractors.py` | Modify | Fix `FunctionSignatureExtractor` params + returns | + +--- + +### Task 1: Generate and commit pre-fix golden values + +**Files:** +- Create: `tests/test_hashing/generate_type_annotation_golden.py` +- Generate: `tests/test_hashing/hash_samples/type_annotation_golden.json` + +This task captures the current (broken) hash values before any fix. The JSON is committed as an immutable record. + +- [ ] **Step 1: Write the golden generator script** + +Create `tests/test_hashing/generate_type_annotation_golden.py`: + +```python +"""Generate pre-fix golden hash values for type annotation hashing. + +Run once (before the ITL-638 fix) with: + uv run python tests/test_hashing/generate_type_annotation_golden.py + +Outputs: tests/test_hashing/hash_samples/type_annotation_golden.json +""" +from __future__ import annotations + +import inspect +import json +import pathlib +import typing +from uuid import UUID + +import orcapod as op +from orcapod.hashing.defaults import get_default_semantic_hasher +from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, +) +from orcapod.logical_types.file_type import File +from orcapod.logical_types.directory_type import Directory + +GOLDEN_PATH = pathlib.Path(__file__).parent / "hash_samples" / "type_annotation_golden.json" + +# --------------------------------------------------------------------------- +# Annotation types to hash individually (bare type objects) +# --------------------------------------------------------------------------- +ANNOTATION_CASES: dict[str, object] = { + # Builtins + "int": int, + "str": str, + "float": float, + "bytes": bytes, + # orcapod logical types + "op.File": op.File, + "op.Directory": op.Directory, + "op.Path": op.Path, + "op.UUID": UUID, + # Generic aliases + "list[int]": list[int], + "dict[str, int]": dict[str, int], + "list[op.File]": list[op.File], + "dict[str, op.File]": dict[str, op.File], + # Unions + "int | str": int | str, + "op.File | None": op.File | None, + "Optional[op.File]": typing.Optional[op.File], +} + +# --------------------------------------------------------------------------- +# Functions whose full hash_object(func) and extract_function_info output +# are both captured. +# --------------------------------------------------------------------------- + +def fn_no_annotations(): + return None + +def fn_builtin_param(x: int, y: str) -> float: + return float(x) + +def fn_orcapod_param(f: op.File) -> str: + return str(f) + +def fn_orcapod_return(s: str) -> op.File: + return op.File(s) # type: ignore[arg-type] + +def fn_generic_orcapod(files: list[op.File]) -> list[str]: + return [] + +def fn_union_orcapod(f: op.File | None) -> op.File | None: + return f + +def fn_mixed(f: op.File, n: int) -> op.Directory: + return op.Directory(str(f)) # type: ignore[arg-type] + +FUNCTION_CASES: dict[str, object] = { + "fn_no_annotations": fn_no_annotations, + "fn_builtin_param": fn_builtin_param, + "fn_orcapod_param": fn_orcapod_param, + "fn_orcapod_return": fn_orcapod_return, + "fn_generic_orcapod": fn_generic_orcapod, + "fn_union_orcapod": fn_union_orcapod, + "fn_mixed": fn_mixed, +} + + +def main() -> None: + hasher = get_default_semantic_hasher() + extractor = FunctionSignatureExtractor(include_module=True, include_defaults=True) + + result: dict = {"annotation_hashes": {}, "function_info_hashes": {}, "function_object_hashes": {}} + + # Hash bare annotations through TypeObjectHandler + for key, ann in ANNOTATION_CASES.items(): + result["annotation_hashes"][key] = hasher.hash_object(ann).to_string() + + # Hash FunctionSignatureExtractor output dict, and full function object + for key, func in FUNCTION_CASES.items(): + info = extractor.extract_function_info(func) + result["function_info_hashes"][key] = hasher.hash_object(info).to_string() + result["function_object_hashes"][key] = hasher.hash_object(func).to_string() + + GOLDEN_PATH.parent.mkdir(parents=True, exist_ok=True) + GOLDEN_PATH.write_text(json.dumps(result, indent=2) + "\n") + print(f"Wrote golden values to {GOLDEN_PATH}") + + +if __name__ == "__main__": + main() +``` + +- [ ] **Step 2: Run the generator to produce the golden JSON** + +```bash +cd /path/to/orcapod-python +uv run python tests/test_hashing/generate_type_annotation_golden.py +``` + +Expected output: +``` +Wrote golden values to tests/test_hashing/hash_samples/type_annotation_golden.json +``` + +- [ ] **Step 3: Inspect the golden JSON to verify it looks reasonable** + +```bash +uv run python -c " +import json, pathlib +d = json.loads(pathlib.Path('tests/test_hashing/hash_samples/type_annotation_golden.json').read_text()) +for section, entries in d.items(): + print(f'\\n=== {section} ===') + for k, v in entries.items(): + print(f' {k}: {v}') +" +``` + +Verify that: +- `op.File`, `op.Directory`, `op.Path`, `op.UUID` annotation hashes contain the full module path (e.g. the hash input is `"type:orcapod.logical_types.file_type.File"`) +- Builtin hashes (`int`, `str`, etc.) are present + +- [ ] **Step 4: Commit the generator script and the golden JSON** + +```bash +git add tests/test_hashing/generate_type_annotation_golden.py +git add tests/test_hashing/hash_samples/type_annotation_golden.json +git commit -m "test(hashing): add pre-fix golden hash values for type annotation hashing (ITL-638)" +``` + +--- + +### Task 2: Write golden-comparison test skeleton (failing) + +**Files:** +- Create: `tests/test_hashing/test_type_annotation_golden.py` + +This test loads the golden JSON and will assert (post-fix) that builtins are unchanged and orcapod types changed to canonical names. Writing it now (before the fix) means the "changed" assertions will fail until the fix lands. + +- [ ] **Step 1: Write the test file** + +Create `tests/test_hashing/test_type_annotation_golden.py`: + +```python +"""Golden-value regression tests for type annotation hashing (ITL-638). + +Two test classes: + TestGoldenStability -- builtins must be UNCHANGED after the fix. + TestGoldenCanonical -- orcapod logical types must produce NEW canonical hashes. + +The golden JSON was generated pre-fix by generate_type_annotation_golden.py. +""" +from __future__ import annotations + +import json +import pathlib +import typing +from uuid import UUID + +import pytest + +import orcapod as op +from orcapod.hashing.defaults import get_default_semantic_hasher +from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, +) + +GOLDEN_PATH = ( + pathlib.Path(__file__).parent / "hash_samples" / "type_annotation_golden.json" +) + +# --------------------------------------------------------------------------- +# The same annotation and function cases as the generator — must stay in sync. +# --------------------------------------------------------------------------- + +ANNOTATION_CASES: dict[str, object] = { + "int": int, + "str": str, + "float": float, + "bytes": bytes, + "op.File": op.File, + "op.Directory": op.Directory, + "op.Path": op.Path, + "op.UUID": UUID, + "list[int]": list[int], + "dict[str, int]": dict[str, int], + "list[op.File]": list[op.File], + "dict[str, op.File]": dict[str, op.File], + "int | str": int | str, + "op.File | None": op.File | None, + "Optional[op.File]": typing.Optional[op.File], +} + +# Annotation keys that are expected to change after the fix. +# Any key not in this set must have an UNCHANGED hash. +EXPECTED_CHANGED_KEYS: frozenset[str] = frozenset({ + "op.File", + "op.Directory", + "op.Path", + "op.UUID", + "list[op.File]", + "dict[str, op.File]", + "op.File | None", + "Optional[op.File]", +}) + + +def fn_no_annotations(): + return None + +def fn_builtin_param(x: int, y: str) -> float: + return float(x) + +def fn_orcapod_param(f: op.File) -> str: + return str(f) + +def fn_orcapod_return(s: str) -> op.File: + return op.File(s) # type: ignore[arg-type] + +def fn_generic_orcapod(files: list[op.File]) -> list[str]: + return [] + +def fn_union_orcapod(f: op.File | None) -> op.File | None: + return f + +def fn_mixed(f: op.File, n: int) -> op.Directory: + return op.Directory(str(f)) # type: ignore[arg-type] + +FUNCTION_CASES: dict[str, object] = { + "fn_no_annotations": fn_no_annotations, + "fn_builtin_param": fn_builtin_param, + "fn_orcapod_param": fn_orcapod_param, + "fn_orcapod_return": fn_orcapod_return, + "fn_generic_orcapod": fn_generic_orcapod, + "fn_union_orcapod": fn_union_orcapod, + "fn_mixed": fn_mixed, +} + +# Functions expected to have different hashes after the fix. +EXPECTED_CHANGED_FUNCTIONS: frozenset[str] = frozenset({ + "fn_orcapod_param", + "fn_orcapod_return", + "fn_generic_orcapod", + "fn_union_orcapod", + "fn_mixed", +}) + + +@pytest.fixture(scope="module") +def golden() -> dict: + assert GOLDEN_PATH.exists(), ( + f"Golden file not found: {GOLDEN_PATH}. " + "Run generate_type_annotation_golden.py first." + ) + return json.loads(GOLDEN_PATH.read_text()) + + +@pytest.fixture(scope="module") +def hasher(): + return get_default_semantic_hasher() + + +@pytest.fixture(scope="module") +def extractor(): + return FunctionSignatureExtractor(include_module=True, include_defaults=True) + + +class TestGoldenStability: + """Builtins and non-orcapod annotations must hash identically before and after the fix.""" + + def test_builtin_annotation_hashes_unchanged(self, golden, hasher): + stable_keys = {k for k in ANNOTATION_CASES if k not in EXPECTED_CHANGED_KEYS} + mismatches = {} + for key in stable_keys: + ann = ANNOTATION_CASES[key] + current = hasher.hash_object(ann).to_string() + expected = golden["annotation_hashes"][key] + if current != expected: + mismatches[key] = {"expected": expected, "current": current} + assert not mismatches, ( + f"Unexpected hash changes in stable annotations:\n" + + "\n".join(f" {k}: {v}" for k, v in mismatches.items()) + ) + + def test_builtin_function_hashes_unchanged(self, golden, hasher, extractor): + stable_fns = {k for k in FUNCTION_CASES if k not in EXPECTED_CHANGED_FUNCTIONS} + mismatches = {} + for key in stable_fns: + func = FUNCTION_CASES[key] + info = extractor.extract_function_info(func) + current = hasher.hash_object(info).to_string() + expected = golden["function_info_hashes"][key] + if current != expected: + mismatches[key] = {"expected": expected, "current": current} + assert not mismatches, ( + f"Unexpected hash changes in stable functions:\n" + + "\n".join(f" {k}: {v}" for k, v in mismatches.items()) + ) + + +class TestGoldenCanonical: + """orcapod logical types must produce NEW hashes (canonical name, not module path).""" + + def test_orcapod_annotation_hashes_changed(self, golden, hasher): + """After the fix, orcapod type annotation hashes must DIFFER from golden.""" + unchanged = {} + for key in EXPECTED_CHANGED_KEYS: + if key not in ANNOTATION_CASES: + continue + ann = ANNOTATION_CASES[key] + current = hasher.hash_object(ann).to_string() + expected = golden["annotation_hashes"][key] + if current == expected: + unchanged[key] = current + assert not unchanged, ( + f"Expected these annotation hashes to change after the fix, but they didn't:\n" + + "\n".join(f" {k}: {v}" for k, v in unchanged.items()) + ) + + def test_orcapod_function_hashes_changed(self, golden, hasher, extractor): + """After the fix, functions using orcapod types must DIFFER from golden.""" + unchanged = {} + for key in EXPECTED_CHANGED_FUNCTIONS: + func = FUNCTION_CASES[key] + info = extractor.extract_function_info(func) + current = hasher.hash_object(info).to_string() + expected = golden["function_info_hashes"][key] + if current == expected: + unchanged[key] = current + assert not unchanged, ( + f"Expected these function hashes to change after the fix, but they didn't:\n" + + "\n".join(f" {k}: {v}" for k, v in unchanged.items()) + ) +``` + +- [ ] **Step 2: Verify the stability tests pass (pre-fix) and canonical tests fail** + +```bash +uv run pytest tests/test_hashing/test_type_annotation_golden.py::TestGoldenStability -v +``` + +Expected: All PASS (hashes match golden before any fix) + +```bash +uv run pytest tests/test_hashing/test_type_annotation_golden.py::TestGoldenCanonical -v +``` + +Expected: All FAIL (hashes still match golden — the fix hasn't landed yet) + +- [ ] **Step 3: Commit** + +```bash +git add tests/test_hashing/test_type_annotation_golden.py +git commit -m "test(hashing): add golden-diff regression tests for type annotation canonicalization (ITL-638)" +``` + +--- + +### Task 3: Extend `canonical_annotation_str` with registry support + +**Files:** +- Modify: `src/orcapod/hashing/hash_utils.py` +- Test: `tests/test_hashing/test_hash_utils.py` + +The existing `canonical_annotation_str` only handles union ordering. We extend it to accept an optional `LogicalTypeRegistry` and also handle generic alias recursion. + +- [ ] **Step 1: Write failing tests for the extended function** + +Add to `tests/test_hashing/test_hash_utils.py` (append to existing file — do not replace existing tests): + +```python +# --------------------------------------------------------------------------- +# Tests for canonical_annotation_str with registry (ITL-638) +# --------------------------------------------------------------------------- +import typing +from uuid import UUID + +import orcapod as op +from orcapod.contexts import get_default_logical_type_registry +from orcapod.hashing.hash_utils import canonical_annotation_str + + +class TestCanonicalAnnotationStrWithRegistry: + """canonical_annotation_str resolves registered logical types to stable names.""" + + @pytest.fixture + def registry(self): + return get_default_logical_type_registry() + + def test_builtin_type_unchanged(self, registry): + assert canonical_annotation_str(int, registry) == "int" + + def test_builtin_str_unchanged(self, registry): + assert canonical_annotation_str(str, registry) == "str" + + def test_registered_type_uses_logical_name(self, registry): + result = canonical_annotation_str(op.File, registry) + assert result == "orcapod.file" + + def test_registered_directory_uses_logical_name(self, registry): + result = canonical_annotation_str(op.Directory, registry) + assert result == "orcapod.directory" + + def test_registered_path_uses_logical_name(self, registry): + import pathlib + result = canonical_annotation_str(pathlib.Path, registry) + assert result == "orcapod.path" + + def test_registered_uuid_uses_logical_name(self, registry): + result = canonical_annotation_str(UUID, registry) + assert result == "orcapod.uuid" + + def test_generic_list_of_registered_type(self, registry): + result = canonical_annotation_str(list[op.File], registry) + assert result == "list[orcapod.file]" + + def test_generic_dict_with_registered_value(self, registry): + result = canonical_annotation_str(dict[str, op.File], registry) + assert result == "dict[str, orcapod.file]" + + def test_union_with_registered_type(self, registry): + result = canonical_annotation_str(op.File | None, registry) + # Members sorted; NoneType sorts before orcapod.file + assert result == "NoneType | orcapod.file" + + def test_optional_registered_type(self, registry): + result = canonical_annotation_str(typing.Optional[op.File], registry) + assert result == "NoneType | orcapod.file" + + def test_no_registry_fallback(self): + """Without registry, behaviour is identical to the existing function.""" + import inspect + result = canonical_annotation_str(op.File, None) + assert result == inspect.formatannotation(op.File) + + def test_stable_across_calls(self, registry): + r1 = canonical_annotation_str(op.File, registry) + r2 = canonical_annotation_str(op.File, registry) + assert r1 == r2 +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +```bash +uv run pytest tests/test_hashing/test_hash_utils.py::TestCanonicalAnnotationStrWithRegistry -v +``` + +Expected: All FAIL (function not yet extended) + +- [ ] **Step 3: Extend `canonical_annotation_str` in `hash_utils.py`** + +Replace the existing `canonical_annotation_str` function (currently lines 35-58) with this extended version. Add the `TYPE_CHECKING` guard at the top of the file if not already present: + +```python +# At top of file, inside the existing imports block: +from __future__ import annotations +import inspect +import types as _types +import typing +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from orcapod.logical_types.registry import LogicalTypeRegistry +``` + +Replace the `canonical_annotation_str` function body: + +```python +def canonical_annotation_str( + annotation: object, + registry: "LogicalTypeRegistry | None" = None, +) -> str: + """Return a stable, canonical string for a type annotation. + + Resolves types registered in *registry* (e.g. ``orcapod.File``) to their + stable ``logical_type_name`` (e.g. ``"orcapod.file"``) so that internal + module relocations do not change the string representation. + + For union types (both PEP 604 ``X | Y`` and ``typing.Union[X, Y]``), + members are sorted byte-wise so that ``str | Path`` and ``Path | str`` + produce the same canonical string. + + For generic aliases (``list[X]``, ``dict[K, V]``), args are recursed with + the same registry so nested orcapod types are also canonicalized. + + Non-union, non-generic types not found in the registry fall through to + ``inspect.formatannotation``, preserving existing behaviour exactly. + + Args: + annotation: A type annotation object. + registry: Optional ``LogicalTypeRegistry``. When provided, registered + logical types resolve to their stable ``logical_type_name``. + + Returns: + A canonical string representation. + """ + # Registered logical type: use stable canonical name (e.g. "orcapod.file") + if registry is not None and isinstance(annotation, type): + lt = registry.get_by_python_type(annotation) + if lt is not None: + return lt.logical_type_name + + # Union types (PEP 604 X | Y and typing.Union): sort members for order-independence + if is_union_annotation(annotation): + args = getattr(annotation, "__args__", ()) or () + member_strs = sorted(canonical_annotation_str(a, registry) for a in args) + return " | ".join(member_strs) + + # Generic aliases (list[X], dict[K, V], typing.List[X], etc.): recurse args + origin = getattr(annotation, "__origin__", None) + if origin is not None and not is_union_annotation(annotation): + args = getattr(annotation, "__args__", None) or () + origin_str = canonical_annotation_str(origin, registry) + if args: + args_str = ", ".join(canonical_annotation_str(a, registry) for a in args) + return f"{origin_str}[{args_str}]" + return origin_str + + return inspect.formatannotation(annotation) +``` + +- [ ] **Step 4: Run the new tests to confirm they pass** + +```bash +uv run pytest tests/test_hashing/test_hash_utils.py::TestCanonicalAnnotationStrWithRegistry -v +``` + +Expected: All PASS + +- [ ] **Step 5: Run the full `test_hash_utils.py` to confirm no regressions** + +```bash +uv run pytest tests/test_hashing/test_hash_utils.py -v +``` + +Expected: All PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/orcapod/hashing/hash_utils.py tests/test_hashing/test_hash_utils.py +git commit -m "feat(hashing): extend canonical_annotation_str to resolve registered logical types (ITL-638)" +``` + +--- + +### Task 4: Fix `TypeObjectHandler` to use registry lookup + +**Files:** +- Modify: `src/orcapod/hashing/semantic_hashing/builtin_handlers.py` +- Test: `tests/test_hashing/test_semantic_hasher.py` + +- [ ] **Step 1: Write failing tests for registry-aware `TypeObjectHandler`** + +Add to `tests/test_hashing/test_semantic_hasher.py` (append after the existing `TestTypeObjectHandler` class): + +```python +class TestTypeObjectHandlerWithRegistry: + """TypeObjectHandler uses stable canonical names for registered logical types.""" + + @pytest.fixture + def registry(self): + from orcapod.contexts import get_default_logical_type_registry + return get_default_logical_type_registry() + + @pytest.fixture + def handler(self, registry): + from orcapod.hashing.semantic_hashing.builtin_handlers import TypeObjectHandler + return TypeObjectHandler(logical_type_registry=registry) + + def test_registered_type_returns_canonical_name(self, handler, hasher): + import orcapod as op + result = handler.handle(op.File, hasher) + assert result == "type:orcapod.file" + + def test_registered_directory_returns_canonical_name(self, handler, hasher): + import orcapod as op + result = handler.handle(op.Directory, hasher) + assert result == "type:orcapod.directory" + + def test_unregistered_type_falls_back_to_module_qualname(self, handler, hasher): + result = handler.handle(int, hasher) + assert result == "type:builtins.int" + + def test_custom_class_falls_back_to_module_qualname(self, handler, hasher): + class _Local: + pass + result = handler.handle(_Local, hasher) + assert "type:" in result + assert "_Local" in result + + def test_hash_stable_across_calls(self, registry): + """Hashing op.File twice with registry produces identical ContentHash.""" + from orcapod.hashing.semantic_hashing.builtin_handlers import ( + TypeObjectHandler, + register_builtin_python_type_handlers, + ) + from orcapod.hashing.semantic_hashing.type_handler_registry import ( + PythonTypeHandlerRegistry, + ) + from orcapod.hashing.semantic_hashing.semantic_hasher import ( + SemanticAwarePythonHasher, + ) + import orcapod as op + + reg = PythonTypeHandlerRegistry() + register_builtin_python_type_handlers(reg, logical_type_registry=registry) + h = SemanticAwarePythonHasher(hasher_id="test_v1", type_handler_registry=reg) + assert h.hash_object(op.File) == h.hash_object(op.File) + + def test_simulated_module_relocation_stable(self, registry): + """Relocating a class's __module__ does not change the hash if its + logical_type_name is unchanged in the registry. + """ + from orcapod.hashing.semantic_hashing.builtin_handlers import ( + register_builtin_python_type_handlers, + ) + from orcapod.hashing.semantic_hashing.type_handler_registry import ( + PythonTypeHandlerRegistry, + ) + from orcapod.hashing.semantic_hashing.semantic_hasher import ( + SemanticAwarePythonHasher, + ) + import orcapod as op + + reg = PythonTypeHandlerRegistry() + register_builtin_python_type_handlers(reg, logical_type_registry=registry) + h = SemanticAwarePythonHasher(hasher_id="test_v1", type_handler_registry=reg) + + hash_before = h.hash_object(op.File) + + # Simulate module relocation by temporarily patching __module__ + original_module = op.File.__module__ + try: + op.File.__module__ = "orcapod.extension_types.file_type" + hash_after = h.hash_object(op.File) + finally: + op.File.__module__ = original_module + + assert hash_before == hash_after, ( + "Hash changed when op.File.__module__ was altered — " + "registry lookup is not being used." + ) +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +```bash +uv run pytest tests/test_hashing/test_semantic_hasher.py::TestTypeObjectHandlerWithRegistry -v +``` + +Expected: All FAIL (`TypeObjectHandler.__init__` does not accept `logical_type_registry` yet) + +- [ ] **Step 3: Update `TypeObjectHandler` in `builtin_handlers.py`** + +Replace the `TypeObjectHandler` class (currently lines 83-96): + +```python +class TypeObjectHandler: + """Hasher for type objects (classes passed as values). + + Resolves types registered in *logical_type_registry* to their stable + ``logical_type_name`` (e.g. ``"type:orcapod.file"`` for ``op.File``). + Falls back to ``"type:."`` for unregistered types. + + Args: + logical_type_registry: Optional ``LogicalTypeRegistry``. When ``None``, + the default context's registry is resolved lazily at call time, + following the same pattern as ``ArrowTableHandler``. + """ + + def __init__(self, logical_type_registry: Any = None) -> None: + self._logical_type_registry = logical_type_registry + + def _get_registry(self) -> Any: + if self._logical_type_registry is not None: + return self._logical_type_registry + from orcapod.contexts import get_default_context + return get_default_context().logical_type_registry + + def handle(self, obj: Any, hasher: "SemanticHasherProtocol") -> Any: + if not isinstance(obj, type): + raise TypeError( + f"TypeObjectHandler: expected a type/class, got {type(obj)!r}" + ) + registry = self._get_registry() + lt = registry.get_by_python_type(obj) + if lt is not None: + return f"type:{lt.logical_type_name}" + module: str = obj.__module__ or "" + qualname: str = obj.__qualname__ + return f"type:{module}.{qualname}" +``` + +- [ ] **Step 4: Run the new tests to confirm they pass** + +```bash +uv run pytest tests/test_hashing/test_semantic_hasher.py::TestTypeObjectHandlerWithRegistry -v +``` + +Expected: All PASS + +- [ ] **Step 5: Run the full `test_semantic_hasher.py` to confirm no regressions** + +```bash +uv run pytest tests/test_hashing/test_semantic_hasher.py -v +``` + +Expected: All PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/orcapod/hashing/semantic_hashing/builtin_handlers.py \ + tests/test_hashing/test_semantic_hasher.py +git commit -m "feat(hashing): make TypeObjectHandler resolve logical types to canonical names (ITL-638)" +``` + +--- + +### Task 5: Fix `FunctionSignatureExtractor` — consistent canonicalization for params and returns + +**Files:** +- Modify: `src/orcapod/hashing/semantic_hashing/function_info_extractors.py` +- Test: `tests/test_hashing/test_function_info_extractors.py` + +Currently: params embed annotation string via `str(param)` (module path), returns stores raw type object. After fix: both use `canonical_annotation_str(annotation, registry)`. + +- [ ] **Step 1: Write failing tests for registry-aware `FunctionSignatureExtractor`** + +Add to `tests/test_hashing/test_function_info_extractors.py` (append after existing tests): + +```python +class TestFunctionSignatureExtractorWithRegistry: + """FunctionSignatureExtractor canonicalizes both param and return annotations.""" + + @pytest.fixture + def registry(self): + from orcapod.contexts import get_default_logical_type_registry + return get_default_logical_type_registry() + + @pytest.fixture + def extractor(self, registry): + from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, + ) + return FunctionSignatureExtractor( + include_module=True, + include_defaults=True, + logical_type_registry=registry, + ) + + def test_return_annotation_is_canonical_string(self, extractor): + """parts['returns'] must be a string, not a type object.""" + import orcapod as op + + def fn(s: str) -> op.File: + ... + + info = extractor.extract_function_info(fn) + assert isinstance(info["returns"], str), ( + f"Expected str, got {type(info['returns'])}: {info['returns']!r}" + ) + assert info["returns"] == "orcapod.file" + + def test_param_annotation_is_canonical_string(self, extractor): + """Parameter annotation in params string uses logical_type_name.""" + import orcapod as op + + def fn(f: op.File) -> str: + ... + + info = extractor.extract_function_info(fn) + assert "orcapod.file" in info["params"], ( + f"Expected 'orcapod.file' in params, got: {info['params']!r}" + ) + assert "logical_types" not in info["params"], ( + f"Module path leaked into params: {info['params']!r}" + ) + + def test_generic_param_annotation_canonical(self, extractor): + """list[op.File] in a parameter is canonicalized.""" + import orcapod as op + + def fn(files: list[op.File]) -> str: + ... + + info = extractor.extract_function_info(fn) + assert "orcapod.file" in info["params"] + assert "logical_types" not in info["params"] + + def test_union_return_annotation_canonical(self, extractor): + """op.File | None return is canonicalized.""" + import orcapod as op + + def fn(s: str) -> op.File | None: + ... + + info = extractor.extract_function_info(fn) + assert isinstance(info["returns"], str) + assert "orcapod.file" in info["returns"] + assert "logical_types" not in info["returns"] + + def test_builtin_annotations_unchanged(self, extractor): + """Functions with only builtin annotations are unaffected.""" + def fn(x: int, y: str) -> float: + ... + + info = extractor.extract_function_info(fn) + assert "int" in info["params"] + assert "str" in info["params"] + assert info["returns"] == "float" + + def test_return_and_param_use_same_canonical_form(self, extractor): + """op.File in param and op.File as return use the same canonical string.""" + import orcapod as op + + def fn_param(f: op.File) -> str: + ... + + def fn_return(s: str) -> op.File: + ... + + info_param = extractor.extract_function_info(fn_param) + info_return = extractor.extract_function_info(fn_return) + + # Both should contain "orcapod.file" in their respective places + assert "orcapod.file" in info_param["params"] + assert info_return["returns"] == "orcapod.file" + + def test_simulated_relocation_stable(self, extractor): + """Patching __module__ on op.File does not change the extracted info.""" + import orcapod as op + + def fn(f: op.File) -> op.File: + ... + + info_before = extractor.extract_function_info(fn) + + original = op.File.__module__ + try: + op.File.__module__ = "orcapod.extension_types.file_type" + info_after = extractor.extract_function_info(fn) + finally: + op.File.__module__ = original + + assert info_before["params"] == info_after["params"] + assert info_before["returns"] == info_after["returns"] + + def test_no_registry_behaves_like_before(self): + """Without registry, returns is still a type object (pre-fix behavior).""" + import orcapod as op + from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, + ) + + extractor_no_reg = FunctionSignatureExtractor( + include_module=True, include_defaults=True, logical_type_registry=None + ) + + def fn(s: str) -> op.File: + ... + + info = extractor_no_reg.extract_function_info(fn) + # Without registry, returns is the raw type object (legacy behavior) + assert isinstance(info["returns"], type) +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +```bash +uv run pytest tests/test_hashing/test_function_info_extractors.py::TestFunctionSignatureExtractorWithRegistry -v +``` + +Expected: All FAIL (`logical_type_registry` param not accepted, `returns` is a type object not a string) + +- [ ] **Step 3: Update `FunctionSignatureExtractor` in `function_info_extractors.py`** + +Replace the `FunctionSignatureExtractor` class entirely: + +```python +class FunctionSignatureExtractor: + """Extractor that uses the function signature for information extraction. + + Canonicalizes type annotations for both parameters and the return type + using ``canonical_annotation_str``. When *logical_type_registry* is + provided (or resolved lazily from the default context), registered + orcapod types (e.g. ``op.File``) are serialized to their stable + ``logical_type_name`` (e.g. ``"orcapod.file"``) rather than to their + fully-qualified import path. + + This ensures that internal module reorganizations (e.g. moving + ``File`` between subpackages) do not invalidate function pod caches. + + Args: + include_module: Include ``func.__module__`` in the extracted info. + include_defaults: Include default parameter values. + logical_type_registry: Optional ``LogicalTypeRegistry``. When + ``None``, the default context's registry is resolved lazily at + call time, following the same pattern as ``ArrowTableHandler``. + """ + + def __init__( + self, + include_module: bool = True, + include_defaults: bool = True, + logical_type_registry: Any = None, + ) -> None: + self.include_module = include_module + self.include_defaults = include_defaults + self._logical_type_registry = logical_type_registry + + def _get_registry(self) -> Any: + if self._logical_type_registry is not None: + return self._logical_type_registry + from orcapod.contexts import get_default_context + return get_default_context().logical_type_registry + + # FIXME: Fix this implementation!! + # BUG: Currently this is not using the input_types and output_types parameters + def extract_function_info( + self, + func: Callable[..., Any], + function_name: str | None = None, + input_typespec: Schema | None = None, + output_typespec: Schema | None = None, + ) -> dict[str, Any]: + if not callable(func): + raise TypeError("Provided object is not callable") + + # Use eval_str=True so that string annotations produced by + # ``from __future__ import annotations`` (PEP 563) are resolved to live + # type objects before canonicalization. + try: + sig = inspect.signature(func, eval_str=True) + except (NameError, TypeError, AttributeError, SyntaxError): + sig = inspect.signature(func) + + registry = self._get_registry() + parts: dict[str, Any] = {} + + if self.include_module and hasattr(func, "__module__"): + parts["module"] = func.__module__ + + parts["name"] = function_name or func.__name__ + + param_strs = [] + for name, param in sig.parameters.items(): + param_str = str(param) + annotation = param.annotation + if annotation is not inspect.Parameter.empty: + old_ann = inspect.formatannotation(annotation) + new_ann = canonical_annotation_str(annotation, registry) + if old_ann != new_ann: + # Replace ": " with ": " (first occurrence). + # The ": " prefix distinguishes the annotation from the default. + param_str = param_str.replace(f": {old_ann}", f": {new_ann}", 1) + if not self.include_defaults and "=" in param_str: + param_str = param_str.split("=")[0].strip() + param_strs.append(param_str) + + parts["params"] = ", ".join(param_strs) + + if sig.return_annotation is not inspect.Signature.empty: + # Store as canonical string (not raw type object) for consistency + # with the params representation and for registry-stable hashing. + parts["returns"] = canonical_annotation_str( + sig.return_annotation, registry + ) + + return parts +``` + +Add the import of `canonical_annotation_str` at the top of `function_info_extractors.py` (it's already imported from `hash_utils`; verify the import line includes it): + +```python +from orcapod.hashing.hash_utils import canonical_annotation_str, is_union_annotation +``` + +Also add `Any` to the imports if not present: + +```python +from typing import Any, Literal +``` + +- [ ] **Step 4: Run the new tests to confirm they pass** + +```bash +uv run pytest tests/test_hashing/test_function_info_extractors.py::TestFunctionSignatureExtractorWithRegistry -v +``` + +Expected: All PASS + +- [ ] **Step 5: Run the full `test_function_info_extractors.py` to confirm no regressions** + +```bash +uv run pytest tests/test_hashing/test_function_info_extractors.py -v +``` + +Expected: All PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/orcapod/hashing/semantic_hashing/function_info_extractors.py \ + tests/test_hashing/test_function_info_extractors.py +git commit -m "feat(hashing): canonicalize param and return annotations in FunctionSignatureExtractor (ITL-638)" +``` + +--- + +### Task 6: Wire registry into `register_builtin_python_type_handlers` + +**Files:** +- Modify: `src/orcapod/hashing/semantic_hashing/builtin_handlers.py` + +Pass the `LogicalTypeRegistry` through to both `TypeObjectHandler` and `FunctionSignatureExtractor` so callers who already have a registry don't rely on lazy context lookup. + +- [ ] **Step 1: Update `register_builtin_python_type_handlers` signature and body** + +In `builtin_handlers.py`, update the function signature (currently around line 415): + +```python +def register_builtin_python_type_handlers( + registry: "HandlerRegistryProtocol", + file_hasher: Any = None, + function_info_extractor: Any = None, + arrow_hasher: "ArrowHasherProtocol | None" = None, + directory_hasher: Any = None, + logical_type_registry: Any = None, +) -> None: + """Register all built-in semantic hashers into *registry*. + + Args: + registry: The ``HandlerRegistryProtocol`` instance to populate. + file_hasher: Optional ``FileContentHasherProtocol`` for file content hashing. + Defaults to ``FileHasher(sha256)``. + function_info_extractor: Optional ``FunctionInfoExtractorProtocol``. + Defaults to ``FunctionSignatureExtractor``. + arrow_hasher: Optional ``ArrowHasherProtocol`` for nested table hashing. + When ``None``, lazy resolution via the default context is used. + directory_hasher: Optional ``DirectoryHasherProtocol`` for directory tree hashing. + Defaults to ``BasicDirectoryHasher(sha256)``. + logical_type_registry: Optional ``LogicalTypeRegistry`` forwarded to + ``TypeObjectHandler`` and ``FunctionSignatureExtractor`` for stable + canonical-name resolution. When ``None``, both handlers resolve the + default context's registry lazily at call time. + """ +``` + +Then inside the function body, update the two relevant instantiation lines: + +```python + # Replace the existing FunctionSignatureExtractor instantiation: + if function_info_extractor is None: + from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, + ) + function_info_extractor = FunctionSignatureExtractor( + include_module=True, + include_defaults=True, + logical_type_registry=logical_type_registry, + ) + + # Replace the existing TypeObjectHandler registration line: + registry.register(type, TypeObjectHandler(logical_type_registry=logical_type_registry)) +``` + +All other lines in the function body remain unchanged. + +- [ ] **Step 2: Run the full hashing test suite to confirm nothing is broken** + +```bash +uv run pytest tests/test_hashing/ -v --tb=short 2>&1 | tail -40 +``` + +Expected: All PASS + +- [ ] **Step 3: Commit** + +```bash +git add src/orcapod/hashing/semantic_hashing/builtin_handlers.py +git commit -m "feat(hashing): thread LogicalTypeRegistry through register_builtin_python_type_handlers (ITL-638)" +``` + +--- + +### Task 7: Run golden-diff tests and verify impact is limited to orcapod types + +At this point all three changes are live. The golden-diff tests from Task 2 should now behave correctly. + +- [ ] **Step 1: Run `TestGoldenStability` — builtins must be unchanged** + +```bash +uv run pytest tests/test_hashing/test_type_annotation_golden.py::TestGoldenStability -v +``` + +Expected: All PASS (builtin hashes unchanged — `int`, `str`, `list[int]`, `dict[str, int]`, `int | str`, pure-builtin functions) + +- [ ] **Step 2: Run `TestGoldenCanonical` — orcapod types must have changed** + +```bash +uv run pytest tests/test_hashing/test_type_annotation_golden.py::TestGoldenCanonical -v +``` + +Expected: All PASS (orcapod type hashes differ from golden — `op.File`, `op.Directory`, `op.Path`, `op.UUID` and any function using them) + +- [ ] **Step 3: If any `TestGoldenStability` test fails, investigate before proceeding** + +A failure in `TestGoldenStability` means an unintended hash change. Diagnose by printing the `info` dict from `extract_function_info` for the failing case and comparing to what the golden fixture captured. Fix the root cause; do not adjust the golden file. + +- [ ] **Step 4: Run the full test suite** + +```bash +uv run pytest tests/ -v --tb=short -q 2>&1 | tail -60 +``` + +Expected: All PASS + +- [ ] **Step 5: Commit the final test results note** + +No code change needed here; just record that validation passed. + +--- + +### Task 8: Create PR + +- [ ] **Step 1: Push the branch** + +```bash +git push -u origin eywalker/itl-638-function-pod-signature-hashing-uses-full-type-import-paths +``` + +- [ ] **Step 2: Open the PR** + +```bash +gh pr create \ + --base main \ + --title "fix(hashing): use stable canonical names for type annotation hashing (ITL-638)" \ + --body "$(cat <<'EOF' +## Summary + +Fixes ITL-638. Function pod signature hashing previously encoded types by their fully-qualified import path (e.g. `type:orcapod.logical_types.file_type.File`), causing cache invalidation whenever a type moved between internal modules. + +Changes: +- **`hash_utils.py`**: Extended `canonical_annotation_str` with an optional `LogicalTypeRegistry` parameter. Registered orcapod types resolve to their stable `logical_type_name` (e.g. `"orcapod.file"`). Generic aliases (`list[op.File]`) and union types (`op.File | None`) are recursed so nested types are also canonicalized. +- **`builtin_handlers.py`**: `TypeObjectHandler` now accepts a `LogicalTypeRegistry` (lazy fallback to default context). Registered types return `"type:orcapod.file"` instead of `"type:orcapod.logical_types.file_type.File"`. +- **`function_info_extractors.py`**: `FunctionSignatureExtractor` accepts a `LogicalTypeRegistry` and uses `canonical_annotation_str` for **both** parameter annotation strings and the return annotation (previously stored as a raw type object — now consistently a canonical string). +- **`register_builtin_python_type_handlers`**: Threads `logical_type_registry` through to both handlers. + +**Backward compatibility:** One-time cache invalidation for any function pod whose signature contained `op.File`, `op.Directory`, `op.Path`, or `op.UUID`. This is accepted (pre-v0.1.0, per CLAUDE.md). Builtins and non-orcapod types are unaffected (verified by golden-diff tests). + +## Test plan + +- [ ] `TestGoldenStability` passes — builtin annotation hashes unchanged +- [ ] `TestGoldenCanonical` passes — orcapod type hashes differ from pre-fix golden +- [ ] `TestTypeObjectHandlerWithRegistry` passes — includes module-relocation stability test +- [ ] `TestFunctionSignatureExtractorWithRegistry` passes — params and returns use same canonical form +- [ ] `TestCanonicalAnnotationStrWithRegistry` passes — registry-aware helper works for all annotation forms +- [ ] Full test suite passes + +Fixes ITL-638 +EOF +)" +``` + +--- + +## Self-Review Checklist + +**Spec coverage:** +- ✅ Locate where hashing serializes types → Tasks 3–5 (all three sites) +- ✅ Replace path-based identity with canonical name → TypeObjectHandler (Task 4) + FunctionSignatureExtractor (Task 5) +- ✅ Canonical source of truth → `LogicalTypeRegistry.logical_type_name` threaded via injection +- ✅ Consistent treatment of param and return annotations → Task 5 uses same `canonical_annotation_str` for both +- ✅ Regression tests for module-relocation scenario → `test_simulated_relocation_stable` in Tasks 4 and 5 +- ✅ Golden-value capture + diff assertion → Tasks 1 and 7 +- ✅ Backward-compat decision documented → PR body; one-time bust accepted + +**Placeholder scan:** None found. + +**Type consistency:** +- `canonical_annotation_str(annotation, registry)` — consistent across Tasks 3, 5 +- `TypeObjectHandler(logical_type_registry=...)` — consistent across Tasks 4, 6 +- `FunctionSignatureExtractor(logical_type_registry=...)` — consistent across Tasks 5, 6 +- `register_builtin_python_type_handlers(..., logical_type_registry=...)` — Task 6 only diff --git a/superpowers/specs/2026-09-02-itl-638-stable-type-annotation-hashing-design.md b/superpowers/specs/2026-09-02-itl-638-stable-type-annotation-hashing-design.md new file mode 100644 index 000000000..2f9432145 --- /dev/null +++ b/superpowers/specs/2026-09-02-itl-638-stable-type-annotation-hashing-design.md @@ -0,0 +1,215 @@ +# Stable Type Annotation Hashing Design (ITL-638) + +## Overview + +Function pod signature hashing currently serializes type objects using their fully-qualified +import path (e.g. `"type:orcapod.logical_types.file_type.File"`). Any internal reorganization +of orcapod's own type modules — even a pure move with no semantic change — silently invalidates +all cached function pod signatures that use those types. This design replaces path-based type +identity with stable canonical names drawn from `LogicalTypeRegistry`. + +## Goals & Success Criteria + +- Type annotations in function pod signatures are serialized using the type's `logical_type_name` + from `LogicalTypeRegistry` (e.g. `"orcapod.file"`) rather than its `__module__.__qualname__`. +- Moving a type between internal modules (e.g. `extension_types` → `logical_types`) does **not** + change the hash of any function pod signature, provided the type's `logical_type_name` is + unchanged. +- Parameter annotations are canonicalized at extraction time via `canonical_annotation_str`; + return annotations are stored as raw type objects and canonicalized by `TypeObjectHandler` + at hash time. The end result is stable across module relocations in both positions. +- Builtin types (`int`, `str`, `list[int]`, etc.) and user types not registered in the logical + type registry are unaffected — their hashes remain identical to the pre-fix values. +- A pre-fix golden-value fixture captures current hash values. Post-fix tests assert that only + orcapod logical types changed and everything else is stable. +- One-time cache invalidation for any function pod signature that contained `op.File`, + `op.Directory`, `op.Path`, or `op.UUID` is accepted (pre-v0.1.0 project). + +## Scope & Boundaries + +In scope: + +- `TypeObjectHandler` — resolves bare type objects to canonical names via registry. +- `FunctionSignatureExtractor` — parameter annotation substrings are replaced with canonical + forms via `canonical_annotation_str` at extraction time. Return annotations remain as raw + type objects (`parts["returns"] = sig.return_annotation`); `TypeObjectHandler` (configured + with the same `type_converter`) canonicalises them at hash time. This avoids special-casing + in the extractor and keeps the `"type:"` prefix consistent for all return types. +- `canonical_annotation_str` in `hash_utils.py` — extended with an optional `LogicalTypeRegistry` + parameter and generic-alias recursion. +- `register_builtin_python_type_handlers` — threads `logical_type_registry` through to both + handlers. +- Golden-value test fixture and diff-assertion regression tests. + +Out of scope: + +- Broader redesign of the typing system or `LogicalTypeRegistry`. +- Renaming public `op.*` names — those *should* invalidate caches. +- Other serialization surfaces (persistence, wire format, logging keys) — swept and noted but + not changed here. +- Backward-compatibility shims or grace-period hash migration (pre-v0.1.0 policy). + +## Architecture + +### Canonical name source of truth + +`LogicalTypeRegistry` (in `src/orcapod/logical_types/registry.py`) already maintains a +three-way binding: `(logical_type_name, arrow_extension_name, python_type) → LogicalType`. +Each registered type has a stable `logical_type_name` such as `"orcapod.file"`, +`"orcapod.directory"`, `"orcapod.path"`, `"orcapod.uuid"`. This name is already used as the +stable Arrow extension identifier and is decoupled from the Python module path. It is the +natural source of truth for hashing identity. + +For types not in the registry (builtins, user types), the existing fallback +`"type:{module}.{qualname}"` is preserved unchanged. + +### Registry access pattern + +Both `TypeObjectHandler` and `FunctionSignatureExtractor` accept an optional +`logical_type_registry` constructor argument. When `None`, they resolve the default context's +registry lazily at call time via `get_default_context().logical_type_registry`. This is +identical to the pattern already used by `ArrowTableHandler` and avoids construction-time +circular dependencies. + +``` +register_builtin_python_type_handlers(registry, ..., logical_type_registry=lt_registry) + ├── TypeObjectHandler(logical_type_registry=lt_registry) + └── FunctionSignatureExtractor(logical_type_registry=lt_registry) + └── canonical_annotation_str(annotation, lt_registry) +``` + +### `canonical_annotation_str` extension + +The existing function in `hash_utils.py` only sorts union members for order-independence. It +is extended (backward-compatibly — new optional parameter defaults to `None`) to: + +1. **Registered bare type** (`isinstance(annotation, type)` and found in registry) → return + `lt.logical_type_name` (e.g. `"orcapod.file"`). +2. **Union** (`X | Y`, `Optional[X]`) → recurse each member with registry, sort, join with + `" | "`. Existing behaviour preserved; nested orcapod types now also canonicalized. +3. **Generic alias** (`list[X]`, `dict[K, V]`) → recurse `__origin__` and each `__args__` + member with registry, reconstruct `"origin[arg1, arg2]"` string. +4. **Anything else** → `inspect.formatannotation(annotation)` fallback (unchanged). + +When `registry=None`, the function behaves identically to the pre-fix version. + +### `TypeObjectHandler` change + +```python +# Before +return f"type:{module}.{qualname}" + +# After +lt = registry.get_by_python_type(obj) +if lt is not None: + return f"type:{lt.logical_type_name}" # e.g. "type:orcapod.file" +return f"type:{module}.{qualname}" # unchanged fallback +``` + +### `FunctionSignatureExtractor` change + +**Parameter annotations** — the existing union-only post-processing is generalized: + +```python +# Before: only union types were post-processed +if is_union_annotation(annotation): + old_ann = inspect.formatannotation(annotation) + new_ann = canonical_annotation_str(annotation) + param_str = param_str.replace(f": {old_ann}", f": {new_ann}", 1) + +# After: all annotations are post-processed (no-op when old_ann == new_ann) +old_ann = inspect.formatannotation(annotation) +new_ann = canonical_annotation_str(annotation, registry) +if old_ann != new_ann: + param_str = param_str.replace(f": {old_ann}", f": {new_ann}", 1) +``` + +**Return annotation** — stored as the raw type object, unchanged from before: + +```python +parts["returns"] = sig.return_annotation # raw type object (same as before) +``` + +`TypeObjectHandler` (configured with the same `type_converter`) canonicalises it at hash time +via `type_converter.get_logical_type(obj)`, producing `"type:orcapod.file"` instead of +`"type:orcapod.logical_types.file_type.File"`. The `"type:"` prefix is therefore present +consistently for all return types, registered or not — there is no special-casing in the +extractor. + +**Concrete example.** For `def fn(f: op.File, n: int) -> op.Directory`, `extract_function_info` +currently produces: + +```python +# Before +{ + "module": "mymodule", + "name": "fn", + "params": "f: orcapod.logical_types.file_type.File, n: int", # full module path baked in + "returns": , # raw type object +} +``` + +After the fix: + +```python +# After +{ + "module": "mymodule", + "name": "fn", + "params": "f: orcapod.file, n: int", # canonical name; int unchanged + "returns": , # still a raw type object +} +# TypeObjectHandler then hashes parts["returns"] to "type:orcapod.directory" +``` + +## Affected Hash Values + +| Annotation form | Before fix | After fix | +|---|---|---| +| `int`, `str`, `float`, `bytes` | `type:builtins.int` etc. | **unchanged** | +| `list[int]`, `dict[str, int]` | `list[int]` etc. | **unchanged** | +| `int \| str` | `int \| str` (sorted) | **unchanged** | +| `op.File` | `type:orcapod.logical_types.file_type.File` | `type:orcapod.file` | +| `op.Directory` | `type:orcapod.logical_types.directory_type.Directory` | `type:orcapod.directory` | +| `op.Path` | `type:pathlib.Path` | `type:orcapod.path` | +| `op.UUID` | `type:uuid.UUID` | `type:orcapod.uuid` | +| `list[op.File]` | `list[orcapod.logical_types.file_type.File]` | `list[orcapod.file]` | +| `op.File \| None` | `NoneType \| orcapod.logical_types.file_type.File` | `NoneType \| orcapod.file` | + +Any function pod signature containing any of the "After fix" rows will produce a different +hash, triggering a one-time recompute on next run. This is the intended and accepted outcome. + +## Testing Strategy + +**Pre-fix golden fixture** (`tests/test_hashing/hash_samples/type_annotation_golden.json`): +Generated by a standalone script before any code change. Committed as an immutable record of +the broken state. Captures annotation hashes, `FunctionSignatureExtractor` output hashes, and +full `hash_object(func)` hashes for the full matrix of annotation forms above. + +**`TestGoldenStability`**: After the fix, every builtin and non-orcapod annotation must hash +identically to the golden. Any deviation is an unintended regression. + +**`TestGoldenCanonical`**: After the fix, every orcapod logical type annotation must hash +*differently* from the golden. If any still matches, the fix did not apply correctly. + +**Unit tests per component**: +- `TestCanonicalAnnotationStrWithRegistry` — covers all four branches of the extended function +- `TestTypeObjectHandlerWithRegistry` — includes a module-relocation simulation test (patches + `op.File.__module__` at runtime and verifies hash stability) +- `TestFunctionSignatureExtractorWithRegistry` — verifies canonical form for params and returns, + asserts consistency between the two paths, includes relocation simulation + +## Dependencies & Risks + +- **Circular imports**: `TypeObjectHandler` and `FunctionSignatureExtractor` are in + `orcapod.hashing.*`, which must not import from `orcapod.contexts` at module load time. + Mitigated by the lazy fallback pattern (deferred import inside `_get_registry()`). +- **Registry not yet populated at hash time**: In tests that construct a fresh hasher without + the default context, the lazy fallback will return a fresh empty registry and registered types + will fall back to module-path form. Tests that need canonical names must inject the registry + explicitly. This is correct behavior and is tested. +- **`FunctionInfoExtractorProtocol`**: The protocol in `hashing_protocols.py` defines + `extract_function_info(func, ...) -> dict`. The `"returns"` value remains a raw type + object (or union/generic alias) for all annotations — only `"params"` changes for + functions with orcapod type annotations (the annotation substring is replaced with the + canonical name). `TypeObjectHandler` canonicalises `"returns"` at hash time. diff --git a/tests/test_hashing/generate_type_annotation_golden.py b/tests/test_hashing/generate_type_annotation_golden.py new file mode 100644 index 000000000..112d8f6ed --- /dev/null +++ b/tests/test_hashing/generate_type_annotation_golden.py @@ -0,0 +1,57 @@ +"""Generate pre-fix golden hash values for type annotation hashing. + +Run once (before the ITL-638 fix) with: + uv run python tests/test_hashing/generate_type_annotation_golden.py + +Outputs: tests/test_hashing/hash_samples/type_annotation_golden.json + +Note: ANNOTATION_CASES and FUNCTION_CASES are imported from test_type_annotation_golden +so that function hashes are produced with the correct module path (the test module). +""" +from __future__ import annotations + +import json +import pathlib +import sys + +# Ensure the project root is on sys.path so that `tests.*` imports work when +# this script is run directly (e.g. `uv run python tests/test_hashing/...`). +_PROJECT_ROOT = pathlib.Path(__file__).parent.parent.parent +if str(_PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(_PROJECT_ROOT)) + +from orcapod.hashing.defaults import get_default_semantic_hasher +from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, +) +from tests.test_hashing.test_type_annotation_golden import ( + ANNOTATION_CASES, + FUNCTION_CASES, +) + +GOLDEN_PATH = pathlib.Path(__file__).parent / "hash_samples" / "type_annotation_golden.json" + + +def main() -> None: + hasher = get_default_semantic_hasher() + extractor = FunctionSignatureExtractor(include_module=True, include_defaults=True) + + result: dict = {"annotation_hashes": {}, "function_info_hashes": {}, "function_object_hashes": {}} + + # Hash bare annotations through TypeObjectHandler + for key, ann in ANNOTATION_CASES.items(): + result["annotation_hashes"][key] = hasher.hash_object(ann).to_string() + + # Hash FunctionSignatureExtractor output dict, and full function object + for key, func in FUNCTION_CASES.items(): + info = extractor.extract_function_info(func) + result["function_info_hashes"][key] = hasher.hash_object(info).to_string() + result["function_object_hashes"][key] = hasher.hash_object(func).to_string() + + GOLDEN_PATH.parent.mkdir(parents=True, exist_ok=True) + GOLDEN_PATH.write_text(json.dumps(result, indent=2) + "\n") + print(f"Wrote golden values to {GOLDEN_PATH}") + + +if __name__ == "__main__": + main() diff --git a/tests/test_hashing/hash_samples/schema_hash_golden.json b/tests/test_hashing/hash_samples/schema_hash_golden.json new file mode 100644 index 000000000..cda6c74b8 --- /dev/null +++ b/tests/test_hashing/hash_samples/schema_hash_golden.json @@ -0,0 +1,10 @@ +{ + "Schema({x: int})": "semantic_v0.1:158169963faab1bebfc2362a12c0bcd08a151025297bb82bac32f76fd3919892", + "Schema({x: int, y: str})": "semantic_v0.1:aea243d4bb7e401c0c64522207f569e3cd4e3bbacbd825ee59858a875b20af3a", + "Schema({f: op.File})": "semantic_v0.1:0b6ba627f6df1cc38924fa523ac58eed8794504a3c433ee9996526a6e4d92e99", + "Schema({d: op.Directory})": "semantic_v0.1:4771cda927b245f75922f4adbae627f8b326d37562871d00f56bd8f49cf0913a", + "Schema({p: op.Path})": "semantic_v0.1:339f1894b80c5992fba7f0a9065aaa8192fb9fa858b631a837df0cc3260781b0", + "Schema({x: int, f: op.File})": "semantic_v0.1:5736104378d995bbc5a3b4fb3bbf1d15054a648d1ac357004cbc2a6997265472", + "Schema({f: op.File, d: op.Directory})": "semantic_v0.1:eb2bbcffef31c18e291616e46d1d135363a5ccdd5df4ef1ce3bda123e5f3ad27", + "Schema({f: op.File | None})": "semantic_v0.1:d03ce7c8cce4c8f4b96932cb4d1198d4b29b30e38180f98c93d0dba7fe53aea3" +} diff --git a/tests/test_hashing/hash_samples/type_annotation_golden.json b/tests/test_hashing/hash_samples/type_annotation_golden.json new file mode 100644 index 000000000..9252c82ff --- /dev/null +++ b/tests/test_hashing/hash_samples/type_annotation_golden.json @@ -0,0 +1,37 @@ +{ + "annotation_hashes": { + "int": "semantic_v0.1:24be397c58c33686e96b4f85a9f443fd2483564ec7dee1c14b96da3e60ec16ac", + "str": "semantic_v0.1:309d607de6633b959ae5118171f079b2830f4b1340064fac065652e305b038ad", + "float": "semantic_v0.1:0059c08fc7dfe19dc8d095b7e95d56e8d377bbe49ac30376a00105eca2e7c75f", + "bytes": "semantic_v0.1:f65cea8d1f29ca5b96d67caf1ced2f630b509e32574a7b78b9071958818a1c59", + "op.File": "semantic_v0.1:4d23315b4ab558db547fb9e23e4c2ddfc68602ccd54ab5f13875d76cb30a917a", + "op.Directory": "semantic_v0.1:089ecc3b98bd9289499bfedf61b4129ad7ebc213f75ac81d76e432cb2b9c9d1c", + "op.Path": "semantic_v0.1:0cd349d3a0de173ca0f433ba66905a77254bb482bc895438720bb59c8e686a49", + "op.UUID": "semantic_v0.1:b250d78a250c09a0e88200a76bd4ed3c52201d0207f9abffe713418b0e5aa25a", + "list[int]": "semantic_v0.1:5c376e29b0cfebd85277c71c07c9996afaf06fab35f81681b82f705c9feafa3e", + "dict[str, int]": "semantic_v0.1:f0d36b085fa65f3368d1d98687ffcb6c80099a418419e45a77d434b37e4393dd", + "list[op.File]": "semantic_v0.1:b398d17ead5323e1f1ceeac4846b294fa7d16d18d5a3f79ea0fb570fea7edcce", + "dict[str, op.File]": "semantic_v0.1:f43466ebdb6e8f5eba007c982edaef7a1dd8cecff597fc6f2e2ca3542b49ed54", + "int | str": "semantic_v0.1:99532275ec7a3749a300da6f3f6b7d056eb1a024815308b7557cf86eea9f80b7", + "op.File | None": "semantic_v0.1:7816cc078cf2984dd36907c954daf1faff0272a8f7b66faa1ca5443e0552d8f9", + "Optional[op.File]": "semantic_v0.1:7816cc078cf2984dd36907c954daf1faff0272a8f7b66faa1ca5443e0552d8f9" + }, + "function_info_hashes": { + "fn_no_annotations": "semantic_v0.1:45aaeae8585a01d32864c5bc9b6419cec10c58ac6a4edab012f37fde0125f300", + "fn_builtin_param": "semantic_v0.1:32a86b953b01e9b9b7c0a77a8fe1001b4d2f0a024d24200664eadd1158d4c25f", + "fn_orcapod_param": "semantic_v0.1:f6d172eb51f2e924ef5955fca69d873ce35a7a454d7be20903295cb2dd4cfcc5", + "fn_orcapod_return": "semantic_v0.1:2ec283815474854f4f869f7f426b9b7d1836baa8179a62958f24298a4628e2a2", + "fn_generic_orcapod": "semantic_v0.1:bf9bf058a6b7570e5c9a55a788008d7f12ed310fb4d0e3a433f6ac7a635b5f9b", + "fn_union_orcapod": "semantic_v0.1:f4b17f2b2fd685ac97251175edb9c9a032840187cdfd6560af4ccb483ef7f23f", + "fn_mixed": "semantic_v0.1:cd311a2c94580a0b176784daa422445497a9e305c1bf580b340a71b1b8c3b974" + }, + "function_object_hashes": { + "fn_no_annotations": "semantic_v0.1:45aaeae8585a01d32864c5bc9b6419cec10c58ac6a4edab012f37fde0125f300", + "fn_builtin_param": "semantic_v0.1:32a86b953b01e9b9b7c0a77a8fe1001b4d2f0a024d24200664eadd1158d4c25f", + "fn_orcapod_param": "semantic_v0.1:f6d172eb51f2e924ef5955fca69d873ce35a7a454d7be20903295cb2dd4cfcc5", + "fn_orcapod_return": "semantic_v0.1:2ec283815474854f4f869f7f426b9b7d1836baa8179a62958f24298a4628e2a2", + "fn_generic_orcapod": "semantic_v0.1:bf9bf058a6b7570e5c9a55a788008d7f12ed310fb4d0e3a433f6ac7a635b5f9b", + "fn_union_orcapod": "semantic_v0.1:f4b17f2b2fd685ac97251175edb9c9a032840187cdfd6560af4ccb483ef7f23f", + "fn_mixed": "semantic_v0.1:cd311a2c94580a0b176784daa422445497a9e305c1bf580b340a71b1b8c3b974" + } +} diff --git a/tests/test_hashing/test_function_info_extractors.py b/tests/test_hashing/test_function_info_extractors.py index a89b9d852..c272085a1 100644 --- a/tests/test_hashing/test_function_info_extractors.py +++ b/tests/test_hashing/test_function_info_extractors.py @@ -2,9 +2,13 @@ from __future__ import annotations +import types as _types +import typing +import uuid from pathlib import Path import pytest +import orcapod as op class TestFunctionNameExtractor: @@ -82,19 +86,14 @@ def fn(x: int = 42, y: str = "hi") -> None: pass result = self._make(include_defaults=False).extract_function_info(fn) - # Default values should be stripped - assert "42" not in result["params"] - assert "hi" not in result["params"] - # Parameter names should still be present - assert "x" in result["params"] - assert "y" in result["params"] + assert result["params"] == "x: int, y: str" def test_include_defaults_true_keeps_defaults(self): def fn(x: int = 42) -> None: pass result = self._make(include_defaults=True).extract_function_info(fn) - assert "42" in result["params"] + assert result["params"] == "x: int = 42" def test_return_annotation_present(self): def fn() -> str: @@ -119,7 +118,7 @@ def fn(): assert result["name"] == "overridden" def test_union_annotation_canonicalized(self): - """Union annotations are canonicalized for order stability.""" + """Union annotations are sorted byte-wise for order stability.""" def fn1(x: str | Path) -> None: pass @@ -128,7 +127,60 @@ def fn2(x: Path | str) -> None: r1 = self._make(include_module=False).extract_function_info(fn1, function_name="fn") r2 = self._make(include_module=False).extract_function_info(fn2, function_name="fn") - assert r1["params"] == r2["params"] + assert r1["params"] == "x: pathlib.Path | str" + assert r2["params"] == "x: pathlib.Path | str" + + def test_annotation_containing_equals_preserved_when_defaults_stripped(self): + """Annotations containing ``=`` are not truncated when include_defaults=False. + + The old approach stripped defaults via ``str(param).split('=')[0].strip()``, + which corrupts annotations that contain ``=``. For example + ``Literal["a=b"] = "v"`` would be split to ``Literal["a``. Confirmed that + this test FAILS with the old implementation and PASSES with ``_format_param``, + which checks ``param.default is not inspect.Parameter.empty`` directly. + """ + from typing import Literal + + def fn(x: Literal["a=b"] = "default_value") -> None: + pass + + result = self._make(include_defaults=False).extract_function_info(fn) + # With from __future__ import annotations the annotation is stored as a + # string; eval_str=True cannot resolve Literal from fn's globals, so it + # falls back and formatannotation wraps the string literal in quotes. + # The key property is that the full annotation is present and the default + # value ("default_value") is absent — verified exactly here. + assert "Literal" in result["params"] and "a=b" in result["params"] + assert "default_value" not in result["params"] + # No '=' from the default assignment should survive (annotation may contain + # its own '=' as part of Literal, but the default is stripped cleanly). + assert result["params"].count("=") == result["params"].count("a=b") + + def test_varargs_and_kwargs_with_annotations(self): + """*args and **kwargs with annotations are formatted correctly. + + Verifies that ``_format_param`` produces the right ``*`` / ``**`` prefix + for VAR_POSITIONAL and VAR_KEYWORD parameters. + """ + + def fn(*args: int, **kwargs: str) -> None: + pass + + result = self._make().extract_function_info(fn) + assert result["params"] == "*args: int, **kwargs: str" + + def test_keyword_only_param_formatted_without_star_prefix(self): + """Keyword-only parameters appear without a ``*`` prefix in params. + + The bare ``*`` separator is a signature-level token, not a parameter + attribute; it must not be prepended to keyword-only params. + """ + + def fn(a: int, *, b: str) -> None: + pass + + result = self._make().extract_function_info(fn) + assert result["params"] == "a: int, b: str" def test_eval_str_fallback_on_unresolvable_annotation(self): """When eval_str=True fails, falls back to unresolved signature.""" @@ -168,3 +220,242 @@ def test_default_strategy_is_signature(self): def test_invalid_strategy_raises_value_error(self): with pytest.raises(ValueError, match="Unknown strategy"): self._factory().create_function_info_extractor("invalid_strategy") + + +class TestFunctionSignatureExtractorWithRegistry: + """FunctionSignatureExtractor canonicalizes both param and return annotations.""" + + @pytest.fixture + def extractor(self): + from orcapod.contexts import get_default_context + from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, + ) + return FunctionSignatureExtractor( + include_module=True, + include_defaults=True, + type_converter=get_default_context().type_converter, + ) + + def test_return_annotation_registered_type_is_type_object(self, extractor): + """parts['returns'] is the raw type object even for registered orcapod types. + + TypeObjectHandler (wired with the same type_converter) canonicalises it to + ``"type:orcapod.file"`` at hash time — no special casing in the extractor. + """ + def fn(s: str) -> op.File: + ... + + info = extractor.extract_function_info(fn) + assert not isinstance(info["returns"], str), ( + f"parts['returns'] must not be a string — got {info['returns']!r}" + ) + assert info["returns"] is op.File, ( + f"Expected op.File type object, got {type(info['returns'])}: {info['returns']!r}" + ) + + def test_return_annotation_builtin_keeps_type_object(self, extractor): + """Non-registered return types keep their type-object form (hash unchanged).""" + def fn(x: int) -> float: + ... + + info = extractor.extract_function_info(fn) + assert not isinstance(info["returns"], str), ( + f"parts['returns'] must not be a string — got {info['returns']!r}" + ) + assert info["returns"] is float, ( + f"Expected float type object, got {info['returns']!r}" + ) + + def test_param_annotation_is_canonical_string(self, extractor): + """Parameter annotation for a registered orcapod type uses logical_type_name.""" + def fn(f: op.File) -> str: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "f: orcapod.file" + + def test_generic_param_annotation_canonical(self, extractor): + """list[op.File] in a parameter is canonicalized.""" + def fn(files: list[op.File]) -> str: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "files: list[orcapod.file]" + + def test_union_return_annotation_is_raw_union(self, extractor): + """op.File | None return annotation is stored as a raw union type object. + + TypeObjectHandler handles each union member individually at hash time; + no string conversion happens in the extractor. + """ + def fn(s: str) -> op.File | None: + ... + + info = extractor.extract_function_info(fn) + assert not isinstance(info["returns"], str), ( + f"parts['returns'] must not be a string — got {info['returns']!r}" + ) + assert isinstance(info["returns"], _types.UnionType), ( + f"Expected UnionType, got {type(info['returns'])}: {info['returns']!r}" + ) + + def test_return_annotation_is_never_a_string(self, extractor): + """``parts['returns']`` must never be a plain ``str`` for any annotation shape. + + Regression guard: an earlier implementation converted registered return + types to their ``logical_type_name`` string (e.g. ``"orcapod.file"``) and + stored that in ``parts["returns"]``, losing the ``"type:"`` prefix that + ``TypeObjectHandler`` would normally add. Every case below asserts the + stored value is NOT a str, regardless of whether the annotation contains + a registered orcapod type, a builtin, a union, or a generic alias. + """ + def fn_bare_file(s: str) -> op.File: ... + def fn_bare_directory(s: str) -> op.Directory: ... + def fn_union_file_none(s: str) -> op.File | None: ... + def fn_optional_file(s: str) -> typing.Optional[op.File]: ... # noqa: UP007 + def fn_union_two(s: str) -> op.File | op.Directory: ... + def fn_generic_file(s: str) -> list[op.File]: ... + def fn_float(s: str) -> float: ... + def fn_list_int(s: str) -> list[int]: ... + + cases = [ + (fn_bare_file, "op.File bare registered type"), + (fn_bare_directory, "op.Directory bare registered type"), + (fn_union_file_none, "op.File | None union"), + (fn_optional_file, "typing.Optional[op.File] union"), + (fn_union_two, "op.File | op.Directory multi-registered union"), + (fn_generic_file, "list[op.File] generic wrapping registered type"), + (fn_float, "float builtin"), + (fn_list_int, "list[int] builtin generic"), + ] + + for fn, label in cases: + info = extractor.extract_function_info(fn, function_name="fn") + assert "returns" in info, f"{label}: 'returns' key missing from info dict" + assert not isinstance(info["returns"], str), ( + f"{label}: parts['returns'] must not be a plain string — " + f"TypeObjectHandler is responsible for serialisation, not the extractor. " + f"Got {type(info['returns'])!r}: {info['returns']!r}" + ) + + def test_builtin_annotations_unchanged(self, extractor): + """Functions with only builtin annotations are unaffected. + + In particular, the 'returns' for float stays as the float type object + so existing cached hashes are not invalidated. + """ + def fn(x: int, y: str) -> float: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "x: int, y: str" + assert info["returns"] is float # type object, not string "float" + + def test_param_canonical_string_and_return_type_object(self, extractor): + """op.File in a param uses canonical string; op.File as return is type object. + + Params are embedded in strings so ``canonical_annotation_str`` replaces the + module path inline. Return annotations are raw objects — ``TypeObjectHandler`` + canonicalises them (to ``"type:orcapod.file"``) at hash time. + """ + def fn_param(f: op.File) -> str: + ... + + def fn_return(s: str) -> op.File: + ... + + info_param = extractor.extract_function_info(fn_param) + info_return = extractor.extract_function_info(fn_return) + + assert info_param["params"] == "f: orcapod.file" + assert info_return["returns"] is op.File + + def test_simulated_relocation_stable(self, extractor): + """Patching __module__ on op.File does not change the extracted info.""" + def fn(f: op.File) -> op.File: + ... + + info_before = extractor.extract_function_info(fn) + + original = op.File.__module__ + try: + op.File.__module__ = "orcapod.extension_types.file_type" + info_after = extractor.extract_function_info(fn) + finally: + op.File.__module__ = original + + assert info_before["params"] == info_after["params"] + assert info_before["returns"] == info_after["returns"] + + def test_path_param_annotation_is_canonical_string(self, extractor): + """pathlib.Path in a param annotation is canonicalized to 'orcapod.path'. + + op.Path is pathlib.Path — the same type object registered under + logical_type_name 'orcapod.path'. With the registry wired in, the + extractor must emit the canonical name rather than the module path. + """ + def fn(p: Path) -> str: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "p: orcapod.path" + + def test_uuid_param_annotation_is_canonical_string(self, extractor): + """uuid.UUID in a param annotation is canonicalized to 'orcapod.uuid'. + + op.UUID is uuid.UUID — the same type object registered under + logical_type_name 'orcapod.uuid'. + """ + def fn(u: uuid.UUID) -> str: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "u: orcapod.uuid" + + def test_union_with_path_canonical(self, extractor): + """str | Path union param: Path is canonicalized to 'orcapod.path'. + + Without the registry, str | Path produces 'pathlib.Path | str' (sorting + by the repr string). With the registry, Path maps to 'orcapod.path', + so the output is 'orcapod.path | str' — sort order is preserved since + 'orcapod.path' < 'str' lexicographically. + """ + def fn(x: str | Path) -> None: + ... + + info = extractor.extract_function_info(fn) + assert info["params"] == "x: orcapod.path | str" + + def test_path_return_annotation_type_object_hashes_to_canonical(self, extractor): + """pathlib.Path as return: extractor stores raw type, TypeObjectHandler gives 'type:orcapod.path'. + + Two-step verification: + 1. The extractor stores the raw ``pathlib.Path`` type object in ``parts["returns"]`` + (not a string — TypeObjectHandler, not the extractor, is responsible for serialisation). + 2. When that raw type object is passed through ``TypeObjectHandler`` (wired with the + same ``type_converter``), it produces ``"type:orcapod.path"`` — the stable canonical + form — not ``"type:pathlib.Path"`` (the old module-path form). + """ + from orcapod.contexts import get_default_context + from orcapod.hashing.semantic_hashing.builtin_handlers import TypeObjectHandler + + def fn(s: str) -> Path: + ... + + info = extractor.extract_function_info(fn) + + # Step 1: extractor stores raw type object, never a string + assert info["returns"] is Path, ( + f"Expected pathlib.Path type object in returns, got {info['returns']!r}" + ) + + # Step 2: TypeObjectHandler serializes it to the canonical name + tc = get_default_context().type_converter + handler = TypeObjectHandler(type_converter=tc) + hasher = get_default_context().semantic_hasher + serialized = handler.handle(info["returns"], hasher) + assert serialized == "type:orcapod.path", ( + f"Expected 'type:orcapod.path', got {serialized!r}" + ) + diff --git a/tests/test_hashing/test_hash_utils.py b/tests/test_hashing/test_hash_utils.py index 7de747e0f..6589447d1 100644 --- a/tests/test_hashing/test_hash_utils.py +++ b/tests/test_hashing/test_hash_utils.py @@ -1,6 +1,11 @@ """Tests for hash_utils helpers, specifically canonical union annotation strings.""" import inspect +import typing from pathlib import Path +from uuid import UUID + +import orcapod as op +import pytest from orcapod.hashing.hash_utils import ( canonical_annotation_str, @@ -369,3 +374,67 @@ def my_func(): result = self._get(my_func) assert isinstance(result, list) + + +# --------------------------------------------------------------------------- +# Tests for canonical_annotation_str with registry (ITL-638) +# --------------------------------------------------------------------------- + + +class TestCanonicalAnnotationStrWithRegistry: + """canonical_annotation_str resolves registered logical types to stable names.""" + + @pytest.fixture + def type_converter(self): + from orcapod.contexts import get_default_context + return get_default_context().type_converter + + def test_builtin_type_unchanged(self, type_converter): + assert canonical_annotation_str(int, type_converter) == "int" + + def test_builtin_str_unchanged(self, type_converter): + assert canonical_annotation_str(str, type_converter) == "str" + + def test_registered_file_uses_logical_name(self, type_converter): + result = canonical_annotation_str(op.File, type_converter) + assert result == "orcapod.file" + + def test_registered_directory_uses_logical_name(self, type_converter): + result = canonical_annotation_str(op.Directory, type_converter) + assert result == "orcapod.directory" + + def test_registered_path_uses_logical_name(self, type_converter): + import pathlib + result = canonical_annotation_str(pathlib.Path, type_converter) + assert result == "orcapod.path" + + def test_registered_uuid_uses_logical_name(self, type_converter): + result = canonical_annotation_str(UUID, type_converter) + assert result == "orcapod.uuid" + + def test_generic_list_of_registered_type(self, type_converter): + result = canonical_annotation_str(list[op.File], type_converter) + assert result == "list[orcapod.file]" + + def test_generic_dict_with_registered_value(self, type_converter): + result = canonical_annotation_str(dict[str, op.File], type_converter) + assert result == "dict[str, orcapod.file]" + + def test_union_with_registered_type(self, type_converter): + result = canonical_annotation_str(op.File | None, type_converter) + # Members sorted; NoneType sorts before orcapod.file + assert result == "NoneType | orcapod.file" + + def test_optional_registered_type(self, type_converter): + result = canonical_annotation_str(typing.Optional[op.File], type_converter) + assert result == "NoneType | orcapod.file" + + def test_no_type_converter_fallback(self): + """Without type_converter, falls back to inspect.formatannotation.""" + result = canonical_annotation_str(op.File, None) + assert result == inspect.formatannotation(op.File) + + def test_stable_across_calls(self, type_converter): + r1 = canonical_annotation_str(op.File, type_converter) + r2 = canonical_annotation_str(op.File, type_converter) + assert r1 == r2 diff --git a/tests/test_hashing/test_semantic_hasher.py b/tests/test_hashing/test_semantic_hasher.py index 5d87e43e0..58de7c0f4 100644 --- a/tests/test_hashing/test_semantic_hasher.py +++ b/tests/test_hashing/test_semantic_hasher.py @@ -1472,3 +1472,108 @@ def test_cache_keyed_by_hasher_id_avoids_recomputation(self): own = inner.content_hash() assert own is not first assert own.method == "hasher_a" + + +# --------------------------------------------------------------------------- +# 20. TypeObjectHandler with registry (ITL-638) +# --------------------------------------------------------------------------- + + +class TestTypeObjectHandlerWithRegistry: + """TypeObjectHandler uses stable canonical names for registered logical types.""" + + @pytest.fixture + def type_converter(self): + from orcapod.contexts import get_default_context + return get_default_context().type_converter + + @pytest.fixture + def handler(self, type_converter): + from orcapod.hashing.semantic_hashing.builtin_handlers import TypeObjectHandler + return TypeObjectHandler(type_converter=type_converter) + + def test_registered_file_returns_canonical_name(self, handler, hasher): + import orcapod as op + result = handler.handle(op.File, hasher) + assert result == "type:orcapod.file" + + def test_registered_directory_returns_canonical_name(self, handler, hasher): + import orcapod as op + result = handler.handle(op.Directory, hasher) + assert result == "type:orcapod.directory" + + def test_registered_path_returns_canonical_name(self, handler, hasher): + """pathlib.Path (op.Path) resolves to 'type:orcapod.path', not 'type:pathlib.Path'.""" + from pathlib import Path + result = handler.handle(Path, hasher) + assert result == "type:orcapod.path" + + def test_registered_uuid_returns_canonical_name(self, handler, hasher): + """uuid.UUID (op.UUID) resolves to 'type:orcapod.uuid', not 'type:uuid.UUID'.""" + import uuid + result = handler.handle(uuid.UUID, hasher) + assert result == "type:orcapod.uuid" + + def test_unregistered_type_falls_back_to_module_qualname(self, handler, hasher): + result = handler.handle(int, hasher) + assert result == "type:builtins.int" + + def test_custom_class_falls_back_to_module_qualname(self, handler, hasher): + class _Local: + pass + result = handler.handle(_Local, hasher) + assert "type:" in result + assert "_Local" in result + + def test_no_type_converter_falls_back_to_module_qualname(self, hasher): + """TypeObjectHandler with no type_converter always uses module.qualname.""" + from orcapod.hashing.semantic_hashing.builtin_handlers import TypeObjectHandler + import orcapod as op + handler_plain = TypeObjectHandler() # no type_converter + result = handler_plain.handle(op.File, hasher) + # Without a type_converter, canonical resolution is unavailable + assert result.startswith("type:") + assert "orcapod" in result + assert "File" in result + + def test_simulated_module_relocation_stable(self, type_converter, hasher): + """Relocating a class's __module__ does not change the hash if its + logical_type_name is unchanged in the registry. + + Uses an isolated ``PythonTypeHandlerRegistry`` with the explicit + ``type_converter`` fixture wired into ``TypeObjectHandler`` so this test + does not rely on the global default context's registry state. + """ + from orcapod.hashing.semantic_hashing.builtin_handlers import ( + TypeObjectHandler, + register_builtin_python_type_handlers, + ) + from orcapod.hashing.semantic_hashing.type_handler_registry import ( + PythonTypeHandlerRegistry, + ) + from orcapod.hashing.semantic_hashing.semantic_hasher import ( + SemanticAwarePythonHasher, + ) + import orcapod as op + + reg = PythonTypeHandlerRegistry() + register_builtin_python_type_handlers(reg) + # Explicitly override the type handler with one that has the test + # type_converter wired in, so we test canonical-name resolution. + reg.register(type, TypeObjectHandler(type_converter=type_converter)) + h = SemanticAwarePythonHasher(hasher_id="test_v1", type_handler_registry=reg) + + hash_before = h.hash_object(op.File) + + # Simulate module relocation by temporarily patching __module__ + original_module = op.File.__module__ + try: + op.File.__module__ = "orcapod.extension_types.file_type" + hash_after = h.hash_object(op.File) + finally: + op.File.__module__ = original_module + + assert hash_before == hash_after, ( + "Hash changed when op.File.__module__ was altered — " + "registry lookup is not being used." + ) diff --git a/tests/test_hashing/test_type_annotation_golden.py b/tests/test_hashing/test_type_annotation_golden.py new file mode 100644 index 000000000..02b17828f --- /dev/null +++ b/tests/test_hashing/test_type_annotation_golden.py @@ -0,0 +1,273 @@ +"""Golden-value regression tests for type annotation hashing (ITL-638). + +Three test classes: + TestGoldenStability -- builtins must be UNCHANGED after the fix. + TestGoldenCanonical -- orcapod logical types must produce NEW canonical hashes. + TestSchemaHashStability -- Schema hashes (including orcapod types) must stay stable + after any future code change. + +The annotation/function golden JSON was generated pre-fix by generate_type_annotation_golden.py. +The schema golden JSON (hash_samples/schema_hash_golden.json) was generated post-fix and +locks in the stable canonical hash values. +""" +from __future__ import annotations + +import json +import pathlib +import typing +from uuid import UUID + +import pytest + +import orcapod as op +from orcapod.hashing.defaults import get_default_semantic_hasher +from orcapod.hashing.semantic_hashing.function_info_extractors import ( + FunctionSignatureExtractor, +) +from orcapod.types import Schema + +GOLDEN_PATH = ( + pathlib.Path(__file__).parent / "hash_samples" / "type_annotation_golden.json" +) +SCHEMA_GOLDEN_PATH = ( + pathlib.Path(__file__).parent / "hash_samples" / "schema_hash_golden.json" +) + +# --------------------------------------------------------------------------- +# The same annotation and function cases as the generator — must stay in sync. +# --------------------------------------------------------------------------- + +ANNOTATION_CASES: dict[str, object] = { + "int": int, + "str": str, + "float": float, + "bytes": bytes, + "op.File": op.File, + "op.Directory": op.Directory, + "op.Path": op.Path, + "op.UUID": UUID, # UUID *class* as annotation; routes through TypeObjectHandler, not UUIDHandler + "list[int]": list[int], + "dict[str, int]": dict[str, int], + "list[op.File]": list[op.File], + "dict[str, op.File]": dict[str, op.File], + "int | str": int | str, + "op.File | None": op.File | None, + "Optional[op.File]": typing.Optional[op.File], +} + +# Annotation keys that are expected to change after the fix. +# Any key not in this set must have an UNCHANGED hash. +EXPECTED_CHANGED_KEYS: frozenset[str] = frozenset({ + "op.File", + "op.Directory", + "op.Path", + "op.UUID", + "list[op.File]", + "dict[str, op.File]", + "op.File | None", + "Optional[op.File]", +}) + + +def fn_no_annotations(): + return None + +def fn_builtin_param(x: int, y: str) -> float: + return float(x) + +def fn_orcapod_param(f: op.File) -> str: + return str(f) + +def fn_orcapod_return(s: str) -> op.File: + return op.File(s) # type: ignore[arg-type] + +def fn_generic_orcapod(files: list[op.File]) -> list[str]: + return [] + +def fn_union_orcapod(f: op.File | None) -> op.File | None: + return f + +def fn_mixed(f: op.File, n: int) -> op.Directory: + return op.Directory(str(f)) # type: ignore[arg-type] + +FUNCTION_CASES: dict[str, object] = { + "fn_no_annotations": fn_no_annotations, + "fn_builtin_param": fn_builtin_param, + "fn_orcapod_param": fn_orcapod_param, + "fn_orcapod_return": fn_orcapod_return, + "fn_generic_orcapod": fn_generic_orcapod, + "fn_union_orcapod": fn_union_orcapod, + "fn_mixed": fn_mixed, +} + +# Functions expected to have different hashes after the fix. +EXPECTED_CHANGED_FUNCTIONS: frozenset[str] = frozenset({ + "fn_orcapod_param", + "fn_orcapod_return", + "fn_generic_orcapod", + "fn_union_orcapod", + "fn_mixed", +}) + + +@pytest.fixture(scope="module") +def golden() -> dict: + assert GOLDEN_PATH.exists(), ( + f"Golden file not found: {GOLDEN_PATH}. " + "Run generate_type_annotation_golden.py first." + ) + return json.loads(GOLDEN_PATH.read_text()) + + +@pytest.fixture(scope="module") +def hasher(): + return get_default_semantic_hasher() + + +@pytest.fixture(scope="module") +def extractor(): + from orcapod.contexts import get_default_context + return FunctionSignatureExtractor( + include_module=True, + include_defaults=True, + type_converter=get_default_context().type_converter, + ) + + +class TestGoldenStability: + """Builtins and non-orcapod annotations must hash identically before and after the fix.""" + + def test_builtin_annotation_hashes_unchanged(self, golden, hasher): + stable_keys = {k for k in ANNOTATION_CASES if k not in EXPECTED_CHANGED_KEYS} + mismatches = {} + for key in stable_keys: + ann = ANNOTATION_CASES[key] + current = hasher.hash_object(ann).to_string() + expected = golden["annotation_hashes"][key] + if current != expected: + mismatches[key] = {"expected": expected, "current": current} + assert not mismatches, ( + f"Unexpected hash changes in stable annotations:\n" + + "\n".join(f" {k}: {v}" for k, v in mismatches.items()) + ) + + def test_builtin_function_hashes_unchanged(self, golden, hasher, extractor): + stable_fns = {k for k in FUNCTION_CASES if k not in EXPECTED_CHANGED_FUNCTIONS} + mismatches = {} + for key in stable_fns: + func = FUNCTION_CASES[key] + info = extractor.extract_function_info(func) + current = hasher.hash_object(info).to_string() + expected = golden["function_info_hashes"][key] + if current != expected: + mismatches[key] = {"expected": expected, "current": current} + assert not mismatches, ( + f"Unexpected hash changes in stable functions:\n" + + "\n".join(f" {k}: {v}" for k, v in mismatches.items()) + ) + + +class TestGoldenCanonical: + """orcapod logical types must produce NEW hashes (canonical name, not module path).""" + + def test_orcapod_annotation_hashes_changed(self, golden, hasher): + """After the fix, orcapod type annotation hashes must DIFFER from golden.""" + unchanged = {} + for key in EXPECTED_CHANGED_KEYS: + if key not in ANNOTATION_CASES: + continue + ann = ANNOTATION_CASES[key] + current = hasher.hash_object(ann).to_string() + expected = golden["annotation_hashes"][key] + if current == expected: + unchanged[key] = current + assert not unchanged, ( + f"Expected these annotation hashes to change after the fix, but they didn't:\n" + + "\n".join(f" {k}: {v}" for k, v in unchanged.items()) + ) + + def test_orcapod_function_hashes_changed(self, golden, hasher, extractor): + """After the fix, functions using orcapod types must DIFFER from golden.""" + unchanged = {} + for key in EXPECTED_CHANGED_FUNCTIONS: + func = FUNCTION_CASES[key] + info = extractor.extract_function_info(func) + current = hasher.hash_object(info).to_string() + expected = golden["function_info_hashes"][key] + if current == expected: + unchanged[key] = current + assert not unchanged, ( + f"Expected these function hashes to change after the fix, but they didn't:\n" + + "\n".join(f" {k}: {v}" for k, v in unchanged.items()) + ) + + +# --------------------------------------------------------------------------- +# Schema hash cases — covers both builtin and orcapod logical types. +# All entries are POST-FIX canonical values and must remain stable forever. +# --------------------------------------------------------------------------- + +SCHEMA_CASES: dict[str, Schema] = { + "Schema({x: int})": Schema({"x": int}), + "Schema({x: int, y: str})": Schema({"x": int, "y": str}), + "Schema({f: op.File})": Schema({"f": op.File}), + "Schema({d: op.Directory})": Schema({"d": op.Directory}), + "Schema({p: op.Path})": Schema({"p": op.Path}), + "Schema({x: int, f: op.File})": Schema({"x": int, "f": op.File}), + "Schema({f: op.File, d: op.Directory})": Schema({"f": op.File, "d": op.Directory}), + "Schema({f: op.File | None})": Schema({"f": op.File | None}), +} + + +@pytest.fixture(scope="module") +def schema_golden() -> dict: + assert SCHEMA_GOLDEN_PATH.exists(), ( + f"Schema golden file not found: {SCHEMA_GOLDEN_PATH}. " + "Regenerate it by running: " + "uv run python -c \"\"" + ) + return json.loads(SCHEMA_GOLDEN_PATH.read_text()) + + +class TestSchemaHashStability: + """Schema hashes must stay stable after any code change. + + The golden file (hash_samples/schema_hash_golden.json) was generated + post-fix (ITL-638) and captures the canonical hash for each schema. + Every entry — including schemas that contain orcapod logical types such + as ``op.File`` and ``op.Directory`` — must remain byte-identical across + future refactors. + + If a hash changes unexpectedly, investigate whether: + - ``TypeObjectHandler`` serialization changed for a logical type. + - A logical type's ``logical_type_name`` was renamed. + - The semantic hasher version was bumped (which intentionally changes all hashes). + + To intentionally update the golden values (e.g. after a deliberate hash-scheme + change), recompute and overwrite ``hash_samples/schema_hash_golden.json``. + """ + + def test_schema_hashes_stable(self, schema_golden, hasher): + """Every schema in the golden file must hash to the same value.""" + mismatches = {} + for key, schema in SCHEMA_CASES.items(): + current = hasher.hash_object(schema).to_string() + expected = schema_golden[key] + if current != expected: + mismatches[key] = {"expected": expected, "current": current} + assert not mismatches, ( + "Schema hash values changed — this may indicate an unintended regression " + "in TypeObjectHandler or canonical_annotation_str:\n" + + "\n".join( + f" {k}:\n expected: {v['expected']}\n current: {v['current']}" + for k, v in mismatches.items() + ) + ) + + def test_all_golden_keys_covered(self, schema_golden): + """Every key in the golden file must have a corresponding SCHEMA_CASES entry.""" + missing = set(schema_golden.keys()) - set(SCHEMA_CASES.keys()) + assert not missing, ( + f"Golden file contains keys not covered by SCHEMA_CASES: {missing}\n" + "Add the missing schema(s) to SCHEMA_CASES or regenerate the golden file." + )