From 6e8a3181f35012526f69cbd9cf7ce1eb95dd6427 Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 14:29:45 -0700
Subject: [PATCH 1/6] chore(review): capture pre-v0.2 guided review scaffolding
 (ITL-564)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Leg 0 review annotations (#! / #!?) across system_constants, errors,
types, and config; the canonical review docs under superpowers/reviews/
(force-added past the superpowers/* ignore so the artifact the review
Linear issues reference lives in-repo); a collect() -> Sequence widening
in channels.py; and removal of the src/sample.py scratch stub.

Review scaffolding for the "Orcapod Python: Pre-v0.2 Guided Codebase
Review" project — NOT intended to merge into main as-is; findings are
extracted into clean fix PRs by the synthesis issue (ITL-576).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/channels.py                       |   5 +-
 src/orcapod/config.py                         |  46 +-
 src/orcapod/errors.py                         |  18 +-
 src/orcapod/system_constants.py               |  49 +-
 src/orcapod/types.py                          |  78 +++-
 src/sample.py                                 |   7 -
 .../reviews/pre-v0.2-codebase-review.md       | 157 +++++++
 .../reviews/pre-v0.2-thematic-findings.md     | 440 ++++++++++++++++++
 8 files changed, 768 insertions(+), 32 deletions(-)
 delete mode 100644 src/sample.py
 create mode 100644 superpowers/reviews/pre-v0.2-codebase-review.md
 create mode 100644 superpowers/reviews/pre-v0.2-thematic-findings.md

diff --git a/src/orcapod/channels.py b/src/orcapod/channels.py
index 68cfd028d..8552fb211 100644
--- a/src/orcapod/channels.py
+++ b/src/orcapod/channels.py
@@ -7,7 +7,7 @@
 from __future__ import annotations
 
 import asyncio
-from collections.abc import AsyncIterator
+from collections.abc import AsyncIterator, Sequence
 from dataclasses import dataclass, field
 from typing import Generic, Protocol, TypeVar, runtime_checkable
 
@@ -41,7 +41,6 @@ class ChannelClosed(Exception):
 # Protocol types
 # ---------------------------------------------------------------------------
 
-
 @runtime_checkable
 class ReadableChannel(Protocol[T_co]):
     """Consumer side of a channel."""
@@ -54,7 +53,7 @@ def __aiter__(self) -> AsyncIterator[T_co]: ...
 
     async def __anext__(self) -> T_co: ...
 
-    async def collect(self) -> list[T_co]:
+    async def collect(self) -> Sequence[T_co]:
         """Drain all remaining items into a list."""
         ...
 
diff --git a/src/orcapod/config.py b/src/orcapod/config.py
index 112d06d66..7cf53ab47 100644
--- a/src/orcapod/config.py
+++ b/src/orcapod/config.py
@@ -1,4 +1,18 @@
 # config.py
+#!? SECOND config system: OrcapodConfig (hashing/display/datetime, TOML-loaded, global) is entirely
+#!? separate from the execution configs in types.py (PipelineConfig/PodConfig/NodeConfig). The two
+#!? even use DIFFERENT merge semantics — here "non-DEFAULT value in other overrides"; NodeConfig uses
+#!? "non-NONE in other overrides". A reader must know which convention applies where. Consider
+#!? documenting the split (library/global vs per-execution) and unifying the merge philosophy.
+#! There are legitimately two types of "config" although their interfaces should be as intuitively similar as possible
+#! Namely one set of configs are what would be passed into configure target objects such as Pod, Node, etc. You'd expect
+#! to create these configs and then pass them in. On the other than, there is the second category of config(s) which is
+#! the configuration of Orcapod itself, largely globally. A lot of top level system in Orcapod can optionally take in an
+#! instance of OrcapodConfig to override the default config set globally but canonical usage would be to configure
+#! the entire library globally. We should remove unnecessary duplication/divergence such as disagreeing implementation of
+#! merge but otherwise, we should make it very clear when something is about configuring Orcapod itself, whereas other is
+#! about configuring specific objects/system within Orcapod library. We should consider actually have object-specific config
+#! to reside *together* with the object it is meant to configure. This needs design spike for evaluation.
 from __future__ import annotations
 
 import logging
@@ -10,7 +24,7 @@
 logger = logging.getLogger(__name__)
 
 
-@dataclass(frozen=True)
+@dataclass(frozen=True, slots=True)
 class _SectionConfig:
     """Base class for frozen leaf config section dataclasses.
 
@@ -22,6 +36,16 @@ class _SectionConfig:
     so that ``type(self)()`` produces a valid defaults instance.
     """
 
+    #!? DEAD + subtly broken. `_SectionConfig.merge` is only reached via `OrcapodConfig.merge`,
+    #!? which itself has NO external callers (verified) — `load_config` deliberately avoids it.
+    #!? WHY it's avoided: "non-default overrides" cannot express "reset a field back to its default"
+    #!? (a default in `other` reads as "unset"), which is exactly what `load_config` needs for a
+    #!? higher-precedence file to win. So merge() cannot do the one job the module actually requires.
+    #!? Decide: remove the merge() chain, or redesign it with an explicit "unset" sentinel and route
+    #!? load_config through it (killing the duplication below).
+    #! This provides confusing diverging implementation to merge logic found in some config object found in types.py
+    #! The implementations and inheritance hierarchy (if there is to be any) should be clearned up and
+    #! fully documented to avoid confusion
     def merge(self, other: Self) -> Self:
         """Return a new instance with non-default fields from ``other`` applied.
 
@@ -47,7 +71,7 @@ def merge(self, other: Self) -> Self:
         return replace(self, **updates)
 
 
-@dataclass(frozen=True)
+@dataclass(frozen=True, slots=True)
 class DatetimeConfig(_SectionConfig):
     """Datetime handling policy settings.
 
@@ -69,7 +93,7 @@ class DatetimeConfig(_SectionConfig):
     timezone_policy: Literal["strict", "coerce_utc"] = "strict"
 
 
-@dataclass(frozen=True)
+@dataclass(frozen=True, slots=True)
 class HashingConfig(_SectionConfig):
     """Hash length settings for system-tag column names, schema hashes, and path scoping.
 
@@ -122,6 +146,8 @@ def with_updates(self, **kwargs: Any) -> Self:
         """
         return replace(self, **kwargs)
 
+    #! yet another implementation of merge that should be avoided
+    #! as mentioned above, we should consolidate config merging logic
     def merge(self, other: "OrcapodConfig") -> "OrcapodConfig":
         """Merge with another ``OrcapodConfig``; other takes precedence for non-default values.
 
@@ -169,6 +195,14 @@ def from_dict(
             ``OrcapodConfig`` populated from ``data``; missing sections and fields
             fall back to built-in defaults.
         """
+        #! Indeed this is non-sense duplication -- there ought to be targetted issue(s) to address/refactor
+        #! duplication
+        #!? MASSIVE duplication: the unknown-section-warn + unknown-field-warn + filter block is
+        #!? copy-pasted 3× here (hashing/display/datetime) AND re-implemented again in load_config()
+        #!? below (~150 lines total). Extract one helper `_parse_section(name, raw, cls, path_str)`
+        #!? and have BOTH from_dict and load_config call it. Also: from_dict checks isinstance(...,
+        #!? Mapping) while load_config checks isinstance(..., dict) — pick one. The known-sections set
+        #!? {"hashing","display","datetime"} is hardcoded in 3 places; derive from the dataclass fields.
         known_sections = {"hashing", "display", "datetime"}
         path_str = f" in {source_path}" if source_path is not None else ""
 
@@ -261,6 +295,12 @@ def load_config(
     """
     import tomllib
 
+    #! indeed this should make use of existing methods rather than providing bare implementation again
+    #! furthermore, config should optionally support getting values set through env vars (low priority)
+    #!? Reimplements from_dict()'s per-section parsing instead of reusing it. The "apply only keys
+    #!? explicitly present so a higher-precedence file can reset to default" trick (below) is the real
+    #!? reason merge() is bypassed — see the note on _SectionConfig.merge. If a shared _parse_section
+    #!? helper returned the explicit-keys dict, load_config could layer files via replace() cleanly.
     _user_path = (
         Path(user_config_path)
         if user_config_path is not None
diff --git a/src/orcapod/errors.py b/src/orcapod/errors.py
index 81c653627..f7e2a43a3 100644
--- a/src/orcapod/errors.py
+++ b/src/orcapod/errors.py
@@ -1,3 +1,14 @@
+#!? No common base class. The 14 errors here inherit from a mix of Exception(7)/ValueError(3)/
+#!? RuntimeError(3)/LookupError(1) — a caller cannot `except OrcapodError`. Consider a single
+#!? `OrcapodError(Exception)` root (builtin-semantic subclasses can still multiply-inherit if wanted).
+#!? Also: error classes are NOT all here — ContextValidationError/ContextResolutionError live in
+#!? contexts/core.py, SemanticHashingError in hashing/visitors.py. Decide whether errors.py is the
+#!? single registry (re-export at least) or accept scatter. Not re-exported from top-level __init__.
+#!? MISSING (Area 3 / ITL-557): no execution-failure taxonomy at all — no PodExecutionError / OOM /
+#!? Transient / Retryable types. This is the groundwork the retry+memory-scaling work needs, and the
+#!? reason InvocationStatus (hooks.py) has only a single undifferentiated ERROR bucket.
+#! All errors defined should ultimately inherit from OrcapodError(Exception) and also all exception/error
+#! classes must be present/collected in this module. We do want to add execution-failure taxonomy
 class InputValidationError(Exception):
     """
     Exception raised when the inputs are not valid.
@@ -64,6 +75,8 @@ class SourceSpecMismatchError(ValueError):
 
     The class name ``SourceSpecMismatchError`` is preserved for compatibility
     with any code that catches it by name.
+    #!? "preserved for compatibility" — this project is pre-v0.1 greenfield (CLAUDE.md: no
+    #!? back-compat shims). If nothing external depends on the old name, drop this note / rename freely.
 
     Contains the slot name and a description of the incompatible field(s).
     Raised at ``bind()`` time — schema mismatches are rejected before execution.
@@ -148,7 +161,10 @@ def __init__(self, empty_data: object) -> None:
         super().__init__(
             "content_hash() called on EmptyData with no cached_content_hash. "
             "The pipeline DB row that produced this token is missing the stored "
-            "hash column (OUTPUT_DATA_HASH_COL or INPUT_DATA_HASH_COL). "
+            "hash column (OUTPUT_DATA_HASH_COL). "
+            #!? Misleading: flow-through keys ONLY off OUTPUT_DATA_HASH_COL (function_node.py:2009);
+            #!? INPUT_DATA_HASH_COL is written but never read for flow-through (dead — Area 4 finding).
+            #!? Drop the INPUT reference from this message.
             "Flow-through is unavailable for this row."
         )
 
diff --git a/src/orcapod/system_constants.py b/src/orcapod/system_constants.py
index 204dd9180..153e6335c 100644
--- a/src/orcapod/system_constants.py
+++ b/src/orcapod/system_constants.py
@@ -1,32 +1,58 @@
+##! Add detailed docstring to summarize key categories of constants and how they are used/what do they control
+
+
 # Constants used for source info keys
+#! Must add appropriate comment to clarify the use of each & every constant
 SYSTEM_COLUMN_PREFIX = "__"
 DATAGRAM_PREFIX = "_"
 SOURCE_INFO_PREFIX = "source_"
-POD_ID_PREFIX = "pod_id_"
-PF_VARIATION_PREFIX = "pf_var_"
-PF_EXECUTION_PREFIX = "pf_exec_"
+
+#! Indeed it's not clear what this was meant to be used for -- I'm imagining things that used to be part of the fucntion
+#! data is now captured by the data function's own e.g. variation datagram
+POD_ID_PREFIX = "pod_id_"  #!? dead: 0 uses outside this file (its property below is also unused). Wire or remove.
+PF_VARIATION_PREFIX = "pf_var_" #! Needs comment to clarify what this is -- applies to everything
+PF_EXECUTION_PREFIX = "pf_exec_" #! Very likely PF = packet_function which is actually an old naming for data function
 DATA_CONTEXT_KEY = "context_key"
-INPUT_DATA_HASH_COL = "input_data_hash"
+INPUT_DATA_HASH_COL = "input_data_hash" #! There is constant naming inconsistency -- generally remove "col" and have hash named after it's content
 OUTPUT_DATA_HASH_COL = "output_data_hash"
-DATA_RECORD_ID = "data_id"
 NODE_CONTENT_HASH_COL = "node_content_hash"
+DATA_RECORD_ID = "data_id"  #! Add more explanation as to what this is about
 SYSTEM_TAG_PREFIX_NAME = "tag"
 SYSTEM_TAG_SOURCE_ID_FIELD = "source_id"
 SYSTEM_TAG_RECORD_ID_FIELD = "record_id"
-POD_VERSION = "pod_version"
-EXECUTION_ENGINE = "execution_engine"
-POD_TIMESTAMP = "pod_ts"
+
+#! This is indeed very likely no longer relevant -- corresponding but more refined versioning measure of pod occurs now through versioning of data function
+#! concretely, noting version info of the OSS code, running config, etc
+
+#! This variable is likely no longer relevant -- verify it's not used anywhere and just delete this constant
+POD_VERSION = "pod_version"  #!? dead: intended pod-version system column never wired. Version lives only in the table path + pf_var_* observational cols (Area 2). Decide: wire as a real column or remove.
+
+EXECUTION_ENGINE = "execution_engine"  #!? dead: intended executor-identity column never wired. Executor info only in pf_exec_* observational cols; never in pipeline/tag records (Area 2, X2).
+POD_TIMESTAMP = "pod_ts" #! If this is still used, it's awkward as to why POD_TIMESTAMP is left alone
+
+#! Must add explanation as to what is the semantic differentiation between the field and block separator with concrete examples
 FIELD_SEPARATOR = ":"
 BLOCK_SEPARATOR = "::"
-ENV_INFO = "env_info"
-IS_EPHEMERAL_COL = "is_ephemeral"
+
+#! If completed outdated/not used, we should consider fully dropping this
+ENV_INFO = "env_info"  #!? dead: 0 uses. Env/provenance stamping never implemented (overlaps PLT-1950).
+IS_EPHEMERAL_COL = "is_ephemeral" #! Give short description
+
+#! The following are highly related and should be given its own subsection
 PIPELINE_DB_SCHEMA_VERSION = "pdb_v1"
 RESULT_DB_SCHEMA_VERSION = "rdb_v1"
 TRACKING_DB_SCHEMA_VERSION = "tdb_v1"
 
 
+#! Every constant should be further investigated to ensure that the use of particular prefix
+#! makes sense and it actually happens through the use of proper prefix as provided by the class's property, rather
+#! than skipping and obtaining information directly from the constants as found in this file
+#! E.g., use self.BLOCK_SEPARATOR instead of the bare module-level constant/variable BLOCK_SEPARATOR
 class SystemConstant:
     def __init__(self, global_prefix: str = ""):
+        #!? global_prefix is never set non-empty anywhere — only `constants = SystemConstant()` exists.
+        #!? Inert namespacing hook: all the f-string prefixing below is dead flexibility. Keep only if a
+        #!? multi-tenant/namespaced column scheme is actually planned for v0.2; otherwise simplify.
         self._global_prefix = global_prefix
 
     @property
@@ -114,4 +140,7 @@ def IS_EPHEMERAL_COL(self) -> str:
         return f"{self._global_prefix}{SYSTEM_COLUMN_PREFIX}{IS_EPHEMERAL_COL}"
 
 
+# create a singleton instance for use everywhere
+#! We are going to stick to singleton design IN CASE there will ever be a future where we want to model closely
+#! that cannot be covered by the use of fetch API per se
 constants = SystemConstant()
diff --git a/src/orcapod/types.py b/src/orcapod/types.py
index 9c1ff170c..b5b073fad 100644
--- a/src/orcapod/types.py
+++ b/src/orcapod/types.py
@@ -9,6 +9,9 @@
       a raw digest, with convenience conversions to hex, int, UUID, and base64.
 """
 
+#! The docstring is out of date but generally this module has grown too big
+#! Consider turning it into a subpackage and split into smaller modules
+
 from __future__ import annotations
 
 import logging
@@ -21,6 +24,7 @@
 from types import UnionType
 from typing import TYPE_CHECKING, Any, Generic, Self, TypeAlias, TypeVar
 
+
 if TYPE_CHECKING:
     import pyarrow as pa
 
@@ -33,11 +37,13 @@
 logger = logging.getLogger(__name__)
 
 # TODO: revisit and consider a way to incorporate older Union type
+#! Turn this into an actionable issue
 DataType: TypeAlias = type | UnionType  # | type[Union]
 """A Python type or union of types used to describe the data type of a single
 field within a ``Schema``."""
 
 # TODO: accomodate other Path-like objects
+#! Consider expanding this to include UPath - should be a spike issue inclusive of evaluation
 PathLike: TypeAlias = str | os.PathLike
 """Convenience alias for any filesystem-path-like object (``str`` or
 ``os.PathLike``)."""
@@ -47,18 +53,27 @@
 arbitrarily nested collection thereof. Tags are used to label and organise
 data and datagrams."""
 
+#! Usage of all these type alias should be analyzed -- if any of them are not used anywehre in the codebase
+#! we should consider removing them
 PathSet: TypeAlias = PathLike | Collection[PathLike | None]
 """A single path or an arbitrarily nested collection of paths (with optional
 ``None`` entries). Used when operations need to address multiple files at
 once, e.g. batch hashing."""
 
+#! This should ideally be more tightly linked to the definition of the support native types and how
+#! it relates to arrow data types -- consider moving this along with related arrow-related definitions
+#! into their own module
 SupportedNativePythonData: TypeAlias = str | int | float | bool | bytes | date | datetime
 """The simple Python scalar types that have a direct Arrow / Polars
 correspondence, including temporal types (date and datetime)."""
 
+#! Utility of this extension is questionable -- subject to usage analyses and simplification
 ExtendedSupportedPythonData: TypeAlias = SupportedNativePythonData | PathSet
 """Native scalar types extended with filesystem paths."""
 
+#! Consider drastically simplifying this definition or at least try to make this capable of
+#! working with arbitrary Python types as we now support arbitrary classes through the extension
+#! system.
 DataValue: TypeAlias = ExtendedSupportedPythonData | Collection["DataValue"] | None
 """The universe of values that can appear in a data column -- scalars,
 paths, arbitrarily nested collections, or ``None``."""
@@ -73,12 +88,23 @@
 
 _T = TypeVar("_T")
 
+#! this should be moved into its own module for clarity
 class Schema(Mapping[str, DataType]):
     """Immutable schema representing a mapping of field names to Python types.
 
     Serves as the canonical internal schema representation in OrcaPod,
-    with interop to/from Arrow schemas. Hashable and suitable for use
+    with interop to/from Arrow schemas. Semantic hasher hashable and suitable for use
     in content-addressable contexts.
+    #! This should be fixed such that __hash__ is correctly implemented to yield
+    #! properly hashable data type -- till that is achieved, however, we should
+    #! actively raise an error whenever someone (system) tries to use this in hashable
+    #! context. Till this is implemented, adding qualifier that this is semantic hasher
+    #! hashable
+    #!? BUG: NOT hashable. `__eq__` is overridden below without a `__hash__`, so Python set
+    #!? `Schema.__hash__ = None` → `hash(Schema(...))`, `{schema}`, dict-keying all raise TypeError
+    #!? (verified). Either add `__hash__` (safe — Schema is effectively immutable: hash on
+    #!? (frozenset(self._data.items()), self._optional)) or correct this docstring. If "hashable"
+    #!? here means "content-hashable via semantic hashers" (not Python hash()), say that explicitly.
 
     Args:
         fields: An optional mapping of field names to their data types.
@@ -138,6 +164,12 @@ def __eq__(self, other: object) -> bool:
             return self._data == other._data and self._optional == other._optional
         if isinstance(other, Mapping):
             return self._data == dict(other)
+        #!? BUG: should `return NotImplemented` (the singleton), NOT raise. Raising means
+        #!? `schema == 5` throws instead of being False, and `schema != 5` throws too (verified).
+        #!? Breaks `in` checks, unittest/pytest equality, and any generic comparison. One-line fix.
+        #! Is it more proper to return NotImplemented? Since I want to signify that this is not available
+        #! for use in context where one triest to hash it, I felt it would be appropriate to raise an error
+        #! actively
         raise NotImplementedError(
             f"Equality check is not implemented for object of type {type(other)}"
         )
@@ -288,6 +320,7 @@ def empty(cls) -> Schema:
         return cls({})
 
 
+#! Is this even used anymore?
 class OrchestratorType(Enum):
     """Pipeline orchestrator selection.
 
@@ -324,6 +357,15 @@ class PipelineConfig:
             via ``with_options()`` (e.g. ``{"num_cpus": 4}``).
     """
 
+    #!? DEAD CONFIG (X3): `execution_engine` / `execution_engine_opts` are read NOWHERE and
+    #!? `with_options()` is never called from any path. This docstring describes behavior that
+    #!? doesn't happen — pipeline-level executor selection is currently non-functional. Wire it
+    #!? (thread into node execution) or remove both fields + the docstring claims.
+    #!? ALSO (Area 3 / ITL-557): none of PipelineConfig/PodConfig/NodeConfig carry retry or
+    #!? memory-scaling fields (max_retries, mem_mb baseline, scaling factor, ceiling). This is
+    #!? where the Snakemake-style config surface will need to land.
+    #! While fields related to retry would be added, any field that's currently completely unused
+    #! should be removed
     orchestrator: OrchestratorType = OrchestratorType.SYNCHRONOUS
     channel_buffer_size: int = 64
     default_max_concurrency: int | None = None
@@ -341,6 +383,13 @@ class PodConfig:
             ``1`` means sequential (rate-limited APIs, preserves ordering).
     """
 
+    #!? The PodConfig-vs-NodeConfig axis is undocumented and confusing: why is `max_concurrency`
+    #!? on PodConfig but `is_result_ephemeral`/`ignore_schema` on NodeConfig? (Presumably: PodConfig
+    #!? = reusable pod object, NodeConfig = pod-placed-in-a-pipeline.) Document the split explicitly.
+    #!? Also asymmetric: NodeConfig has merge() + resolve_concurrency reads PodConfig, but there is
+    #!? no equivalent resolve for NodeConfig-vs-PipelineConfig. Consider unifying or documenting.
+    #! I agree that having max-concurrency in both PodConfig and NodeConfig is confusing and thus they should
+    #! be clearly named to be different things rather than necessitating explanation of a confusing name collision
     max_concurrency: int | None = None
 
 
@@ -365,6 +414,9 @@ class NodeConfig:
     is_result_ephemeral: bool | None = None
     ignore_schema: tuple[str, ...] | None = None
 
+    #! It doesn't make much sense that this is only available on NodeConfig -- rather we should consider
+    #! having some kind of base class that offers utility methods like this for all configs
+    #! Also all config related types should be grouped into its own module for clarity
     def merge(self, other: "NodeConfig") -> "NodeConfig":
         """Return a new ``NodeConfig`` with ``other``'s non-``None`` fields overriding self.
 
@@ -398,6 +450,8 @@ def merge(self, other: "NodeConfig") -> "NodeConfig":
         )
 
 
+#! The existence of this suggests that concurrency config is unnecessary distributed or that there
+#! are really more than one kind of concurrency that we are talking about
 def resolve_concurrency(
     pod_config: PodConfig, pipeline_config: PipelineConfig
 ) -> int | None:
@@ -418,6 +472,9 @@ def resolve_concurrency(
     return result
 
 
+#! Should this be renamed to something more specific -- it'd be fine for it to be generic name
+#! like what we have if it's used in multiple places but in that case the docstring
+#! should be updated to be NOT specific to operator pod
 class CacheMode(Enum):
     """Controls operator pod caching behaviour.
 
@@ -471,6 +528,9 @@ class ColumnConfig:
         >>> {"meta": True, "source": True}
     """
 
+    #!? Docstring Attributes list is stale: it documents meta/context/source/system_tags/all_info
+    #!? but omits `content_hash` and `sort_by_tags` (both real fields below). Update the docstring.
+    #! Docstring must be updated to correctly reflect the actual set of fields that matter
     meta: bool | Collection[str] = False
     context: bool = False
     source: bool = False  # Only relevant for DataProtocol
@@ -498,6 +558,8 @@ def data_only(cls) -> Self:
         return cls()
 
     # TODO: consider renaming this to something more intuitive
+    #! Agreed that this is not sensible name -- consider renaming to something like
+    #! `from` which is a common name for factory method in Rust (though may not be suitable for Python)
     @classmethod
     def handle_config(
         cls, config: Self | dict[str, Any] | None, all_info: bool = False
@@ -515,6 +577,8 @@ def handle_config(
         if all_info:
             return cls.all()
         # TODO: properly handle non-boolean values when using all_info
+        #! Not clear what's meant by "properly handle" -- In Python, we generally do not do runtime type check
+        #! Unless its vital
 
         if config is None:
             column_config = cls()
@@ -530,8 +594,7 @@ def handle_config(
 
         return column_config
 
-
-@dataclass(frozen=True)
+@dataclass(frozen=True, slot=True)
 class ColumnInfo:
     """Metadata for a single relational database column with its Arrow-mapped type.
 
@@ -550,7 +613,7 @@ class ColumnInfo:
     arrow_type: pa.DataType
     nullable: bool = True
 
-
+#! move this to its own module
 @dataclass(frozen=True, slots=True)
 class ContentHash:
     """Content-addressable hash pairing a hashing method with a raw digest.
@@ -569,7 +632,8 @@ class ContentHash:
     method: str
     digest: bytes
 
-    # TODO: make the default char count configurable
+    #! TODO: make the default char count configurable but this must be really carefully assessed
+    #! as this directly controls what gets written to the database
     def to_hex(self, char_count: int | None = None) -> str:
         """Convert the digest to a hexadecimal string.
 
@@ -706,7 +770,7 @@ def display_name(self, length: int = 8) -> str:
         return f"{self.method}:{self.to_hex(length)}"
 
 
-@dataclass
+@dataclass(frozen=True, slots=True)
 class Cursor(Generic[_T]):
     """Marks the current position in a DynamicSource's data stream.
 
@@ -788,5 +852,3 @@ def __post_init__(self) -> None:
                 f"PollingConfig.error_backoff_base must be > 0, "
                 f"got {self.error_backoff_base}"
             )
-
-
diff --git a/src/sample.py b/src/sample.py
deleted file mode 100644
index 3c10555b1..000000000
--- a/src/sample.py
+++ /dev/null
@@ -1,7 +0,0 @@
-from collections.abc import Mapping
-
-
-def test() -> Mapping[str, type] | int: ...
-
-
-x = test()
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
new file mode 100644
index 000000000..6ee286540
--- /dev/null
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -0,0 +1,157 @@
+# Orcapod Python — Pre-v0.2 Guided Codebase Review
+
+**The canonical living document for the "Orcapod Python: Pre-v0.2 Guided Codebase Review" Linear
+project.** Every review issue (one per leg) appends its findings here — this file is never
+forked per-issue.
+
+- **Status:** in progress (Leg 0 complete).
+- **Companion:** `pre-v0.2-thematic-findings.md` — the initial 5-anchor-area thematic pass
+  (ITL-564) that seeded this review: cross-reference matrix, cross-cutting themes X1–X3,
+  and the per-area analysis. Findings below cite those (Area 1–5, X1–X3).
+
+## Methodology — guided, agent-led, `#!` / `#!?`
+
+We visit one **leg** (a cluster of used-together modules) at a time, bottom-up so each leg's
+vocabulary supports the next. Per file, the cycle is:
+
+1. **Prep** — the agent reads the leg's modules and verifies claims (fanning out Explore
+   subagents when the leg is large), then lists the exact files.
+2. **Guided pass** — for each file the agent posts a "what it is / what to watch" briefing and
+   inserts inline **`#!?`** annotations at attention points. The reviewer reads the file and
+   marks confirmed problems with **`#!`**. They discuss.
+3. **Enumerate** — the agent appends findings to the table at the bottom of this doc
+   (`file:line`, note, proposed tier, reviewer disposition).
+4. **Wrap** — files checked off below; a leg's issue closes when all its files are done and its
+   findings are logged.
+
+- Conventions: `#!` = reviewer-confirmed problem; `#!?` = agent attention/question. Annotations
+  live on the review branch and are removed once findings are converted to tracked work.
+- **Enumeration first.** Reviews don't chase fixes — but the reviewer may apply a fix in place
+  when it is simple and low-risk (not restricted to docs). Anything larger becomes a follow-up
+  filed by the synthesis issue.
+
+## Review order & estimates
+
+Order = leg order (dependency-driven). Estimate = review effort. Priority = importance to
+review before the v0.2 cut (findings concentration). 🔴 = P0/P1 concentration; 🟡 = P2 cleanup.
+
+| Order | Leg | Issue | Est | Priority |
+|-------|-----|-------|-----|----------|
+| 1 | 0 — Foundations & vocabulary 🟡 | ITL-564 (kickoff) | M | High |
+| 2 | 1 — Type system & semantic conversion | ITL-565 | L | Medium |
+| 3 | 2 — Hashing & identity infra 🟡 | ITL-566 | L | High |
+| 4 | 3 — Protocols | ITL-567 | M | Medium |
+| 5 | 4 — Datagrams 🔴 | ITL-568 | M | High |
+| 6 | 5 — Streams & Sources | ITL-569 | L | Medium |
+| 7 | 6 — Invocation & hooks 🔴 | ITL-570 | S | High |
+| 8 | 7 — Data functions & executors 🔴 | ITL-571 | L | High |
+| 9 | 8 — Nodes (function_node ⚠️) 🔴🔴 | ITL-572 | XL | Urgent |
+| 10 | 9 — Operators | ITL-573 | M | Medium |
+| 11 | 10 — Databases & migrations 🟡 | ITL-574 | L | Medium |
+| 12 | 11 — Pipeline / orchestration / observability 🔴 | ITL-575 | L | High |
+| — | Synthesis → file all findings as tracked work | ITL-576 | M | High |
+
+---
+
+## Leg 0 — Foundations & shared vocabulary  🟡
+Everything downstream imports these. Establish the vocabulary first.
+- [x] `system_constants.py` — column prefixes/separators; **dead constants** `POD_VERSION`, `EXECUTION_ENGINE`, `ENV_INFO`, `POD_ID_PREFIX` (X3) → I-1…I-5
+- [x] `errors.py` — exception taxonomy; note the *absence* of execution/OOM error types (Area 3) → I-6…I-9
+- [x] `types.py` — Schema, ColumnConfig, ContentHash, config dataclasses; **2 verified bugs** (unhashable Schema, `__eq__` raises) + dead exec config (X3) + missing retry fields (Area 3) → I-10…I-14
+- [x] `config.py` — 2nd (global/TOML) config system; dead+broken merge; heavy duplication → I-17…I-19
+- [ ] `utils/lazy_module.py`, `utils/name.py`, `utils/git_utils.py`, `utils/function_info.py`, `utils/object_spec.py`
+
+## Leg 1 — Type system & semantic conversion (Python ↔ Arrow)
+- [ ] `semantic_types/` — `type_inference.py`, `universal_converter.py`, `precomputed_converters.py`, `pydata_utils.py`
+- [ ] `extension_types/` — logical types, `file_type.py`/`directory_type.py` (the `op.File`/`op.Dir` from ITL-557 image-rebuild friction), `numpy_type.py`, `pandas_type.py`, `registry.py`, `schema_walker.py`, `type_utils.py`
+- [ ] `contexts/` — `core.py` (DataContext), `registry.py`
+- [ ] `utils/schema_utils.py`, `utils/arrow_utils.py`, `utils/polars_data_utils.py`
+
+## Leg 2 — Hashing & identity infrastructure  🟡
+- [ ] `protocols/hashing_protocols.py` — `PipelineElementProtocol`, `ContentIdentifiableProtocol`
+- [ ] `hashing/semantic_hashing/` — `semantic_hasher.py`, `builtin_handlers.py`, `type_handler_registry.py`, `function_info_extractors.py`, `content_identifiable_mixin.py`
+- [ ] `hashing/` — `arrow_hashers.py`, `file_hashers.py`, `directory_hashers.py`, `hash_cachers.py`/`string_cachers.py`/`postgres_hash_cacher.py`, `versioned_hashers.py`, `defaults.py`, `schema_cleaner.py`, `visitors.py` (FileHasher perf — ITL-519/520/522)
+- [ ] `core/base.py` — `ContentIdentifiableBase`, `PipelineElementBase`, `TraceableBase` — **the two identity chains** (`content_hash` vs `pipeline_hash`)
+
+## Leg 3 — Protocols (the contract map, read before implementations)
+- [ ] `protocols/core_protocols/` — `datagrams.py`, `streams.py`, `sources.py`, `pod.py`, `function_pod.py`, `operator_pod.py`, `side_effect_pod.py`, `data_function.py`, `executor.py`, `trackers.py`, `traceable.py`, `temporal.py`, `labelable.py`
+- [ ] `protocols/database_protocols.py`, `db_connector_protocol.py` — **no transactional/referential primitive** (X1, Area 5)
+- [ ] `protocols/observability_protocols.py` — **phantom `log_cache_hits`** (Area 1)
+- [ ] `protocols/node_protocols.py`, `pipeline_protocols.py`, `semantic_types_protocols.py`
+
+## Leg 4 — Datagrams (core data containers)  🔴
+- [ ] `core/datagrams/datagram.py` — lazy dict↔Arrow backing
+- [ ] `core/datagrams/tag_data.py` — `Tag`, `Data`, **`EmptyData`** (Area 4: inert `empty_source_info`, accessor guards)
+
+## Leg 5 — Streams & Sources
+- [ ] `core/streams/base.py`, `arrow_table_stream.py`
+- [ ] `core/sources/base.py`, `arrow_table_source.py`
+- [ ] `core/sources/derived_source.py` — **strips version/executor metadata on re-expose** (Area 2)
+- [ ] delegating sources: `csv/dict/list/data_frame/delta_table/db_table/sqlite_table/postgresql_table/spiraldb_table_source.py`, `cached_source.py`, `source_proxy.py`, `stream_builder.py`
+- [ ] `core/sources/polling_source.py` — the *only* existing backoff/retry (contrast w/ Area 3), `source_registry.py`
+- [ ] `core/nodes/source_node.py`
+
+## Leg 6 — Invocation identity & hooks (the identity channel)  🔴
+- [ ] `invocation.py` — `InvocationContext`, `invocation_hash` format (Area 1/2)
+- [ ] `hooks.py` — `PostRunPayload`, `RunStats`, **`InvocationStatus` single ERROR bucket** (Area 3), disjoint from structured logs (Area 1)
+
+## Leg 7 — Data functions & executors (compute layer)  🔴
+- [ ] `core/data_function.py` — `uri`/identity, `major_version` coarse control (by design), **missing fine-grained match** (Area 2, finding #5), `get_execution_data` schema drift
+- [ ] `core/data_function_proxy.py`
+- [ ] `core/executors/base.py`, `local.py`, `ray.py`, `capture_wrapper.py` — `with_options` never called (X3), no OOM/retry (Area 3)
+- [ ] `execution_engines/ray_execution_engine.py` — **orphaned `RayEngine`** (X2)
+- [ ] `core/function_pod.py` — `ctx_arg`/`InvocationContext`, post-run hooks, sync/async paths (Areas 1/2/3)
+- [ ] `core/cached_function_pod.py` — write path
+- [ ] `core/result_cache.py` — **`lookup` matches INPUT hash only** (Area 2 match granularity), `store` metadata columns, `RESULT_COMPUTED_FLAG`
+
+## Leg 8 — Nodes (DB-backed execution)  🔴🔴 THE hotspot
+- [ ] `core/operators/static_output_pod.py` — `StaticOutputPod` base, `DynamicPodStream`
+- [ ] `core/nodes/function_node.py` — **the big one**: `_fetch_joined_records` (Area 5 silent drop = P0, Area 4 EmptyData path), `add_pipeline_record` (write ordering X1, dead `INPUT_DATA_HASH_COL`), `execute`/`_process_data_internal` + async twin (Area 3 error_policy divergence)
+- [ ] `core/nodes/operator_node.py` — single inline table (Area 5 N/A), **no-op `set_ephemeral_store`** (Area 4, ITL-509)
+- [ ] `core/tracker.py` — graph-construction tracking (not runtime logging)
+
+## Leg 9 — Operators
+- [ ] `core/operators/base.py` — `UnaryOperator`/`BinaryOperator`/`NonZeroInputOperator`, `argument_symmetry`
+- [ ] `join.py`, `merge_join.py`, `semijoin.py`, `batch.py`, `column_selection.py`, `mappers.py`, `filters.py`, `index.py`, `pick.py`
+
+## Leg 10 — Databases & migrations  🟡
+- [ ] `databases/` — `in_memory_databases.py`, `delta_lake_databases.py`, `noop_database.py`, `connector_arrow_database.py`, `extension_aware_database.py`, `postgresql_connector.py`, `sqlite_connector.py`, `spiraldb_connector.py`, `storage_utils.py`, `file_utils.py`, `utils.py` — **no cross-store transaction** (X1)
+- [ ] `migrations/` — `pipeline_db.py`, `result_db.py`, `types.py` (ITL-535 v0→v1)
+- [ ] `cli/migrate.py`, `cli/warm_cache.py`
+
+## Leg 11 — Pipeline, orchestration & observability  🔴
+- [ ] `pipeline/graph.py`, `dag.py`, `networkx_backend.py` — graph model
+- [ ] `pipeline/base.py` — `Pipeline`, `set_ephemeral_store` fan-out
+- [ ] `pipeline/job.py` — **`run_id` generation** (Area 1 format inconsistency)
+- [ ] `pipeline/sync_orchestrator.py`, `async_orchestrator.py` — **`error_policy` sync/async divergence** (Area 3, P1)
+- [ ] `pipeline/execution_context.py`, `pod_invocation.py`, `result.py`, `serialization.py`
+- [ ] `pipeline/observer.py`, `composite_observer.py`, `logging_observer.py`, `status_observer.py`, `logging_capture.py`, `observability_reader.py` — **structured logs lack record_id/invocation_hash** (Area 1, P1)
+- [ ] `side_effects.py` — invocation-log write via ctx pods
+- [ ] `channels.py`, `extensions.py`
+
+---
+
+## Enumerated issues (grows as we walk)
+
+| ID | File:line | Note | Reviewer tier | Linked finding |
+|----|-----------|------|---------------|----------------|
+| I-1 | system_constants.py:5,16,17,21 | Dead constants + properties: `POD_ID_PREFIX`, `POD_VERSION`, `EXECUTION_ENGINE`, `ENV_INFO` (0 external uses) — intended version/executor/env system columns never wired. Reviewer: POD_VERSION/EXECUTION_ENGINE/ENV_INFO likely obsolete (superseded by DataFunction variation datagram) → verify & delete | _pending (lean delete)_ | X3, Area 2 |
+| I-2 | system_constants.py:29 | `global_prefix` never set non-empty; entire prefixing machinery is inert. Reviewer: keep singleton design deliberately for future modeling | _keep (deliberate)_ | X3 |
+| I-3 | system_constants.py:13,14 | `PF_` = legacy `PacketFunction` naming (confirmed CHANGELOG.md:102-109) baked into on-disk columns `pf_var_`/`pf_exec_`; renaming = schema migration, not cosmetic | _pending_ | Area 2 |
+| I-4 | system_constants.py:31 | `POD_TIMESTAMP` live (result_cache.py:208/282, "latest result wins" ordering) but is really a result-cache column, not a pod column → `POD_` grouping inconsistent | _pending_ | — |
+| I-5 | system_constants.py (whole) | Missing per-constant docstrings/section headers; schema-version trio (pdb/rdb/tdb_v1) should be its own subsection (they're legitimately the only bare-imported constants, correctly unprefixed) | _pending_ | doc-debt |
+| I-6 | errors.py (whole) | No common `OrcapodError` base — 14 errors across Exception/ValueError/RuntimeError/LookupError; can't `except OrcapodError`. **Reviewer: CONFIRMED — add `OrcapodError(Exception)` root, all inherit from it** | _accepted_ | api-design |
+| I-7 | errors.py + contexts/core.py + hashing/visitors.py | Error classes scattered outside errors.py. **Reviewer: CONFIRMED — all error/exception classes must be collected in this module** | _accepted_ | api-design |
+| I-8 | errors.py (missing) | **No execution-failure taxonomy** (no PodExecution/OOM/Transient/Retryable) — groundwork gap for retry. **Reviewer: CONFIRMED — we want to add it** | _accepted (feeds ITL-557)_ | Area 3 |
+| I-9 | errors.py:164, 76 | `EmptyDataHashMissingError` msg cited dead `INPUT_DATA_HASH_COL` → **reviewer FIXED in place**; `SourceSpecMismatchError` "preserved for compatibility" note still open (greenfield) | _partially applied_ | Area 4, cleanup |
+| **I-10** | types.py:93 (Schema) | **BUG (verified): Schema is unhashable** — `__eq__` override w/o `__hash__`. **Reviewer: implement real `__hash__` eventually; until then keep "semantic-hasher-hashable" qualifier + make hashable-context use raise an *informative* error (not bare TypeError)** | _accepted (correctness)_ | new bug |
+| **I-11** | types.py:163 (Schema.__eq__) | **BUG (verified): raises `NotImplementedError` instead of returning `NotImplemented`**. **Reviewer confirmed → return `NotImplemented` singleton** | _accepted → return NotImplemented_ | new bug |
+| I-15 | types.py (whole) | Module too big; docstring stale. **Reviewer: split into a subpackage** — `Schema` own module, Arrow-related type aliases own module | _accepted (refactor follow-up)_ | tech-debt |
+| I-16 | types.py:40-88 (aliases) | Type-alias hygiene: usage-analyze & prune unused aliases; simplify `DataValue` to leverage extension system (arbitrary types); evaluate `UPath` for `PathLike` (spike); convert `DataType` older-Union TODO to an issue | _accepted (follow-ups)_ | tech-debt |
+| I-17 | config.py + types.py | Two config systems (`OrcapodConfig` global/lib vs `PipelineConfig`/`PodConfig`/`NodeConfig` object-config) with divergent merge. **Reviewer: both categories are legitimate — global-lib-config vs object-config — but interfaces should be as similar as possible; make the distinction explicit.** → see I-20 spike | _accepted → I-20_ | X3-adjacent |
+| I-18 | config.py:37 (_SectionConfig.merge), 137 | `OrcapodConfig.merge`/`_SectionConfig.merge` **dead** (no external callers) AND can't express "reset to default". **Reviewer: consolidate config-merge logic across modules; clean up + fully document the inheritance hierarchy.** | _accepted → I-20_ | dead-code |
+| I-19 | config.py:162 (from_dict), 236 (load_config) | ~150 lines of duplicated per-section parse/warn/filter; `load_config` reimplements `from_dict`; Mapping-vs-dict inconsistency; hardcoded section set ×3. **Reviewer: confirmed — targeted refactor issue(s); also add optional env-var config override (low priority).** | _accepted_ | DRY |
+| **I-20** | config.py + types.py (design) | **Config-system unification design spike (reviewer-requested).** Evaluate: (a) two categories = global-lib-config vs object-config, with maximally-similar interfaces; (b) single consolidated merge; (c) **co-locating object-specific config WITH the object it configures**; (d) optional env-var overrides. Needs a spike before refactor. | _accepted (design spike)_ | design |
+| I-12 | types.py:330-331 | Dead `PipelineConfig.execution_engine`/`execution_engine_opts` w/ misleading docstring (`with_options` never called) | _pending_ | X3, Area 2 |
+| I-13 | types.py:335-366 | No retry/memory config fields on any config (Area 3); PodConfig-vs-NodeConfig axis undocumented; only NodeConfig has merge() | _pending_ | Area 3 |
+| I-14 | types.py:474 (ColumnConfig) | Docstring Attributes omits real fields `content_hash`, `sort_by_tags` | _pending_ | doc-drift |
diff --git a/superpowers/reviews/pre-v0.2-thematic-findings.md b/superpowers/reviews/pre-v0.2-thematic-findings.md
new file mode 100644
index 000000000..fd0dbd97a
--- /dev/null
+++ b/superpowers/reviews/pre-v0.2-thematic-findings.md
@@ -0,0 +1,440 @@
+# ITL-564 — Guided codebase review for v0.2 targeted revision/development
+
+**Status:** Draft (findings complete, priorities proposed — pending review)
+**Author:** Edgar Y. Walker (with Claude Code)
+**Date:** 2026-07-22
+**Linear:** [ITL-564](https://linear.app/metamorphic/issue/ITL-564)
+**Branch:** `eywalker/itl-564-spike-guided-codebase-review-to-identify-targeted`
+
+---
+
+## How to read this document
+
+This is a **directed** review anchored on five suspected problem areas (plus cross-cutting
+themes and candidate net-new areas). It is not an open-ended audit. Each anchor area is written
+to the ITL-564 design-axes template: **Current behavior** (cited by file/function) → **Intended
+behavior** (for v0.2 coherence) → **Delta** → **Ripple effects** → **Risk of not fixing** →
+**Priority + rationale**, with rough **sizing** (S/M/L) and **suggested follow-ups**.
+
+Priorities are **proposed**, not final — they assume the v0.2 bar is "ships correct and
+coherent; observability and ergonomics complete enough to run real workloads." Confirm/adjust
+the P0/P1 cutoff against the actual feature-freeze bar.
+
+Tiers: **P0** = blocker (ships broken/unsafe) · **P1** = must-have (coherence gap users hit) ·
+**P2** = nice-to-have · **P3** = defer.
+
+---
+
+## Cross-reference matrix
+
+Four of the five referenced issues are **already merged** — this review is largely an audit of
+landed code plus its unresolved design axes, not speculation about unbuilt features.
+
+| Issue | Topic | Status | How this review touches it |
+|---|---|---|---|
+| [ITL-534](https://linear.app/metamorphic/issue/ITL-534) | `EmptyData` + ephemeral propagation via cached input hash | ✅ Merged (PR #229) | Areas 4 & 5 audit what landed vs. its many deferred design axes |
+| [ITL-535](https://linear.app/metamorphic/issue/ITL-535) | pdb/rdb v0→v1 schema versioning + migration | ✅ Merged (PR #232) | Area 4 (degraded old-format rows), Area 5 |
+| [ITL-544](https://linear.app/metamorphic/issue/ITL-544) | `ctx_arg` side-effect path + ray empty-opts | ✅ Merged (PR #235) | Areas 2 & 3 (ctx_arg identity, Ray baseline) |
+| [ITL-523](https://linear.app/metamorphic/issue/ITL-523) | Post-run hook + `PostRunPayload` | ✅ Merged (PR #226) | Area 1 (this is the observability primitive) |
+| [ITL-557](https://linear.app/metamorphic/issue/ITL-557) | Auto-retry w/ memory scaling | 🕓 Backlog | Area 3 supplies its baseline + delta |
+| ITL-509 | Operator ephemeral support | (referenced) | Area 4 (operator `set_ephemeral_store` no-op) |
+| DESIGN_ISSUES P3/P6/P7 | version threading, match_tier, redundant hashing | logged | Area 2 |
+
+---
+
+## Cross-cutting themes
+
+Three problems recur across multiple anchor areas. They are called out here once and referenced
+from each area rather than repeated.
+
+### X1. Non-atomic two-store write (result-then-tag) — no cross-store transaction
+The result table is written **first** (`ResultCache.store` → `result_database.add_record`,
+`result_cache.py:286`), the pipeline/tag table **second** (`add_pipeline_record`,
+`function_node.py:1385/1404`), the in-memory cache **last** (`function_node.py:1418`). The two
+DBs can be different backends; there is no enclosing transaction. A crash between the writes
+leaves an **orphaned result row** (result exists, no tag row) that is permanent, invisible, and
+never garbage-collected. This is the root cause behind findings in Areas 3, 4, and 5.
+**Proposed: P1, size M.**
+
+### X2. Two parallel Ray implementations
+`core/executors/ray.py::RayExecutor` (id `"ray.v0"`, actually wired into the executor protocol)
+vs. `execution_engines/ray_execution_engine.py::RayEngine` (exported but referenced by no
+pod/node/data-function path). Divergent config surfaces, a maintenance trap, and an ambiguity
+that ITL-557 must resolve before building retry/memory-scaling. **Proposed: P2, size S** (decide
+which survives; likely delete `RayEngine`) — but it is a **prerequisite** for ITL-557.
+
+### X3. Dead / misleading configuration surface
+Multiple config knobs and constants are defined, documented, and never consumed:
+`system_constants.py` `POD_VERSION` (`__pod_version`), `EXECUTION_ENGINE` (`__execution_engine`),
+`ENV_INFO`, `POD_ID_PREFIX` (none referenced outside their definition);
+`PipelineConfig.execution_engine` / `execution_engine_opts` (`types.py:330-331`, read nowhere);
+`executor.with_options(**opts)` (never called from any path); the documented `log_cache_hits`
+observability flag (referenced only in a docstring, `observability_protocols.py:40`, no
+parameter exists). Either wire or remove; today they mislead readers into thinking capabilities
+exist. **Proposed: P2, size S** (sweep).
+
+---
+
+## Area 1 — Data logging structure during pipeline execution
+
+**Current behavior.** There are **two largely disconnected logging systems**, plus a third
+identity channel:
+- **System A — stdlib `logging`.** ~30 modules `getLogger(__name__)`; in the hot path e.g.
+  `function_pod.py:334/558/599`, `data_function.py:988/1015` (cache checks at `INFO`),
+  `cached_function_pod.py:133/182`, `function_node.py:875/1280/1942/1958/2011`,
+  `ray_execution_engine.py:41-166`. F-string messages, no run_id/node/record_id fields, land
+  wherever the host configures handlers (default: nowhere).
+- **System B — structured observability.** `ExecutionObserverProtocol` +
+  `DataExecutionLoggerProtocol` (`protocols/observability_protocols.py:36/51`). `DataLogger.record`
+  (`pipeline/logging_observer.py:93`) writes one Arrow row per data execution to an
+  `execution_logs` table: `_log_id`, `_log_run_id`, `_log_timestamp`, `_log_<field>` (stdout,
+  stderr, python_logs, traceback, success), plus one column per tag key. Orchestrators own
+  run-level hooks + `run_id` (`sync_orchestrator.py:76-129`, `async_orchestrator.py:136-250`);
+  nodes own node/data hooks + logger creation (`function_node.py:1264-1291`); executors are what
+  actually call `logger.record(...)` (`executors/local.py:51/123`, `executors/ray.py:282/306`).
+  `sys.stdout/stderr` + root-logger capture bridges A→B *only when* capture is installed and an
+  observer is active (`pipeline/logging_capture.py:150/181`).
+- **Identity channel (disjoint).** `PostRunPayload` (`hooks.py:74`) carries
+  `record_id_hash = str(output.datagram_uuid)` and `InvocationContext.invocation_hash =
+  "{pipeline_hash}::{record_id_hash}[::{run_id}]"` (`invocation.py:106-127`). This is the only
+  place record_id / invocation_hash exist at runtime — and it never reaches `DataLogger`.
+- **Correlation IDs.** `run_id` is primary; generated as `uuid4().hex[:16]` in `job.py:823` but
+  falls back to full dashed `uuid4()` in the orchestrators (`sync_orchestrator.py:76`) — **two
+  formats for the same field.** `pipeline_uri` is stored by `StatusObserver` but not
+  `LoggingObserver`.
+
+**Intended behavior (v0.2).** A structured execution-log row should be **joinable to the exact
+output record it describes** (via `record_id` / `invocation_hash`), correlated by a single
+canonical `run_id` format, and should cover all node types at a defined granularity. The two
+logging systems should have a clear relationship (diagnostic vs. structured), and structured-log
+write failures should not be silently swallowed.
+
+**Delta.** (1) `DataLogger` rows carry no `record_id`/`invocation_hash`/`node` columns — logs
+can only be heuristically joined to outputs by (tag values + run_id + DB path). (2) The identity
+channel (`PostRunPayload`) and the I/O-capture channel (`pkt_logger.record`) never meet. (3)
+`run_id` has two formats. (4) Operators emit no data-level logs (`operator_node.py` has no
+`create_data_logger`/`on_data_start`). (5) Cache hits skip logging entirely
+(`function_node.py:1270`); the documented `log_cache_hits` knob doesn't exist (X3). (6)
+`DataLogger.record` / `StatusObserver._write_event` swallow DB-write errors via
+`logger.exception` (`logging_observer.py:131`, `status_observer.py:299`).
+
+**Ripple effects.** Adding identity columns touches the observer protocol signature
+(`create_data_logger`/`record`), both observer implementations, both executors' `record` calls,
+the node/pod call sites, and the `PostRunPayload` build site (natural join point,
+`function_pod.py:341-389`). run_id normalization touches `job.py` + both orchestrators.
+
+**Risk of not fixing.** Execution logs that can't be joined to their outputs undercut the whole
+provenance story the post-run hook / EDI work (PLT-1950) is meant to enable — you can log that
+*something* ran but not prove *which record* it produced. Two run_id formats will silently break
+joins across the two structured stores.
+
+**Priority.** **P1** for the record_id/invocation_hash join gap + run_id normalization (size
+**M**); **P2** for operator-log granularity, swallowed-write-errors, and the two-system bridge
+clarity (size **M**); **P3** for the phantom `log_cache_hits` doc fix (size **S**).
+
+**Follow-ups:** *"Thread record_id/invocation_hash into structured execution-log rows"* (P1);
+*"Normalize run_id format across job/orchestrators"* (P1, small); *"Data-level logging for
+operator nodes"* (P2).
+
+---
+
+## Area 2 — Data function + executor information behavior
+
+**Current behavior.** A DataFunction's canonical identity is `uri`
+(`data_function.py:188-195`): `(canonical_function_name, output_data_schema_hash,
+f"v{major_version}", data_function_type_id)`. Both identity chains collapse to it
+(`identity_structure`/`pipeline_identity_structure` both return `uri`). **Function code and git
+hash are computed** (`_function_content_hash` `:467`, `_git_hash` `:450`) **but intentionally
+excluded from identity** — stored only observationally via `get_function_variation_data()` and
+persisted to the result DB `PF_VARIATION_*` columns (`result_cache.py:255-261`). Executor
+identity/config is likewise observational-only: surfaced through `get_execution_data()`
+(`data_function.py:494-517`) into `PF_EXECUTION_*` columns; **never enters any hash and never
+reaches the pipeline/tag table.** The pipeline record (`add_pipeline_record`,
+`function_node.py:1643-1760`) writes `NODE_CONTENT_HASH_COL`, `INPUT/OUTPUT_DATA_HASH_COL`,
+`IS_EPHEMERAL_COL`, etc., but **no** version/executor/variation columns — version info is carried
+only by the **table path** (`uri` embeds `f"v{major_version}"`). `DerivedSource` re-exposes a
+node's records via `origin.get_all_records()` with default column config, stripping all
+version/executor metadata (`derived_source.py:76-90`).
+
+**Intended behavior (v0.2).**
+- **Coarse cache-invalidation via `major_version` is correct and should stay** — it is the user's
+  deliberate control over the granularity of change that triggers recomputation. Code edits
+  *should not* invalidate the cache unless the author bumps the version.
+- **Missing but intended:** an **opt-in fine-grained** control letting a pod author pin matching
+  to the exact function content hash and/or git hash ("treat any code change as a new
+  computation," or "match only this git commit"). The observational hashes are the raw material;
+  the selectable-granularity matching layer is absent.
+- Executor identity/config should be **coherently observable** (consistent schema) and, where it
+  affects results, reachable from stored records — today it's inconsistent and path-only.
+
+**Delta.**
+1. **No fine-grained match granularity (feature gap, not a bug).** `ResultCache.lookup` matches
+   on `INPUT_DATA_HASH_COL` only (`result_cache.py:185-195`); there is no per-pod config to
+   escalate matching to `_function_content_hash`/`_git_hash`. This is the intended-but-missing
+   capability (relates to `match_tier`, DESIGN_ISSUES P6). **NOT** the "stale cache = bug" framing.
+2. **Executor identity absent from identity hash and pipeline record;** dead
+   `__pod_version`/`__execution_engine` constants that were meant to carry it (X3).
+3. **Inconsistent executor metadata schema:** `get_execution_data` hard-codes
+   `executor_info: dict[str,str]` + json-stringifies (`data_function.py:502-526`), diverging from
+   each executor's own `get_executor_data_schema` (e.g. `ray.py:348-361`), which is effectively
+   unused.
+4. **Duplicated identity state:** `DataFunctionWrapper` leaves `_major_version`/`_minor_version`
+   as dead defaulted state (DESIGN_ISSUES P3); `NODE_CONTENT_HASH_COL` is both written to records
+   and excluded from the entry-id preimage as "determined by path" (P7) — two sources of one fact.
+5. `minor_version` parsed then discarded from identity (`data_function.py:149-156`) — consistent
+   with the coarse-control design, but worth an explicit doc note.
+6. Two Ray implementations (X2).
+
+**Ripple effects.** A fine-grained match layer touches `ResultCache.lookup`/`store`, a new
+`PodConfig`/`NodeConfig` granularity field (`types.py`), and docs (`hashing.md`). Wiring executor
+identity touches `add_pipeline_record`, `system_constants.py` (revive or delete the dead
+constants), and `get_execution_data`/executor schemas.
+
+**Risk of not fixing.** The fine-grained control is a genuine capability gap for users who *want*
+code-level cache invalidation on specific pods — without it they must abuse `major_version`. The
+dead config actively misleads (X3). Executor-blind records weaken provenance for the
+distributed/Ray case.
+
+**Priority.** **P1** for the opt-in fine-grained match-granularity feature (size **M** — needs a
+config surface + lookup path + tests). **P2** for executor-identity-in-records + dead-config
+sweep + executor-schema consistency (size **M**). **P3** for minor-version doc note + P3/P7
+cleanup.
+
+**Follow-ups:** *"Opt-in fine-grained cache match granularity (function-content / git hash)"*
+(P1); *"Wire or remove executor/version columns + dead constants"* (P2); *"Consolidate Ray
+implementations"* (P2, = X2).
+
+---
+
+## Area 3 — Pipeline execution / node retry policy
+
+**Current behavior.** **No retry logic exists anywhere** in the pod execution path (`git grep`
+for `max_retries`/`retry_exceptions`/`max_task_retries` → zero hits; the only backoff is
+source-polling, `polling_source.py:664-682`). Ray tasks dispatch with user-supplied
+`_remote_opts` and nothing sets Ray's own `max_retries`, so Ray uses defaults (retry system
+failures, not application errors). Execution path: `orchestrator.run` → `FunctionNode.execute`
+(`function_node.py:1210`, the **only** execution-level `try/except`, `1275-1289`) →
+`_process_data_internal` → `CachedFunctionPod.process_data` → `FunctionPod.process_data` →
+`PythonDataFunction.call` → `executor.execute_callable` (local `local.py:45` `fn(**kwargs)`; Ray
+`ray.py:185` `.remote` + `188` `ray.get`). Every handler catches **bare `Exception`** for
+capture-and-re-raise; none recover. The top-level handler is binary: `error_policy=="fail_fast"`
+re-raises, else (`"continue"`, default) **silently skips the failed row**.
+`InvocationStatus` (`hooks.py:23-35`) has exactly `COMPUTED`/`HIT`/`ERROR` — one undifferentiated
+error bucket, **no failure classification**, no OOM detection. Ray `memory` is a documented but
+unused pass-through (`ray.py:67`).
+
+**Intended behavior (v0.2 / ITL-557).** Snakemake-style: submit at a modest baseline memory, let
+OOM outliers fail, auto-resubmit failed jobs with scaled memory (`base * attempt`), capped by
+max-attempts and a memory ceiling — **only** for memory-class failures, never for bad-input/bug
+failures. Retries visible in logs/stats.
+
+**Delta.** (1) No retry loop / attempt counter anywhere. (2) No failure classification —
+prerequisite for "retry only on OOM." (3) No OOM detection (Ray `OutOfMemoryError` /
+`WorkerCrashedError` not inspected). (4) No per-attempt resource escalation (`with_options` exists
+but is never called and never recomputes memory). (5) **Sync/async divergence** (see below). (6)
+No config surface (`NodeConfig`/`PodConfig`/`PipelineConfig` have no `max_retries`/`mem_mb`/
+scaling fields). (7) Partial-failure ordering risk (X1) — a retry layer must be idempotent w.r.t.
+result-then-tag write ordering.
+
+**Ripple effects.** Retry loop lands in `FunctionNode.execute`/`_process_data_internal` (+ async
+twin); OOM detection in `RayExecutor._handle_worker_error`; new classification exceptions in
+`errors.py`; `InvocationStatus`/`RunStats` extended in `hooks.py`; config fields in `types.py`;
+the `error_policy` surface in both orchestrators becomes a retry policy. Must pick one Ray impl
+(X2).
+
+**Risk of not fixing.** This is the single highest-leverage feature from the ephys field trial —
+memory is the real cluster bottleneck. Shipping v0.2 without it means users keep over-provisioning
+memory and losing parallelism. But note: it is already scoped as its own spike (ITL-557), so the
+review's job is to hand it a clean baseline, not to build it here.
+
+**Priority.** The retry feature itself: **P2 for v0.2** (important, field-driven, but explicitly a
+separate spike — don't let it block the release). The **sync/async error-policy divergence is a
+separate real bug: P1** — the async streaming path always skips-and-continues
+(`function_pod.py:557-561`, `function_node.py:2478-2489`), **ignoring `fail_fast`**, so the same
+pipeline behaves differently sync vs. async. Failure classification groundwork: **P2, size M**
+(unblocks ITL-557).
+
+**Follow-ups:** feed baseline + delta into ITL-557; file *"Async execution path ignores
+`error_policy=fail_fast`"* (P1, size S); *"Execution-failure classification (OOM/transient/
+permanent)"* (P2, prerequisite for ITL-557).
+
+---
+
+## Area 4 — Interface for ephemeral storage
+
+**Current behavior (what actually landed in ITL-534 / PR #229).**
+- **`EmptyData(Data)`** (`datagrams/tag_data.py:483`) — a first-class missing-data token. Carries
+  `cached_content_hash` + optional `empty_source_info`; **every payload accessor raises**
+  `EmptyDataAccessError` (`as_dict`/`as_table`/`keys`/`schema`/`identity_structure`), so it can't
+  be mistaken for a row of nulls. `content_hash()` returns the cached hash or raises
+  `EmptyDataHashMissingError`.
+- **Ephemerality is declared per-node**, not per-pod: `NodeConfig.is_result_ephemeral`
+  (`types.py:365`) + a separately-injected store via `set_ephemeral_store` (`function_node.py:1118`;
+  `Pipeline.set_ephemeral_store` fans out, `pipeline/base.py:141`). **Operators are no-op stubs**
+  (`operator_node.py:589`, deferred to ITL-509) — only function-pod nodes support ephemerality.
+- **Storage lifecycle:** ephemeral results live in whatever `ArrowDatabaseProtocol` is injected;
+  a single pipeline tag table with an `IS_EPHEMERAL_COL` discriminator per row; two separate result
+  DB instances (persistent vs. ephemeral). **No expiry/pruning/retention/TTL exists anywhere** —
+  `grep expir|prune|retention|ttl|evict` finds only docstrings. The code only *reacts* to already-
+  missing data.
+- **Presence/absence reasoning:** `_fetch_joined_records` (`function_node.py:1862`) partitions tag
+  rows by `IS_EPHEMERAL_COL`, inner-joins each against its store, and turns unmatched **ephemeral**
+  rows into `EmptyData` keyed by `OUTPUT_DATA_HASH_COL`. Downstream (`_process_data_internal:1347`)
+  guards on `isinstance(data, EmptyData)`, looks up by cached hash → hit emits cached result, miss
+  raises `EphemeralResultMissingError`.
+
+**Intended behavior (v0.2).** The ephemeral interface should have a coherent lifecycle contract
+(even if expiry is external, the *interface* should state it), symmetric treatment across node
+types (or an explicit "function-pods-only in v0.2" statement), validated provenance (an
+`EmptyData` should be distinguishable from corruption), and a clean cross-session flow-through
+story.
+
+**Delta (impl vs. ITL-534 design).**
+1. **Flow-through key silently changed INPUT→OUTPUT hash** during implementation
+   (`function_node.py:2009`, comment rejects `INPUT_DATA_HASH_COL`). `INPUT_DATA_HASH_COL` is
+   **written but never read** — dead write. Correctness is fine; the dead column + plan/tests
+   drift is confusing. **P2, size S.**
+2. **Downstream tag-row write on `EmptyData` cache-hit was intentionally dropped**
+   (`function_node.py:1344-1346`) because `EmptyData.as_table()` raises. Consequence: a node that
+   serves a cached result for an `EmptyData` input records **no tag row**, so a further-downstream
+   node or a fresh cross-session read **cannot reconstruct the chain**. This is the crux of
+   ITL-534's deferred tag-row-reconstruction extension. **P1** if cross-session ephemeral
+   flow-through is a v0.2 use case; size **M**.
+3. `empty_source_info` is defined but **never populated** (no producer in `src/`) — provenance
+   surface is inert. **P3, size S** (data-model-only as designed).
+4. **No upstream-ephemerality validation:** any `EmptyData` miss raises regardless of whether the
+   upstream was actually declared ephemeral — corruption looks identical to legitimate absence.
+   **P2, size S.**
+5. **No lifecycle/retention interface** (see above) — the contract is silent on who deletes and
+   when. **P2, size M** (at minimum document; ideally a retention hook).
+6. **No read-back verification** and **no cross-store atomicity** (X1) between tag-row and result
+   write.
+7. Old-format rows lacking `OUTPUT_DATA_HASH_COL` → `cached_content_hash=None` + warning; backfill
+   is ITL-535 (merged).
+
+**Ripple effects.** Tag-row reconstruction touches `_process_data_internal` (both sync/async),
+`add_pipeline_record`, and `EmptyData.empty_source_info`. Retention touches a new module (none
+exists under `databases/`). Operator support is ITL-509.
+
+**Risk of not fixing.** Without the downstream tag-row write, ephemeral flow-through works only
+within a single in-memory session — the headline "survive missing intermediate data across runs"
+benefit is not actually realized cross-session. Silent dead `INPUT_DATA_HASH_COL` and inert
+`empty_source_info` are low-risk but confuse future implementers.
+
+**Priority.** **P1**: cross-session tag-row reconstruction on `EmptyData` hit (delta 2). **P2**:
+upstream-ephemerality validation, lifecycle/retention contract, dead-column cleanup. **P3**:
+populate `empty_source_info`.
+
+**Follow-ups:** *"Write reconstructed downstream tag row on EmptyData cache-hit (cross-session
+flow-through)"* (P1); *"Validate EmptyData originates from a declared-ephemeral upstream"* (P2);
+*"Define ephemeral-result lifecycle/retention interface"* (P2); *"Remove dead INPUT_DATA_HASH_COL
+write / reconcile plan"* (P2).
+
+---
+
+## Area 5 — "Tag table present but result table missing" (degraded/mixed state)
+
+**Scope note.** The two-table split exists **only for `FunctionJobNode`**. `OperatorJobNode`
+stores tags + data inline in one table (`operator_node.py:837/885`) and reads with a plain
+`get_all_records` — no join, so the mixed-state problem doesn't arise there. All findings below
+are in `core/nodes/function_node.py`.
+
+**Current behavior.** The single join site is `_fetch_joined_records`
+(`function_node.py:1862-2067`): reads the tag table, partitions by `IS_EPHEMERAL_COL`, inner-joins
+each partition against its result store on `DATA_RECORD_ID`. Miss handling is **asymmetric**:
+- **Persistent tag row, result missing → silently dropped** with only a `logger.warning`
+  ("These inputs will be recomputed", `:1958`). The inner join `:1950` discards unmatched rows.
+  The "will be recomputed" claim holds **only in FULL execute mode**; in
+  `iter_data`/`CACHE_ONLY`/`READ_ONLY`/`get_all_records` there is **no recompute** — the row just
+  vanishes, and a downstream consumer receives a **short stream that looks complete**. This is the
+  primary silent-correctness site.
+- **Ephemeral tag row, result missing → `EmptyData`** (ITL-534, first-class).
+- **Result row with no tag row (orphan) → silently excluded**, undetected, never GC'd (join is
+  inner and tag-driven; no reverse anti-join). Created by the non-atomic write ordering (X1).
+
+**Invariants:** essentially none — no count check, no referential integrity, no orphan detection,
+no cross-table validation (`grep transaction|atomic|referential|integrity|orphan` finds only
+unrelated single-DB rollback code). `get_all_records` returns `None` for both "zero rows" and "all
+rows dropped due to missing results" (`:1836`) — **lossy conflation**.
+
+**Intended behavior (v0.2).** Missing persistent results should be **loud, not silent** —
+either a typed error, or a first-class degraded token (like `EmptyData`) that downstream must
+handle explicitly, never a silently-shortened stream. Orphans should be detectable. The
+empty-vs-degraded distinction should survive the `get_all_records` return.
+
+**Delta.** (1) Asymmetric miss handling — persistent misses silently dropped vs. ephemeral →
+`EmptyData`. (2) No orphan (result-without-tag) detection. (3) No cross-table invariant/count/
+referential check. (4) Non-atomic two-DB write (X1). (5) Lossy `None` return conflates empty vs.
+degraded. (6) `IS_EPHEMERAL_COL` is the sole routing authority — a wrong/missing flag routes a
+recoverable case into the silent-drop path. (7) Degraded state emits only `logger.warning`, no
+structured signal an orchestrator can react to.
+
+**Ripple effects.** Touches `_fetch_joined_records`, `_load_cached_entries`, `get_all_records`,
+`add_pipeline_record`, and the write path (`_process_data_internal` + async twin). A "persistent
+result missing" typed state/error would live in `errors.py`; any transactional/referential
+primitive would need `protocols/database_protocols.py` + a connector (none exists today).
+
+**Risk of not fixing.** **This is the most dangerous finding in the review**: in read-only /
+cache-only mode, lost persistent results produce a silently-shortened, apparently-complete
+stream — wrong results with no error. For a data-provenance/pipeline tool, silent data loss is a
+credibility-critical failure.
+
+**Priority.** **P0** (proposed) for the silent-drop-of-persistent-misses in non-recompute read
+modes — this should fail loudly or emit a degraded token, never silently shorten a stream (size
+**M**). **P1** for cross-store write atomicity (X1) and orphan detection (size **M**). **P2** for
+`get_all_records` empty-vs-degraded distinction and cross-table invariant checks (size **M**).
+
+**Follow-ups:** *"Persistent result missing must not silently drop rows in read-only/cache-only
+modes"* (P0); *"Cross-store write atomicity for result+tag"* (P1, = X1); *"Orphan-result
+detection / consistency check"* (P2).
+
+---
+
+## Candidate net-new focus areas (ITL-564 caps at 2–3 — your call)
+
+1. **Sync/async execution-path divergence.** Beyond the `error_policy` bug (Area 3), the sync and
+   async paths differ in error handling, logging, and cleanup. Worth a dedicated
+   reconciliation pass rather than fixing case-by-case. *Recommend adopting as a focus area.*
+2. **Cross-store write atomicity / transactionality (X1).** Currently folded into Area 5, but it
+   underpins Areas 3, 4, and 5 and may deserve its own design treatment. *Recommend keeping folded
+   unless you want a standalone design.*
+
+---
+
+## Proposed priority summary
+
+| # | Finding | Area | Tier | Size |
+|---|---|---|---|---|
+| 1 | Persistent result miss silently drops rows in read-only/cache-only modes | 5 | **P0** | M |
+| 2 | Cross-store write atomicity (result+tag) / orphan creation (X1) | 5/3/4 | P1 | M |
+| 3 | Structured log rows lack record_id/invocation_hash (can't join logs→outputs) | 1 | P1 | M |
+| 4 | Async path ignores `error_policy=fail_fast` (sync/async divergence) | 3 | P1 | S |
+| 5 | Opt-in fine-grained cache match granularity (fn-content/git hash) | 2 | P1 | M |
+| 6 | Cross-session tag-row reconstruction on EmptyData hit | 4 | P1 | M |
+| 7 | run_id format normalization | 1 | P1 | S |
+| 8 | Failure classification groundwork (unblocks ITL-557) | 3 | P2 | M |
+| 9 | Consolidate two Ray implementations (X2) | 2/3 | P2 | S |
+| 10 | Wire-or-remove dead config/constants (X3) | 2/all | P2 | S |
+| 11 | Upstream-ephemerality validation of EmptyData | 4 | P2 | S |
+| 12 | Ephemeral lifecycle/retention interface | 4 | P2 | M |
+| 13 | Orphan-result detection / cross-table invariants | 5 | P2 | M |
+| 14 | Operator-node data-level logging | 1 | P2 | M |
+| 15 | Dead INPUT_DATA_HASH_COL write cleanup | 4 | P2 | S |
+| 16 | `get_all_records` empty-vs-degraded distinction | 5 | P2 | M |
+| 17 | Auto-retry w/ memory scaling (→ ITL-557) | 3 | P2* | L |
+| 18 | Populate `empty_source_info` / provenance surface | 4 | P3 | S |
+| 19 | Phantom `log_cache_hits` doc fix | 1 | P3 | S |
+
+\* Retry feature is a separate spike (ITL-557); tier reflects v0.2 release priority, not the
+spike's own importance.
+
+---
+
+## Next steps
+
+1. Review/adjust priorities against the real v0.2 feature-freeze bar (especially the single P0
+   and the six P1s).
+2. Decide the two candidate net-new focus areas (adopt / fold / drop).
+3. On approval, file follow-up Linear issues for each P0/P1 (and P2s you want tracked) under
+   *Orcapod Python v0.2 Feature Sprint*, linking back to this doc and cross-referencing
+   ITL-534/535/544/557/523 where relevant. **No issues will be filed until you approve the list.**

From ea25b9a82d6d754eee39b9338036b6e3cbbf0d34 Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 14:38:02 -0700
Subject: [PATCH 2/6] fix(types): correct slot->slots typo breaking import;
 review Leg 0 utils (ITL-564)

- types.py: @dataclass(frozen=True, slot=True) -> slots=True on ColumnInfo;
  the invalid `slot` kwarg raised TypeError at import (broke `import orcapod`).
- Leg 0 utils review annotations (#!?) across name/git_utils/function_info/
  object_spec/lazy_module; findings I-21..I-27 in the canonical review doc.
- Leg 0 complete.

Review scaffolding for the guided review branch (see ITL-564 / PR #243);
not for direct merge to main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/types.py                            |  2 +-
 src/orcapod/utils/function_info.py              |  8 +++++++-
 src/orcapod/utils/git_utils.py                  |  3 +++
 src/orcapod/utils/lazy_module.py                |  4 ++++
 src/orcapod/utils/name.py                       | 10 +++++++++-
 src/orcapod/utils/object_spec.py                |  5 +++++
 superpowers/reviews/pre-v0.2-codebase-review.md |  9 ++++++++-
 7 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/orcapod/types.py b/src/orcapod/types.py
index b5b073fad..a4a1f674f 100644
--- a/src/orcapod/types.py
+++ b/src/orcapod/types.py
@@ -594,7 +594,7 @@ def handle_config(
 
         return column_config
 
-@dataclass(frozen=True, slot=True)
+@dataclass(frozen=True, slots=True)
 class ColumnInfo:
     """Metadata for a single relational database column with its Arrow-mapped type.
 
diff --git a/src/orcapod/utils/function_info.py b/src/orcapod/utils/function_info.py
index c76f8e628..0af41bc76 100644
--- a/src/orcapod/utils/function_info.py
+++ b/src/orcapod/utils/function_info.py
@@ -3,6 +3,11 @@
 from collections.abc import Callable
 
 
+#!? This module produces the function-identity components consumed by data_function.py's
+#!? `_function_content_hash` / signature hash (Area 2, Leg 7). Comment-stripping accuracy here
+#!? affects that hash. `_is_in_string` below is a self-described "simplified check" — it does NOT
+#!? handle triple-quoted strings or `#` inside multiline strings, so comment removal can mis-fire
+#!? and change the hashed source. Low-probability but it's on the identity path — worth hardening.
 def _is_in_string(line: str, pos: int) -> bool:
     """Helper to check if a position in a line is inside a string literal."""
     # This is a simplified check - would need proper parsing for robust handling
@@ -141,7 +146,8 @@ def get_function_components(
     include_closure_info: bool = True,
     include_decorators: bool = True,
     include_extended_code_props: bool = True,
-    use_ast_parsing: bool = True,
+    use_ast_parsing: bool = True,  #!? DEAD PARAM: the AST path is commented out below (only the
+    #!? string-fallback runs), so this flag does nothing. Either restore the AST path or drop the param.
     skip_source_for_performance: bool = False,
 ) -> list[str]:
     """
diff --git a/src/orcapod/utils/git_utils.py b/src/orcapod/utils/git_utils.py
index d6ee89430..1e43cbe87 100644
--- a/src/orcapod/utils/git_utils.py
+++ b/src/orcapod/utils/git_utils.py
@@ -60,6 +60,9 @@ def get_git_info(path):
             "repo_root": repo.working_dir,
         }
 
+    #!? Bare `except:` catches KeyboardInterrupt/SystemExit too — use `except Exception:` (or a
+    #!? specific git exception). Also swallowing all errors → git info silently becomes None, which
+    #!? feeds the observational git-hash provenance (Area 2 fine-grained-match feature, I-16).
     except:  # TODO: specify exception
         return None
 
diff --git a/src/orcapod/utils/lazy_module.py b/src/orcapod/utils/lazy_module.py
index 75cf05756..b26d55117 100644
--- a/src/orcapod/utils/lazy_module.py
+++ b/src/orcapod/utils/lazy_module.py
@@ -42,6 +42,10 @@ def _load_module(self) -> ModuleType:
     def __getattr__(self, name: str) -> Any:
         """Get attribute from the wrapped module, loading it if necessary."""
         if name.startswith("_"):
+            #!? Side effect: this also blocks lazily accessing legitimate module dunders like
+            #!? `pa.__version__` / `mod.__all__` (they raise AttributeError instead of loading).
+            #!? Fine for the current pyarrow/polars/git usage, but a lurking gotcha. Consider
+            #!? special-casing known module dunders, or documenting the limitation.
             # Avoid infinite recursion for internal attributes
             raise AttributeError(
                 f"'{self.__class__.__name__}' object has no attribute '{name}'"
diff --git a/src/orcapod/utils/name.py b/src/orcapod/utils/name.py
index 2211ef6d0..81347076f 100644
--- a/src/orcapod/utils/name.py
+++ b/src/orcapod/utils/name.py
@@ -5,6 +5,7 @@
 import re
 
 
+#!? Stale TODO: these functions ARE already in utils/. Remove.
 # TODO: move these functions to util
 def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
     """
@@ -33,7 +34,14 @@ def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
         >>> escape_with_postfix("no_separators", "end")
         'no__separators_end'
     """
-
+    #!? BUG (verified): the postfix delimiter is hardcoded to "_" (`f"_{postfix}"`), NOT `separator`.
+    #!? So docstring examples with separator="-" are WRONG: escape("data-info","temp","-") actually
+    #!? returns 'data--info_temp' (not 'data--info-temp'), and unescape can't recover the postfix.
+    #!? Worse, latent corruption when separator != "_" and a field contains "_":
+    #!? unescape("field1--field2-meta", separator="-") → ('field1-field2-meta', None), not (...,'meta').
+    #!? Fix: use `separator` for the postfix delimiter too (f"{separator}{postfix}"), or fix the docs
+    #!? to state postfix is always "_"-delimited. Also the Returns line ("empty string if postfix is
+    #!? None") is wrong — it returns the escaped field.
     return field.replace(separator, separator * 2) + (f"_{postfix}" if postfix else "")
 
 
diff --git a/src/orcapod/utils/object_spec.py b/src/orcapod/utils/object_spec.py
index e7e22d780..2a538791a 100644
--- a/src/orcapod/utils/object_spec.py
+++ b/src/orcapod/utils/object_spec.py
@@ -2,6 +2,11 @@
 from typing import Any
 
 
+#!? TRUST BOUNDARY: this resolves `{"_class": "module.Cls", "_config": {...}}` specs by importing
+#!? arbitrary modules and instantiating arbitrary classes (importlib + getattr + cls(**config)).
+#!? That's effectively arbitrary-code-execution if a spec ever comes from an untrusted source.
+#!? Used to build DataContext from JSON — fine if those JSONs are user-owned/trusted, but this
+#!? must NEVER parse untrusted input. Worth a docstring warning + confirming the trust model.
 def parse_objectspec(
     obj_spec: Any,
     ref_lut: dict[str, Any] | None = None,
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
index 6ee286540..50778955a 100644
--- a/superpowers/reviews/pre-v0.2-codebase-review.md
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -59,7 +59,7 @@ Everything downstream imports these. Establish the vocabulary first.
 - [x] `errors.py` — exception taxonomy; note the *absence* of execution/OOM error types (Area 3) → I-6…I-9
 - [x] `types.py` — Schema, ColumnConfig, ContentHash, config dataclasses; **2 verified bugs** (unhashable Schema, `__eq__` raises) + dead exec config (X3) + missing retry fields (Area 3) → I-10…I-14
 - [x] `config.py` — 2nd (global/TOML) config system; dead+broken merge; heavy duplication → I-17…I-19
-- [ ] `utils/lazy_module.py`, `utils/name.py`, `utils/git_utils.py`, `utils/function_info.py`, `utils/object_spec.py`
+- [x] `utils/lazy_module.py`, `utils/name.py`, `utils/git_utils.py`, `utils/function_info.py`, `utils/object_spec.py` → I-21…I-27  **(Leg 0 COMPLETE)**
 
 ## Leg 1 — Type system & semantic conversion (Python ↔ Arrow)
 - [ ] `semantic_types/` — `type_inference.py`, `universal_converter.py`, `precomputed_converters.py`, `pydata_utils.py`
@@ -152,6 +152,13 @@ Everything downstream imports these. Establish the vocabulary first.
 | I-18 | config.py:37 (_SectionConfig.merge), 137 | `OrcapodConfig.merge`/`_SectionConfig.merge` **dead** (no external callers) AND can't express "reset to default". **Reviewer: consolidate config-merge logic across modules; clean up + fully document the inheritance hierarchy.** | _accepted → I-20_ | dead-code |
 | I-19 | config.py:162 (from_dict), 236 (load_config) | ~150 lines of duplicated per-section parse/warn/filter; `load_config` reimplements `from_dict`; Mapping-vs-dict inconsistency; hardcoded section set ×3. **Reviewer: confirmed — targeted refactor issue(s); also add optional env-var config override (low priority).** | _accepted_ | DRY |
 | **I-20** | config.py + types.py (design) | **Config-system unification design spike (reviewer-requested).** Evaluate: (a) two categories = global-lib-config vs object-config, with maximally-similar interfaces; (b) single consolidated merge; (c) **co-locating object-specific config WITH the object it configures**; (d) optional env-var overrides. Needs a spike before refactor. | _accepted (design spike)_ | design |
+| **I-21** | types.py:597 (ColumnInfo) | **BLOCKING BUG (verified): `@dataclass(frozen=True, slot=True)` — `slot` should be `slots`; raised `TypeError` at import → `import orcapod` broken.** Slipped into commit 6e8a3181. **FIXED in place** (slots=True). | _fixed in place_ | new bug |
+| **I-22** | utils/name.py:37,68 | **BUG (verified): postfix delimiter hardcoded `_`, ignores `separator` arg** → docstring non-`_` examples wrong + latent data corruption when separator≠`_` and field contains `_`. Also wrong "returns empty string" doc; stale "move to util" TODO; dead commented `get_function_signature` block. | _pending (correctness)_ | new bug |
+| I-23 | utils/git_utils.py:63 | Bare `except:` (catches KeyboardInterrupt/SystemExit) → git info silently None; feeds observational git-hash provenance (I-16 fine-grained match). Use `except Exception`. | _pending_ | robustness |
+| I-24 | utils/function_info.py:144, 6 | `use_ast_parsing` is a **dead param** (AST path commented out; only string fallback runs). `_is_in_string` is a fragile comment-detector (no triple-quote/multiline handling) on the function-identity-hash path (Area 2). | _pending_ | dead-code / robustness |
+| I-25 | utils/object_spec.py:5 | **Trust boundary:** `parse_objectspec` imports+instantiates arbitrary classes from a spec (RCE if input untrusted). Used for DataContext-from-JSON. Add docstring warning + confirm trust model; dead commented block; `_validate_config_for_class` swallows all exceptions. | _pending (security-awareness)_ | security |
+| I-26 | utils/lazy_module.py:44 | `_`-prefixed `__getattr__` guard also blocks lazy access to module dunders (`__version__`/`__all__`). Minor; `Optional`/`str|None` typing inconsistency. | _pending (minor)_ | minor |
+| I-27 | name.py, function_info.py, object_spec.py | Recurring **dead commented-out code** blocks (old `get_function_signature`, AST docstring-removal methods, old `parse_objectspec`). Sweep/remove. | _pending_ | cleanup |
 | I-12 | types.py:330-331 | Dead `PipelineConfig.execution_engine`/`execution_engine_opts` w/ misleading docstring (`with_options` never called) | _pending_ | X3, Area 2 |
 | I-13 | types.py:335-366 | No retry/memory config fields on any config (Area 3); PodConfig-vs-NodeConfig axis undocumented; only NodeConfig has merge() | _pending_ | Area 3 |
 | I-14 | types.py:474 (ColumnConfig) | Docstring Attributes omits real fields `content_hash`, `sort_by_tags` | _pending_ | doc-drift |

From ea360ab70c47551ab285c47031bc859f0e90df1a Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 15:05:32 -0700
Subject: [PATCH 3/6] fix(utils): resolve Leg 0 findings I-22/I-23/I-24
 (ITL-564)

- name.py (I-22): escape/unescape_with_postfix now use `separator` for the
  postfix delimiter instead of a hardcoded "_", fixing data corruption when
  separator != "_" and a field contains "_". Byte-identical for the "_"
  default; docstring corrected. Roundtrips verified for _ / - / : separators.
- git_utils.py (I-23): bare `except:` -> `except Exception:` so
  KeyboardInterrupt/SystemExit propagate (return-None behavior preserved).
- function_info.py (I-24): removed the dead `use_ast_parsing` param, its
  docstring entry, and the commented-out AST branch (only caller passed no
  args). `_is_in_string` left as-is: dormant (comments kept by default) and
  changing comment-stripping would alter function hashes.

Hashing suite: 622 passed. `import orcapod` OK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/utils/function_info.py              | 15 ---------------
 src/orcapod/utils/git_utils.py                  |  8 ++++----
 src/orcapod/utils/name.py                       | 13 +++++++------
 superpowers/reviews/pre-v0.2-codebase-review.md |  6 +++---
 4 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/orcapod/utils/function_info.py b/src/orcapod/utils/function_info.py
index 0af41bc76..776ac8cf2 100644
--- a/src/orcapod/utils/function_info.py
+++ b/src/orcapod/utils/function_info.py
@@ -3,11 +3,6 @@
 from collections.abc import Callable
 
 
-#!? This module produces the function-identity components consumed by data_function.py's
-#!? `_function_content_hash` / signature hash (Area 2, Leg 7). Comment-stripping accuracy here
-#!? affects that hash. `_is_in_string` below is a self-described "simplified check" — it does NOT
-#!? handle triple-quoted strings or `#` inside multiline strings, so comment removal can mis-fire
-#!? and change the hashed source. Low-probability but it's on the identity path — worth hardening.
 def _is_in_string(line: str, pos: int) -> bool:
     """Helper to check if a position in a line is inside a string literal."""
     # This is a simplified check - would need proper parsing for robust handling
@@ -146,8 +141,6 @@ def get_function_components(
     include_closure_info: bool = True,
     include_decorators: bool = True,
     include_extended_code_props: bool = True,
-    use_ast_parsing: bool = True,  #!? DEAD PARAM: the AST path is commented out below (only the
-    #!? string-fallback runs), so this flag does nothing. Either restore the AST path or drop the param.
     skip_source_for_performance: bool = False,
 ) -> list[str]:
     """
@@ -169,7 +162,6 @@ def get_function_components(
         include_closure_info: Whether to include closure variable information
         include_decorators: Whether to include decorator information
         include_extended_code_props: Whether to include extended code properties
-        use_ast_parsing: Whether to use AST-based parsing for robust processing
         skip_source_for_performance: Skip expensive source code operations
 
     Returns:
@@ -214,13 +206,6 @@ def get_function_components(
 
             # Process docstrings and comments
             if not include_docstring or not include_comments:
-                # if use_ast_parsing:
-                #     source = SourceProcessor.remove_docstrings_and_comments_ast(
-                #         source,
-                #         remove_docstrings=not include_docstring,
-                #         remove_comments=not include_comments,
-                #     )
-                # else:
                 source = SourceProcessor._remove_docstrings_and_comments_fallback(
                     source,
                     remove_docstrings=not include_docstring,
diff --git a/src/orcapod/utils/git_utils.py b/src/orcapod/utils/git_utils.py
index 1e43cbe87..737a226ef 100644
--- a/src/orcapod/utils/git_utils.py
+++ b/src/orcapod/utils/git_utils.py
@@ -60,10 +60,10 @@ def get_git_info(path):
             "repo_root": repo.working_dir,
         }
 
-    #!? Bare `except:` catches KeyboardInterrupt/SystemExit too — use `except Exception:` (or a
-    #!? specific git exception). Also swallowing all errors → git info silently becomes None, which
-    #!? feeds the observational git-hash provenance (Area 2 fine-grained-match feature, I-16).
-    except:  # TODO: specify exception
+    except Exception:
+        # Not a git repo, git not installed, or a git call failed — no git info
+        # available. Kept broad on purpose (git backends raise varied errors), but
+        # scoped to Exception so KeyboardInterrupt/SystemExit propagate.
         return None
 
 
diff --git a/src/orcapod/utils/name.py b/src/orcapod/utils/name.py
index 81347076f..79bc1e8a8 100644
--- a/src/orcapod/utils/name.py
+++ b/src/orcapod/utils/name.py
@@ -5,8 +5,6 @@
 import re
 
 
-#!? Stale TODO: these functions ARE already in utils/. Remove.
-# TODO: move these functions to util
 def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
     """
     Escape the field string by doubling separators and optionally append a postfix.
@@ -20,8 +18,9 @@ def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
         separator (str, optional): The separator character to escape and use for
                                  prefixing the postfix. Defaults to "_".
     Returns:
-        str: The escaped string with optional postfix. Returns empty string if
-             fields is provided but postfix is None.
+        str: The escaped string with ``postfix`` appended after a single
+             ``separator`` when ``postfix`` is given, or just the escaped
+             string when ``postfix`` is None.
     Examples:
         >>> escape_with_postfix("field1_field2", "suffix")
         'field1__field2_suffix'
@@ -42,7 +41,9 @@ def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
     #!? Fix: use `separator` for the postfix delimiter too (f"{separator}{postfix}"), or fix the docs
     #!? to state postfix is always "_"-delimited. Also the Returns line ("empty string if postfix is
     #!? None") is wrong — it returns the escaped field.
-    return field.replace(separator, separator * 2) + (f"_{postfix}" if postfix else "")
+    return field.replace(separator, separator * 2) + (
+        f"{separator}{postfix}" if postfix else ""
+    )
 
 
 def unescape_with_postfix(field: str, separator="_") -> tuple[str, str | None]:
@@ -73,7 +74,7 @@ def unescape_with_postfix(field: str, separator="_") -> tuple[str, str | None]:
     """
 
     parts = field.split(separator * 2)
-    parts[-1], *meta = parts[-1].split("_", 1)
+    parts[-1], *meta = parts[-1].split(separator, 1)
     return separator.join(parts), meta[0] if meta else None
 
 
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
index 50778955a..4ddc00de0 100644
--- a/superpowers/reviews/pre-v0.2-codebase-review.md
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -153,9 +153,9 @@ Everything downstream imports these. Establish the vocabulary first.
 | I-19 | config.py:162 (from_dict), 236 (load_config) | ~150 lines of duplicated per-section parse/warn/filter; `load_config` reimplements `from_dict`; Mapping-vs-dict inconsistency; hardcoded section set ×3. **Reviewer: confirmed — targeted refactor issue(s); also add optional env-var config override (low priority).** | _accepted_ | DRY |
 | **I-20** | config.py + types.py (design) | **Config-system unification design spike (reviewer-requested).** Evaluate: (a) two categories = global-lib-config vs object-config, with maximally-similar interfaces; (b) single consolidated merge; (c) **co-locating object-specific config WITH the object it configures**; (d) optional env-var overrides. Needs a spike before refactor. | _accepted (design spike)_ | design |
 | **I-21** | types.py:597 (ColumnInfo) | **BLOCKING BUG (verified): `@dataclass(frozen=True, slot=True)` — `slot` should be `slots`; raised `TypeError` at import → `import orcapod` broken.** Slipped into commit 6e8a3181. **FIXED in place** (slots=True). | _fixed in place_ | new bug |
-| **I-22** | utils/name.py:37,68 | **BUG (verified): postfix delimiter hardcoded `_`, ignores `separator` arg** → docstring non-`_` examples wrong + latent data corruption when separator≠`_` and field contains `_`. Also wrong "returns empty string" doc; stale "move to util" TODO; dead commented `get_function_signature` block. | _pending (correctness)_ | new bug |
-| I-23 | utils/git_utils.py:63 | Bare `except:` (catches KeyboardInterrupt/SystemExit) → git info silently None; feeds observational git-hash provenance (I-16 fine-grained match). Use `except Exception`. | _pending_ | robustness |
-| I-24 | utils/function_info.py:144, 6 | `use_ast_parsing` is a **dead param** (AST path commented out; only string fallback runs). `_is_in_string` is a fragile comment-detector (no triple-quote/multiline handling) on the function-identity-hash path (Area 2). | _pending_ | dead-code / robustness |
+| **I-22** | utils/name.py | **BUG (verified): postfix delimiter hardcoded `_`, ignored `separator`** → corruption for non-`_` separators. **FIXED**: delimiter now follows `separator` (byte-identical for `_`), docstring corrected + verified roundtrips for `_`/`-`/`:`. (Stale TODO + dead commented block left for I-27.) | _fixed + pushed_ | new bug |
+| I-23 | utils/git_utils.py | Bare `except:` caught KeyboardInterrupt/SystemExit. **FIXED**: → `except Exception:` (return None preserved; comment explains breadth). | _fixed + pushed_ | robustness |
+| I-24 | utils/function_info.py | `use_ast_parsing` dead param + commented AST branch. **FIXED**: removed the param, docstring line, and dead branch (only caller `data_function.py:468` passed no args). `_is_in_string` **left as-is** — dormant (comments included by default) and changing comment-stripping would alter function hashes. | _fixed (param); _is_in_string noted_ | dead-code |
 | I-25 | utils/object_spec.py:5 | **Trust boundary:** `parse_objectspec` imports+instantiates arbitrary classes from a spec (RCE if input untrusted). Used for DataContext-from-JSON. Add docstring warning + confirm trust model; dead commented block; `_validate_config_for_class` swallows all exceptions. | _pending (security-awareness)_ | security |
 | I-26 | utils/lazy_module.py:44 | `_`-prefixed `__getattr__` guard also blocks lazy access to module dunders (`__version__`/`__all__`). Minor; `Optional`/`str|None` typing inconsistency. | _pending (minor)_ | minor |
 | I-27 | name.py, function_info.py, object_spec.py | Recurring **dead commented-out code** blocks (old `get_function_signature`, AST docstring-removal methods, old `parse_objectspec`). Sweep/remove. | _pending_ | cleanup |

From 6dc98a13149459db893ab8150d731e13db5db3d2 Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 16:40:49 -0700
Subject: [PATCH 4/6] chore(review): complete Leg 0 utils review; delete dead
 name.py (ITL-564)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Delete src/orcapod/utils/name.py — entirely dead: the module is never
  imported and all 5 functions (escape/unescape_with_postfix,
  find_noncolliding_name, pascal_to_snake, snake_to_pascal) have zero uses
  in src + tests. Full suite green afterward (4611 passed). This also
  removes the earlier I-22 fix, which was a bug in dead code.
- git_utils.py: reviewer restored LazyModule via the TYPE_CHECKING pattern,
  added GitRepoInfo TypedDict + `-> GitRepoInfo | None`.
- Reviewer #!/#!? annotations captured on function_info / lazy_module /
  object_spec; findings I-21..I-29 recorded in the canonical review doc.
- Leg 0 reviewer pass complete.

Follow-ups noted (for synthesis / ITL-576): object_spec trust-boundary
whitelist + docs (dedicated issue); function_info & git_utils test coverage;
lazy_module underscore-guard revisit.

Review scaffolding on the guided-review branch; not for direct merge to main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/utils/function_info.py            |   2 +
 src/orcapod/utils/git_utils.py                |  40 +++--
 src/orcapod/utils/lazy_module.py              |   1 +
 src/orcapod/utils/name.py                     | 168 ------------------
 src/orcapod/utils/object_spec.py              |  29 +--
 .../reviews/pre-v0.2-codebase-review.md       |  12 +-
 6 files changed, 42 insertions(+), 210 deletions(-)
 delete mode 100644 src/orcapod/utils/name.py

diff --git a/src/orcapod/utils/function_info.py b/src/orcapod/utils/function_info.py
index 776ac8cf2..85e888414 100644
--- a/src/orcapod/utils/function_info.py
+++ b/src/orcapod/utils/function_info.py
@@ -2,6 +2,7 @@
 import inspect
 from collections.abc import Callable
 
+#! How comprehensively this is these functions are tested should be fully assessed
 
 def _is_in_string(line: str, pos: int) -> bool:
     """Helper to check if a position in a line is inside a string literal."""
@@ -19,6 +20,7 @@ def _is_in_string(line: str, pos: int) -> bool:
 class SourceProcessor:
     """Handles AST-based and fallback source code processing."""
 
+    #! What's the use of this section? Why do we only have *_fallback version that's in use?
     # @staticmethod
     # def remove_docstrings_and_comments_ast(
     #     source: str, remove_docstrings: bool = True, remove_comments: bool = True
diff --git a/src/orcapod/utils/git_utils.py b/src/orcapod/utils/git_utils.py
index 737a226ef..0ae4516a2 100644
--- a/src/orcapod/utils/git_utils.py
+++ b/src/orcapod/utils/git_utils.py
@@ -1,31 +1,47 @@
 from __future__ import annotations
 
 import inspect
-from typing import Any
+from typing import Any, TYPE_CHECKING
+from pathlib import Path
+from typing import TypedDict
 from orcapod.utils.lazy_module import LazyModule
 
-git = LazyModule("git")
+if TYPE_CHECKING:
+    import git
+else:
+    git =  LazyModule("git")
 
+#! Generally need better documentation and some of the items are only weakly typed
+#! largely stemming from some of the properties as found in git package is only
+#! partially typed. This module needs thorough testing against a real git repo
 
-def is_git_repo(path):
+def is_git_repo(path: str | Path) -> bool:
     """Check if path is a git repository"""
     try:
         git.Repo(path, search_parent_directories=True)
         return True
-    except git.exc.InvalidGitRepositoryError:
+    except git.InvalidGitRepositoryError:
         return False
 
 
-def get_root_git_dir(path):
+def get_root_git_dir(path: str | Path) -> str | None:
     """Get the root .git directory for a path"""
     try:
         repo = git.Repo(path, search_parent_directories=True)
         return repo.git.rev_parse("--show-toplevel")
-    except git.exc.InvalidGitRepositoryError:
+    except git.InvalidGitRepositoryError:
         return None
 
+class GitRepoInfo(TypedDict):
+    is_repo: bool
+    commit_hash: str
+    short_hash: str
+    is_dirty: bool
+    has_untracked_files: bool
+    branch: str
+    repo_root: str
 
-def get_git_info(path):
+def get_git_info(path: str | Path) -> GitRepoInfo | None:
     """Get comprehensive git information for a path"""
     try:
         # This will search parent directories for .git
@@ -50,15 +66,15 @@ def get_git_info(path):
         # covers staged/unstaged changes to tracked files).
         has_untracked_files = len(repo.untracked_files) > 0
 
-        return {
+        return GitRepoInfo({
             "is_repo": True,
             "commit_hash": commit_hash,
             "short_hash": short_hash,
             "is_dirty": is_dirty,
             "has_untracked_files": has_untracked_files,
             "branch": branch_name,
-            "repo_root": repo.working_dir,
-        }
+            "repo_root": str(repo.working_dir),
+        })
 
     except Exception:
         # Not a git repo, git not installed, or a git call failed — no git info
@@ -80,9 +96,9 @@ def get_git_info_for_python_object(python_object, try_cwd:bool=False) -> dict[st
 
             if git_info is None:
                 return None
-        
+
             git_source = "cwd"
-            
+
         env_info = {}
         env_info["git_commit_hash"] = git_info.get("commit_hash")
         env_info["git_repo_status"] = "dirty" if git_info.get("is_dirty") else "clean"
diff --git a/src/orcapod/utils/lazy_module.py b/src/orcapod/utils/lazy_module.py
index b26d55117..a699e3d8c 100644
--- a/src/orcapod/utils/lazy_module.py
+++ b/src/orcapod/utils/lazy_module.py
@@ -47,6 +47,7 @@ def __getattr__(self, name: str) -> Any:
             #!? Fine for the current pyarrow/polars/git usage, but a lurking gotcha. Consider
             #!? special-casing known module dunders, or documenting the limitation.
             # Avoid infinite recursion for internal attributes
+            #! Yes this should be checked/revisited for an improved logic
             raise AttributeError(
                 f"'{self.__class__.__name__}' object has no attribute '{name}'"
             )
diff --git a/src/orcapod/utils/name.py b/src/orcapod/utils/name.py
deleted file mode 100644
index 79bc1e8a8..000000000
--- a/src/orcapod/utils/name.py
+++ /dev/null
@@ -1,168 +0,0 @@
-"""
-Utility functions for handling names
-"""
-
-import re
-
-
-def escape_with_postfix(field: str, postfix=None, separator="_") -> str:
-    """
-    Escape the field string by doubling separators and optionally append a postfix.
-    This function takes a field string and escapes any occurrences of the separator
-    by doubling them, then optionally appends a postfix with a separator prefix.
-
-    Args:
-        field (str): The input string containing to be escaped.
-        postfix (str, optional): An optional postfix to append to the escaped string.
-                               If None, no postfix is added. Defaults to None.
-        separator (str, optional): The separator character to escape and use for
-                                 prefixing the postfix. Defaults to "_".
-    Returns:
-        str: The escaped string with ``postfix`` appended after a single
-             ``separator`` when ``postfix`` is given, or just the escaped
-             string when ``postfix`` is None.
-    Examples:
-        >>> escape_with_postfix("field1_field2", "suffix")
-        'field1__field2_suffix'
-        >>> escape_with_postfix("name_age_city", "backup", "_")
-        'name__age__city_backup'
-        >>> escape_with_postfix("data-info", "temp", "-")
-        'data--info-temp'
-        >>> escape_with_postfix("simple", None)
-        'simple'
-        >>> escape_with_postfix("no_separators", "end")
-        'no__separators_end'
-    """
-    #!? BUG (verified): the postfix delimiter is hardcoded to "_" (`f"_{postfix}"`), NOT `separator`.
-    #!? So docstring examples with separator="-" are WRONG: escape("data-info","temp","-") actually
-    #!? returns 'data--info_temp' (not 'data--info-temp'), and unescape can't recover the postfix.
-    #!? Worse, latent corruption when separator != "_" and a field contains "_":
-    #!? unescape("field1--field2-meta", separator="-") → ('field1-field2-meta', None), not (...,'meta').
-    #!? Fix: use `separator` for the postfix delimiter too (f"{separator}{postfix}"), or fix the docs
-    #!? to state postfix is always "_"-delimited. Also the Returns line ("empty string if postfix is
-    #!? None") is wrong — it returns the escaped field.
-    return field.replace(separator, separator * 2) + (
-        f"{separator}{postfix}" if postfix else ""
-    )
-
-
-def unescape_with_postfix(field: str, separator="_") -> tuple[str, str | None]:
-    """
-    Unescape a string by converting double separators back to single separators and extract postfix metadata.
-    This function reverses the escaping process where single separators were doubled to avoid
-    conflicts with metadata delimiters. It splits the input on double separators, then extracts
-    any postfix metadata from the last part.
-
-    Args:
-        field (str): The escaped string containing doubled separators and optional postfix metadata
-        separator (str, optional): The separator character used for escaping. Defaults to "_"
-    Returns:
-        tuple[str, str | None]: A tuple containing:
-            - The unescaped string with single separators restored
-            - The postfix metadata if present, None otherwise
-    Examples:
-        >>> unescape_with_postfix("field1__field2__field3")
-        ('field1_field2_field3', None)
-        >>> unescape_with_postfix("field1__field2_metadata")
-        ('field1_field2', 'metadata')
-        >>> unescape_with_postfix("simple")
-        ('simple', None)
-        >>> unescape_with_postfix("field1--field2", separator="-")
-        ('field1-field2', None)
-        >>> unescape_with_postfix("field1--field2-meta", separator="-")
-        ('field1-field2', 'meta')
-    """
-
-    parts = field.split(separator * 2)
-    parts[-1], *meta = parts[-1].split(separator, 1)
-    return separator.join(parts), meta[0] if meta else None
-
-
-def find_noncolliding_name(name: str, lut: dict) -> str:
-    """
-    Generate a unique name that does not collide with existing keys in a lookup table (lut).
-
-    If the given name already exists in the lookup table, a numeric suffix is appended
-    to the name (e.g., "name_1", "name_2") until a non-colliding name is found.
-
-    Parameters:
-        name (str): The base name to check for collisions.
-        lut (dict): A dictionary representing the lookup table of existing names.
-
-    Returns:
-        str: A unique name that does not collide with any key in the lookup table.
-
-    Example:
-        >>> lut = {"name": 1, "name_1": 2}
-        >>> find_noncolliding_name("name", lut)
-        'name_2'
-    """
-    if name not in lut:
-        return name
-
-    suffix = 1
-    while f"{name}_{suffix}" in lut:
-        suffix += 1
-
-    return f"{name}_{suffix}"
-
-
-def pascal_to_snake(name: str) -> str:
-    # Convert PascalCase to snake_case
-    # if already in snake_case, return as is
-    # TODO: replace this crude check with a more robust one
-    if "_" in name:
-        # put everything into lowercase and return
-        return name.lower()
-    # Replace uppercase letters with underscore followed by lowercase letter
-    s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", name)
-    return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower()
-
-
-def snake_to_pascal(name: str) -> str:
-    # Convert snake_case to PascalCase
-    # if already in PascalCase, return as is
-    if "_" not in name:
-        # capitalize the name and return
-        return name.capitalize()
-    # Split the string by underscores and capitalize each component
-    components = name.split("_")
-    return "".join(x.title() for x in components)
-
-
-# def get_function_signature(func):
-#     """
-#     Returns a string representation of how the function arguments were defined.
-#     Example output: f(a, b, c, d=0, **kwargs)
-#     """
-#     sig = inspect.signature(func)
-#     function_name = func.__name__
-
-#     param_strings = []
-
-#     for name, param in sig.parameters.items():
-#         # Handle different parameter kinds
-#         if param.kind == param.POSITIONAL_ONLY:
-#             formatted_param = f"{name}, /"
-#         elif param.kind == param.POSITIONAL_OR_KEYWORD:
-#             if param.default is param.empty:
-#                 formatted_param = name
-#             else:
-#                 # Format the default value
-#                 default = repr(param.default)
-#                 formatted_param = f"{name}={default}"
-#         elif param.kind == param.VAR_POSITIONAL:
-#             formatted_param = f"*{name}"
-#         elif param.kind == param.KEYWORD_ONLY:
-#             if param.default is param.empty:
-#                 formatted_param = f"*, {name}"
-#             else:
-#                 default = repr(param.default)
-#                 formatted_param = f"{name}={default}"
-#         elif param.kind == param.VAR_KEYWORD:
-#             formatted_param = f"**{name}"
-
-#         param_strings.append(formatted_param)
-
-#     params_str = ", ".join(param_strings)
-#     return f"{function_name}({params_str})"
diff --git a/src/orcapod/utils/object_spec.py b/src/orcapod/utils/object_spec.py
index 2a538791a..51afd771a 100644
--- a/src/orcapod/utils/object_spec.py
+++ b/src/orcapod/utils/object_spec.py
@@ -2,6 +2,8 @@
 from typing import Any
 
 
+#! Indeed this has trust_boundary issue that should actually 1) clearly documented and 2) have a way to reject
+#! auto imports for modules not found within whiltelisted modules. This should be done in a dedicated issue.
 #!? TRUST BOUNDARY: this resolves `{"_class": "module.Cls", "_config": {...}}` specs by importing
 #!? arbitrary modules and instantiating arbitrary classes (importlib + getattr + cls(**config)).
 #!? That's effectively arbitrary-code-execution if a spec ever comes from an untrusted source.
@@ -128,30 +130,3 @@ def _validate_config_for_class(cls: type, config: dict[str, Any]) -> None:
     except Exception:
         # Skip validation if introspection fails
         pass
-
-
-# def parse_objectspec(obj_spec: Any) -> Any:
-#     if isinstance(obj_spec, dict):
-#         if "_class" in obj_spec:
-#             # if _class is specified, treat the dict as an object specification, looking for
-#             # _config key to extract configuration parameters
-#             module_name, class_name = obj_spec["_class"].rsplit(".", 1)
-#             module = importlib.import_module(module_name)
-#             cls = getattr(module, class_name)
-#             configs = parse_objectspec(obj_spec.get("_config", {}))
-#             return cls(**configs)
-#         else:
-#             # otherwise, parse through the dictionary recursively
-#             parsed_object = obj_spec
-#             for k, v in obj_spec.items():
-#                 parsed_object[k] = parse_objectspec(v)
-#             return parsed_object
-#     elif isinstance(obj_spec, list):
-#         # if it's a list, parse each item in the list
-#         return [parse_objectspec(item) for item in obj_spec]
-#     elif isinstance(obj_spec, tuple):
-#         # if it's a tuple, parse each item in the tuple
-#         return tuple(parse_objectspec(item) for item in obj_spec)
-#     else:
-#         # if it's neither a dict nor a list, return it as is
-#         return obj_spec
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
index 4ddc00de0..103ad914d 100644
--- a/superpowers/reviews/pre-v0.2-codebase-review.md
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -59,7 +59,9 @@ Everything downstream imports these. Establish the vocabulary first.
 - [x] `errors.py` — exception taxonomy; note the *absence* of execution/OOM error types (Area 3) → I-6…I-9
 - [x] `types.py` — Schema, ColumnConfig, ContentHash, config dataclasses; **2 verified bugs** (unhashable Schema, `__eq__` raises) + dead exec config (X3) + missing retry fields (Area 3) → I-10…I-14
 - [x] `config.py` — 2nd (global/TOML) config system; dead+broken merge; heavy duplication → I-17…I-19
-- [x] `utils/lazy_module.py`, `utils/name.py`, `utils/git_utils.py`, `utils/function_info.py`, `utils/object_spec.py` → I-21…I-27  **(Leg 0 COMPLETE)**
+- [x] `utils/lazy_module.py`, `utils/name.py`, `utils/git_utils.py`, `utils/function_info.py`, `utils/object_spec.py` → I-21…I-29 (reviewer-reviewed) — **one open decision: delete dead `name.py` (I-29)**
+
+> **Leg 0 status:** all files reviewer-reviewed. Remaining: reviewer's go-ahead to delete the dead `name.py` module (I-29), then commit the utils batch and close ITL-564. Checkbox legend: `[x]` = reviewer-reviewed · `[~]` = agent pass only.
 
 ## Leg 1 — Type system & semantic conversion (Python ↔ Arrow)
 - [ ] `semantic_types/` — `type_inference.py`, `universal_converter.py`, `precomputed_converters.py`, `pydata_utils.py`
@@ -153,12 +155,16 @@ Everything downstream imports these. Establish the vocabulary first.
 | I-19 | config.py:162 (from_dict), 236 (load_config) | ~150 lines of duplicated per-section parse/warn/filter; `load_config` reimplements `from_dict`; Mapping-vs-dict inconsistency; hardcoded section set ×3. **Reviewer: confirmed — targeted refactor issue(s); also add optional env-var config override (low priority).** | _accepted_ | DRY |
 | **I-20** | config.py + types.py (design) | **Config-system unification design spike (reviewer-requested).** Evaluate: (a) two categories = global-lib-config vs object-config, with maximally-similar interfaces; (b) single consolidated merge; (c) **co-locating object-specific config WITH the object it configures**; (d) optional env-var overrides. Needs a spike before refactor. | _accepted (design spike)_ | design |
 | **I-21** | types.py:597 (ColumnInfo) | **BLOCKING BUG (verified): `@dataclass(frozen=True, slot=True)` — `slot` should be `slots`; raised `TypeError` at import → `import orcapod` broken.** Slipped into commit 6e8a3181. **FIXED in place** (slots=True). | _fixed in place_ | new bug |
-| **I-22** | utils/name.py | **BUG (verified): postfix delimiter hardcoded `_`, ignored `separator`** → corruption for non-`_` separators. **FIXED**: delimiter now follows `separator` (byte-identical for `_`), docstring corrected + verified roundtrips for `_`/`-`/`:`. (Stale TODO + dead commented block left for I-27.) | _fixed + pushed_ | new bug |
+| **I-22** | utils/name.py | **BUG (verified): postfix delimiter hardcoded `_`, ignored `separator`** → corruption for non-`_` separators. **FIXED** (delimiter follows `separator`; roundtrips verified). Stale `#!?` comment removed. **⚠️ but see I-29 — the whole module is dead, so this fix is in dead code.** | _fixed; module deletion pending (I-29)_ | new bug |
 | I-23 | utils/git_utils.py | Bare `except:` caught KeyboardInterrupt/SystemExit. **FIXED**: → `except Exception:` (return None preserved; comment explains breadth). | _fixed + pushed_ | robustness |
-| I-24 | utils/function_info.py | `use_ast_parsing` dead param + commented AST branch. **FIXED**: removed the param, docstring line, and dead branch (only caller `data_function.py:468` passed no args). `_is_in_string` **left as-is** — dormant (comments included by default) and changing comment-stripping would alter function hashes. | _fixed (param); _is_in_string noted_ | dead-code |
+| I-24 | utils/function_info.py | `use_ast_parsing` dead param + commented AST branch. **FIXED** (removed). `_is_in_string` left as-is (dormant). **Reviewer:** test coverage of these functions must be fully assessed (they feed function-identity hashing, Area 2) → test-coverage follow-up. | _fixed (param); test-coverage follow-up_ | dead-code / test-coverage |
+| I-25 | utils/object_spec.py | Arbitrary-class instantiation trust boundary. **Reviewer: (1) clearly document it, (2) reject auto-imports for modules not in a whitelist — as a DEDICATED issue.** `_validate_config_for_class` also swallows all exceptions. | _accepted (dedicated issue: whitelist + docs)_ | security |
+| I-26 | utils/lazy_module.py | `_`-prefix `__getattr__` guard blocks lazy module-dunder access. **Reviewer: confirmed — revisit for improved logic.** | _accepted_ | minor |
+| **I-29** | utils/name.py (whole module) | **Entire module is dead: `orcapod.utils.name` is never imported; all 5 functions (escape/unescape_with_postfix, find_noncolliding_name, pascal_to_snake, snake_to_pascal) have 0 uses in src + tests.** Strong deletion candidate (reviewer inclined). If deleted, I-22 fix goes with it. | _pending reviewer go-ahead to delete_ | dead-code |
 | I-25 | utils/object_spec.py:5 | **Trust boundary:** `parse_objectspec` imports+instantiates arbitrary classes from a spec (RCE if input untrusted). Used for DataContext-from-JSON. Add docstring warning + confirm trust model; dead commented block; `_validate_config_for_class` swallows all exceptions. | _pending (security-awareness)_ | security |
 | I-26 | utils/lazy_module.py:44 | `_`-prefixed `__getattr__` guard also blocks lazy access to module dunders (`__version__`/`__all__`). Minor; `Optional`/`str|None` typing inconsistency. | _pending (minor)_ | minor |
 | I-27 | name.py, function_info.py, object_spec.py | Recurring **dead commented-out code** blocks (old `get_function_signature`, AST docstring-removal methods, old `parse_objectspec`). Sweep/remove. | _pending_ | cleanup |
+| I-28 | utils/git_utils.py | **Reviewer:** module needs better docs; weakly typed (partly due to GitPython's partial type stubs); **needs thorough testing against a real git repo**. Reviewer also added `GitRepoInfo` TypedDict + type hints. **Import:** RESOLVED — reviewer restored `LazyModule("git")` via the `TYPE_CHECKING` pattern (typed for checkers, lazy at runtime). **Annotation:** reviewer added `-> GitRepoInfo`, but the except branch returns None → should be `-> GitRepoInfo | None` (open nit). Docs/weak-typing/real-repo-test notes remain as follow-ups. | _pending (reviewer decision on import)_ | robustness / docs / test-coverage |
 | I-12 | types.py:330-331 | Dead `PipelineConfig.execution_engine`/`execution_engine_opts` w/ misleading docstring (`with_options` never called) | _pending_ | X3, Area 2 |
 | I-13 | types.py:335-366 | No retry/memory config fields on any config (Area 3); PodConfig-vs-NodeConfig axis undocumented; only NodeConfig has merge() | _pending_ | Area 3 |
 | I-14 | types.py:474 (ColumnConfig) | Docstring Attributes omits real fields `content_hash`, `sort_by_tags` | _pending_ | doc-drift |

From d8394606e7d778f9ddbeecb6697d14b72eae56ca Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 17:09:48 -0700
Subject: [PATCH 5/6] fix: address Copilot PR review comments on #243 (ITL-564)

- types.py: Schema.__eq__ now returns NotImplemented for unsupported types
  instead of raising NotImplementedError (finding I-11). `schema == 5` -> False,
  `schema != 5` -> True, membership works. (Hashing/__hash__ is a separate
  concern tracked as I-10.)
- channels.py: fix collect() docstring to say "sequence" (return type is
  Sequence[T_co], not list).
- object_spec.py: tidy the trust-boundary review note (typo "whiltelisted").
- function_info.py: fix grammar in the test-coverage review note.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/channels.py                         |  2 +-
 src/orcapod/types.py                            | 13 ++++---------
 src/orcapod/utils/function_info.py              |  2 +-
 src/orcapod/utils/object_spec.py                |  4 ++--
 superpowers/reviews/pre-v0.2-codebase-review.md |  4 ++--
 5 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/orcapod/channels.py b/src/orcapod/channels.py
index 8552fb211..0e0d92eee 100644
--- a/src/orcapod/channels.py
+++ b/src/orcapod/channels.py
@@ -54,7 +54,7 @@ def __aiter__(self) -> AsyncIterator[T_co]: ...
     async def __anext__(self) -> T_co: ...
 
     async def collect(self) -> Sequence[T_co]:
-        """Drain all remaining items into a list."""
+        """Drain all remaining items into a sequence."""
         ...
 
 
diff --git a/src/orcapod/types.py b/src/orcapod/types.py
index a4a1f674f..985affc2b 100644
--- a/src/orcapod/types.py
+++ b/src/orcapod/types.py
@@ -164,15 +164,10 @@ def __eq__(self, other: object) -> bool:
             return self._data == other._data and self._optional == other._optional
         if isinstance(other, Mapping):
             return self._data == dict(other)
-        #!? BUG: should `return NotImplemented` (the singleton), NOT raise. Raising means
-        #!? `schema == 5` throws instead of being False, and `schema != 5` throws too (verified).
-        #!? Breaks `in` checks, unittest/pytest equality, and any generic comparison. One-line fix.
-        #! Is it more proper to return NotImplemented? Since I want to signify that this is not available
-        #! for use in context where one triest to hash it, I felt it would be appropriate to raise an error
-        #! actively
-        raise NotImplementedError(
-            f"Equality check is not implemented for object of type {type(other)}"
-        )
+        # Not comparable with this type: return the NotImplemented singleton so Python
+        # falls back to identity (``schema == 5`` -> False, ``schema != 5`` -> True)
+        # instead of raising. Hashability is a separate concern (see finding I-10).
+        return NotImplemented
 
     # ==================== Optionality ====================
 
diff --git a/src/orcapod/utils/function_info.py b/src/orcapod/utils/function_info.py
index 85e888414..2bc3c6f55 100644
--- a/src/orcapod/utils/function_info.py
+++ b/src/orcapod/utils/function_info.py
@@ -2,7 +2,7 @@
 import inspect
 from collections.abc import Callable
 
-#! How comprehensively this is these functions are tested should be fully assessed
+#! Test coverage of these functions should be fully assessed.
 
 def _is_in_string(line: str, pos: int) -> bool:
     """Helper to check if a position in a line is inside a string literal."""
diff --git a/src/orcapod/utils/object_spec.py b/src/orcapod/utils/object_spec.py
index 51afd771a..ab766306d 100644
--- a/src/orcapod/utils/object_spec.py
+++ b/src/orcapod/utils/object_spec.py
@@ -2,8 +2,8 @@
 from typing import Any
 
 
-#! Indeed this has trust_boundary issue that should actually 1) clearly documented and 2) have a way to reject
-#! auto imports for modules not found within whiltelisted modules. This should be done in a dedicated issue.
+#! Trust-boundary issue: this should (1) be clearly documented and (2) reject auto-imports of
+#! modules that are not on an allowlist. Track as a dedicated issue.
 #!? TRUST BOUNDARY: this resolves `{"_class": "module.Cls", "_config": {...}}` specs by importing
 #!? arbitrary modules and instantiating arbitrary classes (importlib + getattr + cls(**config)).
 #!? That's effectively arbitrary-code-execution if a spec ever comes from an untrusted source.
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
index 103ad914d..bfac47794 100644
--- a/superpowers/reviews/pre-v0.2-codebase-review.md
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -146,8 +146,8 @@ Everything downstream imports these. Establish the vocabulary first.
 | I-7 | errors.py + contexts/core.py + hashing/visitors.py | Error classes scattered outside errors.py. **Reviewer: CONFIRMED — all error/exception classes must be collected in this module** | _accepted_ | api-design |
 | I-8 | errors.py (missing) | **No execution-failure taxonomy** (no PodExecution/OOM/Transient/Retryable) — groundwork gap for retry. **Reviewer: CONFIRMED — we want to add it** | _accepted (feeds ITL-557)_ | Area 3 |
 | I-9 | errors.py:164, 76 | `EmptyDataHashMissingError` msg cited dead `INPUT_DATA_HASH_COL` → **reviewer FIXED in place**; `SourceSpecMismatchError` "preserved for compatibility" note still open (greenfield) | _partially applied_ | Area 4, cleanup |
-| **I-10** | types.py:93 (Schema) | **BUG (verified): Schema is unhashable** — `__eq__` override w/o `__hash__`. **Reviewer: implement real `__hash__` eventually; until then keep "semantic-hasher-hashable" qualifier + make hashable-context use raise an *informative* error (not bare TypeError)** | _accepted (correctness)_ | new bug |
-| **I-11** | types.py:163 (Schema.__eq__) | **BUG (verified): raises `NotImplementedError` instead of returning `NotImplemented`**. **Reviewer confirmed → return `NotImplemented` singleton** | _accepted → return NotImplemented_ | new bug |
+| **I-10** | types.py (Schema) | **Schema is unhashable** — `__eq__` override w/o `__hash__`. Reviewer: implement real `__hash__` eventually; interim = keep "semantic-hasher-hashable" qualifier + make hashable-context use raise an *informative* `TypeError`. NOTE: reviewer's "raise to block hashing" rationale (removed from `__eq__` when fixing I-11) belongs HERE in `__hash__`. Still open. | _open (interim __hash__ pending)_ | new bug |
+| **I-11** | types.py:162 (Schema.__eq__) | `__eq__` raised `NotImplementedError` instead of returning `NotImplemented`. **FIXED** (returns `NotImplemented`; verified `==5→False`, `!=5→True`, membership OK). Independently confirmed by Copilot on PR #243. | _fixed + pushed_ | new bug |
 | I-15 | types.py (whole) | Module too big; docstring stale. **Reviewer: split into a subpackage** — `Schema` own module, Arrow-related type aliases own module | _accepted (refactor follow-up)_ | tech-debt |
 | I-16 | types.py:40-88 (aliases) | Type-alias hygiene: usage-analyze & prune unused aliases; simplify `DataValue` to leverage extension system (arbitrary types); evaluate `UPath` for `PathLike` (spike); convert `DataType` older-Union TODO to an issue | _accepted (follow-ups)_ | tech-debt |
 | I-17 | config.py + types.py | Two config systems (`OrcapodConfig` global/lib vs `PipelineConfig`/`PodConfig`/`NodeConfig` object-config) with divergent merge. **Reviewer: both categories are legitimate — global-lib-config vs object-config — but interfaces should be as similar as possible; make the distinction explicit.** → see I-20 spike | _accepted → I-20_ | X3-adjacent |

From bf657fc9a53ea9ef8dbe3e6a0d3af50979c74b45 Mon Sep 17 00:00:00 2001
From: "Edgar Y. Walker" <edgar@metamorphic.com>
Date: Wed, 22 Jul 2026 17:46:29 -0700
Subject: [PATCH 6/6] fix(types): add informative __hash__ to Schema (I-10)

Schema opts out of Python built-in hashing (identity comes from the semantic
hasher / content_hash). Replace the implicit `__hash__ = None` (bare
"unhashable type" TypeError) with an explicit __hash__ that raises a clear,
actionable TypeError. Still a TypeError, so `except TypeError` callers are
unaffected. Docstring updated; stale review annotations removed.

Full suite: 4611 passed. A value-based __hash__ remains an optional future
follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/orcapod/types.py                          | 28 +++++++++++--------
 .../reviews/pre-v0.2-codebase-review.md       |  2 +-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/orcapod/types.py b/src/orcapod/types.py
index 985affc2b..89c629b96 100644
--- a/src/orcapod/types.py
+++ b/src/orcapod/types.py
@@ -93,18 +93,10 @@ class Schema(Mapping[str, DataType]):
     """Immutable schema representing a mapping of field names to Python types.
 
     Serves as the canonical internal schema representation in OrcaPod,
-    with interop to/from Arrow schemas. Semantic hasher hashable and suitable for use
-    in content-addressable contexts.
-    #! This should be fixed such that __hash__ is correctly implemented to yield
-    #! properly hashable data type -- till that is achieved, however, we should
-    #! actively raise an error whenever someone (system) tries to use this in hashable
-    #! context. Till this is implemented, adding qualifier that this is semantic hasher
-    #! hashable
-    #!? BUG: NOT hashable. `__eq__` is overridden below without a `__hash__`, so Python set
-    #!? `Schema.__hash__ = None` → `hash(Schema(...))`, `{schema}`, dict-keying all raise TypeError
-    #!? (verified). Either add `__hash__` (safe — Schema is effectively immutable: hash on
-    #!? (frozenset(self._data.items()), self._optional)) or correct this docstring. If "hashable"
-    #!? here means "content-hashable via semantic hashers" (not Python hash()), say that explicitly.
+    with interop to/from Arrow schemas. Not hashable via Python's built-in
+    ``hash()`` — ``__hash__`` raises an informative ``TypeError``; identity is
+    established through the semantic hasher (``content_hash``) instead. A
+    value-based ``__hash__`` may be added later (review finding I-10).
 
     Args:
         fields: An optional mapping of field names to their data types.
@@ -169,6 +161,18 @@ def __eq__(self, other: object) -> bool:
         # instead of raising. Hashability is a separate concern (see finding I-10).
         return NotImplemented
 
+    def __hash__(self) -> int:
+        # Schema deliberately opts out of Python's built-in hashing. Its identity is
+        # established through the semantic hasher (content_hash), not hash(). Raising
+        # an explicit, informative TypeError here — instead of leaving __hash__ = None,
+        # which yields only a bare "unhashable type: 'Schema'" — makes the intent clear.
+        # A value-based __hash__ may be added later (finding I-10).
+        raise TypeError(
+            "Schema is not hashable via hash() and cannot be used as a dict key or "
+            "set member. Its identity is established through the semantic hasher "
+            "(content_hash), not Python's built-in hash()."
+        )
+
     # ==================== Optionality ====================
 
     @property
diff --git a/superpowers/reviews/pre-v0.2-codebase-review.md b/superpowers/reviews/pre-v0.2-codebase-review.md
index bfac47794..b18068edf 100644
--- a/superpowers/reviews/pre-v0.2-codebase-review.md
+++ b/superpowers/reviews/pre-v0.2-codebase-review.md
@@ -146,7 +146,7 @@ Everything downstream imports these. Establish the vocabulary first.
 | I-7 | errors.py + contexts/core.py + hashing/visitors.py | Error classes scattered outside errors.py. **Reviewer: CONFIRMED — all error/exception classes must be collected in this module** | _accepted_ | api-design |
 | I-8 | errors.py (missing) | **No execution-failure taxonomy** (no PodExecution/OOM/Transient/Retryable) — groundwork gap for retry. **Reviewer: CONFIRMED — we want to add it** | _accepted (feeds ITL-557)_ | Area 3 |
 | I-9 | errors.py:164, 76 | `EmptyDataHashMissingError` msg cited dead `INPUT_DATA_HASH_COL` → **reviewer FIXED in place**; `SourceSpecMismatchError` "preserved for compatibility" note still open (greenfield) | _partially applied_ | Area 4, cleanup |
-| **I-10** | types.py (Schema) | **Schema is unhashable** — `__eq__` override w/o `__hash__`. Reviewer: implement real `__hash__` eventually; interim = keep "semantic-hasher-hashable" qualifier + make hashable-context use raise an *informative* `TypeError`. NOTE: reviewer's "raise to block hashing" rationale (removed from `__eq__` when fixing I-11) belongs HERE in `__hash__`. Still open. | _open (interim __hash__ pending)_ | new bug |
+| **I-10** | types.py (Schema) | Schema not hashable via `hash()`. **FIXED (interim):** added explicit `__hash__` that raises an *informative* `TypeError` (still a TypeError, so `except TypeError` callers unaffected) pointing at the semantic hasher; docstring updated; stale annotations removed. Full suite 4611 passed. A real value-based `__hash__` remains an optional future follow-up. | _fixed (interim); value-based hash optional_ | new bug |
 | **I-11** | types.py:162 (Schema.__eq__) | `__eq__` raised `NotImplementedError` instead of returning `NotImplemented`. **FIXED** (returns `NotImplemented`; verified `==5→False`, `!=5→True`, membership OK). Independently confirmed by Copilot on PR #243. | _fixed + pushed_ | new bug |
 | I-15 | types.py (whole) | Module too big; docstring stale. **Reviewer: split into a subpackage** — `Schema` own module, Arrow-related type aliases own module | _accepted (refactor follow-up)_ | tech-debt |
 | I-16 | types.py:40-88 (aliases) | Type-alias hygiene: usage-analyze & prune unused aliases; simplify `DataValue` to leverage extension system (arbitrary types); evaluate `UPath` for `PathLike` (spike); convert `DataType` older-Union TODO to an issue | _accepted (follow-ups)_ | tech-debt |