Brush cache

[timon-schelling]

No description provided.

[cubic-dev-ai]

3 issues found across 3 files

Confidence score: 2/5

• node-graph/graph-craft/src/document/value.rs can convert defaulted Item<BrushCache> inputs into TaggedValue::None, causing brush-cache inputs to be missing at runtime; match BrushCache in from_type.
• node-graph/libraries/brush-types/src/cache.rs may treat distinct brush caches as equal despite nonce-based identity, collapsing per-node cache entries; include the nonces in equality. The same file can also discard a cached value when take receives the wrong state type; restore the boxed value after a failed downcast.

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/graph-craft/src/document/value.rs">

<violation number="1" location="node-graph/graph-craft/src/document/value.rs:423">
P2: When `from_type` receives `item!(BrushCache)`, it compares the element name with `Item<BrushCache>` and returns `None`, so defaulted `Item<BrushCache>` inputs become `TaggedValue::None`. Match `BrushCache` here, as the surrounding `BrushTrace` handling does.</violation>
</file>

<file name="node-graph/libraries/brush-types/src/cache.rs">

<violation number="1" location="node-graph/libraries/brush-types/src/cache.rs:34">
P2: When a caller requests the wrong state type, `take` drops the cached value after `downcast().ok()` returns `None`. Preserve the boxed value on a failed downcast so a type mismatch cannot silently destroy the cache entry.</violation>

<violation number="2" location="node-graph/libraries/brush-types/src/cache.rs:43">
P2: Different brush caches compare equal here even though `cache_hash` deliberately distinguishes their per-node nonces, so derived `TaggedValue` equality can collapse distinct cache identities. Compare the nonces so equality reflects the identity used for deduplication.

(Based on your team's feedback about meaningful equality and consistent hashing.) .</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When from_type receives item!(BrushCache), it compares the element name with Item<BrushCache> and returns None, so defaulted Item<BrushCache> inputs become TaggedValue::None. Match BrushCache here, as the surrounding BrushTrace handling does.

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 423:

<comment>When `from_type` receives `item!(BrushCache)`, it compares the element name with `Item<BrushCache>` and returns `None`, so defaulted `Item<BrushCache>` inputs become `TaggedValue::None`. Match `BrushCache` here, as the surrounding `BrushTrace` handling does.</comment>

<file context>
@@ -413,6 +420,7 @@ macro_rules! tagged_value {
 						$( if name == std::any::type_name::<$ty>() { return Some(TaggedValue::$identifier(Default::default())) } )*
 						if name == std::any::type_name::<BrushTrace>() { return Some(TaggedValue::BrushStrokes(Vec::new())) }
 						if name == std::any::type_name::<List<Stroke>>() { return Some(TaggedValue::Strokes(Vec::new())) }
+					if name == std::any::type_name::<Item<BrushCache>>() { return Some(TaggedValue::BrushCache(Default::default())) }
 						// Unranked types without a variant route through `TypeDefault`, with `to_dynany`/`to_any` constructing the actual default at execution time
 						macro_rules! check_bare {
</file context>

Suggested change

	if name == std::any::type_name::<Item<BrushCache>>() { return Some(TaggedValue::BrushCache(Default::default())) }
	if name == std::any::type_name::<BrushCache>() { return Some(TaggedValue::BrushCache(Default::default())) }

[cubic-dev-ai]

P2: Different brush caches compare equal here even though cache_hash deliberately distinguishes their per-node nonces, so derived TaggedValue equality can collapse distinct cache identities. Compare the nonces so equality reflects the identity used for deduplication.

(Based on your team's feedback about meaningful equality and consistent hashing.) .

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/cache.rs, line 43:

<comment>Different brush caches compare equal here even though `cache_hash` deliberately distinguishes their per-node nonces, so derived `TaggedValue` equality can collapse distinct cache identities. Compare the nonces so equality reflects the identity used for deduplication.

(Based on your team's feedback about meaningful equality and consistent hashing.) .</comment>

<file context>
@@ -0,0 +1,243 @@
+}
+
+impl PartialEq for BrushCache {
+	fn eq(&self, _: &Self) -> bool {
+		true
+	}
</file context>

[cubic-dev-ai]

P2: When a caller requests the wrong state type, take drops the cached value after downcast().ok() returns None. Preserve the boxed value on a failed downcast so a type mismatch cannot silently destroy the cache entry.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/cache.rs, line 34:

<comment>When a caller requests the wrong state type, `take` drops the cached value after `downcast().ok()` returns `None`. Preserve the boxed value on a failed downcast so a type mismatch cannot silently destroy the cache entry.</comment>

<file context>
@@ -0,0 +1,243 @@
+impl BrushCache {
+	pub fn take<S: std::any::Any + Send + Sync>(&self, footprint: &Footprint) -> Option<S> {
+		let state = self.state.lock().unwrap().take(footprint);
+		state.and_then(|state| state.downcast().ok()).map(|state| *state)
+	}
+
</file context>