Migrate usage of the Hash trait for cache invalidation to dedicated CacheHash trait

[TrueDoctor]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a new graphene-hash crate and a CacheHash trait to replace standard Hash implementations for cache invalidation, specifically addressing types containing floats by hashing their bit patterns. The changes include a derive macro for CacheHash and widespread adoption across the codebase, replacing various manual hashing hacks. Feedback highlights a syntax error in the derive macro's where clause generation, an unstable hash implementation using random UUIDs in VectorModification, and an empty implementation for TransformImage that would cause cache collisions. Additionally, it is suggested to use syn::Error instead of panicking in the macro for better developer experience.

[gemini-code-assist]

The current logic for constructing the where clause is flawed. ast.generics.split_for_impl() returns a where_clause that already includes the where keyword. By manually prepending another where (line 40), the macro generates invalid Rust code like where T: ExistingBound where T: CacheHash when bounds already exist.

A cleaner and more robust approach is to modify the generics object directly by adding the CacheHash bound to each type parameter before calling split_for_impl.

Suggested change

	let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl();
	// Build additional `T: CacheHash` bounds for each type parameter.
	let extra_bounds = ast.generics.type_params().map(|tp| {
	let ident = &tp.ident;
	quote! { #ident: graphene_hash::CacheHash }
	});
	let where_clause = if ast.generics.type_params().count() > 0 {
	let existing = where_clause.map(|w| quote! { #w }).unwrap_or_default();
	quote! {
	#existing
	where #(#extra_bounds,)*
	}
	} else {
	where_clause.map(|w| quote! { #w }).unwrap_or_default()
	};
	let mut generics = ast.generics.clone();
	for param in &mut generics.params {
	if let syn::GenericParam::Type(type_param) = param {
	type_param.bounds.push(syn::parse_quote!(graphene_hash::CacheHash));
	}
	}
	let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

[gemini-code-assist]

Panicking in a procedural macro results in a poor developer experience as the compiler output won't point to the specific line in the user's code. It is better to return a compile error using syn::Error so the issue is highlighted correctly in the IDE and compiler output.

Suggested change

	Data::Union(_) => panic!("CacheHash cannot be derived for unions"),
	Data::Union(_) => syn::Error::new(ast.ident.span(), "CacheHash cannot be derived for unions").to_compile_error(),

[cubic-dev-ai]

3 issues found across 56 files

Confidence score: 2/5

• Merge risk is high because cache hashing behavior appears inconsistent across multiple paths, with severity 7/10 and high confidence, which raises concrete regression risk in memoization and invalidation.
• In node-graph/libraries/vector-types/src/vector/vector_modification.rs, using generate_uuid() inside CacheHash makes hashes non-deterministic, so identical values can produce different cache keys and undermine cache stability.
• In node-graph/graph-craft/src/document/value.rs and node-graph/libraries/raster-types/src/image.rs, RenderOutput::cache_hash omits metadata and TransformImage::cache_hash is effectively a no-op, so distinct states/transforms may collide and return stale or incorrect cached results.
• Pay close attention to node-graph/libraries/vector-types/src/vector/vector_modification.rs, node-graph/graph-craft/src/document/value.rs, node-graph/libraries/raster-types/src/image.rs - cache hash semantics are likely to cause both false misses and false hits.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/vector-types/src/vector/vector_modification.rs">

<violation number="1" location="node-graph/libraries/vector-types/src/vector/vector_modification.rs:522">
P1: `CacheHash` is non-deterministic because it hashes `generate_uuid()`, so identical values produce different cache hashes.

(Based on your team's feedback about avoiding misleading or inconsistent hashing implementations.) [FEEDBACK_USED]</violation>
</file>

<file name="node-graph/graph-craft/src/document/value.rs">

<violation number="1" location="node-graph/graph-craft/src/document/value.rs:528">
P1: `RenderOutput::cache_hash` ignores `metadata`, which can cause incorrect memo cache hits when only metadata changes.

(Based on your team's feedback about avoiding misleading/incomplete hashing semantics.) [FEEDBACK_USED]</violation>
</file>

<file name="node-graph/libraries/raster-types/src/image.rs">

<violation number="1" location="node-graph/libraries/raster-types/src/image.rs:67">
P1: `TransformImage::cache_hash` is a no-op, so different transforms all hash identically and can break cache invalidation expectations.

(Based on your team's feedback about avoiding misleading or collision-prone hashing implementations.) [FEEDBACK_USED]</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: RenderOutput::cache_hash ignores metadata, which can cause incorrect memo cache hits when only metadata changes.

(Based on your team's feedback about avoiding misleading/incomplete hashing semantics.)

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/graph-craft/src/document/value.rs, line 528:

<comment>`RenderOutput::cache_hash` ignores `metadata`, which can cause incorrect memo cache hits when only metadata changes.

(Based on your team's feedback about avoiding misleading/incomplete hashing semantics.) </comment>

<file context>
@@ -495,96 +494,43 @@ pub enum RenderOutputType {
-		}
+impl CacheHash for RenderOutput {
+	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
+		self.data.cache_hash(state);
 	}
-	impl FakeHash for (f64, Color) {
</file context>