fix: include class-based UDF instance state in hash

[ilongin]

Fixes #1903.

Class-based UDFs (Mapper / Generator / Aggregator subclasses) hashed to the same value regardless of constructor args, so the second .agg(CountAbove(3), ...) call was served the first .agg(CountAbove(0), ...)'s cached rows. No error, no version bump, wrong numbers.

Changes:

• UDFBase.hash() now mixes in _hash_state() bytes when the UDF is class-based (self._func is None). Default _hash_state() returns filtered_cloudpickle_dumps(self), so instance attributes like self.limit end up in the hash.
• ArrowGenerator and HFGenerator override _hash_state() to skip self.output_schema, which is a dynamically-created pydantic class with a random name suffix. The schema's stable field shape is already in self.output.hash(), so no signal is lost and dc.read_csv / dc.read_parquet hashes stay deterministic across calls.
• Tests: unit hash tests pin the fix and the determinism guarantee; a func test reproduces the exact issue scenario.

Function-based UDFs (plain lambdas and def functions passed to .map()) are untouched - the pickle branch only runs for class-based UDFs.

[cloudflare-workers-and-pages]

Deploying datachain with    Cloudflare Pages

Latest commit:	615bc8a
Status:	✅  Deploy successful!
Preview URL:	https://ce310d55.datachain-2g6.pages.dev
Branch Preview URL:	https://ilongin-1903-class-udf-hash.datachain-2g6.pages.dev

View logs

[codecov]

Codecov Report

❌ Patch coverage is 95.76271% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/datachain/lib/hf.py	78.57%	1 Missing and 2 partials ⚠️
src/datachain/lib/udf.py	94.87%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Updates UDF cache identities to distinguish class instances with different constructor arguments, preventing stale cached results.

Changes:

• Adds normalized value hashing for class-based UDF constructor arguments.
• Preserves deterministic Arrow/Hugging Face and LLM identities.
• Adds unit and functional regression coverage.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/datachain/hash_utils.py	Adds normalized value hashing utilities.
src/datachain/lib/udf.py	Incorporates constructor arguments into UDF hashes.
src/datachain/lib/arrow.py	Excludes dynamic output schemas from constructor hashes.
src/datachain/lib/hf.py	Excludes dynamic output schemas from constructor hashes.
src/datachain/llm/spec.py	Reuses shared hash normalization.
tests/func/test_udf.py	Tests distinct aggregate results by constructor state.
tests/unit/lib/test_arrow.py	Tests Arrow constructor hashing.
tests/unit/lib/test_hf.py	Tests Hugging Face constructor hashing.
tests/unit/lib/test_llm.py	Removes superseded canonicalization coverage.
tests/unit/lib/test_udf.py	Tests UDF hash variation and determinism.
tests/unit/test_hash_utils.py	Tests hash-value normalization.
tests/unit/test_query_steps_hash.py	Updates expected query hashes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[amritghimire]

do we need long docstring even for internal methods?

[ilongin]

Reduced comment

[shcheklein]

@ilongin check please this AI feedback:

[P1] Dict normalization can still cause stale-cache collisions — hash_utils.py:27
Sorting dictionary entries discards insertion order, which Python code can observe. I reproduced identical UDF hashes for {"a": 1, "b": 2} and {"b": 2, "a": 1}, while process() returned "a,b" and "b,a" respectively. This recreates the silent wrong-result failure the PR is intended to fix. UDF constructor hashing should preserve dict order; LLM parameters can retain separate order-insensitive normalization.

[P2] Frozen dataclass UDFs can no longer be instantiated — udf.py:310
Assigning _constructor_state_hash normally invokes the subclass’s setattr. A @DataClass(frozen=True) Mapper that worked before this PR now raises FrozenInstanceError. Using object.setattr() would avoid the regression.

[P2] Cyclic constructor containers crash instead of disabling reuse — hash_utils.py:27
A recursive dict or list causes normalize_hash_value() to recurse until RecursionError; the fallback only catches TypeError. Such configurations were previously accepted and are cloudpickle-compatible. Detect cycles and treat them as unsupported, or include RecursionError in the no-reuse fallback.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

src/datachain/hash_utils.py:26

• Floats are still hashed through repr(normalized), which collapses distinct NaN bit patterns (including positive and negative NaN) to the same text. A UDF can observe that difference, for example with math.copysign or struct.pack, so different constructor arguments can still share a cache key and return stale rows. Normalize floats to their IEEE-754 bytes before hashing rather than retaining the float object.

    if value_type in (bool, int, float, str, bytes):
        return (value_type.__name__, value)

[ilongin]

@ilongin check please this AI feedback:

[P1] Dict normalization can still cause stale-cache collisions — hash_utils.py:27 Sorting dictionary entries discards insertion order, which Python code can observe. I reproduced identical UDF hashes for {"a": 1, "b": 2} and {"b": 2, "a": 1}, while process() returned "a,b" and "b,a" respectively. This recreates the silent wrong-result failure the PR is intended to fix. UDF constructor hashing should preserve dict order; LLM parameters can retain separate order-insensitive normalization.

[P2] Frozen dataclass UDFs can no longer be instantiated — udf.py:310 Assigning _constructor_state_hash normally invokes the subclass’s setattr. A @DataClass(frozen=True) Mapper that worked before this PR now raises FrozenInstanceError. Using object.setattr() would avoid the regression.

[P2] Cyclic constructor containers crash instead of disabling reuse — hash_utils.py:27 A recursive dict or list causes normalize_hash_value() to recurse until RecursionError; the fallback only catches TypeError. Such configurations were previously accepted and are cloudpickle-compatible. Detect cycles and treat them as unsupported, or include RecursionError in the no-reuse fallback.

1. Dictionary order: Valid and fixed. UDF constructor hashing now preserves dict
   insertion order recursively, since user code can observe it. LLM parameter hashing
   keeps its separate order-insensitive behavior. Regression tests cover both policies.

2. Frozen dataclass UDFs: Not changed in this PR. Although the new constructor-hash
   assignment makes standalone construction fail earlier, frozen UDFs were already
   unusable in DataChain: the existing _init() assigns params, output, and
   _func, which frozen dataclasses reject. Proper support would be a separate feature,
   not the proposed one-line fix.

3. Cyclic containers: Valid and fixed. RecursionError now follows the existing
   unsupported-value fallback, giving each UDF instance or LLM operation a safe unique
   identity instead of crashing. Added UDF and LLM regression tests.

[shcheklein]

Q: is it really a "state" hash? does it change during the operation?

[shcheklein]

Q: do we document class based approach at all? where?

[shcheklein]

I wonder what terminology we use there ...

[shcheklein]

"operations" or something else

[shcheklein]

this not clear tbh ... in the user facing docs ... why is that a problem, why we do this, etc

[shcheklein]

is it really a problem to hash it again?

[shcheklein]

Check this feedback also:

[P1] Constructor normalization loses alias topology. [hash_utils.py:34](https://github.com/datachain-ai/datachain/blob/615bc8a5d4b1c0488700fc02b46c7466f26d15e3/src/datachain/hash_utils.py#L34-L49) normalizes each container independently. AliasAware(shared, shared) and AliasAware([], []) therefore hash identically, although the UDF can observe left is right. End-to-end, the second query reused True from the first instead of returning False. Preserve references with a shared memo, or disable reuse when repeated mutable containers are detected.

[P1] Dict output schemas can hide nested user validators. [parse_tabular() marks every dictionary schema as generated](https://github.com/datachain-ai/datachain/blob/615bc8a5d4b1c0488700fc02b46c7466f26d15e3/src/datachain/lib/dc/datachain.py#L2540-L2545), after which [ArrowGenerator drops the entire model](https://github.com/datachain-ai/datachain/blob/615bc8a5d4b1c0488700fc02b46c7466f26d15e3/src/datachain/lib/arrow.py#L104-L107). For output={"payload": Payload}, the wrapper is generated but Payload is user code. Same-shaped models with uppercase versus lowercase validators produced identical query hashes but different values, permitting stale rows. Only omit fully inferred schemas, or retain nested user-model behavior in the identity.

[P2] Conventional custom __new__ implementations now fail at hash time. On the [TypeError path](https://github.com/datachain-ai/datachain/blob/615bc8a5d4b1c0488700fc02b46c7466f26d15e3/src/datachain/lib/udf.py#L300-L317), def __new__(cls, limit): return super().__new__(cls) leaves _constructor_state_hash=""; .hash() then raises the new SHA-256 validation error. Initialize a random safe fallback before returning—pickle restoration can overwrite it.

[P2] Documented PyArrow partitioning objects disable caching. The Arrow whitelist [handles only ParseOptions and CsvFileFormat](https://github.com/datachain-ai/datachain/blob/615bc8a5d4b1c0488700fc02b46c7466f26d15e3/src/datachain/lib/arrow.py#L111-L117). Passing the same DirectoryPartitioning object to two read_parquet() calls produces different query hashes and warnings, despite partitioning accepting any PyArrow schema. Normalize its type, schema, and options deterministically.